On 03/21/2016 09:56 AM, Sergey Sorokin wrote:

š17.03.2016, 18:24, "Peter Maydell" <peter.maydell@linaro.org>:

šššOn 17 March 2016 at 15:21, Sergey Sorokin <afarallax@yandex.ru> wrote:

šššš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.
šššššššššš*/

šššThe comment is correct as it stands.

šššthanks
ššš-- PMM

šI mean in the patch.
šWe need to fix lower bits in descaddrmask anyway.
šSo:

šI could describe in the comment, that the descriptor field is up to bit 47 for ARMv8 (as long as you want it),
š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.
šIt is clearly described in the ARM pseudo-code.
šWhy should we keep in the mask bits from 40 up to 47 if we don't need them? Even if they are all zeroes.
šIt is a bit obfuscating, as I said.

ššI agree with Peter. The original comment is correct.

Looking at the TLBRecord AArch32.TranslationTableWalkLD pseudocode, it is treating the AArch32 address as 48 bits long. For example:
šššššif !IsZero(baseregister<47:40>) then
šššššššššlevel = 0;
šššššššššresult.addrdesc.fault = AArch32.AddressSizeFault(ipaddress, domain, level, acctype, iswrite,
ššššššššššššššššššššššššššššššššššššššššššššššššššššššššššsecondstage, s2fs1walk);
šššššreturn result;

This requires that an AArch32 address have specific values up through bit 47.