Re: [SeaBIOS] [PATCH v3 3/3] pci: recognize RH PCI legacy bridge resource reservation capability

[Marcel Apfelbaum]

On 08/27/2018 05:22 AM, Liu, Jing2 wrote:

Hi Marcel,

On 8/25/2018 11:59 PM, Marcel Apfelbaum wrote:

On 08/24/2018 11:53 AM, Jing Liu wrote:

Enable the firmware recognizing RedHat legacy PCI bridge device ID,
so QEMU can reserve additional PCI bridge resource capability.
Change the debug level lower to 3 when it is non-QEMU bridge.

Signed-off-by: Jing Liu <jing2....@linux.intel.com>
---

  src/fw/pciinit.c | 50 +++++++++++++++++++++++++++++---------------------

  src/hw/pci_ids.h |  1 +
  2 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index 62a32f1..c0634bc 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -525,30 +525,38 @@ static void pci_bios_init_platform(void)
  static u8 pci_find_resource_reserve_capability(u16 bdf)
  {

-    if (pci_config_readw(bdf, PCI_VENDOR_ID) == PCI_VENDOR_ID_REDHAT &&

-        pci_config_readw(bdf, PCI_DEVICE_ID) ==
-                PCI_DEVICE_ID_REDHAT_ROOT_PORT) {
-        u8 cap = 0;
-        do {
-            cap = pci_find_capability(bdf, PCI_CAP_ID_VNDR, cap);
-        } while (cap &&

-                 pci_config_readb(bdf, cap + PCI_CAP_REDHAT_TYPE_OFFSET) !=

-                        REDHAT_CAP_RESOURCE_RESERVE);
-        if (cap) {
-            u8 cap_len = pci_config_readb(bdf, cap + PCI_CAP_FLAGS);
-            if (cap_len < RES_RESERVE_CAP_SIZE) {

-                dprintf(1, "PCI: QEMU resource reserve cap length %d is invalid\n",

-                        cap_len);
-                return 0;
-            }
-        } else {
-            dprintf(1, "PCI: QEMU resource reserve cap not found\n");
+    u16 device_id;
+

+    if (pci_config_readw(bdf, PCI_VENDOR_ID) != PCI_VENDOR_ID_REDHAT) {

+        dprintf(3, "PCI: This is non-QEMU bridge.\n");
+        return 0;
+    }
+
+    device_id = pci_config_readw(bdf, PCI_DEVICE_ID);
+
+    if (device_id != PCI_DEVICE_ID_REDHAT_ROOT_PORT &&
+        device_id != PCI_DEVICE_ID_REDHAT_BRIDGE) {

+        dprintf(1, "PCI: QEMU resource reserve cap device ID doesn't match.\n");

+        return 0;
+    }
+    u8 cap = 0;
+
+    do {
+        cap = pci_find_capability(bdf, PCI_CAP_ID_VNDR, cap);
+    } while (cap &&

+             pci_config_readb(bdf, cap + PCI_CAP_REDHAT_TYPE_OFFSET) !=

+                              REDHAT_CAP_RESOURCE_RESERVE);
+    if (cap) {
+        u8 cap_len = pci_config_readb(bdf, cap + PCI_CAP_FLAGS);
+        if (cap_len < RES_RESERVE_CAP_SIZE) {

+            dprintf(1, "PCI: QEMU resource reserve cap length %d is invalid\n",

+                    cap_len);
+            return 0;
          }
-        return cap;
      } else {

-        dprintf(1, "PCI: QEMU resource reserve cap VID or DID doesn't match.\n");

-        return 0;

I am sorry for the late review.
Did you drop the above line in purpose?

Thanks for the review!

I replaced the above report to following phase.
Check the vendor-id and device-id respectively.

+    if (pci_config_readw(bdf, PCI_VENDOR_ID) != PCI_VENDOR_ID_REDHAT) {
+        dprintf(3, "PCI: This is non-QEMU bridge.\n");
+        return 0;
+    }
+
+    device_id = pci_config_readw(bdf, PCI_DEVICE_ID);
+
+    if (device_id != PCI_DEVICE_ID_REDHAT_ROOT_PORT &&
+        device_id != PCI_DEVICE_ID_REDHAT_BRIDGE) {

+        dprintf(1, "PCI: QEMU resource reserve cap device ID doesn't match.\n");

+        return 0;
+    }

I understand.

Reviewed-by: Marcel Apfelbaum<marcel.apfelb...@gmail.com>


Thanks,
Marcel

Thanks,
Jing

Thanks,
Marcel

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