2020-01-06 19:05:14

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH 00/12] Switchtec Fixes and Gen4 Support

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


2020-01-06 19:05:21

by Logan Gunthorpe

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

From: Wesley Sheng <[email protected]>

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 <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
---
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

2020-01-06 19:05:44

by Logan Gunthorpe

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

Add GEN4 device IDs for quirk_switchtec_ntb_dma_alias().

Signed-off-by: Logan Gunthorpe <[email protected]>
---
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

2020-01-06 19:05:51

by Logan Gunthorpe

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

From: Kelvin Cao <[email protected]>

Add a union with gen3 and gen4 flash_info structs.

Signed-off-by: Kelvin Cao <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
---
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

2020-01-06 19:05:58

by Logan Gunthorpe

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

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 <[email protected]>
---
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

2020-01-06 19:06:03

by Logan Gunthorpe

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

From: Wesley Sheng <[email protected]>

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 <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
---
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

2020-01-06 19:06:04

by Logan Gunthorpe

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

From: Wesley Sheng <[email protected]>

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 <[email protected]>
[[email protected]: reworked commit message]
Signed-off-by: Logan Gunthorpe <[email protected]>
---
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

2020-01-06 19:06:05

by Logan Gunthorpe

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

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 <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
---
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

2020-01-06 19:06:24

by Logan Gunthorpe

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

From: Kelvin Cao <[email protected]>

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 <[email protected]>
[[email protected]: rework commit message]
Signed-off-by: Logan Gunthorpe <[email protected]>
---
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

2020-01-08 21:22:43

by Bjorn Helgaas

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

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 <[email protected]>
> ---
> 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");
> +

2020-01-08 21:25:45

by Bjorn Helgaas

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

On Mon, Jan 06, 2020 at 12:03:34PM -0700, Logan Gunthorpe wrote:
> From: Kelvin Cao <[email protected]>
>
> 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

2020-01-08 21:36:04

by Logan Gunthorpe

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



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 <[email protected]>
>>
>> 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

2020-01-08 21:48:25

by Bjorn Helgaas

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

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

2020-01-08 21:54:26

by Logan Gunthorpe

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



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