qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/2] Enable QEMU NVMe userspace driver on s390x
@ 2025-03-26 18:10 Farhan Ali
  2025-03-26 18:10 ` [PATCH v1 1/2] util: Add functions for s390x mmio read/write Farhan Ali
  2025-03-26 18:10 ` [PATCH v1 2/2] block/nvme: Enable NVMe userspace driver for s390x Farhan Ali
  0 siblings, 2 replies; 7+ messages in thread
From: Farhan Ali @ 2025-03-26 18:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: alifm, mjrosato, schnelle, qemu-s390x, qemu-block, stefanha, fam,
	philmd, kwolf, hreitz, thuth

Hi,

Recently on s390x we have enabled mmap support for vfio-pci devices [1].
This allows us to take advantage and use userspace drivers on s390x. However,
on s390x we have special instructions for MMIO access. Starting with z15 
(and newer platforms) we have new PCI Memory I/O (MIO) instructions which 
operate on virtually mapped PCI memory spaces, and can be used from userspace.
On older platforms we would fallback to using existing system calls for MMIO access.

This patch series introduces support the PCI MIO instructions, and enables s390x
support for the userspace NVMe driver on s390x. I would appreciate any review/feedback
on the patches.

Thanks
Farhan

[1] https://lore.kernel.org/linux-s390/20250226-vfio_pci_mmap-v7-0-c5c0f1d26efd@linux.ibm.com/

Farhan Ali (2):
  util: Add functions for s390x mmio read/write
  block/nvme: Enable NVMe userspace driver for s390x

 block/nvme.c                  |  95 ++++++++++++++++++++++++------
 include/qemu/s390x_pci_mmio.h |  17 ++++++
 util/meson.build              |   2 +
 util/s390x_pci_mmio.c         | 105 ++++++++++++++++++++++++++++++++++
 4 files changed, 201 insertions(+), 18 deletions(-)
 create mode 100644 include/qemu/s390x_pci_mmio.h
 create mode 100644 util/s390x_pci_mmio.c

-- 
2.43.0



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v1 1/2] util: Add functions for s390x mmio read/write
  2025-03-26 18:10 [PATCH v1 0/2] Enable QEMU NVMe userspace driver on s390x Farhan Ali
@ 2025-03-26 18:10 ` Farhan Ali
  2025-03-27 19:20   ` Stefan Hajnoczi
  2025-03-26 18:10 ` [PATCH v1 2/2] block/nvme: Enable NVMe userspace driver for s390x Farhan Ali
  1 sibling, 1 reply; 7+ messages in thread
From: Farhan Ali @ 2025-03-26 18:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: alifm, mjrosato, schnelle, qemu-s390x, qemu-block, stefanha, fam,
	philmd, kwolf, hreitz, thuth

Starting with z15 (or newer) we can execute mmio
instructions from userspace. On older platforms
where we don't have these instructions available
we can fallback to using system calls to access
the PCI mapped resources.

This patch adds helper functions for mmio reads
and writes for s390x.

Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 include/qemu/s390x_pci_mmio.h |  17 ++++++
 util/meson.build              |   2 +
 util/s390x_pci_mmio.c         | 105 ++++++++++++++++++++++++++++++++++
 3 files changed, 124 insertions(+)
 create mode 100644 include/qemu/s390x_pci_mmio.h
 create mode 100644 util/s390x_pci_mmio.c

diff --git a/include/qemu/s390x_pci_mmio.h b/include/qemu/s390x_pci_mmio.h
new file mode 100644
index 0000000000..be61b5ae29
--- /dev/null
+++ b/include/qemu/s390x_pci_mmio.h
@@ -0,0 +1,17 @@
+/*
+ * s390x PCI MMIO definitions
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Farhan Ali <alifm@linux.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef S390X_PCI_MMIO_H
+#define S390X_PCI_MMIO_H
+
+uint64_t s390x_pci_mmio_read_64(const void *ioaddr);
+uint32_t s390x_pci_mmio_read_32(const void *ioaddr);
+void s390x_pci_mmio_write_64(void *ioaddr, uint64_t val);
+void s390x_pci_mmio_write_32(void *ioaddr, uint32_t val);
+
+#endif
diff --git a/util/meson.build b/util/meson.build
index 780b5977a8..acb21592f9 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -131,4 +131,6 @@ elif cpu in ['ppc', 'ppc64']
   util_ss.add(files('cpuinfo-ppc.c'))
 elif cpu in ['riscv32', 'riscv64']
   util_ss.add(files('cpuinfo-riscv.c'))
+elif cpu == 's390x'
+  util_ss.add(files('s390x_pci_mmio.c'))
 endif
diff --git a/util/s390x_pci_mmio.c b/util/s390x_pci_mmio.c
new file mode 100644
index 0000000000..2e0825d617
--- /dev/null
+++ b/util/s390x_pci_mmio.c
@@ -0,0 +1,105 @@
+/*
+ * s390x PCI MMIO definitions
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Farhan Ali <alifm@linux.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include <unistd.h>
+#include <sys/syscall.h>
+#include "qemu/osdep.h"
+#include "qemu/s390x_pci_mmio.h"
+#include "elf.h"
+
+union register_pair {
+    unsigned __int128 pair;
+    struct {
+        uint64_t even;
+        uint64_t odd;
+    };
+};
+
+static bool is_mio_supported;
+
+static __attribute__((constructor)) void check_is_mio_supported(void)
+{
+    is_mio_supported = !!(qemu_getauxval(AT_HWCAP) & HWCAP_S390_PCI_MIO);
+}
+
+static uint64_t s390x_pcilgi(const void *ioaddr, size_t len)
+{
+    union register_pair ioaddr_len = { .even = (uint64_t)ioaddr,
+                                       .odd = len };
+    uint64_t val;
+    int cc;
+
+    asm volatile(
+        /* pcilgi */
+        ".insn   rre,0xb9d60000,%[val],%[ioaddr_len]\n"
+        "ipm     %[cc]\n"
+        "srl     %[cc],28\n"
+        : [cc] "=d"(cc), [val] "=d"(val),
+        [ioaddr_len] "+&d"(ioaddr_len.pair) :: "cc");
+
+    if (cc) {
+        val = -1ULL;
+    }
+
+    return val;
+}
+
+static void s390x_pcistgi(void *ioaddr, uint64_t val, size_t len)
+{
+    union register_pair ioaddr_len = {.even = (uint64_t)ioaddr, .odd = len};
+
+    asm volatile (
+        /* pcistgi */
+        ".insn   rre,0xb9d40000,%[val],%[ioaddr_len]\n"
+        : [ioaddr_len] "+&d" (ioaddr_len.pair)
+        : [val] "d" (val)
+        : "cc", "memory");
+}
+
+uint32_t s390x_pci_mmio_read_32(const void *ioaddr)
+{
+    uint32_t val = 0;
+
+    if (is_mio_supported) {
+        val = s390x_pcilgi(ioaddr, sizeof(val));
+    } else {
+        syscall(__NR_s390_pci_mmio_read, ioaddr, &val, sizeof(val));
+    }
+    return val;
+}
+
+uint64_t s390x_pci_mmio_read_64(const void *ioaddr)
+{
+    uint64_t val = 0;
+
+    if (is_mio_supported) {
+        val = s390x_pcilgi(ioaddr, sizeof(val));
+    } else {
+        syscall(__NR_s390_pci_mmio_read, ioaddr, &val, sizeof(val));
+    }
+    return val;
+}
+
+void s390x_pci_mmio_write_64(void *ioaddr, uint64_t val)
+{
+    if (is_mio_supported) {
+        s390x_pcistgi(ioaddr, val, sizeof(val));
+    } else {
+        syscall(__NR_s390_pci_mmio_write, ioaddr, &val, sizeof(val));
+    }
+}
+
+void s390x_pci_mmio_write_32(void *ioaddr, uint32_t val)
+{
+    if (is_mio_supported) {
+        s390x_pcistgi(ioaddr, val, sizeof(val));
+    } else {
+        syscall(__NR_s390_pci_mmio_write, ioaddr, &val, sizeof(val));
+    }
+}
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v1 2/2] block/nvme: Enable NVMe userspace driver for s390x
  2025-03-26 18:10 [PATCH v1 0/2] Enable QEMU NVMe userspace driver on s390x Farhan Ali
  2025-03-26 18:10 ` [PATCH v1 1/2] util: Add functions for s390x mmio read/write Farhan Ali
@ 2025-03-26 18:10 ` Farhan Ali
  2025-03-27 19:26   ` Stefan Hajnoczi
  1 sibling, 1 reply; 7+ messages in thread
From: Farhan Ali @ 2025-03-26 18:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: alifm, mjrosato, schnelle, qemu-s390x, qemu-block, stefanha, fam,
	philmd, kwolf, hreitz, thuth

On s390x we can now support userspace mmio and mmap
from vfio. This patch uses s390x mmio support to
enable the NVMe userspace driver for s390x.

Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 block/nvme.c | 95 ++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 77 insertions(+), 18 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index bbf7c23dcd..90f5708d9b 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -24,6 +24,9 @@
 #include "qemu/option.h"
 #include "qemu/memalign.h"
 #include "qemu/vfio-helpers.h"
+#ifdef __s390x__
+#include "qemu/s390x_pci_mmio.h"
+#endif
 #include "block/block-io.h"
 #include "block/block_int.h"
 #include "system/block-backend.h"
@@ -60,7 +63,7 @@ typedef struct {
     uint8_t  *queue;
     uint64_t iova;
     /* Hardware MMIO register */
-    volatile uint32_t *doorbell;
+    uint32_t *doorbell;
 } NVMeQueue;
 
 typedef struct {
@@ -100,7 +103,7 @@ struct BDRVNVMeState {
     QEMUVFIOState *vfio;
     void *bar0_wo_map;
     /* Memory mapped registers */
-    volatile struct {
+    struct {
         uint32_t sq_tail;
         uint32_t cq_head;
     } *doorbells;
@@ -164,6 +167,58 @@ static QemuOptsList runtime_opts = {
     },
 };
 
+static inline uint32_t nvme_mmio_read_32(const void *addr)
+{
+    uint32_t ret;
+
+#ifdef __s390x__
+    ret = s390x_pci_mmio_read_32(addr);
+#else
+    /* Prevent the compiler from optimizing away the load */
+    ret = *((volatile uint32_t *)addr);
+#endif
+
+    return le32_to_cpu(ret);
+}
+
+static inline uint64_t nvme_mmio_read_64(const void *addr)
+{
+    uint64_t ret;
+
+#ifdef __s390x__
+    ret = s390x_pci_mmio_read_64(addr);
+#else
+    /* Prevent the compiler from optimizing away the load */
+    ret = *((volatile uint64_t *)addr);
+#endif
+
+    return le64_to_cpu(ret);
+}
+
+static inline void nvme_mmio_write_32(void *addr, uint32_t val)
+{
+    val = cpu_to_le32(val);
+
+#ifdef __s390x__
+    s390x_pci_mmio_write_32(addr, val);
+#else
+    /* Prevent the compiler from optimizing away the store */
+    *((volatile uint32_t *)addr) = val;
+#endif
+}
+
+static inline void nvme_mmio_write_64(void *addr, uint64_t val)
+{
+    val = cpu_to_le64(val);
+
+#ifdef __s390x__
+    s390x_pci_mmio_write_64(addr, val);
+#else
+    /* Prevent the compiler from optimizing away the store */
+    *((volatile uint64_t *)addr) = val;
+#endif
+}
+
 /* Returns true on success, false on failure. */
 static bool nvme_init_queue(BDRVNVMeState *s, NVMeQueue *q,
                             unsigned nentries, size_t entry_bytes, Error **errp)
@@ -292,7 +347,7 @@ static void nvme_kick(NVMeQueuePair *q)
     assert(!(q->sq.tail & 0xFF00));
     /* Fence the write to submission queue entry before notifying the device. */
     smp_wmb();
-    *q->sq.doorbell = cpu_to_le32(q->sq.tail);
+    nvme_mmio_write_32(q->sq.doorbell, q->sq.tail);
     q->inflight += q->need_kick;
     q->need_kick = 0;
 }
@@ -441,7 +496,7 @@ static bool nvme_process_completion(NVMeQueuePair *q)
     if (progress) {
         /* Notify the device so it can post more completions. */
         smp_mb_release();
-        *q->cq.doorbell = cpu_to_le32(q->cq.head);
+        nvme_mmio_write_32(q->cq.doorbell, q->cq.head);
         nvme_wake_free_req_locked(q);
     }
 
@@ -460,7 +515,7 @@ static void nvme_process_completion_bh(void *opaque)
      * so notify the device that it has space to fill in more completions now.
      */
     smp_mb_release();
-    *q->cq.doorbell = cpu_to_le32(q->cq.head);
+    nvme_mmio_write_32(q->cq.doorbell, q->cq.head);
     nvme_wake_free_req_locked(q);
 
     nvme_process_completion(q);
@@ -749,9 +804,10 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     int ret;
     uint64_t cap;
     uint32_t ver;
+    uint32_t cc;
     uint64_t timeout_ms;
     uint64_t deadline, now;
-    volatile NvmeBar *regs = NULL;
+    NvmeBar *regs = NULL;
 
     qemu_co_mutex_init(&s->dma_map_lock);
     qemu_co_queue_init(&s->dma_flush_queue);
@@ -779,7 +835,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     /* Perform initialize sequence as described in NVMe spec "7.6.1
      * Initialization". */
 
-    cap = le64_to_cpu(regs->cap);
+    cap = nvme_mmio_read_64(&regs->cap);
     trace_nvme_controller_capability_raw(cap);
     trace_nvme_controller_capability("Maximum Queue Entries Supported",
                                      1 + NVME_CAP_MQES(cap));
@@ -805,16 +861,17 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     bs->bl.request_alignment = s->page_size;
     timeout_ms = MIN(500 * NVME_CAP_TO(cap), 30000);
 
-    ver = le32_to_cpu(regs->vs);
+    ver = nvme_mmio_read_32(&regs->vs);
     trace_nvme_controller_spec_version(extract32(ver, 16, 16),
                                        extract32(ver, 8, 8),
                                        extract32(ver, 0, 8));
 
     /* Reset device to get a clean state. */
-    regs->cc = cpu_to_le32(le32_to_cpu(regs->cc) & 0xFE);
+    cc = nvme_mmio_read_32(&regs->cc);
+    nvme_mmio_write_32(&regs->cc, (cc & 0xFE));
     /* Wait for CSTS.RDY = 0. */
     deadline = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + timeout_ms * SCALE_MS;
-    while (NVME_CSTS_RDY(le32_to_cpu(regs->csts))) {
+    while (NVME_CSTS_RDY(nvme_mmio_read_32(&regs->csts))) {
         if (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) > deadline) {
             error_setg(errp, "Timeout while waiting for device to reset (%"
                              PRId64 " ms)",
@@ -843,19 +900,21 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     s->queues[INDEX_ADMIN] = q;
     s->queue_count = 1;
     QEMU_BUILD_BUG_ON((NVME_QUEUE_SIZE - 1) & 0xF000);
-    regs->aqa = cpu_to_le32(((NVME_QUEUE_SIZE - 1) << AQA_ACQS_SHIFT) |
-                            ((NVME_QUEUE_SIZE - 1) << AQA_ASQS_SHIFT));
-    regs->asq = cpu_to_le64(q->sq.iova);
-    regs->acq = cpu_to_le64(q->cq.iova);
+    nvme_mmio_write_32(&regs->aqa,
+                       ((NVME_QUEUE_SIZE - 1) << AQA_ACQS_SHIFT) |
+                       ((NVME_QUEUE_SIZE - 1) << AQA_ASQS_SHIFT));
+    nvme_mmio_write_64(&regs->asq, q->sq.iova);
+    nvme_mmio_write_64(&regs->acq, q->cq.iova);
 
     /* After setting up all control registers we can enable device now. */
-    regs->cc = cpu_to_le32((ctz32(NVME_CQ_ENTRY_BYTES) << CC_IOCQES_SHIFT) |
-                           (ctz32(NVME_SQ_ENTRY_BYTES) << CC_IOSQES_SHIFT) |
-                           CC_EN_MASK);
+    nvme_mmio_write_32(&regs->cc,
+                    ((ctz32(NVME_CQ_ENTRY_BYTES) << CC_IOCQES_SHIFT) |
+                    (ctz32(NVME_SQ_ENTRY_BYTES) << CC_IOSQES_SHIFT) |
+                    CC_EN_MASK));
     /* Wait for CSTS.RDY = 1. */
     now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
     deadline = now + timeout_ms * SCALE_MS;
-    while (!NVME_CSTS_RDY(le32_to_cpu(regs->csts))) {
+    while (!NVME_CSTS_RDY(nvme_mmio_read_32(&regs->csts))) {
         if (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) > deadline) {
             error_setg(errp, "Timeout while waiting for device to start (%"
                              PRId64 " ms)",
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v1 1/2] util: Add functions for s390x mmio read/write
  2025-03-26 18:10 ` [PATCH v1 1/2] util: Add functions for s390x mmio read/write Farhan Ali
@ 2025-03-27 19:20   ` Stefan Hajnoczi
  2025-03-27 19:53     ` Farhan Ali
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Hajnoczi @ 2025-03-27 19:20 UTC (permalink / raw)
  To: Farhan Ali
  Cc: qemu-devel, mjrosato, schnelle, qemu-s390x, qemu-block, fam,
	philmd, kwolf, hreitz, thuth

[-- Attachment #1: Type: text/plain, Size: 2776 bytes --]

On Wed, Mar 26, 2025 at 11:10:06AM -0700, Farhan Ali wrote:
> Starting with z15 (or newer) we can execute mmio
> instructions from userspace. On older platforms
> where we don't have these instructions available
> we can fallback to using system calls to access
> the PCI mapped resources.
> 
> This patch adds helper functions for mmio reads
> and writes for s390x.
> 
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
> ---
>  include/qemu/s390x_pci_mmio.h |  17 ++++++
>  util/meson.build              |   2 +
>  util/s390x_pci_mmio.c         | 105 ++++++++++++++++++++++++++++++++++
>  3 files changed, 124 insertions(+)
>  create mode 100644 include/qemu/s390x_pci_mmio.h
>  create mode 100644 util/s390x_pci_mmio.c
> 
> diff --git a/include/qemu/s390x_pci_mmio.h b/include/qemu/s390x_pci_mmio.h
> new file mode 100644
> index 0000000000..be61b5ae29
> --- /dev/null
> +++ b/include/qemu/s390x_pci_mmio.h
> @@ -0,0 +1,17 @@
> +/*
> + * s390x PCI MMIO definitions
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Farhan Ali <alifm@linux.ibm.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef S390X_PCI_MMIO_H
> +#define S390X_PCI_MMIO_H
> +
> +uint64_t s390x_pci_mmio_read_64(const void *ioaddr);
> +uint32_t s390x_pci_mmio_read_32(const void *ioaddr);
> +void s390x_pci_mmio_write_64(void *ioaddr, uint64_t val);
> +void s390x_pci_mmio_write_32(void *ioaddr, uint32_t val);
> +
> +#endif
> diff --git a/util/meson.build b/util/meson.build
> index 780b5977a8..acb21592f9 100644
> --- a/util/meson.build
> +++ b/util/meson.build
> @@ -131,4 +131,6 @@ elif cpu in ['ppc', 'ppc64']
>    util_ss.add(files('cpuinfo-ppc.c'))
>  elif cpu in ['riscv32', 'riscv64']
>    util_ss.add(files('cpuinfo-riscv.c'))
> +elif cpu == 's390x'
> +  util_ss.add(files('s390x_pci_mmio.c'))
>  endif
> diff --git a/util/s390x_pci_mmio.c b/util/s390x_pci_mmio.c
> new file mode 100644
> index 0000000000..2e0825d617
> --- /dev/null
> +++ b/util/s390x_pci_mmio.c
> @@ -0,0 +1,105 @@
> +/*
> + * s390x PCI MMIO definitions
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Farhan Ali <alifm@linux.ibm.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include <unistd.h>
> +#include <sys/syscall.h>
> +#include "qemu/osdep.h"

This should be the first #include in the file. From
docs/devel/style.rst:

  Include directives
  ------------------
  
  Order include directives as follows:
  
  .. code-block:: c
  
      #include "qemu/osdep.h"  /* Always first... */
      #include <...>           /* then system headers... */
      #include "..."           /* and finally QEMU headers. */

Otherwise:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1 2/2] block/nvme: Enable NVMe userspace driver for s390x
  2025-03-26 18:10 ` [PATCH v1 2/2] block/nvme: Enable NVMe userspace driver for s390x Farhan Ali
@ 2025-03-27 19:26   ` Stefan Hajnoczi
  2025-03-27 19:49     ` Farhan Ali
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Hajnoczi @ 2025-03-27 19:26 UTC (permalink / raw)
  To: Farhan Ali
  Cc: qemu-devel, mjrosato, schnelle, qemu-s390x, qemu-block, fam,
	philmd, kwolf, hreitz, thuth

[-- Attachment #1: Type: text/plain, Size: 714 bytes --]

On Wed, Mar 26, 2025 at 11:10:07AM -0700, Farhan Ali wrote:
> +static inline uint32_t nvme_mmio_read_32(const void *addr)
> +{
> +    uint32_t ret;
> +
> +#ifdef __s390x__
> +    ret = s390x_pci_mmio_read_32(addr);
> +#else
> +    /* Prevent the compiler from optimizing away the load */
> +    ret = *((volatile uint32_t *)addr);
> +#endif

The NVMe driver should not need to worry about s390 PCI MMIO specifics.
It would be nice to add a QEMU PCI MMIO load/store API that hides this.

block/nvme.c would use the API to access MMIO registers instead of
accessing them directly. The functions could be static inline in the
header file to minimize overhead on platforms that can directly access
PCI MMIO registers.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1 2/2] block/nvme: Enable NVMe userspace driver for s390x
  2025-03-27 19:26   ` Stefan Hajnoczi
@ 2025-03-27 19:49     ` Farhan Ali
  0 siblings, 0 replies; 7+ messages in thread
From: Farhan Ali @ 2025-03-27 19:49 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: qemu-devel, mjrosato, schnelle, qemu-s390x, qemu-block, fam,
	philmd, kwolf, hreitz, thuth


On 3/27/2025 12:26 PM, Stefan Hajnoczi wrote:
> On Wed, Mar 26, 2025 at 11:10:07AM -0700, Farhan Ali wrote:
>> +static inline uint32_t nvme_mmio_read_32(const void *addr)
>> +{
>> +    uint32_t ret;
>> +
>> +#ifdef __s390x__
>> +    ret = s390x_pci_mmio_read_32(addr);
>> +#else
>> +    /* Prevent the compiler from optimizing away the load */
>> +    ret = *((volatile uint32_t *)addr);
>> +#endif
> The NVMe driver should not need to worry about s390 PCI MMIO specifics.
> It would be nice to add a QEMU PCI MMIO load/store API that hides this.
>
> block/nvme.c would use the API to access MMIO registers instead of
> accessing them directly. The functions could be static inline in the
> header file to minimize overhead on platforms that can directly access
> PCI MMIO registers.

Hi Stefan,

Thanks for reviewing. I do agree, having a generic QEMU PCI MMIO 
load/store API for this would be good. And we can hide architecture 
specifics there. I didn't add that to begin with was NVMe driver would 
be the only place to consume it so thought might be unnecessary. I will 
refactor this in v2.

Thanks

Farhan




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1 1/2] util: Add functions for s390x mmio read/write
  2025-03-27 19:20   ` Stefan Hajnoczi
@ 2025-03-27 19:53     ` Farhan Ali
  0 siblings, 0 replies; 7+ messages in thread
From: Farhan Ali @ 2025-03-27 19:53 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: qemu-devel, mjrosato, schnelle, qemu-s390x, qemu-block, fam,
	philmd, kwolf, hreitz, thuth


On 3/27/2025 12:20 PM, Stefan Hajnoczi wrote:
> On Wed, Mar 26, 2025 at 11:10:06AM -0700, Farhan Ali wrote:
>> Starting with z15 (or newer) we can execute mmio
>> instructions from userspace. On older platforms
>> where we don't have these instructions available
>> we can fallback to using system calls to access
>> the PCI mapped resources.
>>
>> This patch adds helper functions for mmio reads
>> and writes for s390x.
>>
>> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
>> ---
>>   include/qemu/s390x_pci_mmio.h |  17 ++++++
>>   util/meson.build              |   2 +
>>   util/s390x_pci_mmio.c         | 105 ++++++++++++++++++++++++++++++++++
>>   3 files changed, 124 insertions(+)
>>   create mode 100644 include/qemu/s390x_pci_mmio.h
>>   create mode 100644 util/s390x_pci_mmio.c
>>
>> diff --git a/include/qemu/s390x_pci_mmio.h b/include/qemu/s390x_pci_mmio.h
>> new file mode 100644
>> index 0000000000..be61b5ae29
>> --- /dev/null
>> +++ b/include/qemu/s390x_pci_mmio.h
>> @@ -0,0 +1,17 @@
>> +/*
>> + * s390x PCI MMIO definitions
>> + *
>> + * Copyright 2025 IBM Corp.
>> + * Author(s): Farhan Ali <alifm@linux.ibm.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +#ifndef S390X_PCI_MMIO_H
>> +#define S390X_PCI_MMIO_H
>> +
>> +uint64_t s390x_pci_mmio_read_64(const void *ioaddr);
>> +uint32_t s390x_pci_mmio_read_32(const void *ioaddr);
>> +void s390x_pci_mmio_write_64(void *ioaddr, uint64_t val);
>> +void s390x_pci_mmio_write_32(void *ioaddr, uint32_t val);
>> +
>> +#endif
>> diff --git a/util/meson.build b/util/meson.build
>> index 780b5977a8..acb21592f9 100644
>> --- a/util/meson.build
>> +++ b/util/meson.build
>> @@ -131,4 +131,6 @@ elif cpu in ['ppc', 'ppc64']
>>     util_ss.add(files('cpuinfo-ppc.c'))
>>   elif cpu in ['riscv32', 'riscv64']
>>     util_ss.add(files('cpuinfo-riscv.c'))
>> +elif cpu == 's390x'
>> +  util_ss.add(files('s390x_pci_mmio.c'))
>>   endif
>> diff --git a/util/s390x_pci_mmio.c b/util/s390x_pci_mmio.c
>> new file mode 100644
>> index 0000000000..2e0825d617
>> --- /dev/null
>> +++ b/util/s390x_pci_mmio.c
>> @@ -0,0 +1,105 @@
>> +/*
>> + * s390x PCI MMIO definitions
>> + *
>> + * Copyright 2025 IBM Corp.
>> + * Author(s): Farhan Ali <alifm@linux.ibm.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include <unistd.h>
>> +#include <sys/syscall.h>
>> +#include "qemu/osdep.h"
> This should be the first #include in the file. From
> docs/devel/style.rst:
>
>    Include directives
>    ------------------
>    
>    Order include directives as follows:
>    
>    .. code-block:: c
>    
>        #include "qemu/osdep.h"  /* Always first... */
>        #include <...>           /* then system headers... */
>        #include "..."           /* and finally QEMU headers. */
>
> Otherwise:
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Will fix in next revision. Thanks!




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-03-27 19:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-26 18:10 [PATCH v1 0/2] Enable QEMU NVMe userspace driver on s390x Farhan Ali
2025-03-26 18:10 ` [PATCH v1 1/2] util: Add functions for s390x mmio read/write Farhan Ali
2025-03-27 19:20   ` Stefan Hajnoczi
2025-03-27 19:53     ` Farhan Ali
2025-03-26 18:10 ` [PATCH v1 2/2] block/nvme: Enable NVMe userspace driver for s390x Farhan Ali
2025-03-27 19:26   ` Stefan Hajnoczi
2025-03-27 19:49     ` Farhan Ali

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).