Re: [PATCH v4 2/3] vfio/pci: Support 8-byte PCI loads and stores

[Gerd Bayer <gbayer@linux.ibm.com>]

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).

> 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?

> 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
revert to make the conditional compiles depend on those definitions.

But maybe Alex has an opinion on this, too?

Thanks,
Gerd