Re: [PATCH v2 0/2] PCI Quirk Patchset for Microsemi Switchtec NTB

[Logan Gunthorpe]

Thanks for cleaning this up Bjorn.

On 29/06/18 07:50 PM, Bjorn Helgaas wrote:
> Please review and holler if I broke something.

I pulled your branch, reviewed it and lightly tested it. Everything
looks good to me.

> This touches drivers/ntb/hw/..., which isn't my area.  Let me know if you'd
> rather take these through a different tree.

The NTB change is very minor, and there's nothing this will conflict
with so I think it's fine to go through the PCI tree.

Logan

Re: [PATCH v2 0/2] PCI Quirk Patchset for Microsemi Switchtec NTB

[Logan Gunthorpe]

Thanks for cleaning this up Bjorn.

On 29/06/18 07:50 PM, Bjorn Helgaas wrote:
> Please review and holler if I broke something.

I pulled your branch, reviewed it and lightly tested it. Everything
looks good to me.

> This touches drivers/ntb/hw/..., which isn't my area.  Let me know if you'd
> rather take these through a different tree.

The NTB change is very minor, and there's nothing this will conflict
with so I think it's fine to go through the PCI tree.

Logan

Re: [PATCH v2 0/2] PCI Quirk Patchset for Microsemi Switchtec NTB

[Bjorn Helgaas]

On Fri, Jun 29, 2018 at 08:50:02PM -0500, Bjorn Helgaas wrote:
> On Wed, May 23, 2018 at 01:18:04PM -0700, dme...@gigaio.com wrote:
> > From: Doug Meyer 
> > 
> > This is a resend of the patch series to enable Microsemi Switchtec
> > NTB configurations to run with the IOMMU in the hosts turned on.
> > Because of the nature PCI Quirk implementation, it was preferable
> > to migrate the Microsemi PCI vendor and device definitions to the
> > Linux canonical location. Logan Gunthorpe requested that this
> > migration be done as a separate patch in a set, and so this patch
> > series was created as shown here.
> > 
> > The first patch encapsulates the movement of constants from
> > switchtec.h to pci_ids.h, with commensurate changes to the source
> > files. This patch is not dependent on any other work.
> > 
> > The second patch is the PCI quirk implementation itself, and is
> > completely dependent upon the first patch in this series.
> > 
> > Testing of the quirk was done on with a 2-host x86-64 system
> > with all combinations of IOMMU off/on. The ntb_perf module was
> > used as test stimulus. 
> > 
> > Blessings,
> > Doug Meyer
> > 
> > Changes since v1:
> > - Call pci_device_disable() at return points to clean up properly.
> > - Changed all dev_* print macros to pci_* macros.
> > - Removed superfluous variable initializations.
> > 
> > Doug Meyer (2):
> >   NTB: Migrate PCI Constants to Cannonical PCI Header
> >   NTB: PCI Quirk to Enable Switchtec NT Functionality with IOMMU On
> > 
> >  drivers/ntb/hw/mscc/ntb_hw_switchtec.c |   3 +-
> >  drivers/pci/quirks.c   | 197 
> > +
> >  drivers/pci/switch/switchtec.c |  15 ++-
> >  include/linux/pci_ids.h|  32 ++
> >  include/linux/switchtec.h  |   4 -
> >  5 files changed, 238 insertions(+), 13 deletions(-)
> 
> I applied these with Logan's reviewed-by to my pci/switchtec branch for
> v4.19 with the following updates:
> 
>   - You removed the SPDX tag from drivers/pci/switch/switchtec.c.  I assume
> that was a mistake, so I restored it.
> 
>   - I moved the PCI_VENDOR_ID_MICROSEMI definition to keep the file sorted.
> 
>   - I dropped the Device ID definitions per the policy at the top of the
> file (which I mentioned on your v1 posting).
> 
>   - I converted all the Device ID definition uses to raw hex constants.  I
> noticed that the following were defined by your patch, but not used:
> 
>   PCI_DEVICE_ID_MICROSEMI_PSX24XG3
>   PCI_DEVICE_ID_MICROSEMI_PSX32XG3
> 
> I can't tell whether the quirk is supposed to apply to them or not.
> 
> Please review and holler if I broke something.
> 
> This touches drivers/ntb/hw/..., which isn't my area.  Let me know if you'd
> rather take these through a different tree.

I also:

  - Fixed the sparse warnings reported by the kbuild test robot.

  - Removed some (void * __iomem) casts in the ioread32() parameters and
the (u64) cast on the return value.  I noticed that
switchtec_ntb_init_sndev() does an ioread64() of ep_map instead of two
ioread32() calls.  I suspect both places could and should do it the
same way.

  - Used %02x.%d instead of %02d.%d when printing the DMA alias, so it
matches the other places that print PCI device addresses.

  - Replaced DECLARE_PCI_FIXUP_CLASS_FINAL with DECLARE_PCI_FIXUP_FINAL,
since it should be sufficient to filter on Vendor and Device ID,
without worrying about the class.

Here's the diff from what you originally posted to what's currently on my
pci/switchtec branch:

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index f178cecdc001..d54a182a09cf 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4787,14 +4787,13 @@ static void quirk_switchtec_ntb_dma_alias(struct 
pci_dev *pdev)
pci_info(pdev, "Setting Switchtec proxy ID aliases\n");
 
mmio_ntb = mmio + SWITCHTEC_GAS_NTB_OFFSET;
-   mmio_ctrl = (void * __iomem) mmio_ntb + SWITCHTEC_NTB_REG_CTRL_OFFSET;
+   mmio_ctrl = (void __iomem *) mmio_ntb + SWITCHTEC_NTB_REG_CTRL_OFFSET;
mmio_sys_info = mmio + SWITCHTEC_GAS_SYS_INFO_OFFSET;
 
partition = ioread8(_ntb->partition_id);
 
-   partition_map = (u64) ioread32((void * __iomem) _ntb->ep_map);
-   partition_map |=
-   ((u64) ioread32((void * __iomem) _ntb->ep_map + 4)) << 32;
+   partition_map = ioread32(_ntb->ep_map);
+   partition_map |= ((u64) ioread32(_ntb->ep_map + 4)) << 32;
partition_map &= ~(1ULL << partition);
 
for (pp = 0; pp < (sizeof(partition_map) * 8); pp++) {
@@ -4829,7 +4828,7 @@ static void quirk_switchtec_ntb_dma_alias(struct pci_dev 
*pdev)
rid_entry = ioread32(_peer_ctrl->req_id_table[te]);
devfn = (rid_entry >> 1) & 0xFF;
pci_dbg(pdev,
-   "Aliasing Partition %d Proxy ID %02d.%d\n",
+   "Aliasing 

Re: [PATCH v2 0/2] PCI Quirk Patchset for Microsemi Switchtec NTB

[Bjorn Helgaas]

On Fri, Jun 29, 2018 at 08:50:02PM -0500, Bjorn Helgaas wrote:
> On Wed, May 23, 2018 at 01:18:04PM -0700, dme...@gigaio.com wrote:
> > From: Doug Meyer 
> > 
> > This is a resend of the patch series to enable Microsemi Switchtec
> > NTB configurations to run with the IOMMU in the hosts turned on.
> > Because of the nature PCI Quirk implementation, it was preferable
> > to migrate the Microsemi PCI vendor and device definitions to the
> > Linux canonical location. Logan Gunthorpe requested that this
> > migration be done as a separate patch in a set, and so this patch
> > series was created as shown here.
> > 
> > The first patch encapsulates the movement of constants from
> > switchtec.h to pci_ids.h, with commensurate changes to the source
> > files. This patch is not dependent on any other work.
> > 
> > The second patch is the PCI quirk implementation itself, and is
> > completely dependent upon the first patch in this series.
> > 
> > Testing of the quirk was done on with a 2-host x86-64 system
> > with all combinations of IOMMU off/on. The ntb_perf module was
> > used as test stimulus. 
> > 
> > Blessings,
> > Doug Meyer
> > 
> > Changes since v1:
> > - Call pci_device_disable() at return points to clean up properly.
> > - Changed all dev_* print macros to pci_* macros.
> > - Removed superfluous variable initializations.
> > 
> > Doug Meyer (2):
> >   NTB: Migrate PCI Constants to Cannonical PCI Header
> >   NTB: PCI Quirk to Enable Switchtec NT Functionality with IOMMU On
> > 
> >  drivers/ntb/hw/mscc/ntb_hw_switchtec.c |   3 +-
> >  drivers/pci/quirks.c   | 197 
> > +
> >  drivers/pci/switch/switchtec.c |  15 ++-
> >  include/linux/pci_ids.h|  32 ++
> >  include/linux/switchtec.h  |   4 -
> >  5 files changed, 238 insertions(+), 13 deletions(-)
> 
> I applied these with Logan's reviewed-by to my pci/switchtec branch for
> v4.19 with the following updates:
> 
>   - You removed the SPDX tag from drivers/pci/switch/switchtec.c.  I assume
> that was a mistake, so I restored it.
> 
>   - I moved the PCI_VENDOR_ID_MICROSEMI definition to keep the file sorted.
> 
>   - I dropped the Device ID definitions per the policy at the top of the
> file (which I mentioned on your v1 posting).
> 
>   - I converted all the Device ID definition uses to raw hex constants.  I
> noticed that the following were defined by your patch, but not used:
> 
>   PCI_DEVICE_ID_MICROSEMI_PSX24XG3
>   PCI_DEVICE_ID_MICROSEMI_PSX32XG3
> 
> I can't tell whether the quirk is supposed to apply to them or not.
> 
> Please review and holler if I broke something.
> 
> This touches drivers/ntb/hw/..., which isn't my area.  Let me know if you'd
> rather take these through a different tree.

I also:

  - Fixed the sparse warnings reported by the kbuild test robot.

  - Removed some (void * __iomem) casts in the ioread32() parameters and
the (u64) cast on the return value.  I noticed that
switchtec_ntb_init_sndev() does an ioread64() of ep_map instead of two
ioread32() calls.  I suspect both places could and should do it the
same way.

  - Used %02x.%d instead of %02d.%d when printing the DMA alias, so it
matches the other places that print PCI device addresses.

  - Replaced DECLARE_PCI_FIXUP_CLASS_FINAL with DECLARE_PCI_FIXUP_FINAL,
since it should be sufficient to filter on Vendor and Device ID,
without worrying about the class.

Here's the diff from what you originally posted to what's currently on my
pci/switchtec branch:

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index f178cecdc001..d54a182a09cf 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4787,14 +4787,13 @@ static void quirk_switchtec_ntb_dma_alias(struct 
pci_dev *pdev)
pci_info(pdev, "Setting Switchtec proxy ID aliases\n");
 
mmio_ntb = mmio + SWITCHTEC_GAS_NTB_OFFSET;
-   mmio_ctrl = (void * __iomem) mmio_ntb + SWITCHTEC_NTB_REG_CTRL_OFFSET;
+   mmio_ctrl = (void __iomem *) mmio_ntb + SWITCHTEC_NTB_REG_CTRL_OFFSET;
mmio_sys_info = mmio + SWITCHTEC_GAS_SYS_INFO_OFFSET;
 
partition = ioread8(_ntb->partition_id);
 
-   partition_map = (u64) ioread32((void * __iomem) _ntb->ep_map);
-   partition_map |=
-   ((u64) ioread32((void * __iomem) _ntb->ep_map + 4)) << 32;
+   partition_map = ioread32(_ntb->ep_map);
+   partition_map |= ((u64) ioread32(_ntb->ep_map + 4)) << 32;
partition_map &= ~(1ULL << partition);
 
for (pp = 0; pp < (sizeof(partition_map) * 8); pp++) {
@@ -4829,7 +4828,7 @@ static void quirk_switchtec_ntb_dma_alias(struct pci_dev 
*pdev)
rid_entry = ioread32(_peer_ctrl->req_id_table[te]);
devfn = (rid_entry >> 1) & 0xFF;
pci_dbg(pdev,
-   "Aliasing Partition %d Proxy ID %02d.%d\n",
+   "Aliasing 

Re: [PATCH v2 0/2] PCI Quirk Patchset for Microsemi Switchtec NTB

[Bjorn Helgaas]

On Wed, May 23, 2018 at 01:18:04PM -0700, dme...@gigaio.com wrote:
> From: Doug Meyer 
> 
> This is a resend of the patch series to enable Microsemi Switchtec
> NTB configurations to run with the IOMMU in the hosts turned on.
> Because of the nature PCI Quirk implementation, it was preferable
> to migrate the Microsemi PCI vendor and device definitions to the
> Linux canonical location. Logan Gunthorpe requested that this
> migration be done as a separate patch in a set, and so this patch
> series was created as shown here.
> 
> The first patch encapsulates the movement of constants from
> switchtec.h to pci_ids.h, with commensurate changes to the source
> files. This patch is not dependent on any other work.
> 
> The second patch is the PCI quirk implementation itself, and is
> completely dependent upon the first patch in this series.
> 
> Testing of the quirk was done on with a 2-host x86-64 system
> with all combinations of IOMMU off/on. The ntb_perf module was
> used as test stimulus. 
> 
> Blessings,
> Doug Meyer
> 
> Changes since v1:
> - Call pci_device_disable() at return points to clean up properly.
> - Changed all dev_* print macros to pci_* macros.
> - Removed superfluous variable initializations.
> 
> Doug Meyer (2):
>   NTB: Migrate PCI Constants to Cannonical PCI Header
>   NTB: PCI Quirk to Enable Switchtec NT Functionality with IOMMU On
> 
>  drivers/ntb/hw/mscc/ntb_hw_switchtec.c |   3 +-
>  drivers/pci/quirks.c   | 197 
> +
>  drivers/pci/switch/switchtec.c |  15 ++-
>  include/linux/pci_ids.h|  32 ++
>  include/linux/switchtec.h  |   4 -
>  5 files changed, 238 insertions(+), 13 deletions(-)

I applied these with Logan's reviewed-by to my pci/switchtec branch for
v4.19 with the following updates:

  - You removed the SPDX tag from drivers/pci/switch/switchtec.c.  I assume
that was a mistake, so I restored it.

  - I moved the PCI_VENDOR_ID_MICROSEMI definition to keep the file sorted.

  - I dropped the Device ID definitions per the policy at the top of the
file (which I mentioned on your v1 posting).

  - I converted all the Device ID definition uses to raw hex constants.  I
noticed that the following were defined by your patch, but not used:

  PCI_DEVICE_ID_MICROSEMI_PSX24XG3
  PCI_DEVICE_ID_MICROSEMI_PSX32XG3

I can't tell whether the quirk is supposed to apply to them or not.

Please review and holler if I broke something.

This touches drivers/ntb/hw/..., which isn't my area.  Let me know if you'd
rather take these through a different tree.

Bjorn

Re: [PATCH v2 0/2] PCI Quirk Patchset for Microsemi Switchtec NTB

[Bjorn Helgaas]

On Wed, May 23, 2018 at 01:18:04PM -0700, dme...@gigaio.com wrote:
> From: Doug Meyer 
> 
> This is a resend of the patch series to enable Microsemi Switchtec
> NTB configurations to run with the IOMMU in the hosts turned on.
> Because of the nature PCI Quirk implementation, it was preferable
> to migrate the Microsemi PCI vendor and device definitions to the
> Linux canonical location. Logan Gunthorpe requested that this
> migration be done as a separate patch in a set, and so this patch
> series was created as shown here.
> 
> The first patch encapsulates the movement of constants from
> switchtec.h to pci_ids.h, with commensurate changes to the source
> files. This patch is not dependent on any other work.
> 
> The second patch is the PCI quirk implementation itself, and is
> completely dependent upon the first patch in this series.
> 
> Testing of the quirk was done on with a 2-host x86-64 system
> with all combinations of IOMMU off/on. The ntb_perf module was
> used as test stimulus. 
> 
> Blessings,
> Doug Meyer
> 
> Changes since v1:
> - Call pci_device_disable() at return points to clean up properly.
> - Changed all dev_* print macros to pci_* macros.
> - Removed superfluous variable initializations.
> 
> Doug Meyer (2):
>   NTB: Migrate PCI Constants to Cannonical PCI Header
>   NTB: PCI Quirk to Enable Switchtec NT Functionality with IOMMU On
> 
>  drivers/ntb/hw/mscc/ntb_hw_switchtec.c |   3 +-
>  drivers/pci/quirks.c   | 197 
> +
>  drivers/pci/switch/switchtec.c |  15 ++-
>  include/linux/pci_ids.h|  32 ++
>  include/linux/switchtec.h  |   4 -
>  5 files changed, 238 insertions(+), 13 deletions(-)

I applied these with Logan's reviewed-by to my pci/switchtec branch for
v4.19 with the following updates:

  - You removed the SPDX tag from drivers/pci/switch/switchtec.c.  I assume
that was a mistake, so I restored it.

  - I moved the PCI_VENDOR_ID_MICROSEMI definition to keep the file sorted.

  - I dropped the Device ID definitions per the policy at the top of the
file (which I mentioned on your v1 posting).

  - I converted all the Device ID definition uses to raw hex constants.  I
noticed that the following were defined by your patch, but not used:

  PCI_DEVICE_ID_MICROSEMI_PSX24XG3
  PCI_DEVICE_ID_MICROSEMI_PSX32XG3

I can't tell whether the quirk is supposed to apply to them or not.

Please review and holler if I broke something.

This touches drivers/ntb/hw/..., which isn't my area.  Let me know if you'd
rather take these through a different tree.

Bjorn

[PATCH v2 0/2] PCI Quirk Patchset for Microsemi Switchtec NTB

[dmeyer]

From: Doug Meyer 

This is a resend of the patch series to enable Microsemi Switchtec
NTB configurations to run with the IOMMU in the hosts turned on.
Because of the nature PCI Quirk implementation, it was preferable
to migrate the Microsemi PCI vendor and device definitions to the
Linux canonical location. Logan Gunthorpe requested that this
migration be done as a separate patch in a set, and so this patch
series was created as shown here.

The first patch encapsulates the movement of constants from
switchtec.h to pci_ids.h, with commensurate changes to the source
files. This patch is not dependent on any other work.

The second patch is the PCI quirk implementation itself, and is
completely dependent upon the first patch in this series.

Testing of the quirk was done on with a 2-host x86-64 system
with all combinations of IOMMU off/on. The ntb_perf module was
used as test stimulus. 

Blessings,
Doug Meyer

Changes since v1:
- Call pci_device_disable() at return points to clean up properly.
- Changed all dev_* print macros to pci_* macros.
- Removed superfluous variable initializations.

Doug Meyer (2):
  NTB: Migrate PCI Constants to Cannonical PCI Header
  NTB: PCI Quirk to Enable Switchtec NT Functionality with IOMMU On

 drivers/ntb/hw/mscc/ntb_hw_switchtec.c |   3 +-
 drivers/pci/quirks.c   | 197 +
 drivers/pci/switch/switchtec.c |  15 ++-
 include/linux/pci_ids.h|  32 ++
 include/linux/switchtec.h  |   4 -
 5 files changed, 238 insertions(+), 13 deletions(-)

-- 
1.8.3.1

[PATCH v2 0/2] PCI Quirk Patchset for Microsemi Switchtec NTB

[dmeyer]

From: Doug Meyer 

This is a resend of the patch series to enable Microsemi Switchtec
NTB configurations to run with the IOMMU in the hosts turned on.
Because of the nature PCI Quirk implementation, it was preferable
to migrate the Microsemi PCI vendor and device definitions to the
Linux canonical location. Logan Gunthorpe requested that this
migration be done as a separate patch in a set, and so this patch
series was created as shown here.

The first patch encapsulates the movement of constants from
switchtec.h to pci_ids.h, with commensurate changes to the source
files. This patch is not dependent on any other work.

The second patch is the PCI quirk implementation itself, and is
completely dependent upon the first patch in this series.

Testing of the quirk was done on with a 2-host x86-64 system
with all combinations of IOMMU off/on. The ntb_perf module was
used as test stimulus. 

Blessings,
Doug Meyer

Changes since v1:
- Call pci_device_disable() at return points to clean up properly.
- Changed all dev_* print macros to pci_* macros.
- Removed superfluous variable initializations.

Doug Meyer (2):
  NTB: Migrate PCI Constants to Cannonical PCI Header
  NTB: PCI Quirk to Enable Switchtec NT Functionality with IOMMU On

 drivers/ntb/hw/mscc/ntb_hw_switchtec.c |   3 +-
 drivers/pci/quirks.c   | 197 +
 drivers/pci/switch/switchtec.c |  15 ++-
 include/linux/pci_ids.h|  32 ++
 include/linux/switchtec.h  |   4 -
 5 files changed, 238 insertions(+), 13 deletions(-)

-- 
1.8.3.1