| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 24 Feb 2022 21:51:51 +0200
From: Maxim Levitsky <mlevitsk@...hat.com>
To: Suravee Suthikulpanit <suravee.suthikulpanit@....com>,
linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Cc: pbonzini@...hat.com, seanjc@...gle.com, joro@...tes.org,
jon.grimm@....com, wei.huang2@....com, terry.bowman@....com
Subject: Re: [RFC PATCH 10/13] KVM: SVM: Adding support for configuring
x2APIC MSRs interception
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@....com>
> ---
> 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
Powered by blists - more mailing lists