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);
+
+ 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)
+{
+ 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;
+ }
+
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
--
2.48.1On 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()
> + 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 ?
> +{
> + 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.
> 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
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?
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
>
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.
>
> 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
>>
>
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
>>>
>>
>
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-a8219af12d7b@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.
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 <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
>>>>
>>>
>>
>
© 2016 - 2025 Red Hat, Inc.