On 5/22/25 3:02 PM, Anthony Krowiak wrote:
On 5/22/25 9:35 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);
+ 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;
+
extra white line ^
and returning a bool would make more sense.
It may make more sense from a readability standpoint,
but if you look at the caller (chsc_sei_nt0_get_event in
target/s390x/ioinst.c),
it returns an int, so it can simply return whatever is returned from this
function. On the other hand, it really doesn't make a difference, so if
you feel strongly about this, then it can be changed to a bool.
I think I'd rather keep the returns as is because I've went through
several changes that uses a bool return and my code doesn't work as
designed. The ap config doesn't update in real time which allows the
polling time to take over when notifying the guest. Would it suffice to
document the following in ap-bridge.h (the comment bellow)
+}
+
+int ap_chsc_sei_nt0_have_event(void)
+{
+ return !QTAILQ_EMPTY(&cfg_chg_events);
same here for the bool.
same as above
+}
+
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);
Documentation would be nice to have since a "return 1" means failure.
True.
Since I want to keep the return as is, will this documentation suffice?
/**
* 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:
* 0 - An event was available and written to @res
* 1 - No event was available
*/
+
+int ap_chsc_sei_nt0_have_event(void);
+> #endif
Thanks,
C.
