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);
