Re: [PATCH v2 11/25] x86/sev: Adjust directmap to avoid inadvertant RMP faults

[Michael Roth]

On Fri, Jan 26, 2024 at 04:34:51PM +0100, Borislav Petkov wrote:
> On Thu, Jan 25, 2024 at 10:11:11PM -0600, Michael Roth wrote:
> > +static int adjust_direct_map(u64 pfn, int rmp_level)
> > +{
> > +	unsigned long vaddr = (unsigned long)pfn_to_kaddr(pfn);
> > +	unsigned int level;
> > +	int npages, ret;
> > +	pte_t *pte;
> 
> Again, something I asked the last time but no reply:
> 
> Looking at Documentation/arch/x86/x86_64/mm.rst, the direct map starts
> at page_offset_base so this here should at least check
> 
> 	if (vaddr < __PAGE_OFFSET)
> 		return 0;

vaddr comes from pfn_to_kaddr(pfn), i.e. __va(paddr), so it will
necessarily be a direct-mapped address above __PAGE_OFFSET.

> 
> I'm not sure about the upper end. Right now, the adjusting should not

For upper-end, a pfn_valid(pfn) check might suffice, since only a valid
PFN would have a possibly-valid mapping wthin the directmap range.

> happen only for the direct map but also for the whole kernel address
> space range because we don't want to cause any mismatch between page
> mappings anywhere.
> 
> Which means, this function should be called adjust_kernel_map() or so...

These are PFNs that are owned/allocated-to the caller. Due to the nature
of the directmap it's possible non-owners would write to a mapping that
overlaps, but vmalloc()/etc. would not create mappings for any pages that
were not specifically part of an allocation that belongs to the caller,
so I don't see where there's any chance for an overlap there. And the caller
of these functions would not be adjusting directmap for PFNs that might be
mapped into other kernel address ranges like kernel-text/etc unless the
caller was specifically making SNP-aware adjustments to those ranges, in
which case it would be responsible for making those other adjustments,
or implementing the necessary helpers/etc.

I'm not aware of such cases in the current code, and I don't think it makes
sense to attempt to try to handle them here generically until such a case
arises, since it will likely involve more specific requirements than what
we can anticipate from a theoretical/generic standpoint.

-Mike

> 
> Hmmm.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette