On 2019-04-17 4:48 p.m., Bjorn Helgaas wrote:
---
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(&stdev->mmio_sw_event->global_summary);
- s.part_bitmap = ioread32(&stdev->mmio_sw_event->part_event_bitmap);
- s.local_part = ioread32(&stdev->mmio_part_cfg->part_event_summary);
+ s = kzalloc(sizeof(*s), GFP_KERNEL);
+ if (!s)
+ return -ENOMEM;
+
+ s->global = ioread32(&stdev->mmio_sw_event->global_summary);
+ s->part_bitmap = ioread32(&stdev->mmio_sw_event->part_event_bitmap);
+ s->local_part = ioread32(&stdev->mmio_part_cfg->part_event_summary);
for (i = 0; i < stdev->partition_count; i++) {
reg = ioread32(&stdev->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++) {
Should this be "i < stdev->pff_csr_count", as in
check_link_state_events(), enable_link_state_events() and
mask_all_events()? If so, I assume the read and check of vendor_id
would be unnecessary?
Yes, nice catch. I think that would be a good simplification.
The last loop in init_pff() currently checks against
SWITCHTEC_MAX_PFF_CSR:
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)
stdev->pff_local[reg] = 1;
}
Should it check "reg < stdev->pff_csr_count" instead? It looks like
mask_all_events(), the only reader of pff_local[], only looks up to
pff_csr_count anyway.
Yeah, I could go either way. The hardware would have to be broken if reg
was between pff_csr_count and SWITCHTEC_MAX_PFF_CSR. This is mostly to
catch 0xFF which indicates it's unset (if I recall correctly).
Logan
