On Sun, 2022-02-20 at 20:19 -0600, Suravee Suthikulpanit wrote: > When enabling x2APIC virtualization (x2AVIC), the interception of > x2APIC MSRs must be disabled to let the hardware virtualize guest > MSR accesses. > > Current implementation keeps track of MSR interception state > for generic MSRs in the svm_direct_access_msrs array. > For x2APIC MSRs, introduce direct_access_x2apic_msrs array. > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx> > --- > arch/x86/kvm/svm/svm.c | 67 +++++++++++++++++++++++++++++++----------- > arch/x86/kvm/svm/svm.h | 7 +++++ > 2 files changed, 57 insertions(+), 17 deletions(-) > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index 4e6dc1feeac7..afca26aa1f40 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -89,7 +89,7 @@ static uint64_t osvw_len = 4, osvw_status; > static DEFINE_PER_CPU(u64, current_tsc_ratio); > #define TSC_RATIO_DEFAULT 0x0100000000ULL > > -static const struct svm_direct_access_msrs { > +static struct svm_direct_access_msrs { > u32 index; /* Index of the MSR */ > bool always; /* True if intercept is initially cleared */ > } direct_access_msrs[MAX_DIRECT_ACCESS_MSRS] = { > @@ -117,6 +117,9 @@ static const struct svm_direct_access_msrs { > { .index = MSR_INVALID, .always = false }, > }; > > +static struct svm_direct_access_msrs > +direct_access_x2apic_msrs[NUM_DIRECT_ACCESS_X2APIC_MSRS + 1]; > + > /* > * These 2 parameters are used to config the controls for Pause-Loop Exiting: > * pause_filter_count: On processors that support Pause filtering(indicated > @@ -609,41 +612,42 @@ static int svm_cpu_init(int cpu) > > } > > -static int direct_access_msr_slot(u32 msr) > +static int direct_access_msr_slot(u32 msr, struct svm_direct_access_msrs *msrs) > { > u32 i; > > - for (i = 0; direct_access_msrs[i].index != MSR_INVALID; i++) > - if (direct_access_msrs[i].index == msr) > + for (i = 0; msrs[i].index != MSR_INVALID; i++) > + if (msrs[i].index == msr) > return i; > > return -ENOENT; > } > > -static void set_shadow_msr_intercept(struct kvm_vcpu *vcpu, u32 msr, int read, > - int write) > +static void set_shadow_msr_intercept(struct kvm_vcpu *vcpu, > + struct svm_direct_access_msrs *msrs, u32 msr, > + int read, void *read_bits, > + int write, void *write_bits) > { > - struct vcpu_svm *svm = to_svm(vcpu); > - int slot = direct_access_msr_slot(msr); > + int slot = direct_access_msr_slot(msr, msrs); > > if (slot == -ENOENT) > return; > > /* Set the shadow bitmaps to the desired intercept states */ > if (read) > - set_bit(slot, svm->shadow_msr_intercept.read); > + set_bit(slot, read_bits); > else > - clear_bit(slot, svm->shadow_msr_intercept.read); > + clear_bit(slot, read_bits); > > if (write) > - set_bit(slot, svm->shadow_msr_intercept.write); > + set_bit(slot, write_bits); > else > - clear_bit(slot, svm->shadow_msr_intercept.write); > + clear_bit(slot, write_bits); > } > > -static bool valid_msr_intercept(u32 index) > +static bool valid_msr_intercept(u32 index, struct svm_direct_access_msrs *msrs) > { > - return direct_access_msr_slot(index) != -ENOENT; > + return direct_access_msr_slot(index, msrs) != -ENOENT; > } > > static bool msr_write_intercepted(struct kvm_vcpu *vcpu, u32 msr) > @@ -674,9 +678,12 @@ static void set_msr_interception_bitmap(struct kvm_vcpu *vcpu, u32 *msrpm, > > /* > * If this warning triggers extend the direct_access_msrs list at the > - * beginning of the file > + * beginning of the file. The direct_access_x2apic_msrs is only for > + * x2apic MSRs. > */ > - WARN_ON(!valid_msr_intercept(msr)); > + WARN_ON(!valid_msr_intercept(msr, direct_access_msrs) && > + (boot_cpu_has(X86_FEATURE_X2AVIC) && > + !valid_msr_intercept(msr, direct_access_x2apic_msrs))); > > /* Enforce non allowed MSRs to trap */ > if (read && !kvm_msr_allowed(vcpu, msr, KVM_MSR_FILTER_READ)) > @@ -704,7 +711,16 @@ static void set_msr_interception_bitmap(struct kvm_vcpu *vcpu, u32 *msrpm, > void set_msr_interception(struct kvm_vcpu *vcpu, u32 *msrpm, u32 msr, > int read, int write) > { > - set_shadow_msr_intercept(vcpu, msr, read, write); > + struct vcpu_svm *svm = to_svm(vcpu); > + > + if (msr < 0x800 || msr > 0x8ff) > + set_shadow_msr_intercept(vcpu, direct_access_msrs, msr, > + read, svm->shadow_msr_intercept.read, > + write, svm->shadow_msr_intercept.write); > + else > + set_shadow_msr_intercept(vcpu, direct_access_x2apic_msrs, msr, > + read, svm->shadow_x2apic_msr_intercept.read, > + write, svm->shadow_x2apic_msr_intercept.write); > set_msr_interception_bitmap(vcpu, msrpm, msr, read, write); > } > > @@ -786,6 +802,22 @@ static void add_msr_offset(u32 offset) > BUG(); > } > > +static void init_direct_access_x2apic_msrs(void) > +{ > + int i; > + > + /* Initialize x2APIC direct_access_x2apic_msrs entries */ > + for (i = 0; i < NUM_DIRECT_ACCESS_X2APIC_MSRS; i++) { > + direct_access_x2apic_msrs[i].index = boot_cpu_has(X86_FEATURE_X2AVIC) ? > + (0x800 + i) : MSR_INVALID; > + direct_access_x2apic_msrs[i].always = false; > + } > + > + /* Initialize last entry */ > + direct_access_x2apic_msrs[i].index = MSR_INVALID; > + direct_access_x2apic_msrs[i].always = false; > +} > + > static void init_msrpm_offsets(void) > { > int i; > @@ -4752,6 +4784,7 @@ static __init int svm_hardware_setup(void) > memset(iopm_va, 0xff, PAGE_SIZE * (1 << order)); > iopm_base = page_to_pfn(iopm_pages) << PAGE_SHIFT; > > + init_direct_access_x2apic_msrs(); > init_msrpm_offsets(); > > supported_xcr0 &= ~(XFEATURE_MASK_BNDREGS | XFEATURE_MASK_BNDCSR); > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h > index bfbebb933da2..41514df5107e 100644 > --- a/arch/x86/kvm/svm/svm.h > +++ b/arch/x86/kvm/svm/svm.h > @@ -29,6 +29,8 @@ > > #define MAX_DIRECT_ACCESS_MSRS 20 > #define MSRPM_OFFSETS 16 > +#define NUM_DIRECT_ACCESS_X2APIC_MSRS 0x100 > + > extern u32 msrpm_offsets[MSRPM_OFFSETS] __read_mostly; > extern bool npt_enabled; > extern bool intercept_smi; > @@ -242,6 +244,11 @@ struct vcpu_svm { > DECLARE_BITMAP(write, MAX_DIRECT_ACCESS_MSRS); > } shadow_msr_intercept; > > + struct { > + DECLARE_BITMAP(read, NUM_DIRECT_ACCESS_X2APIC_MSRS); > + DECLARE_BITMAP(write, NUM_DIRECT_ACCESS_X2APIC_MSRS); > + } shadow_x2apic_msr_intercept; > + > struct vcpu_sev_es_state sev_es; > > bool guest_state_loaded; I only gave this a cursory look, the whole thing is a bit ugly (not your fault), I feel like it should be refactored a bit. Best regards, Maxim Levitsky