Re: [PATCH 1/2] switchtec: Fix false maximum supported PCIe function number issue
On 2019-04-09 4:36 p.m., Bjorn Helgaas wrote: > Hi Wesley, > > On Mon, Apr 08, 2019 at 10:34:47PM +0800, Wesley Sheng wrote: >> The hardware supports up to 255 PFFs and the driver only supports 48, so >> this patch updates the driver to support them all. >> To be backward compatible, a new ioctl and corresponding data >> structure are created, while keep the deprecated one. > > The above is either one paragraph that needs to be rewrapped, or two > paragraphs that need a blank line between. Wesley can address this. > What's a PFF? PFF is really a concept internal to the Switchtec device. It stands for PCIe Function Framework. Essentially, there is a bank of registers for every PCIe Function (aka endpoint) in the switch. When I originally wrote the driver, I assumed incorrectly there would only ever be one PFF per port and the maximum number of ports for Switchtec parts is 48. In fact, the hardware supports up to 255 and there are typically two PFFs per upstream port (one for the port itself and one for the management endpoint). Logan
Re: [PATCH 1/2] switchtec: Fix false maximum supported PCIe function number issue
Hi Wesley,
On Mon, Apr 08, 2019 at 10:34:47PM +0800, Wesley Sheng wrote:
> The hardware supports up to 255 PFFs and the driver only supports 48, so
> this patch updates the driver to support them all.
> To be backward compatible, a new ioctl and corresponding data
> structure are created, while keep the deprecated one.
The above is either one paragraph that needs to be rewrapped, or two
paragraphs that need a blank line between.
What's a PFF?
> Signed-off-by: Wesley Sheng
> ---
> drivers/pci/switch/switchtec.c | 39
> +---
> include/linux/switchtec.h| 2 +-
> include/uapi/linux/switchtec_ioctl.h | 13 +++-
> 3 files changed, 40 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index e22766c..7df9a69 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -658,19 +658,25 @@ static int ioctl_flash_part_info(struct switchtec_dev
> *stdev,
>
> static int ioctl_event_summary(struct switchtec_dev *stdev,
> struct switchtec_user *stuser,
> - struct switchtec_ioctl_event_summary __user *usum)
> + struct switchtec_ioctl_event_summary __user *usum,
> + size_t size)
> {
> - struct switchtec_ioctl_event_summary s = {0};
> + struct switchtec_ioctl_event_summary *s;
> int i;
> u32 reg;
> + int ret = 0;
>
> - s.global = ioread32(>mmio_sw_event->global_summary);
> - s.part_bitmap = ioread32(>mmio_sw_event->part_event_bitmap);
> - s.local_part = ioread32(>mmio_part_cfg->part_event_summary);
> + s = kzalloc(sizeof(*s), GFP_KERNEL);
> + if (!s)
> + return -ENOMEM;
> +
> + s->global = ioread32(>mmio_sw_event->global_summary);
> + s->part_bitmap = ioread32(>mmio_sw_event->part_event_bitmap);
> + s->local_part = ioread32(>mmio_part_cfg->part_event_summary);
>
> for (i = 0; i < stdev->partition_count; i++) {
> reg = ioread32(>mmio_part_cfg_all[i].part_event_summary);
> - s.part[i] = reg;
> + s->part[i] = reg;
> }
>
> for (i = 0; i < SWITCHTEC_MAX_PFF_CSR; i++) {
> @@ -679,15 +685,19 @@ static int ioctl_event_summary(struct switchtec_dev
> *stdev,
> break;
>
> reg = ioread32(>mmio_pff_csr[i].pff_event_summary);
> - s.pff[i] = reg;
> + s->pff[i] = reg;
> }
>
> - if (copy_to_user(usum, , sizeof(s)))
> - return -EFAULT;
> + if (copy_to_user(usum, s, size)) {
> + ret = -EFAULT;
> + goto error_case;
> + }
>
> stuser->event_cnt = atomic_read(>event_cnt);
>
> - return 0;
> +error_case:
> + kfree(s);
> + return ret;
> }
>
> static u32 __iomem *global_ev_reg(struct switchtec_dev *stdev,
> @@ -977,8 +987,9 @@ static long switchtec_dev_ioctl(struct file *filp,
> unsigned int cmd,
> case SWITCHTEC_IOCTL_FLASH_PART_INFO:
> rc = ioctl_flash_part_info(stdev, argp);
> break;
> - case SWITCHTEC_IOCTL_EVENT_SUMMARY:
> - rc = ioctl_event_summary(stdev, stuser, argp);
> + case SWITCHTEC_IOCTL_EVENT_SUMMARY_LEGACY:
> + rc = ioctl_event_summary(stdev, stuser, argp,
> + sizeof(struct
> switchtec_ioctl_event_summary_legacy));
> break;
> case SWITCHTEC_IOCTL_EVENT_CTL:
> rc = ioctl_event_ctl(stdev, argp);
> @@ -989,6 +1000,10 @@ static long switchtec_dev_ioctl(struct file *filp,
> unsigned int cmd,
> case SWITCHTEC_IOCTL_PORT_TO_PFF:
> rc = ioctl_port_to_pff(stdev, argp);
> break;
> + case SWITCHTEC_IOCTL_EVENT_SUMMARY:
> + rc = ioctl_event_summary(stdev, stuser, argp,
> + sizeof(struct
> switchtec_ioctl_event_summary));
> + break;
> default:
> rc = -ENOTTY;
> break;
> diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
> index 52a079b..0cfc34a 100644
> --- a/include/linux/switchtec.h
> +++ b/include/linux/switchtec.h
> @@ -20,7 +20,7 @@
> #include
>
> #define SWITCHTEC_MRPC_PAYLOAD_SIZE 1024
> -#define SWITCHTEC_MAX_PFF_CSR 48
> +#define SWITCHTEC_MAX_PFF_CSR 255
>
> #define SWITCHTEC_EVENT_OCCURRED BIT(0)
> #define SWITCHTEC_EVENT_CLEARBIT(0)
> diff --git a/include/uapi/linux/switchtec_ioctl.h
> b/include/uapi/linux/switchtec_ioctl.h
> index 4f4daf8..c912b5a 100644
> --- a/include/uapi/linux/switchtec_ioctl.h
> +++ b/include/uapi/linux/switchtec_ioctl.h
> @@ -50,7 +50,7 @@ struct switchtec_ioctl_flash_part_info {
> __u32 active;
> };
>
> -struct switchtec_ioctl_event_summary {
> +struct switchtec_ioctl_event_summary_legacy {
> __u64 global;
> __u64 part_bitmap;
> __u32 local_part;
> @@ -59,6 +59,15 @@ struct switchtec_ioctl_event_summary {
> __u32 pff[48];
> };
>
>
[PATCH 1/2] switchtec: Fix false maximum supported PCIe function number issue
The hardware supports up to 255 PFFs and the driver only supports 48, so
this patch updates the driver to support them all.
To be backward compatible, a new ioctl and corresponding data
structure are created, while keep the deprecated one.
Signed-off-by: Wesley Sheng
---
drivers/pci/switch/switchtec.c | 39 +---
include/linux/switchtec.h| 2 +-
include/uapi/linux/switchtec_ioctl.h | 13 +++-
3 files changed, 40 insertions(+), 14 deletions(-)
diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index e22766c..7df9a69 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -658,19 +658,25 @@ static int ioctl_flash_part_info(struct switchtec_dev
*stdev,
static int ioctl_event_summary(struct switchtec_dev *stdev,
struct switchtec_user *stuser,
- struct switchtec_ioctl_event_summary __user *usum)
+ struct switchtec_ioctl_event_summary __user *usum,
+ size_t size)
{
- struct switchtec_ioctl_event_summary s = {0};
+ struct switchtec_ioctl_event_summary *s;
int i;
u32 reg;
+ int ret = 0;
- s.global = ioread32(>mmio_sw_event->global_summary);
- s.part_bitmap = ioread32(>mmio_sw_event->part_event_bitmap);
- s.local_part = ioread32(>mmio_part_cfg->part_event_summary);
+ s = kzalloc(sizeof(*s), GFP_KERNEL);
+ if (!s)
+ return -ENOMEM;
+
+ s->global = ioread32(>mmio_sw_event->global_summary);
+ s->part_bitmap = ioread32(>mmio_sw_event->part_event_bitmap);
+ s->local_part = ioread32(>mmio_part_cfg->part_event_summary);
for (i = 0; i < stdev->partition_count; i++) {
reg = ioread32(>mmio_part_cfg_all[i].part_event_summary);
- s.part[i] = reg;
+ s->part[i] = reg;
}
for (i = 0; i < SWITCHTEC_MAX_PFF_CSR; i++) {
@@ -679,15 +685,19 @@ static int ioctl_event_summary(struct switchtec_dev
*stdev,
break;
reg = ioread32(>mmio_pff_csr[i].pff_event_summary);
- s.pff[i] = reg;
+ s->pff[i] = reg;
}
- if (copy_to_user(usum, , sizeof(s)))
- return -EFAULT;
+ if (copy_to_user(usum, s, size)) {
+ ret = -EFAULT;
+ goto error_case;
+ }
stuser->event_cnt = atomic_read(>event_cnt);
- return 0;
+error_case:
+ kfree(s);
+ return ret;
}
static u32 __iomem *global_ev_reg(struct switchtec_dev *stdev,
@@ -977,8 +987,9 @@ static long switchtec_dev_ioctl(struct file *filp, unsigned
int cmd,
case SWITCHTEC_IOCTL_FLASH_PART_INFO:
rc = ioctl_flash_part_info(stdev, argp);
break;
- case SWITCHTEC_IOCTL_EVENT_SUMMARY:
- rc = ioctl_event_summary(stdev, stuser, argp);
+ case SWITCHTEC_IOCTL_EVENT_SUMMARY_LEGACY:
+ rc = ioctl_event_summary(stdev, stuser, argp,
+sizeof(struct
switchtec_ioctl_event_summary_legacy));
break;
case SWITCHTEC_IOCTL_EVENT_CTL:
rc = ioctl_event_ctl(stdev, argp);
@@ -989,6 +1000,10 @@ static long switchtec_dev_ioctl(struct file *filp,
unsigned int cmd,
case SWITCHTEC_IOCTL_PORT_TO_PFF:
rc = ioctl_port_to_pff(stdev, argp);
break;
+ case SWITCHTEC_IOCTL_EVENT_SUMMARY:
+ rc = ioctl_event_summary(stdev, stuser, argp,
+sizeof(struct
switchtec_ioctl_event_summary));
+ break;
default:
rc = -ENOTTY;
break;
diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
index 52a079b..0cfc34a 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -20,7 +20,7 @@
#include
#define SWITCHTEC_MRPC_PAYLOAD_SIZE 1024
-#define SWITCHTEC_MAX_PFF_CSR 48
+#define SWITCHTEC_MAX_PFF_CSR 255
#define SWITCHTEC_EVENT_OCCURRED BIT(0)
#define SWITCHTEC_EVENT_CLEARBIT(0)
diff --git a/include/uapi/linux/switchtec_ioctl.h
b/include/uapi/linux/switchtec_ioctl.h
index 4f4daf8..c912b5a 100644
--- a/include/uapi/linux/switchtec_ioctl.h
+++ b/include/uapi/linux/switchtec_ioctl.h
@@ -50,7 +50,7 @@ struct switchtec_ioctl_flash_part_info {
__u32 active;
};
-struct switchtec_ioctl_event_summary {
+struct switchtec_ioctl_event_summary_legacy {
__u64 global;
__u64 part_bitmap;
__u32 local_part;
@@ -59,6 +59,15 @@ struct switchtec_ioctl_event_summary {
__u32 pff[48];
};
+struct switchtec_ioctl_event_summary {
+ __u64 global;
+ __u64 part_bitmap;
+ __u32 local_part;
+ __u32 padding;
+ __u32 part[48];
+ __u32 pff[255];
+};
+
#define SWITCHTEC_IOCTL_EVENT_STACK_ERROR 0
#define SWITCHTEC_IOCTL_EVENT_PPU_ERROR1
#define
