x86/io: Define readq()/writeq() to use 64-bit type
PATCH v2 1/1 x86/io: Define readq()/writeq() to use 64-bit type
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.
---
- resend after v4.17-rc1 as agreed before
arch/x86/include/asm/io.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
arch/x86/include/asm/io.h +4 -4
@@ -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)
Signature · 1 line
2.17.0
arch/x86/include/asm/io.h build_mmio_write(__writel, "l", unsigned int, "r", ) · line 100
Quoted 10 lines · Andy Shevchenko
#ifdef
CONFIG_X86_64arch/x86/include/asm/io.h +4
-4-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
Quoted 14 lines · Thomas Gleixner
arch/x86/include/asm/io.h build_mmio_write(__writel, "l", unsigned int, "r", ) · line 100
Quoted 10 lines · Andy Shevchenko
#ifdef
CONFIG_X86_64arch/x86/include/asm/io.h +4
-4-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.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 16 lines · Andy Shevchenko
Quoted 14 lines · Thomas Gleixner
arch/x86/include/asm/io.h build_mmio_write(__writel, "l", unsigned int, "r", ) · line 100
Quoted 10 lines · Andy Shevchenko
#ifdef
CONFIG_X86_64arch/x86/include/asm/io.h +4
-4-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?
Quoted 4 lines
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
Quoted 26 lines · Thomas Gleixner
Quoted 20 lines · Andy Shevchenko
On Sun, May 13, 2018 at 9:09 PM, Thomas Gleixner <[email protected]
Quoted 4 lines
wrote:
Quoted 2 lines · Andy Shevchenko
#ifdef
CONFIG_X86_64Quoted 14 lines
Quoted 10 lines
arch/x86/include/asm/io.h +1
-4-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")
(quoted diff) +2
-0+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?Quoted 4 lines
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.
Quoted 7 lines
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!
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy