On 1/7/25 19:43, 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 | 37 ++++++++++++++++++++++++++++++++++++
> include/hw/s390x/ap-bridge.h | 17 +++++++++++++++++
> 2 files changed, 54 insertions(+)
>
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index 508c6eed7a..9d1e18f100 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -94,6 +94,43 @@ static void vfio_ap_cfg_chg_notifier_handler(void *opaque)
> css_generate_css_crws(0);
> }
>
> +int ap_chsc_sei_nt0_get_event(void *res)
> +{
> + APConfigChgEvent *cfg_chg_event = QTAILQ_FIRST(&cfg_chg_events);
> + ChscSeiNt0Res *nt0_res = (ChscSeiNt0Res *)res;
> + memset(nt0_res, 0, sizeof(*nt0_res));
please put the memset after the variables.
> + int rc = 1;
'rc' is not very useful. Why not simply return 0 or 1 ?
> +
> + if (cfg_chg_event) {
I would reverse the logic and return early.
if (! cfg_chg_event) {
return 1;
}
> + QTAILQ_REMOVE(&cfg_chg_events, cfg_chg_event, next);
> + free(cfg_chg_event);
g_free()
> +
> + /*
> + * 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 (!QTAILQ_EMPTY(&cfg_chg_events)) {
> + nt0_res->flags |= PENDING_EVENT_INFO_BITMASK;
> + }
> +
> + nt0_res->length = sizeof(ChscSeiNt0Res);
> + nt0_res->code = 1;
> + nt0_res->nt = 0;
> + nt0_res->rs = 5;
> + nt0_res->cc = 3;
The above values are cryptic. Can we have some define possibly ?
> + rc = 0;
> + }
> +
> + return rc;
> +}
> +
> +int ap_chsc_sei_nt0_have_event(void)
> +{
> + return !QTAILQ_EMPTY(&cfg_chg_events);
> +}
Why not use this routine in ap_chsc_sei_nt0_get_event() too ?
Thanks,
C.
> 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..c9beec3db4 100644
> --- a/include/hw/s390x/ap-bridge.h
> +++ b/include/hw/s390x/ap-bridge.h
> @@ -16,4 +16,21 @@
>
> 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;
> +
> +int ap_chsc_sei_nt0_get_event(void *res);
> +
> +int ap_chsc_sei_nt0_have_event(void);
> +
> #endif