Re: [PATCH v10 16/50] x86/sev: Introduce snp leaked pages list

[Kalra, Ashish]

Hello Vlastimil,

On 12/7/2023 10:20 AM, Vlastimil Babka wrote:

> > +
> > +void snp_leak_pages(u64 pfn, unsigned int npages)
> > +{
> > +	struct page *page = pfn_to_page(pfn);
> > +
> > +	pr_debug("%s: leaking PFN range 0x%llx-0x%llx\n", __func__, pfn, pfn + npages);
> > +
> > +	spin_lock(&snp_leaked_pages_list_lock);
> > +	while (npages--) {
> > +		/*
> > +		 * Reuse the page's buddy list for chaining into the leaked
> > +		 * pages list. This page should not be on a free list currently
> > +		 * and is also unsafe to be added to a free list.
> > +		 */
> > +		list_add_tail(&page->buddy_list, &snp_leaked_pages_list);
> > +		sev_dump_rmpentry(pfn);
> > +		pfn++;
>
> You increment pfn, but not page, which is always pointing to the page of the
> initial pfn, so need to do page++ too.

Yes, that is a bug and needs to be fixed.

> But that assumes it's all order-0 pages (hard to tell for me whether that's
> true as we start with a pfn), if there can be compound pages, it would be
> best to only add the head page and skip the tail pages - it's not expected
> to use page->buddy_list of tail pages.

Can't we use PageCompound() to check if the page is a compound page and 
then use page->compound_head to get and add the head page to leaked 
pages list. I understand the tail pages for compound pages are really 
limited for usage.

Thanks,
Ashish