Re: [PATCH v2 6/6] xen_arm: Add virtual PCIe host bridge support

[Volodymyr Babchuk]

Hi Vikram,

Vikram Garhwal <vikram.garhwal@amd.com> writes:

> Hi Volodymyr,
> Thank you sharing this patch. I have few comments below
> On Wed, Nov 22, 2023 at 02:39:46PM -0800, Stefano Stabellini wrote:
>> +Vikram
>> 
>> On Tue, 21 Nov 2023, Volodymyr Babchuk wrote:
>> > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>> > 
>> > The bridge is needed for virtio-pci support, as QEMU can emulate the
>> > whole bridge with any virtio-pci devices connected to it.
>> > 
>> > This patch provides a flexible way to configure PCIe brige resources
>> > with xenstore. We made this for several reasons:
>> > 
>> > - We don't want to clash with vPCI devices, so we need information
>> >   from Xen toolstack on which PCI bus to use.
>> > - The guest memory layout that describes these resources is not stable
>> >   and may vary between guests, so we cannot rely on static resources
>> >   to be always the same for both ends.
>> > - Also the device-models which run in different domains and serve
>> >   virtio-pci devices for the same guest should use different host
>> >   bridge resources for Xen to distinguish. The rule for the guest
>> >   device-tree generation is one PCI host bridge per backend domain.
>> > 
>> > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>> > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>> 
>> There is still a discussion ongoing on xen-devel if / how to register a
>> PCI Root Complex or individual BDFs. In the meantime a couple of
>> comments.
>> 
>> Typically emulated devices are configured in QEMU via QEMU command line
>> parameters, not xenstore. If you are running QEMU in a domU (instead of
>> Dom0) you can always read config parameters from xenstore from a wrapper
>> bash script (using xenstore-read) and then pass the right command line
>> options to QEMU.
>> 
>> If you need help in adding new QEMU command line options, Vikram (CCed)
>> can help.
>> 
>> 
> I agree with Stefano here. Setting properties would be better and easier.

Okay, I'll look at this.

> I have one questions here:
> 1. If there are multiple QEMU backends, which means xen tools will end up
> creating lot of entries in xenstore and we need to remove those xenstore
> entries when backend goes away. Is this already handled in Xen tools?

Well, this is not a classic PV backend, so common code in Xen Tools does
not handle those entries. I am not sure if tools remove entries right
now, because I am not the original author. But we definitely will remove
them in the final version of patches.

-- 
WBR, Volodymyr