[PATCH 1/3] HV: enable #GP for UC lock #gp


Tao, Yuhong
 

From: Tao Yuhong <yuhong.tao@intel.com>

For an atomic operation using bus locking, it would generate LOCK# bus
signal, if it has Non-WB memory operand. This is an UC lock.
If MSR_IA32_CORE_CAPABILITIES[bit4] is 1, then CPU support rising #GP
for instructions which cause UC lock. This feature is controlled by
MSR_TEST_CTL[bit28].
Enable this feature if supported.

Signed-off-by: Tao Yuhong <yuhong.tao@intel.com>
---
hypervisor/arch/x86/Kconfig | 7 +++++++
hypervisor/arch/x86/cpu.c | 18 ++++++++++++++++--
hypervisor/arch/x86/cpu_caps.c | 2 +-
hypervisor/arch/x86/guest/vmsr.c | 4 ++--
hypervisor/include/arch/x86/asm/cpu_caps.h | 4 ++++
hypervisor/include/arch/x86/asm/msr.h | 5 +++++
6 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/hypervisor/arch/x86/Kconfig b/hypervisor/arch/x86/Kconfig
index ded7826b1..a8c4162bd 100644
--- a/hypervisor/arch/x86/Kconfig
+++ b/hypervisor/arch/x86/Kconfig
@@ -343,6 +343,13 @@ config ENFORCE_TURNOFF_AC
If CPU has #AC for split-locked access, HV enable it and VMs can't disable.
Set this to enforce turn off that #AC, for community developer only.

+config ENFORCE_TURNOFF_GP
+ bool "Force to disable #GP for UC lock"
+ default n
+ help
+ If CPU has #GP for UC lock, HV enable it and VMs can't disable.
+ Set this to enforce turn off that #GP, for community developer only.
+
config IVSHMEM_ENABLED
bool "Enable ivshmem inter-vm communication based on hypervisor shared memory"
default n
diff --git a/hypervisor/arch/x86/cpu.c b/hypervisor/arch/x86/cpu.c
index 8cdef8a3e..9419f31ad 100644
--- a/hypervisor/arch/x86/cpu.c
+++ b/hypervisor/arch/x86/cpu.c
@@ -113,14 +113,27 @@ static void enable_ac_for_splitlock(void)
#ifndef CONFIG_ENFORCE_TURNOFF_AC
uint64_t test_ctl;

- if (has_core_cap(1U << 5U)) {
+ if (has_core_cap(CORE_CAP_SPLIT_LOCK)) {
test_ctl = msr_read(MSR_TEST_CTL);
- test_ctl |= (1U << 29U);
+ test_ctl |= MSR_TEST_CTL_AC_SPLITLOCK;
msr_write(MSR_TEST_CTL, test_ctl);
}
#endif /*CONFIG_ENFORCE_TURNOFF_AC*/
}

+static void enable_gp_for_uclock(void)
+{
+#ifndef CONFIG_ENFORCE_TURNOFF_GP
+ uint64_t test_ctl;
+
+ if (has_core_cap(CORE_CAP_UC_LOCK)) {
+ test_ctl = msr_read(MSR_TEST_CTL);
+ test_ctl |= MSR_TEST_CTL_GP_UCLOCK;
+ msr_write(MSR_TEST_CTL, test_ctl);
+ }
+#endif /*CONFIG_ENFORCE_TURNOFF_GP*/
+}
+
void init_pcpu_pre(bool is_bsp)
{
uint16_t pcpu_id;
@@ -210,6 +223,7 @@ void init_pcpu_post(uint16_t pcpu_id)
load_gdtr_and_tr();

enable_ac_for_splitlock();
+ enable_gp_for_uclock();

init_pcpu_xsave();

diff --git a/hypervisor/arch/x86/cpu_caps.c b/hypervisor/arch/x86/cpu_caps.c
index 8a6753620..7cf525517 100644
--- a/hypervisor/arch/x86/cpu_caps.c
+++ b/hypervisor/arch/x86/cpu_caps.c
@@ -154,7 +154,7 @@ bool is_ac_enabled(void)
{
bool ac_enabled = false;

- if (has_core_cap(1U << 5U) && (msr_read(MSR_TEST_CTL) & (1U << 29U))) {
+ if (has_core_cap(CORE_CAP_SPLIT_LOCK) && (msr_read(MSR_TEST_CTL) & MSR_TEST_CTL_AC_SPLITLOCK)) {
ac_enabled = true;
}

diff --git a/hypervisor/arch/x86/guest/vmsr.c b/hypervisor/arch/x86/guest/vmsr.c
index 195dd2567..ff1ad704f 100644
--- a/hypervisor/arch/x86/guest/vmsr.c
+++ b/hypervisor/arch/x86/guest/vmsr.c
@@ -589,7 +589,7 @@ int32_t rdmsr_vmexit_handler(struct acrn_vcpu *vcpu)
/* If has MSR_TEST_CTL, give emulated value
* If don't have MSR_TEST_CTL, trigger #GP
*/
- if (has_core_cap(1U << 5U)) {
+ if (has_core_cap(CORE_CAP_SPLIT_LOCK) || has_core_cap(CORE_CAP_UC_LOCK)) {
v = vcpu_get_guest_msr(vcpu, MSR_TEST_CTL);
} else {
vcpu_inject_gp(vcpu, 0U);
@@ -959,7 +959,7 @@ int32_t wrmsr_vmexit_handler(struct acrn_vcpu *vcpu)
/* If VM has MSR_TEST_CTL, ignore write operation
* If don't have MSR_TEST_CTL, trigger #GP
*/
- if (has_core_cap(1U << 5U)) {
+ if (has_core_cap(CORE_CAP_SPLIT_LOCK) || has_core_cap(CORE_CAP_UC_LOCK)) {
vcpu_set_guest_msr(vcpu, MSR_TEST_CTL, v);
pr_warn("Ignore writting 0x%llx to MSR_TEST_CTL from VM%d", v, vcpu->vm->vm_id);
} else {
diff --git a/hypervisor/include/arch/x86/asm/cpu_caps.h b/hypervisor/include/arch/x86/asm/cpu_caps.h
index 5559eef51..bbe6a601a 100644
--- a/hypervisor/include/arch/x86/asm/cpu_caps.h
+++ b/hypervisor/include/arch/x86/asm/cpu_caps.h
@@ -60,4 +60,8 @@ void init_pcpu_model_name(void);
int32_t detect_hardware_support(void);
struct cpuinfo_x86 *get_pcpu_info(void);

+/* The bits of MSR IA32_CORE_CAPABILITIES */
+#define CORE_CAP_SPLIT_LOCK (1U << 5U) /* support #AC for Split-locked Access */
+#define CORE_CAP_UC_LOCK (1U << 4U) /* support #GP for non-guaranteed-atomic-locked access at Non-WB memory */
+
#endif /* CPUINFO_H */
diff --git a/hypervisor/include/arch/x86/asm/msr.h b/hypervisor/include/arch/x86/asm/msr.h
index 00ed0a289..84b9d1942 100644
--- a/hypervisor/include/arch/x86/asm/msr.h
+++ b/hypervisor/include/arch/x86/asm/msr.h
@@ -596,6 +596,11 @@
/* 5 high-order bits in every field are reserved */
#define PAT_FIELD_RSV_BITS (0xF8UL)

+/* MSR_TEST_CTL bits */
+#define MSR_TEST_CTL_GP_UCLOCK (1U << 28U)
+#define MSR_TEST_CTL_AC_SPLITLOCK (1U << 29U)
+#define MSR_TEST_CTL_DISABLE_LOCK_ASSERTION (1U << 31U)
+
#ifndef ASSEMBLER
static inline bool is_pat_mem_type_invalid(uint64_t x)
{
--
2.17.1


Eddie Dong
 

-----Original Message-----
From: acrn-dev@lists.projectacrn.org <acrn-dev@lists.projectacrn.org> On
Behalf Of Tao, Yuhong
Sent: Thursday, July 1, 2021 10:29 PM
To: acrn-dev@lists.projectacrn.org
Subject: [acrn-dev] [PATCH 1/3] HV: enable #GP for UC lock

From: Tao Yuhong <yuhong.tao@intel.com>

For an atomic operation using bus locking, it would generate LOCK# bus signal,
if it has Non-WB memory operand. This is an UC lock.
Add: it will ruin the RT behavior of the system.

If MSR_IA32_CORE_CAPABILITIES[bit4] is 1, then CPU support rising #GP for
support rising #GP --> Can trigger #GP
instructions which cause UC lock. This feature is controlled by
MSR_TEST_CTL[bit28].
Enable this feature if supported.
This patch enables the #GP for UC lock.


Signed-off-by: Tao Yuhong <yuhong.tao@intel.com>
---
hypervisor/arch/x86/Kconfig | 7 +++++++
hypervisor/arch/x86/cpu.c | 18 ++++++++++++++++--
hypervisor/arch/x86/cpu_caps.c | 2 +-
hypervisor/arch/x86/guest/vmsr.c | 4 ++--
hypervisor/include/arch/x86/asm/cpu_caps.h | 4 ++++
hypervisor/include/arch/x86/asm/msr.h | 5 +++++
6 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/hypervisor/arch/x86/Kconfig b/hypervisor/arch/x86/Kconfig index
ded7826b1..a8c4162bd 100644
--- a/hypervisor/arch/x86/Kconfig
+++ b/hypervisor/arch/x86/Kconfig
@@ -343,6 +343,13 @@ config ENFORCE_TURNOFF_AC
If CPU has #AC for split-locked access, HV enable it and VMs can't
disable.
Set this to enforce turn off that #AC, for community developer only.

+config ENFORCE_TURNOFF_GP
+ bool "Force to disable #GP for UC lock"
+ default n
+ help
+ If CPU has #GP for UC lock, HV enable it and VMs can't disable.
+ Set this to enforce turn off that #GP, for community developer only.
+
Why we need this? What is the problem if the community developers go without this?

config IVSHMEM_ENABLED
bool "Enable ivshmem inter-vm communication based on hypervisor
shared memory"
default n
diff --git a/hypervisor/arch/x86/cpu.c b/hypervisor/arch/x86/cpu.c index
8cdef8a3e..9419f31ad 100644
--- a/hypervisor/arch/x86/cpu.c
+++ b/hypervisor/arch/x86/cpu.c
@@ -113,14 +113,27 @@ static void enable_ac_for_splitlock(void) #ifndef
CONFIG_ENFORCE_TURNOFF_AC
uint64_t test_ctl;

- if (has_core_cap(1U << 5U)) {
+ if (has_core_cap(CORE_CAP_SPLIT_LOCK)) {
test_ctl = msr_read(MSR_TEST_CTL);
- test_ctl |= (1U << 29U);
+ test_ctl |= MSR_TEST_CTL_AC_SPLITLOCK;
msr_write(MSR_TEST_CTL, test_ctl);
}
#endif /*CONFIG_ENFORCE_TURNOFF_AC*/
}

+static void enable_gp_for_uclock(void)
+{
+#ifndef CONFIG_ENFORCE_TURNOFF_GP
+ uint64_t test_ctl;
+
+ if (has_core_cap(CORE_CAP_UC_LOCK)) {
+ test_ctl = msr_read(MSR_TEST_CTL);
+ test_ctl |= MSR_TEST_CTL_GP_UCLOCK;
+ msr_write(MSR_TEST_CTL, test_ctl);
+ }
+#endif /*CONFIG_ENFORCE_TURNOFF_GP*/
+}
+
void init_pcpu_pre(bool is_bsp)
{
uint16_t pcpu_id;
@@ -210,6 +223,7 @@ void init_pcpu_post(uint16_t pcpu_id)
load_gdtr_and_tr();

enable_ac_for_splitlock();
+ enable_gp_for_uclock();

init_pcpu_xsave();

diff --git a/hypervisor/arch/x86/cpu_caps.c
b/hypervisor/arch/x86/cpu_caps.c index 8a6753620..7cf525517 100644
--- a/hypervisor/arch/x86/cpu_caps.c
+++ b/hypervisor/arch/x86/cpu_caps.c
@@ -154,7 +154,7 @@ bool is_ac_enabled(void) {
bool ac_enabled = false;

- if (has_core_cap(1U << 5U) && (msr_read(MSR_TEST_CTL) & (1U <<
29U))) {
+ if (has_core_cap(CORE_CAP_SPLIT_LOCK) && (msr_read(MSR_TEST_CTL)
&
+MSR_TEST_CTL_AC_SPLITLOCK)) {
ac_enabled = true;
}

diff --git a/hypervisor/arch/x86/guest/vmsr.c
b/hypervisor/arch/x86/guest/vmsr.c
index 195dd2567..ff1ad704f 100644
--- a/hypervisor/arch/x86/guest/vmsr.c
+++ b/hypervisor/arch/x86/guest/vmsr.c
@@ -589,7 +589,7 @@ int32_t rdmsr_vmexit_handler(struct acrn_vcpu
*vcpu)
/* If has MSR_TEST_CTL, give emulated value
* If don't have MSR_TEST_CTL, trigger #GP
*/
- if (has_core_cap(1U << 5U)) {
+ if (has_core_cap(CORE_CAP_SPLIT_LOCK) ||
+has_core_cap(CORE_CAP_UC_LOCK)) {
v = vcpu_get_guest_msr(vcpu, MSR_TEST_CTL);
} else {
vcpu_inject_gp(vcpu, 0U);
@@ -959,7 +959,7 @@ int32_t wrmsr_vmexit_handler(struct acrn_vcpu
*vcpu)
/* If VM has MSR_TEST_CTL, ignore write operation
* If don't have MSR_TEST_CTL, trigger #GP
*/
- if (has_core_cap(1U << 5U)) {
+ if (has_core_cap(CORE_CAP_SPLIT_LOCK) ||
+has_core_cap(CORE_CAP_UC_LOCK)) {
vcpu_set_guest_msr(vcpu, MSR_TEST_CTL, v);
pr_warn("Ignore writting 0x%llx to MSR_TEST_CTL from
VM%d", v, vcpu->vm->vm_id);
} else {
diff --git a/hypervisor/include/arch/x86/asm/cpu_caps.h
b/hypervisor/include/arch/x86/asm/cpu_caps.h
index 5559eef51..bbe6a601a 100644
--- a/hypervisor/include/arch/x86/asm/cpu_caps.h
+++ b/hypervisor/include/arch/x86/asm/cpu_caps.h
@@ -60,4 +60,8 @@ void init_pcpu_model_name(void); int32_t
detect_hardware_support(void); struct cpuinfo_x86 *get_pcpu_info(void);

+/* The bits of MSR IA32_CORE_CAPABILITIES */
+#define CORE_CAP_SPLIT_LOCK (1U << 5U) /* support #AC for
Split-locked Access */
+#define CORE_CAP_UC_LOCK (1U << 4U) /* support #GP for
non-guaranteed-atomic-locked access at Non-WB memory */
+
#endif /* CPUINFO_H */
diff --git a/hypervisor/include/arch/x86/asm/msr.h
b/hypervisor/include/arch/x86/asm/msr.h
index 00ed0a289..84b9d1942 100644
--- a/hypervisor/include/arch/x86/asm/msr.h
+++ b/hypervisor/include/arch/x86/asm/msr.h
@@ -596,6 +596,11 @@
/* 5 high-order bits in every field are reserved */
#define PAT_FIELD_RSV_BITS (0xF8UL)

+/* MSR_TEST_CTL bits */
+#define MSR_TEST_CTL_GP_UCLOCK (1U << 28U)
+#define MSR_TEST_CTL_AC_SPLITLOCK (1U << 29U)
+#define MSR_TEST_CTL_DISABLE_LOCK_ASSERTION (1U << 31U)
+
#ifndef ASSEMBLER
static inline bool is_pat_mem_type_invalid(uint64_t x) {
--
2.17.1





Tao, Yuhong
 

Hi,

-----Original Message-----
From: acrn-dev@lists.projectacrn.org <acrn-dev@lists.projectacrn.org> On
Behalf Of Eddie Dong
Sent: Tuesday, July 6, 2021 10:23 AM
To: acrn-dev@lists.projectacrn.org
Subject: Re: [acrn-dev] [PATCH 1/3] HV: enable #GP for UC lock



-----Original Message-----
From: acrn-dev@lists.projectacrn.org <acrn-dev@lists.projectacrn.org>
On Behalf Of Tao, Yuhong
Sent: Thursday, July 1, 2021 10:29 PM
To: acrn-dev@lists.projectacrn.org
Subject: [acrn-dev] [PATCH 1/3] HV: enable #GP for UC lock

From: Tao Yuhong <yuhong.tao@intel.com>

For an atomic operation using bus locking, it would generate LOCK# bus
signal, if it has Non-WB memory operand. This is an UC lock.
Add: it will ruin the RT behavior of the system.
OK


If MSR_IA32_CORE_CAPABILITIES[bit4] is 1, then CPU support rising #GP
for
support rising #GP --> Can trigger #GP
instructions which cause UC lock. This feature is controlled by
MSR_TEST_CTL[bit28].
Enable this feature if supported.
This patch enables the #GP for UC lock.
OK, will modify these words


Signed-off-by: Tao Yuhong <yuhong.tao@intel.com>
---
hypervisor/arch/x86/Kconfig | 7 +++++++
hypervisor/arch/x86/cpu.c | 18 ++++++++++++++++--
hypervisor/arch/x86/cpu_caps.c | 2 +-
hypervisor/arch/x86/guest/vmsr.c | 4 ++--
hypervisor/include/arch/x86/asm/cpu_caps.h | 4 ++++
hypervisor/include/arch/x86/asm/msr.h | 5 +++++
6 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/hypervisor/arch/x86/Kconfig b/hypervisor/arch/x86/Kconfig
index ded7826b1..a8c4162bd 100644
--- a/hypervisor/arch/x86/Kconfig
+++ b/hypervisor/arch/x86/Kconfig
@@ -343,6 +343,13 @@ config ENFORCE_TURNOFF_AC
If CPU has #AC for split-locked access, HV enable it and VMs can't
disable.
Set this to enforce turn off that #AC, for community developer only.

+config ENFORCE_TURNOFF_GP
+ bool "Force to disable #GP for UC lock"
+ default n
+ help
+ If CPU has #GP for UC lock, HV enable it and VMs can't disable.
+ Set this to enforce turn off that #GP, for community developer only.
+
Why we need this? What is the problem if the community developers go without
this?
Hi, Eddie, we will emulate UC lock instruction, so may not need to configure UC-lock detection out.
Only because we have this requirement "Add a config ENFORCE_TURNOFF_UC if developer want to run ACRN on a buggy system."


config IVSHMEM_ENABLED
bool "Enable ivshmem inter-vm communication based on hypervisor
shared memory"
default n
diff --git a/hypervisor/arch/x86/cpu.c b/hypervisor/arch/x86/cpu.c
index 8cdef8a3e..9419f31ad 100644
--- a/hypervisor/arch/x86/cpu.c
+++ b/hypervisor/arch/x86/cpu.c
@@ -113,14 +113,27 @@ static void enable_ac_for_splitlock(void)
#ifndef CONFIG_ENFORCE_TURNOFF_AC
uint64_t test_ctl;

- if (has_core_cap(1U << 5U)) {
+ if (has_core_cap(CORE_CAP_SPLIT_LOCK)) {
test_ctl = msr_read(MSR_TEST_CTL);
- test_ctl |= (1U << 29U);
+ test_ctl |= MSR_TEST_CTL_AC_SPLITLOCK;
msr_write(MSR_TEST_CTL, test_ctl);
}
#endif /*CONFIG_ENFORCE_TURNOFF_AC*/
}

+static void enable_gp_for_uclock(void) { #ifndef
+CONFIG_ENFORCE_TURNOFF_GP
+ uint64_t test_ctl;
+
+ if (has_core_cap(CORE_CAP_UC_LOCK)) {
+ test_ctl = msr_read(MSR_TEST_CTL);
+ test_ctl |= MSR_TEST_CTL_GP_UCLOCK;
+ msr_write(MSR_TEST_CTL, test_ctl);
+ }
+#endif /*CONFIG_ENFORCE_TURNOFF_GP*/
+}
+
void init_pcpu_pre(bool is_bsp)
{
uint16_t pcpu_id;
@@ -210,6 +223,7 @@ void init_pcpu_post(uint16_t pcpu_id)
load_gdtr_and_tr();

enable_ac_for_splitlock();
+ enable_gp_for_uclock();

init_pcpu_xsave();

diff --git a/hypervisor/arch/x86/cpu_caps.c
b/hypervisor/arch/x86/cpu_caps.c index 8a6753620..7cf525517 100644
--- a/hypervisor/arch/x86/cpu_caps.c
+++ b/hypervisor/arch/x86/cpu_caps.c
@@ -154,7 +154,7 @@ bool is_ac_enabled(void) {
bool ac_enabled = false;

- if (has_core_cap(1U << 5U) && (msr_read(MSR_TEST_CTL) & (1U <<
29U))) {
+ if (has_core_cap(CORE_CAP_SPLIT_LOCK) &&
(msr_read(MSR_TEST_CTL)
&
+MSR_TEST_CTL_AC_SPLITLOCK)) {
ac_enabled = true;
}

diff --git a/hypervisor/arch/x86/guest/vmsr.c
b/hypervisor/arch/x86/guest/vmsr.c
index 195dd2567..ff1ad704f 100644
--- a/hypervisor/arch/x86/guest/vmsr.c
+++ b/hypervisor/arch/x86/guest/vmsr.c
@@ -589,7 +589,7 @@ int32_t rdmsr_vmexit_handler(struct acrn_vcpu
*vcpu)
/* If has MSR_TEST_CTL, give emulated value
* If don't have MSR_TEST_CTL, trigger #GP
*/
- if (has_core_cap(1U << 5U)) {
+ if (has_core_cap(CORE_CAP_SPLIT_LOCK) ||
+has_core_cap(CORE_CAP_UC_LOCK)) {
v = vcpu_get_guest_msr(vcpu, MSR_TEST_CTL);
} else {
vcpu_inject_gp(vcpu, 0U);
@@ -959,7 +959,7 @@ int32_t wrmsr_vmexit_handler(struct acrn_vcpu
*vcpu)
/* If VM has MSR_TEST_CTL, ignore write operation
* If don't have MSR_TEST_CTL, trigger #GP
*/
- if (has_core_cap(1U << 5U)) {
+ if (has_core_cap(CORE_CAP_SPLIT_LOCK) ||
+has_core_cap(CORE_CAP_UC_LOCK)) {
vcpu_set_guest_msr(vcpu, MSR_TEST_CTL, v);
pr_warn("Ignore writting 0x%llx to MSR_TEST_CTL
from VM%d", v,
vcpu->vm->vm_id);
} else {
diff --git a/hypervisor/include/arch/x86/asm/cpu_caps.h
b/hypervisor/include/arch/x86/asm/cpu_caps.h
index 5559eef51..bbe6a601a 100644
--- a/hypervisor/include/arch/x86/asm/cpu_caps.h
+++ b/hypervisor/include/arch/x86/asm/cpu_caps.h
@@ -60,4 +60,8 @@ void init_pcpu_model_name(void); int32_t
detect_hardware_support(void); struct cpuinfo_x86
*get_pcpu_info(void);

+/* The bits of MSR IA32_CORE_CAPABILITIES */
+#define CORE_CAP_SPLIT_LOCK (1U << 5U) /* support #AC for
Split-locked Access */
+#define CORE_CAP_UC_LOCK (1U << 4U) /* support #GP for
non-guaranteed-atomic-locked access at Non-WB memory */
+
#endif /* CPUINFO_H */
diff --git a/hypervisor/include/arch/x86/asm/msr.h
b/hypervisor/include/arch/x86/asm/msr.h
index 00ed0a289..84b9d1942 100644
--- a/hypervisor/include/arch/x86/asm/msr.h
+++ b/hypervisor/include/arch/x86/asm/msr.h
@@ -596,6 +596,11 @@
/* 5 high-order bits in every field are reserved */
#define PAT_FIELD_RSV_BITS (0xF8UL)

+/* MSR_TEST_CTL bits */
+#define MSR_TEST_CTL_GP_UCLOCK (1U << 28U)
+#define MSR_TEST_CTL_AC_SPLITLOCK (1U << 29U)
+#define MSR_TEST_CTL_DISABLE_LOCK_ASSERTION (1U << 31U)
+
#ifndef ASSEMBLER
static inline bool is_pat_mem_type_invalid(uint64_t x) {
--
2.17.1