Re: [RFC PATCH 2/6] xen/public: arch-arm: reserve resources for virtio-pci

[Julien Grall <julien@xxxxxxx>]

Hi Oleksandr,

On 15/11/2023 18:14, Oleksandr Tyshchenko wrote:
> On 15.11.23 19:31, Julien Grall wrote:
> > On 15/11/2023 16:51, Oleksandr Tyshchenko wrote:
> > > On 15.11.23 14:33, Julien Grall wrote:
> > > > Thanks for adding support for virtio-pci in Xen. I have some questions.
> > > >
> > > > On 15/11/2023 11:26, Sergiy Kibrik wrote:
> > > > > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > > > >
> > > > > In order to enable more use-cases such as having multiple
> > > > > device-models (Qemu) running in different backend domains which provide
> > > > > virtio-pci devices for the same guest, we allocate and expose one
> > > > > PCI host bridge for every virtio backend domain for that guest.
> > > > OOI, why do you need to expose one PCI host bridge for every stubdomain?
> > > >
> > > > In fact looking at the next patch, it seems you are handling some of the
> > > > hostbridge request in Xen. This is adds a bit more confusion.
> > > >
> > > > I was expecting the virtual PCI device would be in the vPCI and each
> > > > Device emulator would advertise which BDF they are covering.
> > >
> > > This patch series only covers use-cases where the device emulator
> > > handles the *entire* PCI Host bridge and PCI (virtio-pci) devices behind
> > > it (i.e. Qemu). Also this patch series doesn't touch vPCI/PCI
> > > pass-through resources, handling, accounting, nothing.
> > I understood you want to one Device Emulator to handle the entire PCI
> > host bridge. But...
> >
> >   From the
> > > hypervisor we only need a help to intercept the config space accesses
> > > happen in a range [GUEST_VIRTIO_PCI_ECAM_BASE ...
> > > GUEST_VIRTIO_PCI_ECAM_BASE + GUEST_VIRTIO_PCI_TOTAL_ECAM_SIZE] and
> > > forward them to the linked device emulator (if any), that's all.
> > ... I really don't see why you need to add code in Xen to trap the
> > region. If QEMU is dealing with the hostbridge, then it should be able
> > to register the MMIO region and then do the translation.
>
> Hmm, sounds surprising I would say. Are you saying that unmodified Qemu
> will work if we drop #5?
I don't know if an unmodified QEMU will work. My point is I don't view 
the patch in Xen necessary. You should be able to tell QEMU "here is the 
ECAM region, please emulate an hostbridge". QEMU will then register the 
region to Xen and all the accesses will be forwarded.
In the future we may need a patch similar to #5 if we want to have 
multiple DM using the same hostbridge. But this is a different 
discussion and the patch would need some rework.
The ioreq.c code was always meant to be generic and is always for every 
emulated MMIO. So you want to limit any change in it. Checking the MMIO 
region belongs to the hostbridge and doing the translation is IMHO not a 
good idea to do in ioreq.c. Instead you want to do the conversion from 
MMIO to (sbdf, offset) in virtio_pci_mmio{read, write}(). So the job of 
ioreq.c is to simply find the correct Device Model and forward it.
I also don't see why the feature is gated by has_vcpi(). They are two 
distinct features (at least in your current model).
Cheers,

--
Julien Grall