diff mbox

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

Message ID 1476448852-30062-14-git-send-email-peterx@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Xu Oct. 14, 2016, 12:40 p.m. UTC 
During each PCI device init, we walk through all the capability list,
and see what we support. If a cap handler is provided, it'll be
triggered if the cap is detected. MSI cap handler is the first one. 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 | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/pci.h |  2 ++
 2 files changed, 61 insertions(+)

Comments

Andrew Jones Oct. 20, 2016, 1:30 p.m. UTC | #1 
On Fri, Oct 14, 2016 at 08:40:51PM +0800, Peter Xu wrote:
> During each PCI device init, we walk through all the capability list,
> and see what we support. If a cap handler is provided, it'll be
> triggered if the cap is detected. MSI cap handler is the first one. 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 | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/pci.h |  2 ++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/lib/pci.c b/lib/pci.c
> index 044e4db..1037ae3 100644
> --- a/lib/pci.c
> +++ b/lib/pci.c
> @@ -7,6 +7,35 @@
>  #include "pci.h"
>  #include "asm/pci.h"
>  
> +typedef void (*pci_cap_handler)(pci_dev *dev, int cap_offset);
> +
> +static void pci_cap_msi_handler(pci_dev *dev, int cap_offset)
> +{
> +	printf("Detected MSI for device 0x%x offset 0x%x\n",
> +	       dev->pci_addr, cap_offset);
> +	dev->msi_offset = cap_offset;
> +}
> +
> +static pci_cap_handler cap_handlers[0xff] = {
> +	[PCI_CAP_ID_MSI] = pci_cap_msi_handler,
> +};
> +
> +static void pci_cap_walk(pci_dev *dev)
> +{
> +	uint8_t cap_offset;
> +	uint8_t cap_id;
> +
> +	cap_offset = pci_config_readb(dev->pci_addr, PCI_CAPABILITY_LIST);
> +	while (cap_offset) {
> +		cap_id = pci_config_readb(dev->pci_addr, cap_offset);
> +		printf("PCI detected cap 0x%x\n", cap_id);
> +		if (cap_handlers[cap_id]) {
> +			cap_handlers[cap_id](dev, cap_offset);
> +		}

nit: no need for {}

> +		cap_offset = pci_config_readb(dev->pci_addr, cap_offset + 1);
> +	}
> +}
> +
>  static void pci_set_master(pci_dev *dev, int master)
>  {
>  	uint32_t val = pci_config_read(dev->pci_addr, PCI_COMMAND);
> @@ -14,6 +43,35 @@ static void pci_set_master(pci_dev *dev, int master)
>  	pci_config_write(dev->pci_addr, PCI_COMMAND, val);
>  }
>  
> +void pci_setup_msi(pci_dev *dev, uint64_t msi_addr, uint32_t msi_data)
> +{
> +	uint16_t msi_control;
> +	pcidevaddr_t addr;
> +	uint16_t offset;
> +
> +	assert(dev && dev->inited && dev->msi_offset);
> +
> +	offset = dev->msi_offset;
> +	addr = dev->pci_addr;
> +	msi_control = pci_config_readw(addr, offset + PCI_MSI_FLAGS);
> +	pci_config_write(addr, offset + PCI_MSI_ADDRESS_LO,
> +			 msi_addr & 0xffffffff);
> +
> +	if (msi_control & PCI_MSI_FLAGS_64BIT) {
> +		pci_config_write(addr, offset + PCI_MSI_ADDRESS_HI,
> +				 (uint32_t)(msi_addr >> 32));
> +		pci_config_write(addr, offset + PCI_MSI_DATA_64, msi_data);
> +		printf("MSI: dev 0x%x init 64bit address: ", addr);
> +	} else {
> +		pci_config_write(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);
> +}
> +
>  /*
>   * 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.
> @@ -49,6 +107,7 @@ int pci_dev_init(pci_dev *dev, uint16_t vendor_id, uint16_t device_id)
>  	dev->inited = true;
>  
>  	pci_set_master(dev, 1);
> +	pci_cap_walk(dev);

again, not sure we want this in the one and only init function

>  
>  	return 0;
>  }
> diff --git a/lib/pci.h b/lib/pci.h
> index 6a1c3c9..5581446 100644
> --- a/lib/pci.h
> +++ b/lib/pci.h
> @@ -27,6 +27,7 @@ enum {
>  struct pci_dev {
>  	bool inited;
>  	uint16_t pci_addr;
> +	uint16_t msi_offset;
>  	phys_addr_t pci_bar[PCI_BAR_NUM];
>  };
>  typedef struct pci_dev pci_dev;
> @@ -35,6 +36,7 @@ int pci_dev_init(pci_dev *dev, uint16_t vendor_id, uint16_t device_id);
>  phys_addr_t pci_bar_addr(pci_dev *dev, int bar_num);
>  bool pci_bar_is_memory(pci_dev *dev, int bar_num);
>  bool pci_bar_is_valid(pci_dev *dev, int bar_num);
> +void pci_setup_msi(pci_dev *dev, uint64_t msi_addr, uint32_t msi_data);
>  
>  /*
>   * pci-testdev is a driver for the pci-testdev qemu pci device. The
> -- 
> 2.7.4
>

Looks good from my not knowing much about PCI perspective.

drew
--
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 Oct. 25, 2016, 6:21 a.m. UTC | #2 
On Thu, Oct 20, 2016 at 03:30:32PM +0200, Andrew Jones wrote:

[...]

> > +static void pci_cap_walk(pci_dev *dev)
> > +{
> > +	uint8_t cap_offset;
> > +	uint8_t cap_id;
> > +
> > +	cap_offset = pci_config_readb(dev->pci_addr, PCI_CAPABILITY_LIST);
> > +	while (cap_offset) {
> > +		cap_id = pci_config_readb(dev->pci_addr, cap_offset);
> > +		printf("PCI detected cap 0x%x\n", cap_id);
> > +		if (cap_handlers[cap_id]) {
> > +			cap_handlers[cap_id](dev, cap_offset);
> > +		}
> 
> nit: no need for {}

Will fix.

[...]

> > @@ -49,6 +107,7 @@ int pci_dev_init(pci_dev *dev, uint16_t vendor_id, uint16_t device_id)
> >  	dev->inited = true;
> >  
> >  	pci_set_master(dev, 1);
> > +	pci_cap_walk(dev);
> 
> again, not sure we want this in the one and only init function

I'll put it into the new pci_enable_defaults().

> 
> >  
> >  	return 0;
> >  }
> > diff --git a/lib/pci.h b/lib/pci.h
> > index 6a1c3c9..5581446 100644
> > --- a/lib/pci.h
> > +++ b/lib/pci.h
> > @@ -27,6 +27,7 @@ enum {
> >  struct pci_dev {
> >  	bool inited;
> >  	uint16_t pci_addr;
> > +	uint16_t msi_offset;
> >  	phys_addr_t pci_bar[PCI_BAR_NUM];
> >  };
> >  typedef struct pci_dev pci_dev;
> > @@ -35,6 +36,7 @@ int pci_dev_init(pci_dev *dev, uint16_t vendor_id, uint16_t device_id);
> >  phys_addr_t pci_bar_addr(pci_dev *dev, int bar_num);
> >  bool pci_bar_is_memory(pci_dev *dev, int bar_num);
> >  bool pci_bar_is_valid(pci_dev *dev, int bar_num);
> > +void pci_setup_msi(pci_dev *dev, uint64_t msi_addr, uint32_t msi_data);
> >  
> >  /*
> >   * pci-testdev is a driver for the pci-testdev qemu pci device. The
> > -- 
> > 2.7.4
> >
> 
> Looks good from my not knowing much about PCI perspective.

Thanks for review!

-- 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 044e4db..1037ae3 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -7,6 +7,35 @@ 
 #include "pci.h"
 #include "asm/pci.h"
 
+typedef void (*pci_cap_handler)(pci_dev *dev, int cap_offset);
+
+static void pci_cap_msi_handler(pci_dev *dev, int cap_offset)
+{
+	printf("Detected MSI for device 0x%x offset 0x%x\n",
+	       dev->pci_addr, cap_offset);
+	dev->msi_offset = cap_offset;
+}
+
+static pci_cap_handler cap_handlers[0xff] = {
+	[PCI_CAP_ID_MSI] = pci_cap_msi_handler,
+};
+
+static void pci_cap_walk(pci_dev *dev)
+{
+	uint8_t cap_offset;
+	uint8_t cap_id;
+
+	cap_offset = pci_config_readb(dev->pci_addr, PCI_CAPABILITY_LIST);
+	while (cap_offset) {
+		cap_id = pci_config_readb(dev->pci_addr, 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_addr, cap_offset + 1);
+	}
+}
+
 static void pci_set_master(pci_dev *dev, int master)
 {
 	uint32_t val = pci_config_read(dev->pci_addr, PCI_COMMAND);
@@ -14,6 +43,35 @@  static void pci_set_master(pci_dev *dev, int master)
 	pci_config_write(dev->pci_addr, PCI_COMMAND, val);
 }
 
+void pci_setup_msi(pci_dev *dev, uint64_t msi_addr, uint32_t msi_data)
+{
+	uint16_t msi_control;
+	pcidevaddr_t addr;
+	uint16_t offset;
+
+	assert(dev && dev->inited && dev->msi_offset);
+
+	offset = dev->msi_offset;
+	addr = dev->pci_addr;
+	msi_control = pci_config_readw(addr, offset + PCI_MSI_FLAGS);
+	pci_config_write(addr, offset + PCI_MSI_ADDRESS_LO,
+			 msi_addr & 0xffffffff);
+
+	if (msi_control & PCI_MSI_FLAGS_64BIT) {
+		pci_config_write(addr, offset + PCI_MSI_ADDRESS_HI,
+				 (uint32_t)(msi_addr >> 32));
+		pci_config_write(addr, offset + PCI_MSI_DATA_64, msi_data);
+		printf("MSI: dev 0x%x init 64bit address: ", addr);
+	} else {
+		pci_config_write(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);
+}
+
 /*
  * 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.
@@ -49,6 +107,7 @@  int pci_dev_init(pci_dev *dev, uint16_t vendor_id, uint16_t device_id)
 	dev->inited = true;
 
 	pci_set_master(dev, 1);
+	pci_cap_walk(dev);
 
 	return 0;
 }
diff --git a/lib/pci.h b/lib/pci.h
index 6a1c3c9..5581446 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -27,6 +27,7 @@  enum {
 struct pci_dev {
 	bool inited;
 	uint16_t pci_addr;
+	uint16_t msi_offset;
 	phys_addr_t pci_bar[PCI_BAR_NUM];
 };
 typedef struct pci_dev pci_dev;
@@ -35,6 +36,7 @@  int pci_dev_init(pci_dev *dev, uint16_t vendor_id, uint16_t device_id);
 phys_addr_t pci_bar_addr(pci_dev *dev, int bar_num);
 bool pci_bar_is_memory(pci_dev *dev, int bar_num);
 bool pci_bar_is_valid(pci_dev *dev, int bar_num);
+void pci_setup_msi(pci_dev *dev, uint64_t msi_addr, uint32_t msi_data);
 
 /*
  * pci-testdev is a driver for the pci-testdev qemu pci device. The