[SeaBIOS] [PATCH] pci: fix 'io hints' capability for RedHat PCI bridges

Marcel Apfelbaum posted 1 patch 48 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/seabios tags/patchew/20180111201512.43546-1-marcel@redhat.com
src/fw/pciinit.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)

[SeaBIOS] [PATCH] pci: fix 'io hints' capability for RedHat PCI bridges

Posted by Marcel Apfelbaum 48 weeks ago
Commit ec6cb17f (pci: enable RedHat PCI bridges to reserve additional
                 resources on PCI init)
added a new vendor specific PCI capability for RedHat PCI bridges
allowing them to reserve additional buses and/or IO/MEM space.

When adding the IO hints PCI capability to the pcie-root-port
without specifying a value for bus reservation, the subordinate bus
computation is wrong and the guest kernel gets messed up.

Fix it by returning to prev code if the value for bus
reservation is not set.

Removed also a wrong debug print "PCI: invalid QEMU resource reserve
cap offset" which appears if the 'IO hints' capability is not present.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 src/fw/pciinit.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index 7f0e439..3a2f747 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -540,8 +540,6 @@ static u8 pci_find_resource_reserve_capability(u16 bdf)
                 dprintf(1, "PCI: QEMU resource reserve cap length %d is invalid\n",
                         cap_len);
             }
-        } else {
-            dprintf(1, "PCI: invalid QEMU resource reserve cap offset\n");
         }
         return cap;
     } else {
@@ -619,13 +617,11 @@ pci_bios_init_bus_rec(int bus, u8 *pci_bus)
                                 res_bus);
                         res_bus = 0;
                     }
-                }
-                if (secbus + res_bus > *pci_bus) {
-                    dprintf(1, "PCI: QEMU resource reserve cap: bus = %u\n",
-                            res_bus);
-                    res_bus = secbus + res_bus;
-                } else {
-                    res_bus = *pci_bus;
+                    if (secbus + res_bus > *pci_bus) {
+                        dprintf(1, "PCI: QEMU resource reserve cap: bus = %u\n",
+                                res_bus);
+                        res_bus = secbus + res_bus;
+                    }
                 }
             }
             dprintf(1, "PCI: subordinate bus = 0x%x -> 0x%x\n",
-- 
2.13.5


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] pci: fix 'io hints' capability for RedHat PCI bridges

Posted by Marcel Apfelbaum 48 weeks ago
Hi,

I forgot to CC Kevin and Aleksandr (the original committer).

Thanks,
Marcel

On 11/01/2018 22:15, Marcel Apfelbaum wrote:
> Commit ec6cb17f (pci: enable RedHat PCI bridges to reserve additional
>                   resources on PCI init)
> added a new vendor specific PCI capability for RedHat PCI bridges
> allowing them to reserve additional buses and/or IO/MEM space.
> 
> When adding the IO hints PCI capability to the pcie-root-port
> without specifying a value for bus reservation, the subordinate bus
> computation is wrong and the guest kernel gets messed up.
> 
> Fix it by returning to prev code if the value for bus
> reservation is not set.
> 
> Removed also a wrong debug print "PCI: invalid QEMU resource reserve
> cap offset" which appears if the 'IO hints' capability is not present.
> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
>   src/fw/pciinit.c | 14 +++++---------
>   1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index 7f0e439..3a2f747 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -540,8 +540,6 @@ static u8 pci_find_resource_reserve_capability(u16 bdf)
>                   dprintf(1, "PCI: QEMU resource reserve cap length %d is invalid\n",
>                           cap_len);
>               }
> -        } else {
> -            dprintf(1, "PCI: invalid QEMU resource reserve cap offset\n");
>           }
>           return cap;
>       } else {
> @@ -619,13 +617,11 @@ pci_bios_init_bus_rec(int bus, u8 *pci_bus)
>                                   res_bus);
>                           res_bus = 0;
>                       }
> -                }
> -                if (secbus + res_bus > *pci_bus) {
> -                    dprintf(1, "PCI: QEMU resource reserve cap: bus = %u\n",
> -                            res_bus);
> -                    res_bus = secbus + res_bus;
> -                } else {
> -                    res_bus = *pci_bus;
> +                    if (secbus + res_bus > *pci_bus) {
> +                        dprintf(1, "PCI: QEMU resource reserve cap: bus = %u\n",
> +                                res_bus);
> +                        res_bus = secbus + res_bus;
> +                    }
>                   }
>               }
>               dprintf(1, "PCI: subordinate bus = 0x%x -> 0x%x\n",
> 

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] pci: fix 'io hints' capability for RedHat PCI bridges

Posted by Michael S. Tsirkin 48 weeks ago
On Thu, Jan 11, 2018 at 10:15:12PM +0200, Marcel Apfelbaum wrote:
> Commit ec6cb17f (pci: enable RedHat PCI bridges to reserve additional
>                  resources on PCI init)
> added a new vendor specific PCI capability for RedHat PCI bridges
> allowing them to reserve additional buses and/or IO/MEM space.
> 
> When adding the IO hints PCI capability to the pcie-root-port
> without specifying a value for bus reservation, the subordinate bus
> computation is wrong and the guest kernel gets messed up.
> 
> Fix it by returning to prev code if the value for bus
> reservation is not set.
> 
> Removed also a wrong debug print "PCI: invalid QEMU resource reserve
> cap offset" which appears if the 'IO hints' capability is not present.
> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  src/fw/pciinit.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index 7f0e439..3a2f747 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -540,8 +540,6 @@ static u8 pci_find_resource_reserve_capability(u16 bdf)
>                  dprintf(1, "PCI: QEMU resource reserve cap length %d is invalid\n",
>                          cap_len);
>              }
> -        } else {
> -            dprintf(1, "PCI: invalid QEMU resource reserve cap offset\n");
>          }
>          return cap;
>      } else {
> @@ -619,13 +617,11 @@ pci_bios_init_bus_rec(int bus, u8 *pci_bus)
>                                  res_bus);
>                          res_bus = 0;
>                      }
> -                }
> -                if (secbus + res_bus > *pci_bus) {
> -                    dprintf(1, "PCI: QEMU resource reserve cap: bus = %u\n",
> -                            res_bus);
> -                    res_bus = secbus + res_bus;
> -                } else {
> -                    res_bus = *pci_bus;
> +                    if (secbus + res_bus > *pci_bus) {
> +                        dprintf(1, "PCI: QEMU resource reserve cap: bus = %u\n",
> +                                res_bus);
> +                        res_bus = secbus + res_bus;
> +                    }
>                  }
>              }
>              dprintf(1, "PCI: subordinate bus = 0x%x -> 0x%x\n",
> -- 
> 2.13.5

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] pci: fix 'io hints' capability for RedHat PCI bridges

Posted by Kevin O'Connor 47 weeks ago
On Thu, Jan 11, 2018 at 10:15:12PM +0200, Marcel Apfelbaum wrote:
> Commit ec6cb17f (pci: enable RedHat PCI bridges to reserve additional
>                  resources on PCI init)
> added a new vendor specific PCI capability for RedHat PCI bridges
> allowing them to reserve additional buses and/or IO/MEM space.
> 
> When adding the IO hints PCI capability to the pcie-root-port
> without specifying a value for bus reservation, the subordinate bus
> computation is wrong and the guest kernel gets messed up.
> 
> Fix it by returning to prev code if the value for bus
> reservation is not set.
> 
> Removed also a wrong debug print "PCI: invalid QEMU resource reserve
> cap offset" which appears if the 'IO hints' capability is not present.
> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks.  I committed this change.

-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios