Add a generic API for host PCI MMIO reads/writes
(e.g. Linux VFIO BAR accesses). 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/host-pci-mmio.h | 141 +++++++++++++++++++++++++++++++++++
1 file changed, 141 insertions(+)
create mode 100644 include/qemu/host-pci-mmio.h
diff --git a/include/qemu/host-pci-mmio.h b/include/qemu/host-pci-mmio.h
new file mode 100644
index 0000000000..c93f77dcd4
--- /dev/null
+++ b/include/qemu/host-pci-mmio.h
@@ -0,0 +1,141 @@
+/*
+ * API for host PCI MMIO accesses (e.g. Linux VFIO BARs)
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Farhan Ali <alifm@linux.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HOST_PCI_MMIO_H
+#define HOST_PCI_MMIO_H
+
+#include "qemu/bswap.h"
+#include "qemu/s390x_pci_mmio.h"
+
+
+static inline uint8_t host_pci_ldub_p(const void *ioaddr)
+{
+ uint8_t ret = 0;
+#ifdef __s390x__
+ ret = s390x_pci_mmio_read_8(ioaddr);
+#else
+ ret = ldub_p(ioaddr);
+#endif
+
+ return ret;
+}
+
+static inline uint16_t host_pci_lduw_le_p(const void *ioaddr)
+{
+ uint16_t ret = 0;
+#ifdef __s390x__
+ ret = le16_to_cpu(s390x_pci_mmio_read_16(ioaddr));
+#else
+ ret = lduw_le_p(ioaddr);
+#endif
+
+ return ret;
+}
+
+static inline uint32_t host_pci_ldl_le_p(const void *ioaddr)
+{
+ uint32_t ret = 0;
+#ifdef __s390x__
+ ret = le32_to_cpu(s390x_pci_mmio_read_32(ioaddr));
+#else
+ ret = (uint32_t)ldl_le_p(ioaddr);
+#endif
+
+ return ret;
+}
+
+static inline uint64_t host_pci_ldq_le_p(const void *ioaddr)
+{
+ uint64_t ret = 0;
+#ifdef __s390x__
+ ret = le64_to_cpu(s390x_pci_mmio_read_64(ioaddr));
+#else
+ ret = ldq_le_p(ioaddr);
+#endif
+
+ return ret;
+}
+
+static inline void host_pci_stb_le_p(void *ioaddr, uint8_t val)
+{
+
+#ifdef __s390x__
+ s390x_pci_mmio_write_8(ioaddr, val);
+#else
+ stb_p(ioaddr, val);
+#endif
+}
+
+static inline void host_pci_stw_le_p(void *ioaddr, uint16_t val)
+{
+
+#ifdef __s390x__
+ s390x_pci_mmio_write_16(ioaddr, cpu_to_le16(val));
+#else
+ stw_le_p(ioaddr, val);
+#endif
+}
+
+static inline void host_pci_stl_le_p(void *ioaddr, uint32_t val)
+{
+
+#ifdef __s390x__
+ s390x_pci_mmio_write_32(ioaddr, cpu_to_le32(val));
+#else
+ stl_le_p(ioaddr, val);
+#endif
+}
+
+static inline void host_pci_stq_le_p(void *ioaddr, uint64_t val)
+{
+
+#ifdef __s390x__
+ s390x_pci_mmio_write_64(ioaddr, cpu_to_le64(val));
+#else
+ stq_le_p(ioaddr, val);
+#endif
+}
+
+static inline uint64_t host_pci_ldn_le_p(const void *ioaddr, int sz)
+{
+ switch (sz) {
+ case 1:
+ return host_pci_ldub_p(ioaddr);
+ case 2:
+ return host_pci_lduw_le_p(ioaddr);
+ case 4:
+ return host_pci_ldl_le_p(ioaddr);
+ case 8:
+ return host_pci_ldq_le_p(ioaddr);
+ default:
+ g_assert_not_reached();
+ }
+}
+
+static inline void host_pci_stn_le_p(void *ioaddr, int sz, uint64_t v)
+{
+ switch (sz) {
+ case 1:
+ host_pci_stb_le_p(ioaddr, v);
+ break;
+ case 2:
+ host_pci_stw_le_p(ioaddr, v);
+ break;
+ case 4:
+ host_pci_stl_le_p(ioaddr, v);
+ break;
+ case 8:
+ host_pci_stq_le_p(ioaddr, v);
+ break;
+ default:
+ g_assert_not_reached();
+ }
+}
+
+#endif
--
2.43.0On 17/04/2025 19.38, Farhan Ali wrote:
> Add a generic API for host PCI MMIO reads/writes
> (e.g. Linux VFIO BAR accesses). 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/host-pci-mmio.h | 141 +++++++++++++++++++++++++++++++++++
> 1 file changed, 141 insertions(+)
> create mode 100644 include/qemu/host-pci-mmio.h
>
> diff --git a/include/qemu/host-pci-mmio.h b/include/qemu/host-pci-mmio.h
> new file mode 100644
> index 0000000000..c93f77dcd4
> --- /dev/null
> +++ b/include/qemu/host-pci-mmio.h
> @@ -0,0 +1,141 @@
> +/*
> + * API for host PCI MMIO accesses (e.g. Linux VFIO BARs)
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Farhan Ali <alifm@linux.ibm.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HOST_PCI_MMIO_H
> +#define HOST_PCI_MMIO_H
> +
> +#include "qemu/bswap.h"
> +#include "qemu/s390x_pci_mmio.h"
> +
> +
Cosmetic nit: It's more common in QEMU to only use one empty line instead of
two.
> +static inline uint8_t host_pci_ldub_p(const void *ioaddr)
> +{
> + uint8_t ret = 0;
> +#ifdef __s390x__
> + ret = s390x_pci_mmio_read_8(ioaddr);
> +#else
> + ret = ldub_p(ioaddr);
> +#endif
> +
> + return ret;
> +}
> +
> +static inline uint16_t host_pci_lduw_le_p(const void *ioaddr)
> +{
> + uint16_t ret = 0;
> +#ifdef __s390x__
> + ret = le16_to_cpu(s390x_pci_mmio_read_16(ioaddr));
> +#else
> + ret = lduw_le_p(ioaddr);
> +#endif
> +
> + return ret;
> +}
> +
> +static inline uint32_t host_pci_ldl_le_p(const void *ioaddr)
> +{
> + uint32_t ret = 0;
> +#ifdef __s390x__
> + ret = le32_to_cpu(s390x_pci_mmio_read_32(ioaddr));
> +#else
> + ret = (uint32_t)ldl_le_p(ioaddr);
This is the only spot where you used a cast. Is it necessary, or could it be
omitted?
> +#endif
> +
> + return ret;
> +}
> +
> +static inline uint64_t host_pci_ldq_le_p(const void *ioaddr)
> +{
> + uint64_t ret = 0;
> +#ifdef __s390x__
> + ret = le64_to_cpu(s390x_pci_mmio_read_64(ioaddr));
> +#else
> + ret = ldq_le_p(ioaddr);
> +#endif
> +
> + return ret;
> +}
> +
> +static inline void host_pci_stb_le_p(void *ioaddr, uint8_t val)
> +{
> +
Remove the empty line, please.
> +#ifdef __s390x__
> + s390x_pci_mmio_write_8(ioaddr, val);
> +#else
> + stb_p(ioaddr, val);
> +#endif
> +}
> +
> +static inline void host_pci_stw_le_p(void *ioaddr, uint16_t val)
> +{
> +
dito.
> +#ifdef __s390x__
> + s390x_pci_mmio_write_16(ioaddr, cpu_to_le16(val));
> +#else
> + stw_le_p(ioaddr, val);
> +#endif
> +}
> +
> +static inline void host_pci_stl_le_p(void *ioaddr, uint32_t val)
> +{
> +
dito.
> +#ifdef __s390x__
> + s390x_pci_mmio_write_32(ioaddr, cpu_to_le32(val));
> +#else
> + stl_le_p(ioaddr, val);
> +#endif
> +}
> +
> +static inline void host_pci_stq_le_p(void *ioaddr, uint64_t val)
> +{
> +
dito
> +#ifdef __s390x__
> + s390x_pci_mmio_write_64(ioaddr, cpu_to_le64(val));
> +#else
> + stq_le_p(ioaddr, val);
> +#endif
> +}
> +
> +static inline uint64_t host_pci_ldn_le_p(const void *ioaddr, int sz)
> +{
> + switch (sz) {
> + case 1:
> + return host_pci_ldub_p(ioaddr);
> + case 2:
> + return host_pci_lduw_le_p(ioaddr);
> + case 4:
> + return host_pci_ldl_le_p(ioaddr);
> + case 8:
> + return host_pci_ldq_le_p(ioaddr);
> + default:
> + g_assert_not_reached();
> + }
> +}
> +
> +static inline void host_pci_stn_le_p(void *ioaddr, int sz, uint64_t v)
> +{
> + switch (sz) {
> + case 1:
> + host_pci_stb_le_p(ioaddr, v);
> + break;
> + case 2:
> + host_pci_stw_le_p(ioaddr, v);
> + break;
> + case 4:
> + host_pci_stl_le_p(ioaddr, v);
> + break;
> + case 8:
> + host_pci_stq_le_p(ioaddr, v);
> + break;
> + default:
> + g_assert_not_reached();
> + }
> +}
> +
> +#endif
Apart from the nits, patch looks good to me.
Thomas
..snip...
>> +static inline uint32_t host_pci_ldl_le_p(const void *ioaddr)
>> +{
>> + uint32_t ret = 0;
>> +#ifdef __s390x__
>> + ret = le32_to_cpu(s390x_pci_mmio_read_32(ioaddr));
>> +#else
>> + ret = (uint32_t)ldl_le_p(ioaddr);
>
> This is the only spot where you used a cast. Is it necessary, or could
> it be omitted?
Yes, the ldl_le_p returns an int. We do similar cast here
https://github.com/qemu/qemu/blob/73d29ea2417b58ca55fba1aa468ba38e3607b583/include/qemu/bswap.h#L416
>
>> +#endif
>> +
>> + return ret;
>> +}
>> +
>> +static inline uint64_t host_pci_ldq_le_p(const void *ioaddr)
>> +{
>> + uint64_t ret = 0;
>> +#ifdef __s390x__
>> + ret = le64_to_cpu(s390x_pci_mmio_read_64(ioaddr));
>> +#else
>> + ret = ldq_le_p(ioaddr);
>> +#endif
>> +
>> + return ret;
>> +}
>> +
>> +static inline void host_pci_stb_le_p(void *ioaddr, uint8_t val)
>> +{
>> +
>
> Remove the empty line, please.
>
>> +#ifdef __s390x__
>> + s390x_pci_mmio_write_8(ioaddr, val);
>> +#else
>> + stb_p(ioaddr, val);
>> +#endif
>> +}
>> +
>> +static inline void host_pci_stw_le_p(void *ioaddr, uint16_t val)
>> +{
>> +
>
> dito.
>
>> +#ifdef __s390x__
>> + s390x_pci_mmio_write_16(ioaddr, cpu_to_le16(val));
>> +#else
>> + stw_le_p(ioaddr, val);
>> +#endif
>> +}
>> +
>> +static inline void host_pci_stl_le_p(void *ioaddr, uint32_t val)
>> +{
>> +
>
> dito.
>
>> +#ifdef __s390x__
>> + s390x_pci_mmio_write_32(ioaddr, cpu_to_le32(val));
>> +#else
>> + stl_le_p(ioaddr, val);
>> +#endif
>> +}
>> +
>> +static inline void host_pci_stq_le_p(void *ioaddr, uint64_t val)
>> +{
>> +
>
> dito
>
>> +#ifdef __s390x__
>> + s390x_pci_mmio_write_64(ioaddr, cpu_to_le64(val));
>> +#else
>> + stq_le_p(ioaddr, val);
>> +#endif
>> +}
>> +
>> +static inline uint64_t host_pci_ldn_le_p(const void *ioaddr, int sz)
>> +{
>> + switch (sz) {
>> + case 1:
>> + return host_pci_ldub_p(ioaddr);
>> + case 2:
>> + return host_pci_lduw_le_p(ioaddr);
>> + case 4:
>> + return host_pci_ldl_le_p(ioaddr);
>> + case 8:
>> + return host_pci_ldq_le_p(ioaddr);
>> + default:
>> + g_assert_not_reached();
>> + }
>> +}
>> +
>> +static inline void host_pci_stn_le_p(void *ioaddr, int sz, uint64_t v)
>> +{
>> + switch (sz) {
>> + case 1:
>> + host_pci_stb_le_p(ioaddr, v);
>> + break;
>> + case 2:
>> + host_pci_stw_le_p(ioaddr, v);
>> + break;
>> + case 4:
>> + host_pci_stl_le_p(ioaddr, v);
>> + break;
>> + case 8:
>> + host_pci_stq_le_p(ioaddr, v);
>> + break;
>> + default:
>> + g_assert_not_reached();
>> + }
>> +}
>> +
>> +#endif
>
> Apart from the nits, patch looks good to me.
>
> Thomas
Thanks for reviewing! will fix the nits in the next revision.
Thanks
Farhan
On 30/04/2025 18.47, Farhan Ali wrote:
>
> ..snip...
>
>>> +static inline uint32_t host_pci_ldl_le_p(const void *ioaddr)
>>> +{
>>> + uint32_t ret = 0;
>>> +#ifdef __s390x__
>>> + ret = le32_to_cpu(s390x_pci_mmio_read_32(ioaddr));
>>> +#else
>>> + ret = (uint32_t)ldl_le_p(ioaddr);
>>
>> This is the only spot where you used a cast. Is it necessary, or could it
>> be omitted?
>
> Yes, the ldl_le_p returns an int. We do similar cast here https://
> github.com/qemu/qemu/blob/73d29ea2417b58ca55fba1aa468ba38e3607b583/include/
> qemu/bswap.h#L416
... but that function there returns an 64-bit value, while you are assigning
the value to a 32-bit variable here, and you also only return a 32-bit value
from the function here. So there is no way that this could accidentally be
sign-extended, could it?
Thomas
On 4/30/2025 10:52 AM, Thomas Huth wrote:
> On 30/04/2025 18.47, Farhan Ali wrote:
>>
>> ..snip...
>>
>>>> +static inline uint32_t host_pci_ldl_le_p(const void *ioaddr)
>>>> +{
>>>> + uint32_t ret = 0;
>>>> +#ifdef __s390x__
>>>> + ret = le32_to_cpu(s390x_pci_mmio_read_32(ioaddr));
>>>> +#else
>>>> + ret = (uint32_t)ldl_le_p(ioaddr);
>>>
>>> This is the only spot where you used a cast. Is it necessary, or
>>> could it be omitted?
>>
>> Yes, the ldl_le_p returns an int. We do similar cast here https://
>> github.com/qemu/qemu/blob/73d29ea2417b58ca55fba1aa468ba38e3607b583/include/
>> qemu/bswap.h#L416
>
> ... but that function there returns an 64-bit value, while you are
> assigning the value to a 32-bit variable here, and you also only
> return a 32-bit value from the function here. So there is no way that
> this could accidentally be sign-extended, could it?
>
> Thomas
>
>
I checked the Linux kernel and there too we seem to be returning
unsigned 32 bit values (readl) for 32 bit mmio reads. So I think it
should be safe. Do you think we should remove the cast?
Thanks
Farhan
On 30/04/2025 20.32, Farhan Ali wrote:
>
> On 4/30/2025 10:52 AM, Thomas Huth wrote:
>> On 30/04/2025 18.47, Farhan Ali wrote:
>>>
>>> ..snip...
>>>
>>>>> +static inline uint32_t host_pci_ldl_le_p(const void *ioaddr)
>>>>> +{
>>>>> + uint32_t ret = 0;
>>>>> +#ifdef __s390x__
>>>>> + ret = le32_to_cpu(s390x_pci_mmio_read_32(ioaddr));
>>>>> +#else
>>>>> + ret = (uint32_t)ldl_le_p(ioaddr);
>>>>
>>>> This is the only spot where you used a cast. Is it necessary, or could
>>>> it be omitted?
>>>
>>> Yes, the ldl_le_p returns an int. We do similar cast here https://
>>> github.com/qemu/qemu/blob/73d29ea2417b58ca55fba1aa468ba38e3607b583/
>>> include/ qemu/bswap.h#L416
>>
>> ... but that function there returns an 64-bit value, while you are
>> assigning the value to a 32-bit variable here, and you also only return a
>> 32-bit value from the function here. So there is no way that this could
>> accidentally be sign-extended, could it?
>>
>> Thomas
>>
>>
> I checked the Linux kernel and there too we seem to be returning unsigned 32
> bit values (readl) for 32 bit mmio reads. So I think it should be safe. Do
> you think we should remove the cast?
It looks redundant to me, so I'd vote to remove it, yes!
Thomas
On Thu, Apr 17, 2025 at 10:38:00AM -0700, Farhan Ali wrote: > Add a generic API for host PCI MMIO reads/writes > (e.g. Linux VFIO BAR accesses). 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/host-pci-mmio.h | 141 +++++++++++++++++++++++++++++++++++ > 1 file changed, 141 insertions(+) > create mode 100644 include/qemu/host-pci-mmio.h Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
© 2016 - 2025 Red Hat, Inc.