Re: [PATCH] makedumpfile/arm64: Add support for ARMv8.2-LPA (52-bit PA support)

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

 



Hi Kazu,

On 02/13/2019 02:45 AM, Kazuhito Hagio wrote:
On 2/12/2019 2:22 PM, Bhupesh Sharma wrote:
+#define ARM64_HW_PGTABLE_LEVELS(va_bits) (((va_bits) - 4) / (PAGE_SHIFT - 3))

I agree to sync these macros with the kernel, but currently this one
is not used in your patches, and you wrote the path of the source file
(pgtable-hwdef.h) above for reference, so we don't need to import this
one for now. I'd like to import it when it is needed.

Ok, now I understand. You mean 'ARM64_HW_PGTABLE_LEVELS' macro here only
and not 'ARM64_HW_PGTABLE_LEVEL_SHIFT' macro also, right? If yes, I
agree to removing the former. Will fix this in v2.

Yes, it's the ARM64_HW_PGTABLE_LEVELS macro only.

Ok.

My understanding is that with 64k page, we can convert a page table
entry to a physicall address without awareness of 52-bit.

According to this patch, the top 4 bits of a 52-bit physical address
are positioned at bits 12..15 of a page table entry.

commit 75387b92635e7dca410c1ef92cfe510019248f76
Author: Kristina Martsenko <kristina.martsenko@xxxxxxx>
Date:   Wed Dec 13 17:07:21 2017 +0000

      arm64: handle 52-bit physical addresses in page table entries

      The top 4 bits of a 52-bit physical address are positioned at bits
      12..15 of a page table entry. Introduce macros to convert between a
      physical address and its placement in a table entry, and change all
      macros/functions that access PTEs to use them.

With 64k page and non-52-bit kernel, it looks like the bits 12..15
are zero, so we can move the zeros to bits 49..51 because the zeros
don't affect the PA, for example:

        52-bit              non-52-bit (48-bit)
    PTE 0x0000123456789000  0x0000123456780000
             v--------^          v--------^   __pte_to_phys() w/52-bit support
    PA  0x0009123456780000  0x0000123456780000

I think that this was what the upstream maintainers said..
But "if (lpa_52_bit_support_available)" is also fine for me.

Well from my experience on arm32 and arm64 hardware enablement, assuming
values of implementation defined fields for arm hardware can be risky :)

Lets see what the ARMv8 architecture reference manual says about the
Bits [15:12] for a 64KB page size:

"Bits[15:12] of each valid translation table descriptor hold bits[51:48]
of the output address, or of the address of the translation table to be
used for the initial lookup at the next level of translation. If the
implementation does not support 52-bit physical addresses, then it is
IMPLEMENTATION DEFINED whether non-zero values for these bits generate
an Address size fault."

So, it is unsafe to assume that for a 48-bit physical address
implementation the Bits[15:12] cannot have non-zero values on certain
hardware implementations. So assuming that these bits are always 0 and
can be easily moved to Bits[51:48] for a 64K page and non-52bit kernel,
can lead to IMPLEMENTATION DEFINED behavior.

Its better instead to have a predictable behavior in such cases and by
explicitly calculating the paddress values using the right PTE High and
Low masks in these cases we can minimize any hardware IMPLEMENTATION
specific details (just like the handling done in kernel space).

I understood. This is the information I needed, thanks.

Ok.

@@ -425,14 +628,13 @@ vaddr_to_paddr_arm64(unsigned long vaddr)
                           ERRMSG("Can't get a valid pte.\n");
                           return NOT_PADDR;
                   } else {
-
-                       paddr = (PAGEBASE(pte_val(ptev)) & PHYS_MASK)
+                       paddr = (PAGEBASE(pte_val(ptev)) & PTE_ADDR_MASK)
                                           + (vaddr & (PAGESIZE() - 1));

I think __pte_to_phys() is needed also here, not PTE_ADDR_MASK.

I had some issues with the same on ARMv8 simulator, so lets stick with the tested 'PTE_ADDR_MASK' usage
for now.

Did you test this PTE_ADDR_MASK line on a system actually using
52-bit PA? If a 52-bit physical address is actually used, this will
return a wrong address, for example:

    PTE_ADDR_MASK  0x0000fffffffff000
    PTE            0x0000123456789000
    PAGEBASE'd     0x0000123456780000
     v
    paddr          0x0000123456780000 + 64k offset // incorrect

With 52-bit PA, the PTE_ADDR_MASK is used for __phys_to_pte_val(),
not __pte_to_phys().

If I understand correctly, we should need __pte_to_phys() also here.

    paddr = __pte_to_phys(ptev)
                    + (vaddr & (PAGESIZE() - 1));

Could you try this?

Yes, the ARMv8 simulator can support both ARMv8.2 LPA and LVA features
and I tested the above suggestion earlier also on the same and landed
into incorrect paddress calculation issues.

Since the simulator is slow and its time taking to insert rebuilt
user-space components in the Linaro initramfs being used on the same, I
would suggest that we go ahead with the above code for now and later
when I have more results from the simulator/real hardware, I will send
out a follow-up patch (if needed) to fix the paddress calculation.

Hmm, it theoretically needs __pte_to_phys(), right?
So if you have an issue with it, there may be a bug somewhere
and need to debug it. Do you have the detailed information?

Its not very easy to get the detailed UART console logs from the simulator, so it is hard to get all the debug logs from makedumpfile, so I am trying debugging the issue via 'gdb' by adding it to the initramfs.

However till then to fix the regression reported with upstream makedumpfile on arm64 platforms which don't support ARMv8.2-LPA extensions (e.g. Cortex-A57) and run a newer kernel with PA=52-bit configuration, we can apply this patch for now.

I have tested this on non-ARMv8.2-LPA platforms like apm-osprey and huwaei-taishan and the makedumpfile can work fine.

I will come back with a follow-up patch (if needed) after some checks on the ARMv8 Simulator for the __pte_to_phys() part.

Thanks,
Bhupesh




_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux