Re: [Qemu-devel] [PATCH] hw/pci: ensure that only PCI/PCIe bridges can be attached to pxb/pxb-pcie devices

[Marcel Apfelbaum]

On 01/18/2016 08:16 PM, Laszlo Ersek wrote:
> On 01/18/16 19:08, Peter Maydell wrote:
> > On 18 January 2016 at 15:27, Marcel Apfelbaum <address@hidden> wrote:
> > > PCI devices can't be plugged directly into PCI extra root bridges
> > > because their resources can't be computed by firmware before the ACPI
> > > tables are loaded.
> > >
> > > Signed-off-by: Marcel Apfelbaum <address@hidden>
> > > ---
> > >
> > > Hi,
> > >
> > > This patch follows the discussion:
> > > https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg01484.html
> >
> > Is it definitely the case that no current working command lines plug
> > PCI devices directly into these things (including on platforms that
> > don't have anything to do with ACPI at all) ?

Hi,

The PXB devices can work only on ACPI based platforms, but currently work only 
on PC Machines.
So for other platforms are out of the scope.

I understand the issue in putting it generic PCI code, but:
 - Non ACPI platforms (implemented in QEMU) do not support extra PCI host 
bridges (at least yet)
 - Even when extra host bridges will be supported, there are are several ways 
to implement it
   and most of them will not require their pxbs to have a parent_device. The 
presence of a parent device
   is a pretty solid lead that is a "snooping bridge" and as far as I know is 
only typical for the existing solution.

Now the explanation of the issue we want to solve:
 - pxb (PCI expander bridge) - it already has an internal bridge, using
       -device pxb,bus80,id=pxb1 -device e1000,bus=pxb1
   will land the device on a built-in pci bridge.
   - An incorrect command-line will result in a non working device without the 
proposed patch.
 - pxb-pcie (PCIe Root Complex) - it does not have an internal bridge and 
trying to use:
        -device pxb-pcie,bus80,id=pxb1 -device e1000,bus=pxb1
   will fail.

This patch ensures non of that can happen.

Last word:
I did consider another option, adding a "bridges-only" property (defaulted to 
false) to PCIBus class
and leverage the fact that the pxb internal buses derive from it(and it can be 
set to true).

Then we can simply check PCI_BUS_CLASS(bus)->bridges-only but it seemed a 
little odd since we
don't have that limitation on the real world.
I am not against it, if it is preferred I'll submit a new patch.

> No clue about "pxb-pcie", but re: "pxb", the documentation and examples
> by Marcel (see: "docs/pci_expander_bridge.txt") will certainly continue
> working, with this patch place. And, that text file is authoritative for
> pxb, since Marcel (et al) wrote the code directly for the purposes
> described in the txt.

and that reminds me I need to update the doc for pxb-pcie, thanks Laszlo!
Marcel


> (But I'll let Marcel answer too! :))
>
> Thanks
> Laszlo