From patchwork Thu Feb 9 08:27:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 9564175 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8C90B601C3 for ; Thu, 9 Feb 2017 08:30:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 47D5C26E94 for ; Thu, 9 Feb 2017 08:30:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3C54C2850F; Thu, 9 Feb 2017 08:30:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 67C6C26E94 for ; Thu, 9 Feb 2017 08:30:27 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cbk4w-00083J-2J; Thu, 09 Feb 2017 08:27:46 +0000 Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cbk4v-00083D-7e for xen-devel@lists.xen.org; Thu, 09 Feb 2017 08:27:45 +0000 Received: from [193.109.254.147] by server-8.bemta-6.messagelabs.com id 73/21-21675-0082C985; Thu, 09 Feb 2017 08:27:44 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrOIsWRWlGSWpSXmKPExsVywNykWPe/+pw Ig9e9ZhZLPi5mcWD0OLr7N1MAYxRrZl5SfkUCa8aLC3sYC2aaVizoW83SwPhSp4uRk0NIYBqj xMfXTiC2hACvxJFlM1ghbH+J6z8vMHYxcgHVdDBKXP6yhR0kwSKgIvFp2RywIjYBZYmLX3vZQ GwRILv3128WkAZmgcdMEvOvvgBLCAt4S0w+sJoRxOYVsJI4M+8v2FRegcnMEisffYBacZ1JYs bJ1SwQVYISJ2c+AbOZBdQl/sy7xNzFyAFkS0ss/8cBEZaXaN46mxnE5hSwl5jZMg3sIlGg66a c3MY2gVFoFpJJs5BMmoUwaRaSSQsYWVYxahSnFpWlFukamuslFWWmZ5TkJmbm6BoamOnlphYX J6an5iQmFesl5+duYgQGOgMQ7GC8vTHgEKMkB5OSKG/E/9kRQnxJ+SmVGYnFGfFFpTmpxYcYZ Tg4lCR4mYGRIyRYlJqeWpGWmQOMOZi0BAePkggvA0iat7ggMbc4Mx0idYpRl+PUjdMvmYRY8v LzUqXEee1BigRAijJK8+BGwOL/EqOslDAvI9BRQjwFqUW5mSWo8q8YxTkYlYR5eUGm8GTmlcB tegV0BBPQEddPzwI5oiQRISXVwGjfbucz9cAF+S9fW4NzA6J+58w1jzrmHJZvUfWJ/dDjpcsm /i7JCnw6vekKu/L9vb9XfH7rk1wws2H3lWbf333cojKi7/7oBJ9Pe7D24afDPRsNe+6c6l7Kx H743MQ/jVNW9zk97v4r7fNPccqOZoFfzot7/756mndWYWe9rp3MTeMj+U975eOUWIozEg21mI uKEwEFKmAK+gIAAA== X-Env-Sender: chao.gao@intel.com X-Msg-Ref: server-11.tower-27.messagelabs.com!1486628861!55680374!1 X-Originating-IP: [192.55.52.115] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 9.1.1; banners=-,-,- X-VirusChecked: Checked Received: (qmail 19453 invoked from network); 9 Feb 2017 08:27:43 -0000 Received: from mga14.intel.com (HELO mga14.intel.com) (192.55.52.115) by server-11.tower-27.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 9 Feb 2017 08:27:43 -0000 Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 Feb 2017 00:27:41 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,349,1484035200"; d="scan'208";a="931848470" Received: from unknown (HELO localhost.localdomain) ([10.239.159.35]) by orsmga003.jf.intel.com with ESMTP; 09 Feb 2017 00:27:38 -0800 Date: Thu, 9 Feb 2017 16:27:38 +0800 From: Chao Gao To: Jan Beulich Message-ID: <20170209082738.GA15352@localhost.localdomain> Mail-Followup-To: Jan Beulich , Kevin Tian , Stefano Stabellini , George Dunlap , Andrew Cooper , Tim Deegan , Xen-devel , Julien Grall , Jun Nakajima , David Woodhouse References: <1486389313-8326-1-git-send-email-andrew.cooper3@citrix.com> <589895B70200007800136F0F@prv-mh.provo.novell.com> <1486392536.5110.80.camel@infradead.org> <5898A67C0200007800136FA0@prv-mh.provo.novell.com> <589B5DF20200007800137E45@prv-mh.provo.novell.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <589B5DF20200007800137E45@prv-mh.provo.novell.com> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Kevin Tian , Stefano Stabellini , George Dunlap , Andrew Cooper , Tim Deegan , Xen-devel , Julien Grall , Jun Nakajima , David Woodhouse Subject: Re: [Xen-devel] [PATCH] xen/mm: Alter is_iomem_page() to use mfn_t X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Feb 08, 2017 at 10:05:38AM -0700, Jan Beulich wrote: >>>> On 08.02.17 at 08:31, wrote: >>> From: Jan Beulich [mailto:JBeulich@suse.com] >>> Sent: Monday, February 06, 2017 11:38 PM >>> >>> >>> On 06.02.17 at 15:48, wrote: >>> > On Mon, 2017-02-06 at 07:26 -0700, Jan Beulich wrote: >>> >> > >>> >> > > >>> >> > > > >>> >> > > > On 06.02.17 at 14:55, wrote: >>> >> > Switch its return type to bool to match its use, and simplify the >>> >> > ARM >>> >> > implementation slightly. >>> >> > >>> >> > No functional change. >>> >> > >>> >> > Signed-off-by: Andrew Cooper >>> >> Reviewed-by: Jan Beulich >>> >> >>> >> And perhaps that's what should be used in epte_get_entry_emt() >>> >> instead of !mfn_valid() in David's patch. David, would you be okay >>> >> with your patch changed to that effect upon commit? >>> > >>> > I don't think that works, at least not literally >>> > s/!mfn_valid()/is_iomem_page()/ >>> > >>> > In my patch, mfn_valid() is checked *after* we've processed the >>> > 'direct_mmio' case that all MMIO should hit. In a sane world I think >>> > it's *only* actually catching INVALID_MFN, and probably should never >>> > match on any other value of mfn. >>> > >>> > I don't quite understand why we pass 'direct_mmio' in as a separate >>> > argument. Perhaps there's scope for doing a sanity check that >>> > 'direct_mmio == is_iomem_page(mfn)' — because when would that *not* be >>> > true? >>> >>> Likely never. The reasons for why this was done the way it is >>> escape me. Adding VMX maintainers once again. >> >> I'm not the original author of that code. Here is what I found from the >> code: >> >> There are two places caring direct_mmio: resolve_misconfig and ept_ >> set_entry. It's decided upon p2m type: >> >> int emt = epte_get_entry_emt(p2m->domain, gfn, _mfn(e.mfn), >> level * EPT_TABLE_ORDER, &ipat, >> e.sa_p2mt == p2m_mmio_direct); >> >> I'm not sure whether p2m_mmio_direct always makes is_iomem_page >> true. If true then yes we may not require this parameter at all. > >From an abstract perspective the two should always correlate. We >may want to take an intermediate step though and first only add >checking, and perhaps a release later remove the extra parameter >if the check never triggered for anyone. > I add one assertion to the original code, like below. But when I try to create a hvm domain, it failed. logs are below. (XEN) Assertion 'direct_mmio == is_iomem_page(mfn)' failed at mtrr.c:787 (XEN) ----[ Xen-4.9-unstable x86_64 debug=y Not tainted ]---- (XEN) CPU: 44 (XEN) RIP: e008:[] epte_get_entry_emt+0xbc/0x24e [EPTE_EMT_v2] (XEN) RFLAGS: 0000000000010297 CONTEXT: hypervisor (d0v39) (XEN) rax: ffff832077d7e000 rbx: 00000000010107ba rcx: ffff830000000000 (XEN) rdx: 0000000002077d7e rsi: 00000000010107ba rdi: 00000000010107ba (XEN) rbp: ffff832076857a68 rsp: ffff832076857a18 r8: ffff832076857af7 (XEN) r9: 0000000000000001 r10: 0000000000000000 r11: 0000000000000000 (XEN) r12: ffff832077d7e000 r13: 0000000000000000 r14: 0000000000000000 (XEN) r15: ffff832076857af7 cr0: 0000000080050033 cr4: 00000000001526e0 (XEN) cr3: 0000000899980000 cr2: 00007f35eb86d8d1 (XEN) ds: 0000 es: 0000 fs: 0000 gs: 0000 ss: e010 cs: e008 (XEN) Xen code around (epte_get_entry_emt+0xbc/0x24e [EPTE_EMT_v2]): (XEN) 37 4d d7 ff 3c 01 74 02 <0f> 0b 49 33 9c 24 40 06 00 00 44 89 e9 48 d3 eb (XEN) Xen stack trace from rsp=ffff832076857a18: (XEN) ffff832076857b00 00000000000fee00 00000000010107ba 0000000100000000 (XEN) ffff832076857a68 ffff832077da8b90 0000000000000000 ffff832076857b00 (XEN) ffff8200403c8000 0000000000000000 ffff832076857b38 ffff82d0802101b5 (XEN) ffff832000000005 0000000000000206 0000000000000000 ffff832076857aa8 (XEN) ffff82d08013452b ffff831000000000 0000000000000001 0000000700000206 (XEN) ffff832077d7e000 00000000000fee00 00000000010107ba 0000000000000005 (XEN) ffff832077da8b90 0000000000000000 8000000000000000 00ff832077da8b90 (XEN) 0000000000000000 ffff8200403c8000 0000000000000001 00000000010107ba (XEN) 0000000000000000 0000000000000001 00000000000fee00 ffff832077da8b90 (XEN) ffff832076857b98 ffff82d080204d62 00000000ffffffff 0000000776857b68 (XEN) ffff832077d7e000 0000000000000005 ffff832076857b98 ffff832077d7e000 (XEN) 00000000010107ba ffff832077da8b90 ffffffffffffffff 0000000000000007 (XEN) ffff832076857c18 ffff82d080205408 0000000000000000 0000002800000021 (XEN) 0000000776857bc8 00000000010107ba 00000000000fee00 0000000000000005 (XEN) 000000128013452b 0000000000000004 ffff832076857c38 0000000000000000 (XEN) 00000000010107ba ffff832077d7e000 00000000000fee00 0000000000000007 (XEN) ffff832076857c58 ffff82d080206fa5 ffff832076857c58 ffff832077d7e000 (XEN) ffff82e02020f740 00000000010107ba 0000000000000001 0000000000000000 (XEN) ffff832076857c88 ffff82d0801f8ff9 ffff832077d7e000 ffff832077d7e000 (XEN) 0000000000000000 0000000000000001 ffff832076857ca8 ffff82d0801d2a28 (XEN) Xen call trace: (XEN) [] epte_get_entry_emt+0xbc/0x24e [EPTE_EMT_v2] (XEN) [] p2m-ept.c#ept_set_entry+0x2f3/0x798 (XEN) [] p2m_set_entry+0xc9/0x10a (XEN) [] p2m.c#set_typed_p2m_entry+0x447/0x65f (XEN) [] set_mmio_p2m_entry+0x63/0x72 (XEN) [] vmx.c#vmx_domain_initialise+0xb6/0xcd (XEN) [] hvm_domain_initialise+0x2da/0x368 (XEN) [] arch_domain_create+0x4b0/0x68e (XEN) [] domain_create+0x3ee/0x5a7 (XEN) [] do_domctl+0x52e/0x1bae (XEN) [] pv_hypercall+0x1e7/0x3fb (XEN) [] entry.o#test_all_events+0/0x30 (XEN) (XEN) (XEN) **************************************** (XEN) Panic on CPU 44: (XEN) Assertion 'direct_mmio == is_iomem_page(mfn)' failed at mtrr.c:787 (XEN) **************************************** Thanks Chao >Jan > >_______________________________________________ >Xen-devel mailing list >Xen-devel@lists.xen.org >https://lists.xen.org/xen-devel diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c index 86c71be..3d9386a 100644 --- a/xen/arch/x86/hvm/mtrr.c +++ b/xen/arch/x86/hvm/mtrr.c @@ -784,6 +784,8 @@ int epte_get_entry_emt(struct domain *d, unsigned long gfn, if ( direct_mmio ) { + ASSERT(direct_mmio == is_iomem_page(mfn)); + if ( (mfn_x(mfn) ^ d->arch.hvm_domain.vmx.apic_access_mfn) >> order ) return MTRR_TYPE_UNCACHABLE; if ( order )