On 12/8/23 23:10, Kalra, Ashish wrote:
> 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.
Yeah that should work. Need to be careful though, should probably only
process head pages and check if the whole compound_order() is within the
range we are to leak, and then leak the head page and advance the loop
by compound_order(). And if we encounter a tail page, it should probably
be just skipped. I'm looking at snp_reclaim_pages() which seems to
process a number of pages with SEV_CMD_SNP_PAGE_RECLAIM and once any
fails, call snp_leak_pages() on the rest. Could that invoke
snp_leak_pages with the first pfn being a tail page?
> Thanks,
> Ashish
