On 5/23/2024 8:01 AM, Gerd Bayer wrote:
Hi Ramesh,
On Wed, 2024-05-22 at 16:38 -0700, Ramesh Thomas wrote:
The removal of the check for iowrite64 and ioread64 causes build
error because those macros don't get defined anywhere if
CONFIG_GENERIC_IOMAP is not defined. However, I do think the removal
of the checks is correct.
Wait, I believe it is the other way around. If your config *is*
specifying CONFIG_GENERIC_IOMAP, lib/iomap.c will provide
implementations for back-to-back 32bit operations to emulate 64bit
accesses - and you have to "select" which of the two types of emulation
(hi/lo or lo/hi order) get mapped onto ioread64(be) or iowrite64(be) by
including linux/io-64-nonatomic-lo-hi.h (or -hi-lo.h).
Sorry, yes I meant to write they don't get defined anywhere in your code
path if CONFIG_GENERIC_IOMAP *is defined*. The only place in your code
path where iowrit64 and ioread64 get defined is in asm/io.h. Those
definitions are surrounded by #ifndef CONFIG_GENERIC_IOMAP.
CONFIG_GENERIC_IOMAP gets defined for x86.
It is better to include linux/io-64-nonatomic-lo-hi.h which define
those macros mapping to generic implementations in lib/iomap.c. If
the architecture does not implement 64 bit rw functions
(readq/writeq), then it does 32 bit back to back. I have sent a
patch with the change that includes the above header file. Please
review and include in this patch series if ok.
I did find your patch, thank you. I had a very hard time to find a
kernel config that actually showed the unresolved symbols situation:
Some 64bit MIPS config, that relied on GENERIC_IOMAP. And with your
patch applied, I could compile successfully.
Do you have an easier way steer a kernel config into this dead-end?
The generic implementation takes care of all conditions. I guess some
build bot would report error on build failures. But checks like #ifdef
iowrite64 would hide the missing definitions error.
Thanks,
Ramesh
Frankly, I'd rather not make any assumptions in this rather generic
vfio/pci layer about whether hi-lo or lo-hi is the right order to > emulate a 64bit access when the base architecture does not support
64bit accesses naturally. So, if CONFIG_64BIT is no guarantee that
there's a definitive implementation of ioread64/iowrite64, I'd rather
There is already an assumption of the order in the current
implementation regardless e.g. vfio_pci_core_do_io_rw(). If there is no
iowrite64 found, the code does back to back 34 bit writes without
checking for any particular order requirements.
io-64-nonatomic-lo-hi.h and io-64-nonatomic-hi-lo.h would define
ioread64/iowrite64 only if they are not already defined in asm/io.h.
Also since there is a check for CONFIG_64BIT, most likely a 64 bit
readq/writeq will get used in the lib/iomap.c implementations. I think
we can pick either lo-hi or hi-lo for the unlikely 32 bit fall through
when CONFIG_64BIT is defined.
revert to make the conditional compiles depend on those definitions.
But maybe Alex has an opinion on this, too?
Thanks,
Gerd
