* [RFC][PATCH v2 0/1] Add support for Xen access to vmport
@ 2014-10-02 18:36 Don Slutz
2014-10-02 18:36 ` [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT Don Slutz
0 siblings, 1 reply; 25+ messages in thread
From: Don Slutz @ 2014-10-02 18:36 UTC (permalink / raw)
To: xen-devel; +Cc: Don Slutz, Keir Fraser, Ian Campbell, Jan Beulich
Changes v1 to v2:
Fixup usage of hvmtrace_io_assist().
VMware only changes the 32bit part of the register.
Stefano Stabellini -- QEMU part
the registers being passes explicitely by Xen rather than
"hiding" them into other ioreq fields.
Added vmware_ioreq_t
Note: to use this with Xen either a version of:
[Qemu-devel] [PATCH] -machine vmport=off: Allow disabling of VMWare ioport emulation
And a manual adjustment so that QEMU is started with vmport=1
or
>From f70663d9fb86914144ba340b6186cb1e67ac6eec Mon Sep 17 00:00:00 2001
From: Don Slutz <dslutz@verizon.com>
Date: Fri, 26 Sep 2014 08:11:39 -0400
Subject: [PATCH 1/2] hack: force enable vmport
Signed-off-by: Don Slutz <dslutz@verizon.com>
---
hw/i386/pc_piix.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 103d756..b76dfbc 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -234,7 +234,7 @@ static void pc_init1(MachineState *machine,
pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
/* init basic PC hardware */
- pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, xen_enabled(),
+ pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, false,
0x4);
pc_nic_init(isa_bus, pci_bus);
--
1.8.4
needs to be done to QEMU.
Don Slutz (1):
Add IOREQ_TYPE_VMWARE_PORT
xen/arch/x86/hvm/emulate.c | 72 +++++++++++++++++++++++++++++++++++++++
xen/arch/x86/hvm/io.c | 19 +++++++++++
xen/arch/x86/hvm/vmware/vmport.c | 24 ++++++++++++-
xen/include/asm-x86/hvm/emulate.h | 2 ++
xen/include/asm-x86/hvm/vcpu.h | 1 +
xen/include/public/hvm/ioreq.h | 19 +++++++++++
6 files changed, 136 insertions(+), 1 deletion(-)
--
1.8.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-02 18:36 [RFC][PATCH v2 0/1] Add support for Xen access to vmport Don Slutz
@ 2014-10-02 18:36 ` Don Slutz
2014-10-02 20:33 ` Andrew Cooper
` (2 more replies)
0 siblings, 3 replies; 25+ messages in thread
From: Don Slutz @ 2014-10-02 18:36 UTC (permalink / raw)
To: xen-devel; +Cc: Don Slutz, Keir Fraser, Ian Campbell, Jan Beulich
Signed-off-by: Don Slutz <dslutz@verizon.com>
---
v2:
Fixup usage of hvmtrace_io_assist().
VMware only changes the 32bit part of the register.
Added vmware_ioreq_t
xen/arch/x86/hvm/emulate.c | 72 +++++++++++++++++++++++++++++++++++++++
xen/arch/x86/hvm/io.c | 19 +++++++++++
xen/arch/x86/hvm/vmware/vmport.c | 24 ++++++++++++-
xen/include/asm-x86/hvm/emulate.h | 2 ++
xen/include/asm-x86/hvm/vcpu.h | 1 +
xen/include/public/hvm/ioreq.h | 19 +++++++++++
6 files changed, 136 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index c0f47d2..215f049 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -50,6 +50,78 @@ static void hvmtrace_io_assist(int is_mmio, ioreq_t *p)
trace_var(event, 0/*!cycles*/, size, buffer);
}
+int hvmemul_do_vmport(uint16_t addr, int size, int dir,
+ struct cpu_user_regs *regs)
+{
+ struct vcpu *curr = current;
+ struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
+ vmware_ioreq_t p = {
+ .type = IOREQ_TYPE_VMWARE_PORT,
+ .addr = addr,
+ .size = size,
+ .dir = dir,
+ .eax = regs->rax,
+ .ebx = regs->rbx,
+ .ecx = regs->rcx,
+ .edx = regs->rdx,
+ .esi = regs->rsi,
+ .edi = regs->rdi,
+ };
+ ioreq_t *pp = (ioreq_t *)&p;
+ ioreq_t op;
+
+ ASSERT(sizeof(p) == sizeof(op));
+ ASSERT(offsetof(ioreq_t, type) == offsetof(vmware_ioreq_t, type));
+ ASSERT(offsetof(ioreq_t, vp_eport) == offsetof(vmware_ioreq_t, vp_eport));
+
+ switch ( vio->io_state )
+ {
+ case HVMIO_none:
+ break;
+ case HVMIO_completed:
+ vio->io_state = HVMIO_none;
+ goto finish_access_vmport;
+ case HVMIO_dispatched:
+ /* May have to wait for previous cycle of a multi-write to complete. */
+ default:
+ return X86EMUL_UNHANDLEABLE;
+ }
+
+ if ( hvm_io_pending(curr) )
+ {
+ gdprintk(XENLOG_WARNING, "WARNING: io already pending?\n");
+ return X86EMUL_UNHANDLEABLE;
+ }
+
+ vio->io_state = HVMIO_handle_vmport_awaiting_completion;
+ vio->io_size = size;
+
+ /* If there is no backing DM, just ignore accesses */
+ if ( !hvm_has_dm(curr->domain) )
+ {
+ vio->io_state = HVMIO_none;
+ vio->io_data = ~0ul;
+ }
+ else
+ {
+ if ( !hvm_send_assist_req(pp) )
+ vio->io_state = HVMIO_none;
+ return X86EMUL_RETRY;
+ }
+
+ finish_access_vmport:
+ memset(&op, 0, sizeof(op));
+ op.dir = dir;
+ op.addr = (uint16_t)regs->rdx;
+ op.data_is_ptr = 0;
+ op.data = vio->io_data;
+ hvmtrace_io_assist(0, &op);
+
+ memcpy(®s->rax, &vio->io_data, vio->io_size);
+
+ return X86EMUL_OKAY;
+}
+
static int hvmemul_do_io(
int is_mmio, paddr_t addr, unsigned long *reps, int size,
paddr_t ram_gpa, int dir, int df, void *p_data)
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index 68fb890..a8bf324 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -197,6 +197,25 @@ void hvm_io_assist(ioreq_t *p)
else
memcpy(&guest_cpu_user_regs()->rax, &p->data, vio->io_size);
break;
+ case HVMIO_handle_vmport_awaiting_completion:
+ {
+ struct cpu_user_regs *regs = guest_cpu_user_regs();
+ vmware_ioreq_t *vp = (vmware_ioreq_t *)p;
+
+ ASSERT(sizeof(*p) == sizeof(*vp));
+ ASSERT(offsetof(ioreq_t, type) == offsetof(vmware_ioreq_t, type));
+ ASSERT(offsetof(ioreq_t, vp_eport) == offsetof(vmware_ioreq_t, vp_eport));
+
+ /* Always zero extension for eax */
+ regs->rax = vp->eax;
+ /* Only change the 32bit part of the register */
+ regs->rbx = (regs->rbx & 0xffffffff00000000ull) | vp->ebx;
+ regs->rcx = (regs->rcx & 0xffffffff00000000ull) | vp->ecx;
+ regs->rdx = (regs->rdx & 0xffffffff00000000ull) | vp->edx;
+ regs->rsi = (regs->rsi & 0xffffffff00000000ull) | vp->esi;
+ regs->rdi = (regs->rdi & 0xffffffff00000000ull) | vp->edi;
+ }
+ break;
default:
break;
}
diff --git a/xen/arch/x86/hvm/vmware/vmport.c b/xen/arch/x86/hvm/vmware/vmport.c
index 962ee32..0649106 100644
--- a/xen/arch/x86/hvm/vmware/vmport.c
+++ b/xen/arch/x86/hvm/vmware/vmport.c
@@ -147,9 +147,31 @@ int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val)
regs->rax = 0x0;
break;
default:
+ { /* Let backing DM handle */
+ int rc;
+
HVMTRACE_ND(VMPORT_UNKNOWN, 0, 1/*cycles*/, 6,
- (bytes << 8) + dir, cmd, regs->rbx,
+ (bytes << 8) | dir, cmd, regs->rbx,
regs->rcx, regs->rsi, regs->rdi);
+ rc = hvmemul_do_vmport(BDOOR_PORT, bytes, dir, regs);
+ switch (rc)
+ {
+ case X86EMUL_OKAY:
+ break;
+ case X86EMUL_RETRY:
+ {
+ struct hvm_vcpu_io *vio = ¤t->arch.hvm_vcpu.hvm_io;
+
+ if ( vio->io_state != HVMIO_handle_vmport_awaiting_completion )
+ return 0;
+ break;
+ }
+ default:
+ gdprintk(XENLOG_ERR, "Weird HVM ioemulation status %d.\n", rc);
+ domain_crash(current->domain);
+ break;
+ }
+ }
break;
}
diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm-x86/hvm/emulate.h
index 5411302..7d18435 100644
--- a/xen/include/asm-x86/hvm/emulate.h
+++ b/xen/include/asm-x86/hvm/emulate.h
@@ -53,6 +53,8 @@ struct segment_register *hvmemul_get_seg_reg(
int hvmemul_do_pio(
unsigned long port, unsigned long *reps, int size,
paddr_t ram_gpa, int dir, int df, void *p_data);
+int hvmemul_do_vmport(uint16_t addr, int size, int dir,
+ struct cpu_user_regs *regs);
void hvm_dump_emulation_state(const char *prefix,
struct hvm_emulate_ctxt *hvmemul_ctxt);
diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
index 01e0665..1e63d7f 100644
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -36,6 +36,7 @@ enum hvm_io_state {
HVMIO_awaiting_completion,
HVMIO_handle_mmio_awaiting_completion,
HVMIO_handle_pio_awaiting_completion,
+ HVMIO_handle_vmport_awaiting_completion,
HVMIO_completed
};
diff --git a/xen/include/public/hvm/ioreq.h b/xen/include/public/hvm/ioreq.h
index 5b5fedf..7d5ba58 100644
--- a/xen/include/public/hvm/ioreq.h
+++ b/xen/include/public/hvm/ioreq.h
@@ -35,6 +35,7 @@
#define IOREQ_TYPE_PIO 0 /* pio */
#define IOREQ_TYPE_COPY 1 /* mmio ops */
#define IOREQ_TYPE_PCI_CONFIG 2
+#define IOREQ_TYPE_VMWARE_PORT 3
#define IOREQ_TYPE_TIMEOFFSET 7
#define IOREQ_TYPE_INVALIDATE 8 /* mapcache */
@@ -48,6 +49,8 @@
*
* 63....48|47..40|39..35|34..32|31........0
* SEGMENT |BUS |DEV |FN |OFFSET
+ *
+ * For I/O type IOREQ_TYPE_VMWARE_PORT use the vmware_ioreq.
*/
struct ioreq {
uint64_t addr; /* physical address */
@@ -66,6 +69,22 @@ struct ioreq {
};
typedef struct ioreq ioreq_t;
+struct vmware_ioreq {
+ uint32_t esi;
+ uint32_t edi;
+ uint32_t eax;
+ uint32_t ebx;
+ uint32_t ecx;
+ uint32_t edx;
+ uint32_t vp_eport; /* evtchn for notifications to/from device model */
+ uint16_t addr;
+ uint8_t state:4;
+ uint8_t dir:1; /* 1=read, 0=write */
+ uint8_t size:3;
+ uint8_t type; /* I/O type */
+};
+typedef struct vmware_ioreq vmware_ioreq_t;
+
struct shared_iopage {
struct ioreq vcpu_ioreq[1];
};
--
1.8.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-02 18:36 ` [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT Don Slutz
@ 2014-10-02 20:33 ` Andrew Cooper
2014-10-02 21:56 ` Don Slutz
2014-10-03 9:29 ` Paul Durrant
2014-10-03 9:46 ` Paul Durrant
2 siblings, 1 reply; 25+ messages in thread
From: Andrew Cooper @ 2014-10-02 20:33 UTC (permalink / raw)
To: Don Slutz, xen-devel; +Cc: Keir Fraser, Ian Campbell, Jan Beulich
On 02/10/14 19:36, Don Slutz wrote:
> Signed-off-by: Don Slutz <dslutz@verizon.com>
> ---
> v2:
> Fixup usage of hvmtrace_io_assist().
> VMware only changes the 32bit part of the register.
> Added vmware_ioreq_t
>
> xen/arch/x86/hvm/emulate.c | 72 +++++++++++++++++++++++++++++++++++++++
> xen/arch/x86/hvm/io.c | 19 +++++++++++
> xen/arch/x86/hvm/vmware/vmport.c | 24 ++++++++++++-
> xen/include/asm-x86/hvm/emulate.h | 2 ++
> xen/include/asm-x86/hvm/vcpu.h | 1 +
> xen/include/public/hvm/ioreq.h | 19 +++++++++++
> 6 files changed, 136 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index c0f47d2..215f049 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -50,6 +50,78 @@ static void hvmtrace_io_assist(int is_mmio, ioreq_t *p)
> trace_var(event, 0/*!cycles*/, size, buffer);
> }
>
> +int hvmemul_do_vmport(uint16_t addr, int size, int dir,
> + struct cpu_user_regs *regs)
> +{
> + struct vcpu *curr = current;
> + struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
> + vmware_ioreq_t p = {
> + .type = IOREQ_TYPE_VMWARE_PORT,
> + .addr = addr,
> + .size = size,
> + .dir = dir,
> + .eax = regs->rax,
> + .ebx = regs->rbx,
> + .ecx = regs->rcx,
> + .edx = regs->rdx,
> + .esi = regs->rsi,
> + .edi = regs->rdi,
> + };
> + ioreq_t *pp = (ioreq_t *)&p;
> + ioreq_t op;
Eww.
Just because the C type system lets you abuse it like this doesn't mean
it is a clever idea to. Please refer to c/s 15a9f34d1b as an example of
the kinds of bugs it causes.
> +
> + ASSERT(sizeof(p) == sizeof(op));
> + ASSERT(offsetof(ioreq_t, type) == offsetof(vmware_ioreq_t, type));
> + ASSERT(offsetof(ioreq_t, vp_eport) == offsetof(vmware_ioreq_t, vp_eport));
These can be evaluated by the compiler. They should be BUILD_BUG_ON()s
rather than ASSERT()s.
> +
> + switch ( vio->io_state )
> + {
> + case HVMIO_none:
> + break;
> + case HVMIO_completed:
> + vio->io_state = HVMIO_none;
> + goto finish_access_vmport;
> + case HVMIO_dispatched:
> + /* May have to wait for previous cycle of a multi-write to complete. */
> + default:
> + return X86EMUL_UNHANDLEABLE;
> + }
> +
> + if ( hvm_io_pending(curr) )
> + {
> + gdprintk(XENLOG_WARNING, "WARNING: io already pending?\n");
> + return X86EMUL_UNHANDLEABLE;
> + }
> +
> + vio->io_state = HVMIO_handle_vmport_awaiting_completion;
> + vio->io_size = size;
> +
> + /* If there is no backing DM, just ignore accesses */
> + if ( !hvm_has_dm(curr->domain) )
> + {
> + vio->io_state = HVMIO_none;
> + vio->io_data = ~0ul;
> + }
> + else
> + {
> + if ( !hvm_send_assist_req(pp) )
> + vio->io_state = HVMIO_none;
> + return X86EMUL_RETRY;
> + }
> +
> + finish_access_vmport:
> + memset(&op, 0, sizeof(op));
> + op.dir = dir;
> + op.addr = (uint16_t)regs->rdx;
> + op.data_is_ptr = 0;
> + op.data = vio->io_data;
Most of this can be achieved with struct initialisation similar to 'p'
above.
> + hvmtrace_io_assist(0, &op);
> +
> + memcpy(®s->rax, &vio->io_data, vio->io_size);
This is going to need some justification. Clobbering the registers like
this looks bogus.
> +
> + return X86EMUL_OKAY;
> +}
> +
> static int hvmemul_do_io(
> int is_mmio, paddr_t addr, unsigned long *reps, int size,
> paddr_t ram_gpa, int dir, int df, void *p_data)
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index 68fb890..a8bf324 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -197,6 +197,25 @@ void hvm_io_assist(ioreq_t *p)
> else
> memcpy(&guest_cpu_user_regs()->rax, &p->data, vio->io_size);
> break;
> + case HVMIO_handle_vmport_awaiting_completion:
> + {
> + struct cpu_user_regs *regs = guest_cpu_user_regs();
> + vmware_ioreq_t *vp = (vmware_ioreq_t *)p;
> +
> + ASSERT(sizeof(*p) == sizeof(*vp));
> + ASSERT(offsetof(ioreq_t, type) == offsetof(vmware_ioreq_t, type));
> + ASSERT(offsetof(ioreq_t, vp_eport) == offsetof(vmware_ioreq_t, vp_eport));
> +
> + /* Always zero extension for eax */
> + regs->rax = vp->eax;
> + /* Only change the 32bit part of the register */
> + regs->rbx = (regs->rbx & 0xffffffff00000000ull) | vp->ebx;
regs->_e?? allows you to directly access the lower half, avoiding these
bit manipulations.
~Andrew
> + regs->rcx = (regs->rcx & 0xffffffff00000000ull) | vp->ecx;
> + regs->rdx = (regs->rdx & 0xffffffff00000000ull) | vp->edx;
> + regs->rsi = (regs->rsi & 0xffffffff00000000ull) | vp->esi;
> + regs->rdi = (regs->rdi & 0xffffffff00000000ull) | vp->edi;
> + }
> + break;
> default:
> break;
> }
> diff --git a/xen/arch/x86/hvm/vmware/vmport.c b/xen/arch/x86/hvm/vmware/vmport.c
> index 962ee32..0649106 100644
> --- a/xen/arch/x86/hvm/vmware/vmport.c
> +++ b/xen/arch/x86/hvm/vmware/vmport.c
> @@ -147,9 +147,31 @@ int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val)
> regs->rax = 0x0;
> break;
> default:
> + { /* Let backing DM handle */
> + int rc;
> +
> HVMTRACE_ND(VMPORT_UNKNOWN, 0, 1/*cycles*/, 6,
> - (bytes << 8) + dir, cmd, regs->rbx,
> + (bytes << 8) | dir, cmd, regs->rbx,
> regs->rcx, regs->rsi, regs->rdi);
> + rc = hvmemul_do_vmport(BDOOR_PORT, bytes, dir, regs);
> + switch (rc)
> + {
> + case X86EMUL_OKAY:
> + break;
> + case X86EMUL_RETRY:
> + {
> + struct hvm_vcpu_io *vio = ¤t->arch.hvm_vcpu.hvm_io;
> +
> + if ( vio->io_state != HVMIO_handle_vmport_awaiting_completion )
> + return 0;
> + break;
> + }
> + default:
> + gdprintk(XENLOG_ERR, "Weird HVM ioemulation status %d.\n", rc);
> + domain_crash(current->domain);
> + break;
> + }
> + }
> break;
> }
>
> diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm-x86/hvm/emulate.h
> index 5411302..7d18435 100644
> --- a/xen/include/asm-x86/hvm/emulate.h
> +++ b/xen/include/asm-x86/hvm/emulate.h
> @@ -53,6 +53,8 @@ struct segment_register *hvmemul_get_seg_reg(
> int hvmemul_do_pio(
> unsigned long port, unsigned long *reps, int size,
> paddr_t ram_gpa, int dir, int df, void *p_data);
> +int hvmemul_do_vmport(uint16_t addr, int size, int dir,
> + struct cpu_user_regs *regs);
>
> void hvm_dump_emulation_state(const char *prefix,
> struct hvm_emulate_ctxt *hvmemul_ctxt);
> diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
> index 01e0665..1e63d7f 100644
> --- a/xen/include/asm-x86/hvm/vcpu.h
> +++ b/xen/include/asm-x86/hvm/vcpu.h
> @@ -36,6 +36,7 @@ enum hvm_io_state {
> HVMIO_awaiting_completion,
> HVMIO_handle_mmio_awaiting_completion,
> HVMIO_handle_pio_awaiting_completion,
> + HVMIO_handle_vmport_awaiting_completion,
> HVMIO_completed
> };
>
> diff --git a/xen/include/public/hvm/ioreq.h b/xen/include/public/hvm/ioreq.h
> index 5b5fedf..7d5ba58 100644
> --- a/xen/include/public/hvm/ioreq.h
> +++ b/xen/include/public/hvm/ioreq.h
> @@ -35,6 +35,7 @@
> #define IOREQ_TYPE_PIO 0 /* pio */
> #define IOREQ_TYPE_COPY 1 /* mmio ops */
> #define IOREQ_TYPE_PCI_CONFIG 2
> +#define IOREQ_TYPE_VMWARE_PORT 3
> #define IOREQ_TYPE_TIMEOFFSET 7
> #define IOREQ_TYPE_INVALIDATE 8 /* mapcache */
>
> @@ -48,6 +49,8 @@
> *
> * 63....48|47..40|39..35|34..32|31........0
> * SEGMENT |BUS |DEV |FN |OFFSET
> + *
> + * For I/O type IOREQ_TYPE_VMWARE_PORT use the vmware_ioreq.
> */
> struct ioreq {
> uint64_t addr; /* physical address */
> @@ -66,6 +69,22 @@ struct ioreq {
> };
> typedef struct ioreq ioreq_t;
>
> +struct vmware_ioreq {
> + uint32_t esi;
> + uint32_t edi;
> + uint32_t eax;
> + uint32_t ebx;
> + uint32_t ecx;
> + uint32_t edx;
> + uint32_t vp_eport; /* evtchn for notifications to/from device model */
> + uint16_t addr;
> + uint8_t state:4;
> + uint8_t dir:1; /* 1=read, 0=write */
> + uint8_t size:3;
> + uint8_t type; /* I/O type */
> +};
> +typedef struct vmware_ioreq vmware_ioreq_t;
> +
> struct shared_iopage {
> struct ioreq vcpu_ioreq[1];
> };
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-02 20:33 ` Andrew Cooper
@ 2014-10-02 21:56 ` Don Slutz
2014-10-02 22:23 ` Andrew Cooper
0 siblings, 1 reply; 25+ messages in thread
From: Don Slutz @ 2014-10-02 21:56 UTC (permalink / raw)
To: Andrew Cooper, Don Slutz, xen-devel
Cc: Keir Fraser, Ian Campbell, Jan Beulich
On 10/02/14 16:33, Andrew Cooper wrote:
> On 02/10/14 19:36, Don Slutz wrote:
>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>> ---
>> v2:
>> Fixup usage of hvmtrace_io_assist().
>> VMware only changes the 32bit part of the register.
>> Added vmware_ioreq_t
>>
>> xen/arch/x86/hvm/emulate.c | 72 +++++++++++++++++++++++++++++++++++++++
>> xen/arch/x86/hvm/io.c | 19 +++++++++++
>> xen/arch/x86/hvm/vmware/vmport.c | 24 ++++++++++++-
>> xen/include/asm-x86/hvm/emulate.h | 2 ++
>> xen/include/asm-x86/hvm/vcpu.h | 1 +
>> xen/include/public/hvm/ioreq.h | 19 +++++++++++
>> 6 files changed, 136 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
>> index c0f47d2..215f049 100644
>> --- a/xen/arch/x86/hvm/emulate.c
>> +++ b/xen/arch/x86/hvm/emulate.c
>> @@ -50,6 +50,78 @@ static void hvmtrace_io_assist(int is_mmio, ioreq_t *p)
>> trace_var(event, 0/*!cycles*/, size, buffer);
>> }
>>
>> +int hvmemul_do_vmport(uint16_t addr, int size, int dir,
>> + struct cpu_user_regs *regs)
>> +{
>> + struct vcpu *curr = current;
>> + struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
>> + vmware_ioreq_t p = {
>> + .type = IOREQ_TYPE_VMWARE_PORT,
>> + .addr = addr,
>> + .size = size,
>> + .dir = dir,
>> + .eax = regs->rax,
>> + .ebx = regs->rbx,
>> + .ecx = regs->rcx,
>> + .edx = regs->rdx,
>> + .esi = regs->rsi,
>> + .edi = regs->rdi,
>> + };
>> + ioreq_t *pp = (ioreq_t *)&p;
>> + ioreq_t op;
> Eww.
>
> Just because the C type system lets you abuse it like this doesn't mean
> it is a clever idea to. Please refer to c/s 15a9f34d1b as an example of
> the kinds of bugs it causes.
This is a direct result of:
Subject: Re: [PATCH 1/1] xen-hvm.c: Add support for Xen access to vmport
Date: Tue, 30 Sep 2014 11:35:44 +0100
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
To: Don Slutz <dslutz@verizon.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
qemu-devel@nongnu.org, xen-devel@lists.xensource.com, Alexander Graf
<agraf@suse.de>, Andreas Färber <afaerber@suse.de>, Anthony Liguori
<aliguori@amazon.com>, Marcel Apfelbaum <marcel.a@redhat.com>, Markus
Armbruster <armbru@redhat.com>, Michael S. Tsirkin <mst@redhat.com>
On Mon, 29 Sep 2014, Don Slutz wrote:
> On 09/29/14 06:25, Stefano Stabellini wrote:
> > On Mon, 29 Sep 2014, Stefano Stabellini wrote:
> > > On Fri, 26 Sep 2014, Don Slutz wrote:
> > > > This adds synchronisation of the vcpu registers
> > > > between Xen and QEMU.
...
> > > > + CPUX86State *env;
> > > > + ioreq_t fake_req = {
> > > > + .type = IOREQ_TYPE_PIO,
> > > > + .addr = (uint16_t)req->size,
> > > > + .size = 4,
> > > > + .dir = IOREQ_READ,
> > > > + .df = 0,
> > > > + .data_is_ptr = 0,
> > > > + };
> > Why do you need a fake req?
>
> To transport the 6 VCPU registers (only 32bits of them) that vmport.c
> needs to do it's work.
>
> > Couldn't Xen send the real req instead?
>
> Yes, but then a 2nd exchange between QEMU and Xen would be needed
> to fetch the 6 VCPU registers. The ways I know of to fetch the VCPU registers
> from Xen, all need many cycles to do their work and return
> a lot of data that is not needed.
>
> The other option that I have considered was to extend the ioreq_t type
> to have room for these registers, but that reduces by almost half the
> maximum number of VCPUs that are supported (They all live on 1 page).
Urgh. Now that I understand the patch better is think it's horrible, no
offense :-)
Why don't you add another new ioreq type to send out the vcpu state?
Something like IOREQ_TYPE_VCPU_SYNC_REGS? You could send it to QEMU
before IOREQ_TYPE_VMWARE_PORT. Actually this solution looks very imilar
to Alex's suggestion.
...
And the ASSERTs below are the attempt to prevent bugs from being added.
Sigh. Too much with both XEN and QEMU in hard freeze. I may
have a way to avoid the cast.
>> +
>> + ASSERT(sizeof(p) == sizeof(op));
>> + ASSERT(offsetof(ioreq_t, type) == offsetof(vmware_ioreq_t, type));
>> + ASSERT(offsetof(ioreq_t, vp_eport) == offsetof(vmware_ioreq_t, vp_eport));
> These can be evaluated by the compiler. They should be BUILD_BUG_ON()s
> rather than ASSERT()s.
I looked for this, but did not find it. Will change.
>> +
>> + switch ( vio->io_state )
>> + {
>> + case HVMIO_none:
>> + break;
>> + case HVMIO_completed:
>> + vio->io_state = HVMIO_none;
>> + goto finish_access_vmport;
>> + case HVMIO_dispatched:
>> + /* May have to wait for previous cycle of a multi-write to complete. */
>> + default:
>> + return X86EMUL_UNHANDLEABLE;
>> + }
>> +
>> + if ( hvm_io_pending(curr) )
>> + {
>> + gdprintk(XENLOG_WARNING, "WARNING: io already pending?\n");
>> + return X86EMUL_UNHANDLEABLE;
>> + }
>> +
>> + vio->io_state = HVMIO_handle_vmport_awaiting_completion;
>> + vio->io_size = size;
>> +
>> + /* If there is no backing DM, just ignore accesses */
>> + if ( !hvm_has_dm(curr->domain) )
>> + {
>> + vio->io_state = HVMIO_none;
>> + vio->io_data = ~0ul;
>> + }
>> + else
>> + {
>> + if ( !hvm_send_assist_req(pp) )
>> + vio->io_state = HVMIO_none;
>> + return X86EMUL_RETRY;
>> + }
>> +
>> + finish_access_vmport:
>> + memset(&op, 0, sizeof(op));
>> + op.dir = dir;
>> + op.addr = (uint16_t)regs->rdx;
>> + op.data_is_ptr = 0;
>> + op.data = vio->io_data;
> Most of this can be achieved with struct initialisation similar to 'p'
> above.
Ok, will make it a block. I did it here (and will continue here) because
this is an error path that should not be taken.
>> + hvmtrace_io_assist(0, &op);
>> +
>> + memcpy(®s->rax, &vio->io_data, vio->io_size);
> This is going to need some justification. Clobbering the registers like
> this looks bogus.
Copied code from hvmemul_do_io(). Not sure how to justify it. I have
some ideas on why it is good.
>
>> +
>> + return X86EMUL_OKAY;
>> +}
>> +
>> static int hvmemul_do_io(
>> int is_mmio, paddr_t addr, unsigned long *reps, int size,
>> paddr_t ram_gpa, int dir, int df, void *p_data)
>> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
>> index 68fb890..a8bf324 100644
>> --- a/xen/arch/x86/hvm/io.c
>> +++ b/xen/arch/x86/hvm/io.c
>> @@ -197,6 +197,25 @@ void hvm_io_assist(ioreq_t *p)
>> else
>> memcpy(&guest_cpu_user_regs()->rax, &p->data, vio->io_size);
>> break;
>> + case HVMIO_handle_vmport_awaiting_completion:
>> + {
>> + struct cpu_user_regs *regs = guest_cpu_user_regs();
>> + vmware_ioreq_t *vp = (vmware_ioreq_t *)p;
>> +
>> + ASSERT(sizeof(*p) == sizeof(*vp));
>> + ASSERT(offsetof(ioreq_t, type) == offsetof(vmware_ioreq_t, type));
>> + ASSERT(offsetof(ioreq_t, vp_eport) == offsetof(vmware_ioreq_t, vp_eport));
>> +
>> + /* Always zero extension for eax */
>> + regs->rax = vp->eax;
>> + /* Only change the 32bit part of the register */
>> + regs->rbx = (regs->rbx & 0xffffffff00000000ull) | vp->ebx;
> regs->_e?? allows you to directly access the lower half, avoiding these
> bit manipulations.
Also did not know of these (besides being bad names...)
Will change to use them.
-Don Slutz
> ~Andrew
>
>> + regs->rcx = (regs->rcx & 0xffffffff00000000ull) | vp->ecx;
>> + regs->rdx = (regs->rdx & 0xffffffff00000000ull) | vp->edx;
>> + regs->rsi = (regs->rsi & 0xffffffff00000000ull) | vp->esi;
>> + regs->rdi = (regs->rdi & 0xffffffff00000000ull) | vp->edi;
>> + }
>> + break;
>> default:
>> break;
>> }
>> diff --git a/xen/arch/x86/hvm/vmware/vmport.c b/xen/arch/x86/hvm/vmware/vmport.c
>> index 962ee32..0649106 100644
>> --- a/xen/arch/x86/hvm/vmware/vmport.c
>> +++ b/xen/arch/x86/hvm/vmware/vmport.c
>> @@ -147,9 +147,31 @@ int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val)
>> regs->rax = 0x0;
>> break;
>> default:
>> + { /* Let backing DM handle */
>> + int rc;
>> +
>> HVMTRACE_ND(VMPORT_UNKNOWN, 0, 1/*cycles*/, 6,
>> - (bytes << 8) + dir, cmd, regs->rbx,
>> + (bytes << 8) | dir, cmd, regs->rbx,
>> regs->rcx, regs->rsi, regs->rdi);
>> + rc = hvmemul_do_vmport(BDOOR_PORT, bytes, dir, regs);
>> + switch (rc)
>> + {
>> + case X86EMUL_OKAY:
>> + break;
>> + case X86EMUL_RETRY:
>> + {
>> + struct hvm_vcpu_io *vio = ¤t->arch.hvm_vcpu.hvm_io;
>> +
>> + if ( vio->io_state != HVMIO_handle_vmport_awaiting_completion )
>> + return 0;
>> + break;
>> + }
>> + default:
>> + gdprintk(XENLOG_ERR, "Weird HVM ioemulation status %d.\n", rc);
>> + domain_crash(current->domain);
>> + break;
>> + }
>> + }
>> break;
>> }
>>
>> diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm-x86/hvm/emulate.h
>> index 5411302..7d18435 100644
>> --- a/xen/include/asm-x86/hvm/emulate.h
>> +++ b/xen/include/asm-x86/hvm/emulate.h
>> @@ -53,6 +53,8 @@ struct segment_register *hvmemul_get_seg_reg(
>> int hvmemul_do_pio(
>> unsigned long port, unsigned long *reps, int size,
>> paddr_t ram_gpa, int dir, int df, void *p_data);
>> +int hvmemul_do_vmport(uint16_t addr, int size, int dir,
>> + struct cpu_user_regs *regs);
>>
>> void hvm_dump_emulation_state(const char *prefix,
>> struct hvm_emulate_ctxt *hvmemul_ctxt);
>> diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
>> index 01e0665..1e63d7f 100644
>> --- a/xen/include/asm-x86/hvm/vcpu.h
>> +++ b/xen/include/asm-x86/hvm/vcpu.h
>> @@ -36,6 +36,7 @@ enum hvm_io_state {
>> HVMIO_awaiting_completion,
>> HVMIO_handle_mmio_awaiting_completion,
>> HVMIO_handle_pio_awaiting_completion,
>> + HVMIO_handle_vmport_awaiting_completion,
>> HVMIO_completed
>> };
>>
>> diff --git a/xen/include/public/hvm/ioreq.h b/xen/include/public/hvm/ioreq.h
>> index 5b5fedf..7d5ba58 100644
>> --- a/xen/include/public/hvm/ioreq.h
>> +++ b/xen/include/public/hvm/ioreq.h
>> @@ -35,6 +35,7 @@
>> #define IOREQ_TYPE_PIO 0 /* pio */
>> #define IOREQ_TYPE_COPY 1 /* mmio ops */
>> #define IOREQ_TYPE_PCI_CONFIG 2
>> +#define IOREQ_TYPE_VMWARE_PORT 3
>> #define IOREQ_TYPE_TIMEOFFSET 7
>> #define IOREQ_TYPE_INVALIDATE 8 /* mapcache */
>>
>> @@ -48,6 +49,8 @@
>> *
>> * 63....48|47..40|39..35|34..32|31........0
>> * SEGMENT |BUS |DEV |FN |OFFSET
>> + *
>> + * For I/O type IOREQ_TYPE_VMWARE_PORT use the vmware_ioreq.
>> */
>> struct ioreq {
>> uint64_t addr; /* physical address */
>> @@ -66,6 +69,22 @@ struct ioreq {
>> };
>> typedef struct ioreq ioreq_t;
>>
>> +struct vmware_ioreq {
>> + uint32_t esi;
>> + uint32_t edi;
>> + uint32_t eax;
>> + uint32_t ebx;
>> + uint32_t ecx;
>> + uint32_t edx;
>> + uint32_t vp_eport; /* evtchn for notifications to/from device model */
>> + uint16_t addr;
>> + uint8_t state:4;
>> + uint8_t dir:1; /* 1=read, 0=write */
>> + uint8_t size:3;
>> + uint8_t type; /* I/O type */
>> +};
>> +typedef struct vmware_ioreq vmware_ioreq_t;
>> +
>> struct shared_iopage {
>> struct ioreq vcpu_ioreq[1];
>> };
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-02 21:56 ` Don Slutz
@ 2014-10-02 22:23 ` Andrew Cooper
2014-10-03 9:47 ` Stefano Stabellini
0 siblings, 1 reply; 25+ messages in thread
From: Andrew Cooper @ 2014-10-02 22:23 UTC (permalink / raw)
To: Don Slutz, xen-devel; +Cc: Keir Fraser, Ian Campbell, Jan Beulich
On 02/10/2014 22:56, Don Slutz wrote:
> On 10/02/14 16:33, Andrew Cooper wrote:
>> On 02/10/14 19:36, Don Slutz wrote:
>>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>>> ---
>>> v2:
>>> Fixup usage of hvmtrace_io_assist().
>>> VMware only changes the 32bit part of the register.
>>> Added vmware_ioreq_t
>>>
>>> xen/arch/x86/hvm/emulate.c | 72
>>> +++++++++++++++++++++++++++++++++++++++
>>> xen/arch/x86/hvm/io.c | 19 +++++++++++
>>> xen/arch/x86/hvm/vmware/vmport.c | 24 ++++++++++++-
>>> xen/include/asm-x86/hvm/emulate.h | 2 ++
>>> xen/include/asm-x86/hvm/vcpu.h | 1 +
>>> xen/include/public/hvm/ioreq.h | 19 +++++++++++
>>> 6 files changed, 136 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
>>> index c0f47d2..215f049 100644
>>> --- a/xen/arch/x86/hvm/emulate.c
>>> +++ b/xen/arch/x86/hvm/emulate.c
>>> @@ -50,6 +50,78 @@ static void hvmtrace_io_assist(int is_mmio,
>>> ioreq_t *p)
>>> trace_var(event, 0/*!cycles*/, size, buffer);
>>> }
>>> +int hvmemul_do_vmport(uint16_t addr, int size, int dir,
>>> + struct cpu_user_regs *regs)
>>> +{
>>> + struct vcpu *curr = current;
>>> + struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
>>> + vmware_ioreq_t p = {
>>> + .type = IOREQ_TYPE_VMWARE_PORT,
>>> + .addr = addr,
>>> + .size = size,
>>> + .dir = dir,
>>> + .eax = regs->rax,
>>> + .ebx = regs->rbx,
>>> + .ecx = regs->rcx,
>>> + .edx = regs->rdx,
>>> + .esi = regs->rsi,
>>> + .edi = regs->rdi,
>>> + };
>>> + ioreq_t *pp = (ioreq_t *)&p;
>>> + ioreq_t op;
>> Eww.
>>
>> Just because the C type system lets you abuse it like this doesn't mean
>> it is a clever idea to. Please refer to c/s 15a9f34d1b as an example of
>> the kinds of bugs it causes.
>
> This is a direct result of:
>
>
> Subject: Re: [PATCH 1/1] xen-hvm.c: Add support for Xen access to
> vmport
> Date: Tue, 30 Sep 2014 11:35:44 +0100
> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> To: Don Slutz <dslutz@verizon.com>
> CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
> qemu-devel@nongnu.org, xen-devel@lists.xensource.com, Alexander Graf
> <agraf@suse.de>, Andreas Färber <afaerber@suse.de>, Anthony Liguori
> <aliguori@amazon.com>, Marcel Apfelbaum <marcel.a@redhat.com>, Markus
> Armbruster <armbru@redhat.com>, Michael S. Tsirkin <mst@redhat.com>
>
>
>
> On Mon, 29 Sep 2014, Don Slutz wrote:
>> On 09/29/14 06:25, Stefano Stabellini wrote:
>> > On Mon, 29 Sep 2014, Stefano Stabellini wrote:
>> > > On Fri, 26 Sep 2014, Don Slutz wrote:
>> > > > This adds synchronisation of the vcpu registers
>> > > > between Xen and QEMU.
>
> ...
>
>> > > > + CPUX86State *env;
>> > > > + ioreq_t fake_req = {
>> > > > + .type = IOREQ_TYPE_PIO,
>> > > > + .addr = (uint16_t)req->size,
>> > > > + .size = 4,
>> > > > + .dir = IOREQ_READ,
>> > > > + .df = 0,
>> > > > + .data_is_ptr = 0,
>> > > > + };
>> > Why do you need a fake req?
>>
>> To transport the 6 VCPU registers (only 32bits of them) that vmport.c
>> needs to do it's work.
>>
>> > Couldn't Xen send the real req instead?
>>
>> Yes, but then a 2nd exchange between QEMU and Xen would be needed
>> to fetch the 6 VCPU registers. The ways I know of to fetch the VCPU
>> registers
>> from Xen, all need many cycles to do their work and return
>> a lot of data that is not needed.
>>
>> The other option that I have considered was to extend the ioreq_t type
>> to have room for these registers, but that reduces by almost half the
>> maximum number of VCPUs that are supported (They all live on 1 page).
>
> Urgh. Now that I understand the patch better is think it's horrible, no
> offense :-)
>
> Why don't you add another new ioreq type to send out the vcpu state?
> Something like IOREQ_TYPE_VCPU_SYNC_REGS? You could send it to QEMU
> before IOREQ_TYPE_VMWARE_PORT. Actually this solution looks very imilar
> to Alex's suggestion.
>
> ...
>
>
> And the ASSERTs below are the attempt to prevent bugs from being added.
>
> Sigh. Too much with both XEN and QEMU in hard freeze. I may
> have a way to avoid the cast.
I put 2 and 2 together to make something close to 4 after sending the
first email, but unions are the C way of doing things like this.
~Andrew
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-02 18:36 ` [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT Don Slutz
2014-10-02 20:33 ` Andrew Cooper
@ 2014-10-03 9:29 ` Paul Durrant
2014-10-03 19:44 ` Don Slutz
2014-10-03 9:46 ` Paul Durrant
2 siblings, 1 reply; 25+ messages in thread
From: Paul Durrant @ 2014-10-03 9:29 UTC (permalink / raw)
To: Don Slutz, xen-devel; +Cc: Keir (Xen.org), Ian Campbell, Jan Beulich
> -----Original Message-----
> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
> bounces@lists.xen.org] On Behalf Of Don Slutz
> Sent: 02 October 2014 19:36
> To: xen-devel@lists.xen.org
> Cc: Don Slutz; Keir (Xen.org); Ian Campbell; Jan Beulich
> Subject: [Xen-devel] [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
>
> Signed-off-by: Don Slutz <dslutz@verizon.com>
> ---
> v2:
> Fixup usage of hvmtrace_io_assist().
> VMware only changes the 32bit part of the register.
> Added vmware_ioreq_t
>
> xen/arch/x86/hvm/emulate.c | 72
> +++++++++++++++++++++++++++++++++++++++
> xen/arch/x86/hvm/io.c | 19 +++++++++++
> xen/arch/x86/hvm/vmware/vmport.c | 24 ++++++++++++-
> xen/include/asm-x86/hvm/emulate.h | 2 ++
> xen/include/asm-x86/hvm/vcpu.h | 1 +
> xen/include/public/hvm/ioreq.h | 19 +++++++++++
> 6 files changed, 136 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index c0f47d2..215f049 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -50,6 +50,78 @@ static void hvmtrace_io_assist(int is_mmio, ioreq_t *p)
> trace_var(event, 0/*!cycles*/, size, buffer);
> }
>
> +int hvmemul_do_vmport(uint16_t addr, int size, int dir,
> + struct cpu_user_regs *regs)
> +{
> + struct vcpu *curr = current;
> + struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
> + vmware_ioreq_t p = {
> + .type = IOREQ_TYPE_VMWARE_PORT,
> + .addr = addr,
> + .size = size,
> + .dir = dir,
> + .eax = regs->rax,
> + .ebx = regs->rbx,
> + .ecx = regs->rcx,
> + .edx = regs->rdx,
> + .esi = regs->rsi,
> + .edi = regs->rdi,
> + };
> + ioreq_t *pp = (ioreq_t *)&p;
> + ioreq_t op;
> +
> + ASSERT(sizeof(p) == sizeof(op));
> + ASSERT(offsetof(ioreq_t, type) == offsetof(vmware_ioreq_t, type));
> + ASSERT(offsetof(ioreq_t, vp_eport) == offsetof(vmware_ioreq_t,
> vp_eport));
Can we not avoid this overloading of the ioreq structure by having the emulator directly modify the vCPU registers? Since the vCPU is paused for emulation, could it not just do a get context/set context to tweak the values?
Paul
> +
> + switch ( vio->io_state )
> + {
> + case HVMIO_none:
> + break;
> + case HVMIO_completed:
> + vio->io_state = HVMIO_none;
> + goto finish_access_vmport;
> + case HVMIO_dispatched:
> + /* May have to wait for previous cycle of a multi-write to complete. */
> + default:
> + return X86EMUL_UNHANDLEABLE;
> + }
> +
> + if ( hvm_io_pending(curr) )
> + {
> + gdprintk(XENLOG_WARNING, "WARNING: io already pending?\n");
> + return X86EMUL_UNHANDLEABLE;
> + }
> +
> + vio->io_state = HVMIO_handle_vmport_awaiting_completion;
> + vio->io_size = size;
> +
> + /* If there is no backing DM, just ignore accesses */
> + if ( !hvm_has_dm(curr->domain) )
> + {
> + vio->io_state = HVMIO_none;
> + vio->io_data = ~0ul;
> + }
> + else
> + {
> + if ( !hvm_send_assist_req(pp) )
> + vio->io_state = HVMIO_none;
> + return X86EMUL_RETRY;
> + }
> +
> + finish_access_vmport:
> + memset(&op, 0, sizeof(op));
> + op.dir = dir;
> + op.addr = (uint16_t)regs->rdx;
> + op.data_is_ptr = 0;
> + op.data = vio->io_data;
> + hvmtrace_io_assist(0, &op);
> +
> + memcpy(®s->rax, &vio->io_data, vio->io_size);
> +
> + return X86EMUL_OKAY;
> +}
> +
> static int hvmemul_do_io(
> int is_mmio, paddr_t addr, unsigned long *reps, int size,
> paddr_t ram_gpa, int dir, int df, void *p_data)
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index 68fb890..a8bf324 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -197,6 +197,25 @@ void hvm_io_assist(ioreq_t *p)
> else
> memcpy(&guest_cpu_user_regs()->rax, &p->data, vio->io_size);
> break;
> + case HVMIO_handle_vmport_awaiting_completion:
> + {
> + struct cpu_user_regs *regs = guest_cpu_user_regs();
> + vmware_ioreq_t *vp = (vmware_ioreq_t *)p;
> +
> + ASSERT(sizeof(*p) == sizeof(*vp));
> + ASSERT(offsetof(ioreq_t, type) == offsetof(vmware_ioreq_t, type));
> + ASSERT(offsetof(ioreq_t, vp_eport) == offsetof(vmware_ioreq_t,
> vp_eport));
> +
> + /* Always zero extension for eax */
> + regs->rax = vp->eax;
> + /* Only change the 32bit part of the register */
> + regs->rbx = (regs->rbx & 0xffffffff00000000ull) | vp->ebx;
> + regs->rcx = (regs->rcx & 0xffffffff00000000ull) | vp->ecx;
> + regs->rdx = (regs->rdx & 0xffffffff00000000ull) | vp->edx;
> + regs->rsi = (regs->rsi & 0xffffffff00000000ull) | vp->esi;
> + regs->rdi = (regs->rdi & 0xffffffff00000000ull) | vp->edi;
> + }
> + break;
> default:
> break;
> }
> diff --git a/xen/arch/x86/hvm/vmware/vmport.c
> b/xen/arch/x86/hvm/vmware/vmport.c
> index 962ee32..0649106 100644
> --- a/xen/arch/x86/hvm/vmware/vmport.c
> +++ b/xen/arch/x86/hvm/vmware/vmport.c
> @@ -147,9 +147,31 @@ int vmport_ioport(int dir, uint32_t port, uint32_t
> bytes, uint32_t *val)
> regs->rax = 0x0;
> break;
> default:
> + { /* Let backing DM handle */
> + int rc;
> +
> HVMTRACE_ND(VMPORT_UNKNOWN, 0, 1/*cycles*/, 6,
> - (bytes << 8) + dir, cmd, regs->rbx,
> + (bytes << 8) | dir, cmd, regs->rbx,
> regs->rcx, regs->rsi, regs->rdi);
> + rc = hvmemul_do_vmport(BDOOR_PORT, bytes, dir, regs);
> + switch (rc)
> + {
> + case X86EMUL_OKAY:
> + break;
> + case X86EMUL_RETRY:
> + {
> + struct hvm_vcpu_io *vio = ¤t->arch.hvm_vcpu.hvm_io;
> +
> + if ( vio->io_state != HVMIO_handle_vmport_awaiting_completion )
> + return 0;
> + break;
> + }
> + default:
> + gdprintk(XENLOG_ERR, "Weird HVM ioemulation status %d.\n", rc);
> + domain_crash(current->domain);
> + break;
> + }
> + }
> break;
> }
>
> diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm-
> x86/hvm/emulate.h
> index 5411302..7d18435 100644
> --- a/xen/include/asm-x86/hvm/emulate.h
> +++ b/xen/include/asm-x86/hvm/emulate.h
> @@ -53,6 +53,8 @@ struct segment_register *hvmemul_get_seg_reg(
> int hvmemul_do_pio(
> unsigned long port, unsigned long *reps, int size,
> paddr_t ram_gpa, int dir, int df, void *p_data);
> +int hvmemul_do_vmport(uint16_t addr, int size, int dir,
> + struct cpu_user_regs *regs);
>
> void hvm_dump_emulation_state(const char *prefix,
> struct hvm_emulate_ctxt *hvmemul_ctxt);
> diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-
> x86/hvm/vcpu.h
> index 01e0665..1e63d7f 100644
> --- a/xen/include/asm-x86/hvm/vcpu.h
> +++ b/xen/include/asm-x86/hvm/vcpu.h
> @@ -36,6 +36,7 @@ enum hvm_io_state {
> HVMIO_awaiting_completion,
> HVMIO_handle_mmio_awaiting_completion,
> HVMIO_handle_pio_awaiting_completion,
> + HVMIO_handle_vmport_awaiting_completion,
> HVMIO_completed
> };
>
> diff --git a/xen/include/public/hvm/ioreq.h
> b/xen/include/public/hvm/ioreq.h
> index 5b5fedf..7d5ba58 100644
> --- a/xen/include/public/hvm/ioreq.h
> +++ b/xen/include/public/hvm/ioreq.h
> @@ -35,6 +35,7 @@
> #define IOREQ_TYPE_PIO 0 /* pio */
> #define IOREQ_TYPE_COPY 1 /* mmio ops */
> #define IOREQ_TYPE_PCI_CONFIG 2
> +#define IOREQ_TYPE_VMWARE_PORT 3
> #define IOREQ_TYPE_TIMEOFFSET 7
> #define IOREQ_TYPE_INVALIDATE 8 /* mapcache */
>
> @@ -48,6 +49,8 @@
> *
> * 63....48|47..40|39..35|34..32|31........0
> * SEGMENT |BUS |DEV |FN |OFFSET
> + *
> + * For I/O type IOREQ_TYPE_VMWARE_PORT use the vmware_ioreq.
> */
> struct ioreq {
> uint64_t addr; /* physical address */
> @@ -66,6 +69,22 @@ struct ioreq {
> };
> typedef struct ioreq ioreq_t;
>
> +struct vmware_ioreq {
> + uint32_t esi;
> + uint32_t edi;
> + uint32_t eax;
> + uint32_t ebx;
> + uint32_t ecx;
> + uint32_t edx;
> + uint32_t vp_eport; /* evtchn for notifications to/from device model */
> + uint16_t addr;
> + uint8_t state:4;
> + uint8_t dir:1; /* 1=read, 0=write */
> + uint8_t size:3;
> + uint8_t type; /* I/O type */
> +};
> +typedef struct vmware_ioreq vmware_ioreq_t;
> +
> struct shared_iopage {
> struct ioreq vcpu_ioreq[1];
> };
> --
> 1.8.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-02 18:36 ` [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT Don Slutz
2014-10-02 20:33 ` Andrew Cooper
2014-10-03 9:29 ` Paul Durrant
@ 2014-10-03 9:46 ` Paul Durrant
2014-10-03 19:56 ` [Qemu-devel] [Xen-devel] " Don Slutz
2014-10-03 19:56 ` [RFC][PATCH v2 " Don Slutz
2 siblings, 2 replies; 25+ messages in thread
From: Paul Durrant @ 2014-10-03 9:46 UTC (permalink / raw)
To: Don Slutz, xen-devel; +Cc: Keir (Xen.org), Ian Campbell, Jan Beulich
> -----Original Message-----
> From: Paul Durrant
> Sent: 03 October 2014 10:29
> To: 'Don Slutz'; xen-devel@lists.xen.org
> Cc: Keir (Xen.org); Ian Campbell; Jan Beulich
> Subject: RE: [Xen-devel] [RFC][PATCH v2 1/1] Add
> IOREQ_TYPE_VMWARE_PORT
>
> > -----Original Message-----
> > From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
> > bounces@lists.xen.org] On Behalf Of Don Slutz
> > Sent: 02 October 2014 19:36
> > To: xen-devel@lists.xen.org
> > Cc: Don Slutz; Keir (Xen.org); Ian Campbell; Jan Beulich
> > Subject: [Xen-devel] [RFC][PATCH v2 1/1] Add
> IOREQ_TYPE_VMWARE_PORT
> >
> > Signed-off-by: Don Slutz <dslutz@verizon.com>
> > ---
> > v2:
> > Fixup usage of hvmtrace_io_assist().
> > VMware only changes the 32bit part of the register.
> > Added vmware_ioreq_t
> >
> > xen/arch/x86/hvm/emulate.c | 72
> > +++++++++++++++++++++++++++++++++++++++
> > xen/arch/x86/hvm/io.c | 19 +++++++++++
> > xen/arch/x86/hvm/vmware/vmport.c | 24 ++++++++++++-
> > xen/include/asm-x86/hvm/emulate.h | 2 ++
> > xen/include/asm-x86/hvm/vcpu.h | 1 +
> > xen/include/public/hvm/ioreq.h | 19 +++++++++++
> > 6 files changed, 136 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> > index c0f47d2..215f049 100644
> > --- a/xen/arch/x86/hvm/emulate.c
> > +++ b/xen/arch/x86/hvm/emulate.c
> > @@ -50,6 +50,78 @@ static void hvmtrace_io_assist(int is_mmio, ioreq_t
> *p)
> > trace_var(event, 0/*!cycles*/, size, buffer);
> > }
> >
> > +int hvmemul_do_vmport(uint16_t addr, int size, int dir,
> > + struct cpu_user_regs *regs)
> > +{
> > + struct vcpu *curr = current;
> > + struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
> > + vmware_ioreq_t p = {
> > + .type = IOREQ_TYPE_VMWARE_PORT,
> > + .addr = addr,
> > + .size = size,
> > + .dir = dir,
> > + .eax = regs->rax,
> > + .ebx = regs->rbx,
> > + .ecx = regs->rcx,
> > + .edx = regs->rdx,
> > + .esi = regs->rsi,
> > + .edi = regs->rdi,
> > + };
> > + ioreq_t *pp = (ioreq_t *)&p;
> > + ioreq_t op;
> > +
> > + ASSERT(sizeof(p) == sizeof(op));
> > + ASSERT(offsetof(ioreq_t, type) == offsetof(vmware_ioreq_t, type));
> > + ASSERT(offsetof(ioreq_t, vp_eport) == offsetof(vmware_ioreq_t,
> > vp_eport));
>
> Can we not avoid this overloading of the ioreq structure by having the
> emulator directly modify the vCPU registers? Since the vCPU is paused for
> emulation, could it not just do a get context/set context to tweak the values?
>
I should have added...
Or if that doesn't work then surely an extra page of domheap, which can be mapped by the emulator to provide a dedicated channel, is preferable to this mechanism.
> Paul
>
> > +
> > + switch ( vio->io_state )
> > + {
> > + case HVMIO_none:
> > + break;
> > + case HVMIO_completed:
> > + vio->io_state = HVMIO_none;
> > + goto finish_access_vmport;
> > + case HVMIO_dispatched:
> > + /* May have to wait for previous cycle of a multi-write to complete. */
> > + default:
> > + return X86EMUL_UNHANDLEABLE;
> > + }
> > +
> > + if ( hvm_io_pending(curr) )
> > + {
> > + gdprintk(XENLOG_WARNING, "WARNING: io already pending?\n");
> > + return X86EMUL_UNHANDLEABLE;
> > + }
> > +
> > + vio->io_state = HVMIO_handle_vmport_awaiting_completion;
> > + vio->io_size = size;
> > +
> > + /* If there is no backing DM, just ignore accesses */
> > + if ( !hvm_has_dm(curr->domain) )
> > + {
> > + vio->io_state = HVMIO_none;
> > + vio->io_data = ~0ul;
> > + }
> > + else
> > + {
> > + if ( !hvm_send_assist_req(pp) )
> > + vio->io_state = HVMIO_none;
> > + return X86EMUL_RETRY;
> > + }
> > +
> > + finish_access_vmport:
> > + memset(&op, 0, sizeof(op));
> > + op.dir = dir;
> > + op.addr = (uint16_t)regs->rdx;
> > + op.data_is_ptr = 0;
> > + op.data = vio->io_data;
> > + hvmtrace_io_assist(0, &op);
> > +
> > + memcpy(®s->rax, &vio->io_data, vio->io_size);
> > +
> > + return X86EMUL_OKAY;
> > +}
> > +
> > static int hvmemul_do_io(
> > int is_mmio, paddr_t addr, unsigned long *reps, int size,
> > paddr_t ram_gpa, int dir, int df, void *p_data)
> > diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> > index 68fb890..a8bf324 100644
> > --- a/xen/arch/x86/hvm/io.c
> > +++ b/xen/arch/x86/hvm/io.c
> > @@ -197,6 +197,25 @@ void hvm_io_assist(ioreq_t *p)
> > else
> > memcpy(&guest_cpu_user_regs()->rax, &p->data, vio->io_size);
> > break;
> > + case HVMIO_handle_vmport_awaiting_completion:
> > + {
> > + struct cpu_user_regs *regs = guest_cpu_user_regs();
> > + vmware_ioreq_t *vp = (vmware_ioreq_t *)p;
> > +
> > + ASSERT(sizeof(*p) == sizeof(*vp));
> > + ASSERT(offsetof(ioreq_t, type) == offsetof(vmware_ioreq_t, type));
> > + ASSERT(offsetof(ioreq_t, vp_eport) == offsetof(vmware_ioreq_t,
> > vp_eport));
> > +
> > + /* Always zero extension for eax */
> > + regs->rax = vp->eax;
> > + /* Only change the 32bit part of the register */
> > + regs->rbx = (regs->rbx & 0xffffffff00000000ull) | vp->ebx;
> > + regs->rcx = (regs->rcx & 0xffffffff00000000ull) | vp->ecx;
> > + regs->rdx = (regs->rdx & 0xffffffff00000000ull) | vp->edx;
> > + regs->rsi = (regs->rsi & 0xffffffff00000000ull) | vp->esi;
> > + regs->rdi = (regs->rdi & 0xffffffff00000000ull) | vp->edi;
> > + }
> > + break;
> > default:
> > break;
> > }
> > diff --git a/xen/arch/x86/hvm/vmware/vmport.c
> > b/xen/arch/x86/hvm/vmware/vmport.c
> > index 962ee32..0649106 100644
> > --- a/xen/arch/x86/hvm/vmware/vmport.c
> > +++ b/xen/arch/x86/hvm/vmware/vmport.c
> > @@ -147,9 +147,31 @@ int vmport_ioport(int dir, uint32_t port, uint32_t
> > bytes, uint32_t *val)
> > regs->rax = 0x0;
> > break;
> > default:
> > + { /* Let backing DM handle */
> > + int rc;
> > +
> > HVMTRACE_ND(VMPORT_UNKNOWN, 0, 1/*cycles*/, 6,
> > - (bytes << 8) + dir, cmd, regs->rbx,
> > + (bytes << 8) | dir, cmd, regs->rbx,
> > regs->rcx, regs->rsi, regs->rdi);
> > + rc = hvmemul_do_vmport(BDOOR_PORT, bytes, dir, regs);
> > + switch (rc)
> > + {
> > + case X86EMUL_OKAY:
> > + break;
> > + case X86EMUL_RETRY:
> > + {
> > + struct hvm_vcpu_io *vio = ¤t->arch.hvm_vcpu.hvm_io;
> > +
> > + if ( vio->io_state != HVMIO_handle_vmport_awaiting_completion
> )
> > + return 0;
> > + break;
> > + }
> > + default:
> > + gdprintk(XENLOG_ERR, "Weird HVM ioemulation status %d.\n",
> rc);
> > + domain_crash(current->domain);
> > + break;
> > + }
> > + }
> > break;
> > }
> >
> > diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm-
> > x86/hvm/emulate.h
> > index 5411302..7d18435 100644
> > --- a/xen/include/asm-x86/hvm/emulate.h
> > +++ b/xen/include/asm-x86/hvm/emulate.h
> > @@ -53,6 +53,8 @@ struct segment_register *hvmemul_get_seg_reg(
> > int hvmemul_do_pio(
> > unsigned long port, unsigned long *reps, int size,
> > paddr_t ram_gpa, int dir, int df, void *p_data);
> > +int hvmemul_do_vmport(uint16_t addr, int size, int dir,
> > + struct cpu_user_regs *regs);
> >
> > void hvm_dump_emulation_state(const char *prefix,
> > struct hvm_emulate_ctxt *hvmemul_ctxt);
> > diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-
> > x86/hvm/vcpu.h
> > index 01e0665..1e63d7f 100644
> > --- a/xen/include/asm-x86/hvm/vcpu.h
> > +++ b/xen/include/asm-x86/hvm/vcpu.h
> > @@ -36,6 +36,7 @@ enum hvm_io_state {
> > HVMIO_awaiting_completion,
> > HVMIO_handle_mmio_awaiting_completion,
> > HVMIO_handle_pio_awaiting_completion,
> > + HVMIO_handle_vmport_awaiting_completion,
> > HVMIO_completed
> > };
> >
> > diff --git a/xen/include/public/hvm/ioreq.h
> > b/xen/include/public/hvm/ioreq.h
> > index 5b5fedf..7d5ba58 100644
> > --- a/xen/include/public/hvm/ioreq.h
> > +++ b/xen/include/public/hvm/ioreq.h
> > @@ -35,6 +35,7 @@
> > #define IOREQ_TYPE_PIO 0 /* pio */
> > #define IOREQ_TYPE_COPY 1 /* mmio ops */
> > #define IOREQ_TYPE_PCI_CONFIG 2
> > +#define IOREQ_TYPE_VMWARE_PORT 3
> > #define IOREQ_TYPE_TIMEOFFSET 7
> > #define IOREQ_TYPE_INVALIDATE 8 /* mapcache */
> >
> > @@ -48,6 +49,8 @@
> > *
> > * 63....48|47..40|39..35|34..32|31........0
> > * SEGMENT |BUS |DEV |FN |OFFSET
> > + *
> > + * For I/O type IOREQ_TYPE_VMWARE_PORT use the vmware_ioreq.
> > */
> > struct ioreq {
> > uint64_t addr; /* physical address */
> > @@ -66,6 +69,22 @@ struct ioreq {
> > };
> > typedef struct ioreq ioreq_t;
> >
> > +struct vmware_ioreq {
> > + uint32_t esi;
> > + uint32_t edi;
> > + uint32_t eax;
> > + uint32_t ebx;
> > + uint32_t ecx;
> > + uint32_t edx;
> > + uint32_t vp_eport; /* evtchn for notifications to/from device model
> */
> > + uint16_t addr;
> > + uint8_t state:4;
> > + uint8_t dir:1; /* 1=read, 0=write */
> > + uint8_t size:3;
> > + uint8_t type; /* I/O type */
> > +};
> > +typedef struct vmware_ioreq vmware_ioreq_t;
> > +
> > struct shared_iopage {
> > struct ioreq vcpu_ioreq[1];
> > };
> > --
> > 1.8.4
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-02 22:23 ` Andrew Cooper
@ 2014-10-03 9:47 ` Stefano Stabellini
2014-10-03 9:51 ` Ian Campbell
0 siblings, 1 reply; 25+ messages in thread
From: Stefano Stabellini @ 2014-10-03 9:47 UTC (permalink / raw)
To: Andrew Cooper
Cc: Jan Beulich, Keir Fraser, Ian Campbell, Don Slutz, xen-devel
[-- Attachment #1: Type: text/plain, Size: 5350 bytes --]
On Thu, 2 Oct 2014, Andrew Cooper wrote:
> On 02/10/2014 22:56, Don Slutz wrote:
> > On 10/02/14 16:33, Andrew Cooper wrote:
> >> On 02/10/14 19:36, Don Slutz wrote:
> >>> Signed-off-by: Don Slutz <dslutz@verizon.com>
> >>> ---
> >>> v2:
> >>> Fixup usage of hvmtrace_io_assist().
> >>> VMware only changes the 32bit part of the register.
> >>> Added vmware_ioreq_t
> >>>
> >>> xen/arch/x86/hvm/emulate.c | 72
> >>> +++++++++++++++++++++++++++++++++++++++
> >>> xen/arch/x86/hvm/io.c | 19 +++++++++++
> >>> xen/arch/x86/hvm/vmware/vmport.c | 24 ++++++++++++-
> >>> xen/include/asm-x86/hvm/emulate.h | 2 ++
> >>> xen/include/asm-x86/hvm/vcpu.h | 1 +
> >>> xen/include/public/hvm/ioreq.h | 19 +++++++++++
> >>> 6 files changed, 136 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> >>> index c0f47d2..215f049 100644
> >>> --- a/xen/arch/x86/hvm/emulate.c
> >>> +++ b/xen/arch/x86/hvm/emulate.c
> >>> @@ -50,6 +50,78 @@ static void hvmtrace_io_assist(int is_mmio,
> >>> ioreq_t *p)
> >>> trace_var(event, 0/*!cycles*/, size, buffer);
> >>> }
> >>> +int hvmemul_do_vmport(uint16_t addr, int size, int dir,
> >>> + struct cpu_user_regs *regs)
> >>> +{
> >>> + struct vcpu *curr = current;
> >>> + struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
> >>> + vmware_ioreq_t p = {
> >>> + .type = IOREQ_TYPE_VMWARE_PORT,
> >>> + .addr = addr,
> >>> + .size = size,
> >>> + .dir = dir,
> >>> + .eax = regs->rax,
> >>> + .ebx = regs->rbx,
> >>> + .ecx = regs->rcx,
> >>> + .edx = regs->rdx,
> >>> + .esi = regs->rsi,
> >>> + .edi = regs->rdi,
> >>> + };
> >>> + ioreq_t *pp = (ioreq_t *)&p;
> >>> + ioreq_t op;
> >> Eww.
> >>
> >> Just because the C type system lets you abuse it like this doesn't mean
> >> it is a clever idea to. Please refer to c/s 15a9f34d1b as an example of
> >> the kinds of bugs it causes.
> >
> > This is a direct result of:
> >
> >
> > Subject: Re: [PATCH 1/1] xen-hvm.c: Add support for Xen access to
> > vmport
> > Date: Tue, 30 Sep 2014 11:35:44 +0100
> > From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > To: Don Slutz <dslutz@verizon.com>
> > CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
> > qemu-devel@nongnu.org, xen-devel@lists.xensource.com, Alexander Graf
> > <agraf@suse.de>, Andreas Färber <afaerber@suse.de>, Anthony Liguori
> > <aliguori@amazon.com>, Marcel Apfelbaum <marcel.a@redhat.com>, Markus
> > Armbruster <armbru@redhat.com>, Michael S. Tsirkin <mst@redhat.com>
> >
> >
> >
> > On Mon, 29 Sep 2014, Don Slutz wrote:
> >> On 09/29/14 06:25, Stefano Stabellini wrote:
> >> > On Mon, 29 Sep 2014, Stefano Stabellini wrote:
> >> > > On Fri, 26 Sep 2014, Don Slutz wrote:
> >> > > > This adds synchronisation of the vcpu registers
> >> > > > between Xen and QEMU.
> >
> > ...
> >
> >> > > > + CPUX86State *env;
> >> > > > + ioreq_t fake_req = {
> >> > > > + .type = IOREQ_TYPE_PIO,
> >> > > > + .addr = (uint16_t)req->size,
> >> > > > + .size = 4,
> >> > > > + .dir = IOREQ_READ,
> >> > > > + .df = 0,
> >> > > > + .data_is_ptr = 0,
> >> > > > + };
> >> > Why do you need a fake req?
> >>
> >> To transport the 6 VCPU registers (only 32bits of them) that vmport.c
> >> needs to do it's work.
> >>
> >> > Couldn't Xen send the real req instead?
> >>
> >> Yes, but then a 2nd exchange between QEMU and Xen would be needed
> >> to fetch the 6 VCPU registers. The ways I know of to fetch the VCPU
> >> registers
> >> from Xen, all need many cycles to do their work and return
> >> a lot of data that is not needed.
> >>
> >> The other option that I have considered was to extend the ioreq_t type
> >> to have room for these registers, but that reduces by almost half the
> >> maximum number of VCPUs that are supported (They all live on 1 page).
> >
> > Urgh. Now that I understand the patch better is think it's horrible, no
> > offense :-)
> >
> > Why don't you add another new ioreq type to send out the vcpu state?
> > Something like IOREQ_TYPE_VCPU_SYNC_REGS? You could send it to QEMU
> > before IOREQ_TYPE_VMWARE_PORT. Actually this solution looks very imilar
> > to Alex's suggestion.
> >
> > ...
> >
> >
> > And the ASSERTs below are the attempt to prevent bugs from being added.
> >
> > Sigh. Too much with both XEN and QEMU in hard freeze. I may
> > have a way to avoid the cast.
>
> I put 2 and 2 together to make something close to 4 after sending the
> first email, but unions are the C way of doing things like this.
I think that vmware_ioreq_t is still far from ideal but it is an
improvement over the previous version that was silently abusing the
ioreq_t struct. This version at least does that explicitly.
The issue with a union is compatibility with older QEMU versions: we can
introduce the union and retain compatibility only if we use anonymous
unions. However I seem to recall Jan arguing against anonymous unions
in public interfaces in past.
[-- Attachment #2: 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] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-03 9:47 ` Stefano Stabellini
@ 2014-10-03 9:51 ` Ian Campbell
2014-10-03 9:54 ` Stefano Stabellini
0 siblings, 1 reply; 25+ messages in thread
From: Ian Campbell @ 2014-10-03 9:51 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Andrew Cooper, Keir Fraser, Jan Beulich, Don Slutz, xen-devel
On Fri, 2014-10-03 at 10:47 +0100, Stefano Stabellini wrote:
> The issue with a union is compatibility with older QEMU versions: we can
> introduce the union and retain compatibility only if we use anonymous
> unions. However I seem to recall Jan arguing against anonymous unions
> in public interfaces in past.
The canonical headers in xen/include/public are supposed to be strict
ANSI C and anonymous unions are a gcc extension.
However no-one is obliged to use this copy and several projects
(including Linux, *BSD and others) take copies and modify them to suite
their local coding styles/conventions etc. That could include using
anonymous unions if that is preferable. I'm not sure if that helps you
here though (since the issue AIUI is with existing qemu releases...)
Ian.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-03 9:51 ` Ian Campbell
@ 2014-10-03 9:54 ` Stefano Stabellini
2014-10-03 19:27 ` Don Slutz
0 siblings, 1 reply; 25+ messages in thread
From: Stefano Stabellini @ 2014-10-03 9:54 UTC (permalink / raw)
To: Ian Campbell
Cc: Keir Fraser, Stefano Stabellini, Andrew Cooper, Don Slutz,
xen-devel, Jan Beulich
On Fri, 3 Oct 2014, Ian Campbell wrote:
> On Fri, 2014-10-03 at 10:47 +0100, Stefano Stabellini wrote:
> > The issue with a union is compatibility with older QEMU versions: we can
> > introduce the union and retain compatibility only if we use anonymous
> > unions. However I seem to recall Jan arguing against anonymous unions
> > in public interfaces in past.
>
> The canonical headers in xen/include/public are supposed to be strict
> ANSI C and anonymous unions are a gcc extension.
>
> However no-one is obliged to use this copy and several projects
> (including Linux, *BSD and others) take copies and modify them to suite
> their local coding styles/conventions etc. That could include using
> anonymous unions if that is preferable. I'm not sure if that helps you
> here though (since the issue AIUI is with existing qemu releases...)
Right, it doesn't help.
Even upstream QEMU still builds against external Xen header files.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-03 9:54 ` Stefano Stabellini
@ 2014-10-03 19:27 ` Don Slutz
2014-10-06 7:55 ` Jan Beulich
0 siblings, 1 reply; 25+ messages in thread
From: Don Slutz @ 2014-10-03 19:27 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Keir Fraser, Ian Campbell, Andrew Cooper, Don Slutz, xen-devel,
Jan Beulich
On 10/03/14 05:54, Stefano Stabellini wrote:
> On Fri, 3 Oct 2014, Ian Campbell wrote:
>> On Fri, 2014-10-03 at 10:47 +0100, Stefano Stabellini wrote:
>>> The issue with a union is compatibility with older QEMU versions: we can
>>> introduce the union and retain compatibility only if we use anonymous
>>> unions. However I seem to recall Jan arguing against anonymous unions
>>> in public interfaces in past.
>> The canonical headers in xen/include/public are supposed to be strict
>> ANSI C and anonymous unions are a gcc extension.
>>
>> However no-one is obliged to use this copy and several projects
>> (including Linux, *BSD and others) take copies and modify them to suite
>> their local coding styles/conventions etc. That could include using
>> anonymous unions if that is preferable. I'm not sure if that helps you
>> here though (since the issue AIUI is with existing qemu releases...)
> Right, it doesn't help.
> Even upstream QEMU still builds against external Xen header files.
Here is a union that as far as I know is ANSI C and avoids a cast to a
different
type. But it is a lot of changes just to avoid this.
Should I spend the time to make it part of this?
From 84b4f8eb944d436b4e47e4024ebefbbe4460cd32 Mon Sep 17 00:00:00 2001
From: Don Slutz <dslutz@verizon.com>
Date: Fri, 3 Oct 2014 15:15:11 -0400
Subject: [PATCH] Add union_ioreq_t
Signed-off-by: Don Slutz <dslutz@verizon.com>
---
xen/arch/x86/hvm/emulate.c | 90
+++++++++++++++++++++---------------------
xen/arch/x86/hvm/hvm.c | 73 +++++++++++++++++-----------------
xen/arch/x86/hvm/io.c | 27 ++++++-------
xen/include/asm-x86/hvm/hvm.h | 2 +-
xen/include/asm-x86/hvm/io.h | 2 +-
xen/include/public/hvm/ioreq.h | 11 ++++++
6 files changed, 109 insertions(+), 96 deletions(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 5f8c551..cee8a37 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -55,20 +55,18 @@ int hvmemul_do_vmport(uint16_t addr, int size, int dir,
{
struct vcpu *curr = current;
struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
- vmware_ioreq_t p = {
- .type = IOREQ_TYPE_VMWARE_PORT,
- .addr = addr,
- .size = size,
- .dir = dir,
- .eax = regs->rax,
- .ebx = regs->rbx,
- .ecx = regs->rcx,
- .edx = regs->rdx,
- .esi = regs->rsi,
- .edi = regs->rdi,
+ union_ioreq_t up = {
+ .vreq.type = IOREQ_TYPE_VMWARE_PORT,
+ .vreq.addr = addr,
+ .vreq.size = size,
+ .vreq.dir = dir,
+ .vreq.eax = regs->rax,
+ .vreq.ebx = regs->rbx,
+ .vreq.ecx = regs->rcx,
+ .vreq.edx = regs->rdx,
+ .vreq.esi = regs->rsi,
+ .vreq.edi = regs->rdi,
};
- ioreq_t *pp = (ioreq_t *)&p;
- ioreq_t op;
BUILD_BUG_ON(sizeof(ioreq_t) != sizeof(vmware_ioreq_t));
BUILD_BUG_ON(offsetof(ioreq_t, type) != offsetof(vmware_ioreq_t,
type));
@@ -104,21 +102,25 @@ int hvmemul_do_vmport(uint16_t addr, int size, int
dir,
}
else
{
- if ( !hvm_send_assist_req(pp) )
+ if ( !hvm_send_assist_req(&up) )
vio->io_state = HVMIO_none;
return X86EMUL_RETRY;
}
finish_access_vmport:
- memset(&op, 0, sizeof(op));
- op.dir = dir;
- op.addr = (uint16_t)regs->rdx;
- op.data_is_ptr = 0;
- op.data = vio->io_data;
- hvmtrace_io_assist(0, &op);
+ {
+ ioreq_t op = {
+ .type = IOREQ_TYPE_PIO,
+ .dir = dir,
+ .addr = addr,
+ .data_is_ptr = 0,
+ .data = vio->io_data,
+ };
+
+ hvmtrace_io_assist(0, &op);
+ }
memcpy(®s->rax, &vio->io_data, vio->io_size);
-
return X86EMUL_OKAY;
}
@@ -128,14 +130,14 @@ static int hvmemul_do_io(
{
struct vcpu *curr = current;
struct hvm_vcpu_io *vio;
- ioreq_t p = {
- .type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
- .addr = addr,
- .size = size,
- .dir = dir,
- .df = df,
- .data = ram_gpa,
- .data_is_ptr = (p_data == NULL),
+ union_ioreq_t up = {
+ .oreq.type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
+ .oreq.addr = addr,
+ .oreq.size = size,
+ .oreq.dir = dir,
+ .oreq.df = df,
+ .oreq.data = ram_gpa,
+ .oreq.data_is_ptr = (p_data == NULL),
};
unsigned long ram_gfn = paddr_to_pfn(ram_gpa);
p2m_type_t p2mt;
@@ -173,15 +175,15 @@ static int hvmemul_do_io(
return X86EMUL_UNHANDLEABLE;
}
- if ( !p.data_is_ptr && (dir == IOREQ_WRITE) )
+ if ( !up.oreq.data_is_ptr && (dir == IOREQ_WRITE) )
{
- memcpy(&p.data, p_data, size);
+ memcpy(&up.oreq.data, p_data, size);
p_data = NULL;
}
vio = &curr->arch.hvm_vcpu.hvm_io;
- if ( is_mmio && !p.data_is_ptr )
+ if ( is_mmio && !up.oreq.data_is_ptr )
{
/* Part of a multi-cycle read or write? */
if ( dir == IOREQ_WRITE )
@@ -225,7 +227,7 @@ static int hvmemul_do_io(
goto finish_access;
case HVMIO_dispatched:
/* May have to wait for previous cycle of a multi-write to
complete. */
- if ( is_mmio && !p.data_is_ptr && (dir == IOREQ_WRITE) &&
+ if ( is_mmio && !up.oreq.data_is_ptr && (dir == IOREQ_WRITE) &&
(addr == (vio->mmio_large_write_pa +
vio->mmio_large_write_bytes)) )
{
@@ -258,31 +260,31 @@ static int hvmemul_do_io(
if ( vio->mmio_retrying )
*reps = 1;
- p.count = *reps;
+ up.oreq.count = *reps;
if ( dir == IOREQ_WRITE )
- hvmtrace_io_assist(is_mmio, &p);
+ hvmtrace_io_assist(is_mmio, &up.oreq);
if ( is_mmio )
{
- rc = hvm_mmio_intercept(&p);
+ rc = hvm_mmio_intercept(&up.oreq);
if ( rc == X86EMUL_UNHANDLEABLE )
- rc = hvm_buffered_io_intercept(&p);
+ rc = hvm_buffered_io_intercept(&up.oreq);
}
else
{
- rc = hvm_portio_intercept(&p);
+ rc = hvm_portio_intercept(&up.oreq);
}
switch ( rc )
{
case X86EMUL_OKAY:
case X86EMUL_RETRY:
- *reps = p.count;
- p.state = STATE_IORESP_READY;
+ *reps = up.oreq.count;
+ up.oreq.state = STATE_IORESP_READY;
if ( !vio->mmio_retry )
{
- hvm_io_assist(&p);
+ hvm_io_assist(&up);
vio->io_state = HVMIO_none;
}
else
@@ -299,7 +301,7 @@ static int hvmemul_do_io(
else
{
rc = X86EMUL_RETRY;
- if ( !hvm_send_assist_req(&p) )
+ if ( !hvm_send_assist_req(&up) )
vio->io_state = HVMIO_none;
else if ( p_data == NULL )
rc = X86EMUL_OKAY;
@@ -318,12 +320,12 @@ static int hvmemul_do_io(
finish_access:
if ( dir == IOREQ_READ )
- hvmtrace_io_assist(is_mmio, &p);
+ hvmtrace_io_assist(is_mmio, &up.oreq);
if ( p_data != NULL )
memcpy(p_data, &vio->io_data, size);
- if ( is_mmio && !p.data_is_ptr )
+ if ( is_mmio && !up.oreq.data_is_ptr )
{
/* Part of a multi-cycle read or write? */
if ( dir == IOREQ_WRITE )
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 83a7000..d1f3e52 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -372,9 +372,9 @@ void hvm_migrate_pirqs(struct vcpu *v)
spin_unlock(&d->event_lock);
}
-static ioreq_t *get_ioreq(struct hvm_ioreq_server *s, struct vcpu *v)
+static union_ioreq_t *get_ioreq(struct hvm_ioreq_server *s, struct vcpu *v)
{
- shared_iopage_t *p = s->ioreq.va;
+ union_shared_iopage_t *p = s->ioreq.va;
ASSERT((v == current) || !vcpu_runnable(v));
ASSERT(p != NULL);
@@ -391,34 +391,35 @@ bool_t hvm_io_pending(struct vcpu *v)
&d->arch.hvm_domain.ioreq_server.list,
list_entry )
{
- ioreq_t *p = get_ioreq(s, v);
+ union_ioreq_t *up = get_ioreq(s, v);
- if ( p->state != STATE_IOREQ_NONE )
+ if ( up->oreq.state != STATE_IOREQ_NONE )
return 1;
}
return 0;
}
-static bool_t hvm_wait_for_io(struct hvm_ioreq_vcpu *sv, ioreq_t *p)
+static bool_t hvm_wait_for_io(struct hvm_ioreq_vcpu *sv,
+ union_ioreq_t *up)
{
- /* NB. Optimised for common case (p->state == STATE_IOREQ_NONE). */
- while ( p->state != STATE_IOREQ_NONE )
+ /* NB. Optimised for common case (up->oreq.state ==
STATE_IOREQ_NONE). */
+ while ( up->oreq.state != STATE_IOREQ_NONE )
{
- switch ( p->state )
+ switch ( up->oreq.state )
{
case STATE_IORESP_READY: /* IORESP_READY -> NONE */
rmb(); /* see IORESP_READY /then/ read contents of ioreq */
- hvm_io_assist(p);
+ hvm_io_assist(up);
break;
case STATE_IOREQ_READY: /* IOREQ_{READY,INPROCESS} ->
IORESP_READY */
case STATE_IOREQ_INPROCESS:
wait_on_xen_event_channel(sv->ioreq_evtchn,
- (p->state != STATE_IOREQ_READY) &&
- (p->state != STATE_IOREQ_INPROCESS));
+ (up->oreq.state !=
STATE_IOREQ_READY) &&
+ (up->oreq.state !=
STATE_IOREQ_INPROCESS));
break;
default:
- gdprintk(XENLOG_ERR, "Weird HVM iorequest state %d.\n",
p->state);
+ gdprintk(XENLOG_ERR, "Weird HVM iorequest state %d.\n",
up->oreq.state);
domain_crash(sv->vcpu->domain);
return 0; /* bail */
}
@@ -598,9 +599,9 @@ static void hvm_update_ioreq_evtchn(struct
hvm_ioreq_server *s,
if ( s->ioreq.va != NULL )
{
- ioreq_t *p = get_ioreq(s, sv->vcpu);
+ union_ioreq_t *up = get_ioreq(s, sv->vcpu);
- p->vp_eport = sv->ioreq_evtchn;
+ up->oreq.vp_eport = sv->ioreq_evtchn;
}
}
@@ -2526,27 +2527,27 @@ bool_t
hvm_send_assist_req_to_ioreq_server(struct hvm_ioreq_server *s,
if ( sv->vcpu == curr )
{
evtchn_port_t port = sv->ioreq_evtchn;
- ioreq_t *p = get_ioreq(s, curr);
+ union_ioreq_t *up = get_ioreq(s, curr);
- if ( unlikely(p->state != STATE_IOREQ_NONE) )
+ if ( unlikely(up->oreq.state != STATE_IOREQ_NONE) )
{
gdprintk(XENLOG_ERR,
"Device model set bad IO state %d.\n",
- p->state);
+ up->oreq.state);
goto crash;
}
- if ( unlikely(p->vp_eport != port) )
+ if ( unlikely(up->oreq.vp_eport != port) )
{
gdprintk(XENLOG_ERR,
"Device model set bad event channel %d.\n",
- p->vp_eport);
+ up->oreq.vp_eport);
goto crash;
}
proto_p->state = STATE_IOREQ_NONE;
proto_p->vp_eport = port;
- *p = *proto_p;
+ up->oreq = *proto_p;
prepare_wait_on_xen_event_channel(port);
@@ -2555,7 +2556,7 @@ bool_t hvm_send_assist_req_to_ioreq_server(struct
hvm_ioreq_server *s,
* contents. prepare_wait_on_xen_event_channel() is an
implicit
* barrier.
*/
- p->state = STATE_IOREQ_READY;
+ up->oreq.state = STATE_IOREQ_READY;
notify_via_xen_event_channel(d, port);
break;
}
@@ -2568,44 +2569,44 @@ bool_t
hvm_send_assist_req_to_ioreq_server(struct hvm_ioreq_server *s,
return 0;
}
-static bool_t hvm_complete_assist_req(ioreq_t *p)
+static bool_t hvm_complete_assist_req(union_ioreq_t *up)
{
- switch ( p->type )
+ switch ( up->oreq.type )
{
case IOREQ_TYPE_COPY:
case IOREQ_TYPE_PIO:
- if ( p->dir == IOREQ_READ )
+ if ( up->oreq.dir == IOREQ_READ )
{
- if ( !p->data_is_ptr )
- p->data = ~0ul;
+ if ( !up->oreq.data_is_ptr )
+ up->oreq.data = ~0ul;
else
{
- int i, step = p->df ? -p->size : p->size;
+ int i, step = up->oreq.df ? -up->oreq.size : up->oreq.size;
uint32_t data = ~0;
- for ( i = 0; i < p->count; i++ )
- hvm_copy_to_guest_phys(p->data + step * i, &data,
- p->size);
+ for ( i = 0; i < up->oreq.count; i++ )
+ hvm_copy_to_guest_phys(up->oreq.data + step * i, &data,
+ up->oreq.size);
}
}
/* FALLTHRU */
default:
- p->state = STATE_IORESP_READY;
- hvm_io_assist(p);
+ up->oreq.state = STATE_IORESP_READY;
+ hvm_io_assist(up);
break;
}
return 1;
}
-bool_t hvm_send_assist_req(ioreq_t *p)
+bool_t hvm_send_assist_req(union_ioreq_t *up)
{
- struct hvm_ioreq_server *s =
hvm_select_ioreq_server(current->domain, p);
+ struct hvm_ioreq_server *s =
hvm_select_ioreq_server(current->domain, &up->oreq);
if ( !s )
- return hvm_complete_assist_req(p);
+ return hvm_complete_assist_req(up);
- return hvm_send_assist_req_to_ioreq_server(s, p);
+ return hvm_send_assist_req_to_ioreq_server(s, &up->oreq);
}
void hvm_broadcast_assist_req(ioreq_t *p)
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index 5d7d221..a06a507 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -169,13 +169,13 @@ int handle_pio(uint16_t port, unsigned int size,
int dir)
return 1;
}
-void hvm_io_assist(ioreq_t *p)
+void hvm_io_assist(union_ioreq_t *up)
{
struct vcpu *curr = current;
struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
enum hvm_io_state io_state;
- p->state = STATE_IOREQ_NONE;
+ up->oreq.state = STATE_IOREQ_NONE;
io_state = vio->io_state;
vio->io_state = HVMIO_none;
@@ -184,39 +184,38 @@ void hvm_io_assist(ioreq_t *p)
{
case HVMIO_awaiting_completion:
vio->io_state = HVMIO_completed;
- vio->io_data = p->data;
+ vio->io_data = up->oreq.data;
break;
case HVMIO_handle_mmio_awaiting_completion:
vio->io_state = HVMIO_completed;
- vio->io_data = p->data;
+ vio->io_data = up->oreq.data;
(void)handle_mmio();
break;
case HVMIO_handle_pio_awaiting_completion:
if ( vio->io_size == 4 ) /* Needs zero extension. */
- guest_cpu_user_regs()->rax = (uint32_t)p->data;
+ guest_cpu_user_regs()->rax = (uint32_t)up->oreq.data;
else
- memcpy(&guest_cpu_user_regs()->rax, &p->data, vio->io_size);
+ memcpy(&guest_cpu_user_regs()->rax, &up->oreq.data,
vio->io_size);
break;
case HVMIO_handle_vmport_awaiting_completion:
{
struct cpu_user_regs *regs = guest_cpu_user_regs();
- vmware_ioreq_t *vp = (vmware_ioreq_t *)p;
/* Always zero extension for eax */
- regs->rax = vp->eax;
+ regs->rax = up->vreq.eax;
/* Only change the 32bit part of the register */
- regs->_ebx = vp->ebx;
- regs->_ecx = vp->ecx;
- regs->_edx = vp->edx;
- regs->_esi = vp->esi;
- regs->_edi = vp->edi;
+ regs->_ebx = up->vreq.ebx;
+ regs->_ecx = up->vreq.ecx;
+ regs->_edx = up->vreq.edx;
+ regs->_esi = up->vreq.esi;
+ regs->_edi = up->vreq.edi;
}
break;
default:
break;
}
- if ( p->state == STATE_IOREQ_NONE )
+ if ( up->oreq.state == STATE_IOREQ_NONE )
{
msix_write_completion(curr);
vcpu_end_shutdown_deferral(curr);
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 0910147..844271c 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -228,7 +228,7 @@ int hvm_vcpu_cacheattr_init(struct vcpu *v);
void hvm_vcpu_cacheattr_destroy(struct vcpu *v);
void hvm_vcpu_reset_state(struct vcpu *v, uint16_t cs, uint16_t ip);
-bool_t hvm_send_assist_req(ioreq_t *p);
+bool_t hvm_send_assist_req(union_ioreq_t *up);
void hvm_broadcast_assist_req(ioreq_t *p);
void hvm_get_guest_pat(struct vcpu *v, u64 *guest_pat);
diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h
index d257161..1a183a1 100644
--- a/xen/include/asm-x86/hvm/io.h
+++ b/xen/include/asm-x86/hvm/io.h
@@ -123,7 +123,7 @@ int handle_mmio_with_translation(unsigned long gva,
unsigned long gpfn,
struct npfec);
int handle_pio(uint16_t port, unsigned int size, int dir);
void hvm_interrupt_post(struct vcpu *v, int vector, int type);
-void hvm_io_assist(ioreq_t *p);
+void hvm_io_assist(union_ioreq_t *p);
void hvm_dpci_eoi(struct domain *d, unsigned int guest_irq,
const union vioapic_redir_entry *ent);
void msix_write_completion(struct vcpu *);
diff --git a/xen/include/public/hvm/ioreq.h b/xen/include/public/hvm/ioreq.h
index 7d5ba58..d75ce66 100644
--- a/xen/include/public/hvm/ioreq.h
+++ b/xen/include/public/hvm/ioreq.h
@@ -85,11 +85,22 @@ struct vmware_ioreq {
};
typedef struct vmware_ioreq vmware_ioreq_t;
+union union_ioreq {
+ ioreq_t oreq;
+ vmware_ioreq_t vreq;
+};
+typedef union union_ioreq union_ioreq_t;
+
struct shared_iopage {
struct ioreq vcpu_ioreq[1];
};
typedef struct shared_iopage shared_iopage_t;
+struct union_shared_iopage {
+ union union_ioreq vcpu_ioreq[1];
+};
+typedef struct union_shared_iopage union_shared_iopage_t;
+
struct buf_ioreq {
uint8_t type; /* I/O type */
uint8_t pad:1;
--
1.7.11.7
-Don Slutz
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-03 9:29 ` Paul Durrant
@ 2014-10-03 19:44 ` Don Slutz
0 siblings, 0 replies; 25+ messages in thread
From: Don Slutz @ 2014-10-03 19:44 UTC (permalink / raw)
To: Paul Durrant
Cc: Jan Beulich, Keir (Xen.org), Ian Campbell, Don Slutz, xen-devel
On 10/03/14 05:29, Paul Durrant wrote:
>> -----Original Message-----
>> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
>> bounces@lists.xen.org] On Behalf Of Don Slutz
>> Sent: 02 October 2014 19:36
>> To: xen-devel@lists.xen.org
>> Cc: Don Slutz; Keir (Xen.org); Ian Campbell; Jan Beulich
>> Subject: [Xen-devel] [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
>>
>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>> ---
>> v2:
>> Fixup usage of hvmtrace_io_assist().
>> VMware only changes the 32bit part of the register.
>> Added vmware_ioreq_t
>>
>> xen/arch/x86/hvm/emulate.c | 72
>> +++++++++++++++++++++++++++++++++++++++
...
>> +
>> + ASSERT(sizeof(p) == sizeof(op));
>> + ASSERT(offsetof(ioreq_t, type) == offsetof(vmware_ioreq_t, type));
>> + ASSERT(offsetof(ioreq_t, vp_eport) == offsetof(vmware_ioreq_t,
>> vp_eport));
> Can we not avoid this overloading of the ioreq structure by having the emulator directly modify the vCPU registers?
Yes we can at a high cost of cpu overhead. The current ways of accessing
registers are mostly way too many registers and other side effects. Using
the debugger interface (which I do not know as well) has a high cost.
> Since the vCPU is paused for emulation, could it not just do a get context/set context to tweak the values?
It is blocked not paused, and while I have not tried it, I would expect
it to work.
However it does require switching from qemu to the hypervisor and back 2
times
which is not free.
So I feel that adding a lot of overhead to avoid a new type ioreq_t is
the wrong
way to go.
-Don Slutz
> Paul
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [Xen-devel] [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-03 9:46 ` Paul Durrant
@ 2014-10-03 19:56 ` Don Slutz
2014-10-09 14:26 ` [RFC][PATCH v2x prototype " Don Slutz
2014-10-03 19:56 ` [RFC][PATCH v2 " Don Slutz
1 sibling, 1 reply; 25+ messages in thread
From: Don Slutz @ 2014-10-03 19:56 UTC (permalink / raw)
To: Paul Durrant
Cc: Keir (Xen.org),
Ian Campbell, Stefano Stabellini, qemu-devel, Don Slutz,
xen-devel, Jan Beulich
On 10/03/14 05:46, Paul Durrant wrote:
>> -----Original Message-----
>> From: Paul Durrant
>> Sent: 03 October 2014 10:29
>> To: 'Don Slutz'; xen-devel@lists.xen.org
>> Cc: Keir (Xen.org); Ian Campbell; Jan Beulich
>> Subject: RE: [Xen-devel] [RFC][PATCH v2 1/1] Add
>> IOREQ_TYPE_VMWARE_PORT
>>
>>> -----Original Message-----
>>> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
>>> bounces@lists.xen.org] On Behalf Of Don Slutz
>>> Sent: 02 October 2014 19:36
>>> To: xen-devel@lists.xen.org
>>> Cc: Don Slutz; Keir (Xen.org); Ian Campbell; Jan Beulich
>>> Subject: [Xen-devel] [RFC][PATCH v2 1/1] Add
>> IOREQ_TYPE_VMWARE_PORT
>>
...
>> Can we not avoid this overloading of the ioreq structure by having the
>> emulator directly modify the vCPU registers? Since the vCPU is paused for
>> emulation, could it not just do a get context/set context to tweak the values?
>>
> I should have added...
>
> Or if that doesn't work then surely an extra page of domheap, which can be mapped by the emulator to provide a dedicated channel, is preferable to this mechanism.
So for a 1 cpu guest adding a page (4096 bytes) to move 24 bytes of data
is better
for this?
I would still add a new type. And I would need a slot per cpu just like
shared_iopage_t
of the same size or smaller (so it can stay 1 page).
I can prototype this out and see how big a change it would be. It does
have an impact
on QEMU so cc: qemu-devel
-Don Slutz
>> Paul
>>
>>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-03 9:46 ` Paul Durrant
2014-10-03 19:56 ` [Qemu-devel] [Xen-devel] " Don Slutz
@ 2014-10-03 19:56 ` Don Slutz
1 sibling, 0 replies; 25+ messages in thread
From: Don Slutz @ 2014-10-03 19:56 UTC (permalink / raw)
To: Paul Durrant
Cc: Keir (Xen.org),
Ian Campbell, Stefano Stabellini, qemu-devel, Don Slutz,
xen-devel, Jan Beulich
On 10/03/14 05:46, Paul Durrant wrote:
>> -----Original Message-----
>> From: Paul Durrant
>> Sent: 03 October 2014 10:29
>> To: 'Don Slutz'; xen-devel@lists.xen.org
>> Cc: Keir (Xen.org); Ian Campbell; Jan Beulich
>> Subject: RE: [Xen-devel] [RFC][PATCH v2 1/1] Add
>> IOREQ_TYPE_VMWARE_PORT
>>
>>> -----Original Message-----
>>> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
>>> bounces@lists.xen.org] On Behalf Of Don Slutz
>>> Sent: 02 October 2014 19:36
>>> To: xen-devel@lists.xen.org
>>> Cc: Don Slutz; Keir (Xen.org); Ian Campbell; Jan Beulich
>>> Subject: [Xen-devel] [RFC][PATCH v2 1/1] Add
>> IOREQ_TYPE_VMWARE_PORT
>>
...
>> Can we not avoid this overloading of the ioreq structure by having the
>> emulator directly modify the vCPU registers? Since the vCPU is paused for
>> emulation, could it not just do a get context/set context to tweak the values?
>>
> I should have added...
>
> Or if that doesn't work then surely an extra page of domheap, which can be mapped by the emulator to provide a dedicated channel, is preferable to this mechanism.
So for a 1 cpu guest adding a page (4096 bytes) to move 24 bytes of data
is better
for this?
I would still add a new type. And I would need a slot per cpu just like
shared_iopage_t
of the same size or smaller (so it can stay 1 page).
I can prototype this out and see how big a change it would be. It does
have an impact
on QEMU so cc: qemu-devel
-Don Slutz
>> Paul
>>
>>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-03 19:27 ` Don Slutz
@ 2014-10-06 7:55 ` Jan Beulich
2014-10-06 9:21 ` Stefano Stabellini
0 siblings, 1 reply; 25+ messages in thread
From: Jan Beulich @ 2014-10-06 7:55 UTC (permalink / raw)
To: Don Slutz
Cc: Andrew Cooper, xen-devel, Keir Fraser, Ian Campbell, Stefano Stabellini
>>> On 03.10.14 at 21:27, <dslutz@verizon.com> wrote:
> --- a/xen/include/public/hvm/ioreq.h
> +++ b/xen/include/public/hvm/ioreq.h
> @@ -85,11 +85,22 @@ struct vmware_ioreq {
> };
> typedef struct vmware_ioreq vmware_ioreq_t;
>
> +union union_ioreq {
> + ioreq_t oreq;
> + vmware_ioreq_t vreq;
> +};
> +typedef union union_ioreq union_ioreq_t;
> +
> struct shared_iopage {
> struct ioreq vcpu_ioreq[1];
> };
> typedef struct shared_iopage shared_iopage_t;
>
> +struct union_shared_iopage {
> + union union_ioreq vcpu_ioreq[1];
> +};
> +typedef struct union_shared_iopage union_shared_iopage_t;
I don't think either of these really need to be part of the public
interface. (That said, I'm far from being convinced of this
overloading mechanism, and in fact much of this VMware support
code, in the first place.)
Jan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-06 7:55 ` Jan Beulich
@ 2014-10-06 9:21 ` Stefano Stabellini
2014-10-06 9:39 ` Jan Beulich
2014-10-06 9:54 ` Paul Durrant
0 siblings, 2 replies; 25+ messages in thread
From: Stefano Stabellini @ 2014-10-06 9:21 UTC (permalink / raw)
To: Jan Beulich
Cc: Keir Fraser, Ian Campbell, Stefano Stabellini, Andrew Cooper,
Don Slutz, xen-devel
On Mon, 6 Oct 2014, Jan Beulich wrote:
> >>> On 03.10.14 at 21:27, <dslutz@verizon.com> wrote:
> > --- a/xen/include/public/hvm/ioreq.h
> > +++ b/xen/include/public/hvm/ioreq.h
> > @@ -85,11 +85,22 @@ struct vmware_ioreq {
> > };
> > typedef struct vmware_ioreq vmware_ioreq_t;
> >
> > +union union_ioreq {
> > + ioreq_t oreq;
> > + vmware_ioreq_t vreq;
> > +};
> > +typedef union union_ioreq union_ioreq_t;
> > +
> > struct shared_iopage {
> > struct ioreq vcpu_ioreq[1];
> > };
> > typedef struct shared_iopage shared_iopage_t;
> >
> > +struct union_shared_iopage {
> > + union union_ioreq vcpu_ioreq[1];
> > +};
> > +typedef struct union_shared_iopage union_shared_iopage_t;
>
> I don't think either of these really need to be part of the public
> interface.
Do you prefer this union mechanism or the original code of this patch
1412274977-6098-2-git-send-email-dslutz@verizon.com?
I haven't see the QEMU code for the union approach, but both
alternatives would probably be fine from the QEMU POV.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-06 9:21 ` Stefano Stabellini
@ 2014-10-06 9:39 ` Jan Beulich
2014-10-06 19:51 ` Don Slutz
2014-10-06 9:54 ` Paul Durrant
1 sibling, 1 reply; 25+ messages in thread
From: Jan Beulich @ 2014-10-06 9:39 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Andrew Cooper, Keir Fraser, Ian Campbell, Don Slutz, xen-devel
>>> On 06.10.14 at 11:21, <stefano.stabellini@eu.citrix.com> wrote:
> On Mon, 6 Oct 2014, Jan Beulich wrote:
>> >>> On 03.10.14 at 21:27, <dslutz@verizon.com> wrote:
>> > --- a/xen/include/public/hvm/ioreq.h
>> > +++ b/xen/include/public/hvm/ioreq.h
>> > @@ -85,11 +85,22 @@ struct vmware_ioreq {
>> > };
>> > typedef struct vmware_ioreq vmware_ioreq_t;
>> >
>> > +union union_ioreq {
>> > + ioreq_t oreq;
>> > + vmware_ioreq_t vreq;
>> > +};
>> > +typedef union union_ioreq union_ioreq_t;
>> > +
>> > struct shared_iopage {
>> > struct ioreq vcpu_ioreq[1];
>> > };
>> > typedef struct shared_iopage shared_iopage_t;
>> >
>> > +struct union_shared_iopage {
>> > + union union_ioreq vcpu_ioreq[1];
>> > +};
>> > +typedef struct union_shared_iopage union_shared_iopage_t;
>>
>> I don't think either of these really need to be part of the public
>> interface.
>
> Do you prefer this union mechanism or the original code of this patch
> 1412274977-6098-2-git-send-email-dslutz@verizon.com?
>
> I haven't see the QEMU code for the union approach, but both
> alternatives would probably be fine from the QEMU POV.
I'm fine with the union approach (subject to convincing myself that
all this effort and new code is really worth it in the first place, as
said elsewhere before), just not as part of the public interface
(unless there is a clear need for it to be there).
Jan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-06 9:21 ` Stefano Stabellini
2014-10-06 9:39 ` Jan Beulich
@ 2014-10-06 9:54 ` Paul Durrant
1 sibling, 0 replies; 25+ messages in thread
From: Paul Durrant @ 2014-10-06 9:54 UTC (permalink / raw)
To: Jan Beulich
Cc: Keir (Xen.org),
Ian Campbell, Andrew Cooper, Don Slutz, xen-devel,
Stefano Stabellini
> -----Original Message-----
> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
> bounces@lists.xen.org] On Behalf Of Stefano Stabellini
> Sent: 06 October 2014 10:22
> To: Jan Beulich
> Cc: Keir (Xen.org); Ian Campbell; Stefano Stabellini; Andrew Cooper; Don
> Slutz; xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [RFC][PATCH v2 1/1] Add
> IOREQ_TYPE_VMWARE_PORT
>
> On Mon, 6 Oct 2014, Jan Beulich wrote:
> > >>> On 03.10.14 at 21:27, <dslutz@verizon.com> wrote:
> > > --- a/xen/include/public/hvm/ioreq.h
> > > +++ b/xen/include/public/hvm/ioreq.h
> > > @@ -85,11 +85,22 @@ struct vmware_ioreq {
> > > };
> > > typedef struct vmware_ioreq vmware_ioreq_t;
> > >
> > > +union union_ioreq {
> > > + ioreq_t oreq;
> > > + vmware_ioreq_t vreq;
> > > +};
> > > +typedef union union_ioreq union_ioreq_t;
> > > +
> > > struct shared_iopage {
> > > struct ioreq vcpu_ioreq[1];
> > > };
> > > typedef struct shared_iopage shared_iopage_t;
> > >
> > > +struct union_shared_iopage {
> > > + union union_ioreq vcpu_ioreq[1];
> > > +};
> > > +typedef struct union_shared_iopage union_shared_iopage_t;
> >
> > I don't think either of these really need to be part of the public
> > interface.
>
> Do you prefer this union mechanism or the original code of this patch
> 1412274977-6098-2-git-send-email-dslutz@verizon.com?
>
> I haven't see the QEMU code for the union approach, but both
> alternatives would probably be fine from the QEMU POV.
>
Really? To me both seem equally horrible and unnecessary.
Paul
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-06 9:39 ` Jan Beulich
@ 2014-10-06 19:51 ` Don Slutz
2014-10-07 8:05 ` Jan Beulich
0 siblings, 1 reply; 25+ messages in thread
From: Don Slutz @ 2014-10-06 19:51 UTC (permalink / raw)
To: Jan Beulich, Stefano Stabellini
Cc: Andrew Cooper, Keir Fraser, Ian Campbell, Don Slutz, xen-devel
On 10/06/14 05:39, Jan Beulich wrote:
>>>> On 06.10.14 at 11:21, <stefano.stabellini@eu.citrix.com> wrote:
>> On Mon, 6 Oct 2014, Jan Beulich wrote:
>>>>>> On 03.10.14 at 21:27, <dslutz@verizon.com> wrote:
>>>> --- a/xen/include/public/hvm/ioreq.h
>>>> +++ b/xen/include/public/hvm/ioreq.h
>>>> @@ -85,11 +85,22 @@ struct vmware_ioreq {
>>>> };
>>>> typedef struct vmware_ioreq vmware_ioreq_t;
>>>>
>>>> +union union_ioreq {
>>>> + ioreq_t oreq;
>>>> + vmware_ioreq_t vreq;
>>>> +};
>>>> +typedef union union_ioreq union_ioreq_t;
>>>> +
>>>> struct shared_iopage {
>>>> struct ioreq vcpu_ioreq[1];
>>>> };
>>>> typedef struct shared_iopage shared_iopage_t;
>>>>
>>>> +struct union_shared_iopage {
>>>> + union union_ioreq vcpu_ioreq[1];
>>>> +};
>>>> +typedef struct union_shared_iopage union_shared_iopage_t;
>>> I don't think either of these really need to be part of the public
>>> interface.
>> Do you prefer this union mechanism or the original code of this patch
>> 1412274977-6098-2-git-send-email-dslutz@verizon.com?
>>
>> I haven't see the QEMU code for the union approach, but both
>> alternatives would probably be fine from the QEMU POV.
Adding this union mechanism in QEMU would be making it part
of the public interface. So I have not yet coded it up.
> I'm fine with the union approach (subject to convincing myself that
> all this effort and new code is really worth it in the first place, as
> said elsewhere before), just not as part of the public interface
> (unless there is a clear need for it to be there).
I am read this as yes, add the union_ioreq_t to Xen but not to QEMU.
I know of at least 1 custom kernel that requires the VMware "rpc" code.
So I think that vmware_port support is just like viridian support. It allows
more guests to be run under Xen.
With the statement "move VMware rpc to QEMU", this patch was needed.
Hope, this helps.
-Don Slutz
> Jan
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-06 19:51 ` Don Slutz
@ 2014-10-07 8:05 ` Jan Beulich
0 siblings, 0 replies; 25+ messages in thread
From: Jan Beulich @ 2014-10-07 8:05 UTC (permalink / raw)
To: Don Slutz
Cc: Andrew Cooper, xen-devel, Keir Fraser, Ian Campbell, Stefano Stabellini
>>> On 06.10.14 at 21:51, <dslutz@verizon.com> wrote:
> On 10/06/14 05:39, Jan Beulich wrote:
>> I'm fine with the union approach (subject to convincing myself that
>> all this effort and new code is really worth it in the first place, as
>> said elsewhere before), just not as part of the public interface
>> (unless there is a clear need for it to be there).
>
> I am read this as yes, add the union_ioreq_t to Xen but not to QEMU.
Not exactly: Since the union approach is mechanism, not interface,
both Xen and any consumer of the public interface can choose to
follow this route or use (fragile) casts.
> I know of at least 1 custom kernel that requires the VMware "rpc" code.
> So I think that vmware_port support is just like viridian support. It allows
> more guests to be run under Xen.
Not exactly: You're adding much more code than Viridian support
has accumulated over several years, and quite a bit of that based
on observation rather than a publicly available spec.
Jan
^ permalink raw reply [flat|nested] 25+ messages in thread
* [RFC][PATCH v2x prototype 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-03 19:56 ` [Qemu-devel] [Xen-devel] " Don Slutz
@ 2014-10-09 14:26 ` Don Slutz
2014-10-13 13:26 ` Paul Durrant
0 siblings, 1 reply; 25+ messages in thread
From: Don Slutz @ 2014-10-09 14:26 UTC (permalink / raw)
To: xen-devel, Paul Durrant; +Cc: Don Slutz, Keir Fraser, Ian Campbell, Jan Beulich
This adds synchronisation of the 6 vcpu registers (only 32bits of
them) that vmport.c needs between Xen and QEMU.
This is to avoid a 2nd and 3rd exchange between QEMU and Xen to
fetch and put these 6 vcpu registers used by the code in vmport.c
and vmmouse.c
QEMU patch is named "xen-hvm.c: Add support for Xen access to vmport"
Signed-off-by: Don Slutz <dslutz@verizon.com>
---
As requested by Paul Durrant <Paul.Durrant@citrix.com>
Here is a prototype of the QEMU change using a 2nd shared page.
I picked adding HVM_PARAM_VMPORT_IOREQ_PFN as the simple and
fast way to handle QEMU building on older Xen versions.
There is xentrace and debug logging that is TBD for the Xen 4.6
submission of this.
tools/libxc/xc_hvm_build_x86.c | 5 ++-
tools/xentrace/formats | 1 +
xen/arch/x86/hvm/emulate.c | 62 ++++++++++++++++++++++----
xen/arch/x86/hvm/hvm.c | 81 ++++++++++++++++++++++++++++------
xen/arch/x86/hvm/io.c | 40 ++++++++++++++++-
xen/arch/x86/hvm/svm/svm.c | 21 +++++++--
xen/arch/x86/hvm/vmware/vmport.c | 30 ++++++++++++-
xen/arch/x86/hvm/vmx/vmx.c | 23 ++++++++--
xen/arch/x86/x86_emulate/x86_emulate.h | 2 +
xen/include/asm-x86/hvm/domain.h | 1 +
xen/include/asm-x86/hvm/emulate.h | 3 ++
xen/include/asm-x86/hvm/hvm.h | 2 +-
xen/include/asm-x86/hvm/io.h | 2 +-
xen/include/asm-x86/hvm/trace.h | 1 +
xen/include/asm-x86/hvm/vcpu.h | 1 +
xen/include/public/hvm/ioreq.h | 17 +++++++
xen/include/public/hvm/params.h | 4 +-
xen/include/public/trace.h | 1 +
18 files changed, 261 insertions(+), 36 deletions(-)
diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
index c81a25b..e45fa62 100644
--- a/tools/libxc/xc_hvm_build_x86.c
+++ b/tools/libxc/xc_hvm_build_x86.c
@@ -46,7 +46,8 @@
#define SPECIALPAGE_IOREQ 5
#define SPECIALPAGE_IDENT_PT 6
#define SPECIALPAGE_CONSOLE 7
-#define NR_SPECIAL_PAGES 8
+#define SPECIALPAGE_VMPORT_IOREQ 8
+#define NR_SPECIAL_PAGES 9
#define special_pfn(x) (0xff000u - NR_SPECIAL_PAGES + (x))
#define NR_IOREQ_SERVER_PAGES 8
@@ -493,6 +494,8 @@ static int setup_guest(xc_interface *xch,
special_pfn(SPECIALPAGE_BUFIOREQ));
xc_hvm_param_set(xch, dom, HVM_PARAM_IOREQ_PFN,
special_pfn(SPECIALPAGE_IOREQ));
+ xc_hvm_param_set(xch, dom, HVM_PARAM_VMPORT_IOREQ_PFN,
+ special_pfn(SPECIALPAGE_VMPORT_IOREQ));
xc_hvm_param_set(xch, dom, HVM_PARAM_CONSOLE_PFN,
special_pfn(SPECIALPAGE_CONSOLE));
xc_hvm_param_set(xch, dom, HVM_PARAM_PAGING_RING_PFN,
diff --git a/tools/xentrace/formats b/tools/xentrace/formats
index 7b21b22..26e128e 100644
--- a/tools/xentrace/formats
+++ b/tools/xentrace/formats
@@ -92,6 +92,7 @@
0x0008202a CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_BAD [ dir = %(1)d bytes = %(2)d eax = 0x%(3)08x eip = 0x%(4)08x ]
0x0008212a CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_BAD [ dir = %(1)d bytes = %(2)d eax = 0x%(3)08x rip = 0x%(5)08x%(4)08x ]
0x0008202b CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_UNKNOWN [ bytes << 8 + dir = 0x%(1)03x cmd = 0x%(2)x cmd = %(2)d ebx = 0x%(3)08x ecx = 0x%(4)08x esi = 0x%(5)08x edi = 0x%(6)08x ]
+0x0008202c CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_QEMU [ eax = 0x%(1)08x ebx = 0x%(2)08x ecx = 0x%(3)08x edx = 0x%(4)08x esi = 0x%(5)08x edi = 0x%(6)08x ]
0x0010f001 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) page_grant_map [ domid = %(1)d ]
0x0010f002 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) page_grant_unmap [ domid = %(1)d ]
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index c0f47d2..4b8ea8f 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -52,12 +52,14 @@ static void hvmtrace_io_assist(int is_mmio, ioreq_t *p)
static int hvmemul_do_io(
int is_mmio, paddr_t addr, unsigned long *reps, int size,
- paddr_t ram_gpa, int dir, int df, void *p_data)
+ paddr_t ram_gpa, int dir, int df, void *p_data,
+ struct cpu_user_regs *regs)
{
struct vcpu *curr = current;
struct hvm_vcpu_io *vio;
ioreq_t p = {
- .type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
+ .type = regs ? IOREQ_TYPE_VMWARE_PORT :
+ is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
.addr = addr,
.size = size,
.dir = dir,
@@ -65,11 +67,15 @@ static int hvmemul_do_io(
.data = ram_gpa,
.data_is_ptr = (p_data == NULL),
};
+ vmware_ioreq_t vp;
+ vmware_ioreq_t *vpp;
unsigned long ram_gfn = paddr_to_pfn(ram_gpa);
p2m_type_t p2mt;
struct page_info *ram_page;
int rc;
+ BUILD_BUG_ON(sizeof(ioreq_t) < sizeof(vmware_ioreq_t));
+
/* Check for paged out page */
ram_page = get_page_from_gfn(curr->domain, ram_gfn, &p2mt, P2M_UNSHARE);
if ( p2m_is_paging(p2mt) )
@@ -101,7 +107,17 @@ static int hvmemul_do_io(
return X86EMUL_UNHANDLEABLE;
}
- if ( !p.data_is_ptr && (dir == IOREQ_WRITE) )
+ if ( regs )
+ {
+ vpp = &vp;
+ p.data = regs->rax;
+ vp.ebx = regs->rbx;
+ vp.ecx = regs->rcx;
+ vp.edx = regs->rdx;
+ vp.esi = regs->rsi;
+ vp.edi = regs->rdi;
+ }
+ else if ( !p.data_is_ptr && (dir == IOREQ_WRITE) )
{
memcpy(&p.data, p_data, size);
p_data = NULL;
@@ -161,7 +177,19 @@ static int hvmemul_do_io(
put_page(ram_page);
return X86EMUL_RETRY;
}
+ case HVMIO_awaiting_completion:
+ if ( regs )
+ {
+ /* May have to wait for previous cycle of a multi-write to complete. */
+ if ( vio->mmio_retry ) {
+ gdprintk(XENLOG_WARNING, "WARNING: mmio_retry io_state=%d?\n", vio->io_state);
+ return X86EMUL_RETRY;
+ }
+ /* These are expected if we get here via hvmemul_do_io() */
+ break;
+ }
default:
+ gdprintk(XENLOG_WARNING, "WARNING: io_state=%d?\n", vio->io_state);
if ( ram_page )
put_page(ram_page);
return X86EMUL_UNHANDLEABLE;
@@ -175,7 +203,7 @@ static int hvmemul_do_io(
return X86EMUL_UNHANDLEABLE;
}
- vio->io_state =
+ vio->io_state = regs ? HVMIO_handle_vmport_awaiting_completion :
(p_data == NULL) ? HVMIO_dispatched : HVMIO_awaiting_completion;
vio->io_size = size;
@@ -197,6 +225,9 @@ static int hvmemul_do_io(
if ( rc == X86EMUL_UNHANDLEABLE )
rc = hvm_buffered_io_intercept(&p);
}
+ else if ( regs ) {
+ rc = X86EMUL_UNHANDLEABLE;
+ }
else
{
rc = hvm_portio_intercept(&p);
@@ -210,7 +241,7 @@ static int hvmemul_do_io(
p.state = STATE_IORESP_READY;
if ( !vio->mmio_retry )
{
- hvm_io_assist(&p);
+ hvm_io_assist(&p, vpp);
vio->io_state = HVMIO_none;
}
else
@@ -226,13 +257,19 @@ static int hvmemul_do_io(
}
else
{
- rc = X86EMUL_RETRY;
- if ( !hvm_send_assist_req(&p) )
+ if ( regs )
+ rc = X86EMUL_VMPORT_RETRY;
+ else
+ rc = X86EMUL_RETRY;
+ if ( !hvm_send_assist_req(&p, vpp) )
vio->io_state = HVMIO_none;
else if ( p_data == NULL )
rc = X86EMUL_OKAY;
}
break;
+ case X86EMUL_VMPORT_RETRY:
+ rc = X86EMUL_RETRY;
+ break;
default:
BUG();
}
@@ -287,14 +324,21 @@ int hvmemul_do_pio(
unsigned long port, unsigned long *reps, int size,
paddr_t ram_gpa, int dir, int df, void *p_data)
{
- return hvmemul_do_io(0, port, reps, size, ram_gpa, dir, df, p_data);
+ return hvmemul_do_io(0, port, reps, size, ram_gpa, dir, df, p_data, NULL);
+}
+
+int hvmemul_do_vmport(
+ unsigned long port, unsigned long *reps, int size,
+ int dir, void *p_data, struct cpu_user_regs *regs)
+{
+ return hvmemul_do_io(0, port, reps, size, 0, dir, 0, p_data, regs);
}
static int hvmemul_do_mmio(
paddr_t gpa, unsigned long *reps, int size,
paddr_t ram_gpa, int dir, int df, void *p_data)
{
- return hvmemul_do_io(1, gpa, reps, size, ram_gpa, dir, df, p_data);
+ return hvmemul_do_io(1, gpa, reps, size, ram_gpa, dir, df, p_data, NULL);
}
/*
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 8d0a3a0..fd05a85 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -382,6 +382,16 @@ static ioreq_t *get_ioreq(struct hvm_ioreq_server *s, struct vcpu *v)
return &p->vcpu_ioreq[v->vcpu_id];
}
+static vmware_ioreq_t *get_vmport_ioreq(struct hvm_ioreq_server *s, struct vcpu *v)
+{
+ shared_vmport_iopage_t *p = s->vmport_ioreq.va;
+
+ ASSERT((v == current) || !vcpu_runnable(v));
+ ASSERT(p != NULL);
+
+ return &p->vcpu_vmport_ioreq[v->vcpu_id];
+}
+
bool_t hvm_io_pending(struct vcpu *v)
{
struct domain *d = v->domain;
@@ -400,7 +410,8 @@ bool_t hvm_io_pending(struct vcpu *v)
return 0;
}
-static bool_t hvm_wait_for_io(struct hvm_ioreq_vcpu *sv, ioreq_t *p)
+static bool_t hvm_wait_for_io(struct hvm_ioreq_vcpu *sv, ioreq_t *p,
+ vmware_ioreq_t *vp)
{
/* NB. Optimised for common case (p->state == STATE_IOREQ_NONE). */
while ( p->state != STATE_IOREQ_NONE )
@@ -409,7 +420,7 @@ static bool_t hvm_wait_for_io(struct hvm_ioreq_vcpu *sv, ioreq_t *p)
{
case STATE_IORESP_READY: /* IORESP_READY -> NONE */
rmb(); /* see IORESP_READY /then/ read contents of ioreq */
- hvm_io_assist(p);
+ hvm_io_assist(p, vp);
break;
case STATE_IOREQ_READY: /* IOREQ_{READY,INPROCESS} -> IORESP_READY */
case STATE_IOREQ_INPROCESS:
@@ -449,7 +460,8 @@ void hvm_do_resume(struct vcpu *v)
{
if ( sv->vcpu == v )
{
- if ( !hvm_wait_for_io(sv, get_ioreq(s, v)) )
+ if ( !hvm_wait_for_io(sv, get_ioreq(s, v),
+ get_vmport_ioreq(s, v)) )
return;
break;
@@ -491,22 +503,50 @@ static void hvm_free_ioreq_gmfn(struct domain *d, unsigned long gmfn)
clear_bit(i, &d->arch.hvm_domain.ioreq_gmfn.mask);
}
-static void hvm_unmap_ioreq_page(struct hvm_ioreq_server *s, bool_t buf)
+static void hvm_unmap_ioreq_page(struct hvm_ioreq_server *s, int buf)
{
- struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
+ struct hvm_ioreq_page *iorp = NULL;
+
+ switch ( buf )
+ {
+ case 0:
+ iorp = &s->ioreq;
+ break;
+ case 1:
+ iorp = &s->bufioreq;
+ break;
+ case 2:
+ iorp = &s->vmport_ioreq;
+ break;
+ }
+ ASSERT(iorp);
destroy_ring_for_helper(&iorp->va, iorp->page);
}
static int hvm_map_ioreq_page(
- struct hvm_ioreq_server *s, bool_t buf, unsigned long gmfn)
+ struct hvm_ioreq_server *s, int buf, unsigned long gmfn)
{
struct domain *d = s->domain;
- struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
+ struct hvm_ioreq_page *iorp = NULL;
struct page_info *page;
void *va;
int rc;
+ switch ( buf )
+ {
+ case 0:
+ iorp = &s->ioreq;
+ break;
+ case 1:
+ iorp = &s->bufioreq;
+ break;
+ case 2:
+ iorp = &s->vmport_ioreq;
+ break;
+ }
+ ASSERT(iorp);
+
if ( (rc = prepare_ring_for_helper(d, gmfn, &page, &va)) )
return rc;
@@ -717,7 +757,7 @@ static int hvm_ioreq_server_map_pages(struct hvm_ioreq_server *s,
bool_t is_default, bool_t handle_bufioreq)
{
struct domain *d = s->domain;
- unsigned long ioreq_pfn, bufioreq_pfn;
+ unsigned long ioreq_pfn, bufioreq_pfn, vmport_ioreq_pfn = 0;
int rc;
if ( is_default )
@@ -730,6 +770,7 @@ static int hvm_ioreq_server_map_pages(struct hvm_ioreq_server *s,
*/
ASSERT(handle_bufioreq);
bufioreq_pfn = d->arch.hvm_domain.params[HVM_PARAM_BUFIOREQ_PFN];
+ vmport_ioreq_pfn = d->arch.hvm_domain.params[HVM_PARAM_VMPORT_IOREQ_PFN];
}
else
{
@@ -754,10 +795,16 @@ static int hvm_ioreq_server_map_pages(struct hvm_ioreq_server *s,
rc = hvm_map_ioreq_page(s, 1, bufioreq_pfn);
if ( rc )
goto fail4;
+ rc = hvm_map_ioreq_page(s, 2, vmport_ioreq_pfn);
+ if ( rc )
+ goto fail5;
}
return 0;
+fail5:
+ hvm_unmap_ioreq_page(s, 2);
+
fail4:
hvm_unmap_ioreq_page(s, 0);
@@ -2510,7 +2557,8 @@ bool_t hvm_has_dm(struct domain *d)
}
bool_t hvm_send_assist_req_to_ioreq_server(struct hvm_ioreq_server *s,
- ioreq_t *proto_p)
+ ioreq_t *proto_p,
+ vmware_ioreq_t *proto_vp)
{
struct vcpu *curr = current;
struct domain *d = curr->domain;
@@ -2544,6 +2592,12 @@ bool_t hvm_send_assist_req_to_ioreq_server(struct hvm_ioreq_server *s,
goto crash;
}
+ if ( proto_vp )
+ {
+ vmware_ioreq_t *vp = get_vmport_ioreq(s, curr);
+
+ *vp = *proto_vp;
+ }
proto_p->state = STATE_IOREQ_NONE;
proto_p->vp_eport = port;
*p = *proto_p;
@@ -2591,21 +2645,21 @@ static bool_t hvm_complete_assist_req(ioreq_t *p)
/* FALLTHRU */
default:
p->state = STATE_IORESP_READY;
- hvm_io_assist(p);
+ hvm_io_assist(p, NULL);
break;
}
return 1;
}
-bool_t hvm_send_assist_req(ioreq_t *p)
+bool_t hvm_send_assist_req(ioreq_t *p, vmware_ioreq_t *vp)
{
struct hvm_ioreq_server *s = hvm_select_ioreq_server(current->domain, p);
if ( !s )
return hvm_complete_assist_req(p);
- return hvm_send_assist_req_to_ioreq_server(s, p);
+ return hvm_send_assist_req_to_ioreq_server(s, p, vp);
}
void hvm_broadcast_assist_req(ioreq_t *p)
@@ -2618,7 +2672,7 @@ void hvm_broadcast_assist_req(ioreq_t *p)
list_for_each_entry ( s,
&d->arch.hvm_domain.ioreq_server.list,
list_entry )
- (void) hvm_send_assist_req_to_ioreq_server(s, p);
+ (void) hvm_send_assist_req_to_ioreq_server(s, p, NULL);
}
void hvm_hlt(unsigned long rflags)
@@ -5763,6 +5817,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
break;
}
case HVM_PARAM_IOREQ_PFN:
+ case HVM_PARAM_VMPORT_IOREQ_PFN:
case HVM_PARAM_BUFIOREQ_PFN:
case HVM_PARAM_BUFIOREQ_EVTCHN: {
domid_t domid;
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index 68fb890..eb09032 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -154,8 +154,17 @@ int handle_pio(uint16_t port, unsigned int size, int dir)
}
break;
case X86EMUL_RETRY:
- if ( vio->io_state != HVMIO_awaiting_completion )
+ if ( vio->io_state == HVMIO_handle_vmport_awaiting_completion ) {
+ /* Completion in hvm_io_assist() with no re-emulation required. */
+#ifdef VMPORT_IO_LOGGING
+ gdprintk(XENLOG_WARNING, "_vmport_awaiting\n");
+#endif
+ return 1;
+ }
+ if ( vio->io_state != HVMIO_awaiting_completion ) {
+ gdprintk(XENLOG_WARNING, "WARNING: io_state=%d?\n", vio->io_state);
return 0;
+ }
/* Completion in hvm_io_assist() with no re-emulation required. */
ASSERT(dir == IOREQ_READ);
vio->io_state = HVMIO_handle_pio_awaiting_completion;
@@ -169,7 +178,7 @@ int handle_pio(uint16_t port, unsigned int size, int dir)
return 1;
}
-void hvm_io_assist(ioreq_t *p)
+void hvm_io_assist(ioreq_t *p, vmware_ioreq_t *vp)
{
struct vcpu *curr = current;
struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
@@ -197,6 +206,33 @@ void hvm_io_assist(ioreq_t *p)
else
memcpy(&guest_cpu_user_regs()->rax, &p->data, vio->io_size);
break;
+ case HVMIO_handle_vmport_awaiting_completion:
+ {
+ struct cpu_user_regs *regs = guest_cpu_user_regs();
+
+ ASSERT(vp);
+#ifdef VMPORT_IO_LOGGING
+ gdprintk(XENLOG_WARNING, "vmport done ip=0x%lx\n",
+ (long)regs->rip);
+#endif
+ if ( p->dir == IOREQ_READ)
+ {
+ if ( vio->io_size == 4 ) /* Needs zero extension. */
+ regs->rax = (uint32_t)p->data;
+ else
+ memcpy(®s->rax, &p->data, vio->io_size);
+ }
+ /* Only change the 32bit part of the register */
+ regs->_ebx = vp->ebx;
+ regs->_ecx = vp->ecx;
+ regs->_edx = vp->edx;
+ regs->_esi = vp->esi;
+ regs->_edi = vp->edi;
+ HVMTRACE_ND(VMPORT_QEMU, 0, 1/*cycles*/, 6,
+ regs->rax, regs->rbx, regs->rcx,
+ regs->rdx, regs->rsi, regs->rdi);
+ }
+ break;
default:
break;
}
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 8b1185e..ae21356 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2140,10 +2140,24 @@ static void svm_vmexit_gp_intercept(struct cpu_user_regs *regs,
HVMTRACE_C4D(TRAP_GP, inst_len, starting_rdx, vmcb->exitinfo1,
vmcb->exitinfo2);
- if ( !rc )
- __update_guest_eip(regs, inst_len);
- else
+ switch ( rc )
{
+ case X86EMUL_VMPORT_RETRY:
+ rc = X86EMUL_RETRY;
+ /* fall through */
+ case X86EMUL_RETRY:
+ {
+ struct hvm_vcpu_io *vio = &v->arch.hvm_vcpu.hvm_io;
+ if ( vio->io_state != HVMIO_handle_vmport_awaiting_completion ) {
+ gdprintk(XENLOG_WARNING, "WARNING: io_state=%d?\n", vio->io_state);
+ break;
+ }
+ }
+ /* fall through */
+ case X86EMUL_OKAY:
+ __update_guest_eip(regs, inst_len);
+ break;
+ default:
VMPORT_DBG_LOG(VMPORT_LOG_GP_UNKNOWN,
"gp: rc=%d ei1=0x%lx ei2=0x%lx ec=0x%x ip=%"PRIx64
" (0x%lx,%ld) ax=%"PRIx64" bx=%"PRIx64" cx=%"PRIx64
@@ -2159,6 +2173,7 @@ static void svm_vmexit_gp_intercept(struct cpu_user_regs *regs,
HVMTRACE_C5D(TRAP_GP_UNKNOWN, rc, regs->rax, regs->rbx, regs->rcx,
inst_addr);
hvm_inject_hw_exception(TRAP_gp_fault, vmcb->exitinfo1);
+ break;
}
}
diff --git a/xen/arch/x86/hvm/vmware/vmport.c b/xen/arch/x86/hvm/vmware/vmport.c
index 962ee32..d1632bb 100644
--- a/xen/arch/x86/hvm/vmware/vmport.c
+++ b/xen/arch/x86/hvm/vmware/vmport.c
@@ -147,9 +147,34 @@ int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val)
regs->rax = 0x0;
break;
default:
+ { /* Let backing DM handle */
+ unsigned long data, reps = 1;
+
HVMTRACE_ND(VMPORT_UNKNOWN, 0, 1/*cycles*/, 6,
- (bytes << 8) + dir, cmd, regs->rbx,
+ (bytes << 8) | dir, cmd, regs->rbx,
regs->rcx, regs->rsi, regs->rdi);
+ rc = hvmemul_do_vmport(BDOOR_PORT, &reps, bytes, dir, &data, regs);
+ switch (rc)
+ {
+ case X86EMUL_OKAY:
+ break;
+ case X86EMUL_VMPORT_RETRY:
+ case X86EMUL_RETRY:
+ {
+ struct hvm_vcpu_io *vio = ¤t->arch.hvm_vcpu.hvm_io;
+
+ if ( vio->io_state != HVMIO_handle_vmport_awaiting_completion )
+ gdprintk(XENLOG_WARNING, "vio: io_state=%d ==> %d\n",
+ vio->io_state, rc);
+ return rc;
+ break;
+ }
+ default:
+ gdprintk(XENLOG_ERR, "Weird HVM ioemulation status %d.\n", rc);
+ domain_crash(current->domain);
+ break;
+ }
+ }
break;
}
@@ -190,6 +215,9 @@ int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val)
rc = X86EMUL_UNHANDLEABLE;
}
+#ifdef VMPORT_IO_LOGGING
+ gdprintk(XENLOG_WARNING, "vmport: rc=%d\n", rc);
+#endif
return rc;
}
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index c84894a..dd079bd 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2643,10 +2643,24 @@ static void vmx_vmexit_gp_intercept(struct cpu_user_regs *regs,
"Unexpected instruction length difference: %lu vs %lu\n",
orig_inst_len, inst_len);
#endif
- if ( !rc )
- update_guest_eip();
- else
+ switch ( rc )
{
+ case X86EMUL_VMPORT_RETRY:
+ rc = X86EMUL_RETRY;
+ /* fall through */
+ case X86EMUL_RETRY:
+ {
+ struct hvm_vcpu_io *vio = &v->arch.hvm_vcpu.hvm_io;
+ if ( vio->io_state != HVMIO_handle_vmport_awaiting_completion ) {
+ gdprintk(XENLOG_WARNING, "WARNING: io_state=%d?\n", vio->io_state);
+ break;
+ }
+ }
+ /* fall through */
+ case X86EMUL_OKAY:
+ update_guest_eip();
+ break;
+ default:
VMPORT_DBG_LOG(VMPORT_LOG_GP_UNKNOWN,
"gp: rc=%d ecode=0x%lx eq=0x%lx ec=0x%x ip=%"PRIx64
" (0x%lx,%lu=>%lu) ax=%"PRIx64" bx=%"PRIx64
@@ -2661,7 +2675,8 @@ static void vmx_vmexit_gp_intercept(struct cpu_user_regs *regs,
else
HVMTRACE_C5D(TRAP_GP_UNKNOWN, rc, regs->rax, regs->rbx, regs->rcx,
inst_addr);
- hvm_inject_hw_exception(TRAP_gp_fault, ecode);
+ hvm_inject_hw_exception(TRAP_gp_fault, ecode);
+ break;
}
}
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index b059341..1733eca 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -110,6 +110,8 @@ struct __packed segment_register {
#define X86EMUL_RETRY 3
/* (cmpxchg accessor): CMPXCHG failed. Maps to X86EMUL_RETRY in caller. */
#define X86EMUL_CMPXCHG_FAILED 3
+ /* Like X86EMUL_RETRY, but do not change vio->io_state. */
+#define X86EMUL_VMPORT_RETRY 4
/* FPU sub-types which may be requested via ->get_fpu(). */
enum x86_emulate_fpu_type {
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index d4718df..e7e6cd9 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -63,6 +63,7 @@ struct hvm_ioreq_server {
ioservid_t id;
struct hvm_ioreq_page ioreq;
struct list_head ioreq_vcpu_list;
+ struct hvm_ioreq_page vmport_ioreq;
struct hvm_ioreq_page bufioreq;
/* Lock to serialize access to buffered ioreq ring */
diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm-x86/hvm/emulate.h
index 5411302..6c3ff2a 100644
--- a/xen/include/asm-x86/hvm/emulate.h
+++ b/xen/include/asm-x86/hvm/emulate.h
@@ -53,6 +53,9 @@ struct segment_register *hvmemul_get_seg_reg(
int hvmemul_do_pio(
unsigned long port, unsigned long *reps, int size,
paddr_t ram_gpa, int dir, int df, void *p_data);
+int hvmemul_do_vmport(
+ unsigned long port, unsigned long *reps, int size,
+ int dir, void *p_data, struct cpu_user_regs *regs);
void hvm_dump_emulation_state(const char *prefix,
struct hvm_emulate_ctxt *hvmemul_ctxt);
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 0910147..b57c2d7 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -228,7 +228,7 @@ int hvm_vcpu_cacheattr_init(struct vcpu *v);
void hvm_vcpu_cacheattr_destroy(struct vcpu *v);
void hvm_vcpu_reset_state(struct vcpu *v, uint16_t cs, uint16_t ip);
-bool_t hvm_send_assist_req(ioreq_t *p);
+bool_t hvm_send_assist_req(ioreq_t *p, vmware_ioreq_t *vp);
void hvm_broadcast_assist_req(ioreq_t *p);
void hvm_get_guest_pat(struct vcpu *v, u64 *guest_pat);
diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h
index d257161..b100fea 100644
--- a/xen/include/asm-x86/hvm/io.h
+++ b/xen/include/asm-x86/hvm/io.h
@@ -123,7 +123,7 @@ int handle_mmio_with_translation(unsigned long gva, unsigned long gpfn,
struct npfec);
int handle_pio(uint16_t port, unsigned int size, int dir);
void hvm_interrupt_post(struct vcpu *v, int vector, int type);
-void hvm_io_assist(ioreq_t *p);
+void hvm_io_assist(ioreq_t *p, vmware_ioreq_t *vp);
void hvm_dpci_eoi(struct domain *d, unsigned int guest_irq,
const union vioapic_redir_entry *ent);
void msix_write_completion(struct vcpu *);
diff --git a/xen/include/asm-x86/hvm/trace.h b/xen/include/asm-x86/hvm/trace.h
index 8af2d6a..10d78bb 100644
--- a/xen/include/asm-x86/hvm/trace.h
+++ b/xen/include/asm-x86/hvm/trace.h
@@ -66,6 +66,7 @@
#define DO_TRC_HVM_VMPORT_BAD DEFAULT_HVM_IO
#define DO_TRC_HVM_VMPORT_BAD64 DEFAULT_HVM_IO
#define DO_TRC_HVM_VMPORT_UNKNOWN DEFAULT_HVM_IO
+#define DO_TRC_HVM_VMPORT_QEMU DEFAULT_HVM_IO
#define TRC_PAR_LONG(par) ((par)&0xFFFFFFFF),((par)>>32)
diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
index 01e0665..1e63d7f 100644
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -36,6 +36,7 @@ enum hvm_io_state {
HVMIO_awaiting_completion,
HVMIO_handle_mmio_awaiting_completion,
HVMIO_handle_pio_awaiting_completion,
+ HVMIO_handle_vmport_awaiting_completion,
HVMIO_completed
};
diff --git a/xen/include/public/hvm/ioreq.h b/xen/include/public/hvm/ioreq.h
index 5b5fedf..c20b834 100644
--- a/xen/include/public/hvm/ioreq.h
+++ b/xen/include/public/hvm/ioreq.h
@@ -35,6 +35,7 @@
#define IOREQ_TYPE_PIO 0 /* pio */
#define IOREQ_TYPE_COPY 1 /* mmio ops */
#define IOREQ_TYPE_PCI_CONFIG 2
+#define IOREQ_TYPE_VMWARE_PORT 3
#define IOREQ_TYPE_TIMEOFFSET 7
#define IOREQ_TYPE_INVALIDATE 8 /* mapcache */
@@ -48,6 +49,8 @@
*
* 63....48|47..40|39..35|34..32|31........0
* SEGMENT |BUS |DEV |FN |OFFSET
+ *
+ * For I/O type IOREQ_TYPE_VMWARE_PORT also use the vmware_ioreq.
*/
struct ioreq {
uint64_t addr; /* physical address */
@@ -66,11 +69,25 @@ struct ioreq {
};
typedef struct ioreq ioreq_t;
+struct vmware_ioreq {
+ uint32_t esi;
+ uint32_t edi;
+ uint32_t ebx;
+ uint32_t ecx;
+ uint32_t edx;
+};
+typedef struct vmware_ioreq vmware_ioreq_t;
+
struct shared_iopage {
struct ioreq vcpu_ioreq[1];
};
typedef struct shared_iopage shared_iopage_t;
+struct shared_vmport_iopage {
+ struct vmware_ioreq vcpu_vmport_ioreq[1];
+};
+typedef struct shared_vmport_iopage shared_vmport_iopage_t;
+
struct buf_ioreq {
uint8_t type; /* I/O type */
uint8_t pad:1;
diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index c893dc5..2d75bdd 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -50,6 +50,8 @@
#define HVM_PARAM_PAE_ENABLED 4
#define HVM_PARAM_IOREQ_PFN 5
+/* Extra vmport PFN. */
+#define HVM_PARAM_VMPORT_IOREQ_PFN 36
#define HVM_PARAM_BUFIOREQ_PFN 6
#define HVM_PARAM_BUFIOREQ_EVTCHN 26
@@ -192,6 +194,6 @@
/* Params for VMware */
#define HVM_PARAM_VMWARE_HW 35
-#define HVM_NR_PARAMS 36
+#define HVM_NR_PARAMS 37
#endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
diff --git a/xen/include/public/trace.h b/xen/include/public/trace.h
index b231df3..e35a297 100644
--- a/xen/include/public/trace.h
+++ b/xen/include/public/trace.h
@@ -237,6 +237,7 @@
#define TRC_HVM_VMPORT_BAD (TRC_HVM_HANDLER + 0x2a)
#define TRC_HVM_VMPORT_BAD64 (TRC_HVM_HANDLER + TRC_64_FLAG + 0x2a)
#define TRC_HVM_VMPORT_UNKNOWN (TRC_HVM_HANDLER + 0x2b)
+#define TRC_HVM_VMPORT_QEMU (TRC_HVM_HANDLER + 0x2c)
#define TRC_HVM_IOPORT_WRITE (TRC_HVM_HANDLER + 0x216)
#define TRC_HVM_IOMEM_WRITE (TRC_HVM_HANDLER + 0x217)
--
1.8.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2x prototype 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-09 14:26 ` [RFC][PATCH v2x prototype " Don Slutz
@ 2014-10-13 13:26 ` Paul Durrant
2014-10-13 17:11 ` Don Slutz
0 siblings, 1 reply; 25+ messages in thread
From: Paul Durrant @ 2014-10-13 13:26 UTC (permalink / raw)
To: Don Slutz, xen-devel; +Cc: Keir (Xen.org), Ian Campbell, Jan Beulich
> -----Original Message-----
> From: Don Slutz [mailto:dslutz@verizon.com]
> Sent: 09 October 2014 15:26
> To: xen-devel@lists.xen.org; Paul Durrant
> Cc: Jan Beulich; Keir (Xen.org); Ian Campbell; Don Slutz
> Subject: [RFC][PATCH v2x prototype 1/1] Add IOREQ_TYPE_VMWARE_PORT
>
> This adds synchronisation of the 6 vcpu registers (only 32bits of
> them) that vmport.c needs between Xen and QEMU.
>
> This is to avoid a 2nd and 3rd exchange between QEMU and Xen to
> fetch and put these 6 vcpu registers used by the code in vmport.c
> and vmmouse.c
>
> QEMU patch is named "xen-hvm.c: Add support for Xen access to vmport"
>
> Signed-off-by: Don Slutz <dslutz@verizon.com>
> ---
> As requested by Paul Durrant <Paul.Durrant@citrix.com>
>
> Here is a prototype of the QEMU change using a 2nd shared page.
> I picked adding HVM_PARAM_VMPORT_IOREQ_PFN as the simple and
> fast way to handle QEMU building on older Xen versions.
>
> There is xentrace and debug logging that is TBD for the Xen 4.6
> submission of this.
>
> tools/libxc/xc_hvm_build_x86.c | 5 ++-
> tools/xentrace/formats | 1 +
> xen/arch/x86/hvm/emulate.c | 62 ++++++++++++++++++++++----
> xen/arch/x86/hvm/hvm.c | 81 ++++++++++++++++++++++++++++-
> -----
> xen/arch/x86/hvm/io.c | 40 ++++++++++++++++-
> xen/arch/x86/hvm/svm/svm.c | 21 +++++++--
> xen/arch/x86/hvm/vmware/vmport.c | 30 ++++++++++++-
> xen/arch/x86/hvm/vmx/vmx.c | 23 ++++++++--
> xen/arch/x86/x86_emulate/x86_emulate.h | 2 +
> xen/include/asm-x86/hvm/domain.h | 1 +
> xen/include/asm-x86/hvm/emulate.h | 3 ++
> xen/include/asm-x86/hvm/hvm.h | 2 +-
> xen/include/asm-x86/hvm/io.h | 2 +-
> xen/include/asm-x86/hvm/trace.h | 1 +
> xen/include/asm-x86/hvm/vcpu.h | 1 +
> xen/include/public/hvm/ioreq.h | 17 +++++++
> xen/include/public/hvm/params.h | 4 +-
> xen/include/public/trace.h | 1 +
> 18 files changed, 261 insertions(+), 36 deletions(-)
>
> diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
> index c81a25b..e45fa62 100644
> --- a/tools/libxc/xc_hvm_build_x86.c
> +++ b/tools/libxc/xc_hvm_build_x86.c
> @@ -46,7 +46,8 @@
> #define SPECIALPAGE_IOREQ 5
> #define SPECIALPAGE_IDENT_PT 6
> #define SPECIALPAGE_CONSOLE 7
> -#define NR_SPECIAL_PAGES 8
> +#define SPECIALPAGE_VMPORT_IOREQ 8
> +#define NR_SPECIAL_PAGES 9
> #define special_pfn(x) (0xff000u - NR_SPECIAL_PAGES + (x))
>
> #define NR_IOREQ_SERVER_PAGES 8
> @@ -493,6 +494,8 @@ static int setup_guest(xc_interface *xch,
> special_pfn(SPECIALPAGE_BUFIOREQ));
> xc_hvm_param_set(xch, dom, HVM_PARAM_IOREQ_PFN,
> special_pfn(SPECIALPAGE_IOREQ));
> + xc_hvm_param_set(xch, dom, HVM_PARAM_VMPORT_IOREQ_PFN,
> + special_pfn(SPECIALPAGE_VMPORT_IOREQ));
> xc_hvm_param_set(xch, dom, HVM_PARAM_CONSOLE_PFN,
> special_pfn(SPECIALPAGE_CONSOLE));
> xc_hvm_param_set(xch, dom, HVM_PARAM_PAGING_RING_PFN,
> diff --git a/tools/xentrace/formats b/tools/xentrace/formats
> index 7b21b22..26e128e 100644
> --- a/tools/xentrace/formats
> +++ b/tools/xentrace/formats
> @@ -92,6 +92,7 @@
> 0x0008202a CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_BAD [ dir = %(1)d
> bytes = %(2)d eax = 0x%(3)08x eip = 0x%(4)08x ]
> 0x0008212a CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_BAD [ dir = %(1)d
> bytes = %(2)d eax = 0x%(3)08x rip = 0x%(5)08x%(4)08x ]
> 0x0008202b CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_UNKNOWN [
> bytes << 8 + dir = 0x%(1)03x cmd = 0x%(2)x cmd = %(2)d ebx = 0x%(3)08x ecx
> = 0x%(4)08x esi = 0x%(5)08x edi = 0x%(6)08x ]
> +0x0008202c CPU%(cpu)d %(tsc)d (+%(reltsc)8d) VMPORT_QEMU [ eax =
> 0x%(1)08x ebx = 0x%(2)08x ecx = 0x%(3)08x edx = 0x%(4)08x esi = 0x%(5)08x
> edi = 0x%(6)08x ]
>
> 0x0010f001 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) page_grant_map [
> domid = %(1)d ]
> 0x0010f002 CPU%(cpu)d %(tsc)d (+%(reltsc)8d) page_grant_unmap [
> domid = %(1)d ]
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index c0f47d2..4b8ea8f 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -52,12 +52,14 @@ static void hvmtrace_io_assist(int is_mmio, ioreq_t
> *p)
>
> static int hvmemul_do_io(
> int is_mmio, paddr_t addr, unsigned long *reps, int size,
> - paddr_t ram_gpa, int dir, int df, void *p_data)
> + paddr_t ram_gpa, int dir, int df, void *p_data,
> + struct cpu_user_regs *regs)
> {
> struct vcpu *curr = current;
> struct hvm_vcpu_io *vio;
> ioreq_t p = {
> - .type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
> + .type = regs ? IOREQ_TYPE_VMWARE_PORT :
> + is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
> .addr = addr,
> .size = size,
> .dir = dir,
> @@ -65,11 +67,15 @@ static int hvmemul_do_io(
> .data = ram_gpa,
> .data_is_ptr = (p_data == NULL),
> };
> + vmware_ioreq_t vp;
> + vmware_ioreq_t *vpp;
> unsigned long ram_gfn = paddr_to_pfn(ram_gpa);
> p2m_type_t p2mt;
> struct page_info *ram_page;
> int rc;
>
> + BUILD_BUG_ON(sizeof(ioreq_t) < sizeof(vmware_ioreq_t));
> +
> /* Check for paged out page */
> ram_page = get_page_from_gfn(curr->domain, ram_gfn, &p2mt,
> P2M_UNSHARE);
> if ( p2m_is_paging(p2mt) )
> @@ -101,7 +107,17 @@ static int hvmemul_do_io(
> return X86EMUL_UNHANDLEABLE;
> }
>
> - if ( !p.data_is_ptr && (dir == IOREQ_WRITE) )
> + if ( regs )
> + {
> + vpp = &vp;
> + p.data = regs->rax;
> + vp.ebx = regs->rbx;
> + vp.ecx = regs->rcx;
> + vp.edx = regs->rdx;
> + vp.esi = regs->rsi;
> + vp.edi = regs->rdi;
> + }
> + else if ( !p.data_is_ptr && (dir == IOREQ_WRITE) )
> {
> memcpy(&p.data, p_data, size);
> p_data = NULL;
> @@ -161,7 +177,19 @@ static int hvmemul_do_io(
> put_page(ram_page);
> return X86EMUL_RETRY;
> }
> + case HVMIO_awaiting_completion:
> + if ( regs )
> + {
> + /* May have to wait for previous cycle of a multi-write to complete.
> */
> + if ( vio->mmio_retry ) {
> + gdprintk(XENLOG_WARNING, "WARNING: mmio_retry
> io_state=%d?\n", vio->io_state);
> + return X86EMUL_RETRY;
> + }
> + /* These are expected if we get here via hvmemul_do_io() */
> + break;
> + }
> default:
> + gdprintk(XENLOG_WARNING, "WARNING: io_state=%d?\n", vio-
> >io_state);
> if ( ram_page )
> put_page(ram_page);
> return X86EMUL_UNHANDLEABLE;
> @@ -175,7 +203,7 @@ static int hvmemul_do_io(
> return X86EMUL_UNHANDLEABLE;
> }
>
> - vio->io_state =
> + vio->io_state = regs ? HVMIO_handle_vmport_awaiting_completion :
> (p_data == NULL) ? HVMIO_dispatched : HVMIO_awaiting_completion;
> vio->io_size = size;
>
> @@ -197,6 +225,9 @@ static int hvmemul_do_io(
> if ( rc == X86EMUL_UNHANDLEABLE )
> rc = hvm_buffered_io_intercept(&p);
> }
> + else if ( regs ) {
> + rc = X86EMUL_UNHANDLEABLE;
> + }
> else
> {
> rc = hvm_portio_intercept(&p);
> @@ -210,7 +241,7 @@ static int hvmemul_do_io(
> p.state = STATE_IORESP_READY;
> if ( !vio->mmio_retry )
> {
> - hvm_io_assist(&p);
> + hvm_io_assist(&p, vpp);
> vio->io_state = HVMIO_none;
> }
> else
> @@ -226,13 +257,19 @@ static int hvmemul_do_io(
> }
> else
> {
> - rc = X86EMUL_RETRY;
> - if ( !hvm_send_assist_req(&p) )
> + if ( regs )
> + rc = X86EMUL_VMPORT_RETRY;
> + else
> + rc = X86EMUL_RETRY;
> + if ( !hvm_send_assist_req(&p, vpp) )
> vio->io_state = HVMIO_none;
> else if ( p_data == NULL )
> rc = X86EMUL_OKAY;
> }
> break;
> + case X86EMUL_VMPORT_RETRY:
> + rc = X86EMUL_RETRY;
> + break;
> default:
> BUG();
> }
I still fail to see why you need to make such intrusive modifications to this code. I would have thought you could add a new ioreq type, as you've done, but not make it 'data bearing'. I.e. you use your new VMPORT_IOREQ_PFN to carry the register values back and forth between Xen and QEMU, but you still issue a 'normal' ioreq_t structure (with your new type) via the 'normal' shared IOREQ_PFN. That way you need do nothing to the majority of the emulation code path - you'd just need to add code t copy the register values into and out of your new shared page at start and completion of I/O.
Paul
> @@ -287,14 +324,21 @@ int hvmemul_do_pio(
> unsigned long port, unsigned long *reps, int size,
> paddr_t ram_gpa, int dir, int df, void *p_data)
> {
> - return hvmemul_do_io(0, port, reps, size, ram_gpa, dir, df, p_data);
> + return hvmemul_do_io(0, port, reps, size, ram_gpa, dir, df, p_data,
> NULL);
> +}
> +
> +int hvmemul_do_vmport(
> + unsigned long port, unsigned long *reps, int size,
> + int dir, void *p_data, struct cpu_user_regs *regs)
> +{
> + return hvmemul_do_io(0, port, reps, size, 0, dir, 0, p_data, regs);
> }
>
> static int hvmemul_do_mmio(
> paddr_t gpa, unsigned long *reps, int size,
> paddr_t ram_gpa, int dir, int df, void *p_data)
> {
> - return hvmemul_do_io(1, gpa, reps, size, ram_gpa, dir, df, p_data);
> + return hvmemul_do_io(1, gpa, reps, size, ram_gpa, dir, df, p_data, NULL);
> }
>
> /*
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 8d0a3a0..fd05a85 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -382,6 +382,16 @@ static ioreq_t *get_ioreq(struct hvm_ioreq_server
> *s, struct vcpu *v)
> return &p->vcpu_ioreq[v->vcpu_id];
> }
>
> +static vmware_ioreq_t *get_vmport_ioreq(struct hvm_ioreq_server *s,
> struct vcpu *v)
> +{
> + shared_vmport_iopage_t *p = s->vmport_ioreq.va;
> +
> + ASSERT((v == current) || !vcpu_runnable(v));
> + ASSERT(p != NULL);
> +
> + return &p->vcpu_vmport_ioreq[v->vcpu_id];
> +}
> +
> bool_t hvm_io_pending(struct vcpu *v)
> {
> struct domain *d = v->domain;
> @@ -400,7 +410,8 @@ bool_t hvm_io_pending(struct vcpu *v)
> return 0;
> }
>
> -static bool_t hvm_wait_for_io(struct hvm_ioreq_vcpu *sv, ioreq_t *p)
> +static bool_t hvm_wait_for_io(struct hvm_ioreq_vcpu *sv, ioreq_t *p,
> + vmware_ioreq_t *vp)
> {
> /* NB. Optimised for common case (p->state == STATE_IOREQ_NONE). */
> while ( p->state != STATE_IOREQ_NONE )
> @@ -409,7 +420,7 @@ static bool_t hvm_wait_for_io(struct
> hvm_ioreq_vcpu *sv, ioreq_t *p)
> {
> case STATE_IORESP_READY: /* IORESP_READY -> NONE */
> rmb(); /* see IORESP_READY /then/ read contents of ioreq */
> - hvm_io_assist(p);
> + hvm_io_assist(p, vp);
> break;
> case STATE_IOREQ_READY: /* IOREQ_{READY,INPROCESS} ->
> IORESP_READY */
> case STATE_IOREQ_INPROCESS:
> @@ -449,7 +460,8 @@ void hvm_do_resume(struct vcpu *v)
> {
> if ( sv->vcpu == v )
> {
> - if ( !hvm_wait_for_io(sv, get_ioreq(s, v)) )
> + if ( !hvm_wait_for_io(sv, get_ioreq(s, v),
> + get_vmport_ioreq(s, v)) )
> return;
>
> break;
> @@ -491,22 +503,50 @@ static void hvm_free_ioreq_gmfn(struct domain
> *d, unsigned long gmfn)
> clear_bit(i, &d->arch.hvm_domain.ioreq_gmfn.mask);
> }
>
> -static void hvm_unmap_ioreq_page(struct hvm_ioreq_server *s, bool_t
> buf)
> +static void hvm_unmap_ioreq_page(struct hvm_ioreq_server *s, int buf)
> {
> - struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
> + struct hvm_ioreq_page *iorp = NULL;
> +
> + switch ( buf )
> + {
> + case 0:
> + iorp = &s->ioreq;
> + break;
> + case 1:
> + iorp = &s->bufioreq;
> + break;
> + case 2:
> + iorp = &s->vmport_ioreq;
> + break;
> + }
> + ASSERT(iorp);
>
> destroy_ring_for_helper(&iorp->va, iorp->page);
> }
>
> static int hvm_map_ioreq_page(
> - struct hvm_ioreq_server *s, bool_t buf, unsigned long gmfn)
> + struct hvm_ioreq_server *s, int buf, unsigned long gmfn)
> {
> struct domain *d = s->domain;
> - struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
> + struct hvm_ioreq_page *iorp = NULL;
> struct page_info *page;
> void *va;
> int rc;
>
> + switch ( buf )
> + {
> + case 0:
> + iorp = &s->ioreq;
> + break;
> + case 1:
> + iorp = &s->bufioreq;
> + break;
> + case 2:
> + iorp = &s->vmport_ioreq;
> + break;
> + }
> + ASSERT(iorp);
> +
> if ( (rc = prepare_ring_for_helper(d, gmfn, &page, &va)) )
> return rc;
>
> @@ -717,7 +757,7 @@ static int hvm_ioreq_server_map_pages(struct
> hvm_ioreq_server *s,
> bool_t is_default, bool_t handle_bufioreq)
> {
> struct domain *d = s->domain;
> - unsigned long ioreq_pfn, bufioreq_pfn;
> + unsigned long ioreq_pfn, bufioreq_pfn, vmport_ioreq_pfn = 0;
> int rc;
>
> if ( is_default )
> @@ -730,6 +770,7 @@ static int hvm_ioreq_server_map_pages(struct
> hvm_ioreq_server *s,
> */
> ASSERT(handle_bufioreq);
> bufioreq_pfn = d-
> >arch.hvm_domain.params[HVM_PARAM_BUFIOREQ_PFN];
> + vmport_ioreq_pfn = d-
> >arch.hvm_domain.params[HVM_PARAM_VMPORT_IOREQ_PFN];
> }
> else
> {
> @@ -754,10 +795,16 @@ static int hvm_ioreq_server_map_pages(struct
> hvm_ioreq_server *s,
> rc = hvm_map_ioreq_page(s, 1, bufioreq_pfn);
> if ( rc )
> goto fail4;
> + rc = hvm_map_ioreq_page(s, 2, vmport_ioreq_pfn);
> + if ( rc )
> + goto fail5;
> }
>
> return 0;
>
> +fail5:
> + hvm_unmap_ioreq_page(s, 2);
> +
> fail4:
> hvm_unmap_ioreq_page(s, 0);
>
> @@ -2510,7 +2557,8 @@ bool_t hvm_has_dm(struct domain *d)
> }
>
> bool_t hvm_send_assist_req_to_ioreq_server(struct hvm_ioreq_server *s,
> - ioreq_t *proto_p)
> + ioreq_t *proto_p,
> + vmware_ioreq_t *proto_vp)
> {
> struct vcpu *curr = current;
> struct domain *d = curr->domain;
> @@ -2544,6 +2592,12 @@ bool_t
> hvm_send_assist_req_to_ioreq_server(struct hvm_ioreq_server *s,
> goto crash;
> }
>
> + if ( proto_vp )
> + {
> + vmware_ioreq_t *vp = get_vmport_ioreq(s, curr);
> +
> + *vp = *proto_vp;
> + }
> proto_p->state = STATE_IOREQ_NONE;
> proto_p->vp_eport = port;
> *p = *proto_p;
> @@ -2591,21 +2645,21 @@ static bool_t hvm_complete_assist_req(ioreq_t
> *p)
> /* FALLTHRU */
> default:
> p->state = STATE_IORESP_READY;
> - hvm_io_assist(p);
> + hvm_io_assist(p, NULL);
> break;
> }
>
> return 1;
> }
>
> -bool_t hvm_send_assist_req(ioreq_t *p)
> +bool_t hvm_send_assist_req(ioreq_t *p, vmware_ioreq_t *vp)
> {
> struct hvm_ioreq_server *s = hvm_select_ioreq_server(current->domain,
> p);
>
> if ( !s )
> return hvm_complete_assist_req(p);
>
> - return hvm_send_assist_req_to_ioreq_server(s, p);
> + return hvm_send_assist_req_to_ioreq_server(s, p, vp);
> }
>
> void hvm_broadcast_assist_req(ioreq_t *p)
> @@ -2618,7 +2672,7 @@ void hvm_broadcast_assist_req(ioreq_t *p)
> list_for_each_entry ( s,
> &d->arch.hvm_domain.ioreq_server.list,
> list_entry )
> - (void) hvm_send_assist_req_to_ioreq_server(s, p);
> + (void) hvm_send_assist_req_to_ioreq_server(s, p, NULL);
> }
>
> void hvm_hlt(unsigned long rflags)
> @@ -5763,6 +5817,7 @@ long do_hvm_op(unsigned long op,
> XEN_GUEST_HANDLE_PARAM(void) arg)
> break;
> }
> case HVM_PARAM_IOREQ_PFN:
> + case HVM_PARAM_VMPORT_IOREQ_PFN:
> case HVM_PARAM_BUFIOREQ_PFN:
> case HVM_PARAM_BUFIOREQ_EVTCHN: {
> domid_t domid;
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index 68fb890..eb09032 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -154,8 +154,17 @@ int handle_pio(uint16_t port, unsigned int size, int
> dir)
> }
> break;
> case X86EMUL_RETRY:
> - if ( vio->io_state != HVMIO_awaiting_completion )
> + if ( vio->io_state == HVMIO_handle_vmport_awaiting_completion ) {
> + /* Completion in hvm_io_assist() with no re-emulation required. */
> +#ifdef VMPORT_IO_LOGGING
> + gdprintk(XENLOG_WARNING, "_vmport_awaiting\n");
> +#endif
> + return 1;
> + }
> + if ( vio->io_state != HVMIO_awaiting_completion ) {
> + gdprintk(XENLOG_WARNING, "WARNING: io_state=%d?\n", vio-
> >io_state);
> return 0;
> + }
> /* Completion in hvm_io_assist() with no re-emulation required. */
> ASSERT(dir == IOREQ_READ);
> vio->io_state = HVMIO_handle_pio_awaiting_completion;
> @@ -169,7 +178,7 @@ int handle_pio(uint16_t port, unsigned int size, int dir)
> return 1;
> }
>
> -void hvm_io_assist(ioreq_t *p)
> +void hvm_io_assist(ioreq_t *p, vmware_ioreq_t *vp)
> {
> struct vcpu *curr = current;
> struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
> @@ -197,6 +206,33 @@ void hvm_io_assist(ioreq_t *p)
> else
> memcpy(&guest_cpu_user_regs()->rax, &p->data, vio->io_size);
> break;
> + case HVMIO_handle_vmport_awaiting_completion:
> + {
> + struct cpu_user_regs *regs = guest_cpu_user_regs();
> +
> + ASSERT(vp);
> +#ifdef VMPORT_IO_LOGGING
> + gdprintk(XENLOG_WARNING, "vmport done ip=0x%lx\n",
> + (long)regs->rip);
> +#endif
> + if ( p->dir == IOREQ_READ)
> + {
> + if ( vio->io_size == 4 ) /* Needs zero extension. */
> + regs->rax = (uint32_t)p->data;
> + else
> + memcpy(®s->rax, &p->data, vio->io_size);
> + }
> + /* Only change the 32bit part of the register */
> + regs->_ebx = vp->ebx;
> + regs->_ecx = vp->ecx;
> + regs->_edx = vp->edx;
> + regs->_esi = vp->esi;
> + regs->_edi = vp->edi;
> + HVMTRACE_ND(VMPORT_QEMU, 0, 1/*cycles*/, 6,
> + regs->rax, regs->rbx, regs->rcx,
> + regs->rdx, regs->rsi, regs->rdi);
> + }
> + break;
> default:
> break;
> }
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 8b1185e..ae21356 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2140,10 +2140,24 @@ static void svm_vmexit_gp_intercept(struct
> cpu_user_regs *regs,
> HVMTRACE_C4D(TRAP_GP, inst_len, starting_rdx, vmcb->exitinfo1,
> vmcb->exitinfo2);
>
> - if ( !rc )
> - __update_guest_eip(regs, inst_len);
> - else
> + switch ( rc )
> {
> + case X86EMUL_VMPORT_RETRY:
> + rc = X86EMUL_RETRY;
> + /* fall through */
> + case X86EMUL_RETRY:
> + {
> + struct hvm_vcpu_io *vio = &v->arch.hvm_vcpu.hvm_io;
> + if ( vio->io_state != HVMIO_handle_vmport_awaiting_completion ) {
> + gdprintk(XENLOG_WARNING, "WARNING: io_state=%d?\n", vio-
> >io_state);
> + break;
> + }
> + }
> + /* fall through */
> + case X86EMUL_OKAY:
> + __update_guest_eip(regs, inst_len);
> + break;
> + default:
> VMPORT_DBG_LOG(VMPORT_LOG_GP_UNKNOWN,
> "gp: rc=%d ei1=0x%lx ei2=0x%lx ec=0x%x ip=%"PRIx64
> " (0x%lx,%ld) ax=%"PRIx64" bx=%"PRIx64" cx=%"PRIx64
> @@ -2159,6 +2173,7 @@ static void svm_vmexit_gp_intercept(struct
> cpu_user_regs *regs,
> HVMTRACE_C5D(TRAP_GP_UNKNOWN, rc, regs->rax, regs->rbx, regs-
> >rcx,
> inst_addr);
> hvm_inject_hw_exception(TRAP_gp_fault, vmcb->exitinfo1);
> + break;
> }
> }
>
> diff --git a/xen/arch/x86/hvm/vmware/vmport.c
> b/xen/arch/x86/hvm/vmware/vmport.c
> index 962ee32..d1632bb 100644
> --- a/xen/arch/x86/hvm/vmware/vmport.c
> +++ b/xen/arch/x86/hvm/vmware/vmport.c
> @@ -147,9 +147,34 @@ int vmport_ioport(int dir, uint32_t port, uint32_t
> bytes, uint32_t *val)
> regs->rax = 0x0;
> break;
> default:
> + { /* Let backing DM handle */
> + unsigned long data, reps = 1;
> +
> HVMTRACE_ND(VMPORT_UNKNOWN, 0, 1/*cycles*/, 6,
> - (bytes << 8) + dir, cmd, regs->rbx,
> + (bytes << 8) | dir, cmd, regs->rbx,
> regs->rcx, regs->rsi, regs->rdi);
> + rc = hvmemul_do_vmport(BDOOR_PORT, &reps, bytes, dir, &data,
> regs);
> + switch (rc)
> + {
> + case X86EMUL_OKAY:
> + break;
> + case X86EMUL_VMPORT_RETRY:
> + case X86EMUL_RETRY:
> + {
> + struct hvm_vcpu_io *vio = ¤t->arch.hvm_vcpu.hvm_io;
> +
> + if ( vio->io_state != HVMIO_handle_vmport_awaiting_completion )
> + gdprintk(XENLOG_WARNING, "vio: io_state=%d ==> %d\n",
> + vio->io_state, rc);
> + return rc;
> + break;
> + }
> + default:
> + gdprintk(XENLOG_ERR, "Weird HVM ioemulation status %d.\n", rc);
> + domain_crash(current->domain);
> + break;
> + }
> + }
> break;
> }
>
> @@ -190,6 +215,9 @@ int vmport_ioport(int dir, uint32_t port, uint32_t
> bytes, uint32_t *val)
> rc = X86EMUL_UNHANDLEABLE;
> }
>
> +#ifdef VMPORT_IO_LOGGING
> + gdprintk(XENLOG_WARNING, "vmport: rc=%d\n", rc);
> +#endif
> return rc;
> }
>
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index c84894a..dd079bd 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2643,10 +2643,24 @@ static void vmx_vmexit_gp_intercept(struct
> cpu_user_regs *regs,
> "Unexpected instruction length difference: %lu vs %lu\n",
> orig_inst_len, inst_len);
> #endif
> - if ( !rc )
> - update_guest_eip();
> - else
> + switch ( rc )
> {
> + case X86EMUL_VMPORT_RETRY:
> + rc = X86EMUL_RETRY;
> + /* fall through */
> + case X86EMUL_RETRY:
> + {
> + struct hvm_vcpu_io *vio = &v->arch.hvm_vcpu.hvm_io;
> + if ( vio->io_state != HVMIO_handle_vmport_awaiting_completion ) {
> + gdprintk(XENLOG_WARNING, "WARNING: io_state=%d?\n", vio-
> >io_state);
> + break;
> + }
> + }
> + /* fall through */
> + case X86EMUL_OKAY:
> + update_guest_eip();
> + break;
> + default:
> VMPORT_DBG_LOG(VMPORT_LOG_GP_UNKNOWN,
> "gp: rc=%d ecode=0x%lx eq=0x%lx ec=0x%x ip=%"PRIx64
> " (0x%lx,%lu=>%lu) ax=%"PRIx64" bx=%"PRIx64
> @@ -2661,7 +2675,8 @@ static void vmx_vmexit_gp_intercept(struct
> cpu_user_regs *regs,
> else
> HVMTRACE_C5D(TRAP_GP_UNKNOWN, rc, regs->rax, regs->rbx, regs-
> >rcx,
> inst_addr);
> - hvm_inject_hw_exception(TRAP_gp_fault, ecode);
> + hvm_inject_hw_exception(TRAP_gp_fault, ecode);
> + break;
> }
> }
>
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h
> b/xen/arch/x86/x86_emulate/x86_emulate.h
> index b059341..1733eca 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -110,6 +110,8 @@ struct __packed segment_register {
> #define X86EMUL_RETRY 3
> /* (cmpxchg accessor): CMPXCHG failed. Maps to X86EMUL_RETRY in caller.
> */
> #define X86EMUL_CMPXCHG_FAILED 3
> + /* Like X86EMUL_RETRY, but do not change vio->io_state. */
> +#define X86EMUL_VMPORT_RETRY 4
>
> /* FPU sub-types which may be requested via ->get_fpu(). */
> enum x86_emulate_fpu_type {
> diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-
> x86/hvm/domain.h
> index d4718df..e7e6cd9 100644
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -63,6 +63,7 @@ struct hvm_ioreq_server {
> ioservid_t id;
> struct hvm_ioreq_page ioreq;
> struct list_head ioreq_vcpu_list;
> + struct hvm_ioreq_page vmport_ioreq;
> struct hvm_ioreq_page bufioreq;
>
> /* Lock to serialize access to buffered ioreq ring */
> diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm-
> x86/hvm/emulate.h
> index 5411302..6c3ff2a 100644
> --- a/xen/include/asm-x86/hvm/emulate.h
> +++ b/xen/include/asm-x86/hvm/emulate.h
> @@ -53,6 +53,9 @@ struct segment_register *hvmemul_get_seg_reg(
> int hvmemul_do_pio(
> unsigned long port, unsigned long *reps, int size,
> paddr_t ram_gpa, int dir, int df, void *p_data);
> +int hvmemul_do_vmport(
> + unsigned long port, unsigned long *reps, int size,
> + int dir, void *p_data, struct cpu_user_regs *regs);
>
> void hvm_dump_emulation_state(const char *prefix,
> struct hvm_emulate_ctxt *hvmemul_ctxt);
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-
> x86/hvm/hvm.h
> index 0910147..b57c2d7 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -228,7 +228,7 @@ int hvm_vcpu_cacheattr_init(struct vcpu *v);
> void hvm_vcpu_cacheattr_destroy(struct vcpu *v);
> void hvm_vcpu_reset_state(struct vcpu *v, uint16_t cs, uint16_t ip);
>
> -bool_t hvm_send_assist_req(ioreq_t *p);
> +bool_t hvm_send_assist_req(ioreq_t *p, vmware_ioreq_t *vp);
> void hvm_broadcast_assist_req(ioreq_t *p);
>
> void hvm_get_guest_pat(struct vcpu *v, u64 *guest_pat);
> diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h
> index d257161..b100fea 100644
> --- a/xen/include/asm-x86/hvm/io.h
> +++ b/xen/include/asm-x86/hvm/io.h
> @@ -123,7 +123,7 @@ int handle_mmio_with_translation(unsigned long
> gva, unsigned long gpfn,
> struct npfec);
> int handle_pio(uint16_t port, unsigned int size, int dir);
> void hvm_interrupt_post(struct vcpu *v, int vector, int type);
> -void hvm_io_assist(ioreq_t *p);
> +void hvm_io_assist(ioreq_t *p, vmware_ioreq_t *vp);
> void hvm_dpci_eoi(struct domain *d, unsigned int guest_irq,
> const union vioapic_redir_entry *ent);
> void msix_write_completion(struct vcpu *);
> diff --git a/xen/include/asm-x86/hvm/trace.h b/xen/include/asm-
> x86/hvm/trace.h
> index 8af2d6a..10d78bb 100644
> --- a/xen/include/asm-x86/hvm/trace.h
> +++ b/xen/include/asm-x86/hvm/trace.h
> @@ -66,6 +66,7 @@
> #define DO_TRC_HVM_VMPORT_BAD DEFAULT_HVM_IO
> #define DO_TRC_HVM_VMPORT_BAD64 DEFAULT_HVM_IO
> #define DO_TRC_HVM_VMPORT_UNKNOWN DEFAULT_HVM_IO
> +#define DO_TRC_HVM_VMPORT_QEMU DEFAULT_HVM_IO
>
>
> #define TRC_PAR_LONG(par) ((par)&0xFFFFFFFF),((par)>>32)
> diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-
> x86/hvm/vcpu.h
> index 01e0665..1e63d7f 100644
> --- a/xen/include/asm-x86/hvm/vcpu.h
> +++ b/xen/include/asm-x86/hvm/vcpu.h
> @@ -36,6 +36,7 @@ enum hvm_io_state {
> HVMIO_awaiting_completion,
> HVMIO_handle_mmio_awaiting_completion,
> HVMIO_handle_pio_awaiting_completion,
> + HVMIO_handle_vmport_awaiting_completion,
> HVMIO_completed
> };
>
> diff --git a/xen/include/public/hvm/ioreq.h
> b/xen/include/public/hvm/ioreq.h
> index 5b5fedf..c20b834 100644
> --- a/xen/include/public/hvm/ioreq.h
> +++ b/xen/include/public/hvm/ioreq.h
> @@ -35,6 +35,7 @@
> #define IOREQ_TYPE_PIO 0 /* pio */
> #define IOREQ_TYPE_COPY 1 /* mmio ops */
> #define IOREQ_TYPE_PCI_CONFIG 2
> +#define IOREQ_TYPE_VMWARE_PORT 3
> #define IOREQ_TYPE_TIMEOFFSET 7
> #define IOREQ_TYPE_INVALIDATE 8 /* mapcache */
>
> @@ -48,6 +49,8 @@
> *
> * 63....48|47..40|39..35|34..32|31........0
> * SEGMENT |BUS |DEV |FN |OFFSET
> + *
> + * For I/O type IOREQ_TYPE_VMWARE_PORT also use the vmware_ioreq.
> */
> struct ioreq {
> uint64_t addr; /* physical address */
> @@ -66,11 +69,25 @@ struct ioreq {
> };
> typedef struct ioreq ioreq_t;
>
> +struct vmware_ioreq {
> + uint32_t esi;
> + uint32_t edi;
> + uint32_t ebx;
> + uint32_t ecx;
> + uint32_t edx;
> +};
> +typedef struct vmware_ioreq vmware_ioreq_t;
> +
> struct shared_iopage {
> struct ioreq vcpu_ioreq[1];
> };
> typedef struct shared_iopage shared_iopage_t;
>
> +struct shared_vmport_iopage {
> + struct vmware_ioreq vcpu_vmport_ioreq[1];
> +};
> +typedef struct shared_vmport_iopage shared_vmport_iopage_t;
> +
> struct buf_ioreq {
> uint8_t type; /* I/O type */
> uint8_t pad:1;
> diff --git a/xen/include/public/hvm/params.h
> b/xen/include/public/hvm/params.h
> index c893dc5..2d75bdd 100644
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -50,6 +50,8 @@
> #define HVM_PARAM_PAE_ENABLED 4
>
> #define HVM_PARAM_IOREQ_PFN 5
> +/* Extra vmport PFN. */
> +#define HVM_PARAM_VMPORT_IOREQ_PFN 36
>
> #define HVM_PARAM_BUFIOREQ_PFN 6
> #define HVM_PARAM_BUFIOREQ_EVTCHN 26
> @@ -192,6 +194,6 @@
> /* Params for VMware */
> #define HVM_PARAM_VMWARE_HW 35
>
> -#define HVM_NR_PARAMS 36
> +#define HVM_NR_PARAMS 37
>
> #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
> diff --git a/xen/include/public/trace.h b/xen/include/public/trace.h
> index b231df3..e35a297 100644
> --- a/xen/include/public/trace.h
> +++ b/xen/include/public/trace.h
> @@ -237,6 +237,7 @@
> #define TRC_HVM_VMPORT_BAD (TRC_HVM_HANDLER + 0x2a)
> #define TRC_HVM_VMPORT_BAD64 (TRC_HVM_HANDLER +
> TRC_64_FLAG + 0x2a)
> #define TRC_HVM_VMPORT_UNKNOWN (TRC_HVM_HANDLER + 0x2b)
> +#define TRC_HVM_VMPORT_QEMU (TRC_HVM_HANDLER + 0x2c)
>
> #define TRC_HVM_IOPORT_WRITE (TRC_HVM_HANDLER + 0x216)
> #define TRC_HVM_IOMEM_WRITE (TRC_HVM_HANDLER + 0x217)
> --
> 1.8.4
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2x prototype 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-13 13:26 ` Paul Durrant
@ 2014-10-13 17:11 ` Don Slutz
2014-10-14 9:57 ` Paul Durrant
0 siblings, 1 reply; 25+ messages in thread
From: Don Slutz @ 2014-10-13 17:11 UTC (permalink / raw)
To: Paul Durrant, Don Slutz, xen-devel
Cc: Keir (Xen.org), Ian Campbell, Jan Beulich
On 10/13/14 09:26, Paul Durrant wrote:
>> -----Original Message-----
>> From: Don Slutz [mailto:dslutz@verizon.com]
>> Sent: 09 October 2014 15:26
>> To: xen-devel@lists.xen.org; Paul Durrant
>> Cc: Jan Beulich; Keir (Xen.org); Ian Campbell; Don Slutz
>> Subject: [RFC][PATCH v2x prototype 1/1] Add IOREQ_TYPE_VMWARE_PORT
>>
>> This adds synchronisation of the 6 vcpu registers (only 32bits of
>> them) that vmport.c needs between Xen and QEMU.
>>
>> This is to avoid a 2nd and 3rd exchange between QEMU and Xen to
>> fetch and put these 6 vcpu registers used by the code in vmport.c
>> and vmmouse.c
>>
>> QEMU patch is named "xen-hvm.c: Add support for Xen access to vmport"
>>
>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>> ---
>> As requested by Paul Durrant <Paul.Durrant@citrix.com>
>>
>> Here is a prototype of the QEMU change using a 2nd shared page.
>> I picked adding HVM_PARAM_VMPORT_IOREQ_PFN as the simple and
>> fast way to handle QEMU building on older Xen versions.
>>
>> There is xentrace and debug logging that is TBD for the Xen 4.6
>> submission of this.
...
>> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
>> index c0f47d2..4b8ea8f 100644
>> --- a/xen/arch/x86/hvm/emulate.c
>> +++ b/xen/arch/x86/hvm/emulate.c
>> @@ -52,12 +52,14 @@ static void hvmtrace_io_assist(int is_mmio, ioreq_t
>> *p)
>>
>> static int hvmemul_do_io(
>> int is_mmio, paddr_t addr, unsigned long *reps, int size,
>> - paddr_t ram_gpa, int dir, int df, void *p_data)
>> + paddr_t ram_gpa, int dir, int df, void *p_data,
>> + struct cpu_user_regs *regs)
>> {
>> struct vcpu *curr = current;
>> struct hvm_vcpu_io *vio;
>> ioreq_t p = {
>> - .type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
>> + .type = regs ? IOREQ_TYPE_VMWARE_PORT :
>> + is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
>> .addr = addr,
>> .size = size,
>> .dir = dir,
>> @@ -65,11 +67,15 @@ static int hvmemul_do_io(
>> .data = ram_gpa,
>> .data_is_ptr = (p_data == NULL),
>> };
>> + vmware_ioreq_t vp;
>> + vmware_ioreq_t *vpp;
>> unsigned long ram_gfn = paddr_to_pfn(ram_gpa);
>> p2m_type_t p2mt;
>> struct page_info *ram_page;
>> int rc;
>>
>> + BUILD_BUG_ON(sizeof(ioreq_t) < sizeof(vmware_ioreq_t));
>> +
>> /* Check for paged out page */
>> ram_page = get_page_from_gfn(curr->domain, ram_gfn, &p2mt,
>> P2M_UNSHARE);
>> if ( p2m_is_paging(p2mt) )
>> @@ -101,7 +107,17 @@ static int hvmemul_do_io(
>> return X86EMUL_UNHANDLEABLE;
>> }
>>
>> - if ( !p.data_is_ptr && (dir == IOREQ_WRITE) )
>> + if ( regs )
>> + {
>> + vpp = &vp;
>> + p.data = regs->rax;
>> + vp.ebx = regs->rbx;
>> + vp.ecx = regs->rcx;
>> + vp.edx = regs->rdx;
>> + vp.esi = regs->rsi;
>> + vp.edi = regs->rdi;
>> + }
>> + else if ( !p.data_is_ptr && (dir == IOREQ_WRITE) )
>> {
>> memcpy(&p.data, p_data, size);
>> p_data = NULL;
>> @@ -161,7 +177,19 @@ static int hvmemul_do_io(
>> put_page(ram_page);
>> return X86EMUL_RETRY;
>> }
>> + case HVMIO_awaiting_completion:
>> + if ( regs )
>> + {
>> + /* May have to wait for previous cycle of a multi-write to complete.
>> */
>> + if ( vio->mmio_retry ) {
>> + gdprintk(XENLOG_WARNING, "WARNING: mmio_retry
>> io_state=%d?\n", vio->io_state);
>> + return X86EMUL_RETRY;
>> + }
>> + /* These are expected if we get here via hvmemul_do_io() */
>> + break;
>> + }
>> default:
>> + gdprintk(XENLOG_WARNING, "WARNING: io_state=%d?\n", vio-
>>> io_state);
>> if ( ram_page )
>> put_page(ram_page);
>> return X86EMUL_UNHANDLEABLE;
>> @@ -175,7 +203,7 @@ static int hvmemul_do_io(
>> return X86EMUL_UNHANDLEABLE;
>> }
>>
>> - vio->io_state =
>> + vio->io_state = regs ? HVMIO_handle_vmport_awaiting_completion :
>> (p_data == NULL) ? HVMIO_dispatched : HVMIO_awaiting_completion;
>> vio->io_size = size;
>>
>> @@ -197,6 +225,9 @@ static int hvmemul_do_io(
>> if ( rc == X86EMUL_UNHANDLEABLE )
>> rc = hvm_buffered_io_intercept(&p);
>> }
>> + else if ( regs ) {
>> + rc = X86EMUL_UNHANDLEABLE;
>> + }
>> else
>> {
>> rc = hvm_portio_intercept(&p);
>> @@ -210,7 +241,7 @@ static int hvmemul_do_io(
>> p.state = STATE_IORESP_READY;
>> if ( !vio->mmio_retry )
>> {
>> - hvm_io_assist(&p);
>> + hvm_io_assist(&p, vpp);
>> vio->io_state = HVMIO_none;
>> }
>> else
>> @@ -226,13 +257,19 @@ static int hvmemul_do_io(
>> }
>> else
>> {
>> - rc = X86EMUL_RETRY;
>> - if ( !hvm_send_assist_req(&p) )
>> + if ( regs )
>> + rc = X86EMUL_VMPORT_RETRY;
>> + else
>> + rc = X86EMUL_RETRY;
>> + if ( !hvm_send_assist_req(&p, vpp) )
>> vio->io_state = HVMIO_none;
>> else if ( p_data == NULL )
>> rc = X86EMUL_OKAY;
>> }
>> break;
>> + case X86EMUL_VMPORT_RETRY:
>> + rc = X86EMUL_RETRY;
>> + break;
>> default:
>> BUG();
>> }
> I still fail to see why you need to make such intrusive modifications to this code.
I am confused. The code is doing what you say:
> I would have thought you could add a new ioreq type, as you've done, but not make it 'data bearing'. I.e. you use your new VMPORT_IOREQ_PFN to carry the register values back and forth between Xen and QEMU, but you still issue a 'normal' ioreq_t structure (with your new type) via the 'normal' shared IOREQ_PFN. That way you need do nothing to the majority of the emulation code path - you'd just need to add code t copy the register values into and out of your new shared page at start and completion of I/O.
I did adjust hvmemul_do_io() instead of duplicating it's code.
hvmemul_do_io() cannot be used without adjustment because of
the new type.
I will code this up with a new routine.
-Don Slutz
> Paul
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2x prototype 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-13 17:11 ` Don Slutz
@ 2014-10-14 9:57 ` Paul Durrant
2014-10-14 19:06 ` Don Slutz
0 siblings, 1 reply; 25+ messages in thread
From: Paul Durrant @ 2014-10-14 9:57 UTC (permalink / raw)
To: Don Slutz, xen-devel; +Cc: Keir (Xen.org), Ian Campbell, Jan Beulich
> -----Original Message-----
> From: Don Slutz [mailto:dslutz@verizon.com]
> Sent: 13 October 2014 18:11
> To: Paul Durrant; Don Slutz; xen-devel@lists.xen.org
> Cc: Jan Beulich; Keir (Xen.org); Ian Campbell
> Subject: Re: [RFC][PATCH v2x prototype 1/1] Add
> IOREQ_TYPE_VMWARE_PORT
>
> On 10/13/14 09:26, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Don Slutz [mailto:dslutz@verizon.com]
> >> Sent: 09 October 2014 15:26
> >> To: xen-devel@lists.xen.org; Paul Durrant
> >> Cc: Jan Beulich; Keir (Xen.org); Ian Campbell; Don Slutz
> >> Subject: [RFC][PATCH v2x prototype 1/1] Add
> IOREQ_TYPE_VMWARE_PORT
> >>
> >> This adds synchronisation of the 6 vcpu registers (only 32bits of
> >> them) that vmport.c needs between Xen and QEMU.
> >>
> >> This is to avoid a 2nd and 3rd exchange between QEMU and Xen to
> >> fetch and put these 6 vcpu registers used by the code in vmport.c
> >> and vmmouse.c
> >>
> >> QEMU patch is named "xen-hvm.c: Add support for Xen access to
> vmport"
> >>
> >> Signed-off-by: Don Slutz <dslutz@verizon.com>
> >> ---
> >> As requested by Paul Durrant <Paul.Durrant@citrix.com>
> >>
> >> Here is a prototype of the QEMU change using a 2nd shared page.
> >> I picked adding HVM_PARAM_VMPORT_IOREQ_PFN as the simple and
> >> fast way to handle QEMU building on older Xen versions.
> >>
> >> There is xentrace and debug logging that is TBD for the Xen 4.6
> >> submission of this.
> ...
> >> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> >> index c0f47d2..4b8ea8f 100644
> >> --- a/xen/arch/x86/hvm/emulate.c
> >> +++ b/xen/arch/x86/hvm/emulate.c
> >> @@ -52,12 +52,14 @@ static void hvmtrace_io_assist(int is_mmio, ioreq_t
> >> *p)
> >>
> >> static int hvmemul_do_io(
> >> int is_mmio, paddr_t addr, unsigned long *reps, int size,
> >> - paddr_t ram_gpa, int dir, int df, void *p_data)
> >> + paddr_t ram_gpa, int dir, int df, void *p_data,
> >> + struct cpu_user_regs *regs)
> >> {
> >> struct vcpu *curr = current;
> >> struct hvm_vcpu_io *vio;
> >> ioreq_t p = {
> >> - .type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
> >> + .type = regs ? IOREQ_TYPE_VMWARE_PORT :
> >> + is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
> >> .addr = addr,
> >> .size = size,
> >> .dir = dir,
> >> @@ -65,11 +67,15 @@ static int hvmemul_do_io(
> >> .data = ram_gpa,
> >> .data_is_ptr = (p_data == NULL),
> >> };
> >> + vmware_ioreq_t vp;
> >> + vmware_ioreq_t *vpp;
> >> unsigned long ram_gfn = paddr_to_pfn(ram_gpa);
> >> p2m_type_t p2mt;
> >> struct page_info *ram_page;
> >> int rc;
> >>
> >> + BUILD_BUG_ON(sizeof(ioreq_t) < sizeof(vmware_ioreq_t));
> >> +
> >> /* Check for paged out page */
> >> ram_page = get_page_from_gfn(curr->domain, ram_gfn, &p2mt,
> >> P2M_UNSHARE);
> >> if ( p2m_is_paging(p2mt) )
> >> @@ -101,7 +107,17 @@ static int hvmemul_do_io(
> >> return X86EMUL_UNHANDLEABLE;
> >> }
> >>
> >> - if ( !p.data_is_ptr && (dir == IOREQ_WRITE) )
> >> + if ( regs )
> >> + {
> >> + vpp = &vp;
> >> + p.data = regs->rax;
> >> + vp.ebx = regs->rbx;
> >> + vp.ecx = regs->rcx;
> >> + vp.edx = regs->rdx;
> >> + vp.esi = regs->rsi;
> >> + vp.edi = regs->rdi;
> >> + }
> >> + else if ( !p.data_is_ptr && (dir == IOREQ_WRITE) )
> >> {
> >> memcpy(&p.data, p_data, size);
> >> p_data = NULL;
> >> @@ -161,7 +177,19 @@ static int hvmemul_do_io(
> >> put_page(ram_page);
> >> return X86EMUL_RETRY;
> >> }
> >> + case HVMIO_awaiting_completion:
> >> + if ( regs )
> >> + {
> >> + /* May have to wait for previous cycle of a multi-write to
> complete.
> >> */
> >> + if ( vio->mmio_retry ) {
> >> + gdprintk(XENLOG_WARNING, "WARNING: mmio_retry
> >> io_state=%d?\n", vio->io_state);
> >> + return X86EMUL_RETRY;
> >> + }
> >> + /* These are expected if we get here via hvmemul_do_io() */
> >> + break;
> >> + }
> >> default:
> >> + gdprintk(XENLOG_WARNING, "WARNING: io_state=%d?\n", vio-
> >>> io_state);
> >> if ( ram_page )
> >> put_page(ram_page);
> >> return X86EMUL_UNHANDLEABLE;
> >> @@ -175,7 +203,7 @@ static int hvmemul_do_io(
> >> return X86EMUL_UNHANDLEABLE;
> >> }
> >>
> >> - vio->io_state =
> >> + vio->io_state = regs ? HVMIO_handle_vmport_awaiting_completion :
> >> (p_data == NULL) ? HVMIO_dispatched :
> HVMIO_awaiting_completion;
> >> vio->io_size = size;
> >>
> >> @@ -197,6 +225,9 @@ static int hvmemul_do_io(
> >> if ( rc == X86EMUL_UNHANDLEABLE )
> >> rc = hvm_buffered_io_intercept(&p);
> >> }
> >> + else if ( regs ) {
> >> + rc = X86EMUL_UNHANDLEABLE;
> >> + }
> >> else
> >> {
> >> rc = hvm_portio_intercept(&p);
> >> @@ -210,7 +241,7 @@ static int hvmemul_do_io(
> >> p.state = STATE_IORESP_READY;
> >> if ( !vio->mmio_retry )
> >> {
> >> - hvm_io_assist(&p);
> >> + hvm_io_assist(&p, vpp);
> >> vio->io_state = HVMIO_none;
> >> }
> >> else
> >> @@ -226,13 +257,19 @@ static int hvmemul_do_io(
> >> }
> >> else
> >> {
> >> - rc = X86EMUL_RETRY;
> >> - if ( !hvm_send_assist_req(&p) )
> >> + if ( regs )
> >> + rc = X86EMUL_VMPORT_RETRY;
> >> + else
> >> + rc = X86EMUL_RETRY;
> >> + if ( !hvm_send_assist_req(&p, vpp) )
> >> vio->io_state = HVMIO_none;
> >> else if ( p_data == NULL )
> >> rc = X86EMUL_OKAY;
> >> }
> >> break;
> >> + case X86EMUL_VMPORT_RETRY:
> >> + rc = X86EMUL_RETRY;
> >> + break;
> >> default:
> >> BUG();
> >> }
> > I still fail to see why you need to make such intrusive modifications to this
> code.
>
> I am confused. The code is doing what you say:
>
Ok, I guess I was confused as to why you had to pass the vmware_ioreq_t all the way into send_assist_req. My initial reading suggested that you were still overlaying the ioreq_t at that point, but re-reading, I can see you're copying it elsewhere. It would seem like the register info could be copied into the shared page much much earlier though so you would not need to modify send_assist_req at all.
My thinking is that, to get register info to QEMU, you could copy data into the shared page in hvmemul_do_vmport() and then basically just send an assist req with your new type. The code in hvm_io_assist could then be modified to take note if the ioreq type being completed, and if it detects your new type, it can then retrieve updated register info from the shared page and copy it back into the guest registers. So, no need for a new io state. Would that not work?
> > I would have thought you could add a new ioreq type, as you've done, but
> not make it 'data bearing'. I.e. you use your new VMPORT_IOREQ_PFN to
> carry the register values back and forth between Xen and QEMU, but you still
> issue a 'normal' ioreq_t structure (with your new type) via the 'normal'
> shared IOREQ_PFN. That way you need do nothing to the majority of the
> emulation code path - you'd just need to add code t copy the register values
> into and out of your new shared page at start and completion of I/O.
>
> I did adjust hvmemul_do_io() instead of duplicating it's code.
> hvmemul_do_io() cannot be used without adjustment because of
> the new type.
Well, I think duplicating whatever you actually need from hvmemul_do_io() would be better because I think, given what I've said above, you need so little of what it does.
Paul
>
> I will code this up with a new routine.
>
> -Don Slutz
>
>
> > Paul
> >
> >
>
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC][PATCH v2x prototype 1/1] Add IOREQ_TYPE_VMWARE_PORT
2014-10-14 9:57 ` Paul Durrant
@ 2014-10-14 19:06 ` Don Slutz
0 siblings, 0 replies; 25+ messages in thread
From: Don Slutz @ 2014-10-14 19:06 UTC (permalink / raw)
To: Paul Durrant, Don Slutz, xen-devel
Cc: Keir (Xen.org), Ian Campbell, Jan Beulich
On 10/14/14 05:57, Paul Durrant wrote:
>> -----Original Message-----
>> From: Don Slutz [mailto:dslutz@verizon.com]
>> Sent: 13 October 2014 18:11
>> To: Paul Durrant; Don Slutz; xen-devel@lists.xen.org
>> Cc: Jan Beulich; Keir (Xen.org); Ian Campbell
>> Subject: Re: [RFC][PATCH v2x prototype 1/1] Add
>> IOREQ_TYPE_VMWARE_PORT
>>
>> On 10/13/14 09:26, Paul Durrant wrote:
>>>> -----Original Message-----
>>>> From: Don Slutz [mailto:dslutz@verizon.com]
>>>> Sent: 09 October 2014 15:26
>>>> To: xen-devel@lists.xen.org; Paul Durrant
>>>> Cc: Jan Beulich; Keir (Xen.org); Ian Campbell; Don Slutz
>>>> Subject: [RFC][PATCH v2x prototype 1/1] Add
>> IOREQ_TYPE_VMWARE_PORT
>>>> This adds synchronisation of the 6 vcpu registers (only 32bits of
>>>> them) that vmport.c needs between Xen and QEMU.
>>>>
>>>> This is to avoid a 2nd and 3rd exchange between QEMU and Xen to
>>>> fetch and put these 6 vcpu registers used by the code in vmport.c
>>>> and vmmouse.c
>>>>
>>>> QEMU patch is named "xen-hvm.c: Add support for Xen access to
>> vmport"
>>>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>>>> ---
>>>> As requested by Paul Durrant <Paul.Durrant@citrix.com>
>>>>
>>>> Here is a prototype of the QEMU change using a 2nd shared page.
>>>> I picked adding HVM_PARAM_VMPORT_IOREQ_PFN as the simple and
>>>> fast way to handle QEMU building on older Xen versions.
>>>>
>>>> There is xentrace and debug logging that is TBD for the Xen 4.6
>>>> submission of this.
>> ...
>>>> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
>>>> index c0f47d2..4b8ea8f 100644
>>>> --- a/xen/arch/x86/hvm/emulate.c
>>>> +++ b/xen/arch/x86/hvm/emulate.c
>>>> @@ -52,12 +52,14 @@ static void hvmtrace_io_assist(int is_mmio, ioreq_t
>>>> *p)
>>>>
>>>> static int hvmemul_do_io(
>>>> int is_mmio, paddr_t addr, unsigned long *reps, int size,
>>>> - paddr_t ram_gpa, int dir, int df, void *p_data)
>>>> + paddr_t ram_gpa, int dir, int df, void *p_data,
>>>> + struct cpu_user_regs *regs)
>>>> {
>>>> struct vcpu *curr = current;
>>>> struct hvm_vcpu_io *vio;
>>>> ioreq_t p = {
>>>> - .type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
>>>> + .type = regs ? IOREQ_TYPE_VMWARE_PORT :
>>>> + is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
>>>> .addr = addr,
>>>> .size = size,
>>>> .dir = dir,
>>>> @@ -65,11 +67,15 @@ static int hvmemul_do_io(
>>>> .data = ram_gpa,
>>>> .data_is_ptr = (p_data == NULL),
>>>> };
>>>> + vmware_ioreq_t vp;
>>>> + vmware_ioreq_t *vpp;
>>>> unsigned long ram_gfn = paddr_to_pfn(ram_gpa);
>>>> p2m_type_t p2mt;
>>>> struct page_info *ram_page;
>>>> int rc;
>>>>
>>>> + BUILD_BUG_ON(sizeof(ioreq_t) < sizeof(vmware_ioreq_t));
>>>> +
>>>> /* Check for paged out page */
>>>> ram_page = get_page_from_gfn(curr->domain, ram_gfn, &p2mt,
>>>> P2M_UNSHARE);
>>>> if ( p2m_is_paging(p2mt) )
>>>> @@ -101,7 +107,17 @@ static int hvmemul_do_io(
>>>> return X86EMUL_UNHANDLEABLE;
>>>> }
>>>>
>>>> - if ( !p.data_is_ptr && (dir == IOREQ_WRITE) )
>>>> + if ( regs )
>>>> + {
>>>> + vpp = &vp;
>>>> + p.data = regs->rax;
>>>> + vp.ebx = regs->rbx;
>>>> + vp.ecx = regs->rcx;
>>>> + vp.edx = regs->rdx;
>>>> + vp.esi = regs->rsi;
>>>> + vp.edi = regs->rdi;
>>>> + }
>>>> + else if ( !p.data_is_ptr && (dir == IOREQ_WRITE) )
>>>> {
>>>> memcpy(&p.data, p_data, size);
>>>> p_data = NULL;
>>>> @@ -161,7 +177,19 @@ static int hvmemul_do_io(
>>>> put_page(ram_page);
>>>> return X86EMUL_RETRY;
>>>> }
>>>> + case HVMIO_awaiting_completion:
>>>> + if ( regs )
>>>> + {
>>>> + /* May have to wait for previous cycle of a multi-write to
>> complete.
>>>> */
>>>> + if ( vio->mmio_retry ) {
>>>> + gdprintk(XENLOG_WARNING, "WARNING: mmio_retry
>>>> io_state=%d?\n", vio->io_state);
>>>> + return X86EMUL_RETRY;
>>>> + }
>>>> + /* These are expected if we get here via hvmemul_do_io() */
>>>> + break;
>>>> + }
>>>> default:
>>>> + gdprintk(XENLOG_WARNING, "WARNING: io_state=%d?\n", vio-
>>>>> io_state);
>>>> if ( ram_page )
>>>> put_page(ram_page);
>>>> return X86EMUL_UNHANDLEABLE;
>>>> @@ -175,7 +203,7 @@ static int hvmemul_do_io(
>>>> return X86EMUL_UNHANDLEABLE;
>>>> }
>>>>
>>>> - vio->io_state =
>>>> + vio->io_state = regs ? HVMIO_handle_vmport_awaiting_completion :
>>>> (p_data == NULL) ? HVMIO_dispatched :
>> HVMIO_awaiting_completion;
>>>> vio->io_size = size;
>>>>
>>>> @@ -197,6 +225,9 @@ static int hvmemul_do_io(
>>>> if ( rc == X86EMUL_UNHANDLEABLE )
>>>> rc = hvm_buffered_io_intercept(&p);
>>>> }
>>>> + else if ( regs ) {
>>>> + rc = X86EMUL_UNHANDLEABLE;
>>>> + }
>>>> else
>>>> {
>>>> rc = hvm_portio_intercept(&p);
>>>> @@ -210,7 +241,7 @@ static int hvmemul_do_io(
>>>> p.state = STATE_IORESP_READY;
>>>> if ( !vio->mmio_retry )
>>>> {
>>>> - hvm_io_assist(&p);
>>>> + hvm_io_assist(&p, vpp);
>>>> vio->io_state = HVMIO_none;
>>>> }
>>>> else
>>>> @@ -226,13 +257,19 @@ static int hvmemul_do_io(
>>>> }
>>>> else
>>>> {
>>>> - rc = X86EMUL_RETRY;
>>>> - if ( !hvm_send_assist_req(&p) )
>>>> + if ( regs )
>>>> + rc = X86EMUL_VMPORT_RETRY;
>>>> + else
>>>> + rc = X86EMUL_RETRY;
>>>> + if ( !hvm_send_assist_req(&p, vpp) )
>>>> vio->io_state = HVMIO_none;
>>>> else if ( p_data == NULL )
>>>> rc = X86EMUL_OKAY;
>>>> }
>>>> break;
>>>> + case X86EMUL_VMPORT_RETRY:
>>>> + rc = X86EMUL_RETRY;
>>>> + break;
>>>> default:
>>>> BUG();
>>>> }
>>> I still fail to see why you need to make such intrusive modifications to this
>> code.
>>
>> I am confused. The code is doing what you say:
>>
> Ok, I guess I was confused as to why you had to pass the vmware_ioreq_t all the way into send_assist_req. My initial reading suggested that you were still overlaying the ioreq_t at that point, but re-reading, I can see you're copying it elsewhere. It would seem like the register info could be copied into the shared page much much earlier though so you would not need to modify send_assist_req at all.
I changed hvm_send_assist_req() so that this code was not limited to the
default ioreq server. I will code up one that does not change
hvm_send_assist_req().
> My thinking is that, to get register info to QEMU, you could copy data into the shared page in hvmemul_do_vmport() and then basically just send an assist req with your new type. The code in hvm_io_assist could then be modified to take note if the ioreq type being completed, and if it detects your new type, it can then retrieve updated register info from the shared page and copy it back into the guest registers. So, no need for a new io state. Would that not work?
>
I think it can. Will build a version that way.
-Don Slutz
>>> I would have thought you could add a new ioreq type, as you've done, but
>> not make it 'data bearing'. I.e. you use your new VMPORT_IOREQ_PFN to
>> carry the register values back and forth between Xen and QEMU, but you still
>> issue a 'normal' ioreq_t structure (with your new type) via the 'normal'
>> shared IOREQ_PFN. That way you need do nothing to the majority of the
>> emulation code path - you'd just need to add code t copy the register values
>> into and out of your new shared page at start and completion of I/O.
>>
>> I did adjust hvmemul_do_io() instead of duplicating it's code.
>> hvmemul_do_io() cannot be used without adjustment because of
>> the new type.
> Well, I think duplicating whatever you actually need from hvmemul_do_io() would be better because I think, given what I've said above, you need so little of what it does.
>
> Paul
>
>> I will code this up with a new routine.
>>
>> -Don Slutz
>>
>>
>>> Paul
>>>
>>>
>>
>>
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2014-10-14 19:06 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-02 18:36 [RFC][PATCH v2 0/1] Add support for Xen access to vmport Don Slutz
2014-10-02 18:36 ` [RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT Don Slutz
2014-10-02 20:33 ` Andrew Cooper
2014-10-02 21:56 ` Don Slutz
2014-10-02 22:23 ` Andrew Cooper
2014-10-03 9:47 ` Stefano Stabellini
2014-10-03 9:51 ` Ian Campbell
2014-10-03 9:54 ` Stefano Stabellini
2014-10-03 19:27 ` Don Slutz
2014-10-06 7:55 ` Jan Beulich
2014-10-06 9:21 ` Stefano Stabellini
2014-10-06 9:39 ` Jan Beulich
2014-10-06 19:51 ` Don Slutz
2014-10-07 8:05 ` Jan Beulich
2014-10-06 9:54 ` Paul Durrant
2014-10-03 9:29 ` Paul Durrant
2014-10-03 19:44 ` Don Slutz
2014-10-03 9:46 ` Paul Durrant
2014-10-03 19:56 ` [Qemu-devel] [Xen-devel] " Don Slutz
2014-10-09 14:26 ` [RFC][PATCH v2x prototype " Don Slutz
2014-10-13 13:26 ` Paul Durrant
2014-10-13 17:11 ` Don Slutz
2014-10-14 9:57 ` Paul Durrant
2014-10-14 19:06 ` Don Slutz
2014-10-03 19:56 ` [RFC][PATCH v2 " Don Slutz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.