lkml.org 
[lkml]   [2019]   [Apr]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2 1/2] switchtec: Fix false maximum supported PCIe function number issue


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

\
 
 \ /
  Last update: 2019-04-18 17:50    [W:0.101 / U:7.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site