Re: [Qemu-devel] [PATCH v3 3/3] target-arm: get_phys_addr_lpae: more xn control

[Peter Maydell]

On 11 March 2015 at 18:06, Andrew Jones <address@hidden> wrote:
> +    if (is_aa64) {
> +        switch (regime_el(env, mmu_idx)) {
> +        case 1:
> +            if (is_user && !(user_rw & PAGE_READ)) {
> +                wxn = 0;

I still can't figure out the point of this.
This conditional will only be true if this is a
user access and user_rw is zero (indicating a page which
is neither readable nor writeable)...

> +            } else if (!is_user) {
> +                xn = pxn || (user_rw & PAGE_WRITE);
> +            }
> +            break;
> +        case 2:
> +        case 3:
> +            break;
> +        }
> +    } else if (arm_feature(env, ARM_FEATURE_V7)) {
> +        switch (regime_el(env, mmu_idx)) {
> +        case 1:
> +        case 3:
> +            if (is_user) {
> +                xn = xn || !(user_rw & PAGE_READ);
> +            } else {
> +                int uwxn = 0;
> +                if (have_wxn) {
> +                    uwxn = regime_sctlr(env, mmu_idx) & SCTLR_UWXN;
> +                }
> +                xn = xn || !(prot_rw & PAGE_READ) || pxn ||
> +                     (uwxn && (user_rw & PAGE_WRITE));
> +            }
> +            break;
> +        case 2:
> +            break;
> +        }
> +    } else {
> +        xn = wxn = 0;
> +    }

...and in that code path the only place wxn is used
later is in this check:

> +    if (xn || (wxn && (prot_rw & PAGE_WRITE))) {

...but in that case, we know that user_rw == prot_rw,
and (prot_rw & PAGE_WRITE) must be zero. So the
"(wxn && (prot_rw & PAGE_WRITE))" part of the condition
must be false, regardless of whether wxn is true or
false.

So why did we bother changing wxn in the code above?

> +        return prot_rw;
> +    }
> +    return prot_rw | PAGE_EXEC;
> +}

I don't see what I'm missing...

thanks
-- PMM