Re: [Qemu-devel] [PATCH 1/1] Fix device listener interface for PCI to PCI bridges

[Don Slutz]

On 05/28/15 17:05, Michael S. Tsirkin wrote:
> On Thu, May 28, 2015 at 11:03:07PM +0200, Michael S. Tsirkin wrote:
>> On Thu, May 28, 2015 at 03:09:48PM -0400, Don Slutz wrote:
>>> On 05/28/15 08:28, Michael S. Tsirkin wrote:
>>>> On Thu, May 28, 2015 at 07:25:50AM -0400, Don Slutz wrote:
>>>>> On 05/28/15 05:30, Michael S. Tsirkin wrote:
>>>>>> On Thu, May 28, 2015 at 04:46:37AM -0400, Don Slutz wrote:
>>>>>>> The commit 707ff80021ccd7a68f4b3d2c44eebf87efbb41c4 assumed that a
>>>>>>> PCI device has a static address.  This is not true for PCI devices
>>>>>>> that are on the secondary bus of a PCI to PCI bridge.
>>>>>>>

...

>>>> Not really because it's not just secondary bus number.
>>>> Changing subordinate bus numbers can hide/unhide whole buses.
>>>>
>>> You are right.  I have no idea what Paul Durrant was thinking about this
>>> case.
>>> And this would apply to PCI_SUBORDINATE_BUS not PCI_SECONDARY_BUS.
>>>
>>> Since at QEMU 2.2 Xen sends all pci-config requests to QEMU, things
>>> "worked".
>> Why "worked"?
>>

The issue is that PCI to PCI bridges were not configured by hvmloader
nor by SeaBIOS.  I do have a patch
to Xen that fixes this (I was rebasing on the latest Xen to post it and
my test case stopped working).  So PCI devices that exist on the
secondary side of a PCI to PCI bridge could only be used (by Linux with
at least pci=assign-busses) after booting.


>>> It is not clear to me that the complexity of tracking bus
>>> visibility make sense.  Clearly you do.
>> Well what was the point of the change?

To get config cycles that are valid that you do not get today.

>> If you don't care that we get irrelevant config cycles why not
>> just send them all to QEMU?
>>

That would need to be answered by Paul Durrant and/or other people (see
below)

>>
>>>>> Would it be better to have:
>>>>>
>>>>> void device_listener_change_pci_bus_num(PCIBus *b, PCIDevice *d, void
>>>>> *opaque)
>>>>> {
>>>>>     uint8_t oldbus = (uint8_t)opaque;
>>>>>     DEVICE_LISTENER_CALL(change_pci_bus_num, Forward, d, oldbus);
>>>>> }
>>>>>
>>>>> So that the above works, or to add a function to convert args?
>>>>>
>>>>>> To know whether device is accessible to config cycles, you
>>>>>> really need to scan the hierarchy from root downwards.
>>>>>>
>>>>> Yes, that is correct.  However what I am doing here is not
>>>>> changing how QEMU checks if the device is accessible, but
>>>>> changing what pci config Xen sends to QEMU.  If the change
>>>>> to PCI_SECONDARY_BUS hides this PCI to PCI bridge, that is
>>>>> not an issue.
>>>>>
>>>>>
>>>>>    -Don Slutz
>>>>>
>>>>>
>>>> Imagine a bridge with secondary bus number 5
>>>> behind another one with subordinate numbers 1-3.
>>>> You should not send conf cycles for bus number 5 to qemu.
>>>>
>>> That is correct.  How ever unless Paul Durrant has an example of more then 1
>>> "QEMU" where this would make a difference, the cases I am aware of are:
>>>
>>> 1) Xen does not send it, and returns 0xffffffff (or smaller).
>>>
>>> 2) QEMU returns 0xffffffff (or smaller).
>>>
>>> I will grant that #1 is faster, but it also is only happening during start
>>> up and so I do not see the clear win to add more complex code to only do #1.
>>>
>>>    -Don Slutz
>> It's not about faster. I assumed you need to get just the correct
>> stuff out to QEMU to gain the better security as
>> 3996e85c1822e05c50250f8d2d1e57b6bea1229d claims.
> And if that's not the case, please educate me.

I do not know enough about this to answer here.  I have added xen-devel
list to this in the hope
that someone there can answer the "better security" question.

   -Don Slutz


>> -- 
>> MST