Re: [Qemu-arm] [PATCH] target-arm: Fix descriptor address masking in ARM address translation

[Peter Maydell]

On 13 March 2016 at 18:28, Sergey Sorokin <address@hidden> wrote:
> 12.03.2016, 03:18, "Peter Maydell" <address@hidden>:
>>(I agree we have a bug here, I'm just trying to work out when it
>>can trigger; if it's only possible for S2 page tables then it's
>>not a visible bug yet because no CPUs have EL2 support enabled.)
>
> I can not anwer you to this question. The bug was found by our internal
> corporate test suite with EL2 enabled.

OK, sounds like it is the stage-2 only stuff. Thanks for
helping to flush out the bugs.

>>If you want to implement the AddressSize checks that's fine,
>>but otherwise please leave this bit of the code alone.
>
> You said me that my code is not correct, I have proved that it conforms
> to the documentation.
> It's a bit obfuscating when the doc explicitly says to take bits up to 39
> from the descriptor, but in QEMU we take bits up to 47 relying on the check in
> another part of the code, even if both ways are correct.

The way the code in QEMU is structured is that we extract the
descriptor field in one go and then will operate on it
(checking for need to AddressSize fault, etc) as a second
action. The field descriptors themselves are the sizes I said.

> Nevertheless there is another bug in descaddrmask in QEMU.
> From ARM ARM:
> baseaddress = desc<39:grainsize>:Zeros(grainsize);
>
> But currently QEMU does:
> descaddrmask = 0xfffffffff000ULL;
>
> It assumes that grainsize is always 12, but it can be greater
> in AArch64 translation regime.

Yes, we get that wrong at the moment and should fix it.

> The patch fixes the bug,
> and completely conforms to the doc, doesn't it?

It doesn't structure the code the way I would prefer it to
be structured though.

thanks
-- PMM