[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-arm] [PATCH] target-arm: Fix descriptor address m

From: Tom Hanson
Subject: Re: [Qemu-devel] [Qemu-arm] [PATCH] target-arm: Fix descriptor address masking in ARM address translation
Date: Tue, 26 Apr 2016 10:35:50 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0

On 03/21/2016 09:56 AM, Sergey Sorokin wrote:
17.03.2016, 18:24, "Peter Maydell" <address@hidden>:
  On 17 March 2016 at 15:21, Sergey Sorokin <address@hidden> wrote:
   17.03.2016, 14:40, "Peter Maydell" <address@hidden>:
   On 13 March 2016 at 18:28, Sergey Sorokin <address@hidden> 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 
    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.

  -- PMM

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

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 
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,
    return result;

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]