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?
