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 <dsl...@verizon.com>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.durr...@citrix.com>
To: <xen-de...@lists.xenproject.org>
Date: Tue, 30 Jun 2015 14:05:43 +0100
Message-ID: <1435669558-5421-2-git-send-email-paul.durr...@citrix.com>
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 ofwere 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@lists.xen.org http://lists.xen.org/xen-devel
