On Sat, 1 Oct 2022 at 17:49, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> Leave the upper and lower attributes in the place they originate
> from in the descriptor. Shifting them around is confusing, since
> one cannot read the bit numbers out of the manual. Also, new
> attributes have been added which would alter the shifts.
>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> target/arm/ptw.c | 31 +++++++++++++++----------------
> 1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 01a27b30fb..c68fd73617 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -1071,7 +1071,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> uint64_t address,
> hwaddr descaddr, indexmask, indexmask_grainsize;
> uint32_t tableattrs;
> target_ulong page_size;
> - uint32_t attrs;
> + uint64_t attrs;
> int32_t stride;
> int addrsize, inputsize, outputsize;
> uint64_t tcr = regime_tcr(env, mmu_idx);
> @@ -1341,49 +1341,48 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> uint64_t address,
> descaddr &= ~(page_size - 1);
> descaddr |= (address & (page_size - 1));
> /* Extract attributes from the descriptor */
> - attrs = extract64(descriptor, 2, 10)
> - | (extract64(descriptor, 52, 12) << 10);
> + attrs = descriptor & (MAKE_64BIT_MASK(2, 10) | MAKE_64BIT_MASK(52, 12));
So bit positions for the lower-part bits need to have 2 added,
and bit positions for upper-part bits need to have 42 added.
> if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
> ns = mmu_idx == ARMMMUIdx_Stage2;
> - xn = extract32(attrs, 11, 2);
> + xn = extract64(attrs, 54, 2);
...so this one looks wrong, should be 53 ?
> result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
> } else {
> - ns = extract32(attrs, 3, 1);
> - xn = extract32(attrs, 12, 1);
> - pxn = extract32(attrs, 11, 1);
> + ns = extract32(attrs, 5, 1);
> + xn = extract64(attrs, 54, 1);
> + pxn = extract64(attrs, 53, 1);
Compare here where bit 11 in the old code is 53 in the new.
> result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn);
> }
>
Otherwise
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
thanks
-- PMM
