[RFC PATCH v4 0/5] Report vfio-ap configuration changes

[RFC PATCH v4 0/5] Report vfio-ap configuration changes

[Rorie Reyes]

Changelog:

v4:
- allocating cfg_chg_event before inserting into the queue
- calling nt0_have_event in if loop to check if there are any
elemenets in the queue, then calling QTAILQ_FIRST when the check
passes
- moving memset() after the check

v3:
- changes that were made to patch 3/5 should have been made in
patch 2/5

v2:
- removed warnings that weren't needed
- added unregister function
- removed whitelines
- changed variable names for consistency
- removed rc variable and returning 1 or 0 outright
- reversed logics for if statements
- using g_free() instead of free()
- replaced hardcoded numeric values by defining them with #define
in the header

--------------------------------------------------------------------------
This patch series creates and registers a handler that is called when
userspace is notified by the kernel that a guest's AP configuration has
changed. The handler in turn notifies the guest that its AP configuration
has changed. This allows the guest to immediately respond to AP
configuration changes rather than relying on polling or some other
inefficient mechanism for detecting config changes.

Rorie Reyes (5):
  linux-headers: NOTFORMERGE - placeholder uapi updates for AP config
    change
  hw/vfio/ap: notification handler for AP config changed event
  hw/vfio/ap: store object indicating AP config changed in a queue
  hw/vfio/ap: Storing event information for an AP configuration change
    event
  s390: implementing CHSC SEI for AP config change

 hw/vfio/ap.c                 | 82 ++++++++++++++++++++++++++++++++++++
 include/hw/s390x/ap-bridge.h | 22 ++++++++++
 linux-headers/linux/vfio.h   |  1 +
 target/s390x/ioinst.c        | 11 ++++-
 4 files changed, 114 insertions(+), 2 deletions(-)

-- 
2.48.1

[RFC PATCH v4 1/5] linux-headers: NOTFORMERGE - placeholder uapi updates for AP config change

[Rorie Reyes]

This patch adds enumeration constant VFIO_AP_CFG_CHG_IRQ_INDEX to specify
an IRQ index for signaling that a change has been made to the guest's AP
configuration. This is a placeholder for QEMU patches that use this value
since it is a linux-headers update which includes changes that aren't
merged into the kernel. Linux-headers patches should be generated using
scripts/update-linux-headers.sh.

Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
---
 linux-headers/linux/vfio.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index 1b5e254d6a..d0426b5ec0 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -671,6 +671,7 @@ enum {
  */
 enum {
 	VFIO_AP_REQ_IRQ_INDEX,
+	VFIO_AP_CFG_CHG_IRQ_INDEX,
 	VFIO_AP_NUM_IRQS
 };
 
-- 
2.48.1

[RFC PATCH v4 2/5] hw/vfio/ap: notification handler for AP config changed event

[Rorie Reyes]

Register an event notifier handler to process AP configuration
change events by queuing the event and generating a CRW to let
the guest know its AP configuration has changed

Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
Reviewed-by: Anthony Krowiak <akrowiak@linux.ibm.com>
---
 hw/vfio/ap.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index c7ab4ff57a..3614657218 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -18,6 +18,7 @@
 #include "hw/vfio/vfio-common.h"
 #include "system/iommufd.h"
 #include "hw/s390x/ap-device.h"
+#include "hw/s390x/css.h"
 #include "qemu/error-report.h"
 #include "qemu/event_notifier.h"
 #include "qemu/main-loop.h"
@@ -37,6 +38,7 @@ struct VFIOAPDevice {
     APDevice apdev;
     VFIODevice vdev;
     EventNotifier req_notifier;
+    EventNotifier cfg_notifier;
 };
 
 OBJECT_DECLARE_SIMPLE_TYPE(VFIOAPDevice, VFIO_AP_DEVICE)
@@ -70,6 +72,18 @@ static void vfio_ap_req_notifier_handler(void *opaque)
     }
 }
 
+static void vfio_ap_cfg_chg_notifier_handler(void *opaque)
+{
+    VFIOAPDevice *vapdev = opaque;
+
+    if (!event_notifier_test_and_clear(&vapdev->cfg_notifier)) {
+        return;
+    }
+
+    css_generate_css_crws(0);
+
+}
+
 static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
                                           unsigned int irq, Error **errp)
 {
@@ -85,6 +99,10 @@ static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
         notifier = &vapdev->req_notifier;
         fd_read = vfio_ap_req_notifier_handler;
         break;
+    case VFIO_AP_CFG_CHG_IRQ_INDEX:
+        notifier = &vapdev->cfg_notifier;
+        fd_read = vfio_ap_cfg_chg_notifier_handler;
+        break;
     default:
         error_setg(errp, "vfio: Unsupported device irq(%d)", irq);
         return false;
@@ -136,6 +154,9 @@ static void vfio_ap_unregister_irq_notifier(VFIOAPDevice *vapdev,
     case VFIO_AP_REQ_IRQ_INDEX:
         notifier = &vapdev->req_notifier;
         break;
+    case VFIO_AP_CFG_CHG_IRQ_INDEX:
+        notifier = &vapdev->cfg_notifier;
+        break;
     default:
         error_report("vfio: Unsupported device irq(%d)", irq);
         return;
@@ -175,6 +196,15 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
         warn_report_err(err);
     }
 
+    if (!vfio_ap_register_irq_notifier(vapdev, VFIO_AP_CFG_CHG_IRQ_INDEX, &err))
+    {
+        /*
+         * Report this error, but do not make it a failing condition.
+         * Lack of this IRQ in the host does not prevent normal operation.
+         */
+        warn_report_err(err);
+    }
+
     return;
 
 error:
@@ -187,6 +217,7 @@ static void vfio_ap_unrealize(DeviceState *dev)
     VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev);
 
     vfio_ap_unregister_irq_notifier(vapdev, VFIO_AP_REQ_IRQ_INDEX);
+    vfio_ap_unregister_irq_notifier(vapdev, VFIO_AP_CFG_CHG_IRQ_INDEX);
     vfio_detach_device(&vapdev->vdev);
     g_free(vapdev->vdev.name);
 }
-- 
2.48.1

[RFC PATCH v4 3/5] hw/vfio/ap: store object indicating AP config changed in a queue

[Rorie Reyes]

Creates an object indicating that an AP configuration change event
has been received and stores it in a queue. These objects will later
be used to store event information for an AP configuration change
when the CHSC instruction is intercepted.

Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
---
 hw/vfio/ap.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index 3614657218..3fa986ca45 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -41,6 +41,13 @@ struct VFIOAPDevice {
     EventNotifier cfg_notifier;
 };
 
+typedef struct APConfigChgEvent {
+    QTAILQ_ENTRY(APConfigChgEvent) next;
+} APConfigChgEvent;
+
+QTAILQ_HEAD(, APConfigChgEvent) cfg_chg_events =
+    QTAILQ_HEAD_INITIALIZER(cfg_chg_events);
+
 OBJECT_DECLARE_SIMPLE_TYPE(VFIOAPDevice, VFIO_AP_DEVICE)
 
 static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
@@ -74,12 +81,17 @@ static void vfio_ap_req_notifier_handler(void *opaque)
 
 static void vfio_ap_cfg_chg_notifier_handler(void *opaque)
 {
+    APConfigChgEvent *cfg_chg_event;
     VFIOAPDevice *vapdev = opaque;
 
     if (!event_notifier_test_and_clear(&vapdev->cfg_notifier)) {
         return;
     }
 
+    cfg_chg_event = g_new0(APConfigChgEvent, 1);
+
+    QTAILQ_INSERT_TAIL(&cfg_chg_events, cfg_chg_event, next);
+
     css_generate_css_crws(0);
 
 }
-- 
2.48.1

[RFC PATCH v4 4/5] hw/vfio/ap: Storing event information for an AP configuration change event

[Rorie Reyes]

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                 | 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 3fa986ca45..4da246c538 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);
+    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 0;
+
+}
+
+int ap_chsc_sei_nt0_have_event(void)
+{
+    return !QTAILQ_EMPTY(&cfg_chg_events);
+}
+
 static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
                                           unsigned int irq, Error **errp)
 {
diff --git a/include/hw/s390x/ap-bridge.h b/include/hw/s390x/ap-bridge.h
index 470e439a98..f4d838bf99 100644
--- a/include/hw/s390x/ap-bridge.h
+++ b/include/hw/s390x/ap-bridge.h
@@ -16,4 +16,26 @@
 
 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;
+
+int ap_chsc_sei_nt0_get_event(void *res);
+
+int ap_chsc_sei_nt0_have_event(void);
+
 #endif
-- 
2.48.1

[RFC PATCH v4 5/5] s390: implementing CHSC SEI for AP config change

[Rorie Reyes]

Handle interception of the CHSC SEI instruction for requests
indicating the guest's AP configuration has changed.

Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
Reviewed-by: Anthony Krowiak <akrowiak@linux.ibm.com>
Tested-by: Anthony Krowiak <akrowiak@linux.ibm.com>
---
 target/s390x/ioinst.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
index a944f16c25..f061c6db14 100644
--- a/target/s390x/ioinst.c
+++ b/target/s390x/ioinst.c
@@ -17,6 +17,7 @@
 #include "trace.h"
 #include "hw/s390x/s390-pci-bus.h"
 #include "target/s390x/kvm/pv.h"
+#include "hw/s390x/ap-bridge.h"
 
 /* All I/O instructions but chsc use the s format */
 static uint64_t get_address_from_regs(CPUS390XState *env, uint32_t ipb,
@@ -573,13 +574,19 @@ out:
 
 static int chsc_sei_nt0_get_event(void *res)
 {
-    /* no events yet */
+    if (s390_has_feat(S390_FEAT_AP)) {
+        return ap_chsc_sei_nt0_get_event(res);
+    }
+
     return 1;
 }
 
 static int chsc_sei_nt0_have_event(void)
 {
-    /* no events yet */
+    if (s390_has_feat(S390_FEAT_AP)) {
+        return ap_chsc_sei_nt0_have_event();
+    }
+
     return 0;
 }
 
-- 
2.48.1

Re: [RFC PATCH v4 3/5] hw/vfio/ap: store object indicating AP config changed in a queue

[Anthony Krowiak]

On 3/11/25 11:16 AM, Rorie Reyes wrote:
> Creates an object indicating that an AP configuration change event
> has been received and stores it in a queue. These objects will later
> be used to store event information for an AP configuration change
> when the CHSC instruction is intercepted.
>
> Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>

Reviewed-by: Anthony Krowiak <krowiak@linux.ibm.com>

> ---
>   hw/vfio/ap.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index 3614657218..3fa986ca45 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -41,6 +41,13 @@ struct VFIOAPDevice {
>       EventNotifier cfg_notifier;
>   };
>   
> +typedef struct APConfigChgEvent {
> +    QTAILQ_ENTRY(APConfigChgEvent) next;
> +} APConfigChgEvent;
> +
> +QTAILQ_HEAD(, APConfigChgEvent) cfg_chg_events =
> +    QTAILQ_HEAD_INITIALIZER(cfg_chg_events);
> +
>   OBJECT_DECLARE_SIMPLE_TYPE(VFIOAPDevice, VFIO_AP_DEVICE)
>   
>   static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
> @@ -74,12 +81,17 @@ static void vfio_ap_req_notifier_handler(void *opaque)
>   
>   static void vfio_ap_cfg_chg_notifier_handler(void *opaque)
>   {
> +    APConfigChgEvent *cfg_chg_event;
>       VFIOAPDevice *vapdev = opaque;
>   
>       if (!event_notifier_test_and_clear(&vapdev->cfg_notifier)) {
>           return;
>       }
>   
> +    cfg_chg_event = g_new0(APConfigChgEvent, 1);
> +
> +    QTAILQ_INSERT_TAIL(&cfg_chg_events, cfg_chg_event, next);
> +
>       css_generate_css_crws(0);
>   
>   }

Re: [RFC PATCH v4 4/5] hw/vfio/ap: Storing event information for an AP configuration change event

[Anthony Krowiak]

On 3/11/25 11:16 AM, 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>

Reviewed-by: Anthony Krowiak <akrowiak@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 3fa986ca45..4da246c538 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);
> +    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 0;
> +
> +}
> +
> +int ap_chsc_sei_nt0_have_event(void)
> +{
> +    return !QTAILQ_EMPTY(&cfg_chg_events);
> +}
> +
>   static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
>                                             unsigned int irq, Error **errp)
>   {
> diff --git a/include/hw/s390x/ap-bridge.h b/include/hw/s390x/ap-bridge.h
> index 470e439a98..f4d838bf99 100644
> --- a/include/hw/s390x/ap-bridge.h
> +++ b/include/hw/s390x/ap-bridge.h
> @@ -16,4 +16,26 @@
>   
>   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;
> +
> +int ap_chsc_sei_nt0_get_event(void *res);
> +
> +int ap_chsc_sei_nt0_have_event(void);
> +
>   #endif

Re: [RFC PATCH v4 0/5] Report vfio-ap configuration changes

[Cédric Le Goater]

On 3/11/25 16:16, Rorie Reyes wrote:
> Changelog:
> 
> v4:
> - allocating cfg_chg_event before inserting into the queue
> - calling nt0_have_event in if loop to check if there are any
> elemenets in the queue, then calling QTAILQ_FIRST when the check
> passes
> - moving memset() after the check
> 
> v3:
> - changes that were made to patch 3/5 should have been made in
> patch 2/5
> 
> v2:
> - removed warnings that weren't needed
> - added unregister function
> - removed whitelines
> - changed variable names for consistency
> - removed rc variable and returning 1 or 0 outright
> - reversed logics for if statements
> - using g_free() instead of free()
> - replaced hardcoded numeric values by defining them with #define
> in the header
> 
> --------------------------------------------------------------------------
> This patch series creates and registers a handler that is called when
> userspace is notified by the kernel that a guest's AP configuration has
> changed. The handler in turn notifies the guest that its AP configuration
> has changed. This allows the guest to immediately respond to AP
> configuration changes rather than relying on polling or some other
> inefficient mechanism for detecting config changes.
> 
> Rorie Reyes (5):
>    linux-headers: NOTFORMERGE - placeholder uapi updates for AP config
>      change
>    hw/vfio/ap: notification handler for AP config changed event
>    hw/vfio/ap: store object indicating AP config changed in a queue
>    hw/vfio/ap: Storing event information for an AP configuration change
>      event
>    s390: implementing CHSC SEI for AP config change
> 
>   hw/vfio/ap.c                 | 82 ++++++++++++++++++++++++++++++++++++
>   include/hw/s390x/ap-bridge.h | 22 ++++++++++
>   linux-headers/linux/vfio.h   |  1 +
>   target/s390x/ioinst.c        | 11 ++++-
>   4 files changed, 114 insertions(+), 2 deletions(-)

Queued 2-5. linux-headers should be updated in the QEMU 10.1 cycle.

Thanks,

C.

Re: [RFC PATCH v4 5/5] s390: implementing CHSC SEI for AP config change

[Thomas Huth]

On 11/03/2025 16.16, Rorie Reyes wrote:
> Handle interception of the CHSC SEI instruction for requests
> indicating the guest's AP configuration has changed.
> 
> Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
> Reviewed-by: Anthony Krowiak <akrowiak@linux.ibm.com>
> Tested-by: Anthony Krowiak <akrowiak@linux.ibm.com>
> ---
>   target/s390x/ioinst.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
> index a944f16c25..f061c6db14 100644
> --- a/target/s390x/ioinst.c
> +++ b/target/s390x/ioinst.c
> @@ -17,6 +17,7 @@
>   #include "trace.h"
>   #include "hw/s390x/s390-pci-bus.h"
>   #include "target/s390x/kvm/pv.h"
> +#include "hw/s390x/ap-bridge.h"
>   
>   /* All I/O instructions but chsc use the s format */
>   static uint64_t get_address_from_regs(CPUS390XState *env, uint32_t ipb,
> @@ -573,13 +574,19 @@ out:
>   
>   static int chsc_sei_nt0_get_event(void *res)
>   {
> -    /* no events yet */
> +    if (s390_has_feat(S390_FEAT_AP)) {
> +        return ap_chsc_sei_nt0_get_event(res);
> +    }
> +
>       return 1;
>   }
>   
>   static int chsc_sei_nt0_have_event(void)
>   {
> -    /* no events yet */
> +    if (s390_has_feat(S390_FEAT_AP)) {
> +        return ap_chsc_sei_nt0_have_event();
> +    }
> +
>       return 0;
>   }

  Hi!

This unfortunately fails to link when configuring QEMU with the 
"--without-default-devices" configure switch:

/usr/bin/ld: libqemu-s390x-softmmu.a.p/target_s390x_ioinst.c.o: in function 
`ioinst_handle_chsc':
/tmp/qemu-mini/target/s390x/ioinst.c:587:(.text+0x1ce1): undefined reference 
to `ap_chsc_sei_nt0_have_event'
/usr/bin/ld: /tmp/qemu-mini/target/s390x/ioinst.c:578:(.text+0x1d1c): 
undefined reference to `ap_chsc_sei_nt0_get_event'
collect2: error: ld returned 1 exit status

I guess you have to rather use some callback mechanism, stubs or #ifdefs 
here instead.

  Thomas

Re: [RFC PATCH v4 5/5] s390: implementing CHSC SEI for AP config change

[Rorie Reyes]

On 3/17/25 9:41 AM, Thomas Huth wrote:
> On 11/03/2025 16.16, Rorie Reyes wrote:
>> Handle interception of the CHSC SEI instruction for requests
>> indicating the guest's AP configuration has changed.
>>
>> Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
>> Reviewed-by: Anthony Krowiak <akrowiak@linux.ibm.com>
>> Tested-by: Anthony Krowiak <akrowiak@linux.ibm.com>
>> ---
>>   target/s390x/ioinst.c | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
>> index a944f16c25..f061c6db14 100644
>> --- a/target/s390x/ioinst.c
>> +++ b/target/s390x/ioinst.c
>> @@ -17,6 +17,7 @@
>>   #include "trace.h"
>>   #include "hw/s390x/s390-pci-bus.h"
>>   #include "target/s390x/kvm/pv.h"
>> +#include "hw/s390x/ap-bridge.h"
>>     /* All I/O instructions but chsc use the s format */
>>   static uint64_t get_address_from_regs(CPUS390XState *env, uint32_t 
>> ipb,
>> @@ -573,13 +574,19 @@ out:
>>     static int chsc_sei_nt0_get_event(void *res)
>>   {
>> -    /* no events yet */
>> +    if (s390_has_feat(S390_FEAT_AP)) {
>> +        return ap_chsc_sei_nt0_get_event(res);
>> +    }
>> +
>>       return 1;
>>   }
>>     static int chsc_sei_nt0_have_event(void)
>>   {
>> -    /* no events yet */
>> +    if (s390_has_feat(S390_FEAT_AP)) {
>> +        return ap_chsc_sei_nt0_have_event();
>> +    }
>> +
>>       return 0;
>>   }
>
>  Hi!
>
> This unfortunately fails to link when configuring QEMU with the 
> "--without-default-devices" configure switch:
>
> /usr/bin/ld: libqemu-s390x-softmmu.a.p/target_s390x_ioinst.c.o: in 
> function `ioinst_handle_chsc':
> /tmp/qemu-mini/target/s390x/ioinst.c:587:(.text+0x1ce1): undefined 
> reference to `ap_chsc_sei_nt0_have_event'
> /usr/bin/ld: /tmp/qemu-mini/target/s390x/ioinst.c:578:(.text+0x1d1c): 
> undefined reference to `ap_chsc_sei_nt0_get_event'
> collect2: error: ld returned 1 exit status
>
> I guess you have to rather use some callback mechanism, stubs or 
> #ifdefs here instead.
>
>  Thomas
>
Hey Thomas,

Just want to let you know that I saw your review and I will address this 
issue

Re: [RFC PATCH v4 5/5] s390: implementing CHSC SEI for AP config change

[Rorie Reyes]

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


On 3/17/25 9:41 AM, Thomas Huth wrote:
> On 11/03/2025 16.16, Rorie Reyes wrote:
>> Handle interception of the CHSC SEI instruction for requests
>> indicating the guest's AP configuration has changed.
>>
>> Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
>> Reviewed-by: Anthony Krowiak <akrowiak@linux.ibm.com>
>> Tested-by: Anthony Krowiak <akrowiak@linux.ibm.com>
>> ---
>>   target/s390x/ioinst.c | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
>> index a944f16c25..f061c6db14 100644
>> --- a/target/s390x/ioinst.c
>> +++ b/target/s390x/ioinst.c
>> @@ -17,6 +17,7 @@
>>   #include "trace.h"
>>   #include "hw/s390x/s390-pci-bus.h"
>>   #include "target/s390x/kvm/pv.h"
>> +#include "hw/s390x/ap-bridge.h"
>>     /* All I/O instructions but chsc use the s format */
>>   static uint64_t get_address_from_regs(CPUS390XState *env, uint32_t 
>> ipb,
>> @@ -573,13 +574,19 @@ out:
>>     static int chsc_sei_nt0_get_event(void *res)
>>   {
>> -    /* no events yet */
>> +    if (s390_has_feat(S390_FEAT_AP)) {
>> +        return ap_chsc_sei_nt0_get_event(res);
>> +    }
>> +
>>       return 1;
>>   }
>>     static int chsc_sei_nt0_have_event(void)
>>   {
>> -    /* no events yet */
>> +    if (s390_has_feat(S390_FEAT_AP)) {
>> +        return ap_chsc_sei_nt0_have_event();
>> +    }
>> +
>>       return 0;
>>   }
>
>  Hi!
>
> This unfortunately fails to link when configuring QEMU with the 
> "--without-default-devices" configure switch:
>
> /usr/bin/ld: libqemu-s390x-softmmu.a.p/target_s390x_ioinst.c.o: in 
> function `ioinst_handle_chsc':
> /tmp/qemu-mini/target/s390x/ioinst.c:587:(.text+0x1ce1): undefined 
> reference to `ap_chsc_sei_nt0_have_event'
> /usr/bin/ld: /tmp/qemu-mini/target/s390x/ioinst.c:578:(.text+0x1d1c): 
> undefined reference to `ap_chsc_sei_nt0_get_event'
> collect2: error: ld returned 1 exit status
>
> I guess you have to rather use some callback mechanism, stubs or 
> #ifdefs here instead.
>
>  Thomas
>
Hey Thomas,

Sorry for the delay. I was trying out some ways to resolve this issue 
but I'm not sure what I would use for the macro name if I were to

go the #ifdef route. I had something roughly like this but it wasn't 
working. Would you have any recommendations?

static int chsc_sei_nt0_get_event(void *res) { #ifdef 
HW_S390X_AP_BRIDGE_H if (s390_has_feat(S390_FEAT_AP)) { return 
ap_chsc_sei_nt0_get_event(res); } #endif return 1; } static int 
chsc_sei_nt0_have_event(void) { #ifdef HW_S390X_AP_BRIDGE_H if 
(s390_has_feat(S390_FEAT_AP)) { return ap_chsc_sei_nt0_have_event(); } 
#endif return 0; }

[-- Attachment #2: Type: text/html, Size: 4617 bytes --]

Re: [RFC PATCH v4 5/5] s390: implementing CHSC SEI for AP config change

[Thomas Huth]

On 10/04/2025 22.31, Rorie Reyes wrote:
> 
> On 3/17/25 9:41 AM, Thomas Huth wrote:
>> On 11/03/2025 16.16, Rorie Reyes wrote:
>>> Handle interception of the CHSC SEI instruction for requests
>>> indicating the guest's AP configuration has changed.
>>>
>>> Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
>>> Reviewed-by: Anthony Krowiak <akrowiak@linux.ibm.com>
>>> Tested-by: Anthony Krowiak <akrowiak@linux.ibm.com>
>>> ---
>>>   target/s390x/ioinst.c | 11 +++++++++--
>>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
>>> index a944f16c25..f061c6db14 100644
>>> --- a/target/s390x/ioinst.c
>>> +++ b/target/s390x/ioinst.c
>>> @@ -17,6 +17,7 @@
>>>   #include "trace.h"
>>>   #include "hw/s390x/s390-pci-bus.h"
>>>   #include "target/s390x/kvm/pv.h"
>>> +#include "hw/s390x/ap-bridge.h"
>>>     /* All I/O instructions but chsc use the s format */
>>>   static uint64_t get_address_from_regs(CPUS390XState *env, uint32_t ipb,
>>> @@ -573,13 +574,19 @@ out:
>>>     static int chsc_sei_nt0_get_event(void *res)
>>>   {
>>> -    /* no events yet */
>>> +    if (s390_has_feat(S390_FEAT_AP)) {
>>> +        return ap_chsc_sei_nt0_get_event(res);
>>> +    }
>>> +
>>>       return 1;
>>>   }
>>>     static int chsc_sei_nt0_have_event(void)
>>>   {
>>> -    /* no events yet */
>>> +    if (s390_has_feat(S390_FEAT_AP)) {
>>> +        return ap_chsc_sei_nt0_have_event();
>>> +    }
>>> +
>>>       return 0;
>>>   }
>>
>>  Hi!
>>
>> This unfortunately fails to link when configuring QEMU with the "-- 
>> without-default-devices" configure switch:
>>
>> /usr/bin/ld: libqemu-s390x-softmmu.a.p/target_s390x_ioinst.c.o: in 
>> function `ioinst_handle_chsc':
>> /tmp/qemu-mini/target/s390x/ioinst.c:587:(.text+0x1ce1): undefined 
>> reference to `ap_chsc_sei_nt0_have_event'
>> /usr/bin/ld: /tmp/qemu-mini/target/s390x/ioinst.c:578:(.text+0x1d1c): 
>> undefined reference to `ap_chsc_sei_nt0_get_event'
>> collect2: error: ld returned 1 exit status
>>
>> I guess you have to rather use some callback mechanism, stubs or #ifdefs 
>> here instead.
>>
>>  Thomas
>>
> Hey Thomas,
> 
> Sorry for the delay. I was trying out some ways to resolve this issue but 
> I'm not sure what I would use for the macro name if I were to
> 
> go the #ifdef route. I had something roughly like this but it wasn't 
> working. Would you have any recommendations?
> 
> static int chsc_sei_nt0_get_event(void *res) { #ifdef HW_S390X_AP_BRIDGE_H 
> if (s390_has_feat(S390_FEAT_AP)) { return ap_chsc_sei_nt0_get_event(res); } 
> #endif return 1; } static int chsc_sei_nt0_have_event(void) { #ifdef 
> HW_S390X_AP_BRIDGE_H if (s390_has_feat(S390_FEAT_AP)) { return 
> ap_chsc_sei_nt0_have_event(); } #endif return 0; }

  Hi,

right, that's the wrong #ifdef that you were trying here.
The problematic function is defined in hw/vfio/ap.c, so have a look into 
hw/vfio/meson.build, and you'll see that it's conditionally included via the 
CONFIG_VFIO_AP switch, so that's what you want here, I think. To be able to 
use it, you likely have to add a:

#include CONFIG_DEVICES

at the beginning of the ioinst.c file. Then you should be able to do:

#ifdef CONFIG_VFIO_AP
     if (s390_has_feat(S390_FEAT_AP)) {
        return ap_chsc_sei_nt0_get_event(res);
     }
#endif

(or whatever the code should look like).

Alternatively, and this might even be the nicer variant, add a file 
hw/vfio/ap-stub.c and include a dummy ap_chsc_sei_nt0_get_event() function 
there. Then in hw/vfio/meson.build add this line:

vfio_ss.add(when: 'CONFIG_VFIO_AP', if_false: files('ap-stub.c'))


  HTH,
   Thomas

Re: [RFC PATCH v4 5/5] s390: implementing CHSC SEI for AP config change

[Rorie Reyes]

On 4/11/25 2:45 AM, Thomas Huth wrote:

> #include CONFIG_DEVICES
>
> at the beginning of the ioinst.c file. Then you should be able to do:
>
> #ifdef CONFIG_VFIO_AP
>     if (s390_has_feat(S390_FEAT_AP)) {
>        return ap_chsc_sei_nt0_get_event(res);
>     }
> #endif
This worked
>
> (or whatever the code should look like).
>
> Alternatively, and this might even be the nicer variant, add a file 
> hw/vfio/ap-stub.c and include a dummy ap_chsc_sei_nt0_get_event() 
> function there. Then in hw/vfio/meson.build add this line:
>
> vfio_ss.add(when: 'CONFIG_VFIO_AP', if_false: files('ap-stub.c'))
This worked as well. Since you mentioned that this is a nicer variant, 
I'll go with this change. What do you recommend I do for my patches? 
Should I do an interactive rebase to add the new file hw/vfio/ap-stub.c 
and updating hw/vfio/meson.build? Or should I make two new commits for 
each file (ap-stub.c and meson.build)

Re: [RFC PATCH v4 5/5] s390: implementing CHSC SEI for AP config change

[Thomas Huth]

On 14/04/2025 16.37, Rorie Reyes wrote:
> On 4/11/25 2:45 AM, Thomas Huth wrote:
> 
>> #include CONFIG_DEVICES
>>
>> at the beginning of the ioinst.c file. Then you should be able to do:
>>
>> #ifdef CONFIG_VFIO_AP
>>     if (s390_has_feat(S390_FEAT_AP)) {
>>        return ap_chsc_sei_nt0_get_event(res);
>>     }
>> #endif
> This worked
>>
>> (or whatever the code should look like).
>>
>> Alternatively, and this might even be the nicer variant, add a file hw/ 
>> vfio/ap-stub.c and include a dummy ap_chsc_sei_nt0_get_event() function 
>> there. Then in hw/vfio/meson.build add this line:
>>
>> vfio_ss.add(when: 'CONFIG_VFIO_AP', if_false: files('ap-stub.c'))
> This worked as well. Since you mentioned that this is a nicer variant, I'll 
> go with this change. What do you recommend I do for my patches? Should I do 
> an interactive rebase to add the new file hw/vfio/ap-stub.c and updating hw/ 
> vfio/meson.build? Or should I make two new commits for each file (ap-stub.c 
> and meson.build)

Please do a "git rebase -i ..." to fix up the corresponding patch (that's 
what we're doing in the QEMU development workflow).

  Thanks
   Thomas

Re: [RFC PATCH v4 5/5] s390: implementing CHSC SEI for AP config change

[Rorie Reyes]

On 4/14/25 10:54 AM, Thomas Huth wrote:
> On 14/04/2025 16.37, Rorie Reyes wrote:
>> On 4/11/25 2:45 AM, Thomas Huth wrote:
>>
>>> #include CONFIG_DEVICES
>>>
>>> at the beginning of the ioinst.c file. Then you should be able to do:
>>>
>>> #ifdef CONFIG_VFIO_AP
>>>     if (s390_has_feat(S390_FEAT_AP)) {
>>>        return ap_chsc_sei_nt0_get_event(res);
>>>     }
>>> #endif
>> This worked
>>>
>>> (or whatever the code should look like).
>>>
>>> Alternatively, and this might even be the nicer variant, add a file 
>>> hw/ vfio/ap-stub.c and include a dummy ap_chsc_sei_nt0_get_event() 
>>> function there. Then in hw/vfio/meson.build add this line:
>>>
>>> vfio_ss.add(when: 'CONFIG_VFIO_AP', if_false: files('ap-stub.c'))
>> This worked as well. Since you mentioned that this is a nicer 
>> variant, I'll go with this change. What do you recommend I do for my 
>> patches? Should I do an interactive rebase to add the new file 
>> hw/vfio/ap-stub.c and updating hw/ vfio/meson.build? Or should I make 
>> two new commits for each file (ap-stub.c and meson.build)
>
> Please do a "git rebase -i ..." to fix up the corresponding patch 
> (that's what we're doing in the QEMU development workflow).
>
>  Thanks
>   Thomas
>
Thank you. I'll make those changes and send my patches later today