linux-kernel - Re: [PATCH v1 07/26] x86/fault: Add helper for dumping RMP entries

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <625926f9-6c45-4242-ac62-8f36abfcb099@amd.com>
Date: Wed, 10 Jan 2024 09:20:44 -0600
From: Tom Lendacky <thomas.lendacky@....com>
To: Borislav Petkov <bp@...en8.de>, Michael Roth <michael.roth@....com>
Cc: x86@...nel.org, kvm@...r.kernel.org, linux-coco@...ts.linux.dev,
 linux-mm@...ck.org, linux-crypto@...r.kernel.org,
 linux-kernel@...r.kernel.org, tglx@...utronix.de, mingo@...hat.com,
 jroedel@...e.de, hpa@...or.com, ardb@...nel.org, pbonzini@...hat.com,
 seanjc@...gle.com, vkuznets@...hat.com, jmattson@...gle.com,
 luto@...nel.org, dave.hansen@...ux.intel.com, slp@...hat.com,
 pgonda@...gle.com, peterz@...radead.org,
 srinivas.pandruvada@...ux.intel.com, rientjes@...gle.com, tobin@....com,
 vbabka@...e.cz, kirill@...temov.name, ak@...ux.intel.com,
 tony.luck@...el.com, sathyanarayanan.kuppuswamy@...ux.intel.com,
 alpergun@...gle.com, jarkko@...nel.org, ashish.kalra@....com,
 nikunj.dadhania@....com, pankaj.gupta@....com, liam.merwick@...cle.com,
 zhi.a.wang@...el.com, Brijesh Singh <brijesh.singh@....com>
Subject: Re: [PATCH v1 07/26] x86/fault: Add helper for dumping RMP entries

On 1/10/24 05:13, Borislav Petkov wrote:
> On Sat, Dec 30, 2023 at 10:19:35AM -0600, Michael Roth wrote:
>> +	while (pfn_current < pfn_end) {
>> +		e = __snp_lookup_rmpentry(pfn_current, &level);
>> +		if (IS_ERR(e)) {
>> +			pfn_current++;
>> +			continue;
>> +		}
>> +
>> +		e_data = (u64 *)e;
>> +		if (e_data[0] || e_data[1]) {
>> +			pr_info("No assigned RMP entry for PFN 0x%llx, but the 2MB region contains populated RMP entries, e.g.: PFN 0x%llx: [high=0x%016llx low=0x%016llx]\n",
>> +				pfn, pfn_current, e_data[1], e_data[0]);
>> +			return;
>> +		}
>> +		pfn_current++;
>> +	}
>> +
>> +	pr_info("No populated RMP entries in the 2MB region containing PFN 0x%llx\n",
>> +		pfn);
>> +}
> 
> Ok, I went and reworked this, see below.
> 
> Yes, I think it is important - at least in the beginning - to dump the
> whole 2M PFN region for debugging purposes. If that output starts
> becoming too unwieldy and overflowing terminals or log files, we'd
> shorten it or put it behind a debug option or so.
> 
> Thx.
> 
> ---
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index a8cf33b7da71..259a1dd655a7 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c

> +	pr_info("PFN 0x%llx unassigned, dumping the whole 2M PFN region: [0x%llx - 0x%llx]\n",
> +		pfn, pfn_i, pfn_end);

How about saying "... dumping all non-zero entries in the whole ..."

and then removing the print below that prints the PFN and "..."

> +
> +	while (pfn_i < pfn_end) {
> +		e = __snp_lookup_rmpentry(pfn_i, &level);
>   		if (IS_ERR(e)) {
> -			pfn_current++;
> +			pr_err("Error %ld reading RMP entry for PFN 0x%llx\n",
> +				PTR_ERR(e), pfn_i);
> +			pfn_i++;
>   			continue;
>   		}
>   
> -		e_data = (u64 *)e;
> -		if (e_data[0] || e_data[1]) {
> -			pr_info("No assigned RMP entry for PFN 0x%llx, but the 2MB region contains populated RMP entries, e.g.: PFN 0x%llx: [high=0x%016llx low=0x%016llx]\n",
> -				pfn, pfn_current, e_data[1], e_data[0]);
> -			return;
> -		}
> -		pfn_current++;
> -	}
> +		if (e->lo || e->hi)
> +			pr_info("PFN: 0x%llx, [0x%016llx - 0x%016llx]\n", pfn_i, e->lo, e->hi);
> +		else
> +			pr_info("PFN: 0x%llx ...\n", pfn_i);

Remove this one.

That should cut down on excess output since you are really only concerned 
with non-zero RMP entries when the input PFN RMP entry is not assigned.

Thanks,
Tom

>   
> -	pr_info("No populated RMP entries in the 2MB region containing PFN 0x%llx\n",
> -		pfn);
> +		pfn_i++;
> +	}
>   }
>   
>   void snp_dump_hva_rmpentry(unsigned long hva)
> @@ -339,4 +343,3 @@ void snp_dump_hva_rmpentry(unsigned long hva)
>   
>   	dump_rmpentry(pte_pfn(*pte));
>   }
> -EXPORT_SYMBOL_GPL(snp_dump_hva_rmpentry);
>