From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60857) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZygdB-0000nT-RN for qemu-devel@nongnu.org; Tue, 17 Nov 2015 08:49:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zygd6-0006tY-KC for qemu-devel@nongnu.org; Tue, 17 Nov 2015 08:49:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40211) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zygd6-0006tS-9J for qemu-devel@nongnu.org; Tue, 17 Nov 2015 08:49:04 -0500 References: <1447601946-31248-1-git-send-email-marcel@redhat.com> <5649966C.7070702@redhat.com> <5649A770.5070908@redhat.com> <5649A857.406@redhat.com> <5649A9CA.2070309@redhat.com> <5649A9F9.5070405@redhat.com> <5649AB7F.9030702@redhat.com> <5649ABDF.1010901@redhat.com> <5649B123.1050507@redhat.com> <87h9kl2g99.fsf@blackfin.pond.sub.org> <564B04A2.9070007@redhat.com> <87a8qcx15z.fsf@blackfin.pond.sub.org> From: Marcel Apfelbaum Message-ID: <564B304C.4000608@redhat.com> Date: Tue, 17 Nov 2015 15:49:00 +0200 MIME-Version: 1.0 In-Reply-To: <87a8qcx15z.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V2 0/4] hw/pcie: Multi-root support for Q35 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: ehabkost@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, kraxel@redhat.com, Paolo Bonzini , imammedo@redhat.com, rth@twiddle.net On 11/17/2015 02:26 PM, Markus Armbruster wrote: > Marcel Apfelbaum writes: > >> On 11/17/2015 10:15 AM, Markus Armbruster wrote: >>> Marcel Apfelbaum 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 > > [...] >