All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] PCI/switchtec: Read all 64 bits of part_event_bitmap
@ 2019-12-19 18:27 Logan Gunthorpe
  2020-01-01 17:04 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 7+ messages in thread
From: Logan Gunthorpe @ 2019-12-19 18:27 UTC (permalink / raw
  To: Greg Kroah-Hartman
  Cc: Logan Gunthorpe, Doug Meyer, Bjorn Helgaas, stable, Kelvin Cao

commit 6acdf7e19b37cb3a9258603d0eab315079c19c5e upstream.

The part_event_bitmap register is 64 bits wide, so read it with ioread64()
instead of the 32-bit ioread32().

Fixes: 52eabba5bcdb ("switchtec: Add IOCTLs to the Switchtec driver")
Link: https://lore.kernel.org/r/20190910195833.3891-1-logang@deltatee.com
Reported-by: Doug Meyer <dmeyer@gigaio.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: stable@vger.kernel.org	# v4.12+
Cc: Kelvin Cao <Kelvin.Cao@microchip.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---

ioread64() was introduced in v5.1 so the upstream patch won't compile on
stable versions 4.14 or 4.19. This is the same patch but uses readq()
which should be equivalent.

 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 e3aefdafae89..7a788b759c86 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -898,7 +898,7 @@ static int ioctl_event_summary(struct switchtec_dev *stdev,
 	u32 reg;

 	s.global = ioread32(&stdev->mmio_sw_event->global_summary);
-	s.part_bitmap = ioread32(&stdev->mmio_sw_event->part_event_bitmap);
+	s.part_bitmap = readq(&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++) {
--
2.20.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI/switchtec: Read all 64 bits of part_event_bitmap
  2019-12-19 18:27 [PATCH] PCI/switchtec: Read all 64 bits of part_event_bitmap Logan Gunthorpe
@ 2020-01-01 17:04 ` Greg Kroah-Hartman
  2020-01-03  0:18   ` Sasha Levin
  0 siblings, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2020-01-01 17:04 UTC (permalink / raw
  To: Logan Gunthorpe; +Cc: Doug Meyer, Bjorn Helgaas, stable, Kelvin Cao

On Thu, Dec 19, 2019 at 11:27:47AM -0700, Logan Gunthorpe wrote:
> commit 6acdf7e19b37cb3a9258603d0eab315079c19c5e upstream.
> 
> The part_event_bitmap register is 64 bits wide, so read it with ioread64()
> instead of the 32-bit ioread32().
> 
> Fixes: 52eabba5bcdb ("switchtec: Add IOCTLs to the Switchtec driver")
> Link: https://lore.kernel.org/r/20190910195833.3891-1-logang@deltatee.com
> Reported-by: Doug Meyer <dmeyer@gigaio.com>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: stable@vger.kernel.org	# v4.12+
> Cc: Kelvin Cao <Kelvin.Cao@microchip.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> 
> ioread64() was introduced in v5.1 so the upstream patch won't compile on
> stable versions 4.14 or 4.19. This is the same patch but uses readq()
> which should be equivalent.

Now queued up, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI/switchtec: Read all 64 bits of part_event_bitmap
  2020-01-01 17:04 ` Greg Kroah-Hartman
@ 2020-01-03  0:18   ` Sasha Levin
  2020-01-03  0:46     ` Logan Gunthorpe
  0 siblings, 1 reply; 7+ messages in thread
From: Sasha Levin @ 2020-01-03  0:18 UTC (permalink / raw
  To: Greg Kroah-Hartman
  Cc: Logan Gunthorpe, Doug Meyer, Bjorn Helgaas, stable, Kelvin Cao

On Wed, Jan 01, 2020 at 06:04:06PM +0100, Greg Kroah-Hartman wrote:
>On Thu, Dec 19, 2019 at 11:27:47AM -0700, Logan Gunthorpe wrote:
>> commit 6acdf7e19b37cb3a9258603d0eab315079c19c5e upstream.
>>
>> The part_event_bitmap register is 64 bits wide, so read it with ioread64()
>> instead of the 32-bit ioread32().
>>
>> Fixes: 52eabba5bcdb ("switchtec: Add IOCTLs to the Switchtec driver")
>> Link: https://lore.kernel.org/r/20190910195833.3891-1-logang@deltatee.com
>> Reported-by: Doug Meyer <dmeyer@gigaio.com>
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: stable@vger.kernel.org	# v4.12+
>> Cc: Kelvin Cao <Kelvin.Cao@microchip.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> ---
>>
>> ioread64() was introduced in v5.1 so the upstream patch won't compile on
>> stable versions 4.14 or 4.19. This is the same patch but uses readq()
>> which should be equivalent.
>
>Now queued up, thanks.

Hey Logan,

As Guenter has pointed out, readq() is only defined for 64 bits, so this
breaks compilation in i386. I've dropped this backport for now, if you
could fix it up we could queue it up again.

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI/switchtec: Read all 64 bits of part_event_bitmap
  2020-01-03  0:18   ` Sasha Levin
@ 2020-01-03  0:46     ` Logan Gunthorpe
  2020-01-03  3:31       ` Sasha Levin
  0 siblings, 1 reply; 7+ messages in thread
From: Logan Gunthorpe @ 2020-01-03  0:46 UTC (permalink / raw
  To: Sasha Levin, Greg Kroah-Hartman
  Cc: Doug Meyer, Bjorn Helgaas, stable, Kelvin Cao



On 2020-01-02 5:18 p.m., Sasha Levin wrote:
> On Wed, Jan 01, 2020 at 06:04:06PM +0100, Greg Kroah-Hartman wrote:
>> On Thu, Dec 19, 2019 at 11:27:47AM -0700, Logan Gunthorpe wrote:
>>> commit 6acdf7e19b37cb3a9258603d0eab315079c19c5e upstream.
>>>
>>> The part_event_bitmap register is 64 bits wide, so read it with
>>> ioread64()
>>> instead of the 32-bit ioread32().
>>>
>>> Fixes: 52eabba5bcdb ("switchtec: Add IOCTLs to the Switchtec driver")
>>> Link:
>>> https://lore.kernel.org/r/20190910195833.3891-1-logang@deltatee.com
>>> Reported-by: Doug Meyer <dmeyer@gigaio.com>
>>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>>> Cc: stable@vger.kernel.org    # v4.12+
>>> Cc: Kelvin Cao <Kelvin.Cao@microchip.com>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> ---
>>>
>>> ioread64() was introduced in v5.1 so the upstream patch won't compile on
>>> stable versions 4.14 or 4.19. This is the same patch but uses readq()
>>> which should be equivalent.
>>
>> Now queued up, thanks.
> 
> Hey Logan,
> 
> As Guenter has pointed out, readq() is only defined for 64 bits, so this
> breaks compilation in i386. I've dropped this backport for now, if you
> could fix it up we could queue it up again.

Not quiet true. It is in fact defined for 32-bit architectures as two
readl() calls in "linux/io-64-nonatomic-lo-hi.h".

So, unless I'm missing something the patch should be fine.

Logan




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI/switchtec: Read all 64 bits of part_event_bitmap
  2020-01-03  0:46     ` Logan Gunthorpe
@ 2020-01-03  3:31       ` Sasha Levin
  2020-01-03  5:43         ` Logan Gunthorpe
  0 siblings, 1 reply; 7+ messages in thread
From: Sasha Levin @ 2020-01-03  3:31 UTC (permalink / raw
  To: Logan Gunthorpe
  Cc: Greg Kroah-Hartman, Doug Meyer, Bjorn Helgaas, stable, Kelvin Cao

On Thu, Jan 02, 2020 at 05:46:58PM -0700, Logan Gunthorpe wrote:
>
>
>On 2020-01-02 5:18 p.m., Sasha Levin wrote:
>> On Wed, Jan 01, 2020 at 06:04:06PM +0100, Greg Kroah-Hartman wrote:
>>> On Thu, Dec 19, 2019 at 11:27:47AM -0700, Logan Gunthorpe wrote:
>>>> commit 6acdf7e19b37cb3a9258603d0eab315079c19c5e upstream.
>>>>
>>>> The part_event_bitmap register is 64 bits wide, so read it with
>>>> ioread64()
>>>> instead of the 32-bit ioread32().
>>>>
>>>> Fixes: 52eabba5bcdb ("switchtec: Add IOCTLs to the Switchtec driver")
>>>> Link:
>>>> https://lore.kernel.org/r/20190910195833.3891-1-logang@deltatee.com
>>>> Reported-by: Doug Meyer <dmeyer@gigaio.com>
>>>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>>>> Cc: stable@vger.kernel.org    # v4.12+
>>>> Cc: Kelvin Cao <Kelvin.Cao@microchip.com>
>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> ---
>>>>
>>>> ioread64() was introduced in v5.1 so the upstream patch won't compile on
>>>> stable versions 4.14 or 4.19. This is the same patch but uses readq()
>>>> which should be equivalent.
>>>
>>> Now queued up, thanks.
>>
>> Hey Logan,
>>
>> As Guenter has pointed out, readq() is only defined for 64 bits, so this
>> breaks compilation in i386. I've dropped this backport for now, if you
>> could fix it up we could queue it up again.
>
>Not quiet true. It is in fact defined for 32-bit architectures as two
>readl() calls in "linux/io-64-nonatomic-lo-hi.h".
>
>So, unless I'm missing something the patch should be fine.

This is an actual error we're seeing:

drivers/pci/switch/switchtec.c: In function ‘ioctl_event_summary’:
drivers/pci/switch/switchtec.c:636:18: error: implicit declaration of function ‘readq’; did you mean ‘readl’? [-Werror=implicit-function-declaration]
  636 |  s.part_bitmap = readq(&stdev->mmio_sw_event->part_event_bitmap);
      |                  ^~~~~
      |                  readl
cc1: some warnings being treated as errors
make[1]: *** [scripts/Makefile.build:310: drivers/pci/switch/switchtec.o] Error 1
make: *** [Makefile:1695: drivers/pci/switch/] Error 2

So the patch isn't fine :)

You're correct about linux/io-64-nonatomic-lo-hi.h, but sadly it isn't
included in drivers/pci/switch/switchtec.c so it's not getting used.
Something like the following has fixed compilation for me:

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 4042fe1e7361..5035b17fe399 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -16,6 +16,8 @@

 #include <linux/nospec.h>

+#include <linux/io-64-nonatomic-lo-hi.h>
+
 MODULE_DESCRIPTION("Microsemi Switchtec(tm) PCIe Management Driver");
 MODULE_VERSION("0.1");
 MODULE_LICENSE("GPL");

-- 
Thanks,
Sasha

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI/switchtec: Read all 64 bits of part_event_bitmap
  2020-01-03  3:31       ` Sasha Levin
@ 2020-01-03  5:43         ` Logan Gunthorpe
  2020-01-03  7:04           ` Greg Kroah-Hartman
  0 siblings, 1 reply; 7+ messages in thread
From: Logan Gunthorpe @ 2020-01-03  5:43 UTC (permalink / raw
  To: Sasha Levin
  Cc: Greg Kroah-Hartman, Doug Meyer, Bjorn Helgaas, stable, Kelvin Cao



On 2020-01-02 8:31 p.m., Sasha Levin wrote:
> On Thu, Jan 02, 2020 at 05:46:58PM -0700, Logan Gunthorpe wrote:
>>
>>
>> On 2020-01-02 5:18 p.m., Sasha Levin wrote:
>>> On Wed, Jan 01, 2020 at 06:04:06PM +0100, Greg Kroah-Hartman wrote:
>>>> On Thu, Dec 19, 2019 at 11:27:47AM -0700, Logan Gunthorpe wrote:
>>>>> commit 6acdf7e19b37cb3a9258603d0eab315079c19c5e upstream.
>>>>>
>>>>> The part_event_bitmap register is 64 bits wide, so read it with
>>>>> ioread64()
>>>>> instead of the 32-bit ioread32().
>>>>>
>>>>> Fixes: 52eabba5bcdb ("switchtec: Add IOCTLs to the Switchtec driver")
>>>>> Link:
>>>>> https://lore.kernel.org/r/20190910195833.3891-1-logang@deltatee.com
>>>>> Reported-by: Doug Meyer <dmeyer@gigaio.com>
>>>>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>>>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>>>>> Cc: stable@vger.kernel.org    # v4.12+
>>>>> Cc: Kelvin Cao <Kelvin.Cao@microchip.com>
>>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>>> ---
>>>>>
>>>>> ioread64() was introduced in v5.1 so the upstream patch won't
>>>>> compile on
>>>>> stable versions 4.14 or 4.19. This is the same patch but uses readq()
>>>>> which should be equivalent.
>>>>
>>>> Now queued up, thanks.
>>>
>>> Hey Logan,
>>>
>>> As Guenter has pointed out, readq() is only defined for 64 bits, so this
>>> breaks compilation in i386. I've dropped this backport for now, if you
>>> could fix it up we could queue it up again.
>>
>> Not quiet true. It is in fact defined for 32-bit architectures as two
>> readl() calls in "linux/io-64-nonatomic-lo-hi.h".
>>
>> So, unless I'm missing something the patch should be fine.
> 
> This is an actual error we're seeing:
> 
> drivers/pci/switch/switchtec.c: In function ‘ioctl_event_summary’:
> drivers/pci/switch/switchtec.c:636:18: error: implicit declaration of
> function ‘readq’; did you mean ‘readl’?
> [-Werror=implicit-function-declaration]
>  636 |  s.part_bitmap = readq(&stdev->mmio_sw_event->part_event_bitmap);
>      |                  ^~~~~
>      |                  readl
> cc1: some warnings being treated as errors
> make[1]: *** [scripts/Makefile.build:310:
> drivers/pci/switch/switchtec.o] Error 1
> make: *** [Makefile:1695: drivers/pci/switch/] Error 2
> 
> So the patch isn't fine :)
> 
> You're correct about linux/io-64-nonatomic-lo-hi.h, but sadly it isn't
> included in drivers/pci/switch/switchtec.c so it's not getting used.
> Something like the following has fixed compilation for me:

Oh, hmm, yes looks like we added that include in v5.0 so earlier
versions need it for that patch to be correct on non-64bit arches. Sigh.

Can you just add the include line to the patch or do you need me to send
a new one fixed up?

Thanks,

Logan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI/switchtec: Read all 64 bits of part_event_bitmap
  2020-01-03  5:43         ` Logan Gunthorpe
@ 2020-01-03  7:04           ` Greg Kroah-Hartman
  0 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2020-01-03  7:04 UTC (permalink / raw
  To: Logan Gunthorpe
  Cc: Sasha Levin, Doug Meyer, Bjorn Helgaas, stable, Kelvin Cao

On Thu, Jan 02, 2020 at 10:43:22PM -0700, Logan Gunthorpe wrote:
> 
> 
> On 2020-01-02 8:31 p.m., Sasha Levin wrote:
> > On Thu, Jan 02, 2020 at 05:46:58PM -0700, Logan Gunthorpe wrote:
> >>
> >>
> >> On 2020-01-02 5:18 p.m., Sasha Levin wrote:
> >>> On Wed, Jan 01, 2020 at 06:04:06PM +0100, Greg Kroah-Hartman wrote:
> >>>> On Thu, Dec 19, 2019 at 11:27:47AM -0700, Logan Gunthorpe wrote:
> >>>>> commit 6acdf7e19b37cb3a9258603d0eab315079c19c5e upstream.
> >>>>>
> >>>>> The part_event_bitmap register is 64 bits wide, so read it with
> >>>>> ioread64()
> >>>>> instead of the 32-bit ioread32().
> >>>>>
> >>>>> Fixes: 52eabba5bcdb ("switchtec: Add IOCTLs to the Switchtec driver")
> >>>>> Link:
> >>>>> https://lore.kernel.org/r/20190910195833.3891-1-logang@deltatee.com
> >>>>> Reported-by: Doug Meyer <dmeyer@gigaio.com>
> >>>>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> >>>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
> >>>>> Cc: stable@vger.kernel.org    # v4.12+
> >>>>> Cc: Kelvin Cao <Kelvin.Cao@microchip.com>
> >>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>>>> ---
> >>>>>
> >>>>> ioread64() was introduced in v5.1 so the upstream patch won't
> >>>>> compile on
> >>>>> stable versions 4.14 or 4.19. This is the same patch but uses readq()
> >>>>> which should be equivalent.
> >>>>
> >>>> Now queued up, thanks.
> >>>
> >>> Hey Logan,
> >>>
> >>> As Guenter has pointed out, readq() is only defined for 64 bits, so this
> >>> breaks compilation in i386. I've dropped this backport for now, if you
> >>> could fix it up we could queue it up again.
> >>
> >> Not quiet true. It is in fact defined for 32-bit architectures as two
> >> readl() calls in "linux/io-64-nonatomic-lo-hi.h".
> >>
> >> So, unless I'm missing something the patch should be fine.
> > 
> > This is an actual error we're seeing:
> > 
> > drivers/pci/switch/switchtec.c: In function ‘ioctl_event_summary’:
> > drivers/pci/switch/switchtec.c:636:18: error: implicit declaration of
> > function ‘readq’; did you mean ‘readl’?
> > [-Werror=implicit-function-declaration]
> >  636 |  s.part_bitmap = readq(&stdev->mmio_sw_event->part_event_bitmap);
> >      |                  ^~~~~
> >      |                  readl
> > cc1: some warnings being treated as errors
> > make[1]: *** [scripts/Makefile.build:310:
> > drivers/pci/switch/switchtec.o] Error 1
> > make: *** [Makefile:1695: drivers/pci/switch/] Error 2
> > 
> > So the patch isn't fine :)
> > 
> > You're correct about linux/io-64-nonatomic-lo-hi.h, but sadly it isn't
> > included in drivers/pci/switch/switchtec.c so it's not getting used.
> > Something like the following has fixed compilation for me:
> 
> Oh, hmm, yes looks like we added that include in v5.0 so earlier
> versions need it for that patch to be correct on non-64bit arches. Sigh.
> 
> Can you just add the include line to the patch or do you need me to send
> a new one fixed up?

A new one fixed up would be great, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-01-03  7:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-19 18:27 [PATCH] PCI/switchtec: Read all 64 bits of part_event_bitmap Logan Gunthorpe
2020-01-01 17:04 ` Greg Kroah-Hartman
2020-01-03  0:18   ` Sasha Levin
2020-01-03  0:46     ` Logan Gunthorpe
2020-01-03  3:31       ` Sasha Levin
2020-01-03  5:43         ` Logan Gunthorpe
2020-01-03  7:04           ` Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.