qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH v11 3/4] hw/vfio/ap: Storing event information for an AP

From: Rorie Reyes
Subject: Re: [RFC PATCH v11 3/4] hw/vfio/ap: Storing event information for an AP configuration change event
Date: Tue, 3 Jun 2025 14:01:36 -0400
User-agent: Mozilla Thunderbird 

On 6/3/25 10:21 AM, Cédric Le Goater wrote:

On 6/3/25 14:58, Rorie Reyes wrote:

Hey Cedric,

You mentioned the following in my v9 patches
"In that case, let's keep it simple (no mutex) and add a assert(bql_locked()) 
statement where we think the bql should be protecting access to shared
resources. "

Does this still apply down bellow?


Anthony replied :

https://lore.kernel.org/qemu-devel/ed2a2aa3-68a7-480c-a6a4-a8219af12d7b@linux.ibm.com/

Thanks,

C.


So we'll still use WITH_QEMU_LOCK_GUARD?


On 5/26/25 4:40 AM, Cédric Le Goater wrote:

On 5/23/25 18:03, Rorie Reyes wrote:
These functions can be invoked by the function that handles interception of the CHSC SEI instruction for requests indicating the accessibility of 
one or more adjunct processors has changed.

Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
---
  hw/vfio/ap.c                 | 53 ++++++++++++++++++++++++++++++++++++ 
  include/hw/s390x/ap-bridge.h | 39 ++++++++++++++++++++++++++
  2 files changed, 92 insertions(+)

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index fc435f5c5b..97a42a575a 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -10,6 +10,7 @@
   * directory.
   */
  +#include <stdbool.h>
  #include "qemu/osdep.h"
  #include CONFIG_DEVICES /* CONFIG_IOMMUFD */
  #include <linux/vfio.h>
@@ -48,6 +49,8 @@ typedef struct APConfigChgEvent {
  static QTAILQ_HEAD(, APConfigChgEvent) cfg_chg_events =
      QTAILQ_HEAD_INITIALIZER(cfg_chg_events);
  +static QemuMutex cfg_chg_events_lock;
+
  OBJECT_DECLARE_SIMPLE_TYPE(VFIOAPDevice, VFIO_AP_DEVICE)
    static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
@@ -96,6 +99,49 @@ static void vfio_ap_cfg_chg_notifier_handler(void *opaque) 
    }
  +int ap_chsc_sei_nt0_get_event(void *res)
+{
+    ChscSeiNt0Res *nt0_res  = (ChscSeiNt0Res *)res;
+    APConfigChgEvent *cfg_chg_event;
+
+    qemu_mutex_lock(&cfg_chg_events_lock);


please consider using WITH_QEMU_LOCK_GUARD()


See note above about bql_locked

+    if (!ap_chsc_sei_nt0_have_event()) {
+        qemu_mutex_unlock(&cfg_chg_events_lock);
+        return EVENT_INFORMATION_NOT_STORED;
+    }
+
+    cfg_chg_event = QTAILQ_FIRST(&cfg_chg_events);
+    QTAILQ_REMOVE(&cfg_chg_events, cfg_chg_event, next);
+
+    qemu_mutex_unlock(&cfg_chg_events_lock);
+
+    memset(nt0_res, 0, sizeof(*nt0_res));
+    g_free(cfg_chg_event);
+
+    /*
+     * If there are any AP configuration change events in the queue,
+     * indicate to the caller that there is pending event info in
+     * the response block
+     */
+    if (ap_chsc_sei_nt0_have_event()) {
+        nt0_res->flags |= PENDING_EVENT_INFO_BITMASK;
+    }
+
+    nt0_res->length = sizeof(ChscSeiNt0Res);
+    nt0_res->code = NT0_RES_RESPONSE_CODE;
+    nt0_res->nt = NT0_RES_NT_DEFAULT;
+    nt0_res->rs = NT0_RES_RS_AP_CHANGE;
+    nt0_res->cc = NT0_RES_CC_AP_CHANGE;
+
+    return EVENT_INFORMATION_STORED;
+}
+
+bool ap_chsc_sei_nt0_have_event(void)


hmm, no locking ?


See not above for bql_locked

+{
+    return !QTAILQ_EMPTY(&cfg_chg_events);
+}
+
  static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
                                            unsigned int irq, Error **errp) 
  {
@@ -192,6 +238,13 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp) 
      VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev);
      VFIODevice *vbasedev = &vapdev->vdev;
  +    static bool lock_initialized;
+
+    if (!lock_initialized) {
+        qemu_mutex_init(&cfg_chg_events_lock);
+        lock_initialized = true;
+    }


this could be replaced with a constructor routine. See hyperv.


Thanks,

C.


Noted




      if (!vfio_device_get_name(vbasedev, errp)) {
          return;
      }
diff --git a/include/hw/s390x/ap-bridge.h b/include/hw/s390x/ap-bridge.h 
index 470e439a98..7efc52928d 100644
--- a/include/hw/s390x/ap-bridge.h
+++ b/include/hw/s390x/ap-bridge.h
@@ -16,4 +16,43 @@
    void s390_init_ap(void);
  +typedef struct ChscSeiNt0Res {
+    uint16_t length;
+    uint16_t code;
+    uint8_t reserved1;
+    uint16_t reserved2;
+    uint8_t nt;
+#define PENDING_EVENT_INFO_BITMASK 0x80;
+    uint8_t flags;
+    uint8_t reserved3;
+    uint8_t rs;
+    uint8_t cc;
+} QEMU_PACKED ChscSeiNt0Res;
+
+#define NT0_RES_RESPONSE_CODE 1
+#define NT0_RES_NT_DEFAULT    0
+#define NT0_RES_RS_AP_CHANGE  5
+#define NT0_RES_CC_AP_CHANGE  3
+
+#define EVENT_INFORMATION_NOT_STORED 1
+#define EVENT_INFORMATION_STORED     0
+
+/**
+ * ap_chsc_sei_nt0_get_event - Retrieve the next pending AP config
+ * change event
+ * @res: Pointer to a ChscSeiNt0Res struct to be filled with event
+ * data
+ *
+ * This function checks for any pending AP config change events and,
+ * if present, populates the provided response structure with the
+ * appropriate SEI NT0 fields.
+ *
+ * Return:
+ *   EVENT_INFORMATION_STORED - An event was available and written to @res 
+ *   EVENT_INFORMATION_NOT_STORED - No event was available
+ */
+int ap_chsc_sei_nt0_get_event(void *res);
+
+bool ap_chsc_sei_nt0_have_event(void);
+>   #endif
reply via email to
[Prev in Thread] Current Thread [Next in Thread]