[PATCH kvm-unit-tests 0/5] x86/intel-iommu: couple fixups

[PATCH kvm-unit-tests 0/5] x86/intel-iommu: couple fixups

[Andrew Jones]

This series makes two fixups: 1) properly acks EDU device
interrupts. 2) moves printing from library code to unit test
code, as not all unit tests using EDU may want the same
verbosity level.

Andrew Jones (5):
  x86/intel-iommu: make edu_dev global
  x86/intel-iommu: properly ack edu interrupt
  lib/pci: generalize pci_cap_walk
  lib/pci: expose pci_dev_print
  lib/pci: reduce library noise

 lib/pci-edu.h         |  2 ++
 lib/pci.h             |  4 +++-
 lib/pci.c             | 52 +++++++++++++++++++++++++++++----------------------
 lib/x86/intel-iommu.c |  3 +++
 x86/intel-iommu.c     | 23 +++++++++++++----------
 5 files changed, 51 insertions(+), 33 deletions(-)

-- 
2.9.3

[PATCH kvm-unit-tests 1/5] x86/intel-iommu: make edu_dev global

[Andrew Jones]

We'll need to access the edu_dev struct from the ISR, so make it
global.

Cc: Peter Xu <peterx@redhat.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 x86/intel-iommu.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/x86/intel-iommu.c b/x86/intel-iommu.c
index 5a65232f5ad4..fa77914524b5 100644
--- a/x86/intel-iommu.c
+++ b/x86/intel-iommu.c
@@ -18,8 +18,11 @@
 #define VTD_TEST_IR_MSI ("IR MSI")
 #define VTD_TEST_IR_IOAPIC ("IR IOAPIC")
 
-void vtd_test_dmar(struct pci_edu_dev *dev)
+static struct pci_edu_dev edu_dev;
+
+static void vtd_test_dmar(void)
 {
+	struct pci_edu_dev *dev = &edu_dev;
 	void *page = alloc_page();
 
 	report_prefix_push("vtd_dmar");
@@ -66,10 +69,11 @@ static void edu_isr(isr_regs_t *regs)
 	eoi();
 }
 
-static void vtd_test_ir(struct pci_edu_dev *dev)
+static void vtd_test_ir(void)
 {
 #define VTD_TEST_VECTOR_IOAPIC (0xed)
 #define VTD_TEST_VECTOR_MSI (0xee)
+	struct pci_edu_dev *dev = &edu_dev;
 	struct pci_dev *pci_dev = &dev->pci_dev;
 
 	report_prefix_push("vtd_ir");
@@ -127,8 +131,6 @@ static void vtd_test_ir(struct pci_edu_dev *dev)
 
 int main(int argc, char *argv[])
 {
-	struct pci_edu_dev dev;
-
 	vtd_init();
 
 	report_prefix_push("vtd_init");
@@ -145,15 +147,15 @@ int main(int argc, char *argv[])
 
 	report_prefix_pop();
 
-	if (!edu_init(&dev)) {
+	if (!edu_init(&edu_dev)) {
 		printf("Please specify \"-device edu\" to do "
 		       "further IOMMU tests.\n");
 		report_skip(VTD_TEST_DMAR_4B);
 		report_skip(VTD_TEST_IR_IOAPIC);
 		report_skip(VTD_TEST_IR_MSI);
 	} else {
-		vtd_test_dmar(&dev);
-		vtd_test_ir(&dev);
+		vtd_test_dmar();
+		vtd_test_ir();
 	}
 
 	return report_summary();
-- 
2.9.3

[PATCH kvm-unit-tests 2/5] x86/intel-iommu: properly ack edu interrupt

[Andrew Jones]

Writing zero to EDU_REG_INTR_RAISE doesn't clear the status, because
the value is ORed in. The edu spec says that to clear the interrupt,
regardless of using MSI or not, one should write the value of the
interrupt status register to the interrupt acknowledge register from
the interrupt handler.

Cc: Peter Xu <peterx@redhat.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/pci-edu.h     | 2 ++
 x86/intel-iommu.c | 5 ++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/pci-edu.h b/lib/pci-edu.h
index 9fe32c640eee..44b4ba168768 100644
--- a/lib/pci-edu.h
+++ b/lib/pci-edu.h
@@ -32,7 +32,9 @@
 #define EDU_REG_ALIVE               0x4
 #define EDU_REG_FACTORIAL           0x8
 #define EDU_REG_STATUS              0x20
+#define EDU_REG_INTR_STATUS         0x24
 #define EDU_REG_INTR_RAISE          0x60
+#define EDU_REG_INTR_ACK            0x64
 #define EDU_REG_DMA_SRC             0x80
 #define EDU_REG_DMA_DST             0x88
 #define EDU_REG_DMA_COUNT           0x90
diff --git a/x86/intel-iommu.c b/x86/intel-iommu.c
index fa77914524b5..d1fa420aab9e 100644
--- a/x86/intel-iommu.c
+++ b/x86/intel-iommu.c
@@ -67,6 +67,8 @@ static void edu_isr(isr_regs_t *regs)
 {
 	edu_intr_recved = true;
 	eoi();
+	edu_reg_writel(&edu_dev, EDU_REG_INTR_ACK,
+			edu_reg_readl(&edu_dev, EDU_REG_INTR_STATUS));
 }
 
 static void vtd_test_ir(void)
@@ -120,9 +122,6 @@ static void vtd_test_ir(void)
 	while (!edu_intr_recved)
 		cpu_relax();
 
-	/* Clear INTR bits */
-	edu_reg_writel(dev, EDU_REG_INTR_RAISE, 0);
-
 	/* We are good as long as we reach here */
 	report(VTD_TEST_IR_MSI, edu_intr_recved == true);
 
-- 
2.9.3

[PATCH kvm-unit-tests 3/5] lib/pci: generalize pci_cap_walk

[Andrew Jones]

Increase the utility of pci_cap_walk by allowing the caller to supply
the table of cap handlers via a single handler, which implements all
the cases of interest.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/pci.h |  3 ++-
 lib/pci.c | 31 ++++++++++++++-----------------
 2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/lib/pci.h b/lib/pci.h
index 08157544296f..703c1fb689bf 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -30,7 +30,8 @@ struct pci_dev {
 extern void pci_dev_init(struct pci_dev *dev, pcidevaddr_t bdf);
 extern void pci_scan_bars(struct pci_dev *dev);
 extern void pci_cmd_set_clr(struct pci_dev *dev, uint16_t set, uint16_t clr);
-extern void pci_cap_walk(struct pci_dev *dev);
+typedef void (*pci_cap_handler_t)(struct pci_dev *dev, int cap_offset, int cap_id);
+extern void pci_cap_walk(struct pci_dev *dev, pci_cap_handler_t handler);
 extern void pci_enable_defaults(struct pci_dev *dev);
 extern bool pci_setup_msi(struct pci_dev *dev, uint64_t msi_addr,
 			  uint32_t msi_data);
diff --git a/lib/pci.c b/lib/pci.c
index 6ef07df0fd97..99dd830e82ec 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -7,20 +7,7 @@
 #include "pci.h"
 #include "asm/pci.h"
 
-typedef void (*pci_cap_handler)(struct pci_dev *dev, int cap_offset);
-
-static void pci_cap_msi_handler(struct pci_dev *dev, int cap_offset)
-{
-	printf("Detected MSI for device 0x%x offset 0x%x\n",
-	       dev->bdf, cap_offset);
-	dev->msi_offset = cap_offset;
-}
-
-static pci_cap_handler cap_handlers[PCI_CAP_ID_MAX + 1] = {
-	[PCI_CAP_ID_MSI] = pci_cap_msi_handler,
-};
-
-void pci_cap_walk(struct pci_dev *dev)
+void pci_cap_walk(struct pci_dev *dev, pci_cap_handler_t handler)
 {
 	uint8_t cap_offset;
 	uint8_t cap_id;
@@ -31,8 +18,7 @@ void pci_cap_walk(struct pci_dev *dev)
 		cap_id = pci_config_readb(dev->bdf, cap_offset);
 		printf("PCI detected cap 0x%x\n", cap_id);
 		assert(cap_id < PCI_CAP_ID_MAX + 1);
-		if (cap_handlers[cap_id])
-			cap_handlers[cap_id](dev, cap_offset);
+		handler(dev, cap_offset, cap_id);
 		cap_offset = pci_config_readb(dev->bdf, cap_offset + 1);
 		/* Avoid dead loop during cap walk */
 		assert(++count <= 255);
@@ -347,10 +333,21 @@ uint8_t pci_intx_line(struct pci_dev *dev)
 	return pci_config_readb(dev->bdf, PCI_INTERRUPT_LINE);
 }
 
+static void pci_cap_setup(struct pci_dev *dev, int cap_offset, int cap_id)
+{
+	switch (cap_id) {
+	case PCI_CAP_ID_MSI:
+		printf("Detected MSI for device 0x%x offset 0x%x\n",
+			dev->bdf, cap_offset);
+		dev->msi_offset = cap_offset;
+		break;
+	}
+}
+
 void pci_enable_defaults(struct pci_dev *dev)
 {
 	pci_scan_bars(dev);
 	/* Enable device DMA operations */
 	pci_cmd_set_clr(dev, PCI_COMMAND_MASTER, 0);
-	pci_cap_walk(dev);
+	pci_cap_walk(dev, pci_cap_setup);
 }
-- 
2.9.3

[PATCH kvm-unit-tests 4/5] lib/pci: expose pci_dev_print

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/pci.h | 1 +
 lib/pci.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/pci.h b/lib/pci.h
index 703c1fb689bf..3da3ccc8c791 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -65,6 +65,7 @@ extern bool pci_bar_is_memory(struct pci_dev *dev, int bar_num);
 extern bool pci_bar_is_valid(struct pci_dev *dev, int bar_num);
 extern void pci_bar_print(struct pci_dev *dev, int bar_num);
 extern void pci_dev_print_id(pcidevaddr_t dev);
+extern void pci_dev_print(pcidevaddr_t dev);
 extern uint8_t pci_intx_line(struct pci_dev *dev);
 void pci_msi_set_enable(struct pci_dev *dev, bool enabled);
 
diff --git a/lib/pci.c b/lib/pci.c
index 99dd830e82ec..8ceb8ef91c75 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -274,7 +274,7 @@ void pci_dev_print_id(pcidevaddr_t dev)
 		pci_config_readw(dev, PCI_DEVICE_ID));
 }
 
-static void pci_dev_print(pcidevaddr_t dev)
+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);
-- 
2.9.3

[PATCH kvm-unit-tests 5/5] lib/pci: reduce library noise

[Andrew Jones]

Avoid printfs in library code unless in a printing function, e.g.
pci_dev_print. Particularly we can avoid printing anything the
caller can easily print themselves, if so desired.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/pci.c             | 23 +++++++++++++++++------
 lib/x86/intel-iommu.c |  3 +++
 x86/intel-iommu.c     |  2 ++
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/lib/pci.c b/lib/pci.c
index 8ceb8ef91c75..62b1c6b0b7c5 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -16,7 +16,6 @@ void pci_cap_walk(struct pci_dev *dev, pci_cap_handler_t handler)
 	cap_offset = pci_config_readb(dev->bdf, PCI_CAPABILITY_LIST);
 	while (cap_offset) {
 		cap_id = pci_config_readb(dev->bdf, cap_offset);
-		printf("PCI detected cap 0x%x\n", cap_id);
 		assert(cap_id < PCI_CAP_ID_MAX + 1);
 		handler(dev, cap_offset, cap_id);
 		cap_offset = pci_config_readb(dev->bdf, cap_offset + 1);
@@ -64,12 +63,9 @@ bool pci_setup_msi(struct pci_dev *dev, uint64_t msi_addr, uint32_t msi_data)
 		pci_config_writel(addr, offset + PCI_MSI_ADDRESS_HI,
 				  (uint32_t)(msi_addr >> 32));
 		pci_config_writel(addr, offset + PCI_MSI_DATA_64, msi_data);
-		printf("MSI: dev 0x%x init 64bit address: ", addr);
 	} else {
 		pci_config_writel(addr, offset + PCI_MSI_DATA_32, msi_data);
-		printf("MSI: dev 0x%x init 32bit address: ", addr);
 	}
-	printf("addr=0x%" PRIx64 ", data=0x%x\n", msi_addr, msi_data);
 
 	pci_msi_set_enable(dev, true);
 
@@ -274,6 +270,21 @@ void pci_dev_print_id(pcidevaddr_t dev)
 		pci_config_readw(dev, PCI_DEVICE_ID));
 }
 
+static void pci_cap_print(struct pci_dev *dev, int cap_offset, int cap_id)
+{
+	switch (cap_id) {
+	case PCI_CAP_ID_MSI: {
+		uint16_t control = pci_config_readw(dev->bdf, cap_offset + PCI_MSI_FLAGS);
+		printf("\tMSI,%s-bit capability ", control & PCI_MSI_FLAGS_64BIT ? "64" : "32");
+		break;
+	}
+	default:
+		printf("\tcapability 0x%02x ", cap_id);
+		break;
+	}
+	printf("at offset 0x%02x\n", cap_offset);
+}
+
 void pci_dev_print(pcidevaddr_t dev)
 {
 	uint8_t header = pci_config_readb(dev, PCI_HEADER_TYPE);
@@ -289,6 +300,8 @@ void pci_dev_print(pcidevaddr_t dev)
 	printf(" type %02x progif %02x class %02x subclass %02x\n",
 	       header, progif, class, subclass);
 
+	pci_cap_walk(&pci_dev, pci_cap_print);
+
 	if ((header & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_NORMAL)
 		return;
 
@@ -337,8 +350,6 @@ static void pci_cap_setup(struct pci_dev *dev, int cap_offset, int cap_id)
 {
 	switch (cap_id) {
 	case PCI_CAP_ID_MSI:
-		printf("Detected MSI for device 0x%x offset 0x%x\n",
-			dev->bdf, cap_offset);
 		dev->msi_offset = cap_offset;
 		break;
 	}
diff --git a/lib/x86/intel-iommu.c b/lib/x86/intel-iommu.c
index c35b758d1383..7cc5a702aed4 100644
--- a/lib/x86/intel-iommu.c
+++ b/lib/x86/intel-iommu.c
@@ -324,6 +324,9 @@ bool vtd_setup_msi(struct pci_dev *dev, int vector, int dest_id)
 	msi_addr.head = 0xfee;
 	msi_data.subhandle = 0;
 
+	printf("%s: msi_addr=0x%" PRIx64 ", msi_data=0x%x\n", __func__,
+		*(uint64_t *)&msi_addr, *(uint32_t *)&msi_data);
+
 	return pci_setup_msi(dev, *(uint64_t *)&msi_addr,
 			     *(uint32_t *)&msi_data);
 }
diff --git a/x86/intel-iommu.c b/x86/intel-iommu.c
index d1fa420aab9e..a01b23e674a8 100644
--- a/x86/intel-iommu.c
+++ b/x86/intel-iommu.c
@@ -153,6 +153,8 @@ int main(int argc, char *argv[])
 		report_skip(VTD_TEST_IR_IOAPIC);
 		report_skip(VTD_TEST_IR_MSI);
 	} else {
+		printf("Found EDU device:\n");
+		pci_dev_print(edu_dev.pci_dev.bdf);
 		vtd_test_dmar();
 		vtd_test_ir();
 	}
-- 
2.9.3

Re: [PATCH kvm-unit-tests 0/5] x86/intel-iommu: couple fixups

[Peter Xu]

On Mon, Jan 16, 2017 at 04:32:10PM +0100, Andrew Jones wrote:
> This series makes two fixups: 1) properly acks EDU device
> interrupts. 2) moves printing from library code to unit test
> code, as not all unit tests using EDU may want the same
> verbosity level.
> 
> Andrew Jones (5):
>   x86/intel-iommu: make edu_dev global
>   x86/intel-iommu: properly ack edu interrupt
>   lib/pci: generalize pci_cap_walk
>   lib/pci: expose pci_dev_print
>   lib/pci: reduce library noise

Thanks for fixing the edu issue. For the series:

Reviewed-by: Peter Xu <peterx@redhat.com>

Re: [PATCH kvm-unit-tests 0/5] x86/intel-iommu: couple fixups

[Radim Krčmář]

2017-01-16 16:32+0100, Andrew Jones:
> This series makes two fixups: 1) properly acks EDU device
> interrupts. 2) moves printing from library code to unit test
> code, as not all unit tests using EDU may want the same
> verbosity level.

Applied, thanks.