Re: [Qemu-devel] [RFC v2 1/3] pci_expander_bridge: add type TYPE_PXB_PCIE_HOST

[Marcel Apfelbaum <marcel.apfelbaum@gmail.com>]

On 06/13/2018 11:23 AM, Zihan Yang wrote:
> Michael S. Tsirkin <mst@redhat.com> 于2018年6月12日周二 下午9:43写道：
>> On Tue, Jun 12, 2018 at 05:13:22PM +0800, Zihan Yang wrote:
>>> The inner host bridge created by pxb-pcie is TYPE_PXB_PCI_HOST by default,
>>> add a new type TYPE_PXB_PCIE_HOST to better utilize the ECAM of PCIe
>>>
>>> Signed-off-by: Zihan Yang <whois.zihan.yang@gmail.com>
>> I have a concern that there are lots of new properties
>> added here, I'm not sure how are upper layers supposed to
>> manage them all.
>>
>> E.g. bus_nr supplied in several places, domain_nr for which
>> it's not clear how it is supposed to be allocated, etc.
> Indeed they seem to double the properties, but the pxb host is
> an internal structure of pxb-pcie device, created in pxb-pcie's
> realization procedure, and acpi-build queries host bridges instead
> of pxb-pcie devices. This means that users can not directly specify
> the property of pxb host bridge, but must 'inherit' from pxb-pcie
> devices. I had thought about changing the acpi-build process,
> but that would require more modifications.
>
> As for the properties, bus_nr means the start bus number
> of this host bridge. It is used when pxb-pcie is in pci domain 0
> with q35 host to avoid bus number confliction. When it is placed
> in a separate pci domain, it is not used and should be 0.
>
> max_bus means how many buses the user desires, EACH bus in
> PCIe requires 1MB configuration space, thus specifying it could
> reduce the reserved memory in MMCFG as suggested by Marcel.

The max_bus property is optional, you set the default to 255.
I am wondering if 255 is too much as a default for an extra bus,
I would use a smaller value, like 10.

> Typically, the user can specify
>
> -device pxb-pcie,id=br1,bus="pcie.0",sep_domain=on,domain_nr=1,max_bus=130
>
> this will place the buses under this pxb host bridge in pci domain
> 1, and reserve (130 + 1) = 131 buses for it. The start bus number
> is always 0 currently for simplicity.
>
>> Can the management interface be simplified?
>> Ideally we wouldn't have to teach libvirt new tricks,
>> just generalize pxb support slightly.
> We can delete 'sep_domain' property, I just find 'domain_nr'
> already indicates domain number.

Agreed, please remove sep_domain property.

Thanks,


> But domain_nr and
> max_bus seems unremovable, although they look 'redundant'
> because they appear twice.
>
> I'm not familiar with libvirt, but from the perspective of user,
> only 2 properties are added(domain_nr and max_bus, if we
> delete sep_domain), though the internal structure actually has
> changed.