On Sat, Jan 27, 2024 at 12:42:07PM +0100, Borislav Petkov wrote: > On Fri, Jan 26, 2024 at 05:54:20PM -0600, Michael Roth wrote: > > Is something like this close to what you're thinking? I've re-tested with > > SNP guests and it seems to work as expected. > > > > diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c > > index 846e9e53dff0..c09497487c08 100644 > > --- a/arch/x86/virt/svm/sev.c > > +++ b/arch/x86/virt/svm/sev.c > > @@ -421,7 +421,12 @@ static int adjust_direct_map(u64 pfn, int rmp_level) > > if (WARN_ON_ONCE(rmp_level > PG_LEVEL_2M)) > > return -EINVAL; > > > > - if (WARN_ON_ONCE(rmp_level == PG_LEVEL_2M && !IS_ALIGNED(pfn, PTRS_PER_PMD))) > > + if (!pfn_valid(pfn)) > > _text at VA 0xffffffff81000000 is also a valid pfn so no, this is not > enough. directmap maps all physical memory accessible by kernel, including text pages, so those are valid PFNs as far as this function is concerned. We can't generally guard against the caller passing in any random PFN that might also be mapped into additional address ranges, similarly to how we can't guard against something doing a write to some random PFN __va(0x1234) and scribbling over memory that it doesn't own, or just unmapping the entire directmap range and blowing up the kernel. The expectation is that the caller is aware of what PFNs it is passing in, whether those PFNs have additional mappings, and if those mappings are of concern, implement the necessary handlers if new use-cases are ever introducted, like the adjust_kernel_text_mapping() example I mentioned earlier. > > Either this function should not have "direct map" in the name as it > converts *any* valid pfn not just the direct map ones or it should check > whether the pfn belongs to the direct map range. This function only splits mappings in the 0xffff888000000000 directmap range. It would be inaccurate to name it in such a way that suggests that it does anything else. If a use-case ever arises for splitting _text mappings at 0xffffffff81000000, or any other ranges, those too would best served by dedicated helpers adjust_kernel_text_mapping() that *actually* modify mappings for those virtual ranges, and implement bounds-checking appropriate for those physical/virtual ranges. The directmap range maps all kernel-accessible physical memory, so it's appropriate that our bounds-checking for the purpose of adjust_direct_map() is all kernel-accessible physical memory. If that includes PFNs mapped to other virtual ranges as well, the caller needs to consider that and implement additional helpers as necessary, but they'd likely *still* need to call adjust_direct_map() to adjust the directmap range in addition to those other ranges, so even if were possible to do so reliably, we shouldn't try to selectively reject any PFN ranges beyond what mm.rst suggests is valid for 0xffff888000000000. -Mike > > Thx. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette >
