On 5/26/25 4:43 AM, Cédric Le Goater wrote:
On 5/22/25 20:55, Anthony Krowiak wrote:



On 5/22/25 9:30 AM, Cédric Le Goater wrote:
On 5/12/25 20:02, 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                 | 39 ++++++++++++++++++++++++++++++++++++
  include/hw/s390x/ap-bridge.h | 22 ++++++++++++++++++++
  2 files changed, 61 insertions(+)

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index 5ea5dd9cca..4f88f80c54 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -96,6 +96,45 @@ 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;
+
+    if (!ap_chsc_sei_nt0_have_event()) {
+        return 1;
+    }
+
+    cfg_chg_event = QTAILQ_FIRST(&cfg_chg_events);
+    memset(nt0_res, 0, sizeof(*nt0_res));
+
+    QTAILQ_REMOVE(&cfg_chg_events, cfg_chg_event, next);

btw, I don't know if this was discussed. Are we OK to manipulate the
'cfg_chg_events' construct withou locking ?

This has never been discussed, but it's an interesting question. The
ap_chsc_sei_nt0_get_event and ap_chsc_sei_nt0_have_event functions
are called as a result of a SIE exit to handle interception of a
CHSC SEI instruction. Handling of the intercepted instructions is
done under the Big QEMU Lock (see kvm_arch_handle_exit in target/s390x/kvm/kvm.c),
so presumably no other processes will get access to these functions
until the instruction is handled.

On the other hand, the vfio_cfg_chg_notifier_handler function that handles the eventfd indicating the guest's AP configuration has been changed by the host device driver adds  APConfigChgEvent objects to this queue. If, however, you think about the flow, when the notifier handler gets called to handle an AP config changed event, it queues a channel request word (CRW) indicating there is an SEI pending. Consequently, the ap_chsc_sei_nt0_get_event and ap_chsc_sei_nt0_have_event functions will get called only after the guest receives the CRW event and executes the CHSC SEI instruction. Since the Big QEMU Lock is taken when the CHSC SE instruction is intercepted, it can not proceed until whatever the holding process releases it; so for that flow, it seems highly likely if not impossible for conflict given the event will always be added to the queue before an attempt
can be made to retrieve it.

Having gone through this dissertation, I don't see how it can hurt to lock the queue when it is being accessed and would certainly make things bullet proof. What is your opinion?

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

I don't think we want to do that in the vfio_cfg_chg_notifier_handler function because that function is called when the host device driver signals an eventfd to notify userspace that the guest's AP configuration has been changed and I don't think the
Big QEMU Lock is taken during that process.

resources.

Thanks,

C.



Reply via email to