[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v12 7/8] Add IOREQ_TYPE_VMWARE_PORT
- To: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
- From: Don Slutz <don.slutz@xxxxxxxxx>
- Date: Fri, 03 Jul 2015 10:55:34 -0400
- Cc: Tim Deegan <tim@xxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>, Keir Fraser <keir@xxxxxxx>, Ian Campbell <ian.campbell@xxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>, Jun Nakajima <jun.nakajima@xxxxxxxxx>, Eddie Dong <eddie.dong@xxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, Don Slutz <dslutz@xxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxx, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Aravind Gopalakrishnan <Aravind.Gopalakrishnan@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>, Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
- Delivery-date: Fri, 03 Jul 2015 14:55:50 +0000
- List-id: Xen developer discussion <xen-devel.lists.xen.org>
On 07/01/15 16:49, Konrad Rzeszutek Wilk wrote:
On Sat, Jun 27, 2015 at 07:27:44PM -0400, Don Slutz wrote:
From: Don Slutz <dslutz@xxxxxxxxxxx>
This adds synchronization of the 6 vcpu registers (only 32bits of
them) that vmport.c needs between Xen and QEMU.
...
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -169,33 +169,88 @@ static int hvmemul_do_io(
vio->io_state = HVMIO_handle_mmio_awaiting_completion;
break;
case X86EMUL_UNHANDLEABLE:
- {
- struct hvm_ioreq_server *s =
- hvm_select_ioreq_server(curr->domain, &p);
-
- /* If there is no suitable backing DM, just ignore accesses */
- if ( !s )
+ if ( vmport_check_port(p.addr, p.size) )
I would have made this !. Thought if Jan or Andrew said to make it that
way - then please ignore me.
I have not checked, but I do not remember any direct statement either
way. Last was Jan's "looks better".
That would mean you could also change the function to be 'is_port_vmport' or
such.
Happy to make a change here if needed. Note: This part of the patch
conflicts with:
From: Paul Durrant <paul.durrant@xxxxxxxxxx>
To: <xen-devel@xxxxxxxxxxxxxxxxxxxx>
Date: Tue, 30 Jun 2015 14:05:43 +0100
Message-ID: <1435669558-5421-2-git-send-email-paul.durrant@xxxxxxxxxx>
Subject: [Xen-devel] [PATCH v5 01/16] x86/hvm: make sure emulation is
retried if domain is shutting down
As so is waiting till that is sorted out.
{
- hvm_complete_assist_req(&p);
- rc = X86EMUL_OKAY;
- vio->io_state = HVMIO_none;
...
+
+ vio->io_state = HVMIO_handle_pio_awaiting_completion;
+ rc = hvm_send_assist_req(s, &p);
+ if ( rc != X86EMUL_RETRY )
+ {
+ vio->io_state = HVMIO_none;
+ if ( rc == X86EMUL_OKAY )
+ rc = X86EMUL_RETRY;
+ }
+ }
}
break;
- }
default:
BUG();
}
if ( rc != X86EMUL_OKAY )
+ {
+ /*
+ * If rc is X86EMUL_RETRY and vio->io_state is
+ * HVMIO_handle_pio_awaiting_completion, then were are of
were are? Perhaps 'were'?
+ * type IOREQ_TYPE_VMWARE_PORT, so completion in
+ * hvm_io_assist() with no re-emulation required
.. is required.
Ok. Plan to change.
With commit 2df1aa01 (and the result of Paul Durrant's change) I was
considering
just adding a "return X86EMUL_OKAY" as the else of the check on rc
above, and therefore drop the need for this check and comment.
+ */
+ if ( rc == X86EMUL_RETRY &&
+ vio->io_state == HVMIO_handle_pio_awaiting_completion )
+ rc = X86EMUL_OKAY;
return rc;
+ }
finish_access:
if ( dir == IOREQ_READ )
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index f8ec170..ce8cd09 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -394,6 +394,47 @@ static ioreq_t *get_ioreq(struct hvm_ioreq_server *s,
struct vcpu *v)
return &p->vcpu_ioreq[v->vcpu_id];
}
+static vmware_regs_t *get_vmport_regs_one(struct hvm_ioreq_server *s,
+ struct vcpu *v)
+{
+ struct hvm_ioreq_vcpu *sv;
+
+ list_for_each_entry ( sv,
+ &s->ioreq_vcpu_list,
+ list_entry )
Could it be just made it one nice line?
I think so, so plan to change.
.. snip..
@@ -2507,6 +2637,13 @@ struct hvm_ioreq_server *hvm_select_ioreq_server(struct
domain *d,
}
break;
+ case IOREQ_TYPE_VMWARE_PORT:
+ case IOREQ_TYPE_TIMEOFFSET:
+ /* The 'special' range of [1,1] is checked for being enabled */
Missing full stop.
Will add.
+ if ( rangeset_contains_singleton(r, 1) )
+ return s;
+
+ break;
}
}
@@ -2669,6 +2806,7 @@ void hvm_complete_assist_req(ioreq_t *p)
case IOREQ_TYPE_PCI_CONFIG:
ASSERT_UNREACHABLE();
break;
+ case IOREQ_TYPE_VMWARE_PORT:
case IOREQ_TYPE_COPY:
case IOREQ_TYPE_PIO:
if ( p->dir == IOREQ_READ )
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index c0964ec..c1379bf 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -192,6 +192,22 @@ void hvm_io_assist(ioreq_t *p)
(void)handle_mmio();
break;
case HVMIO_handle_pio_awaiting_completion:
+ if ( p->type == IOREQ_TYPE_VMWARE_PORT )
+ {
+ vmware_regs_t *vr = get_vmport_regs_any(NULL, curr);
+
+ if ( vr )
+ {
+ struct cpu_user_regs *regs = guest_cpu_user_regs();
+
+ /* Only change the 32bit part of the register */
Missing full stop.
Will add.
+ regs->_ebx = vr->ebx;
+ regs->_ecx = vr->ecx;
+ regs->_edx = vr->edx;
+ regs->_esi = vr->esi;
+ regs->_edi = vr->edi;
+ }
+ }
if ( vio->io_size == 4 ) /* Needs zero extension. */
guest_cpu_user_regs()->rax = (uint32_t)p->data;
else
diff --git a/xen/arch/x86/hvm/vmware/vmport.c b/xen/arch/x86/hvm/vmware/vmport.c
index f24d8e3..5e14402 100644
--- a/xen/arch/x86/hvm/vmware/vmport.c
+++ b/xen/arch/x86/hvm/vmware/vmport.c
@@ -137,6 +137,19 @@ void vmport_register(struct domain *d)
register_portio_handler(d, BDOOR_PORT, 4, vmport_ioport);
}
+int vmport_check_port(unsigned int port, unsigned int bytes)
+{
+ struct vcpu *curr = current;
+ struct domain *currd = curr->domain;
+
+ if ( port + bytes > BDOOR_PORT && port < BDOOR_PORT + 4 &&
+ is_hvm_domain(currd) &&
+ currd->arch.hvm_domain.is_vmware_port_enabled ) {
The '{' should be on its own line.
Will fix.
+ return 0;
+ }
+ return 1;
+}
+
...
+struct shared_vmport_iopage {
+ struct vmware_regs vcpu_vmport_regs[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 7c73089..130eba9 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. */
s/Extra/optional/ ?
I do not think "optional" is good to use here. This PFN is required for
QEMU (or any other ioreq server) to handle vmport access. If you have a
better name then "extra", please let me know.
-Don Slutz
+#define HVM_PARAM_VMPORT_REGS_PFN 35
#define HVM_PARAM_BUFIOREQ_PFN 6
#define HVM_PARAM_BUFIOREQ_EVTCHN 26
@@ -187,6 +189,6 @@
/* Location of the VM Generation ID in guest physical address space. */
#define HVM_PARAM_VM_GENERATION_ID_ADDR 34
-#define HVM_NR_PARAMS 35
+#define HVM_NR_PARAMS 36
#endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
