[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240112144931.GCZaFRewg1ft-oS_rY@fat_crate.local>
Date: Fri, 12 Jan 2024 15:49:31 +0100
From: Borislav Petkov <bp@...en8.de>
To: 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, thomas.lendacky@....com, 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 10/26] x86/sev: Add helper functions for RMPUPDATE and
PSMASH instruction
On Sat, Dec 30, 2023 at 10:19:38AM -0600, Michael Roth wrote:
> +static int rmpupdate(u64 pfn, struct rmp_state *state)
> +{
> + unsigned long paddr = pfn << PAGE_SHIFT;
> + int ret;
> +
> + if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
> + return -ENODEV;
> +
Let's document this deal here and leave the door open for potential
future improvements:
---
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index c9156ce47c77..1fbf9843c163 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -374,6 +374,21 @@ static int rmpupdate(u64 pfn, struct rmp_state *state)
if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
return -ENODEV;
+ /*
+ * It is expected that those operations are seldom enough so
+ * that no mutual exclusion of updaters is needed and thus the
+ * overlap error condition below should happen very seldomly and
+ * would get resolved relatively quickly by the firmware.
+ *
+ * If not, one could consider introducing a mutex or so here to
+ * sync concurrent RMP updates and thus diminish the amount of
+ * cases where firmware needs to lock 2M ranges to protect
+ * against concurrent updates.
+ *
+ * The optimal solution would be range locking to avoid locking
+ * disjoint regions unnecessarily but there's no support for
+ * that yet.
+ */
do {
/* Binutils version 2.36 supports the RMPUPDATE mnemonic. */
asm volatile(".byte 0xF2, 0x0F, 0x01, 0xFE"
> + do {
> + /* Binutils version 2.36 supports the RMPUPDATE mnemonic. */
> + asm volatile(".byte 0xF2, 0x0F, 0x01, 0xFE"
> + : "=a" (ret)
> + : "a" (paddr), "c" ((unsigned long)state)
> + : "memory", "cc");
> + } while (ret == RMPUPDATE_FAIL_OVERLAP);
> +
> + if (ret) {
> + pr_err("RMPUPDATE failed for PFN %llx, ret: %d\n", pfn, ret);
> + dump_rmpentry(pfn);
> + dump_stack();
> + return -EFAULT;
> + }
> +
> + return 0;
> +}
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Powered by blists - more mailing lists