[PATCH 00/12] Switchtec Fixes and Gen4 Support

[PATCH 00/12] Switchtec Fixes and Gen4 Support

[Logan Gunthorpe]

Hi,

Please find a bunch of patches for the switchtec driver collected over the
last few months.

The first 2 patches fix a couple of minor bugs. Patch 3 adds support for
a new event that is available in specific firmware versions. Patches 4 and
5 are some code cleanup changes to simplify the logic. And the last 6
patches implement support for the new Gen4 hardware.

This patchset is based on v5.5-rc5 and a git branch is available here:

https://github.com/sbates130272/linux-p2pmem switchtec-next

Thanks,

Logan

--

Kelvin Cao (3):
  PCI/switchtec: Add gen4 support in struct flash_info_regs
  PCI/switchtec: Add permission check for the GAS access MRPC commands
  PCI/switchtec: Introduce gen4 variant IDS in the device ID table

Logan Gunthorpe (6):
  PCI/switchtec: Fix vep_vector_number ioread width
  PCI/switchtec: Add support for new events
  PCI/switchtec: Introduce Generation Variable
  PCI/switchtec: Separate out gen3 specific fields in the sys_info_regs
    structure
  PCI/switchtec: Add gen4 support in struct sys_info_regs
  PCI: Apply switchtec DMA aliasing quirk to GEN4 devices

Wesley Sheng (3):
  PCI/switchtec: Use dma_set_mask_and_coherent()
  PCI/switchtec: Remove redundant valid PFF number count
  PCI/switchtec: Move check event id from mask_event() to
    switchtec_event_isr()

 drivers/pci/quirks.c                 |  18 ++
 drivers/pci/switch/switchtec.c       | 365 ++++++++++++++++++++-------
 include/linux/switchtec.h            | 160 ++++++++++--
 include/uapi/linux/switchtec_ioctl.h |  13 +-
 4 files changed, 450 insertions(+), 106 deletions(-)

--
2.20.1

[PATCH 01/12] PCI/switchtec: Use dma_set_mask_and_coherent()

[Logan Gunthorpe]

From: Wesley Sheng <wesley.sheng@microchip.com>

Use dma_set_mask_and_coherent() instead of dma_set_coherent_mask() as
the switchtec hardware fully supports 64bit addressing and we should
set both the streaming and coherent masks the same.

Fixes: aff614c6339c ("switchtec: Set DMA coherent mask")
Signed-off-by: Wesley Sheng <wesley.sheng@microchip.com>
[logang@deltatee.com: reworked commit message]
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/switch/switchtec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 88091bbfe77f..2bf670977b9c 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1349,7 +1349,7 @@ static int switchtec_init_pci(struct switchtec_dev *stdev,
 	if (rc)
 		return rc;
 
-	rc = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
+	rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (rc)
 		return rc;
 
-- 
2.20.1

[PATCH 02/12] PCI/switchtec: Fix vep_vector_number ioread width

[Logan Gunthorpe]

vep_vector_number is actually a 16 bit register which should be read with
ioread16() instead of ioread32().

Fixes: 080b47def5e5 ("MicroSemi Switchtec management interface driver")
Reported-by: Doug Meyer <dmeyer@gigaio.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/switch/switchtec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 2bf670977b9c..9c3ad09d3022 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1276,7 +1276,7 @@ static int switchtec_init_isr(struct switchtec_dev *stdev)
 	if (nvecs < 0)
 		return nvecs;
 
-	event_irq = ioread32(&stdev->mmio_part_cfg->vep_vector_number);
+	event_irq = ioread16(&stdev->mmio_part_cfg->vep_vector_number);
 	if (event_irq < 0 || event_irq >= nvecs)
 		return -EFAULT;
 
-- 
2.20.1

[PATCH 03/12] PCI/switchtec: Add support for new events

[Logan Gunthorpe]

The intercomm notify  event was added for PAX variants of switchtec
hardware and the UEC Port was added for the MR1 release of GEN3 firmware.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/switch/switchtec.c       | 3 +++
 include/linux/switchtec.h            | 7 +++++--
 include/uapi/linux/switchtec_ioctl.h | 4 +++-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 9c3ad09d3022..218e67428cf9 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -751,10 +751,13 @@ static const struct event_reg {
 	EV_PAR(SWITCHTEC_IOCTL_EVENT_MRPC_COMP, mrpc_comp_hdr),
 	EV_PAR(SWITCHTEC_IOCTL_EVENT_MRPC_COMP_ASYNC, mrpc_comp_async_hdr),
 	EV_PAR(SWITCHTEC_IOCTL_EVENT_DYN_PART_BIND_COMP, dyn_binding_hdr),
+	EV_PAR(SWITCHTEC_IOCTL_EVENT_INTERCOMM_REQ_NOTIFY,
+	       intercomm_notify_hdr),
 	EV_PFF(SWITCHTEC_IOCTL_EVENT_AER_IN_P2P, aer_in_p2p_hdr),
 	EV_PFF(SWITCHTEC_IOCTL_EVENT_AER_IN_VEP, aer_in_vep_hdr),
 	EV_PFF(SWITCHTEC_IOCTL_EVENT_DPC, dpc_hdr),
 	EV_PFF(SWITCHTEC_IOCTL_EVENT_CTS, cts_hdr),
+	EV_PFF(SWITCHTEC_IOCTL_EVENT_UEC, uec_hdr),
 	EV_PFF(SWITCHTEC_IOCTL_EVENT_HOTPLUG, hotplug_hdr),
 	EV_PFF(SWITCHTEC_IOCTL_EVENT_IER, ier_hdr),
 	EV_PFF(SWITCHTEC_IOCTL_EVENT_THRESH, threshold_hdr),
diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
index e295515bc3f3..b4ba3a38f30f 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -196,7 +196,9 @@ struct part_cfg_regs {
 	u32 mrpc_comp_async_data[5];
 	u32 dyn_binding_hdr;
 	u32 dyn_binding_data[5];
-	u32 reserved4[159];
+	u32 intercomm_notify_hdr;
+	u32 intercomm_notify_data[5];
+	u32 reserved4[153];
 } __packed;
 
 enum {
@@ -320,7 +322,8 @@ struct pff_csr_regs {
 	u32 dpc_data[5];
 	u32 cts_hdr;
 	u32 cts_data[5];
-	u32 reserved3[6];
+	u32 uec_hdr;
+	u32 uec_data[5];
 	u32 hotplug_hdr;
 	u32 hotplug_data[5];
 	u32 ier_hdr;
diff --git a/include/uapi/linux/switchtec_ioctl.h b/include/uapi/linux/switchtec_ioctl.h
index c912b5a678e4..e8db938985ca 100644
--- a/include/uapi/linux/switchtec_ioctl.h
+++ b/include/uapi/linux/switchtec_ioctl.h
@@ -98,7 +98,9 @@ struct switchtec_ioctl_event_summary {
 #define SWITCHTEC_IOCTL_EVENT_CREDIT_TIMEOUT		27
 #define SWITCHTEC_IOCTL_EVENT_LINK_STATE		28
 #define SWITCHTEC_IOCTL_EVENT_GFMS			29
-#define SWITCHTEC_IOCTL_MAX_EVENTS			30
+#define SWITCHTEC_IOCTL_EVENT_INTERCOMM_REQ_NOTIFY	30
+#define SWITCHTEC_IOCTL_EVENT_UEC			31
+#define SWITCHTEC_IOCTL_MAX_EVENTS			32
 
 #define SWITCHTEC_IOCTL_EVENT_LOCAL_PART_IDX -1
 #define SWITCHTEC_IOCTL_EVENT_IDX_ALL -2
-- 
2.20.1

Re: [PATCH 03/12] PCI/switchtec: Add support for new events

[Bjorn Helgaas]

On Mon, Jan 06, 2020 at 12:03:28PM -0700, Logan Gunthorpe wrote:
> The intercomm notify  event was added for PAX variants of switchtec
> hardware and the UEC Port was added for the MR1 release of GEN3 firmware.

Do they actually spell it "intercomm" in the datasheet?  Seems like
the most common English spelling is "intercom".

Is there some meaningful expansion of "UEC"?

> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---
>  drivers/pci/switch/switchtec.c       | 3 +++
>  include/linux/switchtec.h            | 7 +++++--
>  include/uapi/linux/switchtec_ioctl.h | 4 +++-
>  3 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index 9c3ad09d3022..218e67428cf9 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -751,10 +751,13 @@ static const struct event_reg {
>  	EV_PAR(SWITCHTEC_IOCTL_EVENT_MRPC_COMP, mrpc_comp_hdr),
>  	EV_PAR(SWITCHTEC_IOCTL_EVENT_MRPC_COMP_ASYNC, mrpc_comp_async_hdr),
>  	EV_PAR(SWITCHTEC_IOCTL_EVENT_DYN_PART_BIND_COMP, dyn_binding_hdr),
> +	EV_PAR(SWITCHTEC_IOCTL_EVENT_INTERCOMM_REQ_NOTIFY,
> +	       intercomm_notify_hdr),

Re: [PATCH 03/12] PCI/switchtec: Add support for new events

[Logan Gunthorpe]

On 2020-01-08 2:33 p.m., Bjorn Helgaas wrote:
> On Mon, Jan 06, 2020 at 12:03:28PM -0700, Logan Gunthorpe wrote:
>> The intercomm notify  event was added for PAX variants of switchtec
>> hardware and the UEC Port was added for the MR1 release of GEN3 firmware.
> 
> Do they actually spell it "intercomm" in the datasheet?  Seems like
> the most common English spelling is "intercom".

Hmm, not sure I don't have this event in my datasheet which is a little
out of date. Kelvin, whose CC'd, would know if we can change the
spelling hopefully he can weigh in on this. As far as I know the define
isn't used anywhere yet.

> Is there some meaningful expansion of "UEC"?

Upstream Error Containment -- I'll mention that in the commit message.

Logan


>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>> ---
>>  drivers/pci/switch/switchtec.c       | 3 +++
>>  include/linux/switchtec.h            | 7 +++++--
>>  include/uapi/linux/switchtec_ioctl.h | 4 +++-
>>  3 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
>> index 9c3ad09d3022..218e67428cf9 100644
>> --- a/drivers/pci/switch/switchtec.c
>> +++ b/drivers/pci/switch/switchtec.c
>> @@ -751,10 +751,13 @@ static const struct event_reg {
>>  	EV_PAR(SWITCHTEC_IOCTL_EVENT_MRPC_COMP, mrpc_comp_hdr),
>>  	EV_PAR(SWITCHTEC_IOCTL_EVENT_MRPC_COMP_ASYNC, mrpc_comp_async_hdr),
>>  	EV_PAR(SWITCHTEC_IOCTL_EVENT_DYN_PART_BIND_COMP, dyn_binding_hdr),
>> +	EV_PAR(SWITCHTEC_IOCTL_EVENT_INTERCOMM_REQ_NOTIFY,
>> +	       intercomm_notify_hdr),

RE: [PATCH 03/12] PCI/switchtec: Add support for new events

[Kelvin.Cao]

'InterComm' is the spelling in the latest datasheet, it's short for "Inter Fabric Manager Communication". Thanks.

Kelvin

-----Original Message-----
From: Logan Gunthorpe [mailto:logang@deltatee.com] 
Sent: Thursday, January 9, 2020 5:48 AM
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; Kelvin Cao - A31060 <Kelvin.Cao@microchip.com>; Eric Pilmore <epilmore@gigaio.com>; Doug Meyer <dmeyer@gigaio.com>
Subject: Re: [PATCH 03/12] PCI/switchtec: Add support for new events

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

On 2020-01-08 2:33 p.m., Bjorn Helgaas wrote:
> On Mon, Jan 06, 2020 at 12:03:28PM -0700, Logan Gunthorpe wrote:
>> The intercomm notify  event was added for PAX variants of switchtec 
>> hardware and the UEC Port was added for the MR1 release of GEN3 firmware.
>
> Do they actually spell it "intercomm" in the datasheet?  Seems like 
> the most common English spelling is "intercom".

Hmm, not sure I don't have this event in my datasheet which is a little out of date. Kelvin, whose CC'd, would know if we can change the spelling hopefully he can weigh in on this. As far as I know the define isn't used anywhere yet.

> Is there some meaningful expansion of "UEC"?

Upstream Error Containment -- I'll mention that in the commit message.

Logan


>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>> ---
>>  drivers/pci/switch/switchtec.c       | 3 +++
>>  include/linux/switchtec.h            | 7 +++++--
>>  include/uapi/linux/switchtec_ioctl.h | 4 +++-
>>  3 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/switch/switchtec.c 
>> b/drivers/pci/switch/switchtec.c index 9c3ad09d3022..218e67428cf9 
>> 100644
>> --- a/drivers/pci/switch/switchtec.c
>> +++ b/drivers/pci/switch/switchtec.c
>> @@ -751,10 +751,13 @@ static const struct event_reg {
>>      EV_PAR(SWITCHTEC_IOCTL_EVENT_MRPC_COMP, mrpc_comp_hdr),
>>      EV_PAR(SWITCHTEC_IOCTL_EVENT_MRPC_COMP_ASYNC, mrpc_comp_async_hdr),
>>      EV_PAR(SWITCHTEC_IOCTL_EVENT_DYN_PART_BIND_COMP, 
>> dyn_binding_hdr),
>> +    EV_PAR(SWITCHTEC_IOCTL_EVENT_INTERCOMM_REQ_NOTIFY,
>> +           intercomm_notify_hdr),

Re: [PATCH 03/12] PCI/switchtec: Add support for new events

[Logan Gunthorpe]

Bjorn,

On 2020-01-13 7:07 p.m., Kelvin.Cao@microchip.com wrote:
> 'InterComm' is the spelling in the latest datasheet, it's short for "Inter Fabric Manager Communication". Thanks.

I noticed you fixed up the spelling in your branch but the IOCTL define
is still spelled INTERCOM, which is arguably the most important seeing
it's a userspace interface.

Thanks,

Logan



diff --git a/include/uapi/linux/switchtec_ioctl.h
b/include/uapi/linux/switchtec_ioctl.h
index c912b5a678e4..902fffa9e0f5 100644
--- a/include/uapi/linux/switchtec_ioctl.h
+++ b/include/uapi/linux/switchtec_ioctl.h
@@ -98,7 +98,9 @@ struct switchtec_ioctl_event_summary {
 #define SWITCHTEC_IOCTL_EVENT_CREDIT_TIMEOUT           27
 #define SWITCHTEC_IOCTL_EVENT_LINK_STATE               28
 #define SWITCHTEC_IOCTL_EVENT_GFMS                     29
-#define SWITCHTEC_IOCTL_MAX_EVENTS                     30
+#define SWITCHTEC_IOCTL_EVENT_INTERCOM_REQ_NOTIFY      30
+#define SWITCHTEC_IOCTL_EVENT_UEC                      31
+#define SWITCHTEC_IOCTL_MAX_EVENTS                     32

 #define SWITCHTEC_IOCTL_EVENT_LOCAL_PART_IDX -1
 #define SWITCHTEC_IOCTL_EVENT_IDX_ALL -2

Re: [PATCH 03/12] PCI/switchtec: Add support for new events

[Bjorn Helgaas]

On Tue, Jan 14, 2020 at 11:21:45AM -0700, Logan Gunthorpe wrote:
> Bjorn,
> 
> On 2020-01-13 7:07 p.m., Kelvin.Cao@microchip.com wrote:
> > 'InterComm' is the spelling in the latest datasheet, it's short for "Inter Fabric Manager Communication". Thanks.
> 
> I noticed you fixed up the spelling in your branch but the IOCTL define
> is still spelled INTERCOM, which is arguably the most important seeing
> it's a userspace interface.

Thanks, I missed that one.  Fixed now.

[PATCH 04/12] PCI/switchtec: Remove redundant valid PFF number count

[Logan Gunthorpe]

From: Wesley Sheng <wesley.sheng@microchip.com>

init_pff() function has already counted the valid PFF number, so there
is no requirement to count it again in ioctl_event_summary().

Signed-off-by: Wesley Sheng <wesley.sheng@microchip.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/switch/switchtec.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 218e67428cf9..231499da2899 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -683,11 +683,7 @@ static int ioctl_event_summary(struct switchtec_dev *stdev,
 		s->part[i] = reg;
 	}
 
-	for (i = 0; i < SWITCHTEC_MAX_PFF_CSR; i++) {
-		reg = ioread16(&stdev->mmio_pff_csr[i].vendor_id);
-		if (reg != PCI_VENDOR_ID_MICROSEMI)
-			break;
-
+	for (i = 0; i < stdev->pff_csr_count; i++) {
 		reg = ioread32(&stdev->mmio_pff_csr[i].pff_event_summary);
 		s->pff[i] = reg;
 	}
@@ -1327,16 +1323,16 @@ static void init_pff(struct switchtec_dev *stdev)
 	stdev->pff_csr_count = i;
 
 	reg = ioread32(&pcfg->usp_pff_inst_id);
-	if (reg < SWITCHTEC_MAX_PFF_CSR)
+	if (reg < stdev->pff_csr_count)
 		stdev->pff_local[reg] = 1;
 
 	reg = ioread32(&pcfg->vep_pff_inst_id);
-	if (reg < SWITCHTEC_MAX_PFF_CSR)
+	if (reg < stdev->pff_csr_count)
 		stdev->pff_local[reg] = 1;
 
 	for (i = 0; i < ARRAY_SIZE(pcfg->dsp_pff_inst_id); i++) {
 		reg = ioread32(&pcfg->dsp_pff_inst_id[i]);
-		if (reg < SWITCHTEC_MAX_PFF_CSR)
+		if (reg < stdev->pff_csr_count)
 			stdev->pff_local[reg] = 1;
 	}
 }
-- 
2.20.1

[PATCH 05/12] PCI/switchtec: Move check event id from mask_event() to switchtec_event_isr()

[Logan Gunthorpe]

From: Wesley Sheng <wesley.sheng@microchip.com>

Event id check doesn't depend on anything in the mask_all_events()
to mask_event() path, doing it in switchtec_event_isr() would
avoid the CSR read in mask_event().

Signed-off-by: Wesley Sheng <wesley.sheng@microchip.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/switch/switchtec.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 231499da2899..05d4cb49219b 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1180,10 +1180,6 @@ static int mask_event(struct switchtec_dev *stdev, int eid, int idx)
 	if (!(hdr & SWITCHTEC_EVENT_OCCURRED && hdr & SWITCHTEC_EVENT_EN_IRQ))
 		return 0;
 
-	if (eid == SWITCHTEC_IOCTL_EVENT_LINK_STATE ||
-	    eid == SWITCHTEC_IOCTL_EVENT_MRPC_COMP)
-		return 0;
-
 	dev_dbg(&stdev->dev, "%s: %d %d %x\n", __func__, eid, idx, hdr);
 	hdr &= ~(SWITCHTEC_EVENT_EN_IRQ | SWITCHTEC_EVENT_OCCURRED);
 	iowrite32(hdr, hdr_reg);
@@ -1230,8 +1226,13 @@ static irqreturn_t switchtec_event_isr(int irq, void *dev)
 
 	check_link_state_events(stdev);
 
-	for (eid = 0; eid < SWITCHTEC_IOCTL_MAX_EVENTS; eid++)
+	for (eid = 0; eid < SWITCHTEC_IOCTL_MAX_EVENTS; eid++) {
+		if (eid == SWITCHTEC_IOCTL_EVENT_LINK_STATE ||
+		    eid == SWITCHTEC_IOCTL_EVENT_MRPC_COMP)
+			continue;
+
 		event_count += mask_all_events(stdev, eid);
+	}
 
 	if (event_count) {
 		atomic_inc(&stdev->event_cnt);
-- 
2.20.1

[PATCH 06/12] PCI/switchtec: Introduce Generation Variable

[Logan Gunthorpe]

Add a generation variable passed through the device id table.

This will allow us to add generation 4 and other devices after they are
functionally supported.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/switch/switchtec.c | 66 ++++++++++++++++++----------------
 include/linux/switchtec.h      |  7 ++++
 2 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 05d4cb49219b..674c57c9ae4c 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1420,6 +1420,8 @@ static int switchtec_pci_probe(struct pci_dev *pdev,
 	if (IS_ERR(stdev))
 		return PTR_ERR(stdev);
 
+	stdev->gen = id->driver_data;
+
 	rc = switchtec_init_pci(stdev, pdev);
 	if (rc)
 		goto err_put;
@@ -1467,7 +1469,7 @@ static void switchtec_pci_remove(struct pci_dev *pdev)
 	put_device(&stdev->dev);
 }
 
-#define SWITCHTEC_PCI_DEVICE(device_id) \
+#define SWITCHTEC_PCI_DEVICE(device_id, gen) \
 	{ \
 		.vendor     = PCI_VENDOR_ID_MICROSEMI, \
 		.device     = device_id, \
@@ -1475,6 +1477,7 @@ static void switchtec_pci_remove(struct pci_dev *pdev)
 		.subdevice  = PCI_ANY_ID, \
 		.class      = (PCI_CLASS_MEMORY_OTHER << 8), \
 		.class_mask = 0xFFFFFFFF, \
+		.driver_data = gen, \
 	}, \
 	{ \
 		.vendor     = PCI_VENDOR_ID_MICROSEMI, \
@@ -1483,39 +1486,40 @@ static void switchtec_pci_remove(struct pci_dev *pdev)
 		.subdevice  = PCI_ANY_ID, \
 		.class      = (PCI_CLASS_BRIDGE_OTHER << 8), \
 		.class_mask = 0xFFFFFFFF, \
+		.driver_data = gen, \
 	}
 
 static const struct pci_device_id switchtec_pci_tbl[] = {
-	SWITCHTEC_PCI_DEVICE(0x8531),  //PFX 24xG3
-	SWITCHTEC_PCI_DEVICE(0x8532),  //PFX 32xG3
-	SWITCHTEC_PCI_DEVICE(0x8533),  //PFX 48xG3
-	SWITCHTEC_PCI_DEVICE(0x8534),  //PFX 64xG3
-	SWITCHTEC_PCI_DEVICE(0x8535),  //PFX 80xG3
-	SWITCHTEC_PCI_DEVICE(0x8536),  //PFX 96xG3
-	SWITCHTEC_PCI_DEVICE(0x8541),  //PSX 24xG3
-	SWITCHTEC_PCI_DEVICE(0x8542),  //PSX 32xG3
-	SWITCHTEC_PCI_DEVICE(0x8543),  //PSX 48xG3
-	SWITCHTEC_PCI_DEVICE(0x8544),  //PSX 64xG3
-	SWITCHTEC_PCI_DEVICE(0x8545),  //PSX 80xG3
-	SWITCHTEC_PCI_DEVICE(0x8546),  //PSX 96xG3
-	SWITCHTEC_PCI_DEVICE(0x8551),  //PAX 24XG3
-	SWITCHTEC_PCI_DEVICE(0x8552),  //PAX 32XG3
-	SWITCHTEC_PCI_DEVICE(0x8553),  //PAX 48XG3
-	SWITCHTEC_PCI_DEVICE(0x8554),  //PAX 64XG3
-	SWITCHTEC_PCI_DEVICE(0x8555),  //PAX 80XG3
-	SWITCHTEC_PCI_DEVICE(0x8556),  //PAX 96XG3
-	SWITCHTEC_PCI_DEVICE(0x8561),  //PFXL 24XG3
-	SWITCHTEC_PCI_DEVICE(0x8562),  //PFXL 32XG3
-	SWITCHTEC_PCI_DEVICE(0x8563),  //PFXL 48XG3
-	SWITCHTEC_PCI_DEVICE(0x8564),  //PFXL 64XG3
-	SWITCHTEC_PCI_DEVICE(0x8565),  //PFXL 80XG3
-	SWITCHTEC_PCI_DEVICE(0x8566),  //PFXL 96XG3
-	SWITCHTEC_PCI_DEVICE(0x8571),  //PFXI 24XG3
-	SWITCHTEC_PCI_DEVICE(0x8572),  //PFXI 32XG3
-	SWITCHTEC_PCI_DEVICE(0x8573),  //PFXI 48XG3
-	SWITCHTEC_PCI_DEVICE(0x8574),  //PFXI 64XG3
-	SWITCHTEC_PCI_DEVICE(0x8575),  //PFXI 80XG3
-	SWITCHTEC_PCI_DEVICE(0x8576),  //PFXI 96XG3
+	SWITCHTEC_PCI_DEVICE(0x8531, SWITCHTEC_GEN3),  //PFX 24xG3
+	SWITCHTEC_PCI_DEVICE(0x8532, SWITCHTEC_GEN3),  //PFX 32xG3
+	SWITCHTEC_PCI_DEVICE(0x8533, SWITCHTEC_GEN3),  //PFX 48xG3
+	SWITCHTEC_PCI_DEVICE(0x8534, SWITCHTEC_GEN3),  //PFX 64xG3
+	SWITCHTEC_PCI_DEVICE(0x8535, SWITCHTEC_GEN3),  //PFX 80xG3
+	SWITCHTEC_PCI_DEVICE(0x8536, SWITCHTEC_GEN3),  //PFX 96xG3
+	SWITCHTEC_PCI_DEVICE(0x8541, SWITCHTEC_GEN3),  //PSX 24xG3
+	SWITCHTEC_PCI_DEVICE(0x8542, SWITCHTEC_GEN3),  //PSX 32xG3
+	SWITCHTEC_PCI_DEVICE(0x8543, SWITCHTEC_GEN3),  //PSX 48xG3
+	SWITCHTEC_PCI_DEVICE(0x8544, SWITCHTEC_GEN3),  //PSX 64xG3
+	SWITCHTEC_PCI_DEVICE(0x8545, SWITCHTEC_GEN3),  //PSX 80xG3
+	SWITCHTEC_PCI_DEVICE(0x8546, SWITCHTEC_GEN3),  //PSX 96xG3
+	SWITCHTEC_PCI_DEVICE(0x8551, SWITCHTEC_GEN3),  //PAX 24XG3
+	SWITCHTEC_PCI_DEVICE(0x8552, SWITCHTEC_GEN3),  //PAX 32XG3
+	SWITCHTEC_PCI_DEVICE(0x8553, SWITCHTEC_GEN3),  //PAX 48XG3
+	SWITCHTEC_PCI_DEVICE(0x8554, SWITCHTEC_GEN3),  //PAX 64XG3
+	SWITCHTEC_PCI_DEVICE(0x8555, SWITCHTEC_GEN3),  //PAX 80XG3
+	SWITCHTEC_PCI_DEVICE(0x8556, SWITCHTEC_GEN3),  //PAX 96XG3
+	SWITCHTEC_PCI_DEVICE(0x8561, SWITCHTEC_GEN3),  //PFXL 24XG3
+	SWITCHTEC_PCI_DEVICE(0x8562, SWITCHTEC_GEN3),  //PFXL 32XG3
+	SWITCHTEC_PCI_DEVICE(0x8563, SWITCHTEC_GEN3),  //PFXL 48XG3
+	SWITCHTEC_PCI_DEVICE(0x8564, SWITCHTEC_GEN3),  //PFXL 64XG3
+	SWITCHTEC_PCI_DEVICE(0x8565, SWITCHTEC_GEN3),  //PFXL 80XG3
+	SWITCHTEC_PCI_DEVICE(0x8566, SWITCHTEC_GEN3),  //PFXL 96XG3
+	SWITCHTEC_PCI_DEVICE(0x8571, SWITCHTEC_GEN3),  //PFXI 24XG3
+	SWITCHTEC_PCI_DEVICE(0x8572, SWITCHTEC_GEN3),  //PFXI 32XG3
+	SWITCHTEC_PCI_DEVICE(0x8573, SWITCHTEC_GEN3),  //PFXI 48XG3
+	SWITCHTEC_PCI_DEVICE(0x8574, SWITCHTEC_GEN3),  //PFXI 64XG3
+	SWITCHTEC_PCI_DEVICE(0x8575, SWITCHTEC_GEN3),  //PFXI 80XG3
+	SWITCHTEC_PCI_DEVICE(0x8576, SWITCHTEC_GEN3),  //PFXI 96XG3
 	{0}
 };
 MODULE_DEVICE_TABLE(pci, switchtec_pci_tbl);
diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
index b4ba3a38f30f..0963912c679d 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -32,6 +32,11 @@ enum {
 	SWITCHTEC_GAS_PFF_CSR_OFFSET    = 0x134000,
 };
 
+enum switchtec_gen {
+	SWITCHTEC_GEN3,
+	SWITCHTEC_GEN4,
+};
+
 struct mrpc_regs {
 	u8 input_data[SWITCHTEC_MRPC_PAYLOAD_SIZE];
 	u8 output_data[SWITCHTEC_MRPC_PAYLOAD_SIZE];
@@ -358,6 +363,8 @@ struct switchtec_dev {
 	struct device dev;
 	struct cdev cdev;
 
+	enum switchtec_gen gen;
+
 	int partition;
 	int partition_count;
 	int pff_csr_count;
-- 
2.20.1

[PATCH 07/12] PCI/switchtec: Separate out gen3 specific fields in the sys_info_regs structure

[Logan Gunthorpe]

Since gen4 system info region in GAS is diverged from gen3, separate
oute the gen3 specific stuff in preparation for adding a gen4 variant.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/switch/switchtec.c | 18 +++++++++---------
 include/linux/switchtec.h      | 12 ++++++++----
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 674c57c9ae4c..21d3dd6e74f9 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -317,8 +317,8 @@ static ssize_t field ## _show(struct device *dev, \
 	struct device_attribute *attr, char *buf) \
 { \
 	struct switchtec_dev *stdev = to_stdev(dev); \
-	return io_string_show(buf, &stdev->mmio_sys_info->field, \
-			    sizeof(stdev->mmio_sys_info->field)); \
+	return io_string_show(buf, &stdev->mmio_sys_info->gen3.field, \
+			    sizeof(stdev->mmio_sys_info->gen3.field)); \
 } \
 \
 static DEVICE_ATTR_RO(field)
@@ -332,7 +332,7 @@ static ssize_t component_id_show(struct device *dev,
 	struct device_attribute *attr, char *buf)
 {
 	struct switchtec_dev *stdev = to_stdev(dev);
-	int id = ioread16(&stdev->mmio_sys_info->component_id);
+	int id = ioread16(&stdev->mmio_sys_info->gen3.component_id);
 
 	return sprintf(buf, "PM%04X\n", id);
 }
@@ -342,7 +342,7 @@ static ssize_t component_revision_show(struct device *dev,
 	struct device_attribute *attr, char *buf)
 {
 	struct switchtec_dev *stdev = to_stdev(dev);
-	int rev = ioread8(&stdev->mmio_sys_info->component_revision);
+	int rev = ioread8(&stdev->mmio_sys_info->gen3.component_revision);
 
 	return sprintf(buf, "%d\n", rev);
 }
@@ -599,25 +599,25 @@ static int ioctl_flash_part_info(struct switchtec_dev *stdev,
 	case SWITCHTEC_IOCTL_PART_CFG0:
 		active_addr = ioread32(&fi->active_cfg);
 		set_fw_info_part(&info, &fi->cfg0);
-		if (ioread16(&si->cfg_running) == SWITCHTEC_CFG0_RUNNING)
+		if (ioread16(&si->gen3.cfg_running) == SWITCHTEC_CFG0_RUNNING)
 			info.active |= SWITCHTEC_IOCTL_PART_RUNNING;
 		break;
 	case SWITCHTEC_IOCTL_PART_CFG1:
 		active_addr = ioread32(&fi->active_cfg);
 		set_fw_info_part(&info, &fi->cfg1);
-		if (ioread16(&si->cfg_running) == SWITCHTEC_CFG1_RUNNING)
+		if (ioread16(&si->gen3.cfg_running) == SWITCHTEC_CFG1_RUNNING)
 			info.active |= SWITCHTEC_IOCTL_PART_RUNNING;
 		break;
 	case SWITCHTEC_IOCTL_PART_IMG0:
 		active_addr = ioread32(&fi->active_img);
 		set_fw_info_part(&info, &fi->img0);
-		if (ioread16(&si->img_running) == SWITCHTEC_IMG0_RUNNING)
+		if (ioread16(&si->gen3.img_running) == SWITCHTEC_IMG0_RUNNING)
 			info.active |= SWITCHTEC_IOCTL_PART_RUNNING;
 		break;
 	case SWITCHTEC_IOCTL_PART_IMG1:
 		active_addr = ioread32(&fi->active_img);
 		set_fw_info_part(&info, &fi->img1);
-		if (ioread16(&si->img_running) == SWITCHTEC_IMG1_RUNNING)
+		if (ioread16(&si->gen3.img_running) == SWITCHTEC_IMG1_RUNNING)
 			info.active |= SWITCHTEC_IOCTL_PART_RUNNING;
 		break;
 	case SWITCHTEC_IOCTL_PART_NVLOG:
@@ -1378,7 +1378,7 @@ static int switchtec_init_pci(struct switchtec_dev *stdev,
 	stdev->mmio_sys_info = stdev->mmio + SWITCHTEC_GAS_SYS_INFO_OFFSET;
 	stdev->mmio_flash_info = stdev->mmio + SWITCHTEC_GAS_FLASH_INFO_OFFSET;
 	stdev->mmio_ntb = stdev->mmio + SWITCHTEC_GAS_NTB_OFFSET;
-	stdev->partition = ioread8(&stdev->mmio_sys_info->partition_id);
+	stdev->partition = ioread8(&stdev->mmio_sys_info->gen3.partition_id);
 	stdev->partition_count = ioread8(&stdev->mmio_ntb->partition_count);
 	stdev->mmio_part_cfg_all = stdev->mmio + SWITCHTEC_GAS_PART_CFG_OFFSET;
 	stdev->mmio_part_cfg = &stdev->mmio_part_cfg_all[stdev->partition];
diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
index 0963912c679d..d6ba4b5dbbed 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -109,10 +109,7 @@ enum {
 	SWITCHTEC_IMG1_RUNNING = 0x07,
 };
 
-struct sys_info_regs {
-	u32 device_id;
-	u32 device_version;
-	u32 firmware_version;
+struct sys_info_regs_gen3 {
 	u32 reserved1;
 	u32 vendor_table_revision;
 	u32 table_format_version;
@@ -129,6 +126,13 @@ struct sys_info_regs {
 	u8 component_revision;
 } __packed;
 
+struct sys_info_regs {
+	u32 device_id;
+	u32 device_version;
+	u32 firmware_version;
+	struct sys_info_regs_gen3 gen3;
+} __packed;
+
 struct flash_info_regs {
 	u32 flash_part_map_upd_idx;
 
-- 
2.20.1

[PATCH 08/12] PCI/switchtec: Add gen4 support in struct sys_info_regs

[Logan Gunthorpe]

Add a union with gen3 and gen4 sys_info structs. Ensure any use of the
gen3 struct is gaurded by an if statement on stdev->gen.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/switch/switchtec.c | 41 +++++++++++++++++++++++++++---
 include/linux/switchtec.h      | 46 +++++++++++++++++++++++++++++++++-
 2 files changed, 82 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 21d3dd6e74f9..90d9c00984a7 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -317,8 +317,14 @@ static ssize_t field ## _show(struct device *dev, \
 	struct device_attribute *attr, char *buf) \
 { \
 	struct switchtec_dev *stdev = to_stdev(dev); \
-	return io_string_show(buf, &stdev->mmio_sys_info->gen3.field, \
-			    sizeof(stdev->mmio_sys_info->gen3.field)); \
+	struct sys_info_regs __iomem *si = stdev->mmio_sys_info; \
+	\
+	if (stdev->gen == SWITCHTEC_GEN4) \
+		return io_string_show(buf, &si->gen4.field, \
+				      sizeof(si->gen4.field)); \
+	else \
+		return io_string_show(buf, &si->gen3.field, \
+				      sizeof(si->gen3.field)); \
 } \
 \
 static DEVICE_ATTR_RO(field)
@@ -326,7 +332,21 @@ static DEVICE_ATTR_RO(field)
 DEVICE_ATTR_SYS_INFO_STR(vendor_id);
 DEVICE_ATTR_SYS_INFO_STR(product_id);
 DEVICE_ATTR_SYS_INFO_STR(product_revision);
-DEVICE_ATTR_SYS_INFO_STR(component_vendor);
+
+static ssize_t component_vendor_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct switchtec_dev *stdev = to_stdev(dev);
+	struct sys_info_regs __iomem *si = stdev->mmio_sys_info;
+
+	/* component_vendor field not supported after gen4 */
+	if (stdev->gen != SWITCHTEC_GEN3)
+		return sprintf(buf, "none\n");
+
+	return io_string_show(buf, &si->gen3.component_vendor,
+			      sizeof(si->gen3.component_vendor));
+}
+static DEVICE_ATTR_RO(component_vendor);
 
 static ssize_t component_id_show(struct device *dev,
 	struct device_attribute *attr, char *buf)
@@ -334,6 +354,10 @@ static ssize_t component_id_show(struct device *dev,
 	struct switchtec_dev *stdev = to_stdev(dev);
 	int id = ioread16(&stdev->mmio_sys_info->gen3.component_id);
 
+	/* component_id field not supported after gen4 */
+	if (stdev->gen != SWITCHTEC_GEN3)
+		return sprintf(buf, "none\n");
+
 	return sprintf(buf, "PM%04X\n", id);
 }
 static DEVICE_ATTR_RO(component_id);
@@ -344,6 +368,10 @@ static ssize_t component_revision_show(struct device *dev,
 	struct switchtec_dev *stdev = to_stdev(dev);
 	int rev = ioread8(&stdev->mmio_sys_info->gen3.component_revision);
 
+	/* component_revision field not supported after gen4 */
+	if (stdev->gen != SWITCHTEC_GEN3)
+		return sprintf(buf, "255\n");
+
 	return sprintf(buf, "%d\n", rev);
 }
 static DEVICE_ATTR_RO(component_revision);
@@ -1344,6 +1372,7 @@ static int switchtec_init_pci(struct switchtec_dev *stdev,
 	int rc;
 	void __iomem *map;
 	unsigned long res_start, res_len;
+	u32 __iomem *part_id;
 
 	rc = pcim_enable_device(pdev);
 	if (rc)
@@ -1378,7 +1407,11 @@ static int switchtec_init_pci(struct switchtec_dev *stdev,
 	stdev->mmio_sys_info = stdev->mmio + SWITCHTEC_GAS_SYS_INFO_OFFSET;
 	stdev->mmio_flash_info = stdev->mmio + SWITCHTEC_GAS_FLASH_INFO_OFFSET;
 	stdev->mmio_ntb = stdev->mmio + SWITCHTEC_GAS_NTB_OFFSET;
-	stdev->partition = ioread8(&stdev->mmio_sys_info->gen3.partition_id);
+	if (stdev->gen == SWITCHTEC_GEN4)
+		part_id = &stdev->mmio_sys_info->gen4.partition_id;
+	else
+		part_id = &stdev->mmio_sys_info->gen3.partition_id;
+	stdev->partition = ioread32(part_id);
 	stdev->partition_count = ioread8(&stdev->mmio_ntb->partition_count);
 	stdev->mmio_part_cfg_all = stdev->mmio + SWITCHTEC_GAS_PART_CFG_OFFSET;
 	stdev->mmio_part_cfg = &stdev->mmio_part_cfg_all[stdev->partition];
diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
index d6ba4b5dbbed..0677581eacad 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -126,11 +126,55 @@ struct sys_info_regs_gen3 {
 	u8 component_revision;
 } __packed;
 
+struct sys_info_regs_gen4 {
+	u16 gas_layout_ver;
+	u8 evlist_ver;
+	u8 reserved1;
+	u16 mgmt_cmd_set_ver;
+	u16 fabric_cmd_set_ver;
+	u32 reserved2[2];
+	u8 mrpc_uart_ver;
+	u8 mrpc_twi_ver;
+	u8 mrpc_eth_ver;
+	u8 mrpc_inband_ver;
+	u32 reserved3[7];
+	u32 fw_update_tmo;
+	u32 xml_version_cfg;
+	u32 xml_version_img;
+	u32 partition_id;
+	u16 bl2_running;
+	u16 cfg_running;
+	u16 img_running;
+	u16 key_running;
+	u32 reserved4[43];
+	u32 vendor_seeprom_twi;
+	u32 vendor_table_revision;
+	u32 vendor_specific_info[2];
+	u16 p2p_vendor_id;
+	u16 p2p_device_id;
+	u8 p2p_revision_id;
+	u8 reserved5[3];
+	u32 p2p_class_id;
+	u16 subsystem_vendor_id;
+	u16 subsystem_id;
+	u32 p2p_serial_number[2];
+	u8 mac_addr[6];
+	u8 reserved6[2];
+	u32 reserved7[3];
+	char vendor_id[8];
+	char product_id[24];
+	char  product_revision[2];
+	u16 reserved8;
+} __packed;
+
 struct sys_info_regs {
 	u32 device_id;
 	u32 device_version;
 	u32 firmware_version;
-	struct sys_info_regs_gen3 gen3;
+	union {
+		struct sys_info_regs_gen3 gen3;
+		struct sys_info_regs_gen4 gen4;
+	};
 } __packed;
 
 struct flash_info_regs {
-- 
2.20.1

Re: [PATCH 08/12] PCI/switchtec: Add gen4 support in struct sys_info_regs

[Bjorn Helgaas]

On Mon, Jan 06, 2020 at 12:03:33PM -0700, Logan Gunthorpe wrote:
> Add a union with gen3 and gen4 sys_info structs. Ensure any use of the
> gen3 struct is gaurded by an if statement on stdev->gen.

s/gaurded/guarded/

> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---
>  drivers/pci/switch/switchtec.c | 41 +++++++++++++++++++++++++++---
>  include/linux/switchtec.h      | 46 +++++++++++++++++++++++++++++++++-
>  2 files changed, 82 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index 21d3dd6e74f9..90d9c00984a7 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -317,8 +317,14 @@ static ssize_t field ## _show(struct device *dev, \
>  	struct device_attribute *attr, char *buf) \
>  { \
>  	struct switchtec_dev *stdev = to_stdev(dev); \
> -	return io_string_show(buf, &stdev->mmio_sys_info->gen3.field, \
> -			    sizeof(stdev->mmio_sys_info->gen3.field)); \
> +	struct sys_info_regs __iomem *si = stdev->mmio_sys_info; \
> +	\
> +	if (stdev->gen == SWITCHTEC_GEN4) \
> +		return io_string_show(buf, &si->gen4.field, \
> +				      sizeof(si->gen4.field)); \
> +	else \
> +		return io_string_show(buf, &si->gen3.field, \
> +				      sizeof(si->gen3.field)); \
>  } \
>  \
>  static DEVICE_ATTR_RO(field)
> @@ -326,7 +332,21 @@ static DEVICE_ATTR_RO(field)
>  DEVICE_ATTR_SYS_INFO_STR(vendor_id);
>  DEVICE_ATTR_SYS_INFO_STR(product_id);
>  DEVICE_ATTR_SYS_INFO_STR(product_revision);
> -DEVICE_ATTR_SYS_INFO_STR(component_vendor);
> +
> +static ssize_t component_vendor_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct switchtec_dev *stdev = to_stdev(dev);
> +	struct sys_info_regs __iomem *si = stdev->mmio_sys_info;
> +
> +	/* component_vendor field not supported after gen4 */
> +	if (stdev->gen != SWITCHTEC_GEN3)

I assume the comment should say "after gen3"?  More occurrences below.

> +		return sprintf(buf, "none\n");
> +
> +	return io_string_show(buf, &si->gen3.component_vendor,
> +			      sizeof(si->gen3.component_vendor));
> +}
> +static DEVICE_ATTR_RO(component_vendor);
>  
>  static ssize_t component_id_show(struct device *dev,
>  	struct device_attribute *attr, char *buf)
> @@ -334,6 +354,10 @@ static ssize_t component_id_show(struct device *dev,
>  	struct switchtec_dev *stdev = to_stdev(dev);
>  	int id = ioread16(&stdev->mmio_sys_info->gen3.component_id);
>  
> +	/* component_id field not supported after gen4 */
> +	if (stdev->gen != SWITCHTEC_GEN3)
> +		return sprintf(buf, "none\n");
> +
>  	return sprintf(buf, "PM%04X\n", id);
>  }
>  static DEVICE_ATTR_RO(component_id);
> @@ -344,6 +368,10 @@ static ssize_t component_revision_show(struct device *dev,
>  	struct switchtec_dev *stdev = to_stdev(dev);
>  	int rev = ioread8(&stdev->mmio_sys_info->gen3.component_revision);
>  
> +	/* component_revision field not supported after gen4 */
> +	if (stdev->gen != SWITCHTEC_GEN3)
> +		return sprintf(buf, "255\n");
> +

[PATCH 09/12] PCI/switchtec: Add gen4 support in struct flash_info_regs

[Logan Gunthorpe]

From: Kelvin Cao <kelvin.cao@microchip.com>

Add a union with gen3 and gen4 flash_info structs.

Signed-off-by: Kelvin Cao <kelvin.cao@microchip.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/switch/switchtec.c       | 200 ++++++++++++++++++++++-----
 include/linux/switchtec.h            |  85 ++++++++++--
 include/uapi/linux/switchtec_ioctl.h |   9 ++
 3 files changed, 247 insertions(+), 47 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 90d9c00984a7..524cb4e4bbf7 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -596,8 +596,15 @@ static int ioctl_flash_info(struct switchtec_dev *stdev,
 	struct switchtec_ioctl_flash_info info = {0};
 	struct flash_info_regs __iomem *fi = stdev->mmio_flash_info;
 
-	info.flash_length = ioread32(&fi->flash_length);
-	info.num_partitions = SWITCHTEC_IOCTL_NUM_PARTITIONS;
+	if (stdev->gen == SWITCHTEC_GEN3) {
+		info.flash_length = ioread32(&fi->gen3.flash_length);
+		info.num_partitions = SWITCHTEC_NUM_PARTITIONS_GEN3;
+	} else if (stdev->gen == SWITCHTEC_GEN4) {
+		info.flash_length = ioread32(&fi->gen4.flash_length);
+		info.num_partitions = SWITCHTEC_NUM_PARTITIONS_GEN4;
+	} else {
+		return -ENOTSUPP;
+	}
 
 	if (copy_to_user(uinfo, &info, sizeof(info)))
 		return -EFAULT;
@@ -612,75 +619,200 @@ static void set_fw_info_part(struct switchtec_ioctl_flash_part_info *info,
 	info->length = ioread32(&pi->length);
 }
 
-static int ioctl_flash_part_info(struct switchtec_dev *stdev,
-	struct switchtec_ioctl_flash_part_info __user *uinfo)
+static int flash_part_info_gen3(struct switchtec_dev *stdev,
+	struct switchtec_ioctl_flash_part_info *info)
 {
-	struct switchtec_ioctl_flash_part_info info = {0};
-	struct flash_info_regs __iomem *fi = stdev->mmio_flash_info;
-	struct sys_info_regs __iomem *si = stdev->mmio_sys_info;
+	struct flash_info_regs_gen3 __iomem *fi = &stdev->mmio_flash_info->gen3;
+	struct sys_info_regs_gen3 __iomem *si = &stdev->mmio_sys_info->gen3;
 	u32 active_addr = -1;
 
-	if (copy_from_user(&info, uinfo, sizeof(info)))
-		return -EFAULT;
-
-	switch (info.flash_partition) {
+	switch (info->flash_partition) {
 	case SWITCHTEC_IOCTL_PART_CFG0:
 		active_addr = ioread32(&fi->active_cfg);
-		set_fw_info_part(&info, &fi->cfg0);
-		if (ioread16(&si->gen3.cfg_running) == SWITCHTEC_CFG0_RUNNING)
-			info.active |= SWITCHTEC_IOCTL_PART_RUNNING;
+		set_fw_info_part(info, &fi->cfg0);
+		if (ioread16(&si->cfg_running) == SWITCHTEC_GEN3_CFG0_RUNNING)
+			info->active |= SWITCHTEC_IOCTL_PART_RUNNING;
 		break;
 	case SWITCHTEC_IOCTL_PART_CFG1:
 		active_addr = ioread32(&fi->active_cfg);
-		set_fw_info_part(&info, &fi->cfg1);
-		if (ioread16(&si->gen3.cfg_running) == SWITCHTEC_CFG1_RUNNING)
-			info.active |= SWITCHTEC_IOCTL_PART_RUNNING;
+		set_fw_info_part(info, &fi->cfg1);
+		if (ioread16(&si->cfg_running) == SWITCHTEC_GEN3_CFG1_RUNNING)
+			info->active |= SWITCHTEC_IOCTL_PART_RUNNING;
 		break;
 	case SWITCHTEC_IOCTL_PART_IMG0:
 		active_addr = ioread32(&fi->active_img);
-		set_fw_info_part(&info, &fi->img0);
-		if (ioread16(&si->gen3.img_running) == SWITCHTEC_IMG0_RUNNING)
-			info.active |= SWITCHTEC_IOCTL_PART_RUNNING;
+		set_fw_info_part(info, &fi->img0);
+		if (ioread16(&si->img_running) == SWITCHTEC_GEN3_IMG0_RUNNING)
+			info->active |= SWITCHTEC_IOCTL_PART_RUNNING;
 		break;
 	case SWITCHTEC_IOCTL_PART_IMG1:
 		active_addr = ioread32(&fi->active_img);
-		set_fw_info_part(&info, &fi->img1);
-		if (ioread16(&si->gen3.img_running) == SWITCHTEC_IMG1_RUNNING)
-			info.active |= SWITCHTEC_IOCTL_PART_RUNNING;
+		set_fw_info_part(info, &fi->img1);
+		if (ioread16(&si->img_running) == SWITCHTEC_GEN3_IMG1_RUNNING)
+			info->active |= SWITCHTEC_IOCTL_PART_RUNNING;
 		break;
 	case SWITCHTEC_IOCTL_PART_NVLOG:
-		set_fw_info_part(&info, &fi->nvlog);
+		set_fw_info_part(info, &fi->nvlog);
 		break;
 	case SWITCHTEC_IOCTL_PART_VENDOR0:
-		set_fw_info_part(&info, &fi->vendor[0]);
+		set_fw_info_part(info, &fi->vendor[0]);
 		break;
 	case SWITCHTEC_IOCTL_PART_VENDOR1:
-		set_fw_info_part(&info, &fi->vendor[1]);
+		set_fw_info_part(info, &fi->vendor[1]);
 		break;
 	case SWITCHTEC_IOCTL_PART_VENDOR2:
-		set_fw_info_part(&info, &fi->vendor[2]);
+		set_fw_info_part(info, &fi->vendor[2]);
 		break;
 	case SWITCHTEC_IOCTL_PART_VENDOR3:
-		set_fw_info_part(&info, &fi->vendor[3]);
+		set_fw_info_part(info, &fi->vendor[3]);
 		break;
 	case SWITCHTEC_IOCTL_PART_VENDOR4:
-		set_fw_info_part(&info, &fi->vendor[4]);
+		set_fw_info_part(info, &fi->vendor[4]);
 		break;
 	case SWITCHTEC_IOCTL_PART_VENDOR5:
-		set_fw_info_part(&info, &fi->vendor[5]);
+		set_fw_info_part(info, &fi->vendor[5]);
 		break;
 	case SWITCHTEC_IOCTL_PART_VENDOR6:
-		set_fw_info_part(&info, &fi->vendor[6]);
+		set_fw_info_part(info, &fi->vendor[6]);
 		break;
 	case SWITCHTEC_IOCTL_PART_VENDOR7:
-		set_fw_info_part(&info, &fi->vendor[7]);
+		set_fw_info_part(info, &fi->vendor[7]);
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	if (info.address == active_addr)
-		info.active |= SWITCHTEC_IOCTL_PART_ACTIVE;
+	if (info->address == active_addr)
+		info->active |= SWITCHTEC_IOCTL_PART_ACTIVE;
+
+	return 0;
+}
+
+static int flash_part_info_gen4(struct switchtec_dev *stdev,
+	struct switchtec_ioctl_flash_part_info *info)
+{
+	struct flash_info_regs_gen4 __iomem *fi = &stdev->mmio_flash_info->gen4;
+	struct sys_info_regs_gen4 __iomem *si = &stdev->mmio_sys_info->gen4;
+	struct active_partition_info_gen4 __iomem *af = &fi->active_flag;
+
+	switch (info->flash_partition) {
+	case SWITCHTEC_IOCTL_PART_MAP_0:
+		set_fw_info_part(info, &fi->map0);
+		break;
+	case SWITCHTEC_IOCTL_PART_MAP_1:
+		set_fw_info_part(info, &fi->map1);
+		break;
+	case SWITCHTEC_IOCTL_PART_KEY_0:
+		set_fw_info_part(info, &fi->key0);
+		if (ioread8(&af->key) == SWITCHTEC_GEN4_KEY0_ACTIVE)
+			info->active |= SWITCHTEC_IOCTL_PART_ACTIVE;
+		if (ioread16(&si->key_running) == SWITCHTEC_GEN4_KEY0_RUNNING)
+			info->active |= SWITCHTEC_IOCTL_PART_RUNNING;
+		break;
+	case SWITCHTEC_IOCTL_PART_KEY_1:
+		set_fw_info_part(info, &fi->key1);
+		if (ioread8(&af->key) == SWITCHTEC_GEN4_KEY1_ACTIVE)
+			info->active |= SWITCHTEC_IOCTL_PART_ACTIVE;
+		if (ioread16(&si->key_running) == SWITCHTEC_GEN4_KEY1_RUNNING)
+			info->active |= SWITCHTEC_IOCTL_PART_RUNNING;
+		break;
+	case SWITCHTEC_IOCTL_PART_BL2_0:
+		set_fw_info_part(info, &fi->bl2_0);
+		if (ioread8(&af->bl2) == SWITCHTEC_GEN4_BL2_0_ACTIVE)
+			info->active |= SWITCHTEC_IOCTL_PART_ACTIVE;
+		if (ioread16(&si->bl2_running) == SWITCHTEC_GEN4_BL2_0_RUNNING)
+			info->active |= SWITCHTEC_IOCTL_PART_RUNNING;
+		break;
+	case SWITCHTEC_IOCTL_PART_BL2_1:
+		set_fw_info_part(info, &fi->bl2_1);
+		if (ioread8(&af->bl2) == SWITCHTEC_GEN4_BL2_1_ACTIVE)
+			info->active |= SWITCHTEC_IOCTL_PART_ACTIVE;
+		if (ioread16(&si->bl2_running) == SWITCHTEC_GEN4_BL2_1_RUNNING)
+			info->active |= SWITCHTEC_IOCTL_PART_RUNNING;
+		break;
+	case SWITCHTEC_IOCTL_PART_CFG0:
+		set_fw_info_part(info, &fi->cfg0);
+		if (ioread8(&af->cfg) == SWITCHTEC_GEN4_CFG0_ACTIVE)
+			info->active |= SWITCHTEC_IOCTL_PART_ACTIVE;
+		if (ioread16(&si->cfg_running) == SWITCHTEC_GEN4_CFG0_RUNNING)
+			info->active |= SWITCHTEC_IOCTL_PART_RUNNING;
+		break;
+	case SWITCHTEC_IOCTL_PART_CFG1:
+		set_fw_info_part(info, &fi->cfg1);
+		if (ioread8(&af->cfg) == SWITCHTEC_GEN4_CFG1_ACTIVE)
+			info->active |= SWITCHTEC_IOCTL_PART_ACTIVE;
+		if (ioread16(&si->cfg_running) == SWITCHTEC_GEN4_CFG1_RUNNING)
+			info->active |= SWITCHTEC_IOCTL_PART_RUNNING;
+		break;
+	case SWITCHTEC_IOCTL_PART_IMG0:
+		set_fw_info_part(info, &fi->img0);
+		if (ioread8(&af->img) == SWITCHTEC_GEN4_IMG0_ACTIVE)
+			info->active |= SWITCHTEC_IOCTL_PART_ACTIVE;
+		if (ioread16(&si->img_running) == SWITCHTEC_GEN4_IMG0_RUNNING)
+			info->active |= SWITCHTEC_IOCTL_PART_RUNNING;
+		break;
+	case SWITCHTEC_IOCTL_PART_IMG1:
+		set_fw_info_part(info, &fi->img1);
+		if (ioread8(&af->img) == SWITCHTEC_GEN4_IMG1_ACTIVE)
+			info->active |= SWITCHTEC_IOCTL_PART_ACTIVE;
+		if (ioread16(&si->img_running) == SWITCHTEC_GEN4_IMG1_RUNNING)
+			info->active |= SWITCHTEC_IOCTL_PART_RUNNING;
+		break;
+	case SWITCHTEC_IOCTL_PART_NVLOG:
+		set_fw_info_part(info, &fi->nvlog);
+		break;
+	case SWITCHTEC_IOCTL_PART_VENDOR0:
+		set_fw_info_part(info, &fi->vendor[0]);
+		break;
+	case SWITCHTEC_IOCTL_PART_VENDOR1:
+		set_fw_info_part(info, &fi->vendor[1]);
+		break;
+	case SWITCHTEC_IOCTL_PART_VENDOR2:
+		set_fw_info_part(info, &fi->vendor[2]);
+		break;
+	case SWITCHTEC_IOCTL_PART_VENDOR3:
+		set_fw_info_part(info, &fi->vendor[3]);
+		break;
+	case SWITCHTEC_IOCTL_PART_VENDOR4:
+		set_fw_info_part(info, &fi->vendor[4]);
+		break;
+	case SWITCHTEC_IOCTL_PART_VENDOR5:
+		set_fw_info_part(info, &fi->vendor[5]);
+		break;
+	case SWITCHTEC_IOCTL_PART_VENDOR6:
+		set_fw_info_part(info, &fi->vendor[6]);
+		break;
+	case SWITCHTEC_IOCTL_PART_VENDOR7:
+		set_fw_info_part(info, &fi->vendor[7]);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int ioctl_flash_part_info(struct switchtec_dev *stdev,
+	struct switchtec_ioctl_flash_part_info __user *uinfo)
+{
+	int ret;
+	struct switchtec_ioctl_flash_part_info info = {0};
+
+	if (copy_from_user(&info, uinfo, sizeof(info)))
+		return -EFAULT;
+
+	if (stdev->gen == SWITCHTEC_GEN3) {
+		ret = flash_part_info_gen3(stdev, &info);
+		if (ret)
+			return ret;
+	} else if (stdev->gen == SWITCHTEC_GEN4) {
+		ret = flash_part_info_gen4(stdev, &info);
+		if (ret)
+			return ret;
+	} else {
+		return -ENOTSUPP;
+	}
+
 
 	if (copy_to_user(uinfo, &info, sizeof(info)))
 		return -EFAULT;
diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
index 0677581eacad..e85155244135 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -103,10 +103,34 @@ struct sw_event_regs {
 } __packed;
 
 enum {
-	SWITCHTEC_CFG0_RUNNING = 0x04,
-	SWITCHTEC_CFG1_RUNNING = 0x05,
-	SWITCHTEC_IMG0_RUNNING = 0x03,
-	SWITCHTEC_IMG1_RUNNING = 0x07,
+	SWITCHTEC_GEN3_CFG0_RUNNING = 0x04,
+	SWITCHTEC_GEN3_CFG1_RUNNING = 0x05,
+	SWITCHTEC_GEN3_IMG0_RUNNING = 0x03,
+	SWITCHTEC_GEN3_IMG1_RUNNING = 0x07,
+};
+
+enum {
+	SWITCHTEC_GEN4_MAP0_RUNNING = 0x00,
+	SWITCHTEC_GEN4_MAP1_RUNNING = 0x01,
+	SWITCHTEC_GEN4_KEY0_RUNNING = 0x02,
+	SWITCHTEC_GEN4_KEY1_RUNNING = 0x03,
+	SWITCHTEC_GEN4_BL2_0_RUNNING = 0x04,
+	SWITCHTEC_GEN4_BL2_1_RUNNING = 0x05,
+	SWITCHTEC_GEN4_CFG0_RUNNING = 0x06,
+	SWITCHTEC_GEN4_CFG1_RUNNING = 0x07,
+	SWITCHTEC_GEN4_IMG0_RUNNING = 0x08,
+	SWITCHTEC_GEN4_IMG1_RUNNING = 0x09,
+};
+
+enum {
+	SWITCHTEC_GEN4_KEY0_ACTIVE = 0,
+	SWITCHTEC_GEN4_KEY1_ACTIVE = 1,
+	SWITCHTEC_GEN4_BL2_0_ACTIVE = 0,
+	SWITCHTEC_GEN4_BL2_1_ACTIVE = 1,
+	SWITCHTEC_GEN4_CFG0_ACTIVE = 0,
+	SWITCHTEC_GEN4_CFG1_ACTIVE = 1,
+	SWITCHTEC_GEN4_IMG0_ACTIVE = 0,
+	SWITCHTEC_GEN4_IMG1_ACTIVE = 1,
 };
 
 struct sys_info_regs_gen3 {
@@ -177,26 +201,54 @@ struct sys_info_regs {
 	};
 } __packed;
 
-struct flash_info_regs {
+struct partition_info {
+	u32 address;
+	u32 length;
+};
+
+struct flash_info_regs_gen3 {
 	u32 flash_part_map_upd_idx;
 
-	struct active_partition_info {
+	struct active_partition_info_gen3 {
 		u32 address;
 		u32 build_version;
 		u32 build_string;
 	} active_img;
 
-	struct active_partition_info active_cfg;
-	struct active_partition_info inactive_img;
-	struct active_partition_info inactive_cfg;
+	struct active_partition_info_gen3 active_cfg;
+	struct active_partition_info_gen3 inactive_img;
+	struct active_partition_info_gen3 inactive_cfg;
 
 	u32 flash_length;
 
-	struct partition_info {
-		u32 address;
-		u32 length;
-	} cfg0;
+	struct partition_info cfg0;
+	struct partition_info cfg1;
+	struct partition_info img0;
+	struct partition_info img1;
+	struct partition_info nvlog;
+	struct partition_info vendor[8];
+};
+
+struct flash_info_regs_gen4 {
+	u32 flash_address;
+	u32 flash_length;
 
+	struct active_partition_info_gen4 {
+		unsigned char bl2;
+		unsigned char cfg;
+		unsigned char img;
+		unsigned char key;
+	} active_flag;
+
+	u32 reserved[3];
+
+	struct partition_info map0;
+	struct partition_info map1;
+	struct partition_info key0;
+	struct partition_info key1;
+	struct partition_info bl2_0;
+	struct partition_info bl2_1;
+	struct partition_info cfg0;
 	struct partition_info cfg1;
 	struct partition_info img0;
 	struct partition_info img1;
@@ -204,6 +256,13 @@ struct flash_info_regs {
 	struct partition_info vendor[8];
 };
 
+struct flash_info_regs {
+	union {
+		struct flash_info_regs_gen3 gen3;
+		struct flash_info_regs_gen4 gen4;
+	};
+};
+
 enum {
 	SWITCHTEC_NTB_REG_INFO_OFFSET   = 0x0000,
 	SWITCHTEC_NTB_REG_CTRL_OFFSET   = 0x4000,
diff --git a/include/uapi/linux/switchtec_ioctl.h b/include/uapi/linux/switchtec_ioctl.h
index e8db938985ca..0d28f4a3dd0a 100644
--- a/include/uapi/linux/switchtec_ioctl.h
+++ b/include/uapi/linux/switchtec_ioctl.h
@@ -32,7 +32,16 @@
 #define SWITCHTEC_IOCTL_PART_VENDOR5	10
 #define SWITCHTEC_IOCTL_PART_VENDOR6	11
 #define SWITCHTEC_IOCTL_PART_VENDOR7	12
+#define SWITCHTEC_IOCTL_PART_BL2_0	13
+#define SWITCHTEC_IOCTL_PART_BL2_1	14
+#define SWITCHTEC_IOCTL_PART_MAP_0	15
+#define SWITCHTEC_IOCTL_PART_MAP_1	16
+#define SWITCHTEC_IOCTL_PART_KEY_0	17
+#define SWITCHTEC_IOCTL_PART_KEY_1	18
+
 #define SWITCHTEC_IOCTL_NUM_PARTITIONS	13
+#define SWITCHTEC_NUM_PARTITIONS_GEN3	13
+#define SWITCHTEC_NUM_PARTITIONS_GEN4	19
 
 struct switchtec_ioctl_flash_info {
 	__u64 flash_length;
-- 
2.20.1

Re: [PATCH 09/12] PCI/switchtec: Add gen4 support in struct flash_info_regs

[Bjorn Helgaas]

On Mon, Jan 06, 2020 at 12:03:34PM -0700, Logan Gunthorpe wrote:
> From: Kelvin Cao <kelvin.cao@microchip.com>
> 
> Add a union with gen3 and gen4 flash_info structs.

This does a lot more than add a union :)

I think this looks reasonable, but I would like it even better if this
and related patches could be split up a little bit differently:

  - Rename SWITCHTEC_CFG0_RUNNING to SWITCHTEC_GEN3_CFG0_RUNNING, etc
    (purely mechanical change, so trivial and obvious).

  - Add switchtec_gen and the tests where it's needed, but with only
    SWITCHTEC_GEN3 cases for now.

  - Refactor ioctl_flash_part_info() (still only supports GEN3).
    Maybe adds struct flash_info_regs and union, but only with gen3.

  - Add GEN4 support (patch basically contains only GEN4-related
    things and doesn't touch GEN3 things at all).  Maybe it would
    still make sense to split the GEN4 support into multiple patches
    (as in this series), or maybe they could be squashed into a single
    GEN4 patch?

  - It seems like at least the aliasing quirk and the driver device ID
    update could/should be squashed since they contain the same
    constants.

Bjorn

Re: [PATCH 09/12] PCI/switchtec: Add gen4 support in struct flash_info_regs

[Logan Gunthorpe]

On 2020-01-08 2:23 p.m., Bjorn Helgaas wrote:
> On Mon, Jan 06, 2020 at 12:03:34PM -0700, Logan Gunthorpe wrote:
>> From: Kelvin Cao <kelvin.cao@microchip.com>
>>
>> Add a union with gen3 and gen4 flash_info structs.
> 
> This does a lot more than add a union :)
> 
> I think this looks reasonable, but I would like it even better if this
> and related patches could be split up a little bit differently:
> 
>   - Rename SWITCHTEC_CFG0_RUNNING to SWITCHTEC_GEN3_CFG0_RUNNING, etc
>     (purely mechanical change, so trivial and obvious).
> 
>   - Add switchtec_gen and the tests where it's needed, but with only
>     SWITCHTEC_GEN3 cases for now.
> 
>   - Refactor ioctl_flash_part_info() (still only supports GEN3).
>     Maybe adds struct flash_info_regs and union, but only with gen3.
> 
>   - Add GEN4 support (patch basically contains only GEN4-related
>     things and doesn't touch GEN3 things at all).  Maybe it would
>     still make sense to split the GEN4 support into multiple patches
>     (as in this series), or maybe they could be squashed into a single
>     GEN4 patch?
> 
>   - It seems like at least the aliasing quirk and the driver device ID
>     update could/should be squashed since they contain the same
>     constants.

Thanks for the review. Yes, I should be able to clean this up and submit
a v2 in the next week or two.

Logan

[PATCH 10/12] PCI/switchtec: Add permission check for the GAS access MRPC commands

[Logan Gunthorpe]

From: Kelvin Cao <kelvin.cao@microchip.com>

GEN4 hardware provides new MRPC commands to read and write from
directly from any address in the PCI BAR (which Microsemi refers to
as GAS). Seeing accessing BAR registers can be dangerous and break
the driver, we don't want unpriviliged users to have this ability.

Therefore, for the local and remote GAS access MRPC commands, the
requesting process should need CAP_SYS_ADMIN. Priviligded processes
will already have access to the bar through the sysfs resource file
so this doesn't give userspace any capabilities it didn't already have.

Signed-off-by: Kelvin Cao <kelvin.cao@microchip.com>
[logang@deltatee.com: rework commit message]
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/switch/switchtec.c | 6 ++++++
 include/linux/switchtec.h      | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 524cb4e4bbf7..990e0ee32f7b 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -478,6 +478,12 @@ static ssize_t switchtec_dev_write(struct file *filp, const char __user *data,
 		rc = -EFAULT;
 		goto out;
 	}
+	if (((MRPC_CMD_ID(stuser->cmd) == MRPC_GAS_WRITE) ||
+	     (MRPC_CMD_ID(stuser->cmd) == MRPC_GAS_READ)) &&
+	    !capable(CAP_SYS_ADMIN)) {
+		rc = -EPERM;
+		goto out;
+	}
 
 	data += sizeof(stuser->cmd);
 	rc = copy_from_user(&stuser->data, data, size - sizeof(stuser->cmd));
diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
index e85155244135..1c3e76b535a2 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -21,6 +21,11 @@
 #define SWITCHTEC_EVENT_FATAL    BIT(4)
 
 #define SWITCHTEC_DMA_MRPC_EN	BIT(0)
+
+#define MRPC_GAS_READ 0x29
+#define MRPC_GAS_WRITE 0x87
+#define MRPC_CMD_ID(x) ((x) & 0xffff)
+
 enum {
 	SWITCHTEC_GAS_MRPC_OFFSET       = 0x0000,
 	SWITCHTEC_GAS_TOP_CFG_OFFSET    = 0x1000,
-- 
2.20.1

[PATCH 11/12] PCI/switchtec: Introduce gen4 variant IDS in the device ID table

[Logan Gunthorpe]

From: Kelvin Cao <kelvin.cao@microchip.com>

Now that GEN4 is properly supported, advertise support in the module's
device ID table.

Signed-off-by: Kelvin Cao <kelvin.cao@microchip.com>
[logang@deltatee.com: add commit message]
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/switch/switchtec.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 990e0ee32f7b..db240b70fbdd 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1691,6 +1691,24 @@ static const struct pci_device_id switchtec_pci_tbl[] = {
 	SWITCHTEC_PCI_DEVICE(0x8574, SWITCHTEC_GEN3),  //PFXI 64XG3
 	SWITCHTEC_PCI_DEVICE(0x8575, SWITCHTEC_GEN3),  //PFXI 80XG3
 	SWITCHTEC_PCI_DEVICE(0x8576, SWITCHTEC_GEN3),  //PFXI 96XG3
+	SWITCHTEC_PCI_DEVICE(0x4000, SWITCHTEC_GEN4),  //PFX 100XG4
+	SWITCHTEC_PCI_DEVICE(0x4084, SWITCHTEC_GEN4),  //PFX 84XG4
+	SWITCHTEC_PCI_DEVICE(0x4068, SWITCHTEC_GEN4),  //PFX 68XG4
+	SWITCHTEC_PCI_DEVICE(0x4052, SWITCHTEC_GEN4),  //PFX 52XG4
+	SWITCHTEC_PCI_DEVICE(0x4036, SWITCHTEC_GEN4),  //PFX 36XG4
+	SWITCHTEC_PCI_DEVICE(0x4028, SWITCHTEC_GEN4),  //PFX 28XG4
+	SWITCHTEC_PCI_DEVICE(0x4100, SWITCHTEC_GEN4),  //PSX 100XG4
+	SWITCHTEC_PCI_DEVICE(0x4184, SWITCHTEC_GEN4),  //PSX 84XG4
+	SWITCHTEC_PCI_DEVICE(0x4168, SWITCHTEC_GEN4),  //PSX 68XG4
+	SWITCHTEC_PCI_DEVICE(0x4152, SWITCHTEC_GEN4),  //PSX 52XG4
+	SWITCHTEC_PCI_DEVICE(0x4136, SWITCHTEC_GEN4),  //PSX 36XG4
+	SWITCHTEC_PCI_DEVICE(0x4128, SWITCHTEC_GEN4),  //PSX 28XG4
+	SWITCHTEC_PCI_DEVICE(0x4200, SWITCHTEC_GEN4),  //PAX 100XG4
+	SWITCHTEC_PCI_DEVICE(0x4284, SWITCHTEC_GEN4),  //PAX 84XG4
+	SWITCHTEC_PCI_DEVICE(0x4268, SWITCHTEC_GEN4),  //PAX 68XG4
+	SWITCHTEC_PCI_DEVICE(0x4252, SWITCHTEC_GEN4),  //PAX 52XG4
+	SWITCHTEC_PCI_DEVICE(0x4236, SWITCHTEC_GEN4),  //PAX 36XG4
+	SWITCHTEC_PCI_DEVICE(0x4228, SWITCHTEC_GEN4),  //PAX 28XG4
 	{0}
 };
 MODULE_DEVICE_TABLE(pci, switchtec_pci_tbl);
-- 
2.20.1

[PATCH 12/12] PCI: Apply switchtec DMA aliasing quirk to GEN4 devices

[Logan Gunthorpe]

Add GEN4 device IDs for quirk_switchtec_ntb_dma_alias().

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/quirks.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 4937a088d7d8..d54692bc3af4 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5373,6 +5373,24 @@ SWITCHTEC_QUIRK(0x8573);  /* PFXI 48XG3 */
 SWITCHTEC_QUIRK(0x8574);  /* PFXI 64XG3 */
 SWITCHTEC_QUIRK(0x8575);  /* PFXI 80XG3 */
 SWITCHTEC_QUIRK(0x8576);  /* PFXI 96XG3 */
+SWITCHTEC_QUIRK(0x4000);  /* PFX 100XG4 */
+SWITCHTEC_QUIRK(0x4084);  /* PFX 84XG4  */
+SWITCHTEC_QUIRK(0x4068);  /* PFX 68XG4  */
+SWITCHTEC_QUIRK(0x4052);  /* PFX 52XG4  */
+SWITCHTEC_QUIRK(0x4036);  /* PFX 36XG4  */
+SWITCHTEC_QUIRK(0x4028);  /* PFX 28XG4  */
+SWITCHTEC_QUIRK(0x4100);  /* PSX 100XG4 */
+SWITCHTEC_QUIRK(0x4184);  /* PSX 84XG4  */
+SWITCHTEC_QUIRK(0x4168);  /* PSX 68XG4  */
+SWITCHTEC_QUIRK(0x4152);  /* PSX 52XG4  */
+SWITCHTEC_QUIRK(0x4136);  /* PSX 36XG4  */
+SWITCHTEC_QUIRK(0x4128);  /* PSX 28XG4  */
+SWITCHTEC_QUIRK(0x4200);  /* PAX 100XG4 */
+SWITCHTEC_QUIRK(0x4284);  /* PAX 84XG4  */
+SWITCHTEC_QUIRK(0x4268);  /* PAX 68XG4  */
+SWITCHTEC_QUIRK(0x4252);  /* PAX 52XG4  */
+SWITCHTEC_QUIRK(0x4236);  /* PAX 36XG4  */
+SWITCHTEC_QUIRK(0x4228);  /* PAX 28XG4  */
 
 /*
  * On Lenovo Thinkpad P50 SKUs with a Nvidia Quadro M1000M, the BIOS does
-- 
2.20.1

Re: [PATCH 00/12] Switchtec Fixes and Gen4 Support

[Bjorn Helgaas]

On Mon, Jan 06, 2020 at 12:03:25PM -0700, Logan Gunthorpe wrote:
> Hi,
> 
> Please find a bunch of patches for the switchtec driver collected over the
> last few months.
> 
> The first 2 patches fix a couple of minor bugs. Patch 3 adds support for
> a new event that is available in specific firmware versions. Patches 4 and
> 5 are some code cleanup changes to simplify the logic. And the last 6
> patches implement support for the new Gen4 hardware.
> 
> This patchset is based on v5.5-rc5 and a git branch is available here:
> 
> https://github.com/sbates130272/linux-p2pmem switchtec-next
> 
> Thanks,
> 
> Logan
> 
> --
> 
> Kelvin Cao (3):
>   PCI/switchtec: Add gen4 support in struct flash_info_regs
>   PCI/switchtec: Add permission check for the GAS access MRPC commands
>   PCI/switchtec: Introduce gen4 variant IDS in the device ID table
> 
> Logan Gunthorpe (6):
>   PCI/switchtec: Fix vep_vector_number ioread width
>   PCI/switchtec: Add support for new events
>   PCI/switchtec: Introduce Generation Variable
>   PCI/switchtec: Separate out gen3 specific fields in the sys_info_regs
>     structure
>   PCI/switchtec: Add gen4 support in struct sys_info_regs
>   PCI: Apply switchtec DMA aliasing quirk to GEN4 devices
> 
> Wesley Sheng (3):
>   PCI/switchtec: Use dma_set_mask_and_coherent()
>   PCI/switchtec: Remove redundant valid PFF number count
>   PCI/switchtec: Move check event id from mask_event() to
>     switchtec_event_isr()

Current order is:

  [PATCH 01/12] PCI/switchtec: Use dma_set_mask_and_coherent()
  [PATCH 02/12] PCI/switchtec: Fix vep_vector_number ioread width
  [PATCH 03/12] PCI/switchtec: Add support for new events
  [PATCH 04/12] PCI/switchtec: Remove redundant valid PFF number count
  [PATCH 05/12] PCI/switchtec: Move check event id from mask_event() to switchtec_event_isr()
  [PATCH 06/12] PCI/switchtec: Introduce Generation Variable
  [PATCH 07/12] PCI/switchtec: Separate out gen3 specific fields in the sys_info_regs structure
  [PATCH 08/12] PCI/switchtec: Add gen4 support in struct sys_info_regs
  [PATCH 09/12] PCI/switchtec: Add gen4 support in struct flash_info_regs
  [PATCH 10/12] PCI/switchtec: Add permission check for the GAS access MRPC commands

10/12 looks lonely in the middle of the gen4 stuff, and it looks like
it's unrelated to gen3/gen4?  Maybe it could be moved up after 05/12?

  [PATCH 11/12] PCI/switchtec: Introduce gen4 variant IDS in the device ID table
  [PATCH 12/12] PCI: Apply switchtec DMA aliasing quirk to GEN4 devices

I speculatively reordered the permission check patch and applied these
to my pci/switchtec branch for v5.6 (reverse order from "git log"):

  b96abab6314f ("PCI/switchtec: Add permission check for the GAS access MRPC commands")
  5f23367bd4df ("PCI/switchtec: Move check event ID from mask_event() to switchtec_event_isr()")
  6722d609bc82 ("PCI/switchtec: Remove redundant valid PFF number count")
  3f3a521ecc81 ("PCI/switchtec: Add support for intercom notify and UEC Port")
  9375646b4cf0 ("PCI/switchtec: Fix vep_vector_number ioread width")
  aa82130a22f7 ("PCI/switchtec: Use dma_set_mask_and_coherent()")

If you rework any of the subsequent ones, you can just post those
without reposting these first six.

Bjorn

Re: [PATCH 00/12] Switchtec Fixes and Gen4 Support

[Logan Gunthorpe]

On 2020-01-08 2:47 p.m., Bjorn Helgaas wrote:
> On Mon, Jan 06, 2020 at 12:03:25PM -0700, Logan Gunthorpe wrote:
>> Hi,
>>
>> Please find a bunch of patches for the switchtec driver collected over the
>> last few months.
>>
>> The first 2 patches fix a couple of minor bugs. Patch 3 adds support for
>> a new event that is available in specific firmware versions. Patches 4 and
>> 5 are some code cleanup changes to simplify the logic. And the last 6
>> patches implement support for the new Gen4 hardware.
>>
>> This patchset is based on v5.5-rc5 and a git branch is available here:
>>
>> https://github.com/sbates130272/linux-p2pmem switchtec-next
>>
>> Thanks,
>>
>> Logan
>>
>> --
>>
>> Kelvin Cao (3):
>>   PCI/switchtec: Add gen4 support in struct flash_info_regs
>>   PCI/switchtec: Add permission check for the GAS access MRPC commands
>>   PCI/switchtec: Introduce gen4 variant IDS in the device ID table
>>
>> Logan Gunthorpe (6):
>>   PCI/switchtec: Fix vep_vector_number ioread width
>>   PCI/switchtec: Add support for new events
>>   PCI/switchtec: Introduce Generation Variable
>>   PCI/switchtec: Separate out gen3 specific fields in the sys_info_regs
>>     structure
>>   PCI/switchtec: Add gen4 support in struct sys_info_regs
>>   PCI: Apply switchtec DMA aliasing quirk to GEN4 devices
>>
>> Wesley Sheng (3):
>>   PCI/switchtec: Use dma_set_mask_and_coherent()
>>   PCI/switchtec: Remove redundant valid PFF number count
>>   PCI/switchtec: Move check event id from mask_event() to
>>     switchtec_event_isr()
> 
> Current order is:
> 
>   [PATCH 01/12] PCI/switchtec: Use dma_set_mask_and_coherent()
>   [PATCH 02/12] PCI/switchtec: Fix vep_vector_number ioread width
>   [PATCH 03/12] PCI/switchtec: Add support for new events
>   [PATCH 04/12] PCI/switchtec: Remove redundant valid PFF number count
>   [PATCH 05/12] PCI/switchtec: Move check event id from mask_event() to switchtec_event_isr()
>   [PATCH 06/12] PCI/switchtec: Introduce Generation Variable
>   [PATCH 07/12] PCI/switchtec: Separate out gen3 specific fields in the sys_info_regs structure
>   [PATCH 08/12] PCI/switchtec: Add gen4 support in struct sys_info_regs
>   [PATCH 09/12] PCI/switchtec: Add gen4 support in struct flash_info_regs
>   [PATCH 10/12] PCI/switchtec: Add permission check for the GAS access MRPC commands
> 
> 10/12 looks lonely in the middle of the gen4 stuff, and it looks like
> it's unrelated to gen3/gen4?  Maybe it could be moved up after 05/12?

Yes, sort of: It's related to GEN4 because the GAS access MRPC command
is introduced in GEN4. So it won't really have any effect until after
the GEN4 IDs are added. But there is no harm in applying it earlier.

> I speculatively reordered the permission check patch and applied these
> to my pci/switchtec branch for v5.6 (reverse order from "git log"):
> 
>   b96abab6314f ("PCI/switchtec: Add permission check for the GAS access MRPC commands")
>   5f23367bd4df ("PCI/switchtec: Move check event ID from mask_event() to switchtec_event_isr()")
>   6722d609bc82 ("PCI/switchtec: Remove redundant valid PFF number count")
>   3f3a521ecc81 ("PCI/switchtec: Add support for intercom notify and UEC Port")
>   9375646b4cf0 ("PCI/switchtec: Fix vep_vector_number ioread width")
>   aa82130a22f7 ("PCI/switchtec: Use dma_set_mask_and_coherent()")
> 
> If you rework any of the subsequent ones, you can just post those
> without reposting these first six.

Sounds good, thanks!

Logan