qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Marcel Apfelbaum
Subject: Re: [Qemu-devel] [PATCH V2 0/4] hw/pcie: Multi-root support for Q35
Date: Tue, 17 Nov 2015 15:49:00 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0

On 11/17/2015 02:26 PM, Markus Armbruster wrote:
Marcel Apfelbaum <address@hidden> writes:

On 11/17/2015 10:15 AM, Markus Armbruster wrote:
Marcel Apfelbaum <address@hidden> 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?

First, we need to decide whether they're deprecated or legacy.

Deprecated means their use is discouraged and they may go away
eventually.  Users are advised to switch to the replacement.

Legacy means we keep them around just for compatibility.  Existing uses
are just fine, but new use is discouraged.

We usually remove deprecated interfaces from help and documentation.
Sometimes we mark them deprecated for a while before we remove them.

Legacy interfaces stay documented, but should be clearly marked.

Right now, -device help shows all pluggable devices, i.e. the devices
that don't have cannot_instantiate_with_device_add_yet set.  In other
words, there's no way to suppress deprecated devices, yet.

Two ways to mark them:

1. Set DeviceClass member desc to a string containing "(deprecated)" or
    "(legacy)", respectively.

2. Create a new DeviceCategory, and move the device there.

I prefer 2.  You can easily turn it into a way to suppress deprecated
devices: just skip the "deprecated" category.

Thanks Markus for the detailed information!


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.

Trust your judgement :)

Re-thinking this whole thing, I have a new plan :)

We have the pxb device which is working just fine. Having the internal 
pci-bridge
by default is a good idea (hot-plug support, smaller ACPI tables).

Adding a new device, say "root-complex", that re-uses some of the pxb code
is the best way to go, and it will be easier to maintain when those devices
will diverge.

Thanks,
Marcel


[...]





reply via email to

[Prev in Thread] Current Thread [Next in Thread]