From patchwork Tue Aug 8 12:17:45 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergej Proskurin X-Patchwork-Id: 9887973 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 5701D601EB for ; Tue, 8 Aug 2017 12:20:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 480A32885F for ; Tue, 8 Aug 2017 12:20:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3CAF428861; Tue, 8 Aug 2017 12:20:56 +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 17D972885F for ; Tue, 8 Aug 2017 12:20:54 +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 1df3SN-0004D0-Kg; Tue, 08 Aug 2017 12:17:55 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1df3SM-0004CC-38 for xen-devel@lists.xenproject.org; Tue, 08 Aug 2017 12:17:54 +0000 Received: from [85.158.139.211] by server-1.bemta-5.messagelabs.com id E4/66-01993-1FBA9895; Tue, 08 Aug 2017 12:17:53 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrNIsWRWlGSWpSXmKPExsXSPJ+BQ/fD6s5 Ig427FSy+b5nM5MDocfjDFZYAxijWzLyk/IoE1ozjn+4wFrx1q5ix6xt7A+NLyy5GLg4hgXWM Eh9f7GPqYuTkEBbIl/h+8jSYzSZgIDHl9UpWEFtEoFTif9tmZoiGRYwS81deYwRJ8Ao4SLy8c JkFxGYRUJGYd2YVWFxUIFxi//drzBA1ghInZz4BquHg4BRwlLh/MhAkzCygLvFn3iVmCFtbYt nC11C2uMStJ/OZJjDyzkLSPQtJyywkLbOQtCxgZFnFqF6cWlSWWqRrrpdUlJmeUZKbmJmja2h gqpebWlycmJ6ak5hUrJecn7uJERhsDECwg/HYZOdDjJIcTEqivJu0OyOF+JLyUyozEosz4otK c1KLDzFqcHAIXDl4ZDajFEtefl6qkgRvyiqgOsGi1PTUirTMHGA8wJRKcPAoifB2gaR5iwsSc 4sz0yFSpxiNOe70bfjCxNH0/eN3JiGwSVLivPdWApUKgJRmlObBDYLF6SVGWSlhXkagM4V4Cl KLcjNLUOVfMYpzMCoJ8woAo16IJzOvBG7fK6BTmIBOifAFO6UkESEl1cCoILFn0uw1LN3R07R 0H3xY+Ghm2sWChKwkK8fTPcebTe5LXjpxaLlyeLcA91O+c+du/BfdH+HWtOpQv7R64Nnrx5zF OvJXn7AzU5W5K7NqU53jQtbZzmGf+Ge/2OVskL5FPpp5ncqdb9uP3ApfwN3mECLm+nrdNb220 2K1nWvY/a+v+dO/q0pIiaU4I9FQi7moOBEAB/eAJ84CAAA= X-Env-Sender: proskurin@sec.in.tum.de X-Msg-Ref: server-15.tower-206.messagelabs.com!1502194672!92330765!1 X-Originating-IP: [131.159.0.8] X-SpamReason: No, hits=0.5 required=7.0 tests=BODY_RANDOM_LONG X-StarScan-Received: X-StarScan-Version: 9.4.45; banners=-,-,- X-VirusChecked: Checked Received: (qmail 32638 invoked from network); 8 Aug 2017 12:17:52 -0000 Received: from mail-out1.informatik.tu-muenchen.de (HELO mail-out1.informatik.tu-muenchen.de) (131.159.0.8) by server-15.tower-206.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 8 Aug 2017 12:17:52 -0000 Received: from [131.159.50.36] (ker.sec.in.tum.de [131.159.50.36]) by services.sec.in.tum.de (Postfix) with ESMTPSA id CC4C21096D908; Tue, 8 Aug 2017 14:17:45 +0200 (CEST) From: Sergej Proskurin To: Julien Grall , xen-devel@lists.xenproject.org, Stefano Stabellini References: <20170718122507.11873-1-proskurin@sec.in.tum.de> <79db1022-aa46-45cb-8fb6-b9e26af1678f@arm.com> Message-ID: <1175c88c-876e-fe5c-ed5f-c8f53b2703f3@sec.in.tum.de> Date: Tue, 8 Aug 2017 14:17:45 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US Subject: Re: [Xen-devel] [PATCH v7 00/14] arm/mem_access: Walk guest page tables in SW if mem_access is active 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 Hi Julien, On 08/04/2017 11:15 AM, Sergej Proskurin wrote: > Hi Julien, > > Sorry for the late reply. > > On 07/31/2017 04:38 PM, Julien Grall wrote: >> >> On 18/07/17 13:24, Sergej Proskurin wrote: >>> Hi all, >> Hi, >> >>> The function p2m_mem_access_check_and_get_page is called from the function >>> get_page_from_gva if mem_access is active and the hardware-aided translation of >>> the given guest virtual address (gva) into machine address fails. That is, if >>> the stage-2 translation tables constrain access to the guests's page tables, >>> hardware-assisted translation will fail. The idea of the function >>> p2m_mem_access_check_and_get_page is thus to translate the given gva and check >>> the requested access rights in software. However, as the current implementation >>> of p2m_mem_access_check_and_get_page makes use of the hardware-aided gva to ipa >>> translation, the translation might also fail because of reasons stated above >>> and will become equally relevant for the altp2m implementation on ARM. As >>> such, we provide a software guest translation table walk to address the above >>> mentioned issue. >>> >>> The current version of the implementation supports translation of both the >>> short-descriptor as well as the long-descriptor translation table format on >>> ARMv7 and ARMv8 (AArch32/AArch64). >>> >>> This revised version incorporates the comments of the previous patch series. In >>> this patch version we refine the definition of PAGE_SIZE_GRAN and >>> PAGE_MASK_GRAN. In particular, we use PAGE_SIZE_GRAN to define PAGE_MASK_GRAN >>> and thus avoid these defines to have a differing type. We also changed the >>> previously introduced macro BITS_PER_LONG_LONG to BITS_PER_LLONG. Further >>> changes comprise minor adjustments in comments and renaming of macros and >>> function parameters. Some additional changes comprising code readability and >>> correct type usage have been made and stated in the individual commits. >>> >>> The following patch series can be found on Github[0]. >> I tried this series today with the change [1] in Xen to check the translation >> is valid. However, I got a failure when booting non-LPAE arm32 Dom0: >> > That's odd.. Thanks for the information. I will investigate this issue > next week, as soon as I have access to our ARMv7 board. > >> (XEN) Loading kernel from boot module @ 0000000080008000 >> (XEN) Allocating 1:1 mappings totalling 512MB for dom0: >> (XEN) BANK[0] 0x000000a0000000-0x000000c0000000 (512MB) >> (XEN) Grant table range: 0x000000ffe00000-0x000000ffe6a000 >> (XEN) Loading zImage from 0000000080008000 to 00000000a7800000-00000000a7f50e28 >> (XEN) Allocating PPI 16 for event channel interrupt >> (XEN) Loading dom0 DTB to 0x00000000a8000000-0x00000000a8001f8e >> (XEN) Std. Loglevel: All >> (XEN) Guest Loglevel: All >> (XEN) guest_walk_tables: gva 0xffeff018 pipa 0x1c090018 >> (XEN) access_guest_memory_by_ipa: gpa 0xa0207ff8 >> (XEN) access_guest_memory_by_ipa: gpa 0xffffffffa13aebfc >> (XEN) d0: guestcopy: failed to get table entry. >> (XEN) Xen BUG at traps.c:2737 >> (XEN) ----[ Xen-4.10-unstable arm32 debug=y Not tainted ]---- >> (XEN) CPU: 0 >> (XEN) PC: 00264dc0 do_trap_guest_sync+0x161c/0x1804 >> (XEN) CPSR: a000005a MODE:Hypervisor >> (XEN) R0: ffffffea R1: 00000000 R2: 00000000 R3: 0000004a >> (XEN) R4: 93830007 R5: 47fcff58 R6: 93830007 R7: 00000007 >> (XEN) R8: 1c090000 R9: 00000000 R10:00000000 R11:47fcff54 R12:ffffffea >> (XEN) HYP: SP: 47fcfee4 LR: 00258dec >> (XEN) >> (XEN) VTCR_EL2: 80003558 >> (XEN) VTTBR_EL2: 00010008f3ffc000 >> (XEN) >> (XEN) SCTLR_EL2: 30cd187f >> (XEN) HCR_EL2: 000000000038663f >> (XEN) TTBR0_EL2: 00000000fff02000 >> (XEN) >> (XEN) ESR_EL2: 00000000 >> (XEN) HPFAR_EL2: 00000000001c0900 >> (XEN) HDFAR: ffeff018 >> (XEN) HIFAR: 00000000 >> (XEN) >> (XEN) Xen stack trace from sp=47fcfee4: >> (XEN) 00000000 47fcff34 00256008 47fcfefc 47fcfefc 200000da 00000004 47fd48f4 >> (XEN) 002d5ef0 00000004 002d1f00 00000004 00000000 002d1f00 c163f740 93830007 >> (XEN) ffeff018 1c090018 00000000 47fcff44 c15e70ac 0000005b c15e70ac c074400c >> (XEN) 00000031 00000000 c0743ff8 47fcff58 00268ce0 c15e70ac 0000005b 00000031 >> (XEN) ffeff000 c15e70ac 0000005b c15e70ac c074400c 00000031 00000000 c0743ff8 >> (XEN) 00000000 0000001f ffffffff 00000000 c074401c 200001d3 93830007 00000000 >> (XEN) c161cac0 c161cac0 c1501de0 c0735640 c161cacc c161cacc c161cad8 c161cad8 >> (XEN) 00000000 00000000 00000000 00000000 00000000 c161cae4 c161cae4 400001d3 >> (XEN) 00000000 00000000 00000000 00000000 00000000 dfdfdfcf cfdfdfdf >> (XEN) Xen call trace: >> (XEN) [<00264dc0>] do_trap_guest_sync+0x161c/0x1804 (PC) >> (XEN) [<00258dec>] access_guest_memory_by_ipa+0x25c/0x284 (LR) >> (XEN) [<00268ce0>] entry.o#return_from_trap+0/0x4 >> (XEN) >> (XEN) >> (XEN) **************************************** >> (XEN) Panic on CPU 0: >> (XEN) Xen BUG at traps.c:2737 >> (XEN) **************************************** >> (XEN) >> (XEN) Reboot in five seconds... >> >> The IPA 0xffffffffa13aebfc is not valid for the domain. >> >> Cheers, >> >> [1] >> diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c >> index 4ee07fcea3..89c5ebf3cf 100644 >> --- a/xen/arch/arm/guestcopy.c >> +++ b/xen/arch/arm/guestcopy.c >> @@ -139,6 +139,8 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t gpa, void *buf, >> return -EINVAL; >> } >> >> + printk("%s: gpa 0x%llx\n", __FUNCTION__, gpa); >> + >> page = get_page_from_gfn(d, paddr_to_pfn(gpa), &p2mt, P2M_ALLOC); >> if ( !page ) >> { >> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c >> index c07999b518..904abafcae 100644 >> --- a/xen/arch/arm/traps.c >> +++ b/xen/arch/arm/traps.c >> @@ -2688,6 +2688,8 @@ static bool try_map_mmio(gfn_t gfn) >> return !map_regions_p2mt(d, gfn, 1, mfn, p2m_mmio_direct_c); >> } >> >> +#include >> + >> static void do_trap_data_abort_guest(struct cpu_user_regs *regs, >> const union hsr hsr) >> { >> @@ -2725,6 +2727,17 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs, >> return; /* Try again */ >> } >> >> + { >> + paddr_t ipa, pipa; >> + rc = gva_to_ipa(info.gva, &info.ipa, GV2M_READ); There is no ipa field in mmio_info_t. But even if you used info.gpa instead, the test that you have provided is unfortunately flawed: >> + BUG_ON(rc); >> + printk("guest_walk_tables: gva 0x%x pipa 0x%llx\n", >> + info.gva, pipa); >> + rc = guest_walk_tables(current, info.gva, &ipa, NULL); >> + BUG_ON(rc); >> + BUG_ON(ipa != pipa); In your test-case you don't initialize pipa at all, however you test for it in BUG_ON, which is the reason why it fails. I have adopted your test case and it runs on ARMv7 (non-LPAE guest) and ARMv8 (LPAE guest) without any issues. It would be great if you would verify this behaviour by applying the following patch to the arm-gpt-walk-v7 patch [0] as before: From a28db6321780c442b1c97aa78883dccbd84de7dd Mon Sep 17 00:00:00 2001 From: Sergej Proskurin Date: Tue, 8 Aug 2017 13:30:00 +0200 Subject: [PATCH] Julien Grall's test case --- xen/arch/arm/guestcopy.c | 2 ++ xen/arch/arm/traps.c | 13 +++++++++++++ 2 files changed, 15 insertions(+) case FSC_FLT_PERM: -- 2.13.3 Thanks, ~Sergej [0] https://github.com/sergej-proskurin/xen (branch arm-gpt-walk-v7) diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c index 4ee07fcea3..f2758ebd45 100644 --- a/xen/arch/arm/guestcopy.c +++ b/xen/arch/arm/guestcopy.c @@ -139,6 +139,8 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t gpa, void *buf, return -EINVAL; } + printk("%s: gpa 0x%"PRIpaddr"\n", __FUNCTION__, gpa); + page = get_page_from_gfn(d, paddr_to_pfn(gpa), &p2mt, P2M_ALLOC); if ( !page ) { diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index c07999b518..9b0b79a3fe 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -2688,6 +2688,8 @@ static bool try_map_mmio(gfn_t gfn) return !map_regions_p2mt(d, gfn, 1, mfn, p2m_mmio_direct_c); } +#include + static void do_trap_data_abort_guest(struct cpu_user_regs *regs, const union hsr hsr) { @@ -2725,6 +2727,17 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs, return; /* Try again */ } + { + paddr_t ipa; + rc = gva_to_ipa(info.gva, &info.gpa, GV2M_READ); + BUG_ON(rc); + printk("guest_walk_tables: gva 0x%"PRIvaddr" pipa 0x%"PRIpaddr"\n", + info.gva, info.gpa); + rc = guest_walk_tables(current, info.gva, &ipa, NULL); + BUG_ON(rc); + BUG_ON(ipa != info.gpa); + } + switch ( fsc ) {