[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC PATCH 2/6] xen/public: arch-arm: reserve resources for virtio-pci
Hi Oleksandr, On 15/11/2023 16:51, Oleksandr Tyshchenko wrote: On 15.11.23 14:33, Julien Grall wrote:Hi,Hello Julien Let me please try to explain some bits.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/PCIpass-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. It is not possible (with current series) to run device emulators what emulate only separate PCI (virtio-pci) devices. For it to be possible, I think, much more changes are required than current patch series does. There at least should be special PCI Host bridge emulation in Xen (or reuse vPCI) for the integration. Also Xen should be in charge of forming resulting PCI interrupt based on each PCI device level signaling (if we use legacy interrupts), some kind of x86's XEN_DMOP_set_pci_intx_level, etc. Please note, I am not saying this is not possible in general, likely it is possible, but initial patch series doesn't cover these use-cases) We expose one PCI host bridge per virtio backend domain. This is a separate PCI host bridge to combine all virtio-pci devices running in the same backend domain (in the same device emulator currently). The examples: - if only one domain runs Qemu which servers virtio-blk, virtio-net, virtio-console devices for DomU - only single PCI Host bridge will be exposed for DomU - if we add another domain to run Qemu to serve additionally virtio-gpu, virtio-input and virtio-snd for the *same* DomU - we expose second PCI Host bridge for DomU I am afraid, we cannot end up exposing only single PCI Host bridge with current model (if we use device emulators running in different domains that handles the *entire* PCI Host bridges), this won't work. That makes sense and it is fine. But see above, I think only the #2 is necessary for the hypervisor. Patch #5 should not be necessary at all. [...] Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx> --- xen/include/public/arch-arm.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h index a25e87dbda..e6c9cd5335 100644 --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -466,6 +466,19 @@ typedef uint64_t xen_callback_t; #define GUEST_VPCI_MEM_ADDR xen_mk_ullong(0x23000000) #define GUEST_VPCI_MEM_SIZE xen_mk_ullong(0x10000000) +/* + * 16 MB is reserved for virtio-pci configuration space based on calculation + * 8 bridges * 2 buses x 32 devices x 8 functions x 4 KB = 16 MBCan you explain how youd ecided the "2"?good question, we have a limited free space available in memory layout (we had difficulties to find a suitable holes) also we don't expect a lot of virtio-pci devices, so "256" used vPCI would be too much. It was decided to reduce significantly, but select maximum to fit into free space, with having "2" buses we still fit into the chosen holes. If you don't expect a lot of virtio devices, then why do you need two buses? Wouldn't one be sufficient? + */ +#define GUEST_VIRTIO_PCI_ECAM_BASE xen_mk_ullong(0x33000000) +#define GUEST_VIRTIO_PCI_TOTAL_ECAM_SIZE xen_mk_ullong(0x01000000) +#define GUEST_VIRTIO_PCI_HOST_ECAM_SIZE xen_mk_ullong(0x00200000) + +/* 64 MB is reserved for virtio-pci memory */ +#define GUEST_VIRTIO_PCI_ADDR_TYPE_MEM xen_mk_ullong(0x02000000) +#define GUEST_VIRTIO_PCI_MEM_ADDR xen_mk_ullong(0x34000000) +#define GUEST_VIRTIO_PCI_MEM_SIZE xen_mk_ullong(0x04000000) + /* * 16MB == 4096 pages reserved for guest to use as a region to map its * grant table in. @@ -476,6 +489,11 @@ typedef uint64_t xen_callback_t; #define GUEST_MAGIC_BASE xen_mk_ullong(0x39000000) #define GUEST_MAGIC_SIZE xen_mk_ullong(0x01000000) +/* 64 MB is reserved for virtio-pci Prefetch memory */This doesn't seem a lot depending on your use case. Can you details how you can up with "64 MB"?the same calculation as it was done configuration space. It was observed that only 16K is used per virtio-pci device (maybe it can be bigger for usual PCI device, I don't know). Please look at the example of DomU log below (to strings that contain "*BAR 4: assigned*"): What about virtio-gpu? I would expect a bit more memory is necessary for that use case. Any case, what I am looking for is for some explanation in the commit message of the limits. I don't particularly care about the exact limit because this is not part of a stable ABI. Cheers, -- Julien Grall
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |