diff mbox

[kvm-unit-tests,15/17] pci: add msi support for 32/64bit address

Message ID 1477468040-21034-16-git-send-email-peterx@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Xu Oct. 26, 2016, 7:47 a.m. UTC 
pci_cap_walk() is provided to allow walk through all the capability bits
for a specific PCI device. If a cap handler is provided, it'll be
triggered if the cap is detected along the cap walk.

MSI cap handler is the first one supported. We can add more cap handler
in the future.

Meanwhile, pci_setup_msi() API is provided to support basic 32/64 bit
address MSI setup.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 lib/pci.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/pci.h |  3 +++
 2 files changed, 71 insertions(+)

Comments

Andrew Jones Nov. 4, 2016, 5:33 p.m. UTC | #1 
On Wed, Oct 26, 2016 at 03:47:18PM +0800, Peter Xu wrote:
> pci_cap_walk() is provided to allow walk through all the capability bits
> for a specific PCI device. If a cap handler is provided, it'll be
> triggered if the cap is detected along the cap walk.
> 
> MSI cap handler is the first one supported. We can add more cap handler
> in the future.
> 
> Meanwhile, pci_setup_msi() API is provided to support basic 32/64 bit
> address MSI setup.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  lib/pci.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/pci.h |  3 +++
>  2 files changed, 71 insertions(+)
> 
> diff --git a/lib/pci.c b/lib/pci.c
> index d78472f..8d9a455 100644
> --- a/lib/pci.c
> +++ b/lib/pci.c
> @@ -7,6 +7,69 @@
>  #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->pci_bdf, cap_offset);
> +	dev->msi_offset = cap_offset;
> +}
> +
> +static pci_cap_handler cap_handlers[0xff] = {

Why 0xff? Shouldn't it be (PCI_CAP_ID_MAX + 1)?

> +	[PCI_CAP_ID_MSI] = pci_cap_msi_handler,
> +};
> +
> +void pci_cap_walk(struct pci_dev *dev)
> +{
> +	uint8_t cap_offset;
> +	uint8_t cap_id;
> +
> +	cap_offset = pci_config_readb(dev->pci_bdf, PCI_CAPABILITY_LIST);
> +	while (cap_offset) {
> +		cap_id = pci_config_readb(dev->pci_bdf, cap_offset);
> +		printf("PCI detected cap 0x%x\n", cap_id);
> +		if (cap_handlers[cap_id])
> +			cap_handlers[cap_id](dev, cap_offset);
> +		cap_offset = pci_config_readb(dev->pci_bdf, cap_offset + 1);
> +	}
> +}
> +
> +int pci_setup_msi(struct pci_dev *dev, uint64_t msi_addr, uint32_t msi_data)
> +{
> +	uint16_t msi_control;
> +	uint16_t offset;
> +	pcidevaddr_t addr = dev->pci_bdf;
> +
> +	assert(dev);
> +
> +	if (!dev->msi_offset) {
> +		printf("MSI: dev 0x%x does not support MSI.\n", addr);
> +		return -1;
> +	}
> +
> +	offset = dev->msi_offset;
> +	msi_control = pci_config_readw(addr, offset + PCI_MSI_FLAGS);
> +	pci_config_writel(addr, offset + PCI_MSI_ADDRESS_LO,
> +			  msi_addr & 0xffffffff);
> +
> +	if (msi_control & PCI_MSI_FLAGS_64BIT) {
> +		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%lx, data=0x%x\n", msi_addr, msi_data);
> +
> +	msi_control |= PCI_MSI_FLAGS_ENABLE;
> +	pci_config_writew(addr, offset + PCI_MSI_FLAGS, msi_control);
> +
> +	return 0;
> +}

Again we only have 0/-1 for good/bad. I'd prefer true/false.

> +
>  void pci_set_master(struct pci_dev *dev, int master)
>  {
>  	uint32_t val = pci_config_readw(dev->pci_bdf, PCI_COMMAND);
> @@ -20,6 +83,10 @@ bool pci_dev_exists(pcidevaddr_t dev)
>  		pci_config_readw(dev, PCI_DEVICE_ID) != 0xffff);
>  }
>  
> +/*
> + * Scan bus look for a specific device. Only bus 0 scanned for now.
> + * After the scan, a pci_dev is returned with correct BAR information.
> + */
>  void pci_dev_init(struct pci_dev *dev, pcidevaddr_t bdf)
>  {
>         memset(dev, 0, sizeof(*dev));
> @@ -60,6 +127,7 @@ int pci_enable_defaults(struct pci_dev *dev)
>  {
>  	pci_scan_bars(dev);
>  	pci_set_master(dev, 1);
> +	pci_cap_walk(dev);
>  	return 0;
>  }
>  
> diff --git a/lib/pci.h b/lib/pci.h
> index bc6b91a..a0331ac 100644
> --- a/lib/pci.h
> +++ b/lib/pci.h
> @@ -23,13 +23,16 @@ enum {
>  
>  struct pci_dev {
>  	uint16_t pci_bdf;
> +	uint16_t msi_offset;
>  	phys_addr_t pci_bar[PCI_BAR_NUM];
>  };
>  
>  void pci_dev_init(struct pci_dev *dev, pcidevaddr_t bdf);
>  void pci_scan_bars(struct pci_dev *dev);
>  void pci_set_master(struct pci_dev *dev, int master);
> +void pci_cap_walk(struct pci_dev *dev);
>  int pci_enable_defaults(struct pci_dev *dev);
> +int pci_setup_msi(struct pci_dev *dev, uint64_t msi_addr, uint32_t msi_data);
>  
>  typedef phys_addr_t iova_t;
>  
> -- 
> 2.7.4
>

Otherwise

Reviewed-by: Andrew Jones <drjones@redhat.com> 
--
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
Peter Xu Nov. 7, 2016, 5:58 p.m. UTC | #2 
On Fri, Nov 04, 2016 at 06:33:29PM +0100, Andrew Jones wrote:

[...]

> > +static pci_cap_handler cap_handlers[0xff] = {
> 
> Why 0xff? Shouldn't it be (PCI_CAP_ID_MAX + 1)?

Yes. Will fix.

[...]

> > +int pci_setup_msi(struct pci_dev *dev, uint64_t msi_addr, uint32_t msi_data)
> > +{
> > +	uint16_t msi_control;
> > +	uint16_t offset;
> > +	pcidevaddr_t addr = dev->pci_bdf;
> > +
> > +	assert(dev);
> > +
> > +	if (!dev->msi_offset) {
> > +		printf("MSI: dev 0x%x does not support MSI.\n", addr);
> > +		return -1;
> > +	}
> > +
> > +	offset = dev->msi_offset;
> > +	msi_control = pci_config_readw(addr, offset + PCI_MSI_FLAGS);
> > +	pci_config_writel(addr, offset + PCI_MSI_ADDRESS_LO,
> > +			  msi_addr & 0xffffffff);
> > +
> > +	if (msi_control & PCI_MSI_FLAGS_64BIT) {
> > +		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%lx, data=0x%x\n", msi_addr, msi_data);
> > +
> > +	msi_control |= PCI_MSI_FLAGS_ENABLE;
> > +	pci_config_writew(addr, offset + PCI_MSI_FLAGS, msi_control);
> > +
> > +	return 0;
> > +}
> 
> Again we only have 0/-1 for good/bad. I'd prefer true/false.

Will use bool as return code.

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

Thanks!

-- peterx
--
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
diff mbox

Patch

diff --git a/lib/pci.c b/lib/pci.c
index d78472f..8d9a455 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -7,6 +7,69 @@ 
 #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->pci_bdf, cap_offset);
+	dev->msi_offset = cap_offset;
+}
+
+static pci_cap_handler cap_handlers[0xff] = {
+	[PCI_CAP_ID_MSI] = pci_cap_msi_handler,
+};
+
+void pci_cap_walk(struct pci_dev *dev)
+{
+	uint8_t cap_offset;
+	uint8_t cap_id;
+
+	cap_offset = pci_config_readb(dev->pci_bdf, PCI_CAPABILITY_LIST);
+	while (cap_offset) {
+		cap_id = pci_config_readb(dev->pci_bdf, cap_offset);
+		printf("PCI detected cap 0x%x\n", cap_id);
+		if (cap_handlers[cap_id])
+			cap_handlers[cap_id](dev, cap_offset);
+		cap_offset = pci_config_readb(dev->pci_bdf, cap_offset + 1);
+	}
+}
+
+int pci_setup_msi(struct pci_dev *dev, uint64_t msi_addr, uint32_t msi_data)
+{
+	uint16_t msi_control;
+	uint16_t offset;
+	pcidevaddr_t addr = dev->pci_bdf;
+
+	assert(dev);
+
+	if (!dev->msi_offset) {
+		printf("MSI: dev 0x%x does not support MSI.\n", addr);
+		return -1;
+	}
+
+	offset = dev->msi_offset;
+	msi_control = pci_config_readw(addr, offset + PCI_MSI_FLAGS);
+	pci_config_writel(addr, offset + PCI_MSI_ADDRESS_LO,
+			  msi_addr & 0xffffffff);
+
+	if (msi_control & PCI_MSI_FLAGS_64BIT) {
+		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%lx, data=0x%x\n", msi_addr, msi_data);
+
+	msi_control |= PCI_MSI_FLAGS_ENABLE;
+	pci_config_writew(addr, offset + PCI_MSI_FLAGS, msi_control);
+
+	return 0;
+}
+
 void pci_set_master(struct pci_dev *dev, int master)
 {
 	uint32_t val = pci_config_readw(dev->pci_bdf, PCI_COMMAND);
@@ -20,6 +83,10 @@  bool pci_dev_exists(pcidevaddr_t dev)
 		pci_config_readw(dev, PCI_DEVICE_ID) != 0xffff);
 }
 
+/*
+ * Scan bus look for a specific device. Only bus 0 scanned for now.
+ * After the scan, a pci_dev is returned with correct BAR information.
+ */
 void pci_dev_init(struct pci_dev *dev, pcidevaddr_t bdf)
 {
        memset(dev, 0, sizeof(*dev));
@@ -60,6 +127,7 @@  int pci_enable_defaults(struct pci_dev *dev)
 {
 	pci_scan_bars(dev);
 	pci_set_master(dev, 1);
+	pci_cap_walk(dev);
 	return 0;
 }
 
diff --git a/lib/pci.h b/lib/pci.h
index bc6b91a..a0331ac 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -23,13 +23,16 @@  enum {
 
 struct pci_dev {
 	uint16_t pci_bdf;
+	uint16_t msi_offset;
 	phys_addr_t pci_bar[PCI_BAR_NUM];
 };
 
 void pci_dev_init(struct pci_dev *dev, pcidevaddr_t bdf);
 void pci_scan_bars(struct pci_dev *dev);
 void pci_set_master(struct pci_dev *dev, int master);
+void pci_cap_walk(struct pci_dev *dev);
 int pci_enable_defaults(struct pci_dev *dev);
+int pci_setup_msi(struct pci_dev *dev, uint64_t msi_addr, uint32_t msi_data);
 
 typedef phys_addr_t iova_t;