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 <alex.william...@redhat.com>
---
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 <mar...@redhat.com>
Thanks,
Alex
