LKML Archive on lore.kernel.org
 help / color / Atom feed
* sparse warnings in vop
@ 2020-08-02  4:57 Michael S. Tsirkin
       [not found] ` <CAHp75Vfwhfrse47jRR9msFHA4ZqoVvE8RYHZNKxu-_ZiGyS9Sw@mail.gmail.com>
  0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2020-08-02  4:57 UTC (permalink / raw)
  To: sudeep.dutt; +Cc: ashutosh.dixit, arnd, gregkh, linux-kernel

Hi!
Building vop with make C=1 produces the following:

CHECK   drivers/misc/mic/vop/vop_main.c
drivers/misc/mic/vop/vop_main.c:551:58: warning: incorrect type in argument 1 (different address spaces)
drivers/misc/mic/vop/vop_main.c:551:58:    expected void const volatile [noderef] __iomem *addr
drivers/misc/mic/vop/vop_main.c:551:58:    got restricted __le64 *
drivers/misc/mic/vop/vop_main.c:560:49: warning: incorrect type in argument 1 (different address spaces)
drivers/misc/mic/vop/vop_main.c:560:49:    expected struct mic_device_ctrl *dc
drivers/misc/mic/vop/vop_main.c:560:49:    got struct mic_device_ctrl [noderef] __iomem *dc
drivers/misc/mic/vop/vop_main.c:579:49: warning: incorrect type in argument 1 (different address spaces)
drivers/misc/mic/vop/vop_main.c:579:49:    expected struct mic_device_ctrl *dc
drivers/misc/mic/vop/vop_main.c:579:49:    got struct mic_device_ctrl [noderef] __iomem *dc

Would be nice to fix to silence the noise, but I'm not 100% sure
what the right thing to do here is. Tag struct members with __iomem or
cast with __force on use?

-- 
MST


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

* Re: sparse warnings in vop
       [not found] ` <CAHp75Vfwhfrse47jRR9msFHA4ZqoVvE8RYHZNKxu-_ZiGyS9Sw@mail.gmail.com>
@ 2020-08-02 10:48   ` Arnd Bergmann
  2020-08-02 23:31     ` Dixit, Ashutosh
  0 siblings, 1 reply; 3+ messages in thread
From: Arnd Bergmann @ 2020-08-02 10:48 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Michael S. Tsirkin, sudeep.dutt, ashutosh.dixit, gregkh, linux-kernel

On Sun, Aug 2, 2020 at 9:25 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sunday, August 2, 2020, Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>> Hi!
>> Building vop with make C=1 produces the following:
>>
>> CHECK   drivers/misc/mic/vop/vop_main.c
>> drivers/misc/mic/vop/vop_main.c:551:58: warning: incorrect type in argument 1 (different address spaces)
>> drivers/misc/mic/vop/vop_main.c:551:58:    expected void const volatile [noderef] __iomem *addr
>> drivers/misc/mic/vop/vop_main.c:551:58:    got restricted __le64 *
>> drivers/misc/mic/vop/vop_main.c:560:49: warning: incorrect type in argument 1 (different address spaces)
>> drivers/misc/mic/vop/vop_main.c:560:49:    expected struct mic_device_ctrl *dc
>> drivers/misc/mic/vop/vop_main.c:560:49:    got struct mic_device_ctrl [noderef] __iomem *dc
>> drivers/misc/mic/vop/vop_main.c:579:49: warning: incorrect type in argument 1 (different address spaces)
>> drivers/misc/mic/vop/vop_main.c:579:49:    expected struct mic_device_ctrl *dc
>> drivers/misc/mic/vop/vop_main.c:579:49:    got struct mic_device_ctrl [noderef] __iomem *dc
>>
>> Would be nice to fix to silence the noise, but I'm not 100% sure
>> what the right thing to do here is. Tag struct members with __iomem or
>> cast with __force on use?
>
>
>
> Sounds right to me.

I don't think either of the above, adding __force is almost always wrong,
and __iomem never applies to struct members, only to pointers.

The first problem I see is with:

static struct _vop_vdev *vop_dc_to_vdev(struct mic_device_ctrl *dc)

The argument needs to be an __iomem pointer. In the structure, the
first member has type __le64, which gets mentioned in the warning.
We usually use __u64 instead (or don't use structures at all for __iomem
operations), but I don't think this would cause a warning if the argument
is fixed.

Then there is the question of why in the world you would have an MMIO
register contain a kernel pointer, but that is more a driver design question
than something that causes a warning.

      Arnd

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

* Re: sparse warnings in vop
  2020-08-02 10:48   ` Arnd Bergmann
@ 2020-08-02 23:31     ` Dixit, Ashutosh
  0 siblings, 0 replies; 3+ messages in thread
From: Dixit, Ashutosh @ 2020-08-02 23:31 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andy Shevchenko, Michael S. Tsirkin, sudeep.dutt, gregkh,
	linux-kernel, vincent.whitchurch

On Sun, 02 Aug 2020 03:48:33 -0700, Arnd Bergmann wrote:
>
> On Sun, Aug 2, 2020 at 9:25 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Sunday, August 2, 2020, Michael S. Tsirkin <mst@redhat.com> wrote:
> >>
> >> Hi!
> >> Building vop with make C=1 produces the following:
> >>
> >> CHECK   drivers/misc/mic/vop/vop_main.c
> >> drivers/misc/mic/vop/vop_main.c:551:58: warning: incorrect type in argument 1 (different address spaces)
> >> drivers/misc/mic/vop/vop_main.c:551:58:    expected void const volatile [noderef] __iomem *addr
> >> drivers/misc/mic/vop/vop_main.c:551:58:    got restricted __le64 *
> >> drivers/misc/mic/vop/vop_main.c:560:49: warning: incorrect type in argument 1 (different address spaces)
> >> drivers/misc/mic/vop/vop_main.c:560:49:    expected struct mic_device_ctrl *dc
> >> drivers/misc/mic/vop/vop_main.c:560:49:    got struct mic_device_ctrl [noderef] __iomem *dc
> >> drivers/misc/mic/vop/vop_main.c:579:49: warning: incorrect type in argument 1 (different address spaces)
> >> drivers/misc/mic/vop/vop_main.c:579:49:    expected struct mic_device_ctrl *dc
> >> drivers/misc/mic/vop/vop_main.c:579:49:    got struct mic_device_ctrl [noderef] __iomem *dc
> >>
> >> Would be nice to fix to silence the noise, but I'm not 100% sure
> >> what the right thing to do here is. Tag struct members with __iomem or
> >> cast with __force on use?
> >
> > Sounds right to me.
>
> I don't think either of the above, adding __force is almost always wrong,
> and __iomem never applies to struct members, only to pointers.
>
> The first problem I see is with:
>
> static struct _vop_vdev *vop_dc_to_vdev(struct mic_device_ctrl *dc)
>
> The argument needs to be an __iomem pointer. In the structure, the first
> member has type __le64, which gets mentioned in the warning.  We usually
> use __u64 instead (or don't use structures at all for __iomem
> operations), but I don't think this would cause a warning if the argument
> is fixed.

I have submitted a patch which fixes the warnings.

> Then there is the question of why in the world you would have an MMIO
> register contain a kernel pointer, but that is more a driver design question
> than something that causes a warning.

It is not a MMIO register, it is the OS running on PCI device accessing
system memory which is ioremap'd for the PCI device OS. Also, the data
structure is access both by the system OS (not __iomem) as well as the PCI
device OS (__iomem). Therefore the structure members cannot be tagged
__iomem. Doing so will fix the sparse warning for the PCI device OS but
result in new sparse warnings for the system OS. Thanks.

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-02  4:57 sparse warnings in vop Michael S. Tsirkin
     [not found] ` <CAHp75Vfwhfrse47jRR9msFHA4ZqoVvE8RYHZNKxu-_ZiGyS9Sw@mail.gmail.com>
2020-08-02 10:48   ` Arnd Bergmann
2020-08-02 23:31     ` Dixit, Ashutosh

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git