On 2/10/23 08:58, Pavan Kumar Paluri wrote:
--- a/arch/x86/kernel/sev.c +++ b/arch/x86/kernel/sev.c @@ -648,8 +648,8 @@ static void pvalidate_pages(unsigned long vaddr, unsigned int npages, bool valid unsigned long vaddr_end; int rc;
- vaddr = vaddr & PAGE_MASK;
- vaddr_end = vaddr + (npages << PAGE_SHIFT);
- vaddr_end = PAGE_ALIGN(vaddr + (npages << PAGE_SHIFT));
- vaddr = PAGE_ALIGN_DOWN(vaddr);
Could you folks please go take a look at all of the callers that call into pvalidate_pages() and set_pages_state()?
I think part of the problem here is that the API is quite inconsistent. There have been a few bugs in this area. Ideally, if you have an interface that deals in 'pages', then the addresses should be aligned already before hitting that interface.
But, there are messes like this:
static inline void __init snp_memcpy(void *dst, void *src, size_t sz, unsigned long paddr, bool decrypt) { unsigned long npages = PAGE_ALIGN(sz) >> PAGE_SHIFT; ... early_snp_set_memory_shared((unsigned long)__va(paddr), paddr, npages);
That's taking a theoretically unaligned 'paddr', page-aligning the size, then passing the result into an 'npages' interface. That makes zero sense if, for instance, it tried to do:
paddr = 0x0fff sz = 2
That would pass along:
paddr = 0x0fff npages = 1
npages (the effective end address) is aligned, but paddr is not.
We can keep on hacking around these bugs as they present themselves one-by-one. Could you look a bit more widely, please, and see if there's something more fundamental that should be done?