[kvm-unit-tests PATCH v9 00/12] PCI bus support

[kvm-unit-tests PATCH v9 00/12] PCI bus support

[Alexander Gordeev]

Hi Andrew et al,

This is 9th version of PCI support. Your latest suggestions are incorporated.

Sources are avalable at:
https://github.com/a-gordeev/kvm-unit-tests.git pci-testdev-v9

Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>


Alexander Gordeev (12):
  pci: Fix coding style in generic PCI files
  pci: x86: Rename pci_config_read() to pci_config_readl()
  pci: Add 'extern' to public function declarations
  pci: x86: Add remaining PCI configuration space accessors
  pci: Factor out pci_bar_get()
  pci: Rework pci_bar_addr()
  pci: Add pci_bar_set_addr()
  pci: Add pci_dev_exists()
  pci: Add pci_print()
  pci: Add generic ECAM host support
  pci: Add pci-testdev PCI bus test device
  arm/arm64: pci: Add pci-testdev PCI device operation test

 arm/Makefile.common               |   6 +-
 arm/pci-test.c                    |  27 ++++
 arm/run                           |   7 +-
 arm/unittests.cfg                 |   4 +
 lib/arm/asm/pci.h                 |   1 +
 lib/arm64/asm/pci.h               |   1 +
 lib/asm-generic/pci-host-bridge.h |  26 ++++
 lib/pci-host-generic.c            | 305 ++++++++++++++++++++++++++++++++++++++
 lib/pci-host-generic.h            |  46 ++++++
 lib/pci-testdev.c                 | 192 ++++++++++++++++++++++++
 lib/pci.c                         | 212 +++++++++++++++++++++++---
 lib/pci.h                         |  43 +++++-
 lib/x86/asm/pci.h                 |  31 +++-
 x86/vmexit.c                      |   4 +-
 14 files changed, 874 insertions(+), 31 deletions(-)
 create mode 100644 arm/pci-test.c
 create mode 100644 lib/arm/asm/pci.h
 create mode 100644 lib/arm64/asm/pci.h
 create mode 100644 lib/asm-generic/pci-host-bridge.h
 create mode 100644 lib/pci-host-generic.c
 create mode 100644 lib/pci-host-generic.h
 create mode 100644 lib/pci-testdev.c

-- 
1.8.3.1

[kvm-unit-tests PATCH v9 01/12] pci: Fix coding style in generic PCI files

[Alexander Gordeev]

Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/pci.c | 38 ++++++++++++++++++++------------------
 lib/pci.h |  3 ++-
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/lib/pci.c b/lib/pci.c
index 0058d70c888d..43cd0ea4f2fa 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -10,34 +10,36 @@
 /* Scan bus look for a specific device. Only bus 0 scanned for now. */
 pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
 {
-    unsigned dev;
-    for (dev = 0; dev < 256; ++dev) {
-    uint32_t id = pci_config_read(dev, 0);
-    if ((id & 0xFFFF) == vendor_id && (id >> 16) == device_id) {
-        return dev;
-    }
-    }
-    return PCIDEVADDR_INVALID;
+	pcidevaddr_t dev;
+
+	for (dev = 0; dev < 256; ++dev) {
+		uint32_t id = pci_config_read(dev, 0);
+
+		if ((id & 0xFFFF) == vendor_id && (id >> 16) == device_id)
+			return dev;
+	}
+
+	return PCIDEVADDR_INVALID;
 }
 
 unsigned long pci_bar_addr(pcidevaddr_t dev, int bar_num)
 {
-    uint32_t bar = pci_config_read(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
-    if (bar & PCI_BASE_ADDRESS_SPACE_IO) {
-        return bar & PCI_BASE_ADDRESS_IO_MASK;
-    } else {
-        return bar & PCI_BASE_ADDRESS_MEM_MASK;
-    }
+	uint32_t bar = pci_config_read(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
+
+	if (bar & PCI_BASE_ADDRESS_SPACE_IO)
+		return bar & PCI_BASE_ADDRESS_IO_MASK;
+	else
+		return bar & PCI_BASE_ADDRESS_MEM_MASK;
 }
 
 bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num)
 {
-    uint32_t bar = pci_config_read(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
-    return !(bar & PCI_BASE_ADDRESS_SPACE_IO);
+	uint32_t bar = pci_config_read(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
+
+	return !(bar & PCI_BASE_ADDRESS_SPACE_IO);
 }
 
 bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num)
 {
-    uint32_t bar = pci_config_read(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
-    return bar;
+	return pci_config_read(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
 }
diff --git a/lib/pci.h b/lib/pci.h
index 9160cfb5950d..54fbf22d634a 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -12,8 +12,9 @@
 
 typedef uint16_t pcidevaddr_t;
 enum {
-    PCIDEVADDR_INVALID = 0xffff,
+	PCIDEVADDR_INVALID = 0xffff,
 };
+
 pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
 unsigned long pci_bar_addr(pcidevaddr_t dev, int bar_num);
 bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num);
-- 
1.8.3.1

[kvm-unit-tests PATCH v9 02/12] pci: x86: Rename pci_config_read() to pci_config_readl()

[Alexander Gordeev]

Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/pci.c         | 8 ++++----
 lib/x86/asm/pci.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/pci.c b/lib/pci.c
index 43cd0ea4f2fa..e0b4514244f6 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -13,7 +13,7 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
 	pcidevaddr_t dev;
 
 	for (dev = 0; dev < 256; ++dev) {
-		uint32_t id = pci_config_read(dev, 0);
+		uint32_t id = pci_config_readl(dev, 0);
 
 		if ((id & 0xFFFF) == vendor_id && (id >> 16) == device_id)
 			return dev;
@@ -24,7 +24,7 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
 
 unsigned long pci_bar_addr(pcidevaddr_t dev, int bar_num)
 {
-	uint32_t bar = pci_config_read(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
+	uint32_t bar = pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
 
 	if (bar & PCI_BASE_ADDRESS_SPACE_IO)
 		return bar & PCI_BASE_ADDRESS_IO_MASK;
@@ -34,12 +34,12 @@ unsigned long pci_bar_addr(pcidevaddr_t dev, int bar_num)
 
 bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num)
 {
-	uint32_t bar = pci_config_read(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
+	uint32_t bar = pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
 
 	return !(bar & PCI_BASE_ADDRESS_SPACE_IO);
 }
 
 bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num)
 {
-	return pci_config_read(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
+	return pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
 }
diff --git a/lib/x86/asm/pci.h b/lib/x86/asm/pci.h
index cddde41a7d97..d00438fe91e4 100644
--- a/lib/x86/asm/pci.h
+++ b/lib/x86/asm/pci.h
@@ -9,7 +9,7 @@
 #include "pci.h"
 #include "x86/asm/io.h"
 
-static inline uint32_t pci_config_read(pcidevaddr_t dev, uint8_t reg)
+static inline uint32_t pci_config_readl(pcidevaddr_t dev, uint8_t reg)
 {
     uint32_t index = reg | (dev << 8) | (0x1 << 31);
     outl(index, 0xCF8);
-- 
1.8.3.1

[kvm-unit-tests PATCH v9 03/12] pci: Add 'extern' to public function declarations

[Alexander Gordeev]

Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/pci.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/pci.h b/lib/pci.h
index 54fbf22d634a..066fac77b237 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -15,10 +15,10 @@ enum {
 	PCIDEVADDR_INVALID = 0xffff,
 };
 
-pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
-unsigned long pci_bar_addr(pcidevaddr_t dev, int bar_num);
-bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num);
-bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num);
+extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
+extern unsigned long pci_bar_addr(pcidevaddr_t dev, int bar_num);
+extern bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num);
+extern bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num);
 
 /*
  * pci-testdev is a driver for the pci-testdev qemu pci device. The
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH v9 03/12] pci: Add 'extern' to public function declarations

[Christoph Hellwig]

Why?  The extern keyword for function prototypes is a no-op.

Re: [kvm-unit-tests PATCH v9 03/12] pci: Add 'extern' to public function declarations

[Thomas Huth]

On 03.11.2016 21:30, Christoph Hellwig wrote:
> Why?  The extern keyword for function prototypes is a no-op.

It's used in many of the kvm-unit-test header files already, so I guess
the code looks a little bit more consistent this way.

 Thomas

Re: [kvm-unit-tests PATCH v9 03/12] pci: Add 'extern' to public function declarations

[Andrew Jones]

On Fri, Nov 04, 2016 at 08:36:05AM +0100, Thomas Huth wrote:
> On 03.11.2016 21:30, Christoph Hellwig wrote:
> > Why?  The extern keyword for function prototypes is a no-op.
> 
> It's used in many of the kvm-unit-test header files already, so I guess
> the code looks a little bit more consistent this way.

Yeah, it's a bit of an annoying, historic trait of kvm-unit-tests.
Until we do a big extern removing patch to clean those up, then I
prefer we keep things consistent.

Thanks,
drew

[kvm-unit-tests PATCH v9 04/12] pci: x86: Add remaining PCI configuration space accessors

[Alexander Gordeev]

Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/pci.c         |  5 ++---
 lib/x86/asm/pci.h | 23 +++++++++++++++++++++--
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/lib/pci.c b/lib/pci.c
index e0b4514244f6..b05ecfa3f3da 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -13,9 +13,8 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
 	pcidevaddr_t dev;
 
 	for (dev = 0; dev < 256; ++dev) {
-		uint32_t id = pci_config_readl(dev, 0);
-
-		if ((id & 0xFFFF) == vendor_id && (id >> 16) == device_id)
+		if (pci_config_readw(dev, PCI_VENDOR_ID) == vendor_id &&
+		    pci_config_readw(dev, PCI_DEVICE_ID) == device_id)
 			return dev;
 	}
 
diff --git a/lib/x86/asm/pci.h b/lib/x86/asm/pci.h
index d00438fe91e4..821a2c1e180a 100644
--- a/lib/x86/asm/pci.h
+++ b/lib/x86/asm/pci.h
@@ -9,11 +9,30 @@
 #include "pci.h"
 #include "x86/asm/io.h"
 
+#define PCI_CONF1_ADDRESS(dev, reg)	((0x1 << 31) | (dev << 8) | reg)
+
+static inline uint8_t pci_config_readb(pcidevaddr_t dev, uint8_t reg)
+{
+    outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
+    return inb(0xCFC);
+}
+
+static inline uint16_t pci_config_readw(pcidevaddr_t dev, uint8_t reg)
+{
+    outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
+    return inw(0xCFC);
+}
+
 static inline uint32_t pci_config_readl(pcidevaddr_t dev, uint8_t reg)
 {
-    uint32_t index = reg | (dev << 8) | (0x1 << 31);
-    outl(index, 0xCF8);
+    outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
     return inl(0xCFC);
 }
 
+static inline void pci_config_writel(pcidevaddr_t dev, uint8_t reg, uint32_t val)
+{
+    outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
+    outl(val, 0xCFC);
+}
+
 #endif
-- 
1.8.3.1

[kvm-unit-tests PATCH v9 05/12] pci: Factor out pci_bar_get()

[Alexander Gordeev]

Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/pci.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/pci.c b/lib/pci.c
index b05ecfa3f3da..ce481bbfadb6 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -21,9 +21,14 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
 	return PCIDEVADDR_INVALID;
 }
 
+static uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num)
+{
+	return pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
+}
+
 unsigned long pci_bar_addr(pcidevaddr_t dev, int bar_num)
 {
-	uint32_t bar = pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
+	uint32_t bar = pci_bar_get(dev, bar_num);
 
 	if (bar & PCI_BASE_ADDRESS_SPACE_IO)
 		return bar & PCI_BASE_ADDRESS_IO_MASK;
@@ -33,12 +38,12 @@ unsigned long pci_bar_addr(pcidevaddr_t dev, int bar_num)
 
 bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num)
 {
-	uint32_t bar = pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
+	uint32_t bar = pci_bar_get(dev, bar_num);
 
 	return !(bar & PCI_BASE_ADDRESS_SPACE_IO);
 }
 
 bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num)
 {
-	return pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
+	return pci_bar_get(dev, bar_num);
 }
-- 
1.8.3.1

[kvm-unit-tests PATCH v9 06/12] pci: Rework pci_bar_addr()

[Alexander Gordeev]

This update makes pci_bar_addr() interface 64 bit BARs aware and
introduces a concept of PCI address translation.

An architecutre should implement pci_translate_addr() interface
in order to provide mapping between PCI bus address and CPU
physical address.

Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/pci.c         | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 lib/pci.h         | 17 ++++++++++++-
 lib/x86/asm/pci.h |  6 +++++
 3 files changed, 90 insertions(+), 6 deletions(-)

diff --git a/lib/pci.c b/lib/pci.c
index ce481bbfadb6..3bf45fb1d5bf 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -21,19 +21,71 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
 	return PCIDEVADDR_INVALID;
 }
 
+static uint32_t pci_bar_mask(uint32_t bar)
+{
+	return (bar & PCI_BASE_ADDRESS_SPACE_IO) ?
+		PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
+}
+
 static uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num)
 {
 	return pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
 }
 
-unsigned long pci_bar_addr(pcidevaddr_t dev, int bar_num)
+phys_addr_t pci_bar_addr(pcidevaddr_t dev, int bar_num)
 {
 	uint32_t bar = pci_bar_get(dev, bar_num);
+	uint32_t mask = pci_bar_mask(bar);
+	uint64_t addr = bar & mask;
 
-	if (bar & PCI_BASE_ADDRESS_SPACE_IO)
-		return bar & PCI_BASE_ADDRESS_IO_MASK;
-	else
-		return bar & PCI_BASE_ADDRESS_MEM_MASK;
+	if (pci_bar_is64(dev, bar_num))
+		addr |= (uint64_t)pci_bar_get(dev, bar_num + 1) << 32;
+
+	return pci_translate_addr(dev, addr);
+}
+
+/*
+ * To determine the amount of address space needed by a PCI device,
+ * one must save the original value of the BAR, write a value of
+ * all 1's to the register, and then read it back. The amount of
+ * memory can be then determined by masking the information bits,
+ * performing a bitwise NOT, and incrementing the value by 1.
+ *
+ * The following pci_bar_size_helper() and pci_bar_size() functions
+ * implement the algorithm.
+ */
+static uint32_t pci_bar_size_helper(pcidevaddr_t dev, int bar_num)
+{
+	int off = PCI_BASE_ADDRESS_0 + bar_num * 4;
+	uint32_t bar, val;
+
+	bar = pci_config_readl(dev, off);
+	pci_config_writel(dev, off, ~0u);
+	val = pci_config_readl(dev, off);
+	pci_config_writel(dev, off, bar);
+
+	return val;
+}
+
+phys_addr_t pci_bar_size(pcidevaddr_t dev, int bar_num)
+{
+	uint32_t bar, size;
+
+	size = pci_bar_size_helper(dev, bar_num);
+	if (!size)
+		return 0;
+
+	bar = pci_bar_get(dev, bar_num);
+	size &= pci_bar_mask(bar);
+
+	if (pci_bar_is64(dev, bar_num)) {
+		phys_addr_t size64 = pci_bar_size_helper(dev, bar_num + 1);
+		size64 = (size64 << 32) | size;
+
+		return ~size64 + 1;
+	} else {
+		return ~size + 1;
+	}
 }
 
 bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num)
@@ -47,3 +99,14 @@ bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num)
 {
 	return pci_bar_get(dev, bar_num);
 }
+
+bool pci_bar_is64(pcidevaddr_t dev, int bar_num)
+{
+	uint32_t bar = pci_bar_get(dev, bar_num);
+
+	if (bar & PCI_BASE_ADDRESS_SPACE_IO)
+		return false;
+
+	return (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) ==
+		      PCI_BASE_ADDRESS_MEM_TYPE_64;
+}
diff --git a/lib/pci.h b/lib/pci.h
index 066fac77b237..8eec236cb123 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -16,7 +16,22 @@ enum {
 };
 
 extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
-extern unsigned long pci_bar_addr(pcidevaddr_t dev, int bar_num);
+
+/*
+ * @bar_num in all BAR access functions below is the index of the 32-bit
+ * register starting from the PCI_BASE_ADDRESS_0 offset.
+ *
+ * In cases where the BAR size is 64-bit, a caller should still provide
+ * @bar_num in terms of 32-bit words. For example, if a device has a 64-bit
+ * BAR#0 and a 32-bit BAR#1, then caller should provide 2 to address BAR#1,
+ * not 1.
+ *
+ * It is expected the caller is aware of the device BAR layout and never
+ * tries to address the middle of a 64-bit register.
+ */
+extern phys_addr_t pci_bar_addr(pcidevaddr_t dev, int bar_num);
+extern phys_addr_t pci_bar_size(pcidevaddr_t dev, int bar_num);
+extern bool pci_bar_is64(pcidevaddr_t dev, int bar_num);
 extern bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num);
 extern bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num);
 
diff --git a/lib/x86/asm/pci.h b/lib/x86/asm/pci.h
index 821a2c1e180a..7384b91adba1 100644
--- a/lib/x86/asm/pci.h
+++ b/lib/x86/asm/pci.h
@@ -35,4 +35,10 @@ static inline void pci_config_writel(pcidevaddr_t dev, uint8_t reg, uint32_t val
     outl(val, 0xCFC);
 }
 
+static inline
+phys_addr_t pci_translate_addr(pcidevaddr_t dev __unused, uint64_t addr)
+{
+    return addr;
+}
+
 #endif
-- 
1.8.3.1

[kvm-unit-tests PATCH v9 07/12] pci: Add pci_bar_set_addr()

[Alexander Gordeev]

Because the counterpart to pci_bar_set_addr() setter is
pci_bar_addr() getter, these names become inconsistent.
Rename pci_bar_addr() to pci_bar_get_addr() also to make
the resulting names conform to each other.

Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/pci.c    | 12 +++++++++++-
 lib/pci.h    |  3 ++-
 x86/vmexit.c |  4 ++--
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/lib/pci.c b/lib/pci.c
index 3bf45fb1d5bf..74936f32ab54 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -32,7 +32,7 @@ static uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num)
 	return pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
 }
 
-phys_addr_t pci_bar_addr(pcidevaddr_t dev, int bar_num)
+phys_addr_t pci_bar_get_addr(pcidevaddr_t dev, int bar_num)
 {
 	uint32_t bar = pci_bar_get(dev, bar_num);
 	uint32_t mask = pci_bar_mask(bar);
@@ -44,6 +44,16 @@ phys_addr_t pci_bar_addr(pcidevaddr_t dev, int bar_num)
 	return pci_translate_addr(dev, addr);
 }
 
+void pci_bar_set_addr(pcidevaddr_t dev, int bar_num, phys_addr_t addr)
+{
+	int off = PCI_BASE_ADDRESS_0 + bar_num * 4;
+
+	pci_config_writel(dev, off, (uint32_t)addr);
+
+	if (pci_bar_is64(dev, bar_num))
+		pci_config_writel(dev, off + 4, (uint32_t)(addr >> 32));
+}
+
 /*
  * To determine the amount of address space needed by a PCI device,
  * one must save the original value of the BAR, write a value of
diff --git a/lib/pci.h b/lib/pci.h
index 8eec236cb123..1c20308c9344 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -29,7 +29,8 @@ extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
  * It is expected the caller is aware of the device BAR layout and never
  * tries to address the middle of a 64-bit register.
  */
-extern phys_addr_t pci_bar_addr(pcidevaddr_t dev, int bar_num);
+extern phys_addr_t pci_bar_get_addr(pcidevaddr_t dev, int bar_num);
+extern void pci_bar_set_addr(pcidevaddr_t dev, int bar_num, phys_addr_t addr);
 extern phys_addr_t pci_bar_size(pcidevaddr_t dev, int bar_num);
 extern bool pci_bar_is64(pcidevaddr_t dev, int bar_num);
 extern bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num);
diff --git a/x86/vmexit.c b/x86/vmexit.c
index c2e1e496918d..2d99d5fdf1c2 100644
--- a/x86/vmexit.c
+++ b/x86/vmexit.c
@@ -392,10 +392,10 @@ int main(int ac, char **av)
 				continue;
 			}
 			if (pci_bar_is_memory(pcidev, i)) {
-				membar = pci_bar_addr(pcidev, i);
+				membar = pci_bar_get_addr(pcidev, i);
 				pci_test.memaddr = ioremap(membar, PAGE_SIZE);
 			} else {
-				pci_test.iobar = pci_bar_addr(pcidev, i);
+				pci_test.iobar = pci_bar_get_addr(pcidev, i);
 			}
 		}
 		printf("pci-testdev at 0x%x membar %lx iobar %x\n",
-- 
1.8.3.1

[kvm-unit-tests PATCH v9 08/12] pci: Add pci_dev_exists()

[Alexander Gordeev]

Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/pci.c | 6 ++++++
 lib/pci.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/lib/pci.c b/lib/pci.c
index 74936f32ab54..42f47d93abfd 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -7,6 +7,12 @@
 #include "pci.h"
 #include "asm/pci.h"
 
+bool pci_dev_exists(pcidevaddr_t dev)
+{
+	return (pci_config_readw(dev, PCI_VENDOR_ID) != 0xffff &&
+		pci_config_readw(dev, PCI_DEVICE_ID) != 0xffff);
+}
+
 /* Scan bus look for a specific device. Only bus 0 scanned for now. */
 pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
 {
diff --git a/lib/pci.h b/lib/pci.h
index 1c20308c9344..1462aa2f0e1a 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -15,6 +15,7 @@ enum {
 	PCIDEVADDR_INVALID = 0xffff,
 };
 
+extern bool pci_dev_exists(pcidevaddr_t dev);
 extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
 
 /*
-- 
1.8.3.1

[kvm-unit-tests PATCH v9 09/12] pci: Add pci_print()

[Alexander Gordeev]

Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/pci.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/pci.h |  3 +++
 2 files changed, 92 insertions(+)

diff --git a/lib/pci.c b/lib/pci.c
index 42f47d93abfd..e03c67c1a707 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -126,3 +126,92 @@ bool pci_bar_is64(pcidevaddr_t dev, int bar_num)
 	return (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) ==
 		      PCI_BASE_ADDRESS_MEM_TYPE_64;
 }
+
+static void pci_bar_print(pcidevaddr_t dev, int bar_num)
+{
+	phys_addr_t size, start, end;
+	uint32_t bar;
+
+	size = pci_bar_size(dev, bar_num);
+	if (!size)
+		return;
+
+	bar = pci_bar_get(dev, bar_num);
+	start = pci_bar_get_addr(dev, bar_num);
+	end = start + size - 1;
+
+	if (pci_bar_is64(dev, bar_num)) {
+		printf("BAR#%d,%d [%" PRIx64 "-%" PRIx64 " ",
+		       bar_num, bar_num + 1, start, end);
+	} else {
+		printf("BAR#%d [%02x-%02x ",
+		       bar_num, (uint32_t)start, (uint32_t)end);
+	}
+
+	if (bar & PCI_BASE_ADDRESS_SPACE_IO) {
+		printf("PIO");
+	} else {
+		printf("MEM");
+		switch (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) {
+		case PCI_BASE_ADDRESS_MEM_TYPE_32:
+			printf("32");
+			break;
+		case PCI_BASE_ADDRESS_MEM_TYPE_1M:
+			printf("1M");
+			break;
+		case PCI_BASE_ADDRESS_MEM_TYPE_64:
+			printf("64");
+			break;
+		default:
+			assert(0);
+		}
+	}
+
+	if (bar & PCI_BASE_ADDRESS_MEM_PREFETCH)
+		printf("/p");
+
+	printf("]");
+}
+
+static void pci_dev_print_id(pcidevaddr_t dev)
+{
+	printf("00.%02x.%1x %04x:%04x", dev / 8, dev % 8,
+		pci_config_readw(dev, PCI_VENDOR_ID),
+		pci_config_readw(dev, PCI_DEVICE_ID));
+}
+
+static void pci_dev_print(pcidevaddr_t dev)
+{
+	uint8_t header = pci_config_readb(dev, PCI_HEADER_TYPE);
+	uint8_t progif = pci_config_readb(dev, PCI_CLASS_PROG);
+	uint8_t subclass = pci_config_readb(dev, PCI_CLASS_DEVICE);
+	uint8_t class = pci_config_readb(dev, PCI_CLASS_DEVICE + 1);
+	int i;
+
+	pci_dev_print_id(dev);
+	printf(" type %02x progif %02x class %02x subclass %02x\n",
+	       header, progif, class, subclass);
+
+	if ((header & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_NORMAL)
+		return;
+
+	for (i = 0; i < 6; i++) {
+		if (pci_bar_size(dev, i)) {
+			printf("\t");
+			pci_bar_print(dev, i);
+			printf("\n");
+		}
+		if (pci_bar_is64(dev, i))
+			i++;
+	}
+}
+
+void pci_print(void)
+{
+	pcidevaddr_t dev;
+
+	for (dev = 0; dev < 256; ++dev) {
+		if (pci_dev_exists(dev))
+			pci_dev_print(dev);
+	}
+}
diff --git a/lib/pci.h b/lib/pci.h
index 1462aa2f0e1a..fc0940adc299 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -15,6 +15,7 @@ enum {
 	PCIDEVADDR_INVALID = 0xffff,
 };
 
+extern void pci_print(void);
 extern bool pci_dev_exists(pcidevaddr_t dev);
 extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
 
@@ -57,4 +58,6 @@ struct pci_test_dev_hdr {
 	uint8_t  name[];
 };
 
+#define  PCI_HEADER_TYPE_MASK		0x7f
+
 #endif /* PCI_H */
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH v9 09/12] pci: Add pci_print()

[Andrew Jones]

On Thu, Nov 03, 2016 at 05:19:46PM +0100, Alexander Gordeev wrote:
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Andrew Jones <drjones@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Suggested-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
>  lib/pci.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/pci.h |  3 +++
>  2 files changed, 92 insertions(+)

Reviewed-by: Andrew Jones <drjones@redhat.com>

> 
> diff --git a/lib/pci.c b/lib/pci.c
> index 42f47d93abfd..e03c67c1a707 100644
> --- a/lib/pci.c
> +++ b/lib/pci.c
> @@ -126,3 +126,92 @@ bool pci_bar_is64(pcidevaddr_t dev, int bar_num)
>  	return (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) ==
>  		      PCI_BASE_ADDRESS_MEM_TYPE_64;
>  }
> +
> +static void pci_bar_print(pcidevaddr_t dev, int bar_num)
> +{
> +	phys_addr_t size, start, end;
> +	uint32_t bar;
> +
> +	size = pci_bar_size(dev, bar_num);
> +	if (!size)
> +		return;
> +
> +	bar = pci_bar_get(dev, bar_num);
> +	start = pci_bar_get_addr(dev, bar_num);
> +	end = start + size - 1;
> +
> +	if (pci_bar_is64(dev, bar_num)) {
> +		printf("BAR#%d,%d [%" PRIx64 "-%" PRIx64 " ",
> +		       bar_num, bar_num + 1, start, end);
> +	} else {
> +		printf("BAR#%d [%02x-%02x ",
> +		       bar_num, (uint32_t)start, (uint32_t)end);
> +	}
> +
> +	if (bar & PCI_BASE_ADDRESS_SPACE_IO) {
> +		printf("PIO");
> +	} else {
> +		printf("MEM");
> +		switch (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) {
> +		case PCI_BASE_ADDRESS_MEM_TYPE_32:
> +			printf("32");
> +			break;
> +		case PCI_BASE_ADDRESS_MEM_TYPE_1M:
> +			printf("1M");
> +			break;
> +		case PCI_BASE_ADDRESS_MEM_TYPE_64:
> +			printf("64");
> +			break;
> +		default:
> +			assert(0);
> +		}
> +	}
> +
> +	if (bar & PCI_BASE_ADDRESS_MEM_PREFETCH)
> +		printf("/p");
> +
> +	printf("]");
> +}
> +
> +static void pci_dev_print_id(pcidevaddr_t dev)
> +{
> +	printf("00.%02x.%1x %04x:%04x", dev / 8, dev % 8,
> +		pci_config_readw(dev, PCI_VENDOR_ID),
> +		pci_config_readw(dev, PCI_DEVICE_ID));
> +}
> +
> +static void pci_dev_print(pcidevaddr_t dev)
> +{
> +	uint8_t header = pci_config_readb(dev, PCI_HEADER_TYPE);
> +	uint8_t progif = pci_config_readb(dev, PCI_CLASS_PROG);
> +	uint8_t subclass = pci_config_readb(dev, PCI_CLASS_DEVICE);
> +	uint8_t class = pci_config_readb(dev, PCI_CLASS_DEVICE + 1);
> +	int i;
> +
> +	pci_dev_print_id(dev);
> +	printf(" type %02x progif %02x class %02x subclass %02x\n",
> +	       header, progif, class, subclass);
> +
> +	if ((header & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_NORMAL)
> +		return;
> +
> +	for (i = 0; i < 6; i++) {
> +		if (pci_bar_size(dev, i)) {
> +			printf("\t");
> +			pci_bar_print(dev, i);
> +			printf("\n");
> +		}
> +		if (pci_bar_is64(dev, i))
> +			i++;
> +	}
> +}
> +
> +void pci_print(void)
> +{
> +	pcidevaddr_t dev;
> +
> +	for (dev = 0; dev < 256; ++dev) {
> +		if (pci_dev_exists(dev))
> +			pci_dev_print(dev);
> +	}
> +}
> diff --git a/lib/pci.h b/lib/pci.h
> index 1462aa2f0e1a..fc0940adc299 100644
> --- a/lib/pci.h
> +++ b/lib/pci.h
> @@ -15,6 +15,7 @@ enum {
>  	PCIDEVADDR_INVALID = 0xffff,
>  };
>  
> +extern void pci_print(void);
>  extern bool pci_dev_exists(pcidevaddr_t dev);
>  extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
>  
> @@ -57,4 +58,6 @@ struct pci_test_dev_hdr {
>  	uint8_t  name[];
>  };
>  
> +#define  PCI_HEADER_TYPE_MASK		0x7f
> +
>  #endif /* PCI_H */
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kvm-unit-tests PATCH v9 10/12] pci: Add generic ECAM host support

[Alexander Gordeev]

Unlike x86, other architectures using generic ECAM PCI host
do not have a luxury of PCI bus initialized by a BIOS and
ready to use at start. Thus, we need allocate and assign
resources to all devices, much like an architecture's
firmware would do.

There is no any sort of resource management for memory and
io spaces, since only ones-per-BAR allocations are expected
and no deallocations at all.

Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/asm-generic/pci-host-bridge.h |  26 ++++
 lib/pci-host-generic.c            | 305 ++++++++++++++++++++++++++++++++++++++
 lib/pci-host-generic.h            |  46 ++++++
 lib/pci.c                         |   8 +-
 lib/pci.h                         |   5 +
 5 files changed, 386 insertions(+), 4 deletions(-)
 create mode 100644 lib/asm-generic/pci-host-bridge.h
 create mode 100644 lib/pci-host-generic.c
 create mode 100644 lib/pci-host-generic.h

diff --git a/lib/asm-generic/pci-host-bridge.h b/lib/asm-generic/pci-host-bridge.h
new file mode 100644
index 000000000000..ac32b85198e9
--- /dev/null
+++ b/lib/asm-generic/pci-host-bridge.h
@@ -0,0 +1,26 @@
+#ifndef _ASM_PCI_HOST_BRIDGE_H_
+#define _ASM_PCI_HOST_BRIDGE_H_
+/*
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include "libcflat.h"
+
+phys_addr_t pci_host_bridge_get_paddr(uint64_t addr);
+
+static inline
+phys_addr_t pci_translate_addr(pcidevaddr_t dev __unused, uint64_t addr)
+{
+	/*
+	 * Assume we only have single PCI host bridge in a system.
+	 */
+	return pci_host_bridge_get_paddr(addr);
+}
+
+uint8_t pci_config_readb(pcidevaddr_t dev, uint8_t reg);
+uint16_t pci_config_readw(pcidevaddr_t dev, uint8_t reg);
+uint32_t pci_config_readl(pcidevaddr_t dev, uint8_t reg);
+void pci_config_writel(pcidevaddr_t dev, uint8_t reg, uint32_t val);
+
+#endif
diff --git a/lib/pci-host-generic.c b/lib/pci-host-generic.c
new file mode 100644
index 000000000000..d06fff2c7347
--- /dev/null
+++ b/lib/pci-host-generic.c
@@ -0,0 +1,305 @@
+/*
+ * Generic PCI host controller as described in PCI Bus Binding to Open Firmware
+ *
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include "libcflat.h"
+#include "devicetree.h"
+#include "alloc.h"
+#include "pci.h"
+#include "asm/pci.h"
+#include "asm/io.h"
+#include "pci-host-generic.h"
+#include <linux/pci_regs.h>
+
+static struct pci_host_bridge *pci_host_bridge;
+
+static int of_flags_to_pci_type(u32 of_flags)
+{
+	static int type_map[] = {
+		[1] = PCI_BASE_ADDRESS_SPACE_IO,
+		[2] = PCI_BASE_ADDRESS_MEM_TYPE_32,
+		[3] = PCI_BASE_ADDRESS_MEM_TYPE_64
+	};
+	int idx = (of_flags >> 24) & 0x03;
+	int res;
+
+	assert(idx > 0);
+	res = type_map[idx];
+
+	if (of_flags & 0x40000000)
+		res |= PCI_BASE_ADDRESS_MEM_PREFETCH;
+
+	return res;
+}
+
+static int pci_bar_type(u32 bar)
+{
+	if (bar & PCI_BASE_ADDRESS_SPACE)
+		return PCI_BASE_ADDRESS_SPACE_IO;
+	else
+		return bar & (PCI_BASE_ADDRESS_MEM_TYPE_MASK |
+			      PCI_BASE_ADDRESS_MEM_PREFETCH);
+}
+
+/*
+ * Probe DT for a generic PCI host controller
+ * See kernel Documentation/devicetree/bindings/pci/host-generic-pci.txt
+ * and function gen_pci_probe() in drivers/pci/host/pci-host-generic.c
+ */
+static struct pci_host_bridge *pci_dt_probe(void)
+{
+	struct pci_host_bridge *host;
+	const void *fdt = dt_fdt();
+	const struct fdt_property *prop;
+	struct dt_pbus_reg base;
+	struct dt_device dt_dev;
+	struct dt_bus dt_bus;
+	struct pci_addr_space *as;
+	fdt32_t *data;
+	u32 bus, bus_max;
+	u32 nac, nsc, nac_root, nsc_root;
+	int nr_range_cells, nr_addr_spaces;
+	int ret, node, len, i;
+
+	if (!dt_available()) {
+		printf("No device tree found\n");
+		return NULL;
+	}
+
+	dt_bus_init_defaults(&dt_bus);
+	dt_device_init(&dt_dev, &dt_bus, NULL);
+
+	node = fdt_path_offset(fdt, "/");
+	assert(node >= 0);
+
+	ret = dt_get_nr_cells(node, &nac_root, &nsc_root);
+	assert(ret == 0);
+	assert(nac_root == 1 || nac_root == 2);
+
+	node = fdt_node_offset_by_compatible(fdt, node,
+					     "pci-host-ecam-generic");
+	if (node == -FDT_ERR_NOTFOUND) {
+		printf("No PCIe ECAM compatible controller found\n");
+		return NULL;
+	}
+	assert(node >= 0);
+
+	prop = fdt_get_property(fdt, node, "device_type", &len);
+	assert(prop && len == 4 && !strcmp((char *)prop->data, "pci"));
+
+	dt_device_bind_node(&dt_dev, node);
+	ret = dt_pbus_get_base(&dt_dev, &base);
+	assert(ret == 0);
+
+	prop = fdt_get_property(fdt, node, "bus-range", &len);
+	if (prop == NULL) {
+		assert(len == -FDT_ERR_NOTFOUND);
+		bus		= 0x00;
+		bus_max		= 0xff;
+	} else {
+		data		= (fdt32_t *)prop->data;
+		bus		= fdt32_to_cpu(data[0]);
+		bus_max		= fdt32_to_cpu(data[1]);
+		assert(bus <= bus_max);
+	}
+	assert(bus_max < base.size / (1 << PCI_ECAM_BUS_SHIFT));
+
+	ret = dt_get_nr_cells(node, &nac, &nsc);
+	assert(ret == 0);
+	assert(nac == 3 && nsc == 2);
+
+	prop = fdt_get_property(fdt, node, "ranges", &len);
+	assert(prop != NULL);
+
+	nr_range_cells = nac + nsc + nac_root;
+	nr_addr_spaces = (len / 4) / nr_range_cells;
+	assert(nr_addr_spaces);
+
+	host = malloc(sizeof(*host) +
+		      sizeof(host->addr_space[0]) * nr_addr_spaces);
+	assert(host != NULL);
+
+	host->start		= base.addr;
+	host->size		= base.size;
+	host->bus		= bus;
+	host->bus_max		= bus_max;
+	host->nr_addr_spaces	= nr_addr_spaces;
+
+	data = (fdt32_t *)prop->data;
+	as = &host->addr_space[0];
+
+	for (i = 0; i < nr_addr_spaces; i++) {
+		/*
+		 * The PCI binding encodes the PCI address with three
+		 * cells as follows:
+		 *
+		 * phys.hi  cell: npt000ss bbbbbbbb dddddfff rrrrrrrr
+		 * phys.mid cell: hhhhhhhh hhhhhhhh hhhhhhhh hhhhhhhh
+		 * phys.lo  cell: llllllll llllllll llllllll llllllll
+		 *
+		 * PCI device bus address and flags are encoded into phys.high
+		 * PCI 64 bit address is encoded into phys.mid and phys.low
+		 */
+		as->type = of_flags_to_pci_type(fdt32_to_cpu(data[0]));
+		as->pci_start = ((u64)fdt32_to_cpu(data[1]) << 32) |
+				fdt32_to_cpu(data[2]);
+
+		if (nr_range_cells == 6) {
+			as->start = fdt32_to_cpu(data[3]);
+			as->size  = ((u64)fdt32_to_cpu(data[4]) << 32) |
+				    fdt32_to_cpu(data[5]);
+		} else {
+			as->start = ((u64)fdt32_to_cpu(data[3]) << 32) |
+				    fdt32_to_cpu(data[4]);
+			as->size  = ((u64)fdt32_to_cpu(data[5]) << 32) |
+				    fdt32_to_cpu(data[6]);
+		}
+
+		data += nr_range_cells;
+		as++;
+	}
+
+	return host;
+}
+
+static bool pci_alloc_resource(pcidevaddr_t dev, int bar_num, u64 *addr)
+{
+	struct pci_host_bridge *host = pci_host_bridge;
+	struct pci_addr_space *as = &host->addr_space[0];
+	u32 mask, bar;
+	u64 size;
+	int type, i;
+
+	*addr = ~0;
+
+	size = pci_bar_size(dev, bar_num);
+	if (!size)
+		return false;
+
+	bar = pci_bar_get(dev, bar_num);
+	type = pci_bar_type(bar);
+	if (type & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
+		type &= ~PCI_BASE_ADDRESS_MEM_PREFETCH;
+
+	for (i = 0; i < host->nr_addr_spaces; i++) {
+		if (as->type == type)
+			break;
+		as++;
+	}
+	if (i >= host->nr_addr_spaces) {
+		printf("%s: warning: can't satisfy request for ", __func__);
+		pci_dev_print_id(dev);
+		printf(" ");
+		pci_bar_print(dev, bar_num);
+		printf("\n");
+		return false;
+	}
+
+	mask = pci_bar_mask(bar);
+	size = ALIGN(size, ~mask + 1);
+	assert(as->allocated + size <= as->size);
+
+	*addr = as->pci_start + as->allocated;
+	as->allocated += size;
+
+	return true;
+}
+
+bool pci_probe(void)
+{
+	pcidevaddr_t dev;
+	u8 header;
+	u32 cmd;
+	int i;
+
+	assert(!pci_host_bridge);
+	pci_host_bridge = pci_dt_probe();
+	if (!pci_host_bridge)
+		return false;
+
+	for (dev = 0; dev < 256; dev++) {
+		if (!pci_dev_exists(dev))
+			continue;
+
+		/* We are only interested in normal PCI devices */
+		header = pci_config_readb(dev, PCI_HEADER_TYPE);
+		if ((header & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_NORMAL)
+			continue;
+
+		cmd = PCI_COMMAND_SERR;
+
+		for (i = 0; i < 6; i++) {
+			u64 addr;
+
+			if (pci_alloc_resource(dev, i, &addr)) {
+				pci_bar_set_addr(dev, i, addr);
+
+				if (pci_bar_is_memory(dev, i))
+					cmd |= PCI_COMMAND_MEMORY;
+				else
+					cmd |= PCI_COMMAND_IO;
+			}
+
+			if (pci_bar_is64(dev, i))
+				i++;
+		}
+
+		pci_config_writel(dev, PCI_COMMAND, cmd);
+	}
+
+	return true;
+}
+
+/*
+ * This function is to be called from pci_translate_addr() to provide
+ * mapping between this host bridge's PCI busses address and CPU physical
+ * address.
+ */
+phys_addr_t pci_host_bridge_get_paddr(u64 pci_addr)
+{
+	struct pci_host_bridge *host = pci_host_bridge;
+	struct pci_addr_space *as = &host->addr_space[0];
+	int i;
+
+	for (i = 0; i < host->nr_addr_spaces; i++) {
+		if (pci_addr >= as->pci_start &&
+		    pci_addr < as->pci_start + as->size)
+			return as->start + (pci_addr - as->pci_start);
+		as++;
+	}
+
+	return 0;
+}
+
+static void __iomem *pci_get_dev_conf(struct pci_host_bridge *host, int devfn)
+{
+	return (void __iomem *)(unsigned long)
+		host->start + (devfn << PCI_ECAM_DEVFN_SHIFT);
+}
+
+u8 pci_config_readb(pcidevaddr_t dev, u8 off)
+{
+	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
+	return readb(conf + off);
+}
+
+u16 pci_config_readw(pcidevaddr_t dev, u8 off)
+{
+	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
+	return readw(conf + off);
+}
+
+u32 pci_config_readl(pcidevaddr_t dev, u8 off)
+{
+	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
+	return readl(conf + off);
+}
+
+void pci_config_writel(pcidevaddr_t dev, u8 off, u32 val)
+{
+	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
+	writel(val, conf + off);
+}
diff --git a/lib/pci-host-generic.h b/lib/pci-host-generic.h
new file mode 100644
index 000000000000..fd30e7c74ed8
--- /dev/null
+++ b/lib/pci-host-generic.h
@@ -0,0 +1,46 @@
+#ifndef PCI_HOST_GENERIC_H
+#define PCI_HOST_GENERIC_H
+/*
+ * PCI host bridge supporting structures and constants
+ *
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include "libcflat.h"
+
+struct pci_addr_space {
+	phys_addr_t		pci_start;
+	phys_addr_t		start;
+	phys_addr_t		size;
+	phys_addr_t		allocated;
+	int			type;
+};
+
+struct pci_host_bridge {
+	phys_addr_t		start;
+	phys_addr_t		size;
+	int			bus;
+	int			bus_max;
+	int			nr_addr_spaces;
+	struct pci_addr_space	addr_space[];
+};
+
+/*
+ * The following constants are derived from Linux, see this source:
+ *
+ *         drivers/pci/host/pci-host-generic.c
+ *                 struct gen_pci_cfg_bus_ops::bus_shift
+ *                 int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
+ *
+ * Documentation/devicetree/bindings/pci/host-generic-pci.txt describes
+ * ECAM Configuration Space is be memory-mapped by concatenating the various
+ * components to form an offset:
+ *
+ *	cfg_offset(bus, device, function, register) =
+ *		   bus << 20 | device << 15 | function << 12 | register
+ */
+#define PCI_ECAM_BUS_SHIFT	20
+#define PCI_ECAM_DEVFN_SHIFT	12
+
+#endif
diff --git a/lib/pci.c b/lib/pci.c
index e03c67c1a707..6bd54cbac1bb 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -27,13 +27,13 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
 	return PCIDEVADDR_INVALID;
 }
 
-static uint32_t pci_bar_mask(uint32_t bar)
+uint32_t pci_bar_mask(uint32_t bar)
 {
 	return (bar & PCI_BASE_ADDRESS_SPACE_IO) ?
 		PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
 }
 
-static uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num)
+uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num)
 {
 	return pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
 }
@@ -127,7 +127,7 @@ bool pci_bar_is64(pcidevaddr_t dev, int bar_num)
 		      PCI_BASE_ADDRESS_MEM_TYPE_64;
 }
 
-static void pci_bar_print(pcidevaddr_t dev, int bar_num)
+void pci_bar_print(pcidevaddr_t dev, int bar_num)
 {
 	phys_addr_t size, start, end;
 	uint32_t bar;
@@ -173,7 +173,7 @@ static void pci_bar_print(pcidevaddr_t dev, int bar_num)
 	printf("]");
 }
 
-static void pci_dev_print_id(pcidevaddr_t dev)
+void pci_dev_print_id(pcidevaddr_t dev)
 {
 	printf("00.%02x.%1x %04x:%04x", dev / 8, dev % 8,
 		pci_config_readw(dev, PCI_VENDOR_ID),
diff --git a/lib/pci.h b/lib/pci.h
index fc0940adc299..9d960556c526 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -15,6 +15,7 @@ enum {
 	PCIDEVADDR_INVALID = 0xffff,
 };
 
+extern bool pci_probe(void);
 extern void pci_print(void);
 extern bool pci_dev_exists(pcidevaddr_t dev);
 extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
@@ -34,9 +35,13 @@ extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
 extern phys_addr_t pci_bar_get_addr(pcidevaddr_t dev, int bar_num);
 extern void pci_bar_set_addr(pcidevaddr_t dev, int bar_num, phys_addr_t addr);
 extern phys_addr_t pci_bar_size(pcidevaddr_t dev, int bar_num);
+extern uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num);
+extern uint32_t pci_bar_mask(uint32_t bar);
 extern bool pci_bar_is64(pcidevaddr_t dev, int bar_num);
 extern bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num);
 extern bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num);
+extern void pci_bar_print(pcidevaddr_t dev, int bar_num);
+extern void pci_dev_print_id(pcidevaddr_t dev);
 
 /*
  * pci-testdev is a driver for the pci-testdev qemu pci device. The
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH v9 10/12] pci: Add generic ECAM host support

[Andrew Jones]

On Thu, Nov 03, 2016 at 05:19:47PM +0100, Alexander Gordeev wrote:
> Unlike x86, other architectures using generic ECAM PCI host
> do not have a luxury of PCI bus initialized by a BIOS and
> ready to use at start. Thus, we need allocate and assign
> resources to all devices, much like an architecture's
> firmware would do.
> 
> There is no any sort of resource management for memory and
> io spaces, since only ones-per-BAR allocations are expected
> and no deallocations at all.
> 
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Andrew Jones <drjones@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Suggested-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
>  lib/asm-generic/pci-host-bridge.h |  26 ++++
>  lib/pci-host-generic.c            | 305 ++++++++++++++++++++++++++++++++++++++
>  lib/pci-host-generic.h            |  46 ++++++
>  lib/pci.c                         |   8 +-
>  lib/pci.h                         |   5 +
>  5 files changed, 386 insertions(+), 4 deletions(-)
>  create mode 100644 lib/asm-generic/pci-host-bridge.h
>  create mode 100644 lib/pci-host-generic.c
>  create mode 100644 lib/pci-host-generic.h
> 
> diff --git a/lib/asm-generic/pci-host-bridge.h b/lib/asm-generic/pci-host-bridge.h
> new file mode 100644
> index 000000000000..ac32b85198e9
> --- /dev/null
> +++ b/lib/asm-generic/pci-host-bridge.h
> @@ -0,0 +1,26 @@
> +#ifndef _ASM_PCI_HOST_BRIDGE_H_
> +#define _ASM_PCI_HOST_BRIDGE_H_
> +/*
> + * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include "libcflat.h"
> +
> +phys_addr_t pci_host_bridge_get_paddr(uint64_t addr);
> +
> +static inline
> +phys_addr_t pci_translate_addr(pcidevaddr_t dev __unused, uint64_t addr)
> +{
> +	/*
> +	 * Assume we only have single PCI host bridge in a system.
> +	 */
> +	return pci_host_bridge_get_paddr(addr);
> +}
> +
> +uint8_t pci_config_readb(pcidevaddr_t dev, uint8_t reg);
> +uint16_t pci_config_readw(pcidevaddr_t dev, uint8_t reg);
> +uint32_t pci_config_readl(pcidevaddr_t dev, uint8_t reg);
> +void pci_config_writel(pcidevaddr_t dev, uint8_t reg, uint32_t val);
> +
> +#endif
> diff --git a/lib/pci-host-generic.c b/lib/pci-host-generic.c
> new file mode 100644
> index 000000000000..d06fff2c7347
> --- /dev/null
> +++ b/lib/pci-host-generic.c
> @@ -0,0 +1,305 @@
> +/*
> + * Generic PCI host controller as described in PCI Bus Binding to Open Firmware
> + *
> + * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include "libcflat.h"
> +#include "devicetree.h"
> +#include "alloc.h"
> +#include "pci.h"
> +#include "asm/pci.h"
> +#include "asm/io.h"
> +#include "pci-host-generic.h"
> +#include <linux/pci_regs.h>
> +
> +static struct pci_host_bridge *pci_host_bridge;
> +
> +static int of_flags_to_pci_type(u32 of_flags)
> +{
> +	static int type_map[] = {
> +		[1] = PCI_BASE_ADDRESS_SPACE_IO,
> +		[2] = PCI_BASE_ADDRESS_MEM_TYPE_32,
> +		[3] = PCI_BASE_ADDRESS_MEM_TYPE_64
> +	};
> +	int idx = (of_flags >> 24) & 0x03;
> +	int res;
> +
> +	assert(idx > 0);
> +	res = type_map[idx];
> +
> +	if (of_flags & 0x40000000)
> +		res |= PCI_BASE_ADDRESS_MEM_PREFETCH;
> +
> +	return res;
> +}
> +
> +static int pci_bar_type(u32 bar)
> +{
> +	if (bar & PCI_BASE_ADDRESS_SPACE)
> +		return PCI_BASE_ADDRESS_SPACE_IO;
> +	else
> +		return bar & (PCI_BASE_ADDRESS_MEM_TYPE_MASK |
> +			      PCI_BASE_ADDRESS_MEM_PREFETCH);
> +}
> +
> +/*
> + * Probe DT for a generic PCI host controller
> + * See kernel Documentation/devicetree/bindings/pci/host-generic-pci.txt
> + * and function gen_pci_probe() in drivers/pci/host/pci-host-generic.c
> + */
> +static struct pci_host_bridge *pci_dt_probe(void)
> +{
> +	struct pci_host_bridge *host;
> +	const void *fdt = dt_fdt();
> +	const struct fdt_property *prop;
> +	struct dt_pbus_reg base;
> +	struct dt_device dt_dev;
> +	struct dt_bus dt_bus;
> +	struct pci_addr_space *as;
> +	fdt32_t *data;
> +	u32 bus, bus_max;
> +	u32 nac, nsc, nac_root, nsc_root;
> +	int nr_range_cells, nr_addr_spaces;
> +	int ret, node, len, i;
> +
> +	if (!dt_available()) {
> +		printf("No device tree found\n");
> +		return NULL;
> +	}
> +
> +	dt_bus_init_defaults(&dt_bus);
> +	dt_device_init(&dt_dev, &dt_bus, NULL);
> +
> +	node = fdt_path_offset(fdt, "/");
> +	assert(node >= 0);
> +
> +	ret = dt_get_nr_cells(node, &nac_root, &nsc_root);
> +	assert(ret == 0);
> +	assert(nac_root == 1 || nac_root == 2);
> +
> +	node = fdt_node_offset_by_compatible(fdt, node,
> +					     "pci-host-ecam-generic");
> +	if (node == -FDT_ERR_NOTFOUND) {
> +		printf("No PCIe ECAM compatible controller found\n");
> +		return NULL;
> +	}
> +	assert(node >= 0);
> +
> +	prop = fdt_get_property(fdt, node, "device_type", &len);
> +	assert(prop && len == 4 && !strcmp((char *)prop->data, "pci"));
> +
> +	dt_device_bind_node(&dt_dev, node);
> +	ret = dt_pbus_get_base(&dt_dev, &base);
> +	assert(ret == 0);
> +
> +	prop = fdt_get_property(fdt, node, "bus-range", &len);
> +	if (prop == NULL) {
> +		assert(len == -FDT_ERR_NOTFOUND);
> +		bus		= 0x00;
> +		bus_max		= 0xff;
> +	} else {
> +		data		= (fdt32_t *)prop->data;
> +		bus		= fdt32_to_cpu(data[0]);
> +		bus_max		= fdt32_to_cpu(data[1]);
> +		assert(bus <= bus_max);
> +	}
> +	assert(bus_max < base.size / (1 << PCI_ECAM_BUS_SHIFT));
> +
> +	ret = dt_get_nr_cells(node, &nac, &nsc);
> +	assert(ret == 0);
> +	assert(nac == 3 && nsc == 2);
> +
> +	prop = fdt_get_property(fdt, node, "ranges", &len);
> +	assert(prop != NULL);
> +
> +	nr_range_cells = nac + nsc + nac_root;
> +	nr_addr_spaces = (len / 4) / nr_range_cells;
> +	assert(nr_addr_spaces);
> +
> +	host = malloc(sizeof(*host) +
> +		      sizeof(host->addr_space[0]) * nr_addr_spaces);
> +	assert(host != NULL);
> +
> +	host->start		= base.addr;
> +	host->size		= base.size;
> +	host->bus		= bus;
> +	host->bus_max		= bus_max;
> +	host->nr_addr_spaces	= nr_addr_spaces;
> +
> +	data = (fdt32_t *)prop->data;
> +	as = &host->addr_space[0];
> +
> +	for (i = 0; i < nr_addr_spaces; i++) {
> +		/*
> +		 * The PCI binding encodes the PCI address with three
> +		 * cells as follows:
> +		 *
> +		 * phys.hi  cell: npt000ss bbbbbbbb dddddfff rrrrrrrr
> +		 * phys.mid cell: hhhhhhhh hhhhhhhh hhhhhhhh hhhhhhhh
> +		 * phys.lo  cell: llllllll llllllll llllllll llllllll
> +		 *
> +		 * PCI device bus address and flags are encoded into phys.high
> +		 * PCI 64 bit address is encoded into phys.mid and phys.low
> +		 */
> +		as->type = of_flags_to_pci_type(fdt32_to_cpu(data[0]));
> +		as->pci_start = ((u64)fdt32_to_cpu(data[1]) << 32) |
> +				fdt32_to_cpu(data[2]);
> +
> +		if (nr_range_cells == 6) {
> +			as->start = fdt32_to_cpu(data[3]);
> +			as->size  = ((u64)fdt32_to_cpu(data[4]) << 32) |
> +				    fdt32_to_cpu(data[5]);
> +		} else {
> +			as->start = ((u64)fdt32_to_cpu(data[3]) << 32) |
> +				    fdt32_to_cpu(data[4]);
> +			as->size  = ((u64)fdt32_to_cpu(data[5]) << 32) |
> +				    fdt32_to_cpu(data[6]);
> +		}
> +
> +		data += nr_range_cells;
> +		as++;
> +	}
> +
> +	return host;
> +}
> +
> +static bool pci_alloc_resource(pcidevaddr_t dev, int bar_num, u64 *addr)
> +{
> +	struct pci_host_bridge *host = pci_host_bridge;
> +	struct pci_addr_space *as = &host->addr_space[0];
> +	u32 mask, bar;
> +	u64 size;
> +	int type, i;
> +
> +	*addr = ~0;
> +
> +	size = pci_bar_size(dev, bar_num);
> +	if (!size)
> +		return false;
> +
> +	bar = pci_bar_get(dev, bar_num);
> +	type = pci_bar_type(bar);
> +	if (type & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
> +		type &= ~PCI_BASE_ADDRESS_MEM_PREFETCH;
> +
> +	for (i = 0; i < host->nr_addr_spaces; i++) {
> +		if (as->type == type)
> +			break;
> +		as++;
> +	}

nit: empty line here would be nice

> +	if (i >= host->nr_addr_spaces) {
> +		printf("%s: warning: can't satisfy request for ", __func__);
> +		pci_dev_print_id(dev);
> +		printf(" ");
> +		pci_bar_print(dev, bar_num);
> +		printf("\n");
> +		return false;
> +	}
> +
> +	mask = pci_bar_mask(bar);
> +	size = ALIGN(size, ~mask + 1);
> +	assert(as->allocated + size <= as->size);
> +
> +	*addr = as->pci_start + as->allocated;
> +	as->allocated += size;
> +
> +	return true;
> +}
> +
> +bool pci_probe(void)
> +{
> +	pcidevaddr_t dev;
> +	u8 header;
> +	u32 cmd;
> +	int i;
> +
> +	assert(!pci_host_bridge);
> +	pci_host_bridge = pci_dt_probe();
> +	if (!pci_host_bridge)
> +		return false;
> +
> +	for (dev = 0; dev < 256; dev++) {
> +		if (!pci_dev_exists(dev))
> +			continue;
> +
> +		/* We are only interested in normal PCI devices */
> +		header = pci_config_readb(dev, PCI_HEADER_TYPE);
> +		if ((header & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_NORMAL)
> +			continue;
> +
> +		cmd = PCI_COMMAND_SERR;
> +
> +		for (i = 0; i < 6; i++) {
> +			u64 addr;
> +
> +			if (pci_alloc_resource(dev, i, &addr)) {
> +				pci_bar_set_addr(dev, i, addr);
> +
> +				if (pci_bar_is_memory(dev, i))
> +					cmd |= PCI_COMMAND_MEMORY;
> +				else
> +					cmd |= PCI_COMMAND_IO;
> +			}
> +
> +			if (pci_bar_is64(dev, i))
> +				i++;
> +		}
> +
> +		pci_config_writel(dev, PCI_COMMAND, cmd);

As I asked in the last review, is this PCI_COMMAND_SERR safe/desired
to write, even when no resources are allocated? If not, then above
we need to capture the return value of pci_alloc_resource, add
an 'else break', and then check the return value here before doing
the write.

> +	}
> +
> +	return true;
> +}
> +
> +/*
> + * This function is to be called from pci_translate_addr() to provide
> + * mapping between this host bridge's PCI busses address and CPU physical
> + * address.
> + */
> +phys_addr_t pci_host_bridge_get_paddr(u64 pci_addr)
> +{
> +	struct pci_host_bridge *host = pci_host_bridge;
> +	struct pci_addr_space *as = &host->addr_space[0];
> +	int i;
> +
> +	for (i = 0; i < host->nr_addr_spaces; i++) {
> +		if (pci_addr >= as->pci_start &&
> +		    pci_addr < as->pci_start + as->size)
> +			return as->start + (pci_addr - as->pci_start);
> +		as++;
> +	}
> +
> +	return 0;
> +}
> +
> +static void __iomem *pci_get_dev_conf(struct pci_host_bridge *host, int devfn)
> +{
> +	return (void __iomem *)(unsigned long)
> +		host->start + (devfn << PCI_ECAM_DEVFN_SHIFT);
> +}
> +
> +u8 pci_config_readb(pcidevaddr_t dev, u8 off)
> +{
> +	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
> +	return readb(conf + off);
> +}
> +
> +u16 pci_config_readw(pcidevaddr_t dev, u8 off)
> +{
> +	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
> +	return readw(conf + off);
> +}
> +
> +u32 pci_config_readl(pcidevaddr_t dev, u8 off)
> +{
> +	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
> +	return readl(conf + off);
> +}
> +
> +void pci_config_writel(pcidevaddr_t dev, u8 off, u32 val)
> +{
> +	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
> +	writel(val, conf + off);
> +}
> diff --git a/lib/pci-host-generic.h b/lib/pci-host-generic.h
> new file mode 100644
> index 000000000000..fd30e7c74ed8
> --- /dev/null
> +++ b/lib/pci-host-generic.h
> @@ -0,0 +1,46 @@
> +#ifndef PCI_HOST_GENERIC_H
> +#define PCI_HOST_GENERIC_H
> +/*
> + * PCI host bridge supporting structures and constants
> + *
> + * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include "libcflat.h"
> +
> +struct pci_addr_space {
> +	phys_addr_t		pci_start;
> +	phys_addr_t		start;
> +	phys_addr_t		size;
> +	phys_addr_t		allocated;
> +	int			type;
> +};
> +
> +struct pci_host_bridge {
> +	phys_addr_t		start;
> +	phys_addr_t		size;
> +	int			bus;
> +	int			bus_max;
> +	int			nr_addr_spaces;
> +	struct pci_addr_space	addr_space[];
> +};
> +
> +/*
> + * The following constants are derived from Linux, see this source:
> + *
> + *         drivers/pci/host/pci-host-generic.c
> + *                 struct gen_pci_cfg_bus_ops::bus_shift
> + *                 int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
> + *
> + * Documentation/devicetree/bindings/pci/host-generic-pci.txt describes
> + * ECAM Configuration Space is be memory-mapped by concatenating the various
> + * components to form an offset:
> + *
> + *	cfg_offset(bus, device, function, register) =
> + *		   bus << 20 | device << 15 | function << 12 | register
> + */
> +#define PCI_ECAM_BUS_SHIFT	20
> +#define PCI_ECAM_DEVFN_SHIFT	12
> +
> +#endif
> diff --git a/lib/pci.c b/lib/pci.c
> index e03c67c1a707..6bd54cbac1bb 100644
> --- a/lib/pci.c
> +++ b/lib/pci.c
> @@ -27,13 +27,13 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
>  	return PCIDEVADDR_INVALID;
>  }
>  
> -static uint32_t pci_bar_mask(uint32_t bar)
> +uint32_t pci_bar_mask(uint32_t bar)
>  {
>  	return (bar & PCI_BASE_ADDRESS_SPACE_IO) ?
>  		PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
>  }
>  
> -static uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num)
> +uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num)
>  {
>  	return pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
>  }
> @@ -127,7 +127,7 @@ bool pci_bar_is64(pcidevaddr_t dev, int bar_num)
>  		      PCI_BASE_ADDRESS_MEM_TYPE_64;
>  }
>  
> -static void pci_bar_print(pcidevaddr_t dev, int bar_num)
> +void pci_bar_print(pcidevaddr_t dev, int bar_num)
>  {
>  	phys_addr_t size, start, end;
>  	uint32_t bar;
> @@ -173,7 +173,7 @@ static void pci_bar_print(pcidevaddr_t dev, int bar_num)
>  	printf("]");
>  }
>  
> -static void pci_dev_print_id(pcidevaddr_t dev)
> +void pci_dev_print_id(pcidevaddr_t dev)
>  {
>  	printf("00.%02x.%1x %04x:%04x", dev / 8, dev % 8,
>  		pci_config_readw(dev, PCI_VENDOR_ID),
> diff --git a/lib/pci.h b/lib/pci.h
> index fc0940adc299..9d960556c526 100644
> --- a/lib/pci.h
> +++ b/lib/pci.h
> @@ -15,6 +15,7 @@ enum {
>  	PCIDEVADDR_INVALID = 0xffff,
>  };
>  
> +extern bool pci_probe(void);
>  extern void pci_print(void);
>  extern bool pci_dev_exists(pcidevaddr_t dev);
>  extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
> @@ -34,9 +35,13 @@ extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
>  extern phys_addr_t pci_bar_get_addr(pcidevaddr_t dev, int bar_num);
>  extern void pci_bar_set_addr(pcidevaddr_t dev, int bar_num, phys_addr_t addr);
>  extern phys_addr_t pci_bar_size(pcidevaddr_t dev, int bar_num);
> +extern uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num);
> +extern uint32_t pci_bar_mask(uint32_t bar);
>  extern bool pci_bar_is64(pcidevaddr_t dev, int bar_num);
>  extern bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num);
>  extern bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num);
> +extern void pci_bar_print(pcidevaddr_t dev, int bar_num);
> +extern void pci_dev_print_id(pcidevaddr_t dev);
>  
>  /*
>   * pci-testdev is a driver for the pci-testdev qemu pci device. The
> -- 
> 1.8.3.1

Thanks,
drew

Re: [kvm-unit-tests PATCH v9 10/12] pci: Add generic ECAM host support

[Andrew Jones]

On Fri, Nov 04, 2016 at 03:33:52PM +0100, Andrew Jones wrote:
> On Thu, Nov 03, 2016 at 05:19:47PM +0100, Alexander Gordeev wrote:
> > Unlike x86, other architectures using generic ECAM PCI host
> > do not have a luxury of PCI bus initialized by a BIOS and
> > ready to use at start. Thus, we need allocate and assign
> > resources to all devices, much like an architecture's
> > firmware would do.
> > 
> > There is no any sort of resource management for memory and
> > io spaces, since only ones-per-BAR allocations are expected
> > and no deallocations at all.
> > 
> > Cc: Thomas Huth <thuth@redhat.com>
> > Cc: Andrew Jones <drjones@redhat.com>
> > Cc: Peter Xu <peterx@redhat.com>
> > Suggested-by: Andrew Jones <drjones@redhat.com>
> > Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> > ---
> >  lib/asm-generic/pci-host-bridge.h |  26 ++++
> >  lib/pci-host-generic.c            | 305 ++++++++++++++++++++++++++++++++++++++
> >  lib/pci-host-generic.h            |  46 ++++++
> >  lib/pci.c                         |   8 +-
> >  lib/pci.h                         |   5 +
> >  5 files changed, 386 insertions(+), 4 deletions(-)
> >  create mode 100644 lib/asm-generic/pci-host-bridge.h
> >  create mode 100644 lib/pci-host-generic.c
> >  create mode 100644 lib/pci-host-generic.h
> > 
> > diff --git a/lib/asm-generic/pci-host-bridge.h b/lib/asm-generic/pci-host-bridge.h
> > new file mode 100644
> > index 000000000000..ac32b85198e9
> > --- /dev/null
> > +++ b/lib/asm-generic/pci-host-bridge.h
> > @@ -0,0 +1,26 @@
> > +#ifndef _ASM_PCI_HOST_BRIDGE_H_
> > +#define _ASM_PCI_HOST_BRIDGE_H_
> > +/*
> > + * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include "libcflat.h"
> > +
> > +phys_addr_t pci_host_bridge_get_paddr(uint64_t addr);
> > +
> > +static inline
> > +phys_addr_t pci_translate_addr(pcidevaddr_t dev __unused, uint64_t addr)
> > +{
> > +	/*
> > +	 * Assume we only have single PCI host bridge in a system.
> > +	 */
> > +	return pci_host_bridge_get_paddr(addr);
> > +}
> > +
> > +uint8_t pci_config_readb(pcidevaddr_t dev, uint8_t reg);
> > +uint16_t pci_config_readw(pcidevaddr_t dev, uint8_t reg);
> > +uint32_t pci_config_readl(pcidevaddr_t dev, uint8_t reg);
> > +void pci_config_writel(pcidevaddr_t dev, uint8_t reg, uint32_t val);
> > +
> > +#endif
> > diff --git a/lib/pci-host-generic.c b/lib/pci-host-generic.c
> > new file mode 100644
> > index 000000000000..d06fff2c7347
> > --- /dev/null
> > +++ b/lib/pci-host-generic.c
> > @@ -0,0 +1,305 @@
> > +/*
> > + * Generic PCI host controller as described in PCI Bus Binding to Open Firmware
> > + *
> > + * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include "libcflat.h"
> > +#include "devicetree.h"
> > +#include "alloc.h"
> > +#include "pci.h"
> > +#include "asm/pci.h"
> > +#include "asm/io.h"
> > +#include "pci-host-generic.h"
> > +#include <linux/pci_regs.h>
> > +
> > +static struct pci_host_bridge *pci_host_bridge;
> > +
> > +static int of_flags_to_pci_type(u32 of_flags)
> > +{
> > +	static int type_map[] = {
> > +		[1] = PCI_BASE_ADDRESS_SPACE_IO,
> > +		[2] = PCI_BASE_ADDRESS_MEM_TYPE_32,
> > +		[3] = PCI_BASE_ADDRESS_MEM_TYPE_64
> > +	};
> > +	int idx = (of_flags >> 24) & 0x03;
> > +	int res;
> > +
> > +	assert(idx > 0);
> > +	res = type_map[idx];
> > +
> > +	if (of_flags & 0x40000000)
> > +		res |= PCI_BASE_ADDRESS_MEM_PREFETCH;
> > +
> > +	return res;
> > +}
> > +
> > +static int pci_bar_type(u32 bar)
> > +{
> > +	if (bar & PCI_BASE_ADDRESS_SPACE)
> > +		return PCI_BASE_ADDRESS_SPACE_IO;
> > +	else
> > +		return bar & (PCI_BASE_ADDRESS_MEM_TYPE_MASK |
> > +			      PCI_BASE_ADDRESS_MEM_PREFETCH);
> > +}
> > +
> > +/*
> > + * Probe DT for a generic PCI host controller
> > + * See kernel Documentation/devicetree/bindings/pci/host-generic-pci.txt
> > + * and function gen_pci_probe() in drivers/pci/host/pci-host-generic.c
> > + */
> > +static struct pci_host_bridge *pci_dt_probe(void)
> > +{
> > +	struct pci_host_bridge *host;
> > +	const void *fdt = dt_fdt();
> > +	const struct fdt_property *prop;
> > +	struct dt_pbus_reg base;
> > +	struct dt_device dt_dev;
> > +	struct dt_bus dt_bus;
> > +	struct pci_addr_space *as;
> > +	fdt32_t *data;
> > +	u32 bus, bus_max;
> > +	u32 nac, nsc, nac_root, nsc_root;
> > +	int nr_range_cells, nr_addr_spaces;
> > +	int ret, node, len, i;
> > +
> > +	if (!dt_available()) {
> > +		printf("No device tree found\n");
> > +		return NULL;
> > +	}
> > +
> > +	dt_bus_init_defaults(&dt_bus);
> > +	dt_device_init(&dt_dev, &dt_bus, NULL);
> > +
> > +	node = fdt_path_offset(fdt, "/");
> > +	assert(node >= 0);
> > +
> > +	ret = dt_get_nr_cells(node, &nac_root, &nsc_root);
> > +	assert(ret == 0);
> > +	assert(nac_root == 1 || nac_root == 2);
> > +
> > +	node = fdt_node_offset_by_compatible(fdt, node,
> > +					     "pci-host-ecam-generic");
> > +	if (node == -FDT_ERR_NOTFOUND) {
> > +		printf("No PCIe ECAM compatible controller found\n");
> > +		return NULL;
> > +	}
> > +	assert(node >= 0);
> > +
> > +	prop = fdt_get_property(fdt, node, "device_type", &len);
> > +	assert(prop && len == 4 && !strcmp((char *)prop->data, "pci"));
> > +
> > +	dt_device_bind_node(&dt_dev, node);
> > +	ret = dt_pbus_get_base(&dt_dev, &base);
> > +	assert(ret == 0);
> > +
> > +	prop = fdt_get_property(fdt, node, "bus-range", &len);
> > +	if (prop == NULL) {
> > +		assert(len == -FDT_ERR_NOTFOUND);
> > +		bus		= 0x00;
> > +		bus_max		= 0xff;
> > +	} else {
> > +		data		= (fdt32_t *)prop->data;
> > +		bus		= fdt32_to_cpu(data[0]);
> > +		bus_max		= fdt32_to_cpu(data[1]);
> > +		assert(bus <= bus_max);
> > +	}
> > +	assert(bus_max < base.size / (1 << PCI_ECAM_BUS_SHIFT));
> > +
> > +	ret = dt_get_nr_cells(node, &nac, &nsc);
> > +	assert(ret == 0);
> > +	assert(nac == 3 && nsc == 2);
> > +
> > +	prop = fdt_get_property(fdt, node, "ranges", &len);
> > +	assert(prop != NULL);
> > +
> > +	nr_range_cells = nac + nsc + nac_root;
> > +	nr_addr_spaces = (len / 4) / nr_range_cells;
> > +	assert(nr_addr_spaces);
> > +
> > +	host = malloc(sizeof(*host) +
> > +		      sizeof(host->addr_space[0]) * nr_addr_spaces);
> > +	assert(host != NULL);
> > +
> > +	host->start		= base.addr;
> > +	host->size		= base.size;
> > +	host->bus		= bus;
> > +	host->bus_max		= bus_max;
> > +	host->nr_addr_spaces	= nr_addr_spaces;
> > +
> > +	data = (fdt32_t *)prop->data;
> > +	as = &host->addr_space[0];
> > +
> > +	for (i = 0; i < nr_addr_spaces; i++) {
> > +		/*
> > +		 * The PCI binding encodes the PCI address with three
> > +		 * cells as follows:
> > +		 *
> > +		 * phys.hi  cell: npt000ss bbbbbbbb dddddfff rrrrrrrr
> > +		 * phys.mid cell: hhhhhhhh hhhhhhhh hhhhhhhh hhhhhhhh
> > +		 * phys.lo  cell: llllllll llllllll llllllll llllllll
> > +		 *
> > +		 * PCI device bus address and flags are encoded into phys.high
> > +		 * PCI 64 bit address is encoded into phys.mid and phys.low
> > +		 */
> > +		as->type = of_flags_to_pci_type(fdt32_to_cpu(data[0]));
> > +		as->pci_start = ((u64)fdt32_to_cpu(data[1]) << 32) |
> > +				fdt32_to_cpu(data[2]);
> > +
> > +		if (nr_range_cells == 6) {
> > +			as->start = fdt32_to_cpu(data[3]);
> > +			as->size  = ((u64)fdt32_to_cpu(data[4]) << 32) |
> > +				    fdt32_to_cpu(data[5]);
> > +		} else {
> > +			as->start = ((u64)fdt32_to_cpu(data[3]) << 32) |
> > +				    fdt32_to_cpu(data[4]);
> > +			as->size  = ((u64)fdt32_to_cpu(data[5]) << 32) |
> > +				    fdt32_to_cpu(data[6]);
> > +		}
> > +
> > +		data += nr_range_cells;
> > +		as++;
> > +	}
> > +
> > +	return host;
> > +}
> > +
> > +static bool pci_alloc_resource(pcidevaddr_t dev, int bar_num, u64 *addr)
> > +{
> > +	struct pci_host_bridge *host = pci_host_bridge;
> > +	struct pci_addr_space *as = &host->addr_space[0];
> > +	u32 mask, bar;
> > +	u64 size;
> > +	int type, i;
> > +
> > +	*addr = ~0;
> > +
> > +	size = pci_bar_size(dev, bar_num);
> > +	if (!size)
> > +		return false;
> > +
> > +	bar = pci_bar_get(dev, bar_num);
> > +	type = pci_bar_type(bar);
> > +	if (type & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
> > +		type &= ~PCI_BASE_ADDRESS_MEM_PREFETCH;
> > +
> > +	for (i = 0; i < host->nr_addr_spaces; i++) {
> > +		if (as->type == type)
> > +			break;
> > +		as++;
> > +	}
> 
> nit: empty line here would be nice
> 
> > +	if (i >= host->nr_addr_spaces) {
> > +		printf("%s: warning: can't satisfy request for ", __func__);
> > +		pci_dev_print_id(dev);
> > +		printf(" ");
> > +		pci_bar_print(dev, bar_num);
> > +		printf("\n");
> > +		return false;
> > +	}
> > +
> > +	mask = pci_bar_mask(bar);
> > +	size = ALIGN(size, ~mask + 1);
> > +	assert(as->allocated + size <= as->size);
> > +
> > +	*addr = as->pci_start + as->allocated;
> > +	as->allocated += size;
> > +
> > +	return true;
> > +}
> > +
> > +bool pci_probe(void)
> > +{
> > +	pcidevaddr_t dev;
> > +	u8 header;
> > +	u32 cmd;
> > +	int i;
> > +
> > +	assert(!pci_host_bridge);
> > +	pci_host_bridge = pci_dt_probe();
> > +	if (!pci_host_bridge)
> > +		return false;
> > +
> > +	for (dev = 0; dev < 256; dev++) {
> > +		if (!pci_dev_exists(dev))
> > +			continue;
> > +
> > +		/* We are only interested in normal PCI devices */
> > +		header = pci_config_readb(dev, PCI_HEADER_TYPE);
> > +		if ((header & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_NORMAL)
> > +			continue;
> > +
> > +		cmd = PCI_COMMAND_SERR;
> > +
> > +		for (i = 0; i < 6; i++) {
> > +			u64 addr;
> > +
> > +			if (pci_alloc_resource(dev, i, &addr)) {
> > +				pci_bar_set_addr(dev, i, addr);
> > +
> > +				if (pci_bar_is_memory(dev, i))
> > +					cmd |= PCI_COMMAND_MEMORY;
> > +				else
> > +					cmd |= PCI_COMMAND_IO;
> > +			}
> > +
> > +			if (pci_bar_is64(dev, i))
> > +				i++;
> > +		}
> > +
> > +		pci_config_writel(dev, PCI_COMMAND, cmd);
> 
> As I asked in the last review, is this PCI_COMMAND_SERR safe/desired
> to write, even when no resources are allocated? If not, then above
> we need to capture the return value of pci_alloc_resource, add
> an 'else break', and then check the return value here before doing
> the write.

Another note on this PCI_COMMAND write. Why is it safe [correct?] to
use writel? This is a 16-bit register according to the comment in
lib/linux/pci_regs.h. Don't we need to introduce a pci_config_writew
and use that? If so, then you should grab Peter Xu's patch that
introduces pci_config_writeb/w.

> 
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +/*
> > + * This function is to be called from pci_translate_addr() to provide
> > + * mapping between this host bridge's PCI busses address and CPU physical
> > + * address.
> > + */
> > +phys_addr_t pci_host_bridge_get_paddr(u64 pci_addr)
> > +{
> > +	struct pci_host_bridge *host = pci_host_bridge;
> > +	struct pci_addr_space *as = &host->addr_space[0];
> > +	int i;
> > +
> > +	for (i = 0; i < host->nr_addr_spaces; i++) {
> > +		if (pci_addr >= as->pci_start &&
> > +		    pci_addr < as->pci_start + as->size)
> > +			return as->start + (pci_addr - as->pci_start);
> > +		as++;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void __iomem *pci_get_dev_conf(struct pci_host_bridge *host, int devfn)
> > +{
> > +	return (void __iomem *)(unsigned long)
> > +		host->start + (devfn << PCI_ECAM_DEVFN_SHIFT);
> > +}
> > +
> > +u8 pci_config_readb(pcidevaddr_t dev, u8 off)
> > +{
> > +	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
> > +	return readb(conf + off);
> > +}
> > +
> > +u16 pci_config_readw(pcidevaddr_t dev, u8 off)
> > +{
> > +	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
> > +	return readw(conf + off);
> > +}
> > +
> > +u32 pci_config_readl(pcidevaddr_t dev, u8 off)
> > +{
> > +	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
> > +	return readl(conf + off);
> > +}
> > +
> > +void pci_config_writel(pcidevaddr_t dev, u8 off, u32 val)
> > +{
> > +	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
> > +	writel(val, conf + off);
> > +}
> > diff --git a/lib/pci-host-generic.h b/lib/pci-host-generic.h
> > new file mode 100644
> > index 000000000000..fd30e7c74ed8
> > --- /dev/null
> > +++ b/lib/pci-host-generic.h
> > @@ -0,0 +1,46 @@
> > +#ifndef PCI_HOST_GENERIC_H
> > +#define PCI_HOST_GENERIC_H
> > +/*
> > + * PCI host bridge supporting structures and constants
> > + *
> > + * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include "libcflat.h"
> > +
> > +struct pci_addr_space {
> > +	phys_addr_t		pci_start;
> > +	phys_addr_t		start;
> > +	phys_addr_t		size;
> > +	phys_addr_t		allocated;
> > +	int			type;
> > +};
> > +
> > +struct pci_host_bridge {
> > +	phys_addr_t		start;
> > +	phys_addr_t		size;
> > +	int			bus;
> > +	int			bus_max;
> > +	int			nr_addr_spaces;
> > +	struct pci_addr_space	addr_space[];
> > +};
> > +
> > +/*
> > + * The following constants are derived from Linux, see this source:
> > + *
> > + *         drivers/pci/host/pci-host-generic.c
> > + *                 struct gen_pci_cfg_bus_ops::bus_shift
> > + *                 int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
> > + *
> > + * Documentation/devicetree/bindings/pci/host-generic-pci.txt describes
> > + * ECAM Configuration Space is be memory-mapped by concatenating the various
> > + * components to form an offset:
> > + *
> > + *	cfg_offset(bus, device, function, register) =
> > + *		   bus << 20 | device << 15 | function << 12 | register
> > + */
> > +#define PCI_ECAM_BUS_SHIFT	20
> > +#define PCI_ECAM_DEVFN_SHIFT	12
> > +
> > +#endif
> > diff --git a/lib/pci.c b/lib/pci.c
> > index e03c67c1a707..6bd54cbac1bb 100644
> > --- a/lib/pci.c
> > +++ b/lib/pci.c
> > @@ -27,13 +27,13 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
> >  	return PCIDEVADDR_INVALID;
> >  }
> >  
> > -static uint32_t pci_bar_mask(uint32_t bar)
> > +uint32_t pci_bar_mask(uint32_t bar)
> >  {
> >  	return (bar & PCI_BASE_ADDRESS_SPACE_IO) ?
> >  		PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
> >  }
> >  
> > -static uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num)
> > +uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num)
> >  {
> >  	return pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
> >  }
> > @@ -127,7 +127,7 @@ bool pci_bar_is64(pcidevaddr_t dev, int bar_num)
> >  		      PCI_BASE_ADDRESS_MEM_TYPE_64;
> >  }
> >  
> > -static void pci_bar_print(pcidevaddr_t dev, int bar_num)
> > +void pci_bar_print(pcidevaddr_t dev, int bar_num)
> >  {
> >  	phys_addr_t size, start, end;
> >  	uint32_t bar;
> > @@ -173,7 +173,7 @@ static void pci_bar_print(pcidevaddr_t dev, int bar_num)
> >  	printf("]");
> >  }
> >  
> > -static void pci_dev_print_id(pcidevaddr_t dev)
> > +void pci_dev_print_id(pcidevaddr_t dev)
> >  {
> >  	printf("00.%02x.%1x %04x:%04x", dev / 8, dev % 8,
> >  		pci_config_readw(dev, PCI_VENDOR_ID),
> > diff --git a/lib/pci.h b/lib/pci.h
> > index fc0940adc299..9d960556c526 100644
> > --- a/lib/pci.h
> > +++ b/lib/pci.h
> > @@ -15,6 +15,7 @@ enum {
> >  	PCIDEVADDR_INVALID = 0xffff,
> >  };
> >  
> > +extern bool pci_probe(void);
> >  extern void pci_print(void);
> >  extern bool pci_dev_exists(pcidevaddr_t dev);
> >  extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
> > @@ -34,9 +35,13 @@ extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
> >  extern phys_addr_t pci_bar_get_addr(pcidevaddr_t dev, int bar_num);
> >  extern void pci_bar_set_addr(pcidevaddr_t dev, int bar_num, phys_addr_t addr);
> >  extern phys_addr_t pci_bar_size(pcidevaddr_t dev, int bar_num);
> > +extern uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num);
> > +extern uint32_t pci_bar_mask(uint32_t bar);
> >  extern bool pci_bar_is64(pcidevaddr_t dev, int bar_num);
> >  extern bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num);
> >  extern bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num);
> > +extern void pci_bar_print(pcidevaddr_t dev, int bar_num);
> > +extern void pci_dev_print_id(pcidevaddr_t dev);
> >  
> >  /*
> >   * pci-testdev is a driver for the pci-testdev qemu pci device. The
> > -- 
> > 1.8.3.1
> 
> Thanks,
> drew

Re: [kvm-unit-tests PATCH v9 10/12] pci: Add generic ECAM host support

[Alexander Gordeev]

On Fri, Nov 04, 2016 at 06:07:41PM +0100, Andrew Jones wrote:
> > > +bool pci_probe(void)
> > > +{
> > > +	pcidevaddr_t dev;
> > > +	u8 header;
> > > +	u32 cmd;
> > > +	int i;
> > > +
> > > +	assert(!pci_host_bridge);
> > > +	pci_host_bridge = pci_dt_probe();
> > > +	if (!pci_host_bridge)
> > > +		return false;
> > > +
> > > +	for (dev = 0; dev < 256; dev++) {
> > > +		if (!pci_dev_exists(dev))
> > > +			continue;
> > > +
> > > +		/* We are only interested in normal PCI devices */
> > > +		header = pci_config_readb(dev, PCI_HEADER_TYPE);
> > > +		if ((header & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_NORMAL)
> > > +			continue;
> > > +
> > > +		cmd = PCI_COMMAND_SERR;
> > > +
> > > +		for (i = 0; i < 6; i++) {
> > > +			u64 addr;
> > > +
> > > +			if (pci_alloc_resource(dev, i, &addr)) {
> > > +				pci_bar_set_addr(dev, i, addr);
> > > +
> > > +				if (pci_bar_is_memory(dev, i))
> > > +					cmd |= PCI_COMMAND_MEMORY;
> > > +				else
> > > +					cmd |= PCI_COMMAND_IO;
> > > +			}
> > > +
> > > +			if (pci_bar_is64(dev, i))
> > > +				i++;
> > > +		}
> > > +
> > > +		pci_config_writel(dev, PCI_COMMAND, cmd);
> > 
> > As I asked in the last review, is this PCI_COMMAND_SERR safe/desired
> > to write, even when no resources are allocated? If not, then above
> > we need to capture the return value of pci_alloc_resource, add
> > an 'else break', and then check the return value here before doing
> > the write.
> 
> Another note on this PCI_COMMAND write. Why is it safe [correct?] to
> use writel? This is a 16-bit register according to the comment in
> lib/linux/pci_regs.h. Don't we need to introduce a pci_config_writew
> and use that? If so, then you should grab Peter Xu's patch that
> introduces pci_config_writeb/w.

You are right, this is cleanly a bug.

However, I can not put Peter's patch neither before nor after mine,
since pci_config_write*() are both introduced and used in this patch.
As Peter's patch needs a refreshment anyway (see my comment on it)
I will add pci_config_writew() to this patch. Hope, that works for
everyone.

> > 
> > > +	}
> > > +
> > > +	return true;
> > > +}

> > Thanks,
> > drew

Re: [kvm-unit-tests PATCH v9 10/12] pci: Add generic ECAM host support

[Andrew Jones]

On Sat, Nov 05, 2016 at 06:25:44PM +0100, Alexander Gordeev wrote:
> > Another note on this PCI_COMMAND write. Why is it safe [correct?] to
> > use writel? This is a 16-bit register according to the comment in
> > lib/linux/pci_regs.h. Don't we need to introduce a pci_config_writew
> > and use that? If so, then you should grab Peter Xu's patch that
> > introduces pci_config_writeb/w.
> 
> You are right, this is cleanly a bug.
> 
> However, I can not put Peter's patch neither before nor after mine,
> since pci_config_write*() are both introduced and used in this patch.
> As Peter's patch needs a refreshment anyway (see my comment on it)
> I will add pci_config_writew() to this patch. Hope, that works for
> everyone.

It's a trivial patch, so I guess Peter would be fine with you just
doing it however it works best for this series, and then he'll drop
his patch.

drew

Re: [kvm-unit-tests PATCH v9 10/12] pci: Add generic ECAM host support

[Peter Xu]

On Mon, Nov 07, 2016 at 09:44:39AM +0100, Andrew Jones wrote:
> On Sat, Nov 05, 2016 at 06:25:44PM +0100, Alexander Gordeev wrote:
> > > Another note on this PCI_COMMAND write. Why is it safe [correct?] to
> > > use writel? This is a 16-bit register according to the comment in
> > > lib/linux/pci_regs.h. Don't we need to introduce a pci_config_writew
> > > and use that? If so, then you should grab Peter Xu's patch that
> > > introduces pci_config_writeb/w.
> > 
> > You are right, this is cleanly a bug.
> > 
> > However, I can not put Peter's patch neither before nor after mine,
> > since pci_config_write*() are both introduced and used in this patch.
> > As Peter's patch needs a refreshment anyway (see my comment on it)
> > I will add pci_config_writew() to this patch. Hope, that works for
> > everyone.
> 
> It's a trivial patch, so I guess Peter would be fine with you just
> doing it however it works best for this series, and then he'll drop
> his patch.

Yeah, please. I'll drop mine. Thanks,

-- peterx

Re: [kvm-unit-tests PATCH v9 10/12] pci: Add generic ECAM host support

[Alexander Gordeev]

On Fri, Nov 04, 2016 at 03:33:52PM +0100, Andrew Jones wrote:
> > +bool pci_probe(void)
> > +{
> > +	pcidevaddr_t dev;
> > +	u8 header;
> > +	u32 cmd;
> > +	int i;
> > +
> > +	assert(!pci_host_bridge);
> > +	pci_host_bridge = pci_dt_probe();
> > +	if (!pci_host_bridge)
> > +		return false;
> > +
> > +	for (dev = 0; dev < 256; dev++) {
> > +		if (!pci_dev_exists(dev))
> > +			continue;
> > +
> > +		/* We are only interested in normal PCI devices */
> > +		header = pci_config_readb(dev, PCI_HEADER_TYPE);
> > +		if ((header & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_NORMAL)
> > +			continue;
> > +
> > +		cmd = PCI_COMMAND_SERR;
> > +
> > +		for (i = 0; i < 6; i++) {
> > +			u64 addr;
> > +
> > +			if (pci_alloc_resource(dev, i, &addr)) {
> > +				pci_bar_set_addr(dev, i, addr);
> > +
> > +				if (pci_bar_is_memory(dev, i))
> > +					cmd |= PCI_COMMAND_MEMORY;
> > +				else
> > +					cmd |= PCI_COMMAND_IO;
> > +			}
> > +
> > +			if (pci_bar_is64(dev, i))
> > +				i++;
> > +		}
> > +
> > +		pci_config_writel(dev, PCI_COMMAND, cmd);
> 
> As I asked in the last review, is this PCI_COMMAND_SERR safe/desired
> to write, even when no resources are allocated? If not, then above

Yes, it allows a device to report grave PCI bus errors. Moreover,
I think I should have enabled parity errors check as well.

I assume we want to notice such errors, including the case no
resources were assigned. That would be more troubling even.

In Linux it is up to device drivers to enable these, but I
would just let all devices scream loudly by default.

> we need to capture the return value of pci_alloc_resource, add
> an 'else break', and then check the return value here before doing
> the write.
> 
> > +	}
> > +
> > +	return true;
> > +}
> Thanks,
> drew

[kvm-unit-tests PATCH v10 10/12] pci: Add generic ECAM host support

[Alexander Gordeev]

Unlike x86, other architectures using generic ECAM PCI host
do not have a luxury of PCI bus initialized by a BIOS and
ready to use at start. Thus, we need allocate and assign
resources to all devices, much like an architecture's
firmware would do.

There is no any sort of resource management for memory and
io spaces, since only ones-per-BAR allocations are expected
and no deallocations at all.

Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/asm-generic/pci-host-bridge.h |  26 ++++
 lib/pci-host-generic.c            | 305 ++++++++++++++++++++++++++++++++++++++
 lib/pci-host-generic.h            |  46 ++++++
 lib/pci.c                         |   8 +-
 lib/pci.h                         |   5 +
 5 files changed, 386 insertions(+), 4 deletions(-)
 create mode 100644 lib/asm-generic/pci-host-bridge.h
 create mode 100644 lib/pci-host-generic.c
 create mode 100644 lib/pci-host-generic.h

diff --git a/lib/asm-generic/pci-host-bridge.h b/lib/asm-generic/pci-host-bridge.h
new file mode 100644
index 000000000000..ac32b85198e9
--- /dev/null
+++ b/lib/asm-generic/pci-host-bridge.h
@@ -0,0 +1,26 @@
+#ifndef _ASM_PCI_HOST_BRIDGE_H_
+#define _ASM_PCI_HOST_BRIDGE_H_
+/*
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include "libcflat.h"
+
+phys_addr_t pci_host_bridge_get_paddr(uint64_t addr);
+
+static inline
+phys_addr_t pci_translate_addr(pcidevaddr_t dev __unused, uint64_t addr)
+{
+	/*
+	 * Assume we only have single PCI host bridge in a system.
+	 */
+	return pci_host_bridge_get_paddr(addr);
+}
+
+uint8_t pci_config_readb(pcidevaddr_t dev, uint8_t reg);
+uint16_t pci_config_readw(pcidevaddr_t dev, uint8_t reg);
+uint32_t pci_config_readl(pcidevaddr_t dev, uint8_t reg);
+void pci_config_writel(pcidevaddr_t dev, uint8_t reg, uint32_t val);
+
+#endif
diff --git a/lib/pci-host-generic.c b/lib/pci-host-generic.c
new file mode 100644
index 000000000000..d06fff2c7347
--- /dev/null
+++ b/lib/pci-host-generic.c
@@ -0,0 +1,305 @@
+/*
+ * Generic PCI host controller as described in PCI Bus Binding to Open Firmware
+ *
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include "libcflat.h"
+#include "devicetree.h"
+#include "alloc.h"
+#include "pci.h"
+#include "asm/pci.h"
+#include "asm/io.h"
+#include "pci-host-generic.h"
+#include <linux/pci_regs.h>
+
+static struct pci_host_bridge *pci_host_bridge;
+
+static int of_flags_to_pci_type(u32 of_flags)
+{
+	static int type_map[] = {
+		[1] = PCI_BASE_ADDRESS_SPACE_IO,
+		[2] = PCI_BASE_ADDRESS_MEM_TYPE_32,
+		[3] = PCI_BASE_ADDRESS_MEM_TYPE_64
+	};
+	int idx = (of_flags >> 24) & 0x03;
+	int res;
+
+	assert(idx > 0);
+	res = type_map[idx];
+
+	if (of_flags & 0x40000000)
+		res |= PCI_BASE_ADDRESS_MEM_PREFETCH;
+
+	return res;
+}
+
+static int pci_bar_type(u32 bar)
+{
+	if (bar & PCI_BASE_ADDRESS_SPACE)
+		return PCI_BASE_ADDRESS_SPACE_IO;
+	else
+		return bar & (PCI_BASE_ADDRESS_MEM_TYPE_MASK |
+			      PCI_BASE_ADDRESS_MEM_PREFETCH);
+}
+
+/*
+ * Probe DT for a generic PCI host controller
+ * See kernel Documentation/devicetree/bindings/pci/host-generic-pci.txt
+ * and function gen_pci_probe() in drivers/pci/host/pci-host-generic.c
+ */
+static struct pci_host_bridge *pci_dt_probe(void)
+{
+	struct pci_host_bridge *host;
+	const void *fdt = dt_fdt();
+	const struct fdt_property *prop;
+	struct dt_pbus_reg base;
+	struct dt_device dt_dev;
+	struct dt_bus dt_bus;
+	struct pci_addr_space *as;
+	fdt32_t *data;
+	u32 bus, bus_max;
+	u32 nac, nsc, nac_root, nsc_root;
+	int nr_range_cells, nr_addr_spaces;
+	int ret, node, len, i;
+
+	if (!dt_available()) {
+		printf("No device tree found\n");
+		return NULL;
+	}
+
+	dt_bus_init_defaults(&dt_bus);
+	dt_device_init(&dt_dev, &dt_bus, NULL);
+
+	node = fdt_path_offset(fdt, "/");
+	assert(node >= 0);
+
+	ret = dt_get_nr_cells(node, &nac_root, &nsc_root);
+	assert(ret == 0);
+	assert(nac_root == 1 || nac_root == 2);
+
+	node = fdt_node_offset_by_compatible(fdt, node,
+					     "pci-host-ecam-generic");
+	if (node == -FDT_ERR_NOTFOUND) {
+		printf("No PCIe ECAM compatible controller found\n");
+		return NULL;
+	}
+	assert(node >= 0);
+
+	prop = fdt_get_property(fdt, node, "device_type", &len);
+	assert(prop && len == 4 && !strcmp((char *)prop->data, "pci"));
+
+	dt_device_bind_node(&dt_dev, node);
+	ret = dt_pbus_get_base(&dt_dev, &base);
+	assert(ret == 0);
+
+	prop = fdt_get_property(fdt, node, "bus-range", &len);
+	if (prop == NULL) {
+		assert(len == -FDT_ERR_NOTFOUND);
+		bus		= 0x00;
+		bus_max		= 0xff;
+	} else {
+		data		= (fdt32_t *)prop->data;
+		bus		= fdt32_to_cpu(data[0]);
+		bus_max		= fdt32_to_cpu(data[1]);
+		assert(bus <= bus_max);
+	}
+	assert(bus_max < base.size / (1 << PCI_ECAM_BUS_SHIFT));
+
+	ret = dt_get_nr_cells(node, &nac, &nsc);
+	assert(ret == 0);
+	assert(nac == 3 && nsc == 2);
+
+	prop = fdt_get_property(fdt, node, "ranges", &len);
+	assert(prop != NULL);
+
+	nr_range_cells = nac + nsc + nac_root;
+	nr_addr_spaces = (len / 4) / nr_range_cells;
+	assert(nr_addr_spaces);
+
+	host = malloc(sizeof(*host) +
+		      sizeof(host->addr_space[0]) * nr_addr_spaces);
+	assert(host != NULL);
+
+	host->start		= base.addr;
+	host->size		= base.size;
+	host->bus		= bus;
+	host->bus_max		= bus_max;
+	host->nr_addr_spaces	= nr_addr_spaces;
+
+	data = (fdt32_t *)prop->data;
+	as = &host->addr_space[0];
+
+	for (i = 0; i < nr_addr_spaces; i++) {
+		/*
+		 * The PCI binding encodes the PCI address with three
+		 * cells as follows:
+		 *
+		 * phys.hi  cell: npt000ss bbbbbbbb dddddfff rrrrrrrr
+		 * phys.mid cell: hhhhhhhh hhhhhhhh hhhhhhhh hhhhhhhh
+		 * phys.lo  cell: llllllll llllllll llllllll llllllll
+		 *
+		 * PCI device bus address and flags are encoded into phys.high
+		 * PCI 64 bit address is encoded into phys.mid and phys.low
+		 */
+		as->type = of_flags_to_pci_type(fdt32_to_cpu(data[0]));
+		as->pci_start = ((u64)fdt32_to_cpu(data[1]) << 32) |
+				fdt32_to_cpu(data[2]);
+
+		if (nr_range_cells == 6) {
+			as->start = fdt32_to_cpu(data[3]);
+			as->size  = ((u64)fdt32_to_cpu(data[4]) << 32) |
+				    fdt32_to_cpu(data[5]);
+		} else {
+			as->start = ((u64)fdt32_to_cpu(data[3]) << 32) |
+				    fdt32_to_cpu(data[4]);
+			as->size  = ((u64)fdt32_to_cpu(data[5]) << 32) |
+				    fdt32_to_cpu(data[6]);
+		}
+
+		data += nr_range_cells;
+		as++;
+	}
+
+	return host;
+}
+
+static bool pci_alloc_resource(pcidevaddr_t dev, int bar_num, u64 *addr)
+{
+	struct pci_host_bridge *host = pci_host_bridge;
+	struct pci_addr_space *as = &host->addr_space[0];
+	u32 mask, bar;
+	u64 size;
+	int type, i;
+
+	*addr = ~0;
+
+	size = pci_bar_size(dev, bar_num);
+	if (!size)
+		return false;
+
+	bar = pci_bar_get(dev, bar_num);
+	type = pci_bar_type(bar);
+	if (type & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
+		type &= ~PCI_BASE_ADDRESS_MEM_PREFETCH;
+
+	for (i = 0; i < host->nr_addr_spaces; i++) {
+		if (as->type == type)
+			break;
+		as++;
+	}
+	if (i >= host->nr_addr_spaces) {
+		printf("%s: warning: can't satisfy request for ", __func__);
+		pci_dev_print_id(dev);
+		printf(" ");
+		pci_bar_print(dev, bar_num);
+		printf("\n");
+		return false;
+	}
+
+	mask = pci_bar_mask(bar);
+	size = ALIGN(size, ~mask + 1);
+	assert(as->allocated + size <= as->size);
+
+	*addr = as->pci_start + as->allocated;
+	as->allocated += size;
+
+	return true;
+}
+
+bool pci_probe(void)
+{
+	pcidevaddr_t dev;
+	u8 header;
+	u32 cmd;
+	int i;
+
+	assert(!pci_host_bridge);
+	pci_host_bridge = pci_dt_probe();
+	if (!pci_host_bridge)
+		return false;
+
+	for (dev = 0; dev < 256; dev++) {
+		if (!pci_dev_exists(dev))
+			continue;
+
+		/* We are only interested in normal PCI devices */
+		header = pci_config_readb(dev, PCI_HEADER_TYPE);
+		if ((header & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_NORMAL)
+			continue;
+
+		cmd = PCI_COMMAND_SERR;
+
+		for (i = 0; i < 6; i++) {
+			u64 addr;
+
+			if (pci_alloc_resource(dev, i, &addr)) {
+				pci_bar_set_addr(dev, i, addr);
+
+				if (pci_bar_is_memory(dev, i))
+					cmd |= PCI_COMMAND_MEMORY;
+				else
+					cmd |= PCI_COMMAND_IO;
+			}
+
+			if (pci_bar_is64(dev, i))
+				i++;
+		}
+
+		pci_config_writel(dev, PCI_COMMAND, cmd);
+	}
+
+	return true;
+}
+
+/*
+ * This function is to be called from pci_translate_addr() to provide
+ * mapping between this host bridge's PCI busses address and CPU physical
+ * address.
+ */
+phys_addr_t pci_host_bridge_get_paddr(u64 pci_addr)
+{
+	struct pci_host_bridge *host = pci_host_bridge;
+	struct pci_addr_space *as = &host->addr_space[0];
+	int i;
+
+	for (i = 0; i < host->nr_addr_spaces; i++) {
+		if (pci_addr >= as->pci_start &&
+		    pci_addr < as->pci_start + as->size)
+			return as->start + (pci_addr - as->pci_start);
+		as++;
+	}
+
+	return 0;
+}
+
+static void __iomem *pci_get_dev_conf(struct pci_host_bridge *host, int devfn)
+{
+	return (void __iomem *)(unsigned long)
+		host->start + (devfn << PCI_ECAM_DEVFN_SHIFT);
+}
+
+u8 pci_config_readb(pcidevaddr_t dev, u8 off)
+{
+	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
+	return readb(conf + off);
+}
+
+u16 pci_config_readw(pcidevaddr_t dev, u8 off)
+{
+	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
+	return readw(conf + off);
+}
+
+u32 pci_config_readl(pcidevaddr_t dev, u8 off)
+{
+	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
+	return readl(conf + off);
+}
+
+void pci_config_writel(pcidevaddr_t dev, u8 off, u32 val)
+{
+	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
+	writel(val, conf + off);
+}
diff --git a/lib/pci-host-generic.h b/lib/pci-host-generic.h
new file mode 100644
index 000000000000..fd30e7c74ed8
--- /dev/null
+++ b/lib/pci-host-generic.h
@@ -0,0 +1,46 @@
+#ifndef PCI_HOST_GENERIC_H
+#define PCI_HOST_GENERIC_H
+/*
+ * PCI host bridge supporting structures and constants
+ *
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include "libcflat.h"
+
+struct pci_addr_space {
+	phys_addr_t		pci_start;
+	phys_addr_t		start;
+	phys_addr_t		size;
+	phys_addr_t		allocated;
+	int			type;
+};
+
+struct pci_host_bridge {
+	phys_addr_t		start;
+	phys_addr_t		size;
+	int			bus;
+	int			bus_max;
+	int			nr_addr_spaces;
+	struct pci_addr_space	addr_space[];
+};
+
+/*
+ * The following constants are derived from Linux, see this source:
+ *
+ *         drivers/pci/host/pci-host-generic.c
+ *                 struct gen_pci_cfg_bus_ops::bus_shift
+ *                 int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
+ *
+ * Documentation/devicetree/bindings/pci/host-generic-pci.txt describes
+ * ECAM Configuration Space is be memory-mapped by concatenating the various
+ * components to form an offset:
+ *
+ *	cfg_offset(bus, device, function, register) =
+ *		   bus << 20 | device << 15 | function << 12 | register
+ */
+#define PCI_ECAM_BUS_SHIFT	20
+#define PCI_ECAM_DEVFN_SHIFT	12
+
+#endif
diff --git a/lib/pci.c b/lib/pci.c
index e03c67c1a707..6bd54cbac1bb 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -27,13 +27,13 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
 	return PCIDEVADDR_INVALID;
 }
 
-static uint32_t pci_bar_mask(uint32_t bar)
+uint32_t pci_bar_mask(uint32_t bar)
 {
 	return (bar & PCI_BASE_ADDRESS_SPACE_IO) ?
 		PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
 }
 
-static uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num)
+uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num)
 {
 	return pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
 }
@@ -127,7 +127,7 @@ bool pci_bar_is64(pcidevaddr_t dev, int bar_num)
 		      PCI_BASE_ADDRESS_MEM_TYPE_64;
 }
 
-static void pci_bar_print(pcidevaddr_t dev, int bar_num)
+void pci_bar_print(pcidevaddr_t dev, int bar_num)
 {
 	phys_addr_t size, start, end;
 	uint32_t bar;
@@ -173,7 +173,7 @@ static void pci_bar_print(pcidevaddr_t dev, int bar_num)
 	printf("]");
 }
 
-static void pci_dev_print_id(pcidevaddr_t dev)
+void pci_dev_print_id(pcidevaddr_t dev)
 {
 	printf("00.%02x.%1x %04x:%04x", dev / 8, dev % 8,
 		pci_config_readw(dev, PCI_VENDOR_ID),
diff --git a/lib/pci.h b/lib/pci.h
index fc0940adc299..9d960556c526 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -15,6 +15,7 @@ enum {
 	PCIDEVADDR_INVALID = 0xffff,
 };
 
+extern bool pci_probe(void);
 extern void pci_print(void);
 extern bool pci_dev_exists(pcidevaddr_t dev);
 extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
@@ -34,9 +35,13 @@ extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
 extern phys_addr_t pci_bar_get_addr(pcidevaddr_t dev, int bar_num);
 extern void pci_bar_set_addr(pcidevaddr_t dev, int bar_num, phys_addr_t addr);
 extern phys_addr_t pci_bar_size(pcidevaddr_t dev, int bar_num);
+extern uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num);
+extern uint32_t pci_bar_mask(uint32_t bar);
 extern bool pci_bar_is64(pcidevaddr_t dev, int bar_num);
 extern bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num);
 extern bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num);
+extern void pci_bar_print(pcidevaddr_t dev, int bar_num);
+extern void pci_dev_print_id(pcidevaddr_t dev);
 
 /*
  * pci-testdev is a driver for the pci-testdev qemu pci device. The
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH v10 10/12] pci: Add generic ECAM host support

[Alexander Gordeev]

On Sat, Nov 05, 2016 at 06:20:06PM +0100, Alexander Gordeev wrote:
> +bool pci_probe(void)
> +{
> +	pcidevaddr_t dev;
> +	u8 header;
> +	u32 cmd;
> +	int i;
> +
> +	assert(!pci_host_bridge);
> +	pci_host_bridge = pci_dt_probe();
> +	if (!pci_host_bridge)
> +		return false;
> +
> +	for (dev = 0; dev < 256; dev++) {
> +		if (!pci_dev_exists(dev))
> +			continue;
> +
> +		/* We are only interested in normal PCI devices */
> +		header = pci_config_readb(dev, PCI_HEADER_TYPE);
> +		if ((header & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_NORMAL)
> +			continue;
> +
> +		cmd = PCI_COMMAND_SERR;

Sigh. Please, ignore this patch.
I forgot to add PCI_COMMAND_PARITY here, as intended.

> +		for (i = 0; i < 6; i++) {
> +			u64 addr;
> +
> +			if (pci_alloc_resource(dev, i, &addr)) {
> +				pci_bar_set_addr(dev, i, addr);
> +
> +				if (pci_bar_is_memory(dev, i))
> +					cmd |= PCI_COMMAND_MEMORY;
> +				else
> +					cmd |= PCI_COMMAND_IO;
> +			}
> +
> +			if (pci_bar_is64(dev, i))
> +				i++;
> +		}
> +
> +		pci_config_writel(dev, PCI_COMMAND, cmd);
> +	}
> +
> +	return true;
> +}

[kvm-unit-tests PATCH v10 10/12] pci: Add generic ECAM host support

[Alexander Gordeev]

Unlike x86, other architectures using generic ECAM PCI host
do not have a luxury of PCI bus initialized by a BIOS and
ready to use at start. Thus, we need allocate and assign
resources to all devices, much like an architecture's
firmware would do.

There is no any sort of resource management for memory and
io spaces, since only ones-per-BAR allocations are expected
and no deallocations at all.

Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/asm-generic/pci-host-bridge.h |  27 ++++
 lib/pci-host-generic.c            | 312 ++++++++++++++++++++++++++++++++++++++
 lib/pci-host-generic.h            |  46 ++++++
 lib/pci.c                         |   8 +-
 lib/pci.h                         |   5 +
 5 files changed, 394 insertions(+), 4 deletions(-)
 create mode 100644 lib/asm-generic/pci-host-bridge.h
 create mode 100644 lib/pci-host-generic.c
 create mode 100644 lib/pci-host-generic.h

diff --git a/lib/asm-generic/pci-host-bridge.h b/lib/asm-generic/pci-host-bridge.h
new file mode 100644
index 000000000000..d4c3047cce73
--- /dev/null
+++ b/lib/asm-generic/pci-host-bridge.h
@@ -0,0 +1,27 @@
+#ifndef _ASM_PCI_HOST_BRIDGE_H_
+#define _ASM_PCI_HOST_BRIDGE_H_
+/*
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include "libcflat.h"
+
+phys_addr_t pci_host_bridge_get_paddr(uint64_t addr);
+
+static inline
+phys_addr_t pci_translate_addr(pcidevaddr_t dev __unused, uint64_t addr)
+{
+	/*
+	 * Assume we only have single PCI host bridge in a system.
+	 */
+	return pci_host_bridge_get_paddr(addr);
+}
+
+uint8_t pci_config_readb(pcidevaddr_t dev, uint8_t reg);
+uint16_t pci_config_readw(pcidevaddr_t dev, uint8_t reg);
+uint32_t pci_config_readl(pcidevaddr_t dev, uint8_t reg);
+void pci_config_writew(pcidevaddr_t dev, uint8_t reg, uint32_t val);
+void pci_config_writel(pcidevaddr_t dev, uint8_t reg, uint32_t val);
+
+#endif
diff --git a/lib/pci-host-generic.c b/lib/pci-host-generic.c
new file mode 100644
index 000000000000..b3bc357514db
--- /dev/null
+++ b/lib/pci-host-generic.c
@@ -0,0 +1,312 @@
+/*
+ * Generic PCI host controller as described in PCI Bus Binding to Open Firmware
+ *
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include "libcflat.h"
+#include "devicetree.h"
+#include "alloc.h"
+#include "pci.h"
+#include "asm/pci.h"
+#include "asm/io.h"
+#include "pci-host-generic.h"
+#include <linux/pci_regs.h>
+
+static struct pci_host_bridge *pci_host_bridge;
+
+static int of_flags_to_pci_type(u32 of_flags)
+{
+	static int type_map[] = {
+		[1] = PCI_BASE_ADDRESS_SPACE_IO,
+		[2] = PCI_BASE_ADDRESS_MEM_TYPE_32,
+		[3] = PCI_BASE_ADDRESS_MEM_TYPE_64
+	};
+	int idx = (of_flags >> 24) & 0x03;
+	int res;
+
+	assert(idx > 0);
+	res = type_map[idx];
+
+	if (of_flags & 0x40000000)
+		res |= PCI_BASE_ADDRESS_MEM_PREFETCH;
+
+	return res;
+}
+
+static int pci_bar_type(u32 bar)
+{
+	if (bar & PCI_BASE_ADDRESS_SPACE)
+		return PCI_BASE_ADDRESS_SPACE_IO;
+	else
+		return bar & (PCI_BASE_ADDRESS_MEM_TYPE_MASK |
+			      PCI_BASE_ADDRESS_MEM_PREFETCH);
+}
+
+/*
+ * Probe DT for a generic PCI host controller
+ * See kernel Documentation/devicetree/bindings/pci/host-generic-pci.txt
+ * and function gen_pci_probe() in drivers/pci/host/pci-host-generic.c
+ */
+static struct pci_host_bridge *pci_dt_probe(void)
+{
+	struct pci_host_bridge *host;
+	const void *fdt = dt_fdt();
+	const struct fdt_property *prop;
+	struct dt_pbus_reg base;
+	struct dt_device dt_dev;
+	struct dt_bus dt_bus;
+	struct pci_addr_space *as;
+	fdt32_t *data;
+	u32 bus, bus_max;
+	u32 nac, nsc, nac_root, nsc_root;
+	int nr_range_cells, nr_addr_spaces;
+	int ret, node, len, i;
+
+	if (!dt_available()) {
+		printf("No device tree found\n");
+		return NULL;
+	}
+
+	dt_bus_init_defaults(&dt_bus);
+	dt_device_init(&dt_dev, &dt_bus, NULL);
+
+	node = fdt_path_offset(fdt, "/");
+	assert(node >= 0);
+
+	ret = dt_get_nr_cells(node, &nac_root, &nsc_root);
+	assert(ret == 0);
+	assert(nac_root == 1 || nac_root == 2);
+
+	node = fdt_node_offset_by_compatible(fdt, node,
+					     "pci-host-ecam-generic");
+	if (node == -FDT_ERR_NOTFOUND) {
+		printf("No PCIe ECAM compatible controller found\n");
+		return NULL;
+	}
+	assert(node >= 0);
+
+	prop = fdt_get_property(fdt, node, "device_type", &len);
+	assert(prop && len == 4 && !strcmp((char *)prop->data, "pci"));
+
+	dt_device_bind_node(&dt_dev, node);
+	ret = dt_pbus_get_base(&dt_dev, &base);
+	assert(ret == 0);
+
+	prop = fdt_get_property(fdt, node, "bus-range", &len);
+	if (prop == NULL) {
+		assert(len == -FDT_ERR_NOTFOUND);
+		bus		= 0x00;
+		bus_max		= 0xff;
+	} else {
+		data		= (fdt32_t *)prop->data;
+		bus		= fdt32_to_cpu(data[0]);
+		bus_max		= fdt32_to_cpu(data[1]);
+		assert(bus <= bus_max);
+	}
+	assert(bus_max < base.size / (1 << PCI_ECAM_BUS_SHIFT));
+
+	ret = dt_get_nr_cells(node, &nac, &nsc);
+	assert(ret == 0);
+	assert(nac == 3 && nsc == 2);
+
+	prop = fdt_get_property(fdt, node, "ranges", &len);
+	assert(prop != NULL);
+
+	nr_range_cells = nac + nsc + nac_root;
+	nr_addr_spaces = (len / 4) / nr_range_cells;
+	assert(nr_addr_spaces);
+
+	host = malloc(sizeof(*host) +
+		      sizeof(host->addr_space[0]) * nr_addr_spaces);
+	assert(host != NULL);
+
+	host->start		= base.addr;
+	host->size		= base.size;
+	host->bus		= bus;
+	host->bus_max		= bus_max;
+	host->nr_addr_spaces	= nr_addr_spaces;
+
+	data = (fdt32_t *)prop->data;
+	as = &host->addr_space[0];
+
+	for (i = 0; i < nr_addr_spaces; i++) {
+		/*
+		 * The PCI binding encodes the PCI address with three
+		 * cells as follows:
+		 *
+		 * phys.hi  cell: npt000ss bbbbbbbb dddddfff rrrrrrrr
+		 * phys.mid cell: hhhhhhhh hhhhhhhh hhhhhhhh hhhhhhhh
+		 * phys.lo  cell: llllllll llllllll llllllll llllllll
+		 *
+		 * PCI device bus address and flags are encoded into phys.high
+		 * PCI 64 bit address is encoded into phys.mid and phys.low
+		 */
+		as->type = of_flags_to_pci_type(fdt32_to_cpu(data[0]));
+		as->pci_start = ((u64)fdt32_to_cpu(data[1]) << 32) |
+				fdt32_to_cpu(data[2]);
+
+		if (nr_range_cells == 6) {
+			as->start = fdt32_to_cpu(data[3]);
+			as->size  = ((u64)fdt32_to_cpu(data[4]) << 32) |
+				    fdt32_to_cpu(data[5]);
+		} else {
+			as->start = ((u64)fdt32_to_cpu(data[3]) << 32) |
+				    fdt32_to_cpu(data[4]);
+			as->size  = ((u64)fdt32_to_cpu(data[5]) << 32) |
+				    fdt32_to_cpu(data[6]);
+		}
+
+		data += nr_range_cells;
+		as++;
+	}
+
+	return host;
+}
+
+static bool pci_alloc_resource(pcidevaddr_t dev, int bar_num, u64 *addr)
+{
+	struct pci_host_bridge *host = pci_host_bridge;
+	struct pci_addr_space *as = &host->addr_space[0];
+	u32 mask, bar;
+	u64 size;
+	int type, i;
+
+	*addr = ~0;
+
+	size = pci_bar_size(dev, bar_num);
+	if (!size)
+		return false;
+
+	bar = pci_bar_get(dev, bar_num);
+	type = pci_bar_type(bar);
+	if (type & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
+		type &= ~PCI_BASE_ADDRESS_MEM_PREFETCH;
+
+	for (i = 0; i < host->nr_addr_spaces; i++) {
+		if (as->type == type)
+			break;
+		as++;
+	}
+
+	if (i >= host->nr_addr_spaces) {
+		printf("%s: warning: can't satisfy request for ", __func__);
+		pci_dev_print_id(dev);
+		printf(" ");
+		pci_bar_print(dev, bar_num);
+		printf("\n");
+		return false;
+	}
+
+	mask = pci_bar_mask(bar);
+	size = ALIGN(size, ~mask + 1);
+	assert(as->allocated + size <= as->size);
+
+	*addr = as->pci_start + as->allocated;
+	as->allocated += size;
+
+	return true;
+}
+
+bool pci_probe(void)
+{
+	pcidevaddr_t dev;
+	u8 header;
+	u32 cmd;
+	int i;
+
+	assert(!pci_host_bridge);
+	pci_host_bridge = pci_dt_probe();
+	if (!pci_host_bridge)
+		return false;
+
+	for (dev = 0; dev < 256; dev++) {
+		if (!pci_dev_exists(dev))
+			continue;
+
+		/* We are only interested in normal PCI devices */
+		header = pci_config_readb(dev, PCI_HEADER_TYPE);
+		if ((header & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_NORMAL)
+			continue;
+
+		cmd = PCI_COMMAND_SERR | PCI_COMMAND_PARITY;
+
+		for (i = 0; i < 6; i++) {
+			u64 addr;
+
+			if (pci_alloc_resource(dev, i, &addr)) {
+				pci_bar_set_addr(dev, i, addr);
+
+				if (pci_bar_is_memory(dev, i))
+					cmd |= PCI_COMMAND_MEMORY;
+				else
+					cmd |= PCI_COMMAND_IO;
+			}
+
+			if (pci_bar_is64(dev, i))
+				i++;
+		}
+
+		pci_config_writew(dev, PCI_COMMAND, cmd);
+	}
+
+	return true;
+}
+
+/*
+ * This function is to be called from pci_translate_addr() to provide
+ * mapping between this host bridge's PCI busses address and CPU physical
+ * address.
+ */
+phys_addr_t pci_host_bridge_get_paddr(u64 pci_addr)
+{
+	struct pci_host_bridge *host = pci_host_bridge;
+	struct pci_addr_space *as = &host->addr_space[0];
+	int i;
+
+	for (i = 0; i < host->nr_addr_spaces; i++) {
+		if (pci_addr >= as->pci_start &&
+		    pci_addr < as->pci_start + as->size)
+			return as->start + (pci_addr - as->pci_start);
+		as++;
+	}
+
+	return 0;
+}
+
+static void __iomem *pci_get_dev_conf(struct pci_host_bridge *host, int devfn)
+{
+	return (void __iomem *)(unsigned long)
+		host->start + (devfn << PCI_ECAM_DEVFN_SHIFT);
+}
+
+u8 pci_config_readb(pcidevaddr_t dev, u8 off)
+{
+	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
+	return readb(conf + off);
+}
+
+u16 pci_config_readw(pcidevaddr_t dev, u8 off)
+{
+	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
+	return readw(conf + off);
+}
+
+u32 pci_config_readl(pcidevaddr_t dev, u8 off)
+{
+	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
+	return readl(conf + off);
+}
+
+void pci_config_writew(pcidevaddr_t dev, u8 off, u32 val)
+{
+	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
+	writew(val, conf + off);
+}
+
+void pci_config_writel(pcidevaddr_t dev, u8 off, u32 val)
+{
+	void __iomem *conf = pci_get_dev_conf(pci_host_bridge, dev);
+	writel(val, conf + off);
+}
diff --git a/lib/pci-host-generic.h b/lib/pci-host-generic.h
new file mode 100644
index 000000000000..fd30e7c74ed8
--- /dev/null
+++ b/lib/pci-host-generic.h
@@ -0,0 +1,46 @@
+#ifndef PCI_HOST_GENERIC_H
+#define PCI_HOST_GENERIC_H
+/*
+ * PCI host bridge supporting structures and constants
+ *
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include "libcflat.h"
+
+struct pci_addr_space {
+	phys_addr_t		pci_start;
+	phys_addr_t		start;
+	phys_addr_t		size;
+	phys_addr_t		allocated;
+	int			type;
+};
+
+struct pci_host_bridge {
+	phys_addr_t		start;
+	phys_addr_t		size;
+	int			bus;
+	int			bus_max;
+	int			nr_addr_spaces;
+	struct pci_addr_space	addr_space[];
+};
+
+/*
+ * The following constants are derived from Linux, see this source:
+ *
+ *         drivers/pci/host/pci-host-generic.c
+ *                 struct gen_pci_cfg_bus_ops::bus_shift
+ *                 int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
+ *
+ * Documentation/devicetree/bindings/pci/host-generic-pci.txt describes
+ * ECAM Configuration Space is be memory-mapped by concatenating the various
+ * components to form an offset:
+ *
+ *	cfg_offset(bus, device, function, register) =
+ *		   bus << 20 | device << 15 | function << 12 | register
+ */
+#define PCI_ECAM_BUS_SHIFT	20
+#define PCI_ECAM_DEVFN_SHIFT	12
+
+#endif
diff --git a/lib/pci.c b/lib/pci.c
index e03c67c1a707..6bd54cbac1bb 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -27,13 +27,13 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
 	return PCIDEVADDR_INVALID;
 }
 
-static uint32_t pci_bar_mask(uint32_t bar)
+uint32_t pci_bar_mask(uint32_t bar)
 {
 	return (bar & PCI_BASE_ADDRESS_SPACE_IO) ?
 		PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
 }
 
-static uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num)
+uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num)
 {
 	return pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
 }
@@ -127,7 +127,7 @@ bool pci_bar_is64(pcidevaddr_t dev, int bar_num)
 		      PCI_BASE_ADDRESS_MEM_TYPE_64;
 }
 
-static void pci_bar_print(pcidevaddr_t dev, int bar_num)
+void pci_bar_print(pcidevaddr_t dev, int bar_num)
 {
 	phys_addr_t size, start, end;
 	uint32_t bar;
@@ -173,7 +173,7 @@ static void pci_bar_print(pcidevaddr_t dev, int bar_num)
 	printf("]");
 }
 
-static void pci_dev_print_id(pcidevaddr_t dev)
+void pci_dev_print_id(pcidevaddr_t dev)
 {
 	printf("00.%02x.%1x %04x:%04x", dev / 8, dev % 8,
 		pci_config_readw(dev, PCI_VENDOR_ID),
diff --git a/lib/pci.h b/lib/pci.h
index fc0940adc299..9d960556c526 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -15,6 +15,7 @@ enum {
 	PCIDEVADDR_INVALID = 0xffff,
 };
 
+extern bool pci_probe(void);
 extern void pci_print(void);
 extern bool pci_dev_exists(pcidevaddr_t dev);
 extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
@@ -34,9 +35,13 @@ extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
 extern phys_addr_t pci_bar_get_addr(pcidevaddr_t dev, int bar_num);
 extern void pci_bar_set_addr(pcidevaddr_t dev, int bar_num, phys_addr_t addr);
 extern phys_addr_t pci_bar_size(pcidevaddr_t dev, int bar_num);
+extern uint32_t pci_bar_get(pcidevaddr_t dev, int bar_num);
+extern uint32_t pci_bar_mask(uint32_t bar);
 extern bool pci_bar_is64(pcidevaddr_t dev, int bar_num);
 extern bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num);
 extern bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num);
+extern void pci_bar_print(pcidevaddr_t dev, int bar_num);
+extern void pci_dev_print_id(pcidevaddr_t dev);
 
 /*
  * pci-testdev is a driver for the pci-testdev qemu pci device. The
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH v10 10/12] pci: Add generic ECAM host support

[Andrew Jones]

On Mon, Nov 07, 2016 at 08:56:39AM +0100, Alexander Gordeev wrote:
> +
> +		cmd = PCI_COMMAND_SERR | PCI_COMMAND_PARITY;
> +

I don't think the parity error response bit is necessary for
virtual (qemu emulated) devices. I'd also rather minimize
the amount of setup we do for devices here, leaving most of
the setup to the unit tests that want to drive the devices.
IOW, I wouldn't bother with the parity thing, and, IMO, if
we can't allocate resources here, for whatever reason, then
we shouldn't touch the device at all; we should just leave
it to the unit tests.

Since we're just down to a couple of tweaks that should apply
to your v9, then I'll just do them and send Paolo a pull
request.

Thanks for all your work on this!

Thanks,
drew

Re: [kvm-unit-tests PATCH v10 10/12] pci: Add generic ECAM host support

[Andrew Jones]

On Mon, Nov 07, 2016 at 09:52:35AM +0100, Andrew Jones wrote:
> On Mon, Nov 07, 2016 at 08:56:39AM +0100, Alexander Gordeev wrote:
> > +
> > +		cmd = PCI_COMMAND_SERR | PCI_COMMAND_PARITY;
> > +
> 
> I don't think the parity error response bit is necessary for
> virtual (qemu emulated) devices. I'd also rather minimize
> the amount of setup we do for devices here, leaving most of
> the setup to the unit tests that want to drive the devices.
> IOW, I wouldn't bother with the parity thing, and, IMO, if
> we can't allocate resources here, for whatever reason, then
> we shouldn't touch the device at all; we should just leave
> it to the unit tests.
>

OK, you've convinced me (off-list) that we might as well keep
this. It doesn't hurt and, anyway, rolling things back takes
even more code. Let's just leave it alone. You'll send v11
with the writeb/w updates though.

Thanks,
drew

[kvm-unit-tests PATCH v9 11/12] pci: Add pci-testdev PCI bus test device

[Alexander Gordeev]

Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/pci-testdev.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/pci.h         |   7 ++
 2 files changed, 199 insertions(+)
 create mode 100644 lib/pci-testdev.c

diff --git a/lib/pci-testdev.c b/lib/pci-testdev.c
new file mode 100644
index 000000000000..ad482d3291c7
--- /dev/null
+++ b/lib/pci-testdev.c
@@ -0,0 +1,192 @@
+/*
+ * QEMU "pci-testdev" PCI test device
+ *
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include "pci.h"
+#include "asm/io.h"
+
+struct pci_testdev_ops {
+	u8 (*io_readb)(const volatile void *addr);
+	u16 (*io_readw)(const volatile void *addr);
+	u32 (*io_readl)(const volatile void *addr);
+	void (*io_writeb)(u8 value, volatile void *addr);
+	void (*io_writew)(u16 value, volatile void *addr);
+	void (*io_writel)(u32 value, volatile void *addr);
+};
+
+static u8 pio_readb(const volatile void *addr)
+{
+	return inb((unsigned long)addr);
+}
+
+static u16 pio_readw(const volatile void *addr)
+{
+	return inw((unsigned long)addr);
+}
+
+static u32 pio_readl(const volatile void *addr)
+{
+	return inl((unsigned long)addr);
+}
+
+static void pio_writeb(u8 value, volatile void *addr)
+{
+	outb(value, (unsigned long)addr);
+}
+
+static void pio_writew(u16 value, volatile void *addr)
+{
+	outw(value, (unsigned long)addr);
+}
+
+static void pio_writel(u32 value, volatile void *addr)
+{
+	outl(value, (unsigned long)addr);
+}
+
+static struct pci_testdev_ops pci_testdev_io_ops = {
+	.io_readb	= pio_readb,
+	.io_readw	= pio_readw,
+	.io_readl	= pio_readl,
+	.io_writeb	= pio_writeb,
+	.io_writew	= pio_writew,
+	.io_writel	= pio_writel
+};
+
+static u8 mmio_readb(const volatile void *addr)
+{
+	return *(const volatile u8 __force *)addr;
+}
+
+static u16 mmio_readw(const volatile void *addr)
+{
+	return *(const volatile u16 __force *)addr;
+}
+
+static u32 mmio_readl(const volatile void *addr)
+{
+	return *(const volatile u32 __force *)addr;
+}
+
+static void mmio_writeb(u8 value, volatile void *addr)
+{
+	*(volatile u8 __force *)addr = value;
+}
+
+static void mmio_writew(u16 value, volatile void *addr)
+{
+	*(volatile u16 __force *)addr = value;
+}
+
+static void mmio_writel(u32 value, volatile void *addr)
+{
+	*(volatile u32 __force *)addr = value;
+}
+
+static struct pci_testdev_ops pci_testdev_mem_ops = {
+	.io_readb	= mmio_readb,
+	.io_readw	= mmio_readw,
+	.io_readl	= mmio_readl,
+	.io_writeb	= mmio_writeb,
+	.io_writew	= mmio_writew,
+	.io_writel	= mmio_writel
+};
+
+static bool pci_testdev_one(struct pci_test_dev_hdr *test,
+			    int test_nr,
+			    struct pci_testdev_ops *ops)
+{
+	u8 width;
+	u32 count, sig, off;
+	const int nr_writes = 16;
+	int i;
+
+	ops->io_writeb(test_nr, &test->test);
+	count = ops->io_readl(&test->count);
+	if (count != 0)
+		return false;
+
+	width = ops->io_readb(&test->width);
+	if (width != 1 && width != 2 && width != 4)
+		return false;
+
+	sig = ops->io_readl(&test->data);
+	off = ops->io_readl(&test->offset);
+
+	for (i = 0; i < nr_writes; i++) {
+		switch (width) {
+		case 1: ops->io_writeb(sig, (void *)test + off); break;
+		case 2: ops->io_writew(sig, (void *)test + off); break;
+		case 4: ops->io_writel(sig, (void *)test + off); break;
+		}
+	}
+
+	count = ops->io_readl(&test->count);
+	if (!count)
+		return true;
+
+	return (int)count == nr_writes;
+}
+
+void pci_testdev_print(struct pci_test_dev_hdr *test,
+		       struct pci_testdev_ops *ops)
+{
+	bool io = (ops == &pci_testdev_io_ops);
+	int i;
+
+	printf("pci-testdev %3s: ", io ? "io" : "mem");
+	for (i = 0;; ++i) {
+		char c = ops->io_readb(&test->name[i]);
+		if (!c)
+			break;
+		printf("%c", c);
+	}
+	printf("\n");
+}
+
+static int pci_testdev_all(struct pci_test_dev_hdr *test,
+			   struct pci_testdev_ops *ops)
+{
+	int i;
+
+	for (i = 0;; i++) {
+		if (!pci_testdev_one(test, i, ops))
+			break;
+		pci_testdev_print(test, ops);
+	}
+
+	return i;
+}
+
+int pci_testdev(void)
+{
+	phys_addr_t addr;
+	void __iomem *mem, *io;
+	pcidevaddr_t dev;
+	int nr_tests = 0;
+	bool ret;
+
+	dev = pci_find_dev(PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_TEST);
+	if (dev == PCIDEVADDR_INVALID) {
+		printf("'pci-testdev' device is not found, "
+		       "check QEMU '-device pci-testdev' parameter\n");
+		return -1;
+	}
+
+	ret = pci_bar_is_valid(dev, 0) && pci_bar_is_valid(dev, 1);
+	assert(ret);
+
+	addr = pci_bar_get_addr(dev, 0);
+	mem = ioremap(addr, PAGE_SIZE);
+
+	addr = pci_bar_get_addr(dev, 1);
+	io = (void *)(unsigned long)addr;
+
+	nr_tests += pci_testdev_all(mem, &pci_testdev_mem_ops);
+	nr_tests += pci_testdev_all(io, &pci_testdev_io_ops);
+
+	return nr_tests;
+}
diff --git a/lib/pci.h b/lib/pci.h
index 9d960556c526..30f538110610 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -43,6 +43,8 @@ extern bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num);
 extern void pci_bar_print(pcidevaddr_t dev, int bar_num);
 extern void pci_dev_print_id(pcidevaddr_t dev);
 
+int pci_testdev(void);
+
 /*
  * pci-testdev is a driver for the pci-testdev qemu pci device. The
  * device enables testing mmio and portio exits, and measuring their
@@ -51,7 +53,12 @@ extern void pci_dev_print_id(pcidevaddr_t dev);
 #define PCI_VENDOR_ID_REDHAT		0x1b36
 #define PCI_DEVICE_ID_REDHAT_TEST	0x0005
 
+/*
+ * pci-testdev supports at least three types of tests (via mmio and
+ * portio BARs): no-eventfd, wildcard-eventfd and datamatch-eventfd
+ */
 #define PCI_TESTDEV_NUM_BARS		2
+#define PCI_TESTDEV_NUM_TESTS		3
 
 struct pci_test_dev_hdr {
 	uint8_t  test;
-- 
1.8.3.1

[kvm-unit-tests PATCH v9 12/12] arm/arm64: pci: Add pci-testdev PCI device operation test

[Alexander Gordeev]

Suggested-by: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 arm/Makefile.common |  6 +++++-
 arm/pci-test.c      | 27 +++++++++++++++++++++++++++
 arm/run             |  7 ++++++-
 arm/unittests.cfg   |  4 ++++
 lib/arm/asm/pci.h   |  1 +
 lib/arm64/asm/pci.h |  1 +
 6 files changed, 44 insertions(+), 2 deletions(-)
 create mode 100644 arm/pci-test.c
 create mode 100644 lib/arm/asm/pci.h
 create mode 100644 lib/arm64/asm/pci.h

diff --git a/arm/Makefile.common b/arm/Makefile.common
index ccb554d9251a..f37b5c2a3de4 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -11,7 +11,8 @@ endif
 
 tests-common = \
 	$(TEST_DIR)/selftest.flat \
-	$(TEST_DIR)/spinlock-test.flat
+	$(TEST_DIR)/spinlock-test.flat \
+	$(TEST_DIR)/pci-test.flat
 
 all: test_cases
 
@@ -33,6 +34,9 @@ include scripts/asm-offsets.mak
 cflatobjs += lib/util.o
 cflatobjs += lib/alloc.o
 cflatobjs += lib/devicetree.o
+cflatobjs += lib/pci.o
+cflatobjs += lib/pci-host-generic.o
+cflatobjs += lib/pci-testdev.o
 cflatobjs += lib/virtio.o
 cflatobjs += lib/virtio-mmio.o
 cflatobjs += lib/chr-testdev.o
diff --git a/arm/pci-test.c b/arm/pci-test.c
new file mode 100644
index 000000000000..10a367de5357
--- /dev/null
+++ b/arm/pci-test.c
@@ -0,0 +1,27 @@
+/*
+ * PCI bus operation test
+ *
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include <libcflat.h>
+#include <pci.h>
+
+#define NR_TESTS (PCI_TESTDEV_NUM_BARS * PCI_TESTDEV_NUM_TESTS)
+
+int main(void)
+{
+	int ret;
+
+	if (!pci_probe())
+		report_abort("PCI bus probing failed\n");
+
+	pci_print();
+
+	ret = pci_testdev();
+	report("PCI test device passed %d/%d tests",
+		ret >= NR_TESTS, ret > 0 ? ret : 0, NR_TESTS);
+
+	return report_summary();
+}
diff --git a/arm/run b/arm/run
index a2f35ef6a7e6..1ee6231599d6 100755
--- a/arm/run
+++ b/arm/run
@@ -67,8 +67,13 @@ fi
 chr_testdev='-device virtio-serial-device'
 chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
 
+pci_testdev=
+if $qemu $M -device '?' 2>&1 | grep pci-testdev > /dev/null; then
+	pci_testdev="-device pci-testdev"
+fi
+
 M+=",accel=$ACCEL"
-command="$qemu $M -cpu $processor $chr_testdev"
+command="$qemu $M -cpu $processor $chr_testdev $pci_testdev"
 command+=" -display none -serial stdio -kernel"
 command="$(timeout_cmd) $command"
 echo $command "$@"
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index 3f6fa45c587e..ae32a42a91c3 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -54,3 +54,7 @@ file = selftest.flat
 smp = $MAX_SMP
 extra_params = -append 'smp'
 groups = selftest
+
+[pci-test]
+file = pci-test.flat
+groups = pci
diff --git a/lib/arm/asm/pci.h b/lib/arm/asm/pci.h
new file mode 100644
index 000000000000..50d0fb64c5ca
--- /dev/null
+++ b/lib/arm/asm/pci.h
@@ -0,0 +1 @@
+#include <asm-generic/pci-host-bridge.h>
diff --git a/lib/arm64/asm/pci.h b/lib/arm64/asm/pci.h
new file mode 100644
index 000000000000..f70ef560e2ab
--- /dev/null
+++ b/lib/arm64/asm/pci.h
@@ -0,0 +1 @@
+#include "../../arm/asm/pci.h"
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH v9 00/12] PCI bus support

[Andrew Jones]

[-- Attachment #1: Type: text/plain, Size: 2322 bytes --]

On Thu, Nov 03, 2016 at 05:19:37PM +0100, Alexander Gordeev wrote:
> Hi Andrew et al,
> 
> This is 9th version of PCI support. Your latest suggestions are incorporated.
> 
> Sources are avalable at:
> https://github.com/a-gordeev/kvm-unit-tests.git pci-testdev-v9

I tested this branch on tcg/arm, tcg/aarch64, kvm/aarch64 and, with the
attached patch, I was able to cp arm/pci-test.c to x86 and even test it
with kvm/i386 and kvm/x86_64. It worked great.

Tested-by: Andrew Jones <drjones@redhat.com>

Thanks for this series!!


> 
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Andrew Jones <drjones@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> 
> 
> Alexander Gordeev (12):
>   pci: Fix coding style in generic PCI files
>   pci: x86: Rename pci_config_read() to pci_config_readl()
>   pci: Add 'extern' to public function declarations
>   pci: x86: Add remaining PCI configuration space accessors
>   pci: Factor out pci_bar_get()
>   pci: Rework pci_bar_addr()
>   pci: Add pci_bar_set_addr()
>   pci: Add pci_dev_exists()
>   pci: Add pci_print()
>   pci: Add generic ECAM host support
>   pci: Add pci-testdev PCI bus test device
>   arm/arm64: pci: Add pci-testdev PCI device operation test
> 
>  arm/Makefile.common               |   6 +-
>  arm/pci-test.c                    |  27 ++++
>  arm/run                           |   7 +-
>  arm/unittests.cfg                 |   4 +
>  lib/arm/asm/pci.h                 |   1 +
>  lib/arm64/asm/pci.h               |   1 +
>  lib/asm-generic/pci-host-bridge.h |  26 ++++
>  lib/pci-host-generic.c            | 305 ++++++++++++++++++++++++++++++++++++++
>  lib/pci-host-generic.h            |  46 ++++++
>  lib/pci-testdev.c                 | 192 ++++++++++++++++++++++++
>  lib/pci.c                         | 212 +++++++++++++++++++++++---
>  lib/pci.h                         |  43 +++++-
>  lib/x86/asm/pci.h                 |  31 +++-
>  x86/vmexit.c                      |   4 +-
>  14 files changed, 874 insertions(+), 31 deletions(-)
>  create mode 100644 arm/pci-test.c
>  create mode 100644 lib/arm/asm/pci.h
>  create mode 100644 lib/arm64/asm/pci.h
>  create mode 100644 lib/asm-generic/pci-host-bridge.h
>  create mode 100644 lib/pci-host-generic.c
>  create mode 100644 lib/pci-host-generic.h
>  create mode 100644 lib/pci-testdev.c
> 
> -- 
> 1.8.3.1
> 

[-- Attachment #2: x86-enable-pci-testdev.diff --]
[-- Type: text/plain, Size: 1144 bytes --]

diff --git a/lib/x86/asm/io.h b/lib/x86/asm/io.h
index 35a5c7347411..6f3ba88c5662 100644
--- a/lib/x86/asm/io.h
+++ b/lib/x86/asm/io.h
@@ -2,6 +2,7 @@
 #define _ASM_X86_IO_H_
 
 #define __iomem
+#define __force
 
 #define inb inb
 static inline uint8_t inb(unsigned long port)
diff --git a/lib/x86/io.c b/lib/x86/io.c
index cc5ac5855366..54f59e24bca3 100644
--- a/lib/x86/io.c
+++ b/lib/x86/io.c
@@ -93,7 +93,19 @@ void __iomem *ioremap(phys_addr_t phys_addr, size_t size)
 	 * The kernel sets PTEs for an ioremap() with page cache disabled,
 	 * but we do not do that right now. It would make sense that I/O
 	 * mappings would be uncached - and may help us find bugs when we
-	 * properly map that way.
+	 * properly map that way. One way to improve this is to just use
+	 * the identity mapping for any physical addresses in the MMIO
+	 * ranges 0-2G and 3G-4G. See lib/x86/vm.c:setup_mmu()
 	 */
+
+	if (phys_addr < (1 << 31) || ((3 << 30) <= phys_addr &&
+			phys_addr < ((phys_addr_t)1 << 32)))
+		return (void __iomem *)(unsigned long)phys_addr;
+
 	return vmap(phys_addr, size) + offset;
 }
+
+bool pci_probe(void)
+{
+	return true;
+}

Re: [kvm-unit-tests PATCH v9 00/12] PCI bus support

[Alexander Gordeev]

On Fri, Nov 04, 2016 at 04:44:07PM +0100, Andrew Jones wrote:
> On Thu, Nov 03, 2016 at 05:19:37PM +0100, Alexander Gordeev wrote:
> > This is 9th version of PCI support. Your latest suggestions are incorporated.
> > 
> > Sources are avalable at:
> > https://github.com/a-gordeev/kvm-unit-tests.git pci-testdev-v9

I put v10 with refreshed patch 10/12 at:
https://github.com/a-gordeev/kvm-unit-tests.git pci-testdev-v10

Thanks!

> I tested this branch on tcg/arm, tcg/aarch64, kvm/aarch64 and, with the
> attached patch, I was able to cp arm/pci-test.c to x86 and even test it
> with kvm/i386 and kvm/x86_64. It worked great.
> 
> Tested-by: Andrew Jones <drjones@redhat.com>
> 
> Thanks for this series!!