Hi Bhupesh, -----Original Message----- > Hi Kazu, > > Thanks for the review. > > On 02/21/2019 09:05 PM, Kazuhito Hagio wrote: > > Hi Bhupesh, > > > > -----Original Message----- > >> ARMv8.2-LPA architecture extension (if available on underlying hardware) > >> can support 52-bit physical addresses, while the kernel virtual > >> addresses remain 48-bit. > >> > >> This patch is in accordance with ARMv8 Architecture Reference Manual > >> version D.a > >> > >> Make sure that we read the 52-bit PA address capability from > >> 'MAX_PHYSMEM_BITS' variable (if available in vmcoreinfo) and > >> accordingly change the pte_to_phy() mask values and also traverse > >> the page-table walk accordingly. > >> > >> Also make sure that it works well for the existing 48-bit PA address > >> platforms and also on environments which use newer kernels with 52-bit > >> PA support but hardware which is not ARM8.2-LPA compliant. > >> > >> I have sent a kernel patch upstream to add 'MAX_PHYSMEM_BITS' to > >> vmcoreinfo for arm64 (see [0]). > >> > >> [0]. http://lists.infradead.org/pipermail/kexec/2019-February/022411.html > >> > >> Signed-off-by: Bhupesh Sharma <[email protected]> > > > > This patch looks good to me. > > For two slight things below, I will remove them when merging. > > > >> +/* > >> + * Size mapped by an entry at level n ( 0 <= n <= 3) > >> + * We map (PAGE_SHIFT - 3) at all translation levels and PAGE_SHIFT bits > >> + * in the final page. The maximum number of translation levels supported > >> by > >> + * the architecture is 4. Hence, starting at at level n, we have further > >> + * ((4 - n) - 1) levels of translation excluding the offset within the > >> page. > >> + * So, the total number of bits mapped by an entry at level n is : > >> + * > >> + * ((4 - n) - 1) * (PAGE_SHIFT - 3) + PAGE_SHIFT > >> + * > >> + * Rearranging it a bit we get : > >> + * (4 - n) * (PAGE_SHIFT - 3) + 3 > >> + */ > > > > Will remove this comment. > > Ok. > > >> +#define pmd_offset_pgtbl_lvl_2(dir, vaddr) ((pmd_t *)dir) > >> +#define pmd_offset_pgtbl_lvl_3(dir, vaddr) (pud_page_paddr((*(dir))) + > >> pmd_index(vaddr) * > sizeof(pmd_t)) > > > > Will remove these two macros not in use. > > Ok. > > > > > And, as I said on another thread, I'm thinking to merge the following > > patch after your patch 1/2, it tested OK with 48-bit and 52-bit PA > > without NUMBER(MAX_PHYSMEM_BITS) in vmcoreinfo. > > Do you think of any case that this will not work well? > > > > diff --git a/arch/arm64.c b/arch/arm64.c > > index 29247a7..c7e60e0 100644 > > --- a/arch/arm64.c > > +++ b/arch/arm64.c > > @@ -127,6 +127,9 @@ typedef unsigned long pgdval_t; > > */ > > #define SECTIONS_SIZE_BITS 30 > > > > +#define _MAX_PHYSMEM_BITS_48 48 > > +#define _MAX_PHYSMEM_BITS_52 52 > > + > > /* > > * Hardware page table definitions. > > * > > @@ -402,17 +405,27 @@ get_stext_symbol(void) > > return(found ? kallsym : FALSE); > > } > > > > +static int > > +set_max_physmem_bits_arm64(void) > > +{ > > + long array_len = ARRAY_LENGTH(mem_section); > > + > > + info->max_physmem_bits = _MAX_PHYSMEM_BITS_48; > > + if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) > > + || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT()))) > > + return TRUE; > > + > > + info->max_physmem_bits = _MAX_PHYSMEM_BITS_52; > > + if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) > > + || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT()))) > > + return TRUE; > > + > > + return FALSE; > > +} > > + > > int > > get_machdep_info_arm64(void) > > { > > - /* Determine if the PA address range is 52-bits: ARMv8.2-LPA */ > > - if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) { > > - info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); > > - if (info->max_physmem_bits == 52) > > - lpa_52_bit_support_available = 1; > > - } else > > - info->max_physmem_bits = 48; > > - > > /* Check if va_bits is still not initialized. If still 0, call > > * get_versiondep_info() to initialize the same. > > */ > > @@ -428,9 +441,24 @@ get_machdep_info_arm64(void) > > info->section_size_bits = SECTIONS_SIZE_BITS; > > > > DEBUG_MSG("kimage_voffset : %lx\n", kimage_voffset); > > - DEBUG_MSG("max_physmem_bits : %ld\n", info->max_physmem_bits); > > DEBUG_MSG("section_size_bits: %ld\n", info->section_size_bits); > > > > + /* Determine if the PA address range is 52-bits: ARMv8.2-LPA */ > > + if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) { > > + info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); > > + DEBUG_MSG("max_physmem_bits : %ld (vmcoreinfo)\n", > > + info->max_physmem_bits); > > + } else if (set_max_physmem_bits_arm64()) { > > + DEBUG_MSG("max_physmem_bits : %ld (detected)\n", > > + info->max_physmem_bits); > > + } else { > > + ERRMSG("Can't determine max_physmem_bits value\n"); > > + return FALSE; > > + } > > + > > + if (info->max_physmem_bits == 52) > > + lpa_52_bit_support_available = 1; > > + > > return TRUE; > > } > > I have not tested the above suggestion on a real hardware or emulation > model yet, but as we were discussing in the kernel patch review thread > (see [0]), IMO, we don't need to carry the above hoops for > 'MAX_PHYSMEM_BITS' calculation in makedumpfile code as it makes the code > less portable for a newer kernel version and also since other user-space > utilities (like crash) also need a mechanism to determine the PA_BITS > supported by the underlying kernel, so we can use the same uniform > method of using an exported 'MAX_PHYSMEM_BITS' value in the vmcoreinfo > so that all user-land applications can use the same. > > I think Dave A. (crash utility maintainer) also pointed to a similar > concern in the above thread.
I see. As I replied [1], I also think ideally we should export it. [1] http://lists.infradead.org/pipermail/kexec/2019-February/022474.html Can we export it from kernel/crash_core.c? I'd like to avoid repeating such a discussion every time we need it for each architecture.. Thanks, Kazu > > [0]. http://lists.infradead.org/pipermail/kexec/2019-February/022472.html > > Thanks, > Bhupesh _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
