Re: [PATCH] x86/ioremap: Maintain consistent IORES_MAP_ENCRYPTED for BIOS data

[Dan Williams <dan.j.williams@intel.com>]

Nikolay Borisov wrote:
> 
> 
> On 1.04.25 г. 2:14 ч., Dan Williams wrote:
> > Nikolay reports [1] that accessing BIOS data (first 1MB of the physical
> > address space) via /dev/mem results in an SEPT violation.
> > 
> > The cause is ioremap() (via xlate_dev_mem_ptr()) establishing an
> > unencrypted mapping where the kernel had established an encrypted
> > mapping previously.
> > 
> > Teach __ioremap_check_other() that this address space shall always be
> > mapped as encrypted as historically it is memory resident data, not MMIO
> > with side-effects.
> > 
> > Cc: <x86@kernel.org>
> > Cc: Vishal Annapurve <vannapurve@google.com>
> > Cc: Kirill Shutemov <kirill.shutemov@linux.intel.com>
> > Reported-by: Nikolay Borisov <nik.borisov@suse.com>
> > Closes: http://lore.kernel.org/20250318113604.297726-1-nik.borisov@suse.com [1]
> > Tested-by: Nikolay Borisov <nik.borisov@suse.com>
> > Fixes: 9aa6ea69852c ("x86/tdx: Make pages shared in ioremap()")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> 
> Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
> 
> > ---
> >   arch/x86/mm/ioremap.c |    4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> > diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> > index 42c90b420773..9e81286a631e 100644
> > --- a/arch/x86/mm/ioremap.c
> > +++ b/arch/x86/mm/ioremap.c
> > @@ -122,6 +122,10 @@ static void __ioremap_check_other(resource_size_t addr, struct ioremap_desc *des
> >   		return;
> >   	}
> >   
> > +	/* Ensure BIOS data (see devmem_is_allowed()) is consistently mapped */
> > +	if (PHYS_PFN(addr) < 256)
> > +		desc->flags |= IORES_MAP_ENCRYPTED;
> 
> Side question: Is it guaranteed that this region will be mapped with 4k 
> pages and not some larger size? I.e should the 256 constant be dependent 
> on the current page size?

True, if in some future kernel PAGE_SHIFT changes for x86 then both
devmem and this code would be confused.

I will submit a follow-on patch to clean that up.

That said, I expect PAGE_SHIFT != 12 breaks many other places besides
this in x86. I wrote it this way just for symmetry with
devmem_is_allowed().