On 6/6/25 7:48 AM, Anthony Krowiak wrote:
On 6/5/25 1:57 PM, Rorie Reyes wrote:
On 6/4/25 9:47 AM, Anthony Krowiak wrote:
On 6/3/25 4:30 PM, Cédric Le Goater wrote:
On 6/3/25 20:01, Rorie Reyes wrote:
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-a8219af12...@linux.ibm.com/
Thanks,
C.
So we'll still use WITH_QEMU_LOCK_GUARD?
If a lock is needed to protect the list, then
ap_chsc_sei_nt0_have_event()
should lock/unlock too. WITH_QEMU_LOCK_GUARD() is just a pratical
way to
do so.
Since ap_chsc_sei_nt0_have_event() is a single line that returns
!QTAILQ_EMPTY(&cfg_chg_events), wouldn't it be better to just
use the QEMU_LOCK_GUARD macro which, if I'm not mistaken,
will unlock on the return statement?
Thanks,
C.
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 <rre...@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 ?
How important do we need to lock this? When I lock this method my
guest freezes every time. But when I only lock the get event, my code
continues to work as designed
Try locking in both functions, but instead of calling the have event
function from the get event function, use
!QTAILQ_EMPTY(&cfg_chg_events) in the get event function. If you are
holding the lock when you call the have event function, you will be
locking the same lock twice and it will hang when you lock the second
time because the lock is already held.
Let me give that a shot. Thanks Tony
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)
