|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH V2] x86/emulate: synchronize LOCKed instruction emulation
LOCK-prefixed instructions are currenly allowed to run in parallel
in x86_emulate(), which can lead the guest into an undefined state.
This patch fixes the issue.
Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
Changes since V1:
- Added Andrew Cooper's credit, as he's kept the patch current
througout non-trivial code changes since the initial patch.
- Significantly more patch testing (with XenServer).
- Restricted lock scope.
- Logic fixes.
---
tools/tests/x86_emulator/test_x86_emulator.c | 10 ++++++++++
xen/arch/x86/domain.c | 2 ++
xen/arch/x86/hvm/emulate.c | 26 ++++++++++++++++++++++++++
xen/arch/x86/mm.c | 6 ++++++
xen/arch/x86/mm/shadow/common.c | 2 ++
xen/arch/x86/traps.c | 2 ++
xen/arch/x86/x86_emulate/x86_emulate.c | 14 ++++++++++++--
xen/arch/x86/x86_emulate/x86_emulate.h | 8 ++++++++
xen/include/asm-x86/domain.h | 4 ++++
xen/include/asm-x86/hvm/emulate.h | 3 +++
10 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/tools/tests/x86_emulator/test_x86_emulator.c
b/tools/tests/x86_emulator/test_x86_emulator.c
index 04332bb..86b79a1 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -283,6 +283,14 @@ static int read_msr(
return X86EMUL_UNHANDLEABLE;
}
+static void smp_lock(bool locked)
+{
+}
+
+static void smp_unlock(bool locked)
+{
+}
+
static struct x86_emulate_ops emulops = {
.read = read,
.insn_fetch = fetch,
@@ -293,6 +301,8 @@ static struct x86_emulate_ops emulops = {
.read_cr = emul_test_read_cr,
.read_msr = read_msr,
.get_fpu = emul_test_get_fpu,
+ .smp_lock = smp_lock,
+ .smp_unlock = smp_unlock,
};
int main(int argc, char **argv)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 479aee6..55010f4 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -529,6 +529,8 @@ int arch_domain_create(struct domain *d, unsigned int
domcr_flags,
if ( config == NULL && !is_idle_domain(d) )
return -EINVAL;
+ percpu_rwlock_resource_init(&d->arch.emulate_lock, emulate_locked_rwlock);
+
d->arch.s3_integrity = !!(domcr_flags & DOMCRF_s3_integrity);
INIT_LIST_HEAD(&d->arch.pdev_list);
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index f36d7c9..d5bfbf1 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -24,6 +24,8 @@
#include <asm/hvm/svm/svm.h>
#include <asm/vm_event.h>
+DEFINE_PERCPU_RWLOCK_GLOBAL(emulate_locked_rwlock);
+
static void hvmtrace_io_assist(const ioreq_t *p)
{
unsigned int size, event;
@@ -1682,6 +1684,26 @@ static int hvmemul_vmfunc(
return rc;
}
+void emulate_smp_lock(bool locked)
+{
+ struct domain *d = current->domain;
+
+ if ( locked )
+ percpu_write_lock(emulate_locked_rwlock, &d->arch.emulate_lock);
+ else
+ percpu_read_lock(emulate_locked_rwlock, &d->arch.emulate_lock);
+}
+
+void emulate_smp_unlock(bool locked)
+{
+ struct domain *d = current->domain;
+
+ if ( locked )
+ percpu_write_unlock(emulate_locked_rwlock, &d->arch.emulate_lock);
+ else
+ percpu_read_unlock(emulate_locked_rwlock, &d->arch.emulate_lock);
+}
+
static const struct x86_emulate_ops hvm_emulate_ops = {
.read = hvmemul_read,
.insn_fetch = hvmemul_insn_fetch,
@@ -1706,6 +1728,8 @@ static const struct x86_emulate_ops hvm_emulate_ops = {
.put_fpu = hvmemul_put_fpu,
.invlpg = hvmemul_invlpg,
.vmfunc = hvmemul_vmfunc,
+ .smp_lock = emulate_smp_lock,
+ .smp_unlock = emulate_smp_unlock,
};
static const struct x86_emulate_ops hvm_emulate_ops_no_write = {
@@ -1731,6 +1755,8 @@ static const struct x86_emulate_ops
hvm_emulate_ops_no_write = {
.put_fpu = hvmemul_put_fpu,
.invlpg = hvmemul_invlpg,
.vmfunc = hvmemul_vmfunc,
+ .smp_lock = emulate_smp_lock,
+ .smp_unlock = emulate_smp_unlock,
};
static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt,
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 7bc951d..2fb3325 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5369,6 +5369,8 @@ static const struct x86_emulate_ops ptwr_emulate_ops = {
.cmpxchg = ptwr_emulated_cmpxchg,
.validate = pv_emul_is_mem_write,
.cpuid = pv_emul_cpuid,
+ .smp_lock = emulate_smp_lock,
+ .smp_unlock = emulate_smp_unlock,
};
/* Write page fault handler: check if guest is trying to modify a PTE. */
@@ -5485,6 +5487,8 @@ static const struct x86_emulate_ops mmio_ro_emulate_ops =
{
.write = mmio_ro_emulated_write,
.validate = pv_emul_is_mem_write,
.cpuid = pv_emul_cpuid,
+ .smp_lock = emulate_smp_lock,
+ .smp_unlock = emulate_smp_unlock,
};
int mmcfg_intercept_write(
@@ -5524,6 +5528,8 @@ static const struct x86_emulate_ops mmcfg_intercept_ops =
{
.write = mmcfg_intercept_write,
.validate = pv_emul_is_mem_write,
.cpuid = pv_emul_cpuid,
+ .smp_lock = emulate_smp_lock,
+ .smp_unlock = emulate_smp_unlock,
};
/* Check if guest is trying to modify a r/o MMIO page. */
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index d078d78..3a2f02e 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -310,6 +310,8 @@ static const struct x86_emulate_ops hvm_shadow_emulator_ops
= {
.write = hvm_emulate_write,
.cmpxchg = hvm_emulate_cmpxchg,
.cpuid = hvmemul_cpuid,
+ .smp_lock = emulate_smp_lock,
+ .smp_unlock = emulate_smp_unlock,
};
const struct x86_emulate_ops *shadow_init_emulation(
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 08b0070..8bdc8c8 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2957,6 +2957,8 @@ static const struct x86_emulate_ops priv_op_ops = {
.write_msr = priv_op_write_msr,
.cpuid = pv_emul_cpuid,
.wbinvd = priv_op_wbinvd,
+ .smp_lock = emulate_smp_lock,
+ .smp_unlock = emulate_smp_unlock,
};
static int emulate_privileged_op(struct cpu_user_regs *regs)
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
b/xen/arch/x86/x86_emulate/x86_emulate.c
index 4872f19..bec4af7 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -3037,7 +3037,7 @@ x86_emulate(
struct x86_emulate_stub stub = {};
DECLARE_ALIGNED(mmval_t, mmval);
- ASSERT(ops->read);
+ ASSERT(ops->read && ops->smp_lock && ops->smp_unlock);
rc = x86_decode(&state, ctxt, ops);
if ( rc != X86EMUL_OKAY )
@@ -3065,6 +3065,8 @@ x86_emulate(
d = state.desc;
#define state (&state)
+ ops->smp_lock(lock_prefix);
+
generate_exception_if(state->not_64bit && mode_64bit(), EXC_UD);
if ( ea.type == OP_REG )
@@ -3593,6 +3595,12 @@ x86_emulate(
break;
case 0x86 ... 0x87: xchg: /* xchg */
+ if ( !lock_prefix )
+ {
+ ops->smp_unlock(lock_prefix);
+ lock_prefix = 1;
+ ops->smp_lock(lock_prefix);
+ }
/* Write back the register source. */
switch ( dst.bytes )
{
@@ -3603,7 +3611,6 @@ x86_emulate(
}
/* Write back the memory destination with implicit LOCK prefix. */
dst.val = src.val;
- lock_prefix = 1;
break;
case 0xc6: /* Grp11: mov / xabort */
@@ -7925,8 +7932,11 @@ x86_emulate(
ctxt->regs->eflags &= ~X86_EFLAGS_RF;
done:
+ ops->smp_unlock(lock_prefix);
+
_put_fpu();
put_stub(stub);
+
return rc;
#undef state
}
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h
b/xen/arch/x86/x86_emulate/x86_emulate.h
index 6e98453..3f8bb38 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -448,6 +448,14 @@ struct x86_emulate_ops
/* vmfunc: Emulate VMFUNC via given set of EAX ECX inputs */
int (*vmfunc)(
struct x86_emulate_ctxt *ctxt);
+
+ /* smp_lock: Take a write lock if locked, read lock otherwise. */
+ void (*smp_lock)(
+ bool locked);
+
+ /* smp_unlock: Write unlock if locked, read unlock otherwise. */
+ void (*smp_unlock)(
+ bool locked);
};
struct cpu_user_regs;
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index ff5267f..2afccab 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -271,6 +271,8 @@ struct monitor_write_data {
uint64_t cr4;
};
+DECLARE_PERCPU_RWLOCK_GLOBAL(emulate_locked_rwlock);
+
struct arch_domain
{
struct page_info *perdomain_l3_pg;
@@ -413,6 +415,8 @@ struct arch_domain
/* Emulated devices enabled bitmap. */
uint32_t emulation_flags;
+
+ percpu_rwlock_t emulate_lock;
} __cacheline_aligned;
#define has_vlapic(d) (!!((d)->arch.emulation_flags & XEN_X86_EMU_LAPIC))
diff --git a/xen/include/asm-x86/hvm/emulate.h
b/xen/include/asm-x86/hvm/emulate.h
index 88d6b70..b29a47e 100644
--- a/xen/include/asm-x86/hvm/emulate.h
+++ b/xen/include/asm-x86/hvm/emulate.h
@@ -93,6 +93,9 @@ int hvmemul_do_pio_buffer(uint16_t port,
void hvm_dump_emulation_state(const char *prefix,
struct hvm_emulate_ctxt *hvmemul_ctxt);
+void emulate_smp_lock(bool locked);
+void emulate_smp_unlock(bool locked);
+
#endif /* __ASM_X86_HVM_EMULATE_H__ */
/*
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |