Re: [PATCH v3 01/10] arm64: mm: Flip kernel VA space

[Steve Capper <Steve.Capper@arm.com>]

Hi Anshuman,

Thanks for taking a look at this.

On Fri, Jun 14, 2019 at 05:47:55PM +0530, Anshuman Khandual wrote:
> Hello Steve,
> 
> On 06/12/2019 10:56 PM, Steve Capper wrote:
> > Put the direct linear map in the lower addresses of the kernel VA range
> > and everything else in the higher ranges.
> 
> The reason for this flip has to be more clear in the commit message.
> 

Agreed, that's what I failed to do below ;-).

> > 
> > This allows us to make room for an inline KASAN shadow that operates
> > under both 48 and 52 bit kernel VA sizes. For example with a 52-bit VA,
> > if KASAN_SHADOW_END < 0xFFF8000000000000 (it is in the lower addresses
> > of the kernel VA range), this will be below the start of the minimum
> > 48-bit kernel VA address of 0xFFFF000000000000.
> 
> Though this is true it does not convey to the effect of why the flip is
> required. As you had mentioned previously KASAN_SHADOW_END is fixed because
> it needs to support the highest possible VA (~0UL) in both 48 and 52 bits.
> 
> Hence KASAN_SHADOW_START will have to be variable in order to accommodate
> both 48 bits or 52 bits effective virtual address. Hence not sure what the
> above example based on KASAN_SHADOW_START is trying to convey.
> 
> KASAN_SHADOW_END cannot be in the (52 bit space - 48 bit space) exclusion VA
> range as it would not work for the 48 bits case. But KASAN_SHADOW_START can
> very well be in that space for 52 bits case.
> 
> The current definition
> 
> #define KASAN_SHADOW_START      (VA_START)
> #define KASAN_SHADOW_END        (KASAN_SHADOW_START + KASAN_SHADOW_SIZE)
> 
> This wont work in the new scheme because VA_START is different for 48 bits
> and 52 bits which will make KASAN_SHADOW_END variable as well. Hence we need
> to change this arrangement.

Yes in a future patch? VA_BITS is still constant here.
> 
> What the commit message here does not try to convince is that there are no
> other alternate arrangements where KASAN_SHADOW_END remains fixed (and also
> accessible in 48 bits scheme), KASAN_SHADOW_FIRST is variable accommodating
> both 48 bits and 52 bits case and flipping the kernel VA space is the only
> viable option.
> 
> Ahh I see this in the cover letter.
> 
> " In order to allow for a KASAN shadow that changes size at boot time, one
> must fix the KASAN_SHADOW_END for both 48 & 52-bit VAs and "grow" the
> start address. Also, it is highly desirable to maintain the same
> function addresses in the kernel .text between VA sizes. Both of these
> requirements necessitate us to flip the kernel address space halves s.t.
> the direct linear map occupies the lower addresses."
> 
> Though this is better (please add it to commit message).
> 

Sure :-)

> There were no alternate arrangements to achieve the above two objectives
> without flipping the VA space ? The reasoning here in the commit message is
> not convincing enough even with the above cover letter extract.
> 
> > 
> > We need to adjust:
> >  *) KASAN shadow region placement logic,
> >  *) KASAN_SHADOW_OFFSET computation logic,
> >  *) virt_to_phys, phys_to_virt checks,
> >  *) page table dumper.
> > 
> > These are all small changes, that need to take place atomically, so they
> > are bundled into this commit.
> 
> It will be great to add a before patch and after patch view of the kernel
> virtual address space enlisting different sections to make things apparent
> about what and how the layout has changed.

Hmm, pondering this. I can introduce a documentation document in a new
first patch then modify it as the series changes?

Catalin, would that work for you?

Cheers,
-- 
Steve

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel