On Mon, Aug 18, 2025, Ashish Kalra wrote:
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 2fbdebf79fbb..a7db96a5f56d 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -271,7 +271,7 @@ static void sev_decommission(unsigned int handle)
> static int kvm_rmp_make_shared(struct kvm *kvm, u64 pfn, enum pg_level level)
> {
> if (KVM_BUG_ON(rmp_make_shared(pfn, level), kvm)) {
> - snp_leak_pages(pfn, page_level_size(level) >> PAGE_SHIFT);
> + snp_leak_pages(pfn, page_level_size(level) >> PAGE_SHIFT, false);
> return -EIO;
> }
>
> @@ -300,7 +300,7 @@ static int snp_page_reclaim(struct kvm *kvm, u64 pfn)
> data.paddr = __sme_set(pfn << PAGE_SHIFT);
> rc = sev_do_cmd(SEV_CMD_SNP_PAGE_RECLAIM, &data, &fw_err);
> if (KVM_BUG(rc, kvm, "Failed to reclaim PFN %llx, rc %d fw_err %d", pfn, rc, fw_err)) {
> - snp_leak_pages(pfn, 1);
> + snp_leak_pages(pfn, 1, false);
Open coded true/false literals are ugly, e.g. now I have to go look at the
declaration (or even definition) of snp_leak_pages() to understand what %false
controls.
Assuming "don't dump the RMP entry" is the rare case, then craft the APIs to
reflect that, i.e. make snp_leak_pages() a wrapper for the common case. As a
bonus, you don't need to churn any extra code either.
void __snp_leak_pages(u64 pfn, unsigned int npages, bool dump_rmp);
static inline void snp_leak_pages(u64 pfn, unsigned int npages)
{
__snp_leak_pages(pfn, npages, true);
}
> return -EIO;
> }
>
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index 942372e69b4d..d75659859a07 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -1029,7 +1029,7 @@ int rmp_make_shared(u64 pfn, enum pg_level level)
> }
> EXPORT_SYMBOL_GPL(rmp_make_shared);
>
> -void snp_leak_pages(u64 pfn, unsigned int npages)
> +void snp_leak_pages(u64 pfn, unsigned int npages, bool quiet)
> {
> struct page *page = pfn_to_page(pfn);
>
> @@ -1052,7 +1052,8 @@ void snp_leak_pages(u64 pfn, unsigned int npages)
> (PageHead(page) && compound_nr(page) <= npages))
> list_add_tail(&page->buddy_list, &snp_leaked_pages_list);
>
> - dump_rmpentry(pfn);
> + if (!quiet)
The polarity is arbitrarily odd, and "quiet" is annoyingly ambiguous and arguably
misleading, e.g. one could expect "quiet=true" to suppress the pr_warn() too, but
it does not.
pr_warn("Leaking PFN range 0x%llx-0x%llx\n", pfn, pfn + npages)
If you call it "bool dump_rmp" then it's more precise, self-explanatory, and
doesn't need to be inverted.
> + dump_rmpentry(pfn);
> snp_nr_leaked_pages++;
> pfn++;
> page++;
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 4f000dc2e639..203a43a2df63 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -408,7 +408,7 @@ static int snp_reclaim_pages(unsigned long paddr, unsigned int npages, bool lock
> * If there was a failure reclaiming the page then it is no longer safe
> * to release it back to the system; leak it instead.
> */
> - snp_leak_pages(__phys_to_pfn(paddr), npages - i);
> + snp_leak_pages(__phys_to_pfn(paddr), npages - i, false);
> return ret;
> }
>
> --
> 2.34.1
>
