[Qemu-devel] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Qemu-devel] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Aleksandr Bezzubikov]

Now PCI bridges (and PCIE root port too) get a bus range number in system init,
basing on currently plugged devices. That's why when one wants to hotplug another bridge,
it needs his child bus, which the parent is unable to provide.
The suggested workaround is to have vendor-specific capability in RedHat generic pcie-root-port
that contains number of additional bus to reserve on BIOS PCI init.

Aleksandr Bezzubikov (2):
  pci: add support for direct usage of bdf for capability lookup
  pci: enable RedHat pci bridges to reserve more buses

 src/fw/pciinit.c   | 12 ++++++++++--
 src/hw/pcidevice.c | 24 ++++++++++++++++++++++++
 src/hw/pcidevice.h |  1 +
 3 files changed, 35 insertions(+), 2 deletions(-)

-- 
2.7.4

[Qemu-devel] [RFC PATCH 1/2] pci: add support for direct usage of bdf for capability lookup

[Aleksandr Bezzubikov]

Add a capability lookup function which gets bdf instead of pci_device
as its first argument. It may be useful when we have bdf,
but don't have the whole pci_device structure.

Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
---
 src/hw/pcidevice.c | 24 ++++++++++++++++++++++++
 src/hw/pcidevice.h |  1 +
 2 files changed, 25 insertions(+)

diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c
index cfebf66..3fa240e 100644
--- a/src/hw/pcidevice.c
+++ b/src/hw/pcidevice.c
@@ -158,6 +158,30 @@ u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap)
     return 0;
 }
 
+u8 pci_find_capability_bdf(int bdf, u8 cap_id, u8 cap)
+{
+    int i;
+    u16 status = pci_config_readw(bdf, PCI_STATUS);
+
+    if (!(status & PCI_STATUS_CAP_LIST))
+        return 0;
+
+    if (cap == 0) {
+        /* find first */
+        cap = pci_config_readb(bdf, PCI_CAPABILITY_LIST);
+    } else {
+        /* find next */
+        cap = pci_config_readb(bdf, cap + PCI_CAP_LIST_NEXT);
+    }
+    for (i = 0; cap && i <= 0xff; i++) {
+        if (pci_config_readb(bdf, cap + PCI_CAP_LIST_ID) == cap_id)
+            return cap;
+        cap = pci_config_readb(bdf, cap + PCI_CAP_LIST_NEXT);
+    }
+
+    return 0;
+}
+
 // Enable PCI bus-mastering (ie, DMA) support on a pci device
 void
 pci_enable_busmaster(struct pci_device *pci)
diff --git a/src/hw/pcidevice.h b/src/hw/pcidevice.h
index 354b549..e4ed5cf 100644
--- a/src/hw/pcidevice.h
+++ b/src/hw/pcidevice.h
@@ -70,6 +70,7 @@ int pci_init_device(const struct pci_device_id *ids
 struct pci_device *pci_find_init_device(const struct pci_device_id *ids
                                         , void *arg);
 u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap);
+u8 pci_find_capability_bdf(int bdf, u8 cap_id, u8 cap);
 void pci_enable_busmaster(struct pci_device *pci);
 u16 pci_enable_iobar(struct pci_device *pci, u32 addr);
 void *pci_enable_membar(struct pci_device *pci, u32 addr);
-- 
2.7.4

Re: [Qemu-devel] [RFC PATCH 1/2] pci: add support for direct usage of bdf for capability lookup

[Marcel Apfelbaum]

On 19/07/2017 16:20, Aleksandr Bezzubikov wrote:
> Add a capability lookup function which gets bdf instead of pci_device
> as its first argument. It may be useful when we have bdf,
> but don't have the whole pci_device structure.
> 
> Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
> ---
>   src/hw/pcidevice.c | 24 ++++++++++++++++++++++++
>   src/hw/pcidevice.h |  1 +
>   2 files changed, 25 insertions(+)
> 
> diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c
> index cfebf66..3fa240e 100644
> --- a/src/hw/pcidevice.c
> +++ b/src/hw/pcidevice.c
> @@ -158,6 +158,30 @@ u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap)
>       return 0;
>   }
>   

Hi Aleksandr,

> +u8 pci_find_capability_bdf(int bdf, u8 cap_id, u8 cap)
> +{

Please do not duplicate the code for the 'pci_find_capability'.
In this case you can reuse the code by
making 'pci_find_capability' call 'pci_find_capability_bdf',
or better, instead of a new function, change the signature
as you proposed and make the calling code use bdf
instead of the pci device, since is the only info needed.

Thanks,
Marcel

> +    int i;
> +    u16 status = pci_config_readw(bdf, PCI_STATUS);
> +
> +    if (!(status & PCI_STATUS_CAP_LIST))
> +        return 0;
> +
> +    if (cap == 0) {
> +        /* find first */
> +        cap = pci_config_readb(bdf, PCI_CAPABILITY_LIST);
> +    } else {
> +        /* find next */
> +        cap = pci_config_readb(bdf, cap + PCI_CAP_LIST_NEXT);
> +    }
> +    for (i = 0; cap && i <= 0xff; i++) {
> +        if (pci_config_readb(bdf, cap + PCI_CAP_LIST_ID) == cap_id)
> +            return cap;
> +        cap = pci_config_readb(bdf, cap + PCI_CAP_LIST_NEXT);
> +    }
> +
> +    return 0;
> +}
> +
>   // Enable PCI bus-mastering (ie, DMA) support on a pci device
>   void
>   pci_enable_busmaster(struct pci_device *pci)
> diff --git a/src/hw/pcidevice.h b/src/hw/pcidevice.h
> index 354b549..e4ed5cf 100644
> --- a/src/hw/pcidevice.h
> +++ b/src/hw/pcidevice.h
> @@ -70,6 +70,7 @@ int pci_init_device(const struct pci_device_id *ids
>   struct pci_device *pci_find_init_device(const struct pci_device_id *ids
>                                           , void *arg);
>   u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap);
> +u8 pci_find_capability_bdf(int bdf, u8 cap_id, u8 cap);
>   void pci_enable_busmaster(struct pci_device *pci);
>   u16 pci_enable_iobar(struct pci_device *pci, u32 addr);
>   void *pci_enable_membar(struct pci_device *pci, u32 addr);
> 

[Qemu-devel] [RFC PATCH 2/2] pci: enable RedHat pci bridges to reserve more buses

[Aleksandr Bezzubikov]

In case of RedHat PCI bridges reserve additional buses, which number is provided
in a vendor-specific capability.

Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
---
 src/fw/pciinit.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index 08221e6..b6f3a01 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -578,9 +578,17 @@ pci_bios_init_bus_rec(int bus, u8 *pci_bus)
         pci_bios_init_bus_rec(secbus, pci_bus);
 
         if (subbus != *pci_bus) {
+            u16 vendor = pci_config_readw(bdf, PCI_VENDOR_ID);
+            u8 res_bus = 0;
+            if (vendor == 0x1b36) {
+                u8 cap = pci_find_capability_bdf(bdf, PCI_CAP_ID_VNDR, 0);
+                if (cap) {
+                    res_bus = pci_config_readb(bdf, cap + 16);
+                }
+            }
             dprintf(1, "PCI: subordinate bus = 0x%x -> 0x%x\n",
-                    subbus, *pci_bus);
-            subbus = *pci_bus;
+                    subbus, *pci_bus + res_bus);
+            subbus = *pci_bus + res_bus;
         } else {
             dprintf(1, "PCI: subordinate bus = 0x%x\n", subbus);
         }
-- 
2.7.4

Re: [Qemu-devel] [RFC PATCH 2/2] pci: enable RedHat pci bridges to reserve more buses

[Konrad Rzeszutek Wilk]

On Wed, Jul 19, 2017 at 04:20:14PM +0300, Aleksandr Bezzubikov wrote:
> In case of RedHat PCI bridges reserve additional buses, which number is provided

It is "Red Hat"
> in a vendor-specific capability.

And perhaps also a #define ?
> 
> Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
> ---
>  src/fw/pciinit.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index 08221e6..b6f3a01 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -578,9 +578,17 @@ pci_bios_init_bus_rec(int bus, u8 *pci_bus)
>          pci_bios_init_bus_rec(secbus, pci_bus);
>  
>          if (subbus != *pci_bus) {
> +            u16 vendor = pci_config_readw(bdf, PCI_VENDOR_ID);
> +            u8 res_bus = 0;
> +            if (vendor == 0x1b36) {
> +                u8 cap = pci_find_capability_bdf(bdf, PCI_CAP_ID_VNDR, 0);
> +                if (cap) {
> +                    res_bus = pci_config_readb(bdf, cap + 16);
> +                }
> +            }
>              dprintf(1, "PCI: subordinate bus = 0x%x -> 0x%x\n",
> -                    subbus, *pci_bus);
> -            subbus = *pci_bus;
> +                    subbus, *pci_bus + res_bus);
> +            subbus = *pci_bus + res_bus;
>          } else {
>              dprintf(1, "PCI: subordinate bus = 0x%x\n", subbus);
>          }
> -- 
> 2.7.4
> 
> 

Re: [Qemu-devel] [RFC PATCH 2/2] pci: enable RedHat pci bridges to reserve more buses

[Marcel Apfelbaum]

On 19/07/2017 16:56, Konrad Rzeszutek Wilk wrote:
> On Wed, Jul 19, 2017 at 04:20:14PM +0300, Aleksandr Bezzubikov wrote:
>> In case of RedHat PCI bridges reserve additional buses, which number is provided
> 
> It is "Red Hat"
>> in a vendor-specific capability.
> 
> And perhaps also a #define ?

Right, please add it to src/hw/pci_ids.h.

Thanks,
Marcel


>>
>> Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
>> ---
>>   src/fw/pciinit.c | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
>> index 08221e6..b6f3a01 100644
>> --- a/src/fw/pciinit.c
>> +++ b/src/fw/pciinit.c
>> @@ -578,9 +578,17 @@ pci_bios_init_bus_rec(int bus, u8 *pci_bus)
>>           pci_bios_init_bus_rec(secbus, pci_bus);
>>   
>>           if (subbus != *pci_bus) {
>> +            u16 vendor = pci_config_readw(bdf, PCI_VENDOR_ID);
>> +            u8 res_bus = 0;
>> +            if (vendor == 0x1b36) {
>> +                u8 cap = pci_find_capability_bdf(bdf, PCI_CAP_ID_VNDR, 0);
>> +                if (cap) {
>> +                    res_bus = pci_config_readb(bdf, cap + 16);
>> +                }
>> +            }
>>               dprintf(1, "PCI: subordinate bus = 0x%x -> 0x%x\n",
>> -                    subbus, *pci_bus);
>> -            subbus = *pci_bus;
>> +                    subbus, *pci_bus + res_bus);
>> +            subbus = *pci_bus + res_bus;
>>           } else {
>>               dprintf(1, "PCI: subordinate bus = 0x%x\n", subbus);
>>           }
>> -- 
>> 2.7.4
>>
>>

Re: [Qemu-devel] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Konrad Rzeszutek Wilk]

On Wed, Jul 19, 2017 at 04:20:12PM +0300, Aleksandr Bezzubikov wrote:
> Now PCI bridges (and PCIE root port too) get a bus range number in system init,
> basing on currently plugged devices. That's why when one wants to hotplug another bridge,
> it needs his child bus, which the parent is unable to provide.

Could you explain how you trigger this?

> The suggested workaround is to have vendor-specific capability in RedHat generic pcie-root-port
> that contains number of additional bus to reserve on BIOS PCI init.

But wouldn't the proper fix be for the PCI bridge to have the subordinate
value be extended to fit more bus ranges?

> 
> Aleksandr Bezzubikov (2):
>   pci: add support for direct usage of bdf for capability lookup
>   pci: enable RedHat pci bridges to reserve more buses
> 
>  src/fw/pciinit.c   | 12 ++++++++++--
>  src/hw/pcidevice.c | 24 ++++++++++++++++++++++++
>  src/hw/pcidevice.h |  1 +
>  3 files changed, 35 insertions(+), 2 deletions(-)
> 
> -- 
> 2.7.4
> 
> 

Re: [Qemu-devel] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Alexander Bezzubikov]

ср, 19 июля 2017 г. в 16:57, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:

> On Wed, Jul 19, 2017 at 04:20:12PM +0300, Aleksandr Bezzubikov wrote:
> > Now PCI bridges (and PCIE root port too) get a bus range number in
> system init,
> > basing on currently plugged devices. That's why when one wants to
> hotplug another bridge,
> > it needs his child bus, which the parent is unable to provide.
>
> Could you explain how you trigger this?


I'm trying to hot plug pcie-pci bridge into pcie root port, and Linux says
'cannot allocate bus number for device bla-bla'. This obviously does not
allow me to use the bridge at all.

>
>
> > The suggested workaround is to have vendor-specific capability in RedHat
> generic pcie-root-port
> > that contains number of additional bus to reserve on BIOS PCI init.
>
> But wouldn't the proper fix be for the PCI bridge to have the subordinate
> value be extended to fit more bus ranges?


What do you mean? This is what I'm trying to do. Do you suppose to get rid
of vendor-specific cap and use original register value instead of it?

>
> >
> > Aleksandr Bezzubikov (2):
> >   pci: add support for direct usage of bdf for capability lookup
> >   pci: enable RedHat pci bridges to reserve more buses
> >
> >  src/fw/pciinit.c   | 12 ++++++++++--
> >  src/hw/pcidevice.c | 24 ++++++++++++++++++++++++
> >  src/hw/pcidevice.h |  1 +
> >  3 files changed, 35 insertions(+), 2 deletions(-)
> >
> > --
> > 2.7.4
> >
> >
>
-- 
Alexander Bezzubikov

Re: [Qemu-devel] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Konrad Rzeszutek Wilk]

On Wed, Jul 19, 2017 at 05:14:41PM +0000, Alexander Bezzubikov wrote:
> ср, 19 июля 2017 г. в 16:57, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
> 
> > On Wed, Jul 19, 2017 at 04:20:12PM +0300, Aleksandr Bezzubikov wrote:
> > > Now PCI bridges (and PCIE root port too) get a bus range number in
> > system init,
> > > basing on currently plugged devices. That's why when one wants to
> > hotplug another bridge,
> > > it needs his child bus, which the parent is unable to provide.
> >
> > Could you explain how you trigger this?
> 
> 
> I'm trying to hot plug pcie-pci bridge into pcie root port, and Linux says
> 'cannot allocate bus number for device bla-bla'. This obviously does not
> allow me to use the bridge at all.
> 
> >
> >
> > > The suggested workaround is to have vendor-specific capability in RedHat
> > generic pcie-root-port
> > > that contains number of additional bus to reserve on BIOS PCI init.
> >
> > But wouldn't the proper fix be for the PCI bridge to have the subordinate
> > value be extended to fit more bus ranges?
> 
> 
> What do you mean? This is what I'm trying to do. Do you suppose to get rid
> of vendor-specific cap and use original register value instead of it?

I would suggest a simple fix - each bridge has a a number of bus devices
it can use. You have up to 255 - so you split the number of northbridge
numbers by the amount of NUMA nodes (if that is used) - so for example
if you have 4 NUMA nodes, each bridge would cover 63 bus numbers.

Meaning the root bridge would cover 0->63 bus, 64->128, and so on.
That gives you enough space to plug in your plugged in devices
(up to 63).

And if you need sub-briges then carve out a specific range.


> 
> >
> > >
> > > Aleksandr Bezzubikov (2):
> > >   pci: add support for direct usage of bdf for capability lookup
> > >   pci: enable RedHat pci bridges to reserve more buses
> > >
> > >  src/fw/pciinit.c   | 12 ++++++++++--
> > >  src/hw/pcidevice.c | 24 ++++++++++++++++++++++++
> > >  src/hw/pcidevice.h |  1 +
> > >  3 files changed, 35 insertions(+), 2 deletions(-)
> > >
> > > --
> > > 2.7.4
> > >
> > >
> >
> -- 
> Alexander Bezzubikov

[Qemu-devel] Fwd: [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Alexander Bezzubikov]

2017-07-19 21:18 GMT+03:00 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:

> On Wed, Jul 19, 2017 at 05:14:41PM +0000, Alexander Bezzubikov wrote:
> > ср, 19 июля 2017 г. в 16:57, Konrad Rzeszutek Wilk <
> konrad.wilk@oracle.com>:
> >
> > > On Wed, Jul 19, 2017 at 04:20:12PM +0300, Aleksandr Bezzubikov wrote:
> > > > Now PCI bridges (and PCIE root port too) get a bus range number in
> > > system init,
> > > > basing on currently plugged devices. That's why when one wants to
> > > hotplug another bridge,
> > > > it needs his child bus, which the parent is unable to provide.
> > >
> > > Could you explain how you trigger this?
> >
> >
> > I'm trying to hot plug pcie-pci bridge into pcie root port, and Linux
> says
> > 'cannot allocate bus number for device bla-bla'. This obviously does not
> > allow me to use the bridge at all.
> >
> > >
> > >
> > > > The suggested workaround is to have vendor-specific capability in
> RedHat
> > > generic pcie-root-port
> > > > that contains number of additional bus to reserve on BIOS PCI init.
> > >
> > > But wouldn't the proper fix be for the PCI bridge to have the
> subordinate
> > > value be extended to fit more bus ranges?
> >
> >
> > What do you mean? This is what I'm trying to do. Do you suppose to get
> rid
> > of vendor-specific cap and use original register value instead of it?
>
> I would suggest a simple fix - each bridge has a a number of bus devices
> it can use. You have up to 255 - so you split the number of northbridge
> numbers by the amount of NUMA nodes (if that is used) - so for example
> if you have 4 NUMA nodes, each bridge would cover 63 bus numbers.
>
> Meaning the root bridge would cover 0->63 bus, 64->128, and so on.
> That gives you enough space to plug in your plugged in devices
> (up to 63).
>
> And if you need sub-briges then carve out a specific range.
>

The problem is that we don't know at the init moment how many subbridges we
may need,
and how deep the whole device tree will be. The key moment - PCI bridge
hotplugging
needs either rescan all buses on each bridge device addition, or reserve
space in advance during BIOS init.
In this series the second way was chosen.


>
>
> >
> > >
> > > >
> > > > Aleksandr Bezzubikov (2):
> > > >   pci: add support for direct usage of bdf for capability lookup
> > > >   pci: enable RedHat pci bridges to reserve more buses
> > > >
> > > >  src/fw/pciinit.c   | 12 ++++++++++--
> > > >  src/hw/pcidevice.c | 24 ++++++++++++++++++++++++
> > > >  src/hw/pcidevice.h |  1 +
> > > >  3 files changed, 35 insertions(+), 2 deletions(-)
> > > >
> > > > --
> > > > 2.7.4
> > > >
> > > >
> > >
> > --
> > Alexander Bezzubikov
>



-- 
Alexander Bezzubikov

Re: [Qemu-devel] Fwd: [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Konrad Rzeszutek Wilk]

On Wed, Jul 19, 2017 at 09:38:50PM +0300, Alexander Bezzubikov wrote:
> 2017-07-19 21:18 GMT+03:00 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
> 
> > On Wed, Jul 19, 2017 at 05:14:41PM +0000, Alexander Bezzubikov wrote:
> > > ср, 19 июля 2017 г. в 16:57, Konrad Rzeszutek Wilk <
> > konrad.wilk@oracle.com>:
> > >
> > > > On Wed, Jul 19, 2017 at 04:20:12PM +0300, Aleksandr Bezzubikov wrote:
> > > > > Now PCI bridges (and PCIE root port too) get a bus range number in
> > > > system init,
> > > > > basing on currently plugged devices. That's why when one wants to
> > > > hotplug another bridge,
> > > > > it needs his child bus, which the parent is unable to provide.
> > > >
> > > > Could you explain how you trigger this?
> > >
> > >
> > > I'm trying to hot plug pcie-pci bridge into pcie root port, and Linux
> > says
> > > 'cannot allocate bus number for device bla-bla'. This obviously does not
> > > allow me to use the bridge at all.
> > >
> > > >
> > > >
> > > > > The suggested workaround is to have vendor-specific capability in
> > RedHat
> > > > generic pcie-root-port
> > > > > that contains number of additional bus to reserve on BIOS PCI init.
> > > >
> > > > But wouldn't the proper fix be for the PCI bridge to have the
> > subordinate
> > > > value be extended to fit more bus ranges?
> > >
> > >
> > > What do you mean? This is what I'm trying to do. Do you suppose to get
> > rid
> > > of vendor-specific cap and use original register value instead of it?
> >
> > I would suggest a simple fix - each bridge has a a number of bus devices
> > it can use. You have up to 255 - so you split the number of northbridge
> > numbers by the amount of NUMA nodes (if that is used) - so for example
> > if you have 4 NUMA nodes, each bridge would cover 63 bus numbers.
> >
> > Meaning the root bridge would cover 0->63 bus, 64->128, and so on.
> > That gives you enough space to plug in your plugged in devices
> > (up to 63).
> >
> > And if you need sub-briges then carve out a specific range.
> >
> 
> The problem is that we don't know at the init moment how many subbridges we
> may need,

                                                                                                          
And the explanation above does not either. It just setups at init time                                                              
an range where you can plug in your new devices in. But in a more uniform                                                           
way such that you can also utilize this with NUMA and _PXM topology                                                                 
in the future.               

> and how deep the whole device tree will be. The key moment - PCI bridge
> hotplugging
> needs either rescan all buses on each bridge device addition, or reserve
> space in advance during BIOS init.

can all buses on each bridge device addition, or reserve                                                          
                                                                                                                                    
It is more complex than that - you may need to move devices that are                                                                
below you. And Linux kernel (nor any other OS) can handle that.                                                                     
(They can during bootup)   

> In this series the second way was chosen.
> 
> 
> >
> >
> > >
> > > >
> > > > >
> > > > > Aleksandr Bezzubikov (2):
> > > > >   pci: add support for direct usage of bdf for capability lookup
> > > > >   pci: enable RedHat pci bridges to reserve more buses
> > > > >
> > > > >  src/fw/pciinit.c   | 12 ++++++++++--
> > > > >  src/hw/pcidevice.c | 24 ++++++++++++++++++++++++
> > > > >  src/hw/pcidevice.h |  1 +
> > > > >  3 files changed, 35 insertions(+), 2 deletions(-)
> > > > >
> > > > > --
> > > > > 2.7.4
> > > > >
> > > > >
> > > >
> > > --
> > > Alexander Bezzubikov
> >
> 
> 
> 
> -- 
> Alexander Bezzubikov

Re: [Qemu-devel] Fwd: [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Marcel Apfelbaum]

On 19/07/2017 21:56, Konrad Rzeszutek Wilk wrote:
> On Wed, Jul 19, 2017 at 09:38:50PM +0300, Alexander Bezzubikov wrote:
>> 2017-07-19 21:18 GMT+03:00 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
>>
>>> On Wed, Jul 19, 2017 at 05:14:41PM +0000, Alexander Bezzubikov wrote:
>>>> ср, 19 июля 2017 г. в 16:57, Konrad Rzeszutek Wilk <
>>> konrad.wilk@oracle.com>:
>>>>
>>>>> On Wed, Jul 19, 2017 at 04:20:12PM +0300, Aleksandr Bezzubikov wrote:
>>>>>> Now PCI bridges (and PCIE root port too) get a bus range number in
>>>>> system init,
>>>>>> basing on currently plugged devices. That's why when one wants to
>>>>> hotplug another bridge,
>>>>>> it needs his child bus, which the parent is unable to provide.
>>>>>
>>>>> Could you explain how you trigger this?
>>>>
>>>>
>>>> I'm trying to hot plug pcie-pci bridge into pcie root port, and Linux
>>> says
>>>> 'cannot allocate bus number for device bla-bla'. This obviously does not
>>>> allow me to use the bridge at all.
>>>>
>>>>>
>>>>>
>>>>>> The suggested workaround is to have vendor-specific capability in
>>> RedHat
>>>>> generic pcie-root-port
>>>>>> that contains number of additional bus to reserve on BIOS PCI init.
>>>>>
>>>>> But wouldn't the proper fix be for the PCI bridge to have the
>>> subordinate
>>>>> value be extended to fit more bus ranges?
>>>>
>>>>
>>>> What do you mean? This is what I'm trying to do. Do you suppose to get
>>> rid
>>>> of vendor-specific cap and use original register value instead of it?
>>>
>>> I would suggest a simple fix - each bridge has a a number of bus devices
>>> it can use. You have up to 255 - so you split the number of northbridge
>>> numbers by the amount of NUMA nodes (if that is used) - so for example
>>> if you have 4 NUMA nodes, each bridge would cover 63 bus numbers.
>>>
>>> Meaning the root bridge would cover 0->63 bus, 64->128, and so on.
>>> That gives you enough space to plug in your plugged in devices
>>> (up to 63).
>>>
>>> And if you need sub-briges then carve out a specific range.
>>>
>>

Hi Konrad,

>> The problem is that we don't know at the init moment how many subbridges we
>> may need,
> 

Is possible the explanation was not clear clear and led to
some miscommunication.

>                                                                                                            
> And the explanation above does not either. It just setups at init time
> an range where you can plug in your new devices in. But in a more uniform
> way such that you can also utilize this with NUMA and _PXM topology
> in the future.
> 

I fully agree with you and actually QEMU has already implemented the
exact idea you are describing here, its called a pxb/pxb-pci device,
that can be "bounded" to a specific NUMA node and has a subrange of bus
numbers dedicated to it.

However this problem is different. In a PCI Express machine you
can hotplug PCIe devices only into PCIe Root Ports (or switch
downstream ports, but not in current scope).

We want to be able to hotplug a PCIe-PCI bridge into a PCIe Root Port
so we can then hot-plug legacy PCI devices.

Since the PCIe Root Port is a type of PCI bridge, at boot time
it only gets the bus sub-range (primary bus,subordinate bus]
which is computed by firmware and leaves no bus number that
can be used by a hot-plugged pci-bridge. And this obviously
does not depend on how we arrange NUMA/proximities.

We are also not looking for a fix for a specific guest OS,
so reserving some extra bus-numbers it has minimal impact
on the system. I do agree the problem may be solved differently,
however we can't reach all guest OS vendors and ask them to
support an alternative solution in a reasonable time frame.

Thanks,
Marcel

>> and how deep the whole device tree will be. The key moment - PCI bridge
>> hotplugging
>> needs either rescan all buses on each bridge device addition, or reserve
>> space in advance during BIOS init.
> 
> can all buses on each bridge device addition, or reserve
>                                                                                                                                      
> It is more complex than that - you may need to move devices that are
> below you. And Linux kernel (nor any other OS) can handle that.
> (They can during bootup)
> 
>> In this series the second way was chosen.
>>
>>
>>>
>>>
>>>>
>>>>>
>>>>>>
>>>>>> Aleksandr Bezzubikov (2):
>>>>>>    pci: add support for direct usage of bdf for capability lookup
>>>>>>    pci: enable RedHat pci bridges to reserve more buses
>>>>>>
>>>>>>   src/fw/pciinit.c   | 12 ++++++++++--
>>>>>>   src/hw/pcidevice.c | 24 ++++++++++++++++++++++++
>>>>>>   src/hw/pcidevice.h |  1 +
>>>>>>   3 files changed, 35 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> --
>>>>>> 2.7.4
>>>>>>
>>>>>>
>>>>>
>>>> --
>>>> Alexander Bezzubikov
>>>
>>
>>
>>
>> -- 
>> Alexander Bezzubikov

Re: [Qemu-devel] [SeaBIOS] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Gerd Hoffmann]

On Wed, 2017-07-19 at 16:20 +0300, Aleksandr Bezzubikov wrote:
> Now PCI bridges (and PCIE root port too) get a bus range number in
> system init,
> basing on currently plugged devices. That's why when one wants to
> hotplug another bridge,
> it needs his child bus, which the parent is unable to provide.
> The suggested workaround is to have vendor-specific capability in
> RedHat generic pcie-root-port
> that contains number of additional bus to reserve on BIOS PCI init.

Where is the qemu patch for this?

What about window sizes?  IIRC there was a plan to provide allocation
hints for them too ...

cheers,
  Gerd

Re: [Qemu-devel] [SeaBIOS] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Marcel Apfelbaum]

On 20/07/2017 9:52, Gerd Hoffmann wrote:
> On Wed, 2017-07-19 at 16:20 +0300, Aleksandr Bezzubikov wrote:
>> Now PCI bridges (and PCIE root port too) get a bus range number in
>> system init,
>> basing on currently plugged devices. That's why when one wants to
>> hotplug another bridge,
>> it needs his child bus, which the parent is unable to provide.
>> The suggested workaround is to have vendor-specific capability in
>> RedHat generic pcie-root-port
>> that contains number of additional bus to reserve on BIOS PCI init.
> 

Hi Gerd,
Thanks for looking into this.

> Where is the qemu patch for this?
> 

On the way, Aleksandr is working on it, should
be ready soon.

> What about window sizes?  IIRC there was a plan to provide allocation
> hints for them too ...

Yes, is in my TODO list, however not as part of Aleksandr's series
which aims to provide PCIe-PCI bridge hotplug support.
I should mention that for this case both Windows and Linux guests
(modern OSes) trigger correctly IO/MEM resource re-balancing
process and succeeds to complete the hotplug operation.

Thanks,
Marcel

> 
> cheers,
>    Gerd
> 

Re: [Qemu-devel] [SeaBIOS] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Gerd Hoffmann]

  Hi,

> > What about window sizes?  IIRC there was a plan to provide
> > allocation
> > hints for them too ...
> 
> Yes, is in my TODO list, however not as part of Aleksandr's series
> which aims to provide PCIe-PCI bridge hotplug support.

I'd prefer to have a single vendor capability for all resource
allocation hints provided by qemu.

cheers,
  Gerd

Re: [Qemu-devel] [SeaBIOS] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Marcel Apfelbaum]

On 21/07/2017 13:04, Gerd Hoffmann wrote:
>    Hi,
> 
>>> What about window sizes?  IIRC there was a plan to provide
>>> allocation
>>> hints for them too ...
>>
>> Yes, is in my TODO list, however not as part of Aleksandr's series
>> which aims to provide PCIe-PCI bridge hotplug support.
> 
> I'd prefer to have a single vendor capability for all resource
> allocation hints provided by qemu.
> 

[Adding Laszlo]


Sure, the capability looking something like:

    [flags: reserve-buses|reserve-IO|reserve-MEM|...]
    [extra-buses][IO-size][MEM-size]

    if reserve-buses -> use 'extra-buses' value and so on

would be OK?

Thanks,
Marcel


> cheers,
>    Gerd
> 

Re: [Qemu-devel] [SeaBIOS] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Gerd Hoffmann]

On Fri, 2017-07-21 at 15:15 +0300, Marcel Apfelbaum wrote:
> On 21/07/2017 13:04, Gerd Hoffmann wrote:
> >    Hi,
> > 
> > > > What about window sizes?  IIRC there was a plan to provide
> > > > allocation
> > > > hints for them too ...
> > > 
> > > Yes, is in my TODO list, however not as part of Aleksandr's
> > > series
> > > which aims to provide PCIe-PCI bridge hotplug support.
> > 
> > I'd prefer to have a single vendor capability for all resource
> > allocation hints provided by qemu.
> > 
> 
> [Adding Laszlo]
> 
> 
> Sure, the capability looking something like:
> 
>     [flags: reserve-buses|reserve-IO|reserve-MEM|...]
>     [extra-buses][IO-size][MEM-size]
> 
>     if reserve-buses -> use 'extra-buses' value and so on

Do we need the flags?  We can use "value == 0 -> no hint for you".
Also what about prefetchable vs. non-prefetchable memory?  I guess we
want a size hint for both memory windows?

cheers,
  Gerd

Re: [Qemu-devel] [SeaBIOS] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Marcel Apfelbaum]

On 21/07/2017 15:42, Gerd Hoffmann wrote:
> On Fri, 2017-07-21 at 15:15 +0300, Marcel Apfelbaum wrote:
>> On 21/07/2017 13:04, Gerd Hoffmann wrote:
>>>     Hi,
>>>
>>>>> What about window sizes?  IIRC there was a plan to provide
>>>>> allocation
>>>>> hints for them too ...
>>>>
>>>> Yes, is in my TODO list, however not as part of Aleksandr's
>>>> series
>>>> which aims to provide PCIe-PCI bridge hotplug support.
>>>
>>> I'd prefer to have a single vendor capability for all resource
>>> allocation hints provided by qemu.
>>>
>>
>> [Adding Laszlo]
>>
>>
>> Sure, the capability looking something like:
>>
>>      [flags: reserve-buses|reserve-IO|reserve-MEM|...]
>>      [extra-buses][IO-size][MEM-size]
>>
>>      if reserve-buses -> use 'extra-buses' value and so on
> 
> Do we need the flags?  We can use "value == 0 -> no hint for you".

Maybe we don't want to allocate IO at all,
So value 0 would say do not allocate.

But we can disable IO by other means.

> Also what about prefetchable vs. non-prefetchable memory?  I guess we
> want a size hint for both memory windows?


Good point, thanks!
Marcel

> 
> cheers,
>    Gerd
> 

Re: [Qemu-devel] [SeaBIOS] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Gerd Hoffmann]

  Hi,

> Maybe we don't want to allocate IO at all,
> So value 0 would say do not allocate.
> 
> But we can disable IO by other means.

Indeed.  As IO is optional for pcie we don't need a qemu-specific
extension for that.

cheers,
  Gerd

Re: [Qemu-devel] [SeaBIOS] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Kevin O'Connor]

On Fri, Jul 21, 2017 at 03:15:46PM +0300, Marcel Apfelbaum wrote:
> On 21/07/2017 13:04, Gerd Hoffmann wrote:
> > I'd prefer to have a single vendor capability for all resource
> > allocation hints provided by qemu.
> Sure, the capability looking something like:
> 
>    [flags: reserve-buses|reserve-IO|reserve-MEM|...]
>    [extra-buses][IO-size][MEM-size]
> 
>    if reserve-buses -> use 'extra-buses' value and so on

I don't have any objection to using a PCI capability, but I do wonder
if fw_cfg would be a better fit.  This information is purely qemu ->
firmware, right?

-Kevin

Re: [Qemu-devel] [SeaBIOS] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Marcel Apfelbaum]

On 21/07/2017 20:28, Kevin O'Connor wrote:
> On Fri, Jul 21, 2017 at 03:15:46PM +0300, Marcel Apfelbaum wrote:
>> On 21/07/2017 13:04, Gerd Hoffmann wrote:
>>> I'd prefer to have a single vendor capability for all resource
>>> allocation hints provided by qemu.
>> Sure, the capability looking something like:
>>
>>     [flags: reserve-buses|reserve-IO|reserve-MEM|...]
>>     [extra-buses][IO-size][MEM-size]
>>
>>     if reserve-buses -> use 'extra-buses' value and so on
> 
> I don't have any objection to using a PCI capability, but I do wonder
> if fw_cfg would be a better fit.  This information is purely qemu ->
> firmware, right?
> 

Hi Kevin,

Right, but theoretically speaking a guest OS driver could also
get the hint on hotplug, or simply because the OS chooses to re-assign
resources on its own.

A while ago we discussed the fw_cfg option, but Gerd preferred
the vendor capability, and since the capability looked cleaner
Aleksandr opted for it.

He will send V2 soon together with the QEMU counterpart feature.

Thanks,
Marcel

> -Kevin
> 

Re: [Qemu-devel] [SeaBIOS] [RFC PATCH 0/2] Allow RedHat PCI bridges reserve more buses than necessary during init

[Gerd Hoffmann]

On Fri, 2017-07-21 at 13:28 -0400, Kevin O'Connor wrote:
> On Fri, Jul 21, 2017 at 03:15:46PM +0300, Marcel Apfelbaum wrote:
> > On 21/07/2017 13:04, Gerd Hoffmann wrote:
> > > I'd prefer to have a single vendor capability for all resource
> > > allocation hints provided by qemu.
> > 
> > Sure, the capability looking something like:
> > 
> >    [flags: reserve-buses|reserve-IO|reserve-MEM|...]
> >    [extra-buses][IO-size][MEM-size]
> > 
> >    if reserve-buses -> use 'extra-buses' value and so on
> 
> I don't have any objection to using a PCI capability, but I do wonder
> if fw_cfg would be a better fit.  This information is purely qemu ->
> firmware, right?

fw_cfg quickly becomes messy if you want allow different hints per
bridge.

cheers,
  Gerd