Re: [PATCH v2 2/3] include: Add a header to define PCI MMIO functions

[Farhan Ali <alifm@linux.ibm.com>]

On 3/31/2025 6:46 AM, Stefan Hajnoczi wrote:
> On Fri, Mar 28, 2025 at 12:06:26PM -0700, Farhan Ali wrote:
>> Add a generic QEMU API for PCI MMIO reads/writes.
>> The functions access little endian memory and returns
>> the result in host cpu endianness.
>>
>> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
>> ---
>>   include/qemu/pci-mmio.h | 116 ++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 116 insertions(+)
>>   create mode 100644 include/qemu/pci-mmio.h
>>
>> diff --git a/include/qemu/pci-mmio.h b/include/qemu/pci-mmio.h
>> new file mode 100644
>> index 0000000000..2ef92455b1
>> --- /dev/null
>> +++ b/include/qemu/pci-mmio.h
>> @@ -0,0 +1,116 @@
>> +/*
>> + * QEMU PCI MMIO API
> QEMU also emulates PCI devices that handle MMIO accesses. It is easy to
> get confused between host PCI MMIO accesses and emulated guest PCI MMIO
> accesses if the name is just "PCI MMIO API".
>
> Please update the commit message, filenames, function names, and doc
> comments to make it clear that this is only for host PCI MMIO accesses
> (e.g. Linux VFIO BAR accesses).
>
> For example "qemu/host-pci-mmio.h", "API for host PCI MMIO accesses
> (e.g. Linux VFIO BARs)", and host_pci_mmio_read_8().

yeah this makes sense. Will update the API names in v3.


>> + *
>> + * Copyright 2025 IBM Corp.
>> + * Author(s): Farhan Ali <alifm@linux.ibm.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#ifndef QEMU_PCI_MMIO_H
>> +#define QEMU_PCI_MMIO_H
>> +
>> +#ifdef __s390x__
>> +#include "s390x_pci_mmio.h"
>> +#endif
>> +
>> +static inline uint8_t qemu_pci_mmio_read_8(const void *ioaddr)
>> +{
>> +    uint8_t ret = 0;
>> +#ifdef __s390x__
>> +    ret = s390x_pci_mmio_read_8(ioaddr);
>> +#else
>> +    /* Prevent the compiler from optimizing away the load */
>> +    ret = *((volatile uint8_t *)ioaddr);
>> +#endif
>> +
>> +    return ret;
>> +}
>> +
>> +static inline uint16_t qemu_pci_mmio_read_16(const void *ioaddr)
>> +{
>> +    uint16_t ret = 0;
>> +#ifdef __s390x__
>> +    ret = s390x_pci_mmio_read_16(ioaddr);
>> +#else
>> +    /* Prevent the compiler from optimizing away the load */
>> +    ret = *((volatile uint16_t *)ioaddr);
>> +#endif
>> +
>> +    return le16_to_cpu(ret);
>> +}
>> +
>> +static inline uint32_t qemu_pci_mmio_read_32(const void *ioaddr)
>> +{
>> +    uint32_t ret = 0;
>> +#ifdef __s390x__
>> +    ret = s390x_pci_mmio_read_32(ioaddr);
>> +#else
>> +    /* Prevent the compiler from optimizing away the load */
>> +    ret = *((volatile uint32_t *)ioaddr);
>> +#endif
>> +
>> +    return le32_to_cpu(ret);
>> +}
>> +
>> +static inline uint64_t qemu_pci_mmio_read_64(const void *ioaddr)
>> +{
>> +    uint64_t ret = 0;
>> +#ifdef __s390x__
>> +    ret = s390x_pci_mmio_read_64(ioaddr);
>> +#else
>> +    /* Prevent the compiler from optimizing away the load */
>> +    ret = *((volatile uint64_t *)ioaddr);
>> +#endif
>> +
>> +    return le64_to_cpu(ret);
>> +}
>> +
>> +static inline void qemu_pci_mmio_write_8(void *ioaddr, uint8_t val)
>> +{
>> +
>> +#ifdef __s390x__
>> +    s390x_pci_mmio_write_8(ioaddr, val);
>> +#else
>> +    /* Prevent the compiler from optimizing away the store */
>> +    *((volatile uint8_t *)ioaddr) = val;
>> +#endif
>> +}
>> +
>> +static inline void qemu_pci_mmio_write_16(void *ioaddr, uint16_t val)
>> +{
>> +    val = cpu_to_le16(val);
>> +
>> +#ifdef __s390x__
>> +    s390x_pci_mmio_write_16(ioaddr, val);
>> +#else
>> +    /* Prevent the compiler from optimizing away the store */
>> +    *((volatile uint16_t *)ioaddr) = val;
>> +#endif
>> +}
>> +
>> +static inline void qemu_pci_mmio_write_32(void *ioaddr, uint32_t val)
>> +{
>> +    val = cpu_to_le32(val);
>> +
>> +#ifdef __s390x__
>> +    s390x_pci_mmio_write_32(ioaddr, val);
>> +#else
>> +    /* Prevent the compiler from optimizing away the store */
>> +    *((volatile uint32_t *)ioaddr) = val;
>> +#endif
>> +}
>> +
>> +static inline void qemu_pci_mmio_write_64(void *ioaddr, uint64_t val)
>> +{
>> +    val = cpu_to_le64(val);
>> +
>> +#ifdef __s390x__
>> +    s390x_pci_mmio_write_64(ioaddr, val);
>> +#else
>> +    /* Prevent the compiler from optimizing away the store */
>> +    *((volatile uint64_t *)ioaddr) = val;
>> +#endif
>> +}
>> +
>> +#endif
>> -- 
>> 2.43.0
>>