@@ -96,6 +99,49 @@ static void vfio_ap_cfg_chg_notifier_handler(void
*opaque)
}
+bool 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 true;
+ }
+
+ 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 false;
+}
The boolean return values in the function above do not make logical
sense.
What they are effectively saying is that event information has been
stored
in the response when there is no event information to store (i.e., the
event
queue is empty), and that event information has not been stored if the
response has been filled with event information.
After looking at the calling code in target/s390x/ionst.c, apparently
the meaning of
the int value originally returned from the above function was not in
fact intended to
be a boolean value. It looks like the caller uses this value to
indicate whether the
response code should be set to 0x0001 (this function returns 0) or
0x0005 (this
function returns 1); so, the boolean values returned match what is
expected by
the caller. I think this is why your original pass at returning a
boolean caused your
patch to fail; because you did what made logical sense in this function.
I think the calling code is very convoluted to say the least, so maybe
what makes sense
here is to continue to return an int and use #define to document the
return value; for
example:
#define EVENT_INFORMATION_STORED 0
#define EVENT_INFORMATION_NOT_STORED 1
It would probably make a great deal of sense to refactor the calling
code, but that
could potentially open up a large can of worms, so at least this makes
sense from
the perspective of reading this code.
I agree with you on that, Tony. It makes sense to return an int due to
the calling code. I will make these updates and set up the appropriate
definitions in the header file, 'ap-bridge.h'. I'll also reflect the
same type changes in the stub file to match
+
+bool ap_chsc_sei_nt0_have_event(void)
+{
+ return !QTAILQ_EMPTY(&cfg_chg_events);
+}
It's probably fine to return boolean from the above function because it
makes sense even from the caller's perspective.
I'll keep this as is as, but I'll update the stub file to match
