Re: [PATCH] arm: Extend the check for RAM in /dev/mem

[Raslan, KarimAllah]

On Fri, 2019-07-12 at 08:06 +0530, Anshuman Khandual wrote:
> 
> On 07/12/2019 03:51 AM, KarimAllah Ahmed wrote:
> > 
> > Some valid RAM can live outside kernel control (e.g. using mem= kernel
> > command-line). For these regions, pfn_valid would return "false" causing
> > system RAM to be mapped as uncached. Use memblock instead to identify RAM.
> 
> Once the remaining memory is outside of the kernel (as the admin would have
> intended with mem= command line) what is the particular concern regarding
> the way those get mapped (cached or not) ? It is not to be used any way.

They can be used by user-space which might lead to them being used by theÂ
kernel. One use-case would be using them as guest memory for KVM as I detailedÂ
here:

https://lwn.net/Articles/778240/

> 
> > 
> > 
> > Cc: Russell King <linux@xxxxxxxxxxxxxxx>
> > Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> > Cc: Will Deacon <will@xxxxxxxxxx>
> > Cc: Mike Rapoport <rppt@xxxxxxxxxxxxx>
> > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > Cc: Anders Roxell <anders.roxell@xxxxxxxxxx>
> > Cc: Enrico Weigelt <info@xxxxxxxxx>
> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > Cc: KarimAllah Ahmed <karahmed@xxxxxxxxx>
> > Cc: Mark Rutland <mark.rutland@xxxxxxx>
> > Cc: James Morse <james.morse@xxxxxxx>
> > Cc: Anshuman Khandual <anshuman.khandual@xxxxxxx>
> > Cc: Jun Yao <yaojun8558363@xxxxxxxxx>
> > Cc: Yu Zhao <yuzhao@xxxxxxxxxx>
> > Cc: Robin Murphy <robin.murphy@xxxxxxx>
> > Cc: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > Signed-off-by: KarimAllah Ahmed <karahmed@xxxxxxxxx>
> > ---
> >  arch/arm/mm/mmu.c   | 2 +-
> >  arch/arm64/mm/mmu.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> > index 1aa2586..492774b 100644
> > --- a/arch/arm/mm/mmu.c
> > +++ b/arch/arm/mm/mmu.c
> > @@ -705,7 +705,7 @@ static void __init build_mem_type_table(void)
> >  pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
> >  			      unsigned long size, pgprot_t vma_prot)
> >  {
> > -	if (!pfn_valid(pfn))
> > +	if (!memblock_is_memory(__pfn_to_phys(pfn)))
> >  		return pgprot_noncached(vma_prot);
> >  	else if (file->f_flags & O_SYNC)
> >  		return pgprot_writecombine(vma_prot);
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > index 3645f29..cdc3e8e 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -78,7 +78,7 @@ void set_swapper_pgd(pgd_t *pgdp, pgd_t pgd)
> >  pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
> >  			      unsigned long size, pgprot_t vma_prot)
> >  {
> > -	if (!pfn_valid(pfn))
> > +	if (!memblock_is_memory(__pfn_to_phys(pfn)))
> 
> pfn_valid() on arm64 checks if the memblock region is mapped i.e does it have
> a linear mapping or not. If a segment of RAM is outside linear mapping due to
> mem= directive and lacks a linear mapping then why should it be mapped similarly
> like system RAM on this path ?

I actually struggled a bit here because there is really no *explicit*Â
documentation of what is the expected behavior here, so for me it was open toÂ
interpretation.

It seems like for you the deciding factor between cached and uncached is theÂ
existence of linear mapping. However, for me the deciding factor is whether it
is RAM or not. I choose this interpretation because it helps in the KVM
scenario that I mentioned above :)




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879