2022-04-12 21:17:34

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH v2 07/12] KVM: SVM: Adding support for configuring x2APIC MSRs interception

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 list of MSR interception state
in the svm_direct_access_msrs array. Therefore, extends the array to
include x2APIC MSRs.

Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
arch/x86/kvm/svm/svm.c | 29 ++++++++++++++++++++++++++++-
arch/x86/kvm/svm/svm.h | 5 +++--
2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 5ec770a1b4e8..c85663b62d4e 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -76,7 +76,7 @@ static uint64_t osvw_len = 4, osvw_status;

static DEFINE_PER_CPU(u64, current_tsc_ratio);

-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] = {
@@ -774,6 +774,32 @@ static void add_msr_offset(u32 offset)
BUG();
}

+static void init_direct_access_msrs(void)
+{
+ int i, j;
+
+ /* Find first MSR_INVALID */
+ for (i = 0; i < MAX_DIRECT_ACCESS_MSRS; i++) {
+ if (direct_access_msrs[i].index == MSR_INVALID)
+ break;
+ }
+ BUG_ON(i >= MAX_DIRECT_ACCESS_MSRS);
+
+ /*
+ * Initialize direct_access_msrs entries to intercept X2APIC MSRs
+ * (range 0x800 to 0x8ff)
+ */
+ for (j = 0; j < 0x100; j++) {
+ direct_access_msrs[i + j].index = APIC_BASE_MSR + j;
+ direct_access_msrs[i + j].always = false;
+ }
+ BUG_ON(i + j >= MAX_DIRECT_ACCESS_MSRS);
+
+ /* Initialize last entry */
+ direct_access_msrs[i + j].index = MSR_INVALID;
+ direct_access_msrs[i + j].always = true;
+}
+
static void init_msrpm_offsets(void)
{
int i;
@@ -4739,6 +4765,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_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 c44326eeb3f2..e340c86941be 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -29,8 +29,9 @@
#define IOPM_SIZE PAGE_SIZE * 3
#define MSRPM_SIZE PAGE_SIZE * 2

-#define MAX_DIRECT_ACCESS_MSRS 20
-#define MSRPM_OFFSETS 16
+#define MAX_DIRECT_ACCESS_MSRS (20 + 0x100)
+#define MSRPM_OFFSETS 30
+
extern u32 msrpm_offsets[MSRPM_OFFSETS] __read_mostly;
extern bool npt_enabled;
extern bool intercept_smi;
--
2.25.1


2022-04-19 08:18:15

by Maxim Levitsky

[permalink] [raw]
Subject: Re: [PATCH v2 07/12] KVM: SVM: Adding support for configuring x2APIC MSRs interception

On Tue, 2022-04-12 at 06:58 -0500, 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 list of MSR interception state
> in the svm_direct_access_msrs array. Therefore, extends the array to
> include x2APIC MSRs.
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
> arch/x86/kvm/svm/svm.c | 29 ++++++++++++++++++++++++++++-
> arch/x86/kvm/svm/svm.h | 5 +++--
> 2 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 5ec770a1b4e8..c85663b62d4e 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -76,7 +76,7 @@ static uint64_t osvw_len = 4, osvw_status;
>
> static DEFINE_PER_CPU(u64, current_tsc_ratio);
>
> -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] = {
> @@ -774,6 +774,32 @@ static void add_msr_offset(u32 offset)
> BUG();
> }
>
> +static void init_direct_access_msrs(void)
> +{
> + int i, j;
> +
> + /* Find first MSR_INVALID */
> + for (i = 0; i < MAX_DIRECT_ACCESS_MSRS; i++) {
> + if (direct_access_msrs[i].index == MSR_INVALID)
> + break;
> + }
> + BUG_ON(i >= MAX_DIRECT_ACCESS_MSRS);
> +
> + /*
> + * Initialize direct_access_msrs entries to intercept X2APIC MSRs
> + * (range 0x800 to 0x8ff)
> + */
> + for (j = 0; j < 0x100; j++) {
> + direct_access_msrs[i + j].index = APIC_BASE_MSR + j;
> + direct_access_msrs[i + j].always = false;
> + }

That looks *much cleaner* code wise even though it is slower
because now we have 256 more msrs in this list.

So the best of the two worlds I think would be to add only APIC msrs that AVIC
actually handles to the list.

SDM has a table of these registers in

"15.29.3.1 Virtual APIC Register Accesses"

I would add the registers that are either read/write allowed
or at least cause traps.

Besides this, the patch looks fine.

Best regards,
Maxim Levitsky


> + BUG_ON(i + j >= MAX_DIRECT_ACCESS_MSRS);
> +
> + /* Initialize last entry */
> + direct_access_msrs[i + j].index = MSR_INVALID;
> + direct_access_msrs[i + j].always = true;
> +}
> +
> static void init_msrpm_offsets(void)
> {
> int i;
> @@ -4739,6 +4765,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_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 c44326eeb3f2..e340c86941be 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -29,8 +29,9 @@
> #define IOPM_SIZE PAGE_SIZE * 3
> #define MSRPM_SIZE PAGE_SIZE * 2
>
> -#define MAX_DIRECT_ACCESS_MSRS 20
> -#define MSRPM_OFFSETS 16
> +#define MAX_DIRECT_ACCESS_MSRS (20 + 0x100)
> +#define MSRPM_OFFSETS 30
> +
> extern u32 msrpm_offsets[MSRPM_OFFSETS] __read_mostly;
> extern bool npt_enabled;
> extern bool intercept_smi;