[PATCH qemu] spapr_pci: Create assigned properties for bridges

Alexey Kardashevskiy posted 1 patch 4 years, 3 months ago
Test FreeBSD failed
Test docker-mingw@fedora failed
Test checkpatch passed
Test docker-quick@centos7 failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200129023111.1699-1-aik@ozlabs.ru
Maintainers: David Gibson <david@gibson.dropbear.id.au>
hw/ppc/spapr_pci.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
[PATCH qemu] spapr_pci: Create assigned properties for bridges
Posted by Alexey Kardashevskiy 4 years, 3 months ago
QEMU assigns bus numbers so tell the guest about assigned values.

This also adds an empty "ranges" to let the existing linux kernels proceed
far enough to trigger resource reassignment (which is rather a hack).

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

This is a part of the "kill CAS reboot" effort, the SLOF's side of it was
posted as "[PATCH slof] fdt: Fix creating new nodes at H_CAS"

---
 hw/ppc/spapr_pci.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 723373de732c..877ff1d0d5fa 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1336,7 +1336,23 @@ static int spapr_dt_pci_bus(SpaprPhbState *sphb, PCIBus *bus,
     if (pci_bus_is_root(bus)) {
         owner = OBJECT(sphb);
     } else {
-        owner = OBJECT(pci_bridge_get_device(bus));
+        PCIDevice *pdev = pci_bridge_get_device(bus);
+        uint8_t pri = pci_default_read_config(pdev, PCI_PRIMARY_BUS, 1);
+        uint8_t sec  = pci_default_read_config(pdev, PCI_SECONDARY_BUS, 1);
+        uint8_t sub  = pci_default_read_config(pdev, PCI_SUBORDINATE_BUS, 1);
+        uint32_t range[] = { cpu_to_be32(sec), cpu_to_be32(sub) };
+
+        /*
+         * Create these to get existing Linux kernels proceed far enough to
+         * trigger resource reassignment. We creates these for vPHB already.
+         */
+        _FDT(fdt_setprop_cell(fdt, offset, "primary-bus", pri));
+        _FDT(fdt_setprop_cell(fdt, offset, "secondary-bus", sec));
+        _FDT(fdt_setprop_cell(fdt, offset, "subordinate-bus", sub));
+        _FDT(fdt_setprop(fdt, offset, "bus-range", range, sizeof(range)));
+        _FDT(fdt_setprop_string(fdt, offset, "device_type", "pci"));
+        _FDT(fdt_setprop(fdt, offset, "ranges", NULL, 0));
+        owner = OBJECT(pdev);
     }
 
     ret = spapr_dt_drc(fdt, offset, owner,
-- 
2.17.1


Re: [PATCH qemu] spapr_pci: Create assigned properties for bridges
Posted by no-reply@patchew.org 4 years, 3 months ago
Patchew URL: https://patchew.org/QEMU/20200129023111.1699-1-aik@ozlabs.ru/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20200129023111.1699-1-aik@ozlabs.ru/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH qemu] spapr_pci: Create assigned properties for bridges
Posted by David Gibson 4 years, 3 months ago
On Wed, Jan 29, 2020 at 01:31:11PM +1100, Alexey Kardashevskiy wrote:
> QEMU assigns bus numbers so tell the guest about assigned values.
> 
> This also adds an empty "ranges" to let the existing linux kernels proceed
> far enough to trigger resource reassignment (which is rather a
> hack).

That is rather a hack, but AIUI this makes things better than they
were before, so I've applied it to ppc-for-5.0.

What would a less hacky approach to this look like?

> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> 
> This is a part of the "kill CAS reboot" effort, the SLOF's side of it was
> posted as "[PATCH slof] fdt: Fix creating new nodes at H_CAS"
> 
> ---
>  hw/ppc/spapr_pci.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 723373de732c..877ff1d0d5fa 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1336,7 +1336,23 @@ static int spapr_dt_pci_bus(SpaprPhbState *sphb, PCIBus *bus,
>      if (pci_bus_is_root(bus)) {
>          owner = OBJECT(sphb);
>      } else {
> -        owner = OBJECT(pci_bridge_get_device(bus));
> +        PCIDevice *pdev = pci_bridge_get_device(bus);
> +        uint8_t pri = pci_default_read_config(pdev, PCI_PRIMARY_BUS, 1);
> +        uint8_t sec  = pci_default_read_config(pdev, PCI_SECONDARY_BUS, 1);
> +        uint8_t sub  = pci_default_read_config(pdev, PCI_SUBORDINATE_BUS, 1);
> +        uint32_t range[] = { cpu_to_be32(sec), cpu_to_be32(sub) };
> +
> +        /*
> +         * Create these to get existing Linux kernels proceed far enough to
> +         * trigger resource reassignment. We creates these for vPHB already.
> +         */
> +        _FDT(fdt_setprop_cell(fdt, offset, "primary-bus", pri));
> +        _FDT(fdt_setprop_cell(fdt, offset, "secondary-bus", sec));
> +        _FDT(fdt_setprop_cell(fdt, offset, "subordinate-bus", sub));
> +        _FDT(fdt_setprop(fdt, offset, "bus-range", range, sizeof(range)));
> +        _FDT(fdt_setprop_string(fdt, offset, "device_type", "pci"));
> +        _FDT(fdt_setprop(fdt, offset, "ranges", NULL, 0));
> +        owner = OBJECT(pdev);
>      }
>  
>      ret = spapr_dt_drc(fdt, offset, owner,

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [PATCH qemu] spapr_pci: Create assigned properties for bridges
Posted by Alexey Kardashevskiy 4 years, 3 months ago

On 29/01/2020 14:37, David Gibson wrote:
> On Wed, Jan 29, 2020 at 01:31:11PM +1100, Alexey Kardashevskiy wrote:
>> QEMU assigns bus numbers so tell the guest about assigned values.
>>
>> This also adds an empty "ranges" to let the existing linux kernels proceed
>> far enough to trigger resource reassignment (which is rather a
>> hack).
> 
> That is rather a hack, but AIUI this makes things better than they
> were before, so I've applied it to ppc-for-5.0.
> 
> What would a less hacky approach to this look like?


Assigning the bridge resources in QEMU is the proper fix I suppose. Thanks,

> 
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>
>> This is a part of the "kill CAS reboot" effort, the SLOF's side of it was
>> posted as "[PATCH slof] fdt: Fix creating new nodes at H_CAS"
>>
>> ---
>>  hw/ppc/spapr_pci.c | 18 +++++++++++++++++-
>>  1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index 723373de732c..877ff1d0d5fa 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -1336,7 +1336,23 @@ static int spapr_dt_pci_bus(SpaprPhbState *sphb, PCIBus *bus,
>>      if (pci_bus_is_root(bus)) {
>>          owner = OBJECT(sphb);
>>      } else {
>> -        owner = OBJECT(pci_bridge_get_device(bus));
>> +        PCIDevice *pdev = pci_bridge_get_device(bus);
>> +        uint8_t pri = pci_default_read_config(pdev, PCI_PRIMARY_BUS, 1);
>> +        uint8_t sec  = pci_default_read_config(pdev, PCI_SECONDARY_BUS, 1);
>> +        uint8_t sub  = pci_default_read_config(pdev, PCI_SUBORDINATE_BUS, 1);
>> +        uint32_t range[] = { cpu_to_be32(sec), cpu_to_be32(sub) };
>> +
>> +        /*
>> +         * Create these to get existing Linux kernels proceed far enough to
>> +         * trigger resource reassignment. We creates these for vPHB already.
>> +         */
>> +        _FDT(fdt_setprop_cell(fdt, offset, "primary-bus", pri));
>> +        _FDT(fdt_setprop_cell(fdt, offset, "secondary-bus", sec));
>> +        _FDT(fdt_setprop_cell(fdt, offset, "subordinate-bus", sub));
>> +        _FDT(fdt_setprop(fdt, offset, "bus-range", range, sizeof(range)));
>> +        _FDT(fdt_setprop_string(fdt, offset, "device_type", "pci"));
>> +        _FDT(fdt_setprop(fdt, offset, "ranges", NULL, 0));
>> +        owner = OBJECT(pdev);
>>      }
>>  
>>      ret = spapr_dt_drc(fdt, offset, owner,
> 

-- 
Alexey

Re: [PATCH qemu] spapr_pci: Create assigned properties for bridges
Posted by no-reply@patchew.org 4 years, 3 months ago
Patchew URL: https://patchew.org/QEMU/20200129023111.1699-1-aik@ozlabs.ru/



Hi,

This series failed build test on FreeBSD host. Please find the details below.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
if qemu-system-x86_64 --help >/dev/null 2>&1; then
  QEMU=qemu-system-x86_64
elif /usr/libexec/qemu-kvm --help >/dev/null 2>&1; then
  QEMU=/usr/libexec/qemu-kvm
else
  exit 1
fi
make vm-build-freebsd J=21 QEMU=$QEMU
exit 0
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20200129023111.1699-1-aik@ozlabs.ru/testing.FreeBSD/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH qemu] spapr_pci: Create assigned properties for bridges
Posted by no-reply@patchew.org 4 years, 3 months ago
Patchew URL: https://patchew.org/QEMU/20200129023111.1699-1-aik@ozlabs.ru/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20200129023111.1699-1-aik@ozlabs.ru/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com