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

[Rorie Reyes <rreyes@linux.ibm.com>]

On 5/22/25 9:30 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 <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 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);
>
> btw, I don't know if this was discussed. Are we OK to manipulate the
> 'cfg_chg_events' construct withou locking ?
>
I don't think it was discussed. Since I made it static, should we think 
about locking the construct? If so, would using 'static QemuMutex 
cfg_chg_events_lock;' to declare it and use 'qemu_mutex_lock()' and 
'qemu_mutex_unlock()' to lock and unlock when needed? Tony, do you have 
any thoughts on this process?

>> 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;
>
>
>
> please drop the ending ';'
>
>
> Thanks,
>
> C.
>
Will drop in the next version