[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH V4 05/24] xen/ioreq: Make x86's hvm_ioreq_needs_completion() common



Hi Alex,

On 20/01/2021 08:48, Alex Bennée wrote:

Oleksandr Tyshchenko <olekstysh@xxxxxxxxx> writes:

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>

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@xxxxxxxxxx>


Cheers,

[1] <4cbe37bd-abd2-3d02-535e-cca6f7497ef2@xxxxxxx>


--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.