linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] x86/io: Define readq()/writeq() to use 64-bit type
@ 2018-05-03 10:28 Andy Shevchenko
  2018-05-13 18:09 ` Thomas Gleixner
  0 siblings, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2018-05-03 10:28 UTC (permalink / raw)
  To: Thomas Gleixner, H . Peter Anvin, Ingo Molnar, x86, linux-kernel
  Cc: Andy Shevchenko

Since non atomic readq() and writeq() were added some of the drivers
would like to use it in a manner of:

 #include <io-64-nonatomic-lo-hi.h>
...
 pr_debug("Debug value of some register: %016llx\n", readq(addr));

However, lo_hi_readq() always returns __u64 data, while readq()
on x86_64 defines it as unsigned long. and thus compiler warns
about type mismatch, although they are both 64-bit on x86_64.

Convert readq() and writeq() on x86 to operate on deterministic
64-bit type. The most of architectures in the kernel already are using
either unsigned long long, or u64 type for readq() / writeq().
This change propagates consistency in that sense.

While this is not an issue per se, though if someone wants to address it,
the anchor could be the commit

  797a796a13df ("asm-generic: architecture independent readq/writeq for 32bit environment")

where non-atomic variants had been introduced.

Note, there are only few users of above pattern and they will not be
affected because they do cast returned value. The actual warning has
been issued on not-yet-upstreamed code.

Potentially we might get a new warnings if some 64-bit only code
assigns returned value to unsigned long type of variable. This is
assumed to be addressed on case-by-case basis.

Reported-by: lkp <lkp@intel.com>
Tested-by: Sohil Mehta <sohil.mehta@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---

- resend after v4.17-rc1 as agreed before

 arch/x86/include/asm/io.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index f6e5b9375d8c..6259acc0f484 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -94,10 +94,10 @@ build_mmio_write(__writel, "l", unsigned int, "r", )
 
 #ifdef CONFIG_X86_64
 
-build_mmio_read(readq, "q", unsigned long, "=r", :"memory")
-build_mmio_read(__readq, "q", unsigned long, "=r", )
-build_mmio_write(writeq, "q", unsigned long, "r", :"memory")
-build_mmio_write(__writeq, "q", unsigned long, "r", )
+build_mmio_read(readq, "q", unsigned long long, "=r", :"memory")
+build_mmio_read(__readq, "q", unsigned long long, "=r", )
+build_mmio_write(writeq, "q", unsigned long long, "r", :"memory")
+build_mmio_write(__writeq, "q", unsigned long long, "r", )
 
 #define readq_relaxed(a)	__readq(a)
 #define writeq_relaxed(v, a)	__writeq(v, a)
-- 
2.17.0

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

* Re: [PATCH v2] x86/io: Define readq()/writeq() to use 64-bit type
  2018-05-03 10:28 [PATCH v2] x86/io: Define readq()/writeq() to use 64-bit type Andy Shevchenko
@ 2018-05-13 18:09 ` Thomas Gleixner
  2018-05-13 20:33   ` Andy Shevchenko
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Gleixner @ 2018-05-13 18:09 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: H . Peter Anvin, Ingo Molnar, x86, linux-kernel

On Thu, 3 May 2018, Andy Shevchenko wrote:
>  #ifdef CONFIG_X86_64
>  
> -build_mmio_read(readq, "q", unsigned long, "=r", :"memory")
> -build_mmio_read(__readq, "q", unsigned long, "=r", )
> -build_mmio_write(writeq, "q", unsigned long, "r", :"memory")
> -build_mmio_write(__writeq, "q", unsigned long, "r", )
> +build_mmio_read(readq, "q", unsigned long long, "=r", :"memory")
> +build_mmio_read(__readq, "q", unsigned long long, "=r", )
> +build_mmio_write(writeq, "q", unsigned long long, "r", :"memory")
> +build_mmio_write(__writeq, "q", unsigned long long, "r", )

What's wrong with u64 which we use for expressing io access to a 64bit wide
resource?

Thanks,

	tglx

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

* Re: [PATCH v2] x86/io: Define readq()/writeq() to use 64-bit type
  2018-05-13 18:09 ` Thomas Gleixner
@ 2018-05-13 20:33   ` Andy Shevchenko
  2018-05-14  7:12     ` Thomas Gleixner
  0 siblings, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2018-05-13 20:33 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Andy Shevchenko, H . Peter Anvin, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Linux Kernel Mailing List

On Sun, May 13, 2018 at 9:09 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 3 May 2018, Andy Shevchenko wrote:
>>  #ifdef CONFIG_X86_64
>>
>> -build_mmio_read(readq, "q", unsigned long, "=r", :"memory")
>> -build_mmio_read(__readq, "q", unsigned long, "=r", )
>> -build_mmio_write(writeq, "q", unsigned long, "r", :"memory")
>> -build_mmio_write(__writeq, "q", unsigned long, "r", )
>> +build_mmio_read(readq, "q", unsigned long long, "=r", :"memory")
>> +build_mmio_read(__readq, "q", unsigned long long, "=r", )
>> +build_mmio_write(writeq, "q", unsigned long long, "r", :"memory")
>> +build_mmio_write(__writeq, "q", unsigned long long, "r", )
>
> What's wrong with u64 which we use for expressing io access to a 64bit wide
> resource?

Same answer as per v1, i.e. I would like to be consistent with other
types in this file (unsigned int for readl() and similar for the
rest).
If we would need them, we might change at once for all accessors.

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v2] x86/io: Define readq()/writeq() to use 64-bit type
  2018-05-13 20:33   ` Andy Shevchenko
@ 2018-05-14  7:12     ` Thomas Gleixner
  2018-05-14 11:04       ` Andy Shevchenko
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Gleixner @ 2018-05-14  7:12 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, H . Peter Anvin, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Linux Kernel Mailing List

On Sun, 13 May 2018, Andy Shevchenko wrote:
> On Sun, May 13, 2018 at 9:09 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Thu, 3 May 2018, Andy Shevchenko wrote:
> >>  #ifdef CONFIG_X86_64
> >>
> >> -build_mmio_read(readq, "q", unsigned long, "=r", :"memory")
> >> -build_mmio_read(__readq, "q", unsigned long, "=r", )
> >> -build_mmio_write(writeq, "q", unsigned long, "r", :"memory")
> >> -build_mmio_write(__writeq, "q", unsigned long, "r", )
> >> +build_mmio_read(readq, "q", unsigned long long, "=r", :"memory")
> >> +build_mmio_read(__readq, "q", unsigned long long, "=r", )
> >> +build_mmio_write(writeq, "q", unsigned long long, "r", :"memory")
> >> +build_mmio_write(__writeq, "q", unsigned long long, "r", )
> >
> > What's wrong with u64 which we use for expressing io access to a 64bit wide
> > resource?
> 
> Same answer as per v1, i.e. I would like to be consistent with other
> types in this file (unsigned int for readl() and similar for the
> rest).
> If we would need them, we might change at once for all accessors.

I don;t think we need to fixup everything in one go. Having the patch which
addresses the issue at hand first using u64 makes a lot of sense on its own.

Changing the other instances can be done as a follow up patch. Having
explicit with types for such kind of accessors makes a lot of sense.

Thanks,

	tglx

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

* Re: [PATCH v2] x86/io: Define readq()/writeq() to use 64-bit type
  2018-05-14  7:12     ` Thomas Gleixner
@ 2018-05-14 11:04       ` Andy Shevchenko
  0 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2018-05-14 11:04 UTC (permalink / raw)
  To: Thomas Gleixner, Andy Shevchenko
  Cc: H . Peter Anvin, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Linux Kernel Mailing List

On Mon, 2018-05-14 at 09:12 +0200, Thomas Gleixner wrote:
> On Sun, 13 May 2018, Andy Shevchenko wrote:
> > On Sun, May 13, 2018 at 9:09 PM, Thomas Gleixner <tglx@linutronix.de
> > > wrote:
> > > On Thu, 3 May 2018, Andy Shevchenko wrote:
> > > >  #ifdef CONFIG_X86_64
> > > > 
> > > > -build_mmio_read(readq, "q", unsigned long, "=r", :"memory")
> > > > -build_mmio_read(__readq, "q", unsigned long, "=r", )
> > > > -build_mmio_write(writeq, "q", unsigned long, "r", :"memory")
> > > > -build_mmio_write(__writeq, "q", unsigned long, "r", )
> > > > +build_mmio_read(readq, "q", unsigned long long, "=r",
> > > > :"memory")
> > > > +build_mmio_read(__readq, "q", unsigned long long, "=r", )
> > > > +build_mmio_write(writeq, "q", unsigned long long, "r",
> > > > :"memory")
> > > > +build_mmio_write(__writeq, "q", unsigned long long, "r", )
> > > 
> > > What's wrong with u64 which we use for expressing io access to a
> > > 64bit wide
> > > resource?
> > 
> > Same answer as per v1, i.e. I would like to be consistent with other
> > types in this file (unsigned int for readl() and similar for the
> > rest).
> > If we would need them, we might change at once for all accessors.
> 
> I don;t think we need to fixup everything in one go. Having the patch
> which
> addresses the issue at hand first using u64 makes a lot of sense on
> its own.
> 
> Changing the other instances can be done as a follow up patch. Having
> explicit with types for such kind of accessors makes a lot of sense.

OK, I will re-do it this way.
Thanks for review!

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

end of thread, other threads:[~2018-05-14 11:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-03 10:28 [PATCH v2] x86/io: Define readq()/writeq() to use 64-bit type Andy Shevchenko
2018-05-13 18:09 ` Thomas Gleixner
2018-05-13 20:33   ` Andy Shevchenko
2018-05-14  7:12     ` Thomas Gleixner
2018-05-14 11:04       ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).