Re: [Qemu-devel] [PATCH] target-arm: A64: Correct handling of UXN bit.

[Claudio Fontana]

This patch fixes my issue with page tables switching on OSv guest.

Thank you all!

Tested-by: Claudio Fontana <address@hidden>

On 08.06.2014 15:53, Ian Campbell wrote:
> In v8 page tables bit 54 in the PTE is UXN in the EL0/EL1 translation regimes
> and XN elsewhere. In v7 the bit is always XN. Since we only emulate EL0/EL1 we
> can just treat this bit as UXN whenever we are in v8 mode.
> 
> Also correctly extract the upper attributes from the PTE entry, the v8 version
> tried to avoid extracting the CONTIG bit and ended up with the upper bits 
> being
> off-by-one. Instead behave the same as v7 and extract (but ignore) the CONTIG
> bit.
> 
> This fixes "Bad mode in Synchronous Abort handler detected, code 0x8400000f"
> seen when modprobing modules under Linux.
> 
> Signed-off-by: Ian Campbell <address@hidden>
> Cc: Peter Maydell <address@hidden>
> Cc: Claudio Fontana <address@hidden>
> Cc: Rob Herring <address@hidden>
> ---
>  target-arm/helper.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index ec031f5..5872a00 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3929,13 +3929,8 @@ static int get_phys_addr_lpae(CPUARMState *env, 
> target_ulong address,
>          page_size = (1 << ((granule_sz * (4 - level)) + 3));
>          descaddr |= (address & (page_size - 1));
>          /* Extract attributes from the descriptor and merge with table attrs 
> */
> -        if (arm_feature(env, ARM_FEATURE_V8)) {
> -            attrs = extract64(descriptor, 2, 10)
> -                | (extract64(descriptor, 53, 11) << 10);
> -        } else {
> -            attrs = extract64(descriptor, 2, 10)
> -                | (extract64(descriptor, 52, 12) << 10);
> -        }
> +        attrs = extract64(descriptor, 2, 10)
> +            | (extract64(descriptor, 52, 12) << 10);
>          attrs |= extract32(tableattrs, 0, 2) << 11; /* XN, PXN */
>          attrs |= extract32(tableattrs, 3, 1) << 5; /* APTable[1] => AP[2] */
>          /* The sense of AP[1] vs APTable[0] is reversed, as APTable[0] == 1
> @@ -3961,8 +3956,12 @@ static int get_phys_addr_lpae(CPUARMState *env, 
> target_ulong address,
>          goto do_fault;
>      }
>      *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> -    if (attrs & (1 << 12) || (!is_user && (attrs & (1 << 11)))) {
> -        /* XN or PXN */
> +    if ((arm_feature(env, ARM_FEATURE_V8) && is_user && (attrs & (1 << 12))) 
> ||
> +        (!arm_feature(env, ARM_FEATURE_V8) && (attrs & (1 << 12))) ||
> +        (!is_user && (attrs & (1 << 11)))) {
> +        /* XN/UXN or PXN. Since we only implement EL0/EL1 we unconditionally
> +         * treat XN/UXN as UXN for v8.
> +         */
>          if (access_type == 2) {
>              goto do_fault;
>          }
> 


-- 
Claudio Fontana
Server Virtualization Architect
Huawei Technologies Duesseldorf GmbH
Riesstraße 25 - 80992 München

office: +49 89 158834 4135
mobile: +49 15253060158