| Message ID | 20170724200303.12197-6-brijesh.singh@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 24, 2017 at 03:02:42PM -0500, Brijesh Singh wrote: > SEV-enabled guest must use ASIDs from the defined subset, while non-SEV "A SEV-enabled ..." > guests can use the remaining ASID range. The range of ASID allowed for > SEV-enabled guest is from 1 to a maximum value defined via CPUID > Fn8000_001f[ECX]. I'd rewrite that to: "The range of allowed SEV guest ASIDs is [1 - CPUID_8000_001F[ECX][31:0]]". > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > arch/x86/kvm/svm.c | 23 ++++++++++++++++++++++- > 1 file changed, 22 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 46f41bb..06bd902 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -319,6 +319,9 @@ enum { > > #define VMCB_AVIC_APIC_BAR_MASK 0xFFFFFFFFFF000ULL > > +/* Secure Encrypted Virtualization */ If anything, this comment should explain what that variable is. Basically the comment you have in sev_hardware_setup() now. > +static unsigned int max_sev_asid; > + > static inline void mark_all_dirty(struct vmcb *vmcb) > { > vmcb->control.clean = 0; > @@ -769,7 +772,7 @@ static int svm_hardware_enable(void) > sd->asid_generation = 1; > sd->max_asid = cpuid_ebx(SVM_CPUID_FUNC) - 1; > sd->next_asid = sd->max_asid + 1; > - sd->min_asid = 1; > + sd->min_asid = max_sev_asid + 1; > > gdt = get_current_gdt_rw(); > sd->tss_desc = (struct kvm_ldttss_desc *)(gdt + GDT_ENTRY_TSS); > @@ -1033,6 +1036,21 @@ static int avic_ga_log_notifier(u32 ga_tag) > return 0; > } > > +static __init void sev_hardware_setup(void) > +{ > + int nguests; > + > + /* > + * Get maximum number of encrypted guest supported: Fn8001_001F[ECX] > + * Bit 31:0: Number of supported guest > + */ > + nguests = cpuid_ecx(0x8000001F); > + if (!nguests) > + return; > + > + max_sev_asid = nguests; > +} max_sev_asid is static and it is already initialized to 0 and thus this function can be simplified to: /* * Get maximum number of encrypted guest supported: Fn8001_001F[ECX]. * [31:0]: Number of supported guests. */ static __init void sev_hardware_setup(void) { max_sev_asid = cpuid_ecx(0x8000001F); }
On 09/12/2017 03:04 PM, Borislav Petkov wrote: ... >> SEV-enabled guest is from 1 to a maximum value defined via CPUID >> Fn8000_001f[ECX]. > > I'd rewrite that to: > > "The range of allowed SEV guest ASIDs is [1 - CPUID_8000_001F[ECX][31:0]]". > thanks, will do. ... >> >> +/* Secure Encrypted Virtualization */ > > If anything, this comment should explain what that variable is. > Basically the comment you have in sev_hardware_setup() now. > Will add more comments. ... > > max_sev_asid is static and it is already initialized to 0 and thus this > function can be simplified to: > > /* > * Get maximum number of encrypted guest supported: Fn8001_001F[ECX]. > * [31:0]: Number of supported guests. > */ > static __init void sev_hardware_setup(void) > { > max_sev_asid = cpuid_ecx(0x8000001F); > } > Agreed, I will improve it. thanks
On Tue, Sep 12, 2017 at 03:24:05PM -0500, Brijesh Singh wrote: > > If anything, this comment should explain what that variable is. > > Basically the comment you have in sev_hardware_setup() now. > > > > Will add more comments. I didn't mean that - I meant that /* Secure Encrypted Virtualization */ as a comment doesn't fit over the variable max_sev_asid. If anything, it should explain the variable or not be there at all.
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 46f41bb..06bd902 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -319,6 +319,9 @@ enum { #define VMCB_AVIC_APIC_BAR_MASK 0xFFFFFFFFFF000ULL +/* Secure Encrypted Virtualization */ +static unsigned int max_sev_asid; + static inline void mark_all_dirty(struct vmcb *vmcb) { vmcb->control.clean = 0; @@ -769,7 +772,7 @@ static int svm_hardware_enable(void) sd->asid_generation = 1; sd->max_asid = cpuid_ebx(SVM_CPUID_FUNC) - 1; sd->next_asid = sd->max_asid + 1; - sd->min_asid = 1; + sd->min_asid = max_sev_asid + 1; gdt = get_current_gdt_rw(); sd->tss_desc = (struct kvm_ldttss_desc *)(gdt + GDT_ENTRY_TSS); @@ -1033,6 +1036,21 @@ static int avic_ga_log_notifier(u32 ga_tag) return 0; } +static __init void sev_hardware_setup(void) +{ + int nguests; + + /* + * Get maximum number of encrypted guest supported: Fn8001_001F[ECX] + * Bit 31:0: Number of supported guest + */ + nguests = cpuid_ecx(0x8000001F); + if (!nguests) + return; + + max_sev_asid = nguests; +} + static __init int svm_hardware_setup(void) { int cpu; @@ -1063,6 +1081,9 @@ static __init int svm_hardware_setup(void) kvm_tsc_scaling_ratio_frac_bits = 32; } + if (boot_cpu_has(X86_FEATURE_SEV)) + sev_hardware_setup(); + if (nested) { printk(KERN_INFO "kvm: Nested Virtualization enabled\n"); kvm_enable_efer_bits(EFER_SVME | EFER_LMSLE);
SEV-enabled guest must use ASIDs from the defined subset, while non-SEV guests can use the remaining ASID range. The range of ASID allowed for SEV-enabled guest is from 1 to a maximum value defined via CPUID Fn8000_001f[ECX]. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- arch/x86/kvm/svm.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)