[PATCH V1 2/6] HV: handle #AC for Splitlock Access #ac


Tao, Yuhong
 

In VMX root-mode, CPL != 3, #AC always rise for Splitlock Access.
HV report the rip of the instruction which triggers splitlock, and
temporally disable splitlock detection, allow the instruction to be
executed. A fix to the Splitlock Access is expected.

Tracked-On: #4496
Signed-off-by: Tao Yuhong <yuhong.tao@intel.com>
---
hypervisor/arch/x86/irq.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/hypervisor/arch/x86/irq.c b/hypervisor/arch/x86/irq.c
index ffc9150..d42a32f 100644
--- a/hypervisor/arch/x86/irq.c
+++ b/hypervisor/arch/x86/irq.c
@@ -370,17 +370,26 @@ void dispatch_exception(struct intr_excp_ctx *ctx)
{
uint16_t pcpu_id = get_pcpu_id();

- /* Obtain lock to ensure exception dump doesn't get corrupted */
- spinlock_obtain(&exception_spinlock);
-
- /* Dump exception context */
- dump_exception(ctx, pcpu_id);
-
- /* Release lock to let other CPUs handle exception */
- spinlock_release(&exception_spinlock);
-
- /* Halt the CPU */
- cpu_dead();
+ switch (ctx->vector) {
+ case IDT_AC:
+ /* #AC in HV is always triggered by Splitlock Access */
+ pr_err("HV has detected Splitlock Access at rip:0x%016llX\n"
+ "Temporally disable Splitlock Access", ctx->rip);
+ disable_ac_for_splitlock();
+ break;
+ default:
+ /* Obtain lock to ensure exception dump doesn't get corrupted */
+ spinlock_obtain(&exception_spinlock);
+
+ /* Dump exception context */
+ dump_exception(ctx, pcpu_id);
+
+ /* Release lock to let other CPUs handle exception */
+ spinlock_release(&exception_spinlock);
+
+ /* Halt the CPU */
+ cpu_dead();
+ }
}

void handle_nmi(__unused struct intr_excp_ctx *ctx)
--
2.7.4


Li, Fei1
 

On Wed, Mar 18, 2020 at 09:18:42PM +0800, Tao, Yuhong wrote:
In VMX root-mode, CPL != 3, #AC always rise for Splitlock Access.
HV report the rip of the instruction which triggers splitlock, and
temporally disable splitlock detection, allow the instruction to be
executed. A fix to the Splitlock Access is expected.

Tracked-On: #4496
Signed-off-by: Tao Yuhong <yuhong.tao@intel.com>
---
hypervisor/arch/x86/irq.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/hypervisor/arch/x86/irq.c b/hypervisor/arch/x86/irq.c
index ffc9150..d42a32f 100644
--- a/hypervisor/arch/x86/irq.c
+++ b/hypervisor/arch/x86/irq.c
@@ -370,17 +370,26 @@ void dispatch_exception(struct intr_excp_ctx *ctx)
{
uint16_t pcpu_id = get_pcpu_id();

- /* Obtain lock to ensure exception dump doesn't get corrupted */
- spinlock_obtain(&exception_spinlock);
-
- /* Dump exception context */
- dump_exception(ctx, pcpu_id);
-
- /* Release lock to let other CPUs handle exception */
- spinlock_release(&exception_spinlock);
-
- /* Halt the CPU */
- cpu_dead();
+ switch (ctx->vector) {
+ case IDT_AC:
An #AC doesn't equal to a splitlock access execption. Am I right ?
Besides, do we enable alognment checking?

+ /* #AC in HV is always triggered by Splitlock Access */
+ pr_err("HV has detected Splitlock Access at rip:0x%016llX\n"
+ "Temporally disable Splitlock Access", ctx->rip);
+ disable_ac_for_splitlock();
+ break;
+ default:
+ /* Obtain lock to ensure exception dump doesn't get corrupted */
+ spinlock_obtain(&exception_spinlock);
+
+ /* Dump exception context */
+ dump_exception(ctx, pcpu_id);
+
+ /* Release lock to let other CPUs handle exception */
+ spinlock_release(&exception_spinlock);
+
+ /* Halt the CPU */
+ cpu_dead();
Use if - else may be better here.
+ }
}

void handle_nmi(__unused struct intr_excp_ctx *ctx)
--
2.7.4




Tao, Yuhong
 

Hi, Fei

-----Original Message-----
From: Li, Fei1 <fei1.li@intel.com>
Sent: Thursday, March 19, 2020 10:14 AM
To: acrn-dev@lists.projectacrn.org
Cc: Tao, Yuhong <yuhong.tao@intel.com>
Subject: Re: [acrn-dev] [PATCH V1 2/6] HV: handle #AC for Splitlock Access

On Wed, Mar 18, 2020 at 09:18:42PM +0800, Tao, Yuhong wrote:
In VMX root-mode, CPL != 3, #AC always rise for Splitlock Access.
HV report the rip of the instruction which triggers splitlock, and
temporally disable splitlock detection, allow the instruction to be
executed. A fix to the Splitlock Access is expected.

Tracked-On: #4496
Signed-off-by: Tao Yuhong <yuhong.tao@intel.com>
---
hypervisor/arch/x86/irq.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/hypervisor/arch/x86/irq.c b/hypervisor/arch/x86/irq.c
index ffc9150..d42a32f 100644
--- a/hypervisor/arch/x86/irq.c
+++ b/hypervisor/arch/x86/irq.c
@@ -370,17 +370,26 @@ void dispatch_exception(struct intr_excp_ctx
*ctx) {
uint16_t pcpu_id = get_pcpu_id();

- /* Obtain lock to ensure exception dump doesn't get corrupted */
- spinlock_obtain(&exception_spinlock);
-
- /* Dump exception context */
- dump_exception(ctx, pcpu_id);
-
- /* Release lock to let other CPUs handle exception */
- spinlock_release(&exception_spinlock);
-
- /* Halt the CPU */
- cpu_dead();
+ switch (ctx->vector) {
+ case IDT_AC:
An #AC doesn't equal to a splitlock access execption. Am I right ?
Yes, splitlock access is a subset of un-alignment access. HV runs at ring0, so it must a splitlock access

Besides, do we enable alognment checking?
HV alignment checking is not enabled. The SDM says enable this feature will " Cause #AC(0) exception for split locked access at all CPL irrespective of CR0.AM or EFLAGS.AC.", so the splitlock access can trigger #AC(0), do not need to enable alignment checking.


+ /* #AC in HV is always triggered by Splitlock Access */
+ pr_err("HV has detected Splitlock Access at rip:0x%016llX\n"
+ "Temporally disable Splitlock Access", ctx->rip);
+ disable_ac_for_splitlock();
+ break;
+ default:
+ /* Obtain lock to ensure exception dump doesn't get corrupted
*/
+ spinlock_obtain(&exception_spinlock);
+
+ /* Dump exception context */
+ dump_exception(ctx, pcpu_id);
+
+ /* Release lock to let other CPUs handle exception */
+ spinlock_release(&exception_spinlock);
+
+ /* Halt the CPU */
+ cpu_dead();
Use if - else may be better here.
Yes, it is better, will use it.

+ }
}

void handle_nmi(__unused struct intr_excp_ctx *ctx)
--
2.7.4