[Qemu-devel] [PATCH] pci: fixes to allow booting from extra root pci buses.

[Qemu-devel] [PATCH] pci: fixes to allow booting from extra root pci buses.

[Marcel Apfelbaum]

The PXB device exposes a new pci root bridge with the
fw path:  /pci-root@4/..., in which 4 is the root bus number.
Before this patch the fw path was wrongly computed:
    /pci-root@1/pci@i0cf8/...
Fix the above issues: Correct the bus number and remove the
extra host bridge description.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
Laszlo worked on supporting pxb for OVMF and discovered
that there is a problem when booting devices from a PXB.

This is a link to the latest QEMU series:
    https://www.mail-archive.com/qemu-devel@nongnu.org/msg302493.html

Thanks,
Marcel

 src/boot.c   | 1 -
 src/hw/pci.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/boot.c b/src/boot.c
index ec59c37..a3bb13b 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -114,7 +114,6 @@ build_pci_path(char *buf, int max, const char *devname, struct pci_device *pci)
     } else {
         if (pci->rootbus)
             p += snprintf(p, max, "/pci-root@%x", pci->rootbus);
-        p += snprintf(p, buf+max-p, "%s", FW_PCI_DOMAIN);
     }
 
     int dev = pci_bdf_to_dev(pci->bdf), fn = pci_bdf_to_fn(pci->bdf);
diff --git a/src/hw/pci.c b/src/hw/pci.c
index 0379b55..9e77af4 100644
--- a/src/hw/pci.c
+++ b/src/hw/pci.c
@@ -133,7 +133,7 @@ pci_probe_devices(void)
                 if (bus != lastbus)
                     rootbuses++;
                 lastbus = bus;
-                rootbus = rootbuses;
+                rootbus = bus;
                 if (bus > MaxPCIBus)
                     MaxPCIBus = bus;
             } else {
-- 
2.1.0

Re: [Qemu-devel] [PATCH] pci: fixes to allow booting from extra root pci buses.

[Michael S. Tsirkin]

On Thu, Jun 11, 2015 at 03:41:35PM +0300, Marcel Apfelbaum wrote:
> The PXB device exposes a new pci root bridge with the
> fw path:  /pci-root@4/..., in which 4 is the root bus number.
> Before this patch the fw path was wrongly computed:
>     /pci-root@1/pci@i0cf8/...
> Fix the above issues: Correct the bus number and remove the
> extra host bridge description.
> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>

I would like a unit test for various paths, they
are part of guest ABI so we can never change them.
Could you add a unit test please?

Also can you please quote the open firmware spec text
that says this is the correct format?

> ---
> Laszlo worked on supporting pxb for OVMF and discovered
> that there is a problem when booting devices from a PXB.
> 
> This is a link to the latest QEMU series:
>     https://www.mail-archive.com/qemu-devel@nongnu.org/msg302493.html
> 
> Thanks,
> Marcel
> 
>  src/boot.c   | 1 -
>  src/hw/pci.c | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/boot.c b/src/boot.c
> index ec59c37..a3bb13b 100644
> --- a/src/boot.c
> +++ b/src/boot.c
> @@ -114,7 +114,6 @@ build_pci_path(char *buf, int max, const char *devname, struct pci_device *pci)
>      } else {
>          if (pci->rootbus)
>              p += snprintf(p, max, "/pci-root@%x", pci->rootbus);
> -        p += snprintf(p, buf+max-p, "%s", FW_PCI_DOMAIN);
>      }
>  
>      int dev = pci_bdf_to_dev(pci->bdf), fn = pci_bdf_to_fn(pci->bdf);
> diff --git a/src/hw/pci.c b/src/hw/pci.c
> index 0379b55..9e77af4 100644
> --- a/src/hw/pci.c
> +++ b/src/hw/pci.c
> @@ -133,7 +133,7 @@ pci_probe_devices(void)
>                  if (bus != lastbus)
>                      rootbuses++;
>                  lastbus = bus;
> -                rootbus = rootbuses;
> +                rootbus = bus;
>                  if (bus > MaxPCIBus)
>                      MaxPCIBus = bus;
>              } else {
> -- 
> 2.1.0

Re: [Qemu-devel] [PATCH] pci: fixes to allow booting from extra root pci buses.

[Marcel Apfelbaum]

On 06/11/2015 03:45 PM, Michael S. Tsirkin wrote:
> On Thu, Jun 11, 2015 at 03:41:35PM +0300, Marcel Apfelbaum wrote:
>> The PXB device exposes a new pci root bridge with the
>> fw path:  /pci-root@4/..., in which 4 is the root bus number.
>> Before this patch the fw path was wrongly computed:
>>      /pci-root@1/pci@i0cf8/...
>> Fix the above issues: Correct the bus number and remove the
>> extra host bridge description.
>>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>
> I would like a unit test for various paths, they
> are part of guest ABI so we can never change them.
> Could you add a unit test please?
A QEMU unit-test you mean?
By the way, I found an issue with this patch, please do
not merge it yet.

>
> Also can you please quote the open firmware spec text
> that says this is the correct format?
Laszlo has found something, I'll look up something to quote, sure.

Thanks,
Marcel
>
>> ---
>> Laszlo worked on supporting pxb for OVMF and discovered
>> that there is a problem when booting devices from a PXB.
>>
>> This is a link to the latest QEMU series:
>>      https://www.mail-archive.com/qemu-devel@nongnu.org/msg302493.html
>>
>> Thanks,
>> Marcel
>>
>>   src/boot.c   | 1 -
>>   src/hw/pci.c | 2 +-
>>   2 files changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/src/boot.c b/src/boot.c
>> index ec59c37..a3bb13b 100644
>> --- a/src/boot.c
>> +++ b/src/boot.c
>> @@ -114,7 +114,6 @@ build_pci_path(char *buf, int max, const char *devname, struct pci_device *pci)
>>       } else {
>>           if (pci->rootbus)
>>               p += snprintf(p, max, "/pci-root@%x", pci->rootbus);
>> -        p += snprintf(p, buf+max-p, "%s", FW_PCI_DOMAIN);
>>       }
>>
>>       int dev = pci_bdf_to_dev(pci->bdf), fn = pci_bdf_to_fn(pci->bdf);
>> diff --git a/src/hw/pci.c b/src/hw/pci.c
>> index 0379b55..9e77af4 100644
>> --- a/src/hw/pci.c
>> +++ b/src/hw/pci.c
>> @@ -133,7 +133,7 @@ pci_probe_devices(void)
>>                   if (bus != lastbus)
>>                       rootbuses++;
>>                   lastbus = bus;
>> -                rootbus = rootbuses;
>> +                rootbus = bus;
>>                   if (bus > MaxPCIBus)
>>                       MaxPCIBus = bus;
>>               } else {
>> --
>> 2.1.0

Re: [Qemu-devel] [PATCH] pci: fixes to allow booting from extra root pci buses.

[Laszlo Ersek]

On 06/11/15 14:51, Marcel Apfelbaum wrote:
> On 06/11/2015 03:45 PM, Michael S. Tsirkin wrote:
>> On Thu, Jun 11, 2015 at 03:41:35PM +0300, Marcel Apfelbaum wrote:
>>> The PXB device exposes a new pci root bridge with the
>>> fw path:  /pci-root@4/..., in which 4 is the root bus number.
>>> Before this patch the fw path was wrongly computed:
>>>      /pci-root@1/pci@i0cf8/...
>>> Fix the above issues: Correct the bus number and remove the
>>> extra host bridge description.
>>>
>>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>>
>> I would like a unit test for various paths, they
>> are part of guest ABI so we can never change them.
>> Could you add a unit test please?
> A QEMU unit-test you mean?
> By the way, I found an issue with this patch, please do
> not merge it yet.
> 
>>
>> Also can you please quote the open firmware spec text
>> that says this is the correct format?
> Laszlo has found something, I'll look up something to quote, sure.

The OFW specification has been named (and even linked) in the history of
SeaBIOS, see eg. Markus's recent commit 275672eb. The spec has a base
part and some bindings.

The thing I quoted earlier was IEEE Std 1275-1994:

  IEEE Standard for Boot (Initialization Configuration)
  Firmware: Core Requirements and Practices

  3.2.1.1 Node names

for the generic syntax rules. (I'm not linking it because it seems to
live at a different place every second week, so go ahead and google it.
For example, the particular binding referenced in Markus's commit
275672eb is not available at that location any longer.)

Thanks
Laszlo

> 
> Thanks,
> Marcel
>>
>>> ---
>>> Laszlo worked on supporting pxb for OVMF and discovered
>>> that there is a problem when booting devices from a PXB.
>>>
>>> This is a link to the latest QEMU series:
>>>      https://www.mail-archive.com/qemu-devel@nongnu.org/msg302493.html
>>>
>>> Thanks,
>>> Marcel
>>>
>>>   src/boot.c   | 1 -
>>>   src/hw/pci.c | 2 +-
>>>   2 files changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/src/boot.c b/src/boot.c
>>> index ec59c37..a3bb13b 100644
>>> --- a/src/boot.c
>>> +++ b/src/boot.c
>>> @@ -114,7 +114,6 @@ build_pci_path(char *buf, int max, const char
>>> *devname, struct pci_device *pci)
>>>       } else {
>>>           if (pci->rootbus)
>>>               p += snprintf(p, max, "/pci-root@%x", pci->rootbus);
>>> -        p += snprintf(p, buf+max-p, "%s", FW_PCI_DOMAIN);
>>>       }
>>>
>>>       int dev = pci_bdf_to_dev(pci->bdf), fn = pci_bdf_to_fn(pci->bdf);
>>> diff --git a/src/hw/pci.c b/src/hw/pci.c
>>> index 0379b55..9e77af4 100644
>>> --- a/src/hw/pci.c
>>> +++ b/src/hw/pci.c
>>> @@ -133,7 +133,7 @@ pci_probe_devices(void)
>>>                   if (bus != lastbus)
>>>                       rootbuses++;
>>>                   lastbus = bus;
>>> -                rootbus = rootbuses;
>>> +                rootbus = bus;
>>>                   if (bus > MaxPCIBus)
>>>                       MaxPCIBus = bus;
>>>               } else {
>>> -- 
>>> 2.1.0
>