Re: [PATCH 3/3] [RFC] x86/devmem: remove low 1MB hack for x86-64

[Naveen N Rao]

On Tue, Jun 03, 2025 at 12:36:24PM -0700, Dan Williams wrote:
> Arnd Bergmann wrote:
> > On Tue, Jun 3, 2025, at 20:18, Dan Williams wrote:
> > > [add Naveen]
> > >
> > > Arnd Bergmann wrote:
> > >> On Thu, May 22, 2025, at 00:14, Dan Williams wrote:
> > >> > Arnd Bergmann wrote:
> > >>
> > >> The third one maps the BIOS area at 0xf0000, and as far as I can tell
> > >> the hack explicitly allowed mapping that even though it is marked
> > >> busy on x86-64 since 5d94e81f69d4 ("x86: Introduce pci_map_biosrom()").
> > >> 
> > >> Is there any downside to marking this one non-busy and still allowing
> > >> the ROM to be mapped? Would that bring back the issue of conflicting
> > >> mapping flags between kernel and userspace?
> > >
> > > For the confidential VM case I expect the answer is "yes" per this patch
> > > attempt:
> > >
> > > http://lore.kernel.org/20250403120228.2344377-1-naveen@xxxxxxxxxx
> > 
> > I thought the problem here was the read() on /dev/mem, not
> > the mmap(), are you sure it's both?
> > 
> > With this patch [3/3], the memremap() hack for mem_read() goes away on
> > 64-bit, so there should be no way it gets mapped again using that,
> > and the generic devmem_is_allowed() just forbids it as well.
> > 
> > The mmap() access in turn goes through this function
> > 
> > pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
> >                                 unsigned long size, pgprot_t vma_prot)
> > {
> >         if (!phys_mem_access_encrypted(pfn << PAGE_SHIFT, size))
> >                 vma_prot = pgprot_decrypted(vma_prot);
> > 
> >         return vma_prot;
> > }
> > 
> > which I would expect to return the correct vma_prot value already.
> 
> My understanding is that while that gets the correct vma_prot and solves
> the TDX problem it leaves the SEV-SNP problem that the range may not be
> "accepted" ('pvalidate' invoked for the range) at the time the mapping
> is established. So rather than try to make sure ROMs are accepted early
> the proposal is just block altogether.
> 
> Naveen, did I get that right?

That's correct. This patch series doesn't help address that issue, where 
the mmap() is a problem too.

Thanks,
Naveen