| Message ID | 20210707181506.30489-9-brijesh.singh@amd.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) Guest Support | expand |
On Wed, Jul 07, 2021 at 01:14:38PM -0500, Brijesh Singh wrote: > Virtual Machine Privilege Level (VMPL) is an optional feature in the > SEV-SNP architecture, which allows a guest VM to divide its address space > into four levels. The level can be used to provide the hardware isolated > abstraction layers with a VM. The VMPL0 is the highest privilege, and > VMPL3 is the least privilege. Certain operations must be done by the VMPL0 > software, such as: > > * Validate or invalidate memory range (PVALIDATE instruction) > * Allocate VMSA page (RMPADJUST instruction when VMSA=1) > > The initial SEV-SNP support assumes that the guest kernel is running on > VMPL0. Let's add a check to make sure that kernel is running at VMPL0 > before continuing the boot. There is no easy method to query the current > VMPL level, so use the RMPADJUST instruction to determine whether its > booted at the VMPL0. > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > arch/x86/boot/compressed/sev.c | 41 ++++++++++++++++++++++++++++--- > arch/x86/include/asm/sev-common.h | 1 + > arch/x86/include/asm/sev.h | 3 +++ > 3 files changed, 42 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c > index 7be325d9b09f..2f3081e9c78c 100644 > --- a/arch/x86/boot/compressed/sev.c > +++ b/arch/x86/boot/compressed/sev.c > @@ -134,6 +134,36 @@ static inline bool sev_snp_enabled(void) > return msr_sev_status & MSR_AMD64_SEV_SNP_ENABLED; > } > > +static bool is_vmpl0(void) > +{ > + u64 attrs, va; > + int err; > + > + /* > + * There is no straightforward way to query the current VMPL level. The So this is not nice at all. And this VMPL level checking can't be part of the GHCB MSR protocol because the HV can tell us any VPML level it wants to. Is there a way to disable VMPL levels and say, this guest should run only at VMPL0? Err, I see SYSCFG[VMPLEn]: "VMPLEn. Bit 25. Setting this bit to 1 enables the VMPL feature (Section 15.36.7 “Virtual Machine Privilege Levels,” on page 580). Software should set this bit to 1 when SecureNestedPagingEn is being set to 1. Once SecureNestedPagingEn is set to 1, VMPLEn cannot be changed." But why should that bit be set if SNP is enabled? Can I run a SNP guest without VPMLs, i.e, at an implicit VPML level 0? It says above VPML is optional... Also, why do you even need to do this at all since the guest controls and validates its memory with the RMP? It can simply go and check the VMPLs of every page it owns to make sure it is 0. Also, if you really wanna support guests with multiple VMPLs, then prevalidating its memory is going to be a useless exercise because it'll have to go and revalidate the VMPL levels... I also see this: "When the hypervisor assigns a page to a guest using RMPUPDATE, full permissions are enabled for VMPL0 and are disabled for all other VMPLs." so you get your memory at VMPL0 by the HV. So what is that check for? Questions over questions, I'm sure I'm missing an aspect. > + * simplest method is to use the RMPADJUST instruction to change a page > + * permission to a VMPL level-1, and if the guest kernel is launched at > + * at a level <= 1, then RMPADJUST instruction will return an error. WARNING: Possible repeated word: 'at' #156: FILE: arch/x86/boot/compressed/sev.c:146: + * permission to a VMPL level-1, and if the guest kernel is launched at + * at a level <= 1, then RMPADJUST instruction will return an error. How many times do I have to say: Please integrate scripts/checkpatch.pl into your patch creation workflow. Some of the warnings/errors *actually* make sense. ?
On 8/13/21 2:25 AM, Borislav Petkov wrote: > On Wed, Jul 07, 2021 at 01:14:38PM -0500, Brijesh Singh wrote: >> Virtual Machine Privilege Level (VMPL) is an optional feature in the >> SEV-SNP architecture, which allows a guest VM to divide its address space >> into four levels. The level can be used to provide the hardware isolated >> abstraction layers with a VM. The VMPL0 is the highest privilege, and >> VMPL3 is the least privilege. Certain operations must be done by the VMPL0 >> software, such as: >> >> * Validate or invalidate memory range (PVALIDATE instruction) >> * Allocate VMSA page (RMPADJUST instruction when VMSA=1) >> >> The initial SEV-SNP support assumes that the guest kernel is running on >> VMPL0. Let's add a check to make sure that kernel is running at VMPL0 >> before continuing the boot. There is no easy method to query the current >> VMPL level, so use the RMPADJUST instruction to determine whether its >> booted at the VMPL0. >> >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >> --- >> arch/x86/boot/compressed/sev.c | 41 ++++++++++++++++++++++++++++--- >> arch/x86/include/asm/sev-common.h | 1 + >> arch/x86/include/asm/sev.h | 3 +++ >> 3 files changed, 42 insertions(+), 3 deletions(-) >> >> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c >> index 7be325d9b09f..2f3081e9c78c 100644 >> --- a/arch/x86/boot/compressed/sev.c >> +++ b/arch/x86/boot/compressed/sev.c >> @@ -134,6 +134,36 @@ static inline bool sev_snp_enabled(void) >> return msr_sev_status & MSR_AMD64_SEV_SNP_ENABLED; >> } >> >> +static bool is_vmpl0(void) >> +{ >> + u64 attrs, va; >> + int err; >> + >> + /* >> + * There is no straightforward way to query the current VMPL level. The > So this is not nice at all. > > And this VMPL level checking can't be part of the GHCB MSR protocol > because the HV can tell us any VPML level it wants to. > > Is there a way to disable VMPL levels and say, this guest should run > only at VMPL0? No. > > Err, I see SYSCFG[VMPLEn]: > > "VMPLEn. Bit 25. Setting this bit to 1 enables the VMPL feature (Section > 15.36.7 “Virtual Machine Privilege Levels,” on page 580). Software > should set this bit to 1 when SecureNestedPagingEn is being set to 1. > Once SecureNestedPagingEn is set to 1, VMPLEn cannot be changed." > > But why should that bit be set if SNP is enabled? Can I run a SNP guest > without VPMLs, i.e, at an implicit VPML level 0? During the firmware initialization the PSP requires that the VMPLEn is set. See SNP firmware spec [1] section 8.6. To run the SNP guest you *must* specify a VMPL level during the vCPU creation. > > It says above VPML is optional... I should not say its optional when we know from the SEV-SNP spec that VMPLEn must be set to launch SEV-SNP guest. I will fix the description. > Also, why do you even need to do this at all since the guest controls > and validates its memory with the RMP? It can simply go and check the > VMPLs of every page it owns to make sure it is 0. There is no easy way for a guest to query its VMPL level. The VMPL level is set during the vCPU creation. The boot cpu is created by the HV and thus its VMPL level is set by the HV. If HV chooses a lower VMPL level for the boot CPU then Linux guest will not be able to validate its memory because the PVALIDATE instruction will cause #GP when the vCPU is running at !VMPL0. The patch tries to detect the boot CPU VMPL level and terminate the boot. > > Also, if you really wanna support guests with multiple VMPLs, then > prevalidating its memory is going to be a useless exercise because it'll > have to go and revalidate the VMPL levels... We do not need to re-valiate memory when changing to different VMPL level. The RMPADJUST instruction inside the guest can be used to change the VMPL permission. > I also see this: > > "When the hypervisor assigns a page to a guest using RMPUPDATE, full > permissions are enabled for VMPL0 and are disabled for all other VMPLs." > > so you get your memory at VMPL0 by the HV. So what is that check for? Validating the memory is a two step process: #1 HV adding the memory using the RMPUPDATE in the RMP table. #2 Guest issuing the PVALIDATE If guest is not running at VMPL0 then step #2 will cause #GP. The check is prevent the #GP and terminate the boot early. thanks
On Fri, Aug 13, 2021 at 08:13:20AM -0500, Brijesh Singh wrote: > During the firmware initialization the PSP requires that the VMPLEn is > set. See SNP firmware spec [1] section 8.6. To run the SNP guest you > *must* specify a VMPL level during the vCPU creation. Yes, that's why I said "implicit VMPL level 0"! When you don't specify it, it should implied as 0. Right now that "enable" bit is useless as it is *forced* to be enabled. I sincerely hope querying the VMPL level is going to be made straight-forwaed in future versions. > I should not say its optional when we know from the SEV-SNP spec that > VMPLEn must be set to launch SEV-SNP guest. I will fix the description. It probably wasn't required when that bit was invented - why would you call it "enable" otherwise - but some decision later made it required, I'd guess. > There is no easy way for a guest to query its VMPL level. Yes, and there should be. > The VMPL level is set during the vCPU creation. The boot cpu is > created by the HV and thus its VMPL level is set by the HV. If HV > chooses a lower VMPL level for the boot CPU then Linux guest will > not be able to validate its memory because the PVALIDATE instruction > will cause #GP when the vCPU is running at !VMPL0. The patch tries to > detect the boot CPU VMPL level and terminate the boot. I figured as much. All I don't like is the VMPL checking method. > If guest is not running at VMPL0 then step #2 will cause #GP. The check > is prevent the #GP and terminate the boot early. Yah, Tom helped me understand the design of the permission masks in the RMP on IRC. Thx.
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c index 7be325d9b09f..2f3081e9c78c 100644 --- a/arch/x86/boot/compressed/sev.c +++ b/arch/x86/boot/compressed/sev.c @@ -134,6 +134,36 @@ static inline bool sev_snp_enabled(void) return msr_sev_status & MSR_AMD64_SEV_SNP_ENABLED; } +static bool is_vmpl0(void) +{ + u64 attrs, va; + int err; + + /* + * There is no straightforward way to query the current VMPL level. The + * simplest method is to use the RMPADJUST instruction to change a page + * permission to a VMPL level-1, and if the guest kernel is launched at + * at a level <= 1, then RMPADJUST instruction will return an error. + */ + attrs = 1; + + /* + * Any page aligned virtual address is sufficent to test the VMPL level. + * The boot_ghcb_page is page aligned memory, so lets use for the test. + */ + va = (u64)&boot_ghcb_page; + + /* Instruction mnemonic supported in binutils versions v2.36 and later */ + asm volatile (".byte 0xf3,0x0f,0x01,0xfe\n\t" + : "=a" (err) + : "a" (va), "c" (RMP_PG_SIZE_4K), "d" (attrs) + : "memory", "cc"); + if (err) + return false; + + return true; +} + static bool do_early_sev_setup(void) { if (!sev_es_negotiate_protocol()) @@ -141,10 +171,15 @@ static bool do_early_sev_setup(void) /* * If SEV-SNP is enabled, then check if the hypervisor supports the SEV-SNP - * features. + * features and is launched at VMPL-0 level. */ - if (sev_snp_enabled() && !(sev_hv_features & GHCB_HV_FT_SNP)) - sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED); + if (sev_snp_enabled()) { + if (!sev_hv_features & GHCB_HV_FT_SNP) + sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED); + + if (!is_vmpl0()) + sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_NOT_VMPL0); + } if (set_page_decrypted((unsigned long)&boot_ghcb_page)) return false; diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h index 8abc5eb7ca3d..ea508835ab33 100644 --- a/arch/x86/include/asm/sev-common.h +++ b/arch/x86/include/asm/sev-common.h @@ -78,5 +78,6 @@ #define GHCB_TERM_REGISTER 0 /* GHCB GPA registration failure */ #define GHCB_TERM_PSC 1 /* Page State Change failure */ #define GHCB_TERM_PVALIDATE 2 /* Pvalidate failure */ +#define GHCB_TERM_NOT_VMPL0 3 /* SNP guest is not running at VMPL-0 */ #endif diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h index b308815a2c01..242af1154e49 100644 --- a/arch/x86/include/asm/sev.h +++ b/arch/x86/include/asm/sev.h @@ -62,6 +62,9 @@ extern bool handle_vc_boot_ghcb(struct pt_regs *regs); /* Software defined (when rFlags.CF = 1) */ #define PVALIDATE_FAIL_NOUPDATE 255 +/* RMP page size */ +#define RMP_PG_SIZE_4K 0 + #ifdef CONFIG_AMD_MEM_ENCRYPT extern struct static_key_false sev_es_enable_key; extern void __sev_es_ist_enter(struct pt_regs *regs);
Virtual Machine Privilege Level (VMPL) is an optional feature in the SEV-SNP architecture, which allows a guest VM to divide its address space into four levels. The level can be used to provide the hardware isolated abstraction layers with a VM. The VMPL0 is the highest privilege, and VMPL3 is the least privilege. Certain operations must be done by the VMPL0 software, such as: * Validate or invalidate memory range (PVALIDATE instruction) * Allocate VMSA page (RMPADJUST instruction when VMSA=1) The initial SEV-SNP support assumes that the guest kernel is running on VMPL0. Let's add a check to make sure that kernel is running at VMPL0 before continuing the boot. There is no easy method to query the current VMPL level, so use the RMPADJUST instruction to determine whether its booted at the VMPL0. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- arch/x86/boot/compressed/sev.c | 41 ++++++++++++++++++++++++++++--- arch/x86/include/asm/sev-common.h | 1 + arch/x86/include/asm/sev.h | 3 +++ 3 files changed, 42 insertions(+), 3 deletions(-)