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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux