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

[Sergey Sorokin <afarallax@yandex.ru>]

17.03.2016, 14:40, "Peter Maydell" <peter.maydell@linaro.org>:
> On 13 March 2016 at 18:28, Sergey Sorokin <afarallax@yandex.ru> wrote:
>>> 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.

Well, may be it's enough just to change this comment as you intend:

>> -    /* The address field in the descriptor goes up to bit 39 for ARMv7
>> -     * but up to bit 47 for ARMv8.
>> +    /* The address field in the descriptor goes up to bit 39 for AArch32
>> +     * but up to bit 47 for AArch64.
>>       */
>
>This is not correct -- the descriptor field widths are as the comment
>states before your patch:
> * up to bit 39 for ARMv7
> * up to bit 47 for ARMv8 (whether AArch32 or AArch64)

I could describe there, that the descriptor field is up to bit 47 for ARMv8,
but we use the descaddrmask up to bit 39 for AArch32,
because we don't need other bits in that case to construct next descriptor address,
as it is described in the doc.