| Message ID | 79db1022-aa46-45cb-8fb6-b9e26af1678f@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <asm/guest_walk.h> > + > 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); > + 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); > + } > + > switch ( fsc ) > { > case FSC_FLT_PERM: > >> >> Cheers, >> ~Sergej >> >> [0] https://github.com/sergej-proskurin/xen (branch arm-gpt-walk-v7) >> >> Sergej Proskurin (14): >> arm/mem_access: Add and cleanup (TCR_|TTBCR_)* defines >> arm/mem_access: Move PAGE_*_* macros to xen/page-defs.h >> arm/mem_access: Add defines supporting PTs with varying page sizes >> arm/lpae: Introduce lpae_is_page helper >> arm/mem_access: Add short-descriptor pte typedefs and macros >> arm/mem_access: Introduce GV2M_EXEC permission >> arm/mem_access: Introduce BIT_ULL bit operation >> arm/mem_access: Introduce GENMASK_ULL bit operation >> arm/guest_access: Move vgic_access_guest_memory to guest_access.h >> arm/guest_access: Rename vgic_access_guest_memory >> arm/mem_access: Add software guest-page-table walk >> arm/mem_access: Add long-descriptor based gpt >> arm/mem_access: Add short-descriptor based gpt >> arm/mem_access: Walk the guest's pt in software >> >> xen/arch/arm/Makefile | 1 + >> xen/arch/arm/guest_walk.c | 631 +++++++++++++++++++++++++++++++++++++ >> xen/arch/arm/guestcopy.c | 50 +++ >> xen/arch/arm/mem_access.c | 31 +- >> xen/arch/arm/vgic-v3-its.c | 37 +-- >> xen/arch/arm/vgic.c | 49 --- >> xen/include/asm-arm/bitops.h | 1 + >> xen/include/asm-arm/config.h | 2 + >> xen/include/asm-arm/guest_access.h | 3 + >> xen/include/asm-arm/guest_walk.h | 19 ++ >> xen/include/asm-arm/lpae.h | 66 ++++ >> xen/include/asm-arm/page.h | 1 + >> xen/include/asm-arm/processor.h | 69 +++- >> xen/include/asm-arm/short-desc.h | 130 ++++++++ >> xen/include/asm-arm/vgic.h | 3 - >> xen/include/asm-x86/config.h | 2 + >> xen/include/xen/bitops.h | 3 + >> xen/include/xen/iommu.h | 15 +- >> xen/include/xen/page-defs.h | 24 ++ >> 19 files changed, 1048 insertions(+), 89 deletions(-) >> create mode 100644 xen/arch/arm/guest_walk.c >> create mode 100644 xen/include/asm-arm/guest_walk.h >> create mode 100644 xen/include/asm-arm/short-desc.h >> create mode 100644 xen/include/xen/page-defs.h >> Cheers, ~Sergej
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 <asm/guest_walk.h> + 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); + 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); + } + switch ( fsc ) { case FSC_FLT_PERM: