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

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

On 01/19/2016 10:04 AM, Marcel Apfelbaum wrote:
> 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 <marcel@redhat.com> 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 <marcel@redhat.com>
>>>> ---
>>>>
>>>> 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.

Ping. Can we merge this patch?

Thanks,
Marcel


>
>>
>> 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
>>
>
>