qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RE-RESEND PATCH] pci: Adjust PCI config limit based on


From: Marcel Apfelbaum
Subject: Re: [Qemu-devel] [RE-RESEND PATCH] pci: Adjust PCI config limit based on bus topology
Date: Tue, 19 Jan 2016 18:48:14 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0

On 01/19/2016 06:38 PM, Alex Williamson wrote:
On Tue, 2016-01-19 at 10:54 +0200, Marcel Apfelbaum wrote:
On 01/19/2016 01:06 AM, Alex Williamson wrote:
A conventional PCI bus does not support config space accesses above
the standard 256 byte configuration space.  PCIe-to-PCI bridges are
not permitted to forward transactions if the extended register
address
field is non-zero and must handle it as an unsupported request
(PCIe
bridge spec rev 1.0, 4.1.3, 4.1.4).  Therefore, we should not
support
extended config space if there is a conventional bus anywhere on
the
path to a device.

Signed-off-by: Alex Williamson <address@hidden>
---
Previous postings:
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg05384.html
https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg02422.html

   hw/pci/pci_host.c |   26 ++++++++++++++++++++++++++
   1 file changed, 26 insertions(+)

diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 49f59a5..3a3e294 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -19,6 +19,7 @@
    */

   #include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
   #include "hw/pci/pci_host.h"
   #include "hw/pci/pci_bus.h"
   #include "trace.h"
@@ -49,9 +50,29 @@ static inline PCIDevice
*pci_dev_find_by_addr(PCIBus *bus, uint32_t addr)
       return pci_find_device(bus, bus_num, devfn);
   }

+static void pci_adjust_config_limit(PCIBus *bus, uint32_t *limit)
+{
+    if (*limit > PCI_CONFIG_SPACE_SIZE) {
+        if (!pci_bus_is_express(bus)) {
+            *limit = PCI_CONFIG_SPACE_SIZE;
+            return;
+        }
+
+        if (!pci_bus_is_root(bus)) {
+            PCIDevice *bridge = pci_bridge_get_device(bus);
+            pci_adjust_config_limit(bridge->bus, limit);
+        }
+    }
+}
+
   void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t
addr,
                                     uint32_t limit, uint32_t val,
uint32_t len)
   {
+    pci_adjust_config_limit(pci_dev->bus, &limit);
+    if (limit <= addr) {
+        return;
+    }
+
       assert(len <= 4);
       /* non-zero functions are only exposed when function 0 is
present,
        * allowing direct removal of unexposed functions.
@@ -70,6 +91,11 @@ uint32_t pci_host_config_read_common(PCIDevice
*pci_dev, uint32_t addr,
   {
       uint32_t ret;

+    pci_adjust_config_limit(pci_dev->bus, &limit);
+    if (limit <= addr) {
+        return ~0x0;
+    }
+
       assert(len <= 4);
       /* non-zero functions are only exposed when function 0 is
present,
        * allowing direct removal of unexposed functions.



Quick question: could we check the limit as part of pci_config_size?

If we plugin a physical PCIe card behind a bridge that masks access to
the extended configuration space, does the config size for that card
change?  No,  it's up to the bridge to drop the transactions, which
seems like how we probably want to handle it in QEMU as well.

Is what I thought.

Thanks,
Marcel


Anyway, it looks OK to me.

Reviewed-by: Marcel Apfelbaum <address@hidden>

Thanks,
Alex





reply via email to

[Prev in Thread] Current Thread [Next in Thread]