| Message ID | 1610488352-18494-6-git-send-email-olekstysh@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | IOREQ feature (+ virtio-mmio) on Arm | expand |
Hi Oleksandr, On 12/01/2021 21:52, Oleksandr Tyshchenko wrote: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > The IOREQ is a common feature now and this helper will be used > on Arm as is. Move it to xen/ioreq.h and remove "hvm" prefix. > > Although PIO handling on Arm is not introduced with the current series > (it will be implemented when we add support for vPCI), technically > the PIOs exist on Arm (however they are accessed the same way as MMIO) > and it would be better not to diverge now. > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > Reviewed-by: Paul Durrant <paul@xen.org> > Acked-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Julien Grall <jgrall@amazon.com> Cheers,
Oleksandr Tyshchenko <olekstysh@gmail.com> writes: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > The IOREQ is a common feature now and this helper will be used > on Arm as is. Move it to xen/ioreq.h and remove "hvm" prefix. > > Although PIO handling on Arm is not introduced with the current series > (it will be implemented when we add support for vPCI), technically > the PIOs exist on Arm (however they are accessed the same way as MMIO) > and it would be better not to diverge now. I find this description a little confusing. When you say PIO do you mean using instructions like in/out on the x86? If so then AFAIK it's a legacy feature of x86 as everything I've come across since just does MMIO, including PCI. The code changes look fine to me though: Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Hi Alex, On 20/01/2021 08:48, Alex Bennée wrote: > > Oleksandr Tyshchenko <olekstysh@gmail.com> writes: > >> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> >> The IOREQ is a common feature now and this helper will be used >> on Arm as is. Move it to xen/ioreq.h and remove "hvm" prefix. >> >> Although PIO handling on Arm is not introduced with the current series >> (it will be implemented when we add support for vPCI), technically >> the PIOs exist on Arm (however they are accessed the same way as MMIO) >> and it would be better not to diverge now. > > I find this description a little confusing. When you say PIO do you mean > using instructions like in/out on the x86? If so then AFAIK it's a > legacy feature of x86 as everything I've come across since just does > MMIO, including PCI. Stefano and I had quite a long discussion about this a few months ago (see [1]). From my understanding, while Arm will access the PCI I/O BAR via MMIO, the BAR itself will be configured using an offset from a fixed I/O window base address. IOW, we don't configure the BAR with a full MMIO address. In the case the hostbridge is emulated in Xen, I would like to re-use the TYPE_PIO for such access because it makes the device model arch-agnostic. I believe this would behave the same way as a real PCI device card: you can plug it anywhere without having to understand the underlying architecture. If we were going to use the MMIO type, then we would need: 1) Inform each device model where is the I/O Window (necessary to be able to know we are accessing the I/O BAR) 2) Have arch boiler plate in the device model > > The code changes look fine to me though: > > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > Cheers, [1] <4cbe37bd-abd2-3d02-535e-cca6f7497ef2@xen.org>
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index 60ca465..c3487b5 100644 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -336,7 +336,7 @@ static int hvmemul_do_io( rc = hvm_send_ioreq(s, &p, 0); if ( rc != X86EMUL_RETRY || currd->is_shutting_down ) vio->io_req.state = STATE_IOREQ_NONE; - else if ( !hvm_ioreq_needs_completion(&vio->io_req) ) + else if ( !ioreq_needs_completion(&vio->io_req) ) rc = X86EMUL_OKAY; } break; @@ -2649,7 +2649,7 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt, if ( rc == X86EMUL_OKAY && vio->mmio_retry ) rc = X86EMUL_RETRY; - if ( !hvm_ioreq_needs_completion(&vio->io_req) ) + if ( !ioreq_needs_completion(&vio->io_req) ) completion = HVMIO_no_completion; else if ( completion == HVMIO_no_completion ) completion = (vio->io_req.type != IOREQ_TYPE_PIO || diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c index 11e007d..ef8286b 100644 --- a/xen/arch/x86/hvm/io.c +++ b/xen/arch/x86/hvm/io.c @@ -135,7 +135,7 @@ bool handle_pio(uint16_t port, unsigned int size, int dir) rc = hvmemul_do_pio_buffer(port, size, dir, &data); - if ( hvm_ioreq_needs_completion(&vio->io_req) ) + if ( ioreq_needs_completion(&vio->io_req) ) vio->io_completion = HVMIO_pio_completion; switch ( rc ) diff --git a/xen/common/ioreq.c b/xen/common/ioreq.c index 8a004c4..47e38b6 100644 --- a/xen/common/ioreq.c +++ b/xen/common/ioreq.c @@ -160,7 +160,7 @@ static bool hvm_wait_for_io(struct hvm_ioreq_vcpu *sv, ioreq_t *p) } p = &sv->vcpu->arch.hvm.hvm_io.io_req; - if ( hvm_ioreq_needs_completion(p) ) + if ( ioreq_needs_completion(p) ) p->data = data; sv->pending = false; @@ -186,7 +186,7 @@ bool handle_hvm_io_completion(struct vcpu *v) if ( sv && !hvm_wait_for_io(sv, get_ioreq(s, v)) ) return false; - vio->io_req.state = hvm_ioreq_needs_completion(&vio->io_req) ? + vio->io_req.state = ioreq_needs_completion(&vio->io_req) ? STATE_IORESP_READY : STATE_IOREQ_NONE; msix_write_completion(v); diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h index 5ccd075..6c1feda 100644 --- a/xen/include/asm-x86/hvm/vcpu.h +++ b/xen/include/asm-x86/hvm/vcpu.h @@ -91,13 +91,6 @@ struct hvm_vcpu_io { const struct g2m_ioport *g2m_ioport; }; -static inline bool hvm_ioreq_needs_completion(const ioreq_t *ioreq) -{ - return ioreq->state == STATE_IOREQ_READY && - !ioreq->data_is_ptr && - (ioreq->type != IOREQ_TYPE_PIO || ioreq->dir != IOREQ_WRITE); -} - struct nestedvcpu { bool_t nv_guestmode; /* vcpu in guestmode? */ void *nv_vvmcx; /* l1 guest virtual VMCB/VMCS */ diff --git a/xen/include/xen/ioreq.h b/xen/include/xen/ioreq.h index 7b67950..750d884 100644 --- a/xen/include/xen/ioreq.h +++ b/xen/include/xen/ioreq.h @@ -21,6 +21,13 @@ #include <xen/sched.h> +static inline bool ioreq_needs_completion(const ioreq_t *ioreq) +{ + return ioreq->state == STATE_IOREQ_READY && + !ioreq->data_is_ptr && + (ioreq->type != IOREQ_TYPE_PIO || ioreq->dir != IOREQ_WRITE); +} + #define HANDLE_BUFIOREQ(s) \ ((s)->bufioreq_handling != HVM_IOREQSRV_BUFIOREQ_OFF)