Re: [Qemu-devel] [PATCH V2 0/4] hw/pcie: Multi-root support for Q35

[Marcel Apfelbaum <marcel@redhat.com>]

On 11/17/2015 10:15 AM, Markus Armbruster wrote:
> Marcel Apfelbaum <marcel@redhat.com> writes:
>
>> On 11/16/2015 12:11 PM, Paolo Bonzini wrote:
>>>
>>>
>>> On 16/11/2015 11:10, Marcel Apfelbaum wrote:
>>>>> What would you lose?  Hotplug?
>>>>
>>>> Without the bridge? Yes. However the user can add it manually the
>>>> pci-bridge and have it anyway.
>>>
>>> Ok, I guess that's more or less acceptable.  It's still ugly however, to
>>> the point that I wonder if we should rename the device and call the old
>>> one a failed experiment.
>>>
>>
>> I guess we can rename the pxb to extra-root or something, but in this way
>> will have a deprecated/duplicated device to support and kill in the future.
>>
>> Why not use the compat property as it is?
>> Again, the command line *remains* the same, the difference is where the
>> devices associated with the pxb will land: on the secondary bus (for QEMU < 2.5)
>> or on the root bus itself (QEMU >= 2.5).
>>
>> I know is guest visible, but the guest will see one of them depending
>> on the machine type.
>>
>> Regarding the splitting of pxb into 2 devices (pci/pcie), I have
>> nothing against it,
>> but because the implementation is *exactly* the same I think we should gain more
>> by maintaining one device.
>
> I have no opinion on two devices vs. one device + property in this
> particular case, I just want to interject that I'd expect the difference
> in maintaince to be negligible.
>
> A second device basically takes a copy of the TypeInfo with some
> (trivial) init function to make it different.  Might be a few more lines
> of code than adding a property, but in complexity, it's a wash.
>
> In case you plan to get rid of the old variant: with two devices, you
> deprecate and later delete the old device.  With device + property, you
> deprecate setting the property, and later delete it.  The former might
> be a bit easier to document.

Hi Markus,
Thank you for the review.

Following all the comments I have received, I am going to:

1. Leave the old device (pxb) as is and mark it as deprecated. Maybe
you can point me on the cleanest way to mark a device as deprecated?

2. Create a new device (pci-expander) that will behave as a:
    - PCI root, if the machine's bus 0 is legacy PCI.
    - PCI express root (Root Complex), if machine's bus 0 is PCI Express.

This way I will not need two device and not even a property for (pci/pcie).
We don't really need an extra PCI root on a PCIe machine and vice versa.

By the way, following the same concept I converted the virtio devices to PCIe devices.
I didn't want a new set of devices.

Thanks,
Marcel

>
>>>> I wanted to get rid of the internal pci-bridge as a default, and this
>>>> is why pxb and pxb-pcie are he same device now (except bus type)