Re: [Qemu-devel] [PATCH V4] hw/pxb: add chassis_nr property

[Marcel Apfelbaum <marcel@redhat.com>]

On 03/15/2016 10:00 AM, Cao jin wrote:
> Hi,
>
> On 03/03/2016 10:18 PM, Marcel Apfelbaum wrote:
>> Add a chassis_nr property instead of using PXB bus number
>> as internal bridge's chassis nr.
>>
>> Suggested-by: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>> ---
>> v3->v4:
>>   - re-coded to fit current codebase
>> v2->v3:
>>   - use bus nr if chassis nr is 0 (Micahel S. Tsirkin)
>> v1->v2:
>>   - Rebased on master
>>
>>   docs/pci_expander_bridge.txt        |  7 +++----
>>   hw/pci-bridge/pci_expander_bridge.c | 21 +++++++++++++++++++--
>>   2 files changed, 22 insertions(+), 6 deletions(-)
>>
>
>>
>> @@ -286,6 +294,8 @@ static Property pxb_dev_properties[] = {
>>       /* Note: 0 is not a legal a PXB bus number. */
>
> better remove the latter "a", or I guess it will conflict with my previous pxb cleanup patch.

sure, thanks for bringing it up.

>
>>       DEFINE_PROP_UINT8("bus_nr", PXBDev, bus_nr, 0),
>>       DEFINE_PROP_UINT16("numa_node", PXBDev, numa_node, NUMA_NODE_UNASSIGNED),
>> +    /* Note: 0 is not a legal chassis number. */
>> +    DEFINE_PROP_UINT8("chassis_nr", PXBDev, chassis_nr, 0),
>>       DEFINE_PROP_END_OF_LIST(),
>>   };
>>
>> @@ -322,6 +332,13 @@ static int pxb_pcie_dev_initfn(PCIDevice *dev)
>>       return pxb_dev_init_common(dev, true);
>>   }
>>
>> +static Property pxb_pcie_dev_properties[] = {
>> +    /* Note: 0 is not a legal a PXB bus number. */
>
> likewise

ok

>
>> +    DEFINE_PROP_UINT8("bus_nr", PXBDev, bus_nr, 0),
>> +    DEFINE_PROP_UINT16("numa_node", PXBDev, numa_node, NUMA_NODE_UNASSIGNED),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>
> And I have another personal question: In qemu design, it seems every pci bridge reside in a separate chassis, what`s benefit?  why don`t put them all in the main chassis?

Please have a look on pci-to-pci bridge specification, chapter 13, slot numbering.

Thanks for the review,
Marcel

>