[PATCH v3 0/5] Designware host multivector MSI and 64bit MSI fixes

[PATCH v3 0/5] Designware host multivector MSI and 64bit MSI fixes

[Lucas Stach]

This is a reworked version of the multivector MSI support for the designware
PCIe host controller driver.

v3 added patch 2 to share more code between the single and multivector MSI
code paths.

While not really related to this topic patches 4+5 fix the MSI message setup
to work on 64bit and 32bit PAE systems. I included them in this series as
they depend on patch 2.

Regards,
Lucas

Lucas Stach (5):
  PCI: allow MSI chip providers to implement their own multivector MSI
    setup
  PCI: designware: factor out MSI msg setup
  PCI: designware: implement multivector MSI irq setup
  PCI: designware: change prototype of get_msi_addr
  PCI: designware: set up high part of MSI target address

 drivers/pci/host/pci-keystone-dw.c |  2 +-
 drivers/pci/host/pcie-designware.c | 62 +++++++++++++++++++++++++++++++-------
 drivers/pci/host/pcie-designware.h |  2 +-
 drivers/pci/msi.c                  |  3 ++
 include/linux/msi.h                |  2 ++
 5 files changed, 58 insertions(+), 13 deletions(-)

-- 
2.4.6

[PATCH v3 1/5] PCI: allow MSI chip providers to implement their own multivector MSI setup

[Lucas Stach]

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/pci/msi.c   | 3 +++
 include/linux/msi.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index f66be868ad21..685a83c459c6 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -121,9 +121,12 @@ void __weak arch_teardown_msi_irq(unsigned int irq)
 
 int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 {
+	struct msi_controller *chip = pci_msi_controller(dev);
 	struct msi_desc *entry;
 	int ret;
 
+	if (chip && chip->setup_irqs)
+		return chip->setup_irqs(chip, dev, nvec, type);
 	/*
 	 * If an architecture wants to support multiple MSI, it needs to
 	 * override arch_setup_msi_irqs()
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 8ac4a68ffae2..50fefeb30236 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -114,6 +114,8 @@ struct msi_controller {
 
 	int (*setup_irq)(struct msi_controller *chip, struct pci_dev *dev,
 			 struct msi_desc *desc);
+	int (*setup_irqs)(struct msi_controller *chip, struct pci_dev *dev,
+			  int nvec, int type);
 	void (*teardown_irq)(struct msi_controller *chip, unsigned int irq);
 };
 
-- 
2.4.6

Re: [PATCH v3 1/5] PCI: allow MSI chip providers to implement their own multivector MSI setup

[Pratyush Anand]

On Wed, Aug 12, 2015 at 3:03 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Looks fine to me.

Reviewed-by: Pratyush Anand <pratyush.anand@gmail.com>

[PATCH v3 2/5] PCI: designware: factor out MSI msg setup

[Lucas Stach]

Factor out the PCI MSI message setup from the single MSI setup
function. This will be reused by the multivector MSI setup.

No functional change yet.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/pci/host/pcie-designware.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 69486be7181e..0ec435c56a8b 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -293,19 +293,9 @@ no_valid_irq:
 	return -ENOSPC;
 }
 
-static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
-			struct msi_desc *desc)
+static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
 {
-	int irq, pos;
 	struct msi_msg msg;
-	struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);
-
-	if (desc->msi_attrib.is_msix)
-		return -EINVAL;
-
-	irq = assign_irq(1, desc, &pos);
-	if (irq < 0)
-		return irq;
 
 	if (pp->ops->get_msi_addr)
 		msg.address_lo = pp->ops->get_msi_addr(pp);
@@ -319,6 +309,22 @@ static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
 		msg.data = pos;
 
 	pci_write_msi_msg(irq, &msg);
+}
+
+static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
+			struct msi_desc *desc)
+{
+	int irq, pos;
+	struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);
+
+	if (desc->msi_attrib.is_msix)
+		return -EINVAL;
+
+	irq = assign_irq(1, desc, &pos);
+	if (irq < 0)
+		return irq;
+
+	dw_msi_setup_msg(pp, irq, pos);
 
 	return 0;
 }
-- 
2.4.6

Re: [PATCH v3 2/5] PCI: designware: factor out MSI msg setup

[Pratyush Anand]

On Wed, Aug 12, 2015 at 3:03 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Factor out the PCI MSI message setup from the single MSI setup
> function. This will be reused by the multivector MSI setup.
>
> No functional change yet.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/pci/host/pcie-designware.c | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 69486be7181e..0ec435c56a8b 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -293,19 +293,9 @@ no_valid_irq:
>         return -ENOSPC;
>  }
>
> -static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
> -                       struct msi_desc *desc)
> +static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
>  {
> -       int irq, pos;
>         struct msi_msg msg;
> -       struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);
> -
> -       if (desc->msi_attrib.is_msix)
> -               return -EINVAL;
> -
> -       irq = assign_irq(1, desc, &pos);
> -       if (irq < 0)
> -               return irq;
>
>         if (pp->ops->get_msi_addr)
>                 msg.address_lo = pp->ops->get_msi_addr(pp);
> @@ -319,6 +309,22 @@ static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
>                 msg.data = pos;
>
>         pci_write_msi_msg(irq, &msg);
> +}
> +
> +static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
> +                       struct msi_desc *desc)
> +{
> +       int irq, pos;
> +       struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);
> +
> +       if (desc->msi_attrib.is_msix)
> +               return -EINVAL;
> +
> +       irq = assign_irq(1, desc, &pos);
> +       if (irq < 0)
> +               return irq;
> +
> +       dw_msi_setup_msg(pp, irq, pos);

I had thought of assign_irq also as part of new function with
an additional argument nvec.
But then dw_msi_setup_msg name would not be justified.So.

Acked-by: Pratyush Anand <pratyush.anand@gmail.com>

[PATCH v3 3/5] PCI: designware: implement multivector MSI irq setup

[Lucas Stach]

Allows to set up and use multiple MSI irqs per device.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
v3: use new MSI msg setup function.
---
 drivers/pci/host/pcie-designware.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 0ec435c56a8b..74034395cf2a 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -286,6 +286,9 @@ static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
 	}
 
 	*pos = pos0;
+	desc->nvec_used = no_irqs;
+	desc->msi_attrib.multiple = order_base_2(no_irqs);
+
 	return irq;
 
 no_valid_irq:
@@ -329,6 +332,33 @@ static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
 	return 0;
 }
 
+static int dw_msi_setup_irqs(struct msi_controller *chip, struct pci_dev *pdev,
+			     int nvec, int type)
+{
+#ifdef CONFIG_PCI_MSI
+	int irq, pos;
+	struct msi_desc *desc;
+	struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);
+
+	/* MSI-X interrupts are not supported */
+	if (type == PCI_CAP_ID_MSIX)
+		return -EINVAL;
+
+	WARN_ON(!list_is_singular(&pdev->msi_list));
+	desc = list_entry(pdev->msi_list.next, struct msi_desc, list);
+
+	irq = assign_irq(nvec, desc, &pos);
+	if (irq < 0)
+		return irq;
+
+	dw_msi_setup_msg(pp, irq, pos);
+
+	return 0;
+#else
+	return -ENOSYS;
+#endif
+}
+
 static void dw_msi_teardown_irq(struct msi_controller *chip, unsigned int irq)
 {
 	struct irq_data *data = irq_get_irq_data(irq);
@@ -340,6 +370,7 @@ static void dw_msi_teardown_irq(struct msi_controller *chip, unsigned int irq)
 
 static struct msi_controller dw_pcie_msi_chip = {
 	.setup_irq = dw_msi_setup_irq,
+	.setup_irqs = dw_msi_setup_irqs,
 	.teardown_irq = dw_msi_teardown_irq,
 };
 
-- 
2.4.6

Re: [PATCH v3 3/5] PCI: designware: implement multivector MSI irq setup

[Pratyush Anand]

On Wed, Aug 12, 2015 at 3:03 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Allows to set up and use multiple MSI irqs per device.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Acked-by: Pratyush Anand <pratyush.anand@gmail.com>

[PATCH v3 4/5] PCI: designware: change prototype of get_msi_addr

[Lucas Stach]

Use phys_addr_t to allow the MSI target address to be above the
4GB mark for 64bit or PAE systems. No functional change for the
current 32bit platform users as phys_addr_t maps to u32 for them.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/pci/host/pci-keystone-dw.c | 2 +-
 drivers/pci/host/pcie-designware.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
index f34892e0edb4..bf4e7e9aea4b 100644
--- a/drivers/pci/host/pci-keystone-dw.c
+++ b/drivers/pci/host/pci-keystone-dw.c
@@ -70,7 +70,7 @@ static inline void update_reg_offset_bit_pos(u32 offset, u32 *reg_offset,
 	*bit_pos = offset >> 3;
 }
 
-u32 ks_dw_pcie_get_msi_addr(struct pcie_port *pp)
+phys_addr_t ks_dw_pcie_get_msi_addr(struct pcie_port *pp)
 {
 	struct keystone_pcie *ks_pcie = to_keystone_pcie(pp);
 
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index d0bbd276840d..35123d9362c5 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -70,7 +70,7 @@ struct pcie_host_ops {
 	void (*host_init)(struct pcie_port *pp);
 	void (*msi_set_irq)(struct pcie_port *pp, int irq);
 	void (*msi_clear_irq)(struct pcie_port *pp, int irq);
-	u32 (*get_msi_addr)(struct pcie_port *pp);
+	phys_addr_t (*get_msi_addr)(struct pcie_port *pp);
 	u32 (*get_msi_data)(struct pcie_port *pp, int pos);
 	void (*scan_bus)(struct pcie_port *pp);
 	int (*msi_host_init)(struct pcie_port *pp, struct msi_controller *chip);
-- 
2.4.6

Re: [PATCH v3 4/5] PCI: designware: change prototype of get_msi_addr

[Pratyush Anand]

On Wed, Aug 12, 2015 at 3:03 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Use phys_addr_t to allow the MSI target address to be above the
> 4GB mark for 64bit or PAE systems. No functional change for the
> current 32bit platform users as phys_addr_t maps to u32 for them.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Acked-by: Pratyush Anand <pratyush.anand@gmail.com>

[PATCH v3 5/5] PCI: designware: set up high part of MSI target address

[Lucas Stach]

Set up the high part of the MSI target address in order to allow the
MSI target to reside above the 4GB mark on 64bit and PAE systems.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/pci/host/pcie-designware.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 74034395cf2a..fcb798e47c30 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -299,12 +299,15 @@ no_valid_irq:
 static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
 {
 	struct msi_msg msg;
+	u64 msi_target;
 
 	if (pp->ops->get_msi_addr)
-		msg.address_lo = pp->ops->get_msi_addr(pp);
+		msi_target = pp->ops->get_msi_addr(pp);
 	else
-		msg.address_lo = virt_to_phys((void *)pp->msi_data);
-	msg.address_hi = 0x0;
+		msi_target = virt_to_phys((void *)pp->msi_data);
+
+	msg.address_lo = (u32)(msi_target & 0xffffffff);
+	msg.address_hi = (u32)(msi_target >> 32 & 0xffffffff);
 
 	if (pp->ops->get_msi_data)
 		msg.data = pp->ops->get_msi_data(pp, pos);
-- 
2.4.6

Re: [PATCH v3 5/5] PCI: designware: set up high part of MSI target address

[Pratyush Anand]

On Wed, Aug 12, 2015 at 3:03 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Set up the high part of the MSI target address in order to allow the
> MSI target to reside above the 4GB mark on 64bit and PAE systems.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/pci/host/pcie-designware.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 74034395cf2a..fcb798e47c30 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -299,12 +299,15 @@ no_valid_irq:
>  static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
>  {
>         struct msi_msg msg;
> +       u64 msi_target;
>
>         if (pp->ops->get_msi_addr)
> -               msg.address_lo = pp->ops->get_msi_addr(pp);
> +               msi_target = pp->ops->get_msi_addr(pp);
>         else
> -               msg.address_lo = virt_to_phys((void *)pp->msi_data);
> -       msg.address_hi = 0x0;
> +               msi_target = virt_to_phys((void *)pp->msi_data);
> +
> +       msg.address_lo = (u32)(msi_target & 0xffffffff);
> +       msg.address_hi = (u32)(msi_target >> 32 & 0xffffffff);
>
>         if (pp->ops->get_msi_data)
>                 msg.data = pp->ops->get_msi_data(pp, pos);

Other than above, dw_pcie_msi_init also need to be fixed
for PCIE_MSI_ADDR_HI.

Re: [PATCH v3 5/5] PCI: designware: set up high part of MSI target address

[Lucas Stach]

Hi Bjorn,

I'm going to respin this patch to fix it regarding Pratyushs comment.

In such a situation do you prefer a repost of the whole series or rather
just have the single fixed patch reposted?

Regards,
Lucas

Am Mittwoch, den 12.08.2015, 21:05 +0530 schrieb Pratyush Anand:
> On Wed, Aug 12, 2015 at 3:03 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> > Set up the high part of the MSI target address in order to allow the
> > MSI target to reside above the 4GB mark on 64bit and PAE systems.
> >
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  drivers/pci/host/pcie-designware.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> > index 74034395cf2a..fcb798e47c30 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -299,12 +299,15 @@ no_valid_irq:
> >  static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
> >  {
> >         struct msi_msg msg;
> > +       u64 msi_target;
> >
> >         if (pp->ops->get_msi_addr)
> > -               msg.address_lo = pp->ops->get_msi_addr(pp);
> > +               msi_target = pp->ops->get_msi_addr(pp);
> >         else
> > -               msg.address_lo = virt_to_phys((void *)pp->msi_data);
> > -       msg.address_hi = 0x0;
> > +               msi_target = virt_to_phys((void *)pp->msi_data);
> > +
> > +       msg.address_lo = (u32)(msi_target & 0xffffffff);
> > +       msg.address_hi = (u32)(msi_target >> 32 & 0xffffffff);
> >
> >         if (pp->ops->get_msi_data)
> >                 msg.data = pp->ops->get_msi_data(pp, pos);
> 
> Other than above, dw_pcie_msi_init also need to be fixed
> for PCIE_MSI_ADDR_HI.

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [PATCH v3 5/5] PCI: designware: set up high part of MSI target address

[Bjorn Helgaas]

On Thu, Aug 13, 2015 at 04:16:32PM +0200, Lucas Stach wrote:
> Hi Bjorn,
> 
> I'm going to respin this patch to fix it regarding Pratyushs comment.
> 
> In such a situation do you prefer a repost of the whole series or rather
> just have the single fixed patch reposted?

The easiest thing for me is a new v4 series.  It's easy for me to
make a mistake when assembling from pieces.

> Am Mittwoch, den 12.08.2015, 21:05 +0530 schrieb Pratyush Anand:
> > On Wed, Aug 12, 2015 at 3:03 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> > > Set up the high part of the MSI target address in order to allow the
> > > MSI target to reside above the 4GB mark on 64bit and PAE systems.
> > >
> > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > ---
> > >  drivers/pci/host/pcie-designware.c | 9 ++++++---
> > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> > > index 74034395cf2a..fcb798e47c30 100644
> > > --- a/drivers/pci/host/pcie-designware.c
> > > +++ b/drivers/pci/host/pcie-designware.c
> > > @@ -299,12 +299,15 @@ no_valid_irq:
> > >  static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
> > >  {
> > >         struct msi_msg msg;
> > > +       u64 msi_target;
> > >
> > >         if (pp->ops->get_msi_addr)
> > > -               msg.address_lo = pp->ops->get_msi_addr(pp);
> > > +               msi_target = pp->ops->get_msi_addr(pp);
> > >         else
> > > -               msg.address_lo = virt_to_phys((void *)pp->msi_data);
> > > -       msg.address_hi = 0x0;
> > > +               msi_target = virt_to_phys((void *)pp->msi_data);
> > > +
> > > +       msg.address_lo = (u32)(msi_target & 0xffffffff);
> > > +       msg.address_hi = (u32)(msi_target >> 32 & 0xffffffff);
> > >
> > >         if (pp->ops->get_msi_data)
> > >                 msg.data = pp->ops->get_msi_data(pp, pos);
> > 
> > Other than above, dw_pcie_msi_init also need to be fixed
> > for PCIE_MSI_ADDR_HI.
> 
> -- 
> Pengutronix e.K.             | Lucas Stach                 |
> Industrial Linux Solutions   | http://www.pengutronix.de/  |
>