* [PATCH 1/6] x86/HVM: properly handle MMIO reads and writes wider than a machine word
2013-09-19 15:40 [PATCH 0/6] x86: emulation improvements Jan Beulich
@ 2013-09-19 15:42 ` Jan Beulich
2013-09-19 15:43 ` [PATCH 2/6] x86: fix dependencies of emulator test Jan Beulich
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2013-09-19 15:42 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 6959 bytes --]
Just like real hardware we ought to split such accesses transparently
to the caller. With little extra effort we can at once even handle page
crossing accesses correctly.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -438,6 +438,7 @@ static int __hvmemul_read(
{
struct vcpu *curr = current;
unsigned long addr, reps = 1;
+ unsigned int off, chunk = min_t(unsigned int, bytes, sizeof(long));
uint32_t pfec = PFEC_page_present;
struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
paddr_t gpa;
@@ -447,16 +448,38 @@ static int __hvmemul_read(
seg, offset, bytes, &reps, access_type, hvmemul_ctxt, &addr);
if ( rc != X86EMUL_OKAY )
return rc;
+ off = addr & (PAGE_SIZE - 1);
+ /*
+ * We only need to handle sizes actual instruction operands can have. All
+ * such sizes are either powers of 2 or the sum of two powers of 2. Thus
+ * picking as initial chunk size the largest power of 2 not greater than
+ * the total size will always result in only power-of-2 size requests
+ * issued to hvmemul_do_mmio() (hvmemul_do_io() rejects non-powers-of-2).
+ */
+ while ( chunk & (chunk - 1) )
+ chunk &= chunk - 1;
+ if ( off + bytes > PAGE_SIZE )
+ while ( off & (chunk - 1) )
+ chunk >>= 1;
if ( unlikely(vio->mmio_gva == (addr & PAGE_MASK)) && vio->mmio_gva )
{
- unsigned int off = addr & (PAGE_SIZE - 1);
if ( access_type == hvm_access_insn_fetch )
return X86EMUL_UNHANDLEABLE;
gpa = (((paddr_t)vio->mmio_gpfn << PAGE_SHIFT) | off);
- if ( (off + bytes) <= PAGE_SIZE )
- return hvmemul_do_mmio(gpa, &reps, bytes, 0,
- IOREQ_READ, 0, p_data);
+ while ( (off + chunk) <= PAGE_SIZE )
+ {
+ rc = hvmemul_do_mmio(gpa, &reps, chunk, 0, IOREQ_READ, 0, p_data);
+ if ( rc != X86EMUL_OKAY || bytes == chunk )
+ return rc;
+ addr += chunk;
+ off += chunk;
+ gpa += chunk;
+ p_data += chunk;
+ bytes -= chunk;
+ if ( bytes < chunk )
+ chunk = bytes;
+ }
}
if ( (seg != x86_seg_none) &&
@@ -473,14 +496,32 @@ static int __hvmemul_read(
return X86EMUL_EXCEPTION;
case HVMCOPY_unhandleable:
return X86EMUL_UNHANDLEABLE;
- case HVMCOPY_bad_gfn_to_mfn:
+ case HVMCOPY_bad_gfn_to_mfn:
if ( access_type == hvm_access_insn_fetch )
return X86EMUL_UNHANDLEABLE;
- rc = hvmemul_linear_to_phys(
- addr, &gpa, bytes, &reps, pfec, hvmemul_ctxt);
- if ( rc != X86EMUL_OKAY )
- return rc;
- return hvmemul_do_mmio(gpa, &reps, bytes, 0, IOREQ_READ, 0, p_data);
+ rc = hvmemul_linear_to_phys(addr, &gpa, chunk, &reps, pfec,
+ hvmemul_ctxt);
+ while ( rc == X86EMUL_OKAY )
+ {
+ rc = hvmemul_do_mmio(gpa, &reps, chunk, 0, IOREQ_READ, 0, p_data);
+ if ( rc != X86EMUL_OKAY || bytes == chunk )
+ break;
+ addr += chunk;
+ off += chunk;
+ p_data += chunk;
+ bytes -= chunk;
+ if ( bytes < chunk )
+ chunk = bytes;
+ if ( off < PAGE_SIZE )
+ gpa += chunk;
+ else
+ {
+ rc = hvmemul_linear_to_phys(addr, &gpa, chunk, &reps, pfec,
+ hvmemul_ctxt);
+ off = 0;
+ }
+ }
+ return rc;
case HVMCOPY_gfn_paged_out:
return X86EMUL_RETRY;
case HVMCOPY_gfn_shared:
@@ -537,6 +578,7 @@ static int hvmemul_write(
container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
struct vcpu *curr = current;
unsigned long addr, reps = 1;
+ unsigned int off, chunk = min_t(unsigned int, bytes, sizeof(long));
uint32_t pfec = PFEC_page_present | PFEC_write_access;
struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
paddr_t gpa;
@@ -546,14 +588,30 @@ static int hvmemul_write(
seg, offset, bytes, &reps, hvm_access_write, hvmemul_ctxt, &addr);
if ( rc != X86EMUL_OKAY )
return rc;
+ off = addr & (PAGE_SIZE - 1);
+ /* See the respective comment in __hvmemul_read(). */
+ while ( chunk & (chunk - 1) )
+ chunk &= chunk - 1;
+ if ( off + bytes > PAGE_SIZE )
+ while ( off & (chunk - 1) )
+ chunk >>= 1;
if ( unlikely(vio->mmio_gva == (addr & PAGE_MASK)) && vio->mmio_gva )
{
- unsigned int off = addr & (PAGE_SIZE - 1);
gpa = (((paddr_t)vio->mmio_gpfn << PAGE_SHIFT) | off);
- if ( (off + bytes) <= PAGE_SIZE )
- return hvmemul_do_mmio(gpa, &reps, bytes, 0,
- IOREQ_WRITE, 0, p_data);
+ while ( (off + chunk) <= PAGE_SIZE )
+ {
+ rc = hvmemul_do_mmio(gpa, &reps, chunk, 0, IOREQ_WRITE, 0, p_data);
+ if ( rc != X86EMUL_OKAY || bytes == chunk )
+ return rc;
+ addr += chunk;
+ off += chunk;
+ gpa += chunk;
+ p_data += chunk;
+ bytes -= chunk;
+ if ( bytes < chunk )
+ chunk = bytes;
+ }
}
if ( (seg != x86_seg_none) &&
@@ -569,12 +627,29 @@ static int hvmemul_write(
case HVMCOPY_unhandleable:
return X86EMUL_UNHANDLEABLE;
case HVMCOPY_bad_gfn_to_mfn:
- rc = hvmemul_linear_to_phys(
- addr, &gpa, bytes, &reps, pfec, hvmemul_ctxt);
- if ( rc != X86EMUL_OKAY )
- return rc;
- return hvmemul_do_mmio(gpa, &reps, bytes, 0,
- IOREQ_WRITE, 0, p_data);
+ rc = hvmemul_linear_to_phys(addr, &gpa, chunk, &reps, pfec,
+ hvmemul_ctxt);
+ while ( rc == X86EMUL_OKAY )
+ {
+ rc = hvmemul_do_mmio(gpa, &reps, chunk, 0, IOREQ_WRITE, 0, p_data);
+ if ( rc != X86EMUL_OKAY || bytes == chunk )
+ break;
+ addr += chunk;
+ off += chunk;
+ p_data += chunk;
+ bytes -= chunk;
+ if ( bytes < chunk )
+ chunk = bytes;
+ if ( off < PAGE_SIZE )
+ gpa += chunk;
+ else
+ {
+ rc = hvmemul_linear_to_phys(addr, &gpa, chunk, &reps, pfec,
+ hvmemul_ctxt);
+ off = 0;
+ }
+ }
+ return rc;
case HVMCOPY_gfn_paged_out:
return X86EMUL_RETRY;
case HVMCOPY_gfn_shared:
[-- Attachment #2: x86-HVM-emul-split-large.patch --]
[-- Type: text/plain, Size: 7031 bytes --]
x86/HVM: properly handle MMIO reads and writes wider than a machine word
Just like real hardware we ought to split such accesses transparently
to the caller. With little extra effort we can at once even handle page
crossing accesses correctly.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -438,6 +438,7 @@ static int __hvmemul_read(
{
struct vcpu *curr = current;
unsigned long addr, reps = 1;
+ unsigned int off, chunk = min_t(unsigned int, bytes, sizeof(long));
uint32_t pfec = PFEC_page_present;
struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
paddr_t gpa;
@@ -447,16 +448,38 @@ static int __hvmemul_read(
seg, offset, bytes, &reps, access_type, hvmemul_ctxt, &addr);
if ( rc != X86EMUL_OKAY )
return rc;
+ off = addr & (PAGE_SIZE - 1);
+ /*
+ * We only need to handle sizes actual instruction operands can have. All
+ * such sizes are either powers of 2 or the sum of two powers of 2. Thus
+ * picking as initial chunk size the largest power of 2 not greater than
+ * the total size will always result in only power-of-2 size requests
+ * issued to hvmemul_do_mmio() (hvmemul_do_io() rejects non-powers-of-2).
+ */
+ while ( chunk & (chunk - 1) )
+ chunk &= chunk - 1;
+ if ( off + bytes > PAGE_SIZE )
+ while ( off & (chunk - 1) )
+ chunk >>= 1;
if ( unlikely(vio->mmio_gva == (addr & PAGE_MASK)) && vio->mmio_gva )
{
- unsigned int off = addr & (PAGE_SIZE - 1);
if ( access_type == hvm_access_insn_fetch )
return X86EMUL_UNHANDLEABLE;
gpa = (((paddr_t)vio->mmio_gpfn << PAGE_SHIFT) | off);
- if ( (off + bytes) <= PAGE_SIZE )
- return hvmemul_do_mmio(gpa, &reps, bytes, 0,
- IOREQ_READ, 0, p_data);
+ while ( (off + chunk) <= PAGE_SIZE )
+ {
+ rc = hvmemul_do_mmio(gpa, &reps, chunk, 0, IOREQ_READ, 0, p_data);
+ if ( rc != X86EMUL_OKAY || bytes == chunk )
+ return rc;
+ addr += chunk;
+ off += chunk;
+ gpa += chunk;
+ p_data += chunk;
+ bytes -= chunk;
+ if ( bytes < chunk )
+ chunk = bytes;
+ }
}
if ( (seg != x86_seg_none) &&
@@ -473,14 +496,32 @@ static int __hvmemul_read(
return X86EMUL_EXCEPTION;
case HVMCOPY_unhandleable:
return X86EMUL_UNHANDLEABLE;
- case HVMCOPY_bad_gfn_to_mfn:
+ case HVMCOPY_bad_gfn_to_mfn:
if ( access_type == hvm_access_insn_fetch )
return X86EMUL_UNHANDLEABLE;
- rc = hvmemul_linear_to_phys(
- addr, &gpa, bytes, &reps, pfec, hvmemul_ctxt);
- if ( rc != X86EMUL_OKAY )
- return rc;
- return hvmemul_do_mmio(gpa, &reps, bytes, 0, IOREQ_READ, 0, p_data);
+ rc = hvmemul_linear_to_phys(addr, &gpa, chunk, &reps, pfec,
+ hvmemul_ctxt);
+ while ( rc == X86EMUL_OKAY )
+ {
+ rc = hvmemul_do_mmio(gpa, &reps, chunk, 0, IOREQ_READ, 0, p_data);
+ if ( rc != X86EMUL_OKAY || bytes == chunk )
+ break;
+ addr += chunk;
+ off += chunk;
+ p_data += chunk;
+ bytes -= chunk;
+ if ( bytes < chunk )
+ chunk = bytes;
+ if ( off < PAGE_SIZE )
+ gpa += chunk;
+ else
+ {
+ rc = hvmemul_linear_to_phys(addr, &gpa, chunk, &reps, pfec,
+ hvmemul_ctxt);
+ off = 0;
+ }
+ }
+ return rc;
case HVMCOPY_gfn_paged_out:
return X86EMUL_RETRY;
case HVMCOPY_gfn_shared:
@@ -537,6 +578,7 @@ static int hvmemul_write(
container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
struct vcpu *curr = current;
unsigned long addr, reps = 1;
+ unsigned int off, chunk = min_t(unsigned int, bytes, sizeof(long));
uint32_t pfec = PFEC_page_present | PFEC_write_access;
struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
paddr_t gpa;
@@ -546,14 +588,30 @@ static int hvmemul_write(
seg, offset, bytes, &reps, hvm_access_write, hvmemul_ctxt, &addr);
if ( rc != X86EMUL_OKAY )
return rc;
+ off = addr & (PAGE_SIZE - 1);
+ /* See the respective comment in __hvmemul_read(). */
+ while ( chunk & (chunk - 1) )
+ chunk &= chunk - 1;
+ if ( off + bytes > PAGE_SIZE )
+ while ( off & (chunk - 1) )
+ chunk >>= 1;
if ( unlikely(vio->mmio_gva == (addr & PAGE_MASK)) && vio->mmio_gva )
{
- unsigned int off = addr & (PAGE_SIZE - 1);
gpa = (((paddr_t)vio->mmio_gpfn << PAGE_SHIFT) | off);
- if ( (off + bytes) <= PAGE_SIZE )
- return hvmemul_do_mmio(gpa, &reps, bytes, 0,
- IOREQ_WRITE, 0, p_data);
+ while ( (off + chunk) <= PAGE_SIZE )
+ {
+ rc = hvmemul_do_mmio(gpa, &reps, chunk, 0, IOREQ_WRITE, 0, p_data);
+ if ( rc != X86EMUL_OKAY || bytes == chunk )
+ return rc;
+ addr += chunk;
+ off += chunk;
+ gpa += chunk;
+ p_data += chunk;
+ bytes -= chunk;
+ if ( bytes < chunk )
+ chunk = bytes;
+ }
}
if ( (seg != x86_seg_none) &&
@@ -569,12 +627,29 @@ static int hvmemul_write(
case HVMCOPY_unhandleable:
return X86EMUL_UNHANDLEABLE;
case HVMCOPY_bad_gfn_to_mfn:
- rc = hvmemul_linear_to_phys(
- addr, &gpa, bytes, &reps, pfec, hvmemul_ctxt);
- if ( rc != X86EMUL_OKAY )
- return rc;
- return hvmemul_do_mmio(gpa, &reps, bytes, 0,
- IOREQ_WRITE, 0, p_data);
+ rc = hvmemul_linear_to_phys(addr, &gpa, chunk, &reps, pfec,
+ hvmemul_ctxt);
+ while ( rc == X86EMUL_OKAY )
+ {
+ rc = hvmemul_do_mmio(gpa, &reps, chunk, 0, IOREQ_WRITE, 0, p_data);
+ if ( rc != X86EMUL_OKAY || bytes == chunk )
+ break;
+ addr += chunk;
+ off += chunk;
+ p_data += chunk;
+ bytes -= chunk;
+ if ( bytes < chunk )
+ chunk = bytes;
+ if ( off < PAGE_SIZE )
+ gpa += chunk;
+ else
+ {
+ rc = hvmemul_linear_to_phys(addr, &gpa, chunk, &reps, pfec,
+ hvmemul_ctxt);
+ off = 0;
+ }
+ }
+ return rc;
case HVMCOPY_gfn_paged_out:
return X86EMUL_RETRY;
case HVMCOPY_gfn_shared:
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 2/6] x86: fix dependencies of emulator test
2013-09-19 15:40 [PATCH 0/6] x86: emulation improvements Jan Beulich
2013-09-19 15:42 ` [PATCH 1/6] x86/HVM: properly handle MMIO reads and writes wider than a machine word Jan Beulich
@ 2013-09-19 15:43 ` Jan Beulich
2013-09-19 15:44 ` [PATCH 3/6] x86_emulate: MOVSXD must read source operand just once Jan Beulich
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2013-09-19 15:43 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]
Rather than mentioning the (linked) directory, mention the files thus
making sure things get rebuild as needed when the core emulator files
change.
Also enable debug info generation unconditionally, as this is testing
stuff only anyway.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -37,14 +37,13 @@ clean:
.PHONY: install
install:
-.PHONY: x86_emulate
-x86_emulate:
+x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
HOSTCFLAGS += $(CFLAGS_xeninclude)
-x86_emulate.o: x86_emulate.c x86_emulate
- $(HOSTCC) $(HOSTCFLAGS) -c -o $@ $<
+x86_emulate.o: x86_emulate.c x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
+ $(HOSTCC) $(HOSTCFLAGS) -c -g -o $@ $<
-test_x86_emulator.o: test_x86_emulator.c blowfish.h x86_emulate
- $(HOSTCC) $(HOSTCFLAGS) -c -o $@ $<
+test_x86_emulator.o: test_x86_emulator.c blowfish.h x86_emulate/x86_emulate.h
+ $(HOSTCC) $(HOSTCFLAGS) -c -g -o $@ $<
[-- Attachment #2: x86-emul-test-deps.patch --]
[-- Type: text/plain, Size: 1123 bytes --]
x86: fix dependencies of emulator test
Rather than mentioning the (linked) directory, mention the files thus
making sure things get rebuild as needed when the core emulator files
change.
Also enable debug info generation unconditionally, as this is testing
stuff only anyway.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -37,14 +37,13 @@ clean:
.PHONY: install
install:
-.PHONY: x86_emulate
-x86_emulate:
+x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
HOSTCFLAGS += $(CFLAGS_xeninclude)
-x86_emulate.o: x86_emulate.c x86_emulate
- $(HOSTCC) $(HOSTCFLAGS) -c -o $@ $<
+x86_emulate.o: x86_emulate.c x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
+ $(HOSTCC) $(HOSTCFLAGS) -c -g -o $@ $<
-test_x86_emulator.o: test_x86_emulator.c blowfish.h x86_emulate
- $(HOSTCC) $(HOSTCFLAGS) -c -o $@ $<
+test_x86_emulator.o: test_x86_emulator.c blowfish.h x86_emulate/x86_emulate.h
+ $(HOSTCC) $(HOSTCFLAGS) -c -g -o $@ $<
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 3/6] x86_emulate: MOVSXD must read source operand just once
2013-09-19 15:40 [PATCH 0/6] x86: emulation improvements Jan Beulich
2013-09-19 15:42 ` [PATCH 1/6] x86/HVM: properly handle MMIO reads and writes wider than a machine word Jan Beulich
2013-09-19 15:43 ` [PATCH 2/6] x86: fix dependencies of emulator test Jan Beulich
@ 2013-09-19 15:44 ` Jan Beulich
2013-09-19 15:44 ` [PATCH 4/6] x86_emulate: PUSH <mem> " Jan Beulich
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2013-09-19 15:44 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 6545 bytes --]
... for the case of accessing MMIO.
Also streamline the ARPL emulation a little, and add tests for both
instructions (the MOVSXD one requires a few other adjustments, as we
now need to run in a mode where the emulator's mode_64bit() returns
true).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -21,6 +21,8 @@
#define EFLG_PF (1<<2)
#define EFLG_CF (1<<0)
+static unsigned int bytes_read;
+
static int read(
unsigned int seg,
unsigned long offset,
@@ -28,6 +30,18 @@ static int read(
unsigned int bytes,
struct x86_emulate_ctxt *ctxt)
{
+ bytes_read += bytes;
+ memcpy(p_data, (void *)offset, bytes);
+ return X86EMUL_OKAY;
+}
+
+static int fetch(
+ unsigned int seg,
+ unsigned long offset,
+ void *p_data,
+ unsigned int bytes,
+ struct x86_emulate_ctxt *ctxt)
+{
memcpy(p_data, (void *)offset, bytes);
return X86EMUL_OKAY;
}
@@ -140,7 +154,7 @@ int get_fpu(
static struct x86_emulate_ops emulops = {
.read = read,
- .insn_fetch = read,
+ .insn_fetch = fetch,
.write = write,
.cmpxchg = cmpxchg,
.cpuid = cpuid,
@@ -162,8 +176,8 @@ int main(int argc, char **argv)
ctxt.regs = ®s;
ctxt.force_writeback = 0;
- ctxt.addr_size = 32;
- ctxt.sp_size = 32;
+ ctxt.addr_size = 8 * sizeof(void *);
+ ctxt.sp_size = 8 * sizeof(void *);
res = mmap((void *)0x100000, MMAP_SZ, PROT_READ|PROT_WRITE|PROT_EXEC,
MAP_FIXED|MAP_PRIVATE|MAP_ANONYMOUS, 0, 0);
@@ -428,6 +442,44 @@ int main(int argc, char **argv)
goto fail;
printf("okay\n");
+#ifndef __x86_64__
+ printf("%-40s", "Testing arpl %cx,(%%eax)...");
+ instr[0] = 0x63; instr[1] = 0x08;
+ regs.eflags = 0x200;
+ regs.eip = (unsigned long)&instr[0];
+ regs.ecx = 0x22222222;
+ regs.eax = (unsigned long)res;
+ *res = 0x33331111;
+ bytes_read = 0;
+ rc = x86_emulate(&ctxt, &emulops);
+ if ( (rc != X86EMUL_OKAY) ||
+ (*res != 0x33331112) ||
+ (regs.ecx != 0x22222222) ||
+ !(regs.eflags & EFLG_ZF) ||
+ (regs.eip != (unsigned long)&instr[2]) )
+ goto fail;
+#else
+ printf("%-40s", "Testing movsxd (%%rax),%%rcx...");
+ instr[0] = 0x48; instr[1] = 0x63; instr[2] = 0x08;
+ regs.eip = (unsigned long)&instr[0];
+ regs.ecx = 0x123456789abcdef;
+ regs.eax = (unsigned long)res;
+ *res = 0xfedcba98;
+ bytes_read = 0;
+ rc = x86_emulate(&ctxt, &emulops);
+ if ( (rc != X86EMUL_OKAY) ||
+ (*res != 0xfedcba98) ||
+ (regs.ecx != 0xfffffffffedcba98) ||
+ (regs.eip != (unsigned long)&instr[3]) )
+ goto fail;
+ if ( bytes_read != 4 )
+ {
+ printf("%u bytes read - ", bytes_read);
+ goto fail;
+ }
+#endif
+ printf("okay\n");
+
printf("%-40s", "Testing xadd %%ax,(%%ecx)...");
instr[0] = 0x66; instr[1] = 0x0f; instr[2] = 0xc1; instr[3] = 0x01;
regs.eflags = 0x200;
@@ -445,7 +497,11 @@ int main(int argc, char **argv)
printf("okay\n");
printf("%-40s", "Testing dec %%ax...");
+#ifndef __x86_64__
instr[0] = 0x66; instr[1] = 0x48;
+#else
+ instr[0] = 0x66; instr[1] = 0xff; instr[2] = 0xc8;
+#endif
regs.eflags = 0x200;
regs.eip = (unsigned long)&instr[0];
regs.eax = 0x00000000;
@@ -453,7 +509,7 @@ int main(int argc, char **argv)
if ( (rc != X86EMUL_OKAY) ||
(regs.eax != 0x0000ffff) ||
((regs.eflags&0x240) != 0x200) ||
- (regs.eip != (unsigned long)&instr[2]) )
+ (regs.eip != (unsigned long)&instr[2 + (ctxt.addr_size > 32)]) )
goto fail;
printf("okay\n");
@@ -821,6 +877,8 @@ int main(int argc, char **argv)
if ( j == 2 ) break;
memcpy(res, blowfish32_code, sizeof(blowfish32_code));
#else
+ ctxt.addr_size = 16 << j;
+ ctxt.sp_size = 16 << j;
memcpy(res, (j == 1) ? blowfish32_code : blowfish64_code,
(j == 1) ? sizeof(blowfish32_code) : sizeof(blowfish64_code));
#endif
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -92,7 +92,7 @@ static uint8_t opcode_table[256] = {
ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps|Mov,
ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps|Mov,
/* 0x60 - 0x67 */
- ImplicitOps, ImplicitOps, DstReg|SrcMem|ModRM, DstReg|SrcMem16|ModRM|Mov,
+ ImplicitOps, ImplicitOps, DstReg|SrcMem|ModRM, DstReg|SrcNone|ModRM|Mov,
0, 0, 0, 0,
/* 0x68 - 0x6F */
ImplicitOps|Mov, DstReg|SrcImm|ModRM|Mov,
@@ -2021,9 +2021,9 @@ x86_emulate(
if ( mode_64bit() )
{
/* movsxd */
- if ( src.type == OP_REG )
- src.val = *(int32_t *)src.reg;
- else if ( (rc = read_ulong(src.mem.seg, src.mem.off,
+ if ( ea.type == OP_REG )
+ src.val = *ea.reg;
+ else if ( (rc = read_ulong(ea.mem.seg, ea.mem.off,
&src.val, 4, ctxt, ops)) )
goto done;
dst.val = (int32_t)src.val;
@@ -2031,14 +2031,25 @@ x86_emulate(
else
{
/* arpl */
- uint16_t src_val = dst.val;
- dst = src;
- _regs.eflags &= ~EFLG_ZF;
- _regs.eflags |= ((src_val & 3) > (dst.val & 3)) ? EFLG_ZF : 0;
- if ( _regs.eflags & EFLG_ZF )
- dst.val = (dst.val & ~3) | (src_val & 3);
+ unsigned int src_rpl = dst.val & 3;
+
+ dst = ea;
+ dst.bytes = 2;
+ if ( dst.type == OP_REG )
+ dst.val = *dst.reg;
+ else if ( (rc = read_ulong(dst.mem.seg, dst.mem.off,
+ &dst.val, 2, ctxt, ops)) )
+ goto done;
+ if ( src_rpl > (dst.val & 3) )
+ {
+ _regs.eflags |= EFLG_ZF;
+ dst.val = (dst.val & ~3) | src_rpl;
+ }
else
+ {
+ _regs.eflags &= ~EFLG_ZF;
dst.type = OP_NONE;
+ }
generate_exception_if(!in_protmode(ctxt, ops), EXC_UD, -1);
}
break;
[-- Attachment #2: x86-emul-movsxd.patch --]
[-- Type: text/plain, Size: 6599 bytes --]
x86_emulate: MOVSXD must read source operand just once
... for the case of accessing MMIO.
Also streamline the ARPL emulation a little, and add tests for both
instructions (the MOVSXD one requires a few other adjustments, as we
now need to run in a mode where the emulator's mode_64bit() returns
true).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -21,6 +21,8 @@
#define EFLG_PF (1<<2)
#define EFLG_CF (1<<0)
+static unsigned int bytes_read;
+
static int read(
unsigned int seg,
unsigned long offset,
@@ -28,6 +30,18 @@ static int read(
unsigned int bytes,
struct x86_emulate_ctxt *ctxt)
{
+ bytes_read += bytes;
+ memcpy(p_data, (void *)offset, bytes);
+ return X86EMUL_OKAY;
+}
+
+static int fetch(
+ unsigned int seg,
+ unsigned long offset,
+ void *p_data,
+ unsigned int bytes,
+ struct x86_emulate_ctxt *ctxt)
+{
memcpy(p_data, (void *)offset, bytes);
return X86EMUL_OKAY;
}
@@ -140,7 +154,7 @@ int get_fpu(
static struct x86_emulate_ops emulops = {
.read = read,
- .insn_fetch = read,
+ .insn_fetch = fetch,
.write = write,
.cmpxchg = cmpxchg,
.cpuid = cpuid,
@@ -162,8 +176,8 @@ int main(int argc, char **argv)
ctxt.regs = ®s;
ctxt.force_writeback = 0;
- ctxt.addr_size = 32;
- ctxt.sp_size = 32;
+ ctxt.addr_size = 8 * sizeof(void *);
+ ctxt.sp_size = 8 * sizeof(void *);
res = mmap((void *)0x100000, MMAP_SZ, PROT_READ|PROT_WRITE|PROT_EXEC,
MAP_FIXED|MAP_PRIVATE|MAP_ANONYMOUS, 0, 0);
@@ -428,6 +442,44 @@ int main(int argc, char **argv)
goto fail;
printf("okay\n");
+#ifndef __x86_64__
+ printf("%-40s", "Testing arpl %cx,(%%eax)...");
+ instr[0] = 0x63; instr[1] = 0x08;
+ regs.eflags = 0x200;
+ regs.eip = (unsigned long)&instr[0];
+ regs.ecx = 0x22222222;
+ regs.eax = (unsigned long)res;
+ *res = 0x33331111;
+ bytes_read = 0;
+ rc = x86_emulate(&ctxt, &emulops);
+ if ( (rc != X86EMUL_OKAY) ||
+ (*res != 0x33331112) ||
+ (regs.ecx != 0x22222222) ||
+ !(regs.eflags & EFLG_ZF) ||
+ (regs.eip != (unsigned long)&instr[2]) )
+ goto fail;
+#else
+ printf("%-40s", "Testing movsxd (%%rax),%%rcx...");
+ instr[0] = 0x48; instr[1] = 0x63; instr[2] = 0x08;
+ regs.eip = (unsigned long)&instr[0];
+ regs.ecx = 0x123456789abcdef;
+ regs.eax = (unsigned long)res;
+ *res = 0xfedcba98;
+ bytes_read = 0;
+ rc = x86_emulate(&ctxt, &emulops);
+ if ( (rc != X86EMUL_OKAY) ||
+ (*res != 0xfedcba98) ||
+ (regs.ecx != 0xfffffffffedcba98) ||
+ (regs.eip != (unsigned long)&instr[3]) )
+ goto fail;
+ if ( bytes_read != 4 )
+ {
+ printf("%u bytes read - ", bytes_read);
+ goto fail;
+ }
+#endif
+ printf("okay\n");
+
printf("%-40s", "Testing xadd %%ax,(%%ecx)...");
instr[0] = 0x66; instr[1] = 0x0f; instr[2] = 0xc1; instr[3] = 0x01;
regs.eflags = 0x200;
@@ -445,7 +497,11 @@ int main(int argc, char **argv)
printf("okay\n");
printf("%-40s", "Testing dec %%ax...");
+#ifndef __x86_64__
instr[0] = 0x66; instr[1] = 0x48;
+#else
+ instr[0] = 0x66; instr[1] = 0xff; instr[2] = 0xc8;
+#endif
regs.eflags = 0x200;
regs.eip = (unsigned long)&instr[0];
regs.eax = 0x00000000;
@@ -453,7 +509,7 @@ int main(int argc, char **argv)
if ( (rc != X86EMUL_OKAY) ||
(regs.eax != 0x0000ffff) ||
((regs.eflags&0x240) != 0x200) ||
- (regs.eip != (unsigned long)&instr[2]) )
+ (regs.eip != (unsigned long)&instr[2 + (ctxt.addr_size > 32)]) )
goto fail;
printf("okay\n");
@@ -821,6 +877,8 @@ int main(int argc, char **argv)
if ( j == 2 ) break;
memcpy(res, blowfish32_code, sizeof(blowfish32_code));
#else
+ ctxt.addr_size = 16 << j;
+ ctxt.sp_size = 16 << j;
memcpy(res, (j == 1) ? blowfish32_code : blowfish64_code,
(j == 1) ? sizeof(blowfish32_code) : sizeof(blowfish64_code));
#endif
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -92,7 +92,7 @@ static uint8_t opcode_table[256] = {
ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps|Mov,
ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps|Mov,
/* 0x60 - 0x67 */
- ImplicitOps, ImplicitOps, DstReg|SrcMem|ModRM, DstReg|SrcMem16|ModRM|Mov,
+ ImplicitOps, ImplicitOps, DstReg|SrcMem|ModRM, DstReg|SrcNone|ModRM|Mov,
0, 0, 0, 0,
/* 0x68 - 0x6F */
ImplicitOps|Mov, DstReg|SrcImm|ModRM|Mov,
@@ -2021,9 +2021,9 @@ x86_emulate(
if ( mode_64bit() )
{
/* movsxd */
- if ( src.type == OP_REG )
- src.val = *(int32_t *)src.reg;
- else if ( (rc = read_ulong(src.mem.seg, src.mem.off,
+ if ( ea.type == OP_REG )
+ src.val = *ea.reg;
+ else if ( (rc = read_ulong(ea.mem.seg, ea.mem.off,
&src.val, 4, ctxt, ops)) )
goto done;
dst.val = (int32_t)src.val;
@@ -2031,14 +2031,25 @@ x86_emulate(
else
{
/* arpl */
- uint16_t src_val = dst.val;
- dst = src;
- _regs.eflags &= ~EFLG_ZF;
- _regs.eflags |= ((src_val & 3) > (dst.val & 3)) ? EFLG_ZF : 0;
- if ( _regs.eflags & EFLG_ZF )
- dst.val = (dst.val & ~3) | (src_val & 3);
+ unsigned int src_rpl = dst.val & 3;
+
+ dst = ea;
+ dst.bytes = 2;
+ if ( dst.type == OP_REG )
+ dst.val = *dst.reg;
+ else if ( (rc = read_ulong(dst.mem.seg, dst.mem.off,
+ &dst.val, 2, ctxt, ops)) )
+ goto done;
+ if ( src_rpl > (dst.val & 3) )
+ {
+ _regs.eflags |= EFLG_ZF;
+ dst.val = (dst.val & ~3) | src_rpl;
+ }
else
+ {
+ _regs.eflags &= ~EFLG_ZF;
dst.type = OP_NONE;
+ }
generate_exception_if(!in_protmode(ctxt, ops), EXC_UD, -1);
}
break;
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 4/6] x86_emulate: PUSH <mem> must read source operand just once
2013-09-19 15:40 [PATCH 0/6] x86: emulation improvements Jan Beulich
` (2 preceding siblings ...)
2013-09-19 15:44 ` [PATCH 3/6] x86_emulate: MOVSXD must read source operand just once Jan Beulich
@ 2013-09-19 15:44 ` Jan Beulich
2013-09-19 15:45 ` [PATCH 5/6] x86_emulate: fix flag setting for 8-bit signed multiplication Jan Beulich
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2013-09-19 15:44 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 6109 bytes --]
... for the case of accessing MMIO.
Rather than doing the early operand type adjustment for just for that
case, do it for all of the 0xF6, 0xF7, and 0xFF groups (allowing some
other code to be dropped instead).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1632,6 +1632,41 @@ x86_emulate(
if ( override_seg != -1 )
ea.mem.seg = override_seg;
+ /* Early operand adjustments. */
+ if ( !twobyte )
+ switch ( b )
+ {
+ case 0xf6 ... 0xf7: /* Grp3 */
+ switch ( modrm_reg & 7 )
+ {
+ case 0 ... 1: /* test */
+ d = (d & ~SrcMask) | SrcImm;
+ break;
+ case 4: /* mul */
+ case 5: /* imul */
+ case 6: /* div */
+ case 7: /* idiv */
+ d = (d & (ByteOp | ModRM)) | DstImplicit | SrcMem;
+ break;
+ }
+ break;
+ case 0xff: /* Grp5 */
+ switch ( modrm_reg & 7 )
+ {
+ case 2: /* call (near) */
+ case 4: /* jmp (near) */
+ case 6: /* push */
+ if ( mode_64bit() && op_bytes == 4 )
+ op_bytes = 8;
+ /* fall through */
+ case 3: /* call (far, absolute indirect) */
+ case 5: /* jmp (far, absolute indirect) */
+ d = DstNone|SrcMem|ModRM;
+ break;
+ }
+ break;
+ }
+
/* Decode and fetch the source operand: register, memory or immediate. */
switch ( d & SrcMask )
{
@@ -3318,16 +3353,6 @@ x86_emulate(
switch ( modrm_reg & 7 )
{
case 0 ... 1: /* test */
- /* Special case in Grp3: test has an immediate source operand. */
- src.type = OP_IMM;
- src.bytes = (d & ByteOp) ? 1 : op_bytes;
- if ( src.bytes == 8 ) src.bytes = 4;
- switch ( src.bytes )
- {
- case 1: src.val = insn_fetch_type(int8_t); break;
- case 2: src.val = insn_fetch_type(int16_t); break;
- case 4: src.val = insn_fetch_type(int32_t); break;
- }
goto test;
case 2: /* not */
dst.val = ~dst.val;
@@ -3336,12 +3361,11 @@ x86_emulate(
emulate_1op("neg", dst, _regs.eflags);
break;
case 4: /* mul */
- src = dst;
dst.type = OP_REG;
dst.reg = (unsigned long *)&_regs.eax;
dst.val = *dst.reg;
_regs.eflags &= ~(EFLG_OF|EFLG_CF);
- switch ( src.bytes )
+ switch ( dst.bytes = src.bytes )
{
case 1:
dst.val = (uint8_t)dst.val;
@@ -3377,12 +3401,11 @@ x86_emulate(
}
break;
case 5: /* imul */
- src = dst;
dst.type = OP_REG;
dst.reg = (unsigned long *)&_regs.eax;
dst.val = *dst.reg;
_regs.eflags &= ~(EFLG_OF|EFLG_CF);
- switch ( src.bytes )
+ switch ( dst.bytes = src.bytes )
{
case 1:
dst.val = ((uint16_t)(int8_t)src.val *
@@ -3419,10 +3442,10 @@ x86_emulate(
break;
case 6: /* div */ {
unsigned long u[2], v;
- src = dst;
+
dst.type = OP_REG;
dst.reg = (unsigned long *)&_regs.eax;
- switch ( src.bytes )
+ switch ( dst.bytes = src.bytes )
{
case 1:
u[0] = (uint16_t)_regs.eax;
@@ -3469,10 +3492,10 @@ x86_emulate(
}
case 7: /* idiv */ {
unsigned long u[2], v;
- src = dst;
+
dst.type = OP_REG;
dst.reg = (unsigned long *)&_regs.eax;
- switch ( src.bytes )
+ switch ( dst.bytes = src.bytes )
{
case 1:
u[0] = (int16_t)_regs.eax;
@@ -3564,20 +3587,13 @@ x86_emulate(
emulate_1op("dec", dst, _regs.eflags);
break;
case 2: /* call (near) */
+ dst.val = _regs.eip;
+ _regs.eip = src.val;
+ src.val = dst.val;
+ goto push;
+ break;
case 4: /* jmp (near) */
- if ( (dst.bytes == 4) && mode_64bit() )
- {
- dst.bytes = op_bytes = 8;
- if ( dst.type == OP_REG )
- dst.val = *dst.reg;
- else if ( (rc = read_ulong(dst.mem.seg, dst.mem.off,
- &dst.val, 8, ctxt, ops)) != 0 )
- goto done;
- }
- src.val = _regs.eip;
- _regs.eip = dst.val;
- if ( (modrm_reg & 7) == 2 )
- goto push; /* call */
+ _regs.eip = src.val;
dst.type = OP_NONE;
break;
case 3: /* call (far, absolute indirect) */
@@ -3610,21 +3626,7 @@ x86_emulate(
break;
}
case 6: /* push */
- /* 64-bit mode: PUSH defaults to a 64-bit operand. */
- if ( mode_64bit() && (dst.bytes == 4) )
- {
- dst.bytes = 8;
- if ( dst.type == OP_REG )
- dst.val = *dst.reg;
- else if ( (rc = read_ulong(dst.mem.seg, dst.mem.off,
- &dst.val, 8, ctxt, ops)) != 0 )
- goto done;
- }
- if ( (rc = ops->write(x86_seg_ss, sp_pre_dec(dst.bytes),
- &dst.val, dst.bytes, ctxt)) != 0 )
- goto done;
- dst.type = OP_NONE;
- break;
+ goto push;
case 7:
generate_exception_if(1, EXC_UD, -1);
default:
[-- Attachment #2: x86-emul-push-mem.patch --]
[-- Type: text/plain, Size: 6167 bytes --]
x86_emulate: PUSH <mem> must read source operand just once
... for the case of accessing MMIO.
Rather than doing the early operand type adjustment for just for that
case, do it for all of the 0xF6, 0xF7, and 0xFF groups (allowing some
other code to be dropped instead).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1632,6 +1632,41 @@ x86_emulate(
if ( override_seg != -1 )
ea.mem.seg = override_seg;
+ /* Early operand adjustments. */
+ if ( !twobyte )
+ switch ( b )
+ {
+ case 0xf6 ... 0xf7: /* Grp3 */
+ switch ( modrm_reg & 7 )
+ {
+ case 0 ... 1: /* test */
+ d = (d & ~SrcMask) | SrcImm;
+ break;
+ case 4: /* mul */
+ case 5: /* imul */
+ case 6: /* div */
+ case 7: /* idiv */
+ d = (d & (ByteOp | ModRM)) | DstImplicit | SrcMem;
+ break;
+ }
+ break;
+ case 0xff: /* Grp5 */
+ switch ( modrm_reg & 7 )
+ {
+ case 2: /* call (near) */
+ case 4: /* jmp (near) */
+ case 6: /* push */
+ if ( mode_64bit() && op_bytes == 4 )
+ op_bytes = 8;
+ /* fall through */
+ case 3: /* call (far, absolute indirect) */
+ case 5: /* jmp (far, absolute indirect) */
+ d = DstNone|SrcMem|ModRM;
+ break;
+ }
+ break;
+ }
+
/* Decode and fetch the source operand: register, memory or immediate. */
switch ( d & SrcMask )
{
@@ -3318,16 +3353,6 @@ x86_emulate(
switch ( modrm_reg & 7 )
{
case 0 ... 1: /* test */
- /* Special case in Grp3: test has an immediate source operand. */
- src.type = OP_IMM;
- src.bytes = (d & ByteOp) ? 1 : op_bytes;
- if ( src.bytes == 8 ) src.bytes = 4;
- switch ( src.bytes )
- {
- case 1: src.val = insn_fetch_type(int8_t); break;
- case 2: src.val = insn_fetch_type(int16_t); break;
- case 4: src.val = insn_fetch_type(int32_t); break;
- }
goto test;
case 2: /* not */
dst.val = ~dst.val;
@@ -3336,12 +3361,11 @@ x86_emulate(
emulate_1op("neg", dst, _regs.eflags);
break;
case 4: /* mul */
- src = dst;
dst.type = OP_REG;
dst.reg = (unsigned long *)&_regs.eax;
dst.val = *dst.reg;
_regs.eflags &= ~(EFLG_OF|EFLG_CF);
- switch ( src.bytes )
+ switch ( dst.bytes = src.bytes )
{
case 1:
dst.val = (uint8_t)dst.val;
@@ -3377,12 +3401,11 @@ x86_emulate(
}
break;
case 5: /* imul */
- src = dst;
dst.type = OP_REG;
dst.reg = (unsigned long *)&_regs.eax;
dst.val = *dst.reg;
_regs.eflags &= ~(EFLG_OF|EFLG_CF);
- switch ( src.bytes )
+ switch ( dst.bytes = src.bytes )
{
case 1:
dst.val = ((uint16_t)(int8_t)src.val *
@@ -3419,10 +3442,10 @@ x86_emulate(
break;
case 6: /* div */ {
unsigned long u[2], v;
- src = dst;
+
dst.type = OP_REG;
dst.reg = (unsigned long *)&_regs.eax;
- switch ( src.bytes )
+ switch ( dst.bytes = src.bytes )
{
case 1:
u[0] = (uint16_t)_regs.eax;
@@ -3469,10 +3492,10 @@ x86_emulate(
}
case 7: /* idiv */ {
unsigned long u[2], v;
- src = dst;
+
dst.type = OP_REG;
dst.reg = (unsigned long *)&_regs.eax;
- switch ( src.bytes )
+ switch ( dst.bytes = src.bytes )
{
case 1:
u[0] = (int16_t)_regs.eax;
@@ -3564,20 +3587,13 @@ x86_emulate(
emulate_1op("dec", dst, _regs.eflags);
break;
case 2: /* call (near) */
+ dst.val = _regs.eip;
+ _regs.eip = src.val;
+ src.val = dst.val;
+ goto push;
+ break;
case 4: /* jmp (near) */
- if ( (dst.bytes == 4) && mode_64bit() )
- {
- dst.bytes = op_bytes = 8;
- if ( dst.type == OP_REG )
- dst.val = *dst.reg;
- else if ( (rc = read_ulong(dst.mem.seg, dst.mem.off,
- &dst.val, 8, ctxt, ops)) != 0 )
- goto done;
- }
- src.val = _regs.eip;
- _regs.eip = dst.val;
- if ( (modrm_reg & 7) == 2 )
- goto push; /* call */
+ _regs.eip = src.val;
dst.type = OP_NONE;
break;
case 3: /* call (far, absolute indirect) */
@@ -3610,21 +3626,7 @@ x86_emulate(
break;
}
case 6: /* push */
- /* 64-bit mode: PUSH defaults to a 64-bit operand. */
- if ( mode_64bit() && (dst.bytes == 4) )
- {
- dst.bytes = 8;
- if ( dst.type == OP_REG )
- dst.val = *dst.reg;
- else if ( (rc = read_ulong(dst.mem.seg, dst.mem.off,
- &dst.val, 8, ctxt, ops)) != 0 )
- goto done;
- }
- if ( (rc = ops->write(x86_seg_ss, sp_pre_dec(dst.bytes),
- &dst.val, dst.bytes, ctxt)) != 0 )
- goto done;
- dst.type = OP_NONE;
- break;
+ goto push;
case 7:
generate_exception_if(1, EXC_UD, -1);
default:
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 5/6] x86_emulate: fix flag setting for 8-bit signed multiplication
2013-09-19 15:40 [PATCH 0/6] x86: emulation improvements Jan Beulich
` (3 preceding siblings ...)
2013-09-19 15:44 ` [PATCH 4/6] x86_emulate: PUSH <mem> " Jan Beulich
@ 2013-09-19 15:45 ` Jan Beulich
2013-09-19 15:46 ` [PATCH 6/6] x86_emulate: fold wide reads Jan Beulich
2013-09-19 16:12 ` [PATCH 0/6] x86: emulation improvements Keir Fraser
6 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2013-09-19 15:45 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 4271 bytes --]
We really need to check for a signed overflow of 8 bits, while the
previous check compared the sign-extended 8-bit result with the
zero-extended 16-bit one (which was wrong for all negative results).
Once at it
- also adjust the 16-bit comparison for symmetry
- improve the 8-bit multiplication (no need to zero-extend to 32-bits
the sign-extended to 16 bits original 8-bit value)
- fold both signed multiplication variants
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2096,40 +2096,13 @@ x86_emulate(
goto push;
case 0x69: /* imul imm16/32 */
- case 0x6b: /* imul imm8 */ {
- unsigned long src1; /* ModR/M source operand */
+ case 0x6b: /* imul imm8 */
if ( ea.type == OP_REG )
- src1 = *ea.reg;
+ dst.val = *ea.reg;
else if ( (rc = read_ulong(ea.mem.seg, ea.mem.off,
- &src1, op_bytes, ctxt, ops)) )
+ &dst.val, op_bytes, ctxt, ops)) )
goto done;
- _regs.eflags &= ~(EFLG_OF|EFLG_CF);
- switch ( dst.bytes )
- {
- case 2:
- dst.val = ((uint32_t)(int16_t)src.val *
- (uint32_t)(int16_t)src1);
- if ( (int16_t)dst.val != (uint32_t)dst.val )
- _regs.eflags |= EFLG_OF|EFLG_CF;
- break;
-#ifdef __x86_64__
- case 4:
- dst.val = ((uint64_t)(int32_t)src.val *
- (uint64_t)(int32_t)src1);
- if ( (int32_t)dst.val != dst.val )
- _regs.eflags |= EFLG_OF|EFLG_CF;
- break;
-#endif
- default: {
- unsigned long m[2] = { src.val, src1 };
- if ( imul_dbl(m) )
- _regs.eflags |= EFLG_OF|EFLG_CF;
- dst.val = m[0];
- break;
- }
- }
- break;
- }
+ goto imul;
case 0x6a: /* push imm8 */
src.val = insn_fetch_type(int8_t);
@@ -3404,22 +3377,25 @@ x86_emulate(
dst.type = OP_REG;
dst.reg = (unsigned long *)&_regs.eax;
dst.val = *dst.reg;
+ dst.bytes = src.bytes;
+ imul:
_regs.eflags &= ~(EFLG_OF|EFLG_CF);
- switch ( dst.bytes = src.bytes )
+ switch ( dst.bytes )
{
case 1:
- dst.val = ((uint16_t)(int8_t)src.val *
- (uint16_t)(int8_t)dst.val);
- if ( (int8_t)dst.val != (uint16_t)dst.val )
+ dst.val = (int8_t)src.val * (int8_t)dst.val;
+ if ( (int8_t)dst.val != (int16_t)dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
- dst.bytes = 2;
+ if ( b > 0x6b )
+ dst.bytes = 2;
break;
case 2:
dst.val = ((uint32_t)(int16_t)src.val *
(uint32_t)(int16_t)dst.val);
- if ( (int16_t)dst.val != (uint32_t)dst.val )
+ if ( (int16_t)dst.val != (int32_t)dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
- *(uint16_t *)&_regs.edx = dst.val >> 16;
+ if ( b > 0x6b )
+ *(uint16_t *)&_regs.edx = dst.val >> 16;
break;
#ifdef __x86_64__
case 4:
@@ -3427,14 +3403,16 @@ x86_emulate(
(uint64_t)(int32_t)dst.val);
if ( (int32_t)dst.val != dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
- _regs.edx = (uint32_t)(dst.val >> 32);
+ if ( b > 0x6b )
+ _regs.edx = (uint32_t)(dst.val >> 32);
break;
#endif
default: {
unsigned long m[2] = { src.val, dst.val };
if ( imul_dbl(m) )
_regs.eflags |= EFLG_OF|EFLG_CF;
- _regs.edx = m[1];
+ if ( b > 0x6b )
+ _regs.edx = m[1];
dst.val = m[0];
break;
}
[-- Attachment #2: x86-emul-imul-flags.patch --]
[-- Type: text/plain, Size: 4332 bytes --]
x86_emulate: fix flag setting for 8-bit signed multiplication
We really need to check for a signed overflow of 8 bits, while the
previous check compared the sign-extended 8-bit result with the
zero-extended 16-bit one (which was wrong for all negative results).
Once at it
- also adjust the 16-bit comparison for symmetry
- improve the 8-bit multiplication (no need to zero-extend to 32-bits
the sign-extended to 16 bits original 8-bit value)
- fold both signed multiplication variants
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2096,40 +2096,13 @@ x86_emulate(
goto push;
case 0x69: /* imul imm16/32 */
- case 0x6b: /* imul imm8 */ {
- unsigned long src1; /* ModR/M source operand */
+ case 0x6b: /* imul imm8 */
if ( ea.type == OP_REG )
- src1 = *ea.reg;
+ dst.val = *ea.reg;
else if ( (rc = read_ulong(ea.mem.seg, ea.mem.off,
- &src1, op_bytes, ctxt, ops)) )
+ &dst.val, op_bytes, ctxt, ops)) )
goto done;
- _regs.eflags &= ~(EFLG_OF|EFLG_CF);
- switch ( dst.bytes )
- {
- case 2:
- dst.val = ((uint32_t)(int16_t)src.val *
- (uint32_t)(int16_t)src1);
- if ( (int16_t)dst.val != (uint32_t)dst.val )
- _regs.eflags |= EFLG_OF|EFLG_CF;
- break;
-#ifdef __x86_64__
- case 4:
- dst.val = ((uint64_t)(int32_t)src.val *
- (uint64_t)(int32_t)src1);
- if ( (int32_t)dst.val != dst.val )
- _regs.eflags |= EFLG_OF|EFLG_CF;
- break;
-#endif
- default: {
- unsigned long m[2] = { src.val, src1 };
- if ( imul_dbl(m) )
- _regs.eflags |= EFLG_OF|EFLG_CF;
- dst.val = m[0];
- break;
- }
- }
- break;
- }
+ goto imul;
case 0x6a: /* push imm8 */
src.val = insn_fetch_type(int8_t);
@@ -3404,22 +3377,25 @@ x86_emulate(
dst.type = OP_REG;
dst.reg = (unsigned long *)&_regs.eax;
dst.val = *dst.reg;
+ dst.bytes = src.bytes;
+ imul:
_regs.eflags &= ~(EFLG_OF|EFLG_CF);
- switch ( dst.bytes = src.bytes )
+ switch ( dst.bytes )
{
case 1:
- dst.val = ((uint16_t)(int8_t)src.val *
- (uint16_t)(int8_t)dst.val);
- if ( (int8_t)dst.val != (uint16_t)dst.val )
+ dst.val = (int8_t)src.val * (int8_t)dst.val;
+ if ( (int8_t)dst.val != (int16_t)dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
- dst.bytes = 2;
+ if ( b > 0x6b )
+ dst.bytes = 2;
break;
case 2:
dst.val = ((uint32_t)(int16_t)src.val *
(uint32_t)(int16_t)dst.val);
- if ( (int16_t)dst.val != (uint32_t)dst.val )
+ if ( (int16_t)dst.val != (int32_t)dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
- *(uint16_t *)&_regs.edx = dst.val >> 16;
+ if ( b > 0x6b )
+ *(uint16_t *)&_regs.edx = dst.val >> 16;
break;
#ifdef __x86_64__
case 4:
@@ -3427,14 +3403,16 @@ x86_emulate(
(uint64_t)(int32_t)dst.val);
if ( (int32_t)dst.val != dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
- _regs.edx = (uint32_t)(dst.val >> 32);
+ if ( b > 0x6b )
+ _regs.edx = (uint32_t)(dst.val >> 32);
break;
#endif
default: {
unsigned long m[2] = { src.val, dst.val };
if ( imul_dbl(m) )
_regs.eflags |= EFLG_OF|EFLG_CF;
- _regs.edx = m[1];
+ if ( b > 0x6b )
+ _regs.edx = m[1];
dst.val = m[0];
break;
}
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 6/6] x86_emulate: fold wide reads
2013-09-19 15:40 [PATCH 0/6] x86: emulation improvements Jan Beulich
` (4 preceding siblings ...)
2013-09-19 15:45 ` [PATCH 5/6] x86_emulate: fix flag setting for 8-bit signed multiplication Jan Beulich
@ 2013-09-19 15:46 ` Jan Beulich
2013-09-19 16:12 ` [PATCH 0/6] x86: emulation improvements Keir Fraser
6 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2013-09-19 15:46 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 2312 bytes --]
With HVM's MMIO operand handling now being capable of splitting large
reads, there's no need to issue at most machine word size reads when
we really need wider operands.
Not that this is not done everywhere - there are a couple of cases
where keeping the reads separate is more natural (and folding them
would complicate the code rather than simplifying it).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1116,7 +1116,6 @@ protmode_load_seg(
{
struct segment_register desctab, ss, segr;
struct { uint32_t a, b; } desc;
- unsigned long val;
uint8_t dpl, rpl, cpl;
uint32_t new_desc_b, a_flag = 0x100;
int rc, fault_type = EXC_GP;
@@ -1143,14 +1142,9 @@ protmode_load_seg(
if ( ((sel & 0xfff8) + 7) > desctab.limit )
goto raise_exn;
- if ( (rc = read_ulong(x86_seg_none, desctab.base + (sel & 0xfff8),
- &val, 4, ctxt, ops)) )
+ if ( (rc = ops->read(x86_seg_none, desctab.base + (sel & 0xfff8),
+ &desc, sizeof(desc), ctxt)) )
return rc;
- desc.a = val;
- if ( (rc = read_ulong(x86_seg_none, desctab.base + (sel & 0xfff8) + 4,
- &val, 4, ctxt, ops)) )
- return rc;
- desc.b = val;
/* Segment present in memory? */
if ( !(desc.b & (1u<<15)) )
@@ -4454,7 +4448,6 @@ x86_emulate(
case 0xc7: /* Grp9 (cmpxchg8b/cmpxchg16b) */ {
unsigned long old[2], exp[2], new[2];
- unsigned int i;
generate_exception_if((modrm_reg & 7) != 1, EXC_UD, -1);
generate_exception_if(ea.type != OP_MEM, EXC_UD, -1);
@@ -4463,10 +4456,9 @@ x86_emulate(
op_bytes *= 2;
/* Get actual old value. */
- for ( i = 0; i < (op_bytes/sizeof(long)); i++ )
- if ( (rc = read_ulong(ea.mem.seg, ea.mem.off + i*sizeof(long),
- &old[i], sizeof(long), ctxt, ops)) != 0 )
- goto done;
+ if ( (rc = ops->read(ea.mem.seg, ea.mem.off, old, op_bytes,
+ ctxt)) != 0 )
+ goto done;
/* Get expected and proposed values. */
if ( op_bytes == 8 )
[-- Attachment #2: x86-emul-fold-reads.patch --]
[-- Type: text/plain, Size: 2338 bytes --]
x86_emulate: fold wide reads
With HVM's MMIO operand handling now being capable of splitting large
reads, there's no need to issue at most machine word size reads when
we really need wider operands.
Not that this is not done everywhere - there are a couple of cases
where keeping the reads separate is more natural (and folding them
would complicate the code rather than simplifying it).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1116,7 +1116,6 @@ protmode_load_seg(
{
struct segment_register desctab, ss, segr;
struct { uint32_t a, b; } desc;
- unsigned long val;
uint8_t dpl, rpl, cpl;
uint32_t new_desc_b, a_flag = 0x100;
int rc, fault_type = EXC_GP;
@@ -1143,14 +1142,9 @@ protmode_load_seg(
if ( ((sel & 0xfff8) + 7) > desctab.limit )
goto raise_exn;
- if ( (rc = read_ulong(x86_seg_none, desctab.base + (sel & 0xfff8),
- &val, 4, ctxt, ops)) )
+ if ( (rc = ops->read(x86_seg_none, desctab.base + (sel & 0xfff8),
+ &desc, sizeof(desc), ctxt)) )
return rc;
- desc.a = val;
- if ( (rc = read_ulong(x86_seg_none, desctab.base + (sel & 0xfff8) + 4,
- &val, 4, ctxt, ops)) )
- return rc;
- desc.b = val;
/* Segment present in memory? */
if ( !(desc.b & (1u<<15)) )
@@ -4454,7 +4448,6 @@ x86_emulate(
case 0xc7: /* Grp9 (cmpxchg8b/cmpxchg16b) */ {
unsigned long old[2], exp[2], new[2];
- unsigned int i;
generate_exception_if((modrm_reg & 7) != 1, EXC_UD, -1);
generate_exception_if(ea.type != OP_MEM, EXC_UD, -1);
@@ -4463,10 +4456,9 @@ x86_emulate(
op_bytes *= 2;
/* Get actual old value. */
- for ( i = 0; i < (op_bytes/sizeof(long)); i++ )
- if ( (rc = read_ulong(ea.mem.seg, ea.mem.off + i*sizeof(long),
- &old[i], sizeof(long), ctxt, ops)) != 0 )
- goto done;
+ if ( (rc = ops->read(ea.mem.seg, ea.mem.off, old, op_bytes,
+ ctxt)) != 0 )
+ goto done;
/* Get expected and proposed values. */
if ( op_bytes == 8 )
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/6] x86: emulation improvements
2013-09-19 15:40 [PATCH 0/6] x86: emulation improvements Jan Beulich
` (5 preceding siblings ...)
2013-09-19 15:46 ` [PATCH 6/6] x86_emulate: fold wide reads Jan Beulich
@ 2013-09-19 16:12 ` Keir Fraser
2013-09-20 8:21 ` Jan Beulich
6 siblings, 1 reply; 10+ messages in thread
From: Keir Fraser @ 2013-09-19 16:12 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 19/09/2013 16:40, "Jan Beulich" <JBeulich@suse.com> wrote:
> 1: x86/HVM: properly handle MMIO reads and writes wider than a machine word
> 2: x86: fix dependencies of emulator test
> 3: x86_emulate: MOVSXD must read source operand just once
> 4: x86_emulate: PUSH <mem> must read source operand just once
> 5: x86_emulate: fix flag setting for 8-bit signed multiplication
> 6: x86_emulate: fold wide reads
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Nice.
Acked-by: Keir Fraser <keir@xen.org>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/6] x86: emulation improvements
2013-09-19 16:12 ` [PATCH 0/6] x86: emulation improvements Keir Fraser
@ 2013-09-20 8:21 ` Jan Beulich
2013-09-20 9:10 ` Keir Fraser
0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2013-09-20 8:21 UTC (permalink / raw)
To: Keir Fraser; +Cc: xen-devel
>>> On 19.09.13 at 18:12, Keir Fraser <keir.xen@gmail.com> wrote:
> On 19/09/2013 16:40, "Jan Beulich" <JBeulich@suse.com> wrote:
>
>> 1: x86/HVM: properly handle MMIO reads and writes wider than a machine word
>> 2: x86: fix dependencies of emulator test
>> 3: x86_emulate: MOVSXD must read source operand just once
>> 4: x86_emulate: PUSH <mem> must read source operand just once
>> 5: x86_emulate: fix flag setting for 8-bit signed multiplication
>> 6: x86_emulate: fold wide reads
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Nice.
>
> Acked-by: Keir Fraser <keir@xen.org>
I'm going to commit them with two tiny adjustments:
In the first patch I simplified the initializer of "chunk" to
chunk = min(bytes, 1U << LONG_BYTEORDER)
And in the fourth patch I replaced the newly added conditional in
the 8-bit case with an ASSERT() (which in turn requires the test
utility to be adjusted to have ASSERT() available).
I'm sure you're fine with both.
There's going to be one more, fixing wrap around handling for
repeated string instructions; need to still give this some minimal
testing before submitting.
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/6] x86: emulation improvements
2013-09-20 8:21 ` Jan Beulich
@ 2013-09-20 9:10 ` Keir Fraser
0 siblings, 0 replies; 10+ messages in thread
From: Keir Fraser @ 2013-09-20 9:10 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
On 20/09/2013 09:21, "Jan Beulich" <JBeulich@suse.com> wrote:
>>>> On 19.09.13 at 18:12, Keir Fraser <keir.xen@gmail.com> wrote:
>> On 19/09/2013 16:40, "Jan Beulich" <JBeulich@suse.com> wrote:
>>
>>> 1: x86/HVM: properly handle MMIO reads and writes wider than a machine word
>>> 2: x86: fix dependencies of emulator test
>>> 3: x86_emulate: MOVSXD must read source operand just once
>>> 4: x86_emulate: PUSH <mem> must read source operand just once
>>> 5: x86_emulate: fix flag setting for 8-bit signed multiplication
>>> 6: x86_emulate: fold wide reads
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> Nice.
>>
>> Acked-by: Keir Fraser <keir@xen.org>
>
> I'm going to commit them with two tiny adjustments:
>
> In the first patch I simplified the initializer of "chunk" to
>
> chunk = min(bytes, 1U << LONG_BYTEORDER)
>
> And in the fourth patch I replaced the newly added conditional in
> the 8-bit case with an ASSERT() (which in turn requires the test
> utility to be adjusted to have ASSERT() available).
>
> I'm sure you're fine with both.
Yes, fine!
-- Keir
> There's going to be one more, fixing wrap around handling for
> repeated string instructions; need to still give this some minimal
> testing before submitting.
>
> Jan
>
^ permalink raw reply [flat|nested] 10+ messages in thread