| Message ID | 27fce67472c97b2b2b7cc0412bf0edcaa67cc63f.1712759753.git.andrei.semenov@vates.fr (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | AMD SEV initial work | expand |
On 10/04/2024 4:36 pm, Andrei Semenov wrote: > diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c > index ab92333673..a5903613f0 100644 > --- a/xen/arch/x86/cpu/amd.c > +++ b/xen/arch/x86/cpu/amd.c > @@ -1030,6 +1031,54 @@ static void amd_check_erratum_1485(void) > wrmsrl(MSR_AMD64_BP_CFG, val | chickenbit); > } > > +#ifdef CONFIG_HVM > +static void amd_enable_mem_encrypt(const struct cpuinfo_x86 *c) > +{ > + unsigned int eax, ebx, ecx, edx; > + uint64_t syscfg; > + > + if (!smp_processor_id()) { c == &boot_cpu_info. > + > + cpuid_count(0x80000000,0,&eax, &ebx, &ecx, &edx); > + > + if (eax < 0x8000001f) > + return; Max leaf is already collected. c->extended_cpuid_level > + > + cpuid_count(0x8000001f,0,&eax, &ebx, &ecx, &edx); > + > + if (eax & 0x1) > + setup_force_cpu_cap(X86_FEATURE_SME); > + > + if (eax & 0x2) { > + setup_force_cpu_cap(X86_FEATURE_SEV); > + max_sev_asid = ecx; > + min_sev_asid = edx; > + } > + > + if (eax & 0x3) > + pte_c_bit_mask = 1UL << (ebx & 0x3f); This is decoding the main SEV feature leaf, but outside of normal mechanisms. I've got half a mind to brute-force through the remaining work to un-screw our boot sequence order, and express this in a cpu-policy straight away. This is wanted for the SVM leaf info too. Leave it with me for a bit. > + } > + > + if (!(cpu_has_sme || cpu_has_sev)) > + return; > + > + if (!smp_processor_id()) { > + if (cpu_has_sev) > + printk(XENLOG_INFO "SEV: ASID range [0x%x - 0x%x]\n", > + min_sev_asid, max_sev_asid); Why do we have a min as well as a max? Isn't min always 1? > + } > + > + rdmsrl(MSR_K8_SYSCFG, syscfg); > + > + if (syscfg & SYSCFG_MEM_ENCRYPT) { > + return; > + } > + > + syscfg |= SYSCFG_MEM_ENCRYPT; > + wrmsrl(MSR_K8_SYSCFG, syscfg); > +} > +#endif > + > static void cf_check init_amd(struct cpuinfo_x86 *c) > { > u32 l, h; > @@ -1305,6 +1354,10 @@ static void cf_check init_amd(struct cpuinfo_x86 *c) > check_syscfg_dram_mod_en(); > > amd_log_freq(c); > + > +#ifdef CONFIG_HVM > + amd_enable_mem_encrypt(c); > +#endif I think we want to drop the CONFIG_HVM here. Memory encryption is an all-or-nothing thing. If it's active, it affects all pagetables that Xen controls, even dom0's. And we likely do want get to the point of Xen running on encrypted mappings even if dom0 can't operate it very nicely. Thoughts? > } > > const struct cpu_dev __initconst_cf_clobber amd_cpu_dev = { > diff --git a/xen/arch/x86/hvm/svm/Makefile b/xen/arch/x86/hvm/svm/Makefile > index 760d2954da..9773d539ef 100644 > --- a/xen/arch/x86/hvm/svm/Makefile > +++ b/xen/arch/x86/hvm/svm/Makefile > @@ -6,3 +6,4 @@ obj-y += nestedsvm.o > obj-y += svm.o > obj-y += svmdebug.o > obj-y += vmcb.o > +obj-y += sev.o Please keep this sorted by object file name. > diff --git a/xen/arch/x86/hvm/svm/sev.c b/xen/arch/x86/hvm/svm/sev.c > new file mode 100644 > index 0000000000..336fad25f5 > --- /dev/null > +++ b/xen/arch/x86/hvm/svm/sev.c > @@ -0,0 +1,4 @@ > +#include <asm/sev.h> > +uint64_t __read_mostly pte_c_bit_mask; > +unsigned int __read_mostly min_sev_asid; > +unsigned int __read_mostly max_sev_asid; Several things. All new files should come with an SPDX tag. Unless you have other constraints, GPL-2.0-only is preferred. There also wants to be at least a oneline summary of what's going on here. All these variables look like they should be __ro_after_init. However, it's rather hard to judge, given no users yet. pte_c_bit_mask may want to be an intpte_t rather than uint64_t. ~Andrew
On 4/11/24 20:32, Andrew Cooper wrote: > On 10/04/2024 4:36 pm, Andrei Semenov wrote: >> diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c >> index ab92333673..a5903613f0 100644 >> --- a/xen/arch/x86/cpu/amd.c >> +++ b/xen/arch/x86/cpu/amd.c >> @@ -1030,6 +1031,54 @@ static void amd_check_erratum_1485(void) >> wrmsrl(MSR_AMD64_BP_CFG, val | chickenbit); >> } >> >> +#ifdef CONFIG_HVM >> +static void amd_enable_mem_encrypt(const struct cpuinfo_x86 *c) >> +{ >> + unsigned int eax, ebx, ecx, edx; >> + uint64_t syscfg; >> + >> + if (!smp_processor_id()) { > c == &boot_cpu_info. Agree, will fix. > >> + >> + cpuid_count(0x80000000,0,&eax, &ebx, &ecx, &edx); >> + >> + if (eax < 0x8000001f) >> + return; > Max leaf is already collected. c->extended_cpuid_level Agree, will fix. > > >> + >> + cpuid_count(0x8000001f,0,&eax, &ebx, &ecx, &edx); >> + >> + if (eax & 0x1) >> + setup_force_cpu_cap(X86_FEATURE_SME); >> + >> + if (eax & 0x2) { >> + setup_force_cpu_cap(X86_FEATURE_SEV); >> + max_sev_asid = ecx; >> + min_sev_asid = edx; >> + } >> + >> + if (eax & 0x3) >> + pte_c_bit_mask = 1UL << (ebx & 0x3f); > This is decoding the main SEV feature leaf, but outside of normal > mechanisms. > > I've got half a mind to brute-force through the remaining work to > un-screw our boot sequence order, and express this in a cpu-policy > straight away. This is wanted for the SVM leaf info too. > > Leave it with me for a bit. OK. I wait for your insights on this so. > > >> + } >> + >> + if (!(cpu_has_sme || cpu_has_sev)) >> + return; >> + >> + if (!smp_processor_id()) { >> + if (cpu_has_sev) >> + printk(XENLOG_INFO "SEV: ASID range [0x%x - 0x%x]\n", >> + min_sev_asid, max_sev_asid); > Why do we have a min as well as a max? Isn't min always 1? Well, "normally it is". But this is the part of CPUID leaf specs. Do they plan to potentially change it? No idea. > >> + } >> + >> + rdmsrl(MSR_K8_SYSCFG, syscfg); >> + >> + if (syscfg & SYSCFG_MEM_ENCRYPT) { >> + return; >> + } >> + >> + syscfg |= SYSCFG_MEM_ENCRYPT; >> + wrmsrl(MSR_K8_SYSCFG, syscfg); >> +} >> +#endif >> + >> static void cf_check init_amd(struct cpuinfo_x86 *c) >> { >> u32 l, h; >> @@ -1305,6 +1354,10 @@ static void cf_check init_amd(struct cpuinfo_x86 *c) >> check_syscfg_dram_mod_en(); >> >> amd_log_freq(c); >> + >> +#ifdef CONFIG_HVM >> + amd_enable_mem_encrypt(c); >> +#endif > I think we want to drop the CONFIG_HVM here. > > Memory encryption is an all-or-nothing thing. If it's active, it > affects all pagetables that Xen controls, even dom0's. And we likely do > want get to the point of Xen running on encrypted mappings even if dom0 > can't operate it very nicely. > > Thoughts? Basically I put CONFIG_HVM here because I also wanted to put related variables (max/min_asid) in sev.c. And sev.c is in "HVM" part of the code as SEV is only related to HVM guests. Now, basically I agree that - Xen would like potentially use encrypted memory for itself - in SME case, some encryption could be offered for non-HVM guests, so they can protect their memory (even though the key is shared and the hypervisor can read it). OK, so I will drop CONFIG_HVM and put these variables elsewhere. amd.h is probably a good candidate? >> } >> >> const struct cpu_dev __initconst_cf_clobber amd_cpu_dev = { >> diff --git a/xen/arch/x86/hvm/svm/Makefile b/xen/arch/x86/hvm/svm/Makefile >> index 760d2954da..9773d539ef 100644 >> --- a/xen/arch/x86/hvm/svm/Makefile >> +++ b/xen/arch/x86/hvm/svm/Makefile >> @@ -6,3 +6,4 @@ obj-y += nestedsvm.o >> obj-y += svm.o >> obj-y += svmdebug.o >> obj-y += vmcb.o >> +obj-y += sev.o > Please keep this sorted by object file name. Got it. Will do. > >> diff --git a/xen/arch/x86/hvm/svm/sev.c b/xen/arch/x86/hvm/svm/sev.c >> new file mode 100644 >> index 0000000000..336fad25f5 >> --- /dev/null >> +++ b/xen/arch/x86/hvm/svm/sev.c >> @@ -0,0 +1,4 @@ >> +#include <asm/sev.h> >> +uint64_t __read_mostly pte_c_bit_mask; >> +unsigned int __read_mostly min_sev_asid; >> +unsigned int __read_mostly max_sev_asid; > Several things. All new files should come with an SPDX tag. Unless you > have other constraints, GPL-2.0-only is preferred. There also wants to > be at least a oneline summary of what's going on here. Will do. > > All these variables look like they should be __ro_after_init. However, > it's rather hard to judge, given no users yet. Yes, this is not supposed to dynamically change. Will fix. > > pte_c_bit_mask may want to be an intpte_t rather than uint64_t. Agree. Will fix > > ~Andrew Andrei.
On 4/12/24 4:06 PM, Andrei Semenov wrote: > > On 4/11/24 20:32, Andrew Cooper wrote: >> On 10/04/2024 4:36 pm, Andrei Semenov wrote: >>> diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c >>> index ab92333673..a5903613f0 100644 >>> --- a/xen/arch/x86/cpu/amd.c >>> +++ b/xen/arch/x86/cpu/amd.c >>> @@ -1030,6 +1031,54 @@ static void amd_check_erratum_1485(void) >>> wrmsrl(MSR_AMD64_BP_CFG, val | chickenbit); >>> } >>> +#ifdef CONFIG_HVM >>> +static void amd_enable_mem_encrypt(const struct cpuinfo_x86 *c) >>> +{ >>> + unsigned int eax, ebx, ecx, edx; >>> + uint64_t syscfg; >>> + >>> + if (!smp_processor_id()) { >> c == &boot_cpu_info. > Agree, will fix. >> >>> + >>> + cpuid_count(0x80000000,0,&eax, &ebx, &ecx, &edx); >>> + >>> + if (eax < 0x8000001f) >>> + return; >> Max leaf is already collected. c->extended_cpuid_level > Agree, will fix. >> >> >>> + >>> + cpuid_count(0x8000001f,0,&eax, &ebx, &ecx, &edx); >>> + >>> + if (eax & 0x1) >>> + setup_force_cpu_cap(X86_FEATURE_SME); >>> + >>> + if (eax & 0x2) { >>> + setup_force_cpu_cap(X86_FEATURE_SEV); >>> + max_sev_asid = ecx; >>> + min_sev_asid = edx; >>> + } >>> + >>> + if (eax & 0x3) >>> + pte_c_bit_mask = 1UL << (ebx & 0x3f); >> This is decoding the main SEV feature leaf, but outside of normal >> mechanisms. >> >> I've got half a mind to brute-force through the remaining work to >> un-screw our boot sequence order, and express this in a cpu-policy >> straight away. This is wanted for the SVM leaf info too. >> >> Leave it with me for a bit. > OK. I wait for your insights on this so. >> >> >>> + } >>> + >>> + if (!(cpu_has_sme || cpu_has_sev)) >>> + return; >>> + >>> + if (!smp_processor_id()) { >>> + if (cpu_has_sev) >>> + printk(XENLOG_INFO "SEV: ASID range [0x%x - 0x%x]\n", >>> + min_sev_asid, max_sev_asid); >> Why do we have a min as well as a max? Isn't min always 1? In the case of SEV, it's not true. Some BIOS allow to set the min_asid. So yeah Xen will also need to adapted for the same. I've a WIP patch for allowing dynamic generation of asid in such a case. > Well, "normally it is". But this is the part of CPUID leaf specs. Do they > plan to potentially change it? > > No idea. > >> >>> + } >>> + >>> + rdmsrl(MSR_K8_SYSCFG, syscfg); >>> + >>> + if (syscfg & SYSCFG_MEM_ENCRYPT) { >>> + return; >>> + } >>> + >>> + syscfg |= SYSCFG_MEM_ENCRYPT; >>> + wrmsrl(MSR_K8_SYSCFG, syscfg); >>> +} >>> +#endif >>> + >>> static void cf_check init_amd(struct cpuinfo_x86 *c) >>> { >>> u32 l, h; >>> @@ -1305,6 +1354,10 @@ static void cf_check init_amd(struct cpuinfo_x86 *c) >>> check_syscfg_dram_mod_en(); >>> amd_log_freq(c); >>> + >>> +#ifdef CONFIG_HVM >>> + amd_enable_mem_encrypt(c); >>> +#endif >> I think we want to drop the CONFIG_HVM here. >> >> Memory encryption is an all-or-nothing thing. If it's active, it >> affects all pagetables that Xen controls, even dom0's. And we likely do >> want get to the point of Xen running on encrypted mappings even if dom0 >> can't operate it very nicely. >> >> Thoughts? > > Basically I put CONFIG_HVM here because I also wanted to put related variables > > (max/min_asid) in sev.c. And sev.c is in "HVM" part of the code as SEV > > is only related to HVM guests. Now, basically I agree that > > - Xen would like potentially use encrypted memory for itself > > - in SME case, some encryption could be offered for non-HVM guests, so they > > can protect their memory (even though the key is shared and the hypervisor can > > read it). > > OK, so I will drop CONFIG_HVM and put these variables elsewhere. amd.h is > probably > > a good candidate? > >>> } >>> const struct cpu_dev __initconst_cf_clobber amd_cpu_dev = { >>> diff --git a/xen/arch/x86/hvm/svm/Makefile b/xen/arch/x86/hvm/svm/Makefile >>> index 760d2954da..9773d539ef 100644 >>> --- a/xen/arch/x86/hvm/svm/Makefile >>> +++ b/xen/arch/x86/hvm/svm/Makefile >>> @@ -6,3 +6,4 @@ obj-y += nestedsvm.o >>> obj-y += svm.o >>> obj-y += svmdebug.o >>> obj-y += vmcb.o >>> +obj-y += sev.o >> Please keep this sorted by object file name. > Got it. Will do. >> >>> diff --git a/xen/arch/x86/hvm/svm/sev.c b/xen/arch/x86/hvm/svm/sev.c >>> new file mode 100644 >>> index 0000000000..336fad25f5 >>> --- /dev/null >>> +++ b/xen/arch/x86/hvm/svm/sev.c >>> @@ -0,0 +1,4 @@ >>> +#include <asm/sev.h> >>> +uint64_t __read_mostly pte_c_bit_mask; >>> +unsigned int __read_mostly min_sev_asid; >>> +unsigned int __read_mostly max_sev_asid; >> Several things. All new files should come with an SPDX tag. Unless you >> have other constraints, GPL-2.0-only is preferred. There also wants to >> be at least a oneline summary of what's going on here. > Will do. >> >> All these variables look like they should be __ro_after_init. However, >> it's rather hard to judge, given no users yet. > Yes, this is not supposed to dynamically change. Will fix. >> >> pte_c_bit_mask may want to be an intpte_t rather than uint64_t. > > Agree. Will fix > >> >> ~Andrew > Andrei. > >
On 12/04/2024 3:38 pm, Vaishali Thakkar wrote: > On 4/12/24 4:06 PM, Andrei Semenov wrote: >> On 4/11/24 20:32, Andrew Cooper wrote: >>> On 10/04/2024 4:36 pm, Andrei Semenov wrote: >>>> + } >>>> + >>>> + if (!(cpu_has_sme || cpu_has_sev)) >>>> + return; >>>> + >>>> + if (!smp_processor_id()) { >>>> + if (cpu_has_sev) >>>> + printk(XENLOG_INFO "SEV: ASID range [0x%x - 0x%x]\n", >>>> + min_sev_asid, max_sev_asid); >>> Why do we have a min as well as a max? Isn't min always 1? > > In the case of SEV, it's not true. Some BIOS allow to set the > min_asid. So yeah Xen will also need to adapted for the same. > I've a WIP patch for allowing dynamic generation of asid in such > a case. I also got an answer to this out of a contact of mine at AMD. The ASID space is divided, 1->$N for SEV-ES/SNP guest, and $N->$M for SEV guests. It is a security issue to start a guest as ES/SNP, then "migrate" it to being SEV-only, so the different types are tracked explicitly. ~Andrew
On 4/12/24 5:07 PM, Andrew Cooper wrote: > On 12/04/2024 3:38 pm, Vaishali Thakkar wrote: >> On 4/12/24 4:06 PM, Andrei Semenov wrote: >>> On 4/11/24 20:32, Andrew Cooper wrote: >>>> On 10/04/2024 4:36 pm, Andrei Semenov wrote: >>>>> + } >>>>> + >>>>> + if (!(cpu_has_sme || cpu_has_sev)) >>>>> + return; >>>>> + >>>>> + if (!smp_processor_id()) { >>>>> + if (cpu_has_sev) >>>>> + printk(XENLOG_INFO "SEV: ASID range [0x%x - 0x%x]\n", >>>>> + min_sev_asid, max_sev_asid); >>>> Why do we have a min as well as a max? Isn't min always 1? >> >> In the case of SEV, it's not true. Some BIOS allow to set the >> min_asid. So yeah Xen will also need to adapted for the same. >> I've a WIP patch for allowing dynamic generation of asid in such >> a case. > > I also got an answer to this out of a contact of mine at AMD. > > The ASID space is divided, 1->$N for SEV-ES/SNP guest, and $N->$M for > SEV guests. > > It is a security issue to start a guest as ES/SNP, then "migrate" it to > being SEV-only, so the different types are tracked explicitly. Aha, yeah that seems like a better explanation. Thanks for checking with the AMD person. > ~Andrew
On 10.04.2024 17:36, Andrei Semenov wrote: > Signed-off-by: Andrei Semenov <andrei.semenov@vates.fr> A couple more nits on top of what Andrew said. First: Please no patches (which aren't blindingly trivial) without description. > @@ -1030,6 +1031,54 @@ static void amd_check_erratum_1485(void) > wrmsrl(MSR_AMD64_BP_CFG, val | chickenbit); > } > > +#ifdef CONFIG_HVM > +static void amd_enable_mem_encrypt(const struct cpuinfo_x86 *c) > +{ > + unsigned int eax, ebx, ecx, edx; > + uint64_t syscfg; > + > + if (!smp_processor_id()) { > + > + cpuid_count(0x80000000,0,&eax, &ebx, &ecx, &edx); No blank line above here please. > + if (eax < 0x8000001f) > + return; > + > + cpuid_count(0x8000001f,0,&eax, &ebx, &ecx, &edx); > + > + if (eax & 0x1) > + setup_force_cpu_cap(X86_FEATURE_SME); > + > + if (eax & 0x2) { > + setup_force_cpu_cap(X86_FEATURE_SEV); I guess this goes along with what Andrew said: Using synthetic features here looks suspicious. These want to be recorded as an ordinary leaf. > + max_sev_asid = ecx; > + min_sev_asid = edx; > + } > + > + if (eax & 0x3) > + pte_c_bit_mask = 1UL << (ebx & 0x3f); > + } > + > + if (!(cpu_has_sme || cpu_has_sev)) > + return; > + > + if (!smp_processor_id()) { > + if (cpu_has_sev) Two if()-s like these want folding, unless it is made clear that very soon (see above as to the missing description) further content is going to appear inside the outer one. > + printk(XENLOG_INFO "SEV: ASID range [0x%x - 0x%x]\n", %#x is preferred over 0x%x. > + min_sev_asid, max_sev_asid); > + } > + > + rdmsrl(MSR_K8_SYSCFG, syscfg); > + > + if (syscfg & SYSCFG_MEM_ENCRYPT) { > + return; > + } No need for braces in cases like this one. Jan
Thank you Jan for your feedback. From what I understood, Andrew is planning some "ground" changes on CPUID leaf discovery, so it probably will move here. I'll wait for his changes to apply your and Andrew's remarks and to re-post the patches. Andrei. On 4/18/24 10:13, Jan Beulich wrote: > On 10.04.2024 17:36, Andrei Semenov wrote: >> Signed-off-by: Andrei Semenov <andrei.semenov@vates.fr> > A couple more nits on top of what Andrew said. First: Please no patches > (which aren't blindingly trivial) without description. > >> @@ -1030,6 +1031,54 @@ static void amd_check_erratum_1485(void) >> wrmsrl(MSR_AMD64_BP_CFG, val | chickenbit); >> } >> >> +#ifdef CONFIG_HVM >> +static void amd_enable_mem_encrypt(const struct cpuinfo_x86 *c) >> +{ >> + unsigned int eax, ebx, ecx, edx; >> + uint64_t syscfg; >> + >> + if (!smp_processor_id()) { >> + >> + cpuid_count(0x80000000,0,&eax, &ebx, &ecx, &edx); > No blank line above here please. > >> + if (eax < 0x8000001f) >> + return; >> + >> + cpuid_count(0x8000001f,0,&eax, &ebx, &ecx, &edx); >> + >> + if (eax & 0x1) >> + setup_force_cpu_cap(X86_FEATURE_SME); >> + >> + if (eax & 0x2) { >> + setup_force_cpu_cap(X86_FEATURE_SEV); > I guess this goes along with what Andrew said: Using synthetic features here > looks suspicious. These want to be recorded as an ordinary leaf. > >> + max_sev_asid = ecx; >> + min_sev_asid = edx; >> + } >> + >> + if (eax & 0x3) >> + pte_c_bit_mask = 1UL << (ebx & 0x3f); >> + } >> + >> + if (!(cpu_has_sme || cpu_has_sev)) >> + return; >> + >> + if (!smp_processor_id()) { >> + if (cpu_has_sev) > Two if()-s like these want folding, unless it is made clear that very > soon (see above as to the missing description) further content is going > to appear inside the outer one. > >> + printk(XENLOG_INFO "SEV: ASID range [0x%x - 0x%x]\n", > %#x is preferred over 0x%x. > >> + min_sev_asid, max_sev_asid); >> + } >> + >> + rdmsrl(MSR_K8_SYSCFG, syscfg); >> + >> + if (syscfg & SYSCFG_MEM_ENCRYPT) { >> + return; >> + } > No need for braces in cases like this one. > > Jan
diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c index ab92333673..a5903613f0 100644 --- a/xen/arch/x86/cpu/amd.c +++ b/xen/arch/x86/cpu/amd.c @@ -16,6 +16,7 @@ #include <asm/acpi.h> #include <asm/apic.h> #include <asm/microcode.h> +#include <asm/sev.h> #include "cpu.h" @@ -1030,6 +1031,54 @@ static void amd_check_erratum_1485(void) wrmsrl(MSR_AMD64_BP_CFG, val | chickenbit); } +#ifdef CONFIG_HVM +static void amd_enable_mem_encrypt(const struct cpuinfo_x86 *c) +{ + unsigned int eax, ebx, ecx, edx; + uint64_t syscfg; + + if (!smp_processor_id()) { + + cpuid_count(0x80000000,0,&eax, &ebx, &ecx, &edx); + + if (eax < 0x8000001f) + return; + + cpuid_count(0x8000001f,0,&eax, &ebx, &ecx, &edx); + + if (eax & 0x1) + setup_force_cpu_cap(X86_FEATURE_SME); + + if (eax & 0x2) { + setup_force_cpu_cap(X86_FEATURE_SEV); + max_sev_asid = ecx; + min_sev_asid = edx; + } + + if (eax & 0x3) + pte_c_bit_mask = 1UL << (ebx & 0x3f); + } + + if (!(cpu_has_sme || cpu_has_sev)) + return; + + if (!smp_processor_id()) { + if (cpu_has_sev) + printk(XENLOG_INFO "SEV: ASID range [0x%x - 0x%x]\n", + min_sev_asid, max_sev_asid); + } + + rdmsrl(MSR_K8_SYSCFG, syscfg); + + if (syscfg & SYSCFG_MEM_ENCRYPT) { + return; + } + + syscfg |= SYSCFG_MEM_ENCRYPT; + wrmsrl(MSR_K8_SYSCFG, syscfg); +} +#endif + static void cf_check init_amd(struct cpuinfo_x86 *c) { u32 l, h; @@ -1305,6 +1354,10 @@ static void cf_check init_amd(struct cpuinfo_x86 *c) check_syscfg_dram_mod_en(); amd_log_freq(c); + +#ifdef CONFIG_HVM + amd_enable_mem_encrypt(c); +#endif } const struct cpu_dev __initconst_cf_clobber amd_cpu_dev = { diff --git a/xen/arch/x86/hvm/svm/Makefile b/xen/arch/x86/hvm/svm/Makefile index 760d2954da..9773d539ef 100644 --- a/xen/arch/x86/hvm/svm/Makefile +++ b/xen/arch/x86/hvm/svm/Makefile @@ -6,3 +6,4 @@ obj-y += nestedsvm.o obj-y += svm.o obj-y += svmdebug.o obj-y += vmcb.o +obj-y += sev.o diff --git a/xen/arch/x86/hvm/svm/sev.c b/xen/arch/x86/hvm/svm/sev.c new file mode 100644 index 0000000000..336fad25f5 --- /dev/null +++ b/xen/arch/x86/hvm/svm/sev.c @@ -0,0 +1,4 @@ +#include <asm/sev.h> +uint64_t __read_mostly pte_c_bit_mask; +unsigned int __read_mostly min_sev_asid; +unsigned int __read_mostly max_sev_asid; diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h index 743f11f989..a41374d0b7 100644 --- a/xen/arch/x86/include/asm/cpufeature.h +++ b/xen/arch/x86/include/asm/cpufeature.h @@ -231,6 +231,9 @@ static inline bool boot_cpu_has(unsigned int feat) #define cpu_has_msr_tsc_aux (cpu_has_rdtscp || cpu_has_rdpid) +#define cpu_has_sme boot_cpu_has(X86_FEATURE_SME) +#define cpu_has_sev boot_cpu_has(X86_FEATURE_SEV) + /* Bugs. */ #define cpu_bug_fpu_ptrs boot_cpu_has(X86_BUG_FPU_PTRS) #define cpu_bug_null_seg boot_cpu_has(X86_BUG_NULL_SEG) diff --git a/xen/arch/x86/include/asm/cpufeatures.h b/xen/arch/x86/include/asm/cpufeatures.h index ba3df174b7..9b67ea2427 100644 --- a/xen/arch/x86/include/asm/cpufeatures.h +++ b/xen/arch/x86/include/asm/cpufeatures.h @@ -42,6 +42,8 @@ XEN_CPUFEATURE(XEN_SHSTK, X86_SYNTH(26)) /* Xen uses CET Shadow Stacks * XEN_CPUFEATURE(XEN_IBT, X86_SYNTH(27)) /* Xen uses CET Indirect Branch Tracking */ XEN_CPUFEATURE(IBPB_ENTRY_PV, X86_SYNTH(28)) /* MSR_PRED_CMD used by Xen for PV */ XEN_CPUFEATURE(IBPB_ENTRY_HVM, X86_SYNTH(29)) /* MSR_PRED_CMD used by Xen for HVM */ +XEN_CPUFEATURE(SME, X86_SYNTH(30)) /* AMD Secure Memory Encrypion */ +XEN_CPUFEATURE(SEV, X86_SYNTH(31)) /* AMD Secure Encryption Virtualization */ /* Bug words follow the synthetic words. */ #define X86_NR_BUG 1 diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h index 92dd9fa496..318e8ca0c0 100644 --- a/xen/arch/x86/include/asm/msr-index.h +++ b/xen/arch/x86/include/asm/msr-index.h @@ -221,6 +221,7 @@ #define SYSCFG_MTRR_VAR_DRAM_EN (_AC(1, ULL) << 20) #define SYSCFG_MTRR_TOM2_EN (_AC(1, ULL) << 21) #define SYSCFG_TOM2_FORCE_WB (_AC(1, ULL) << 22) +#define SYSCFG_MEM_ENCRYPT (_AC(1, ULL) << 23) #define MSR_K8_IORR_BASE0 _AC(0xc0010016, U) #define MSR_K8_IORR_MASK0 _AC(0xc0010017, U) diff --git a/xen/arch/x86/include/asm/sev.h b/xen/arch/x86/include/asm/sev.h new file mode 100644 index 0000000000..7bec230c7b --- /dev/null +++ b/xen/arch/x86/include/asm/sev.h @@ -0,0 +1,11 @@ +#ifndef __XEN_SEV_H__ +#define __XEN_SEV_H__ + +#include <xen/types.h> +#include <asm/cache.h> + +extern uint64_t __read_mostly pte_c_bit_mask; +extern unsigned int __read_mostly min_sev_asid; +extern unsigned int __read_mostly max_sev_asid; + +#endif
Signed-off-by: Andrei Semenov <andrei.semenov@vates.fr> --- xen/arch/x86/cpu/amd.c | 53 ++++++++++++++++++++++++++ xen/arch/x86/hvm/svm/Makefile | 1 + xen/arch/x86/hvm/svm/sev.c | 4 ++ xen/arch/x86/include/asm/cpufeature.h | 3 ++ xen/arch/x86/include/asm/cpufeatures.h | 2 + xen/arch/x86/include/asm/msr-index.h | 1 + xen/arch/x86/include/asm/sev.h | 11 ++++++ 7 files changed, 75 insertions(+) create mode 100644 xen/arch/x86/hvm/svm/sev.c create mode 100644 xen/arch/x86/include/asm/sev.h