RE: [PATCH v2 1/2] 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 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 <bhsharma@xxxxxxxxxx>
> >
> > 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
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