Re: [RFC PATCH v4 5/5] s390: implementing CHSC SEI for AP config change

[Thomas Huth <thuth@redhat.com>]

On 10/04/2025 22.31, Rorie Reyes wrote:
> 
> On 3/17/25 9:41 AM, Thomas Huth wrote:
>> On 11/03/2025 16.16, Rorie Reyes wrote:
>>> Handle interception of the CHSC SEI instruction for requests
>>> indicating the guest's AP configuration has changed.
>>>
>>> Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com>
>>> Reviewed-by: Anthony Krowiak <akrowiak@linux.ibm.com>
>>> Tested-by: Anthony Krowiak <akrowiak@linux.ibm.com>
>>> ---
>>>   target/s390x/ioinst.c | 11 +++++++++--
>>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
>>> index a944f16c25..f061c6db14 100644
>>> --- a/target/s390x/ioinst.c
>>> +++ b/target/s390x/ioinst.c
>>> @@ -17,6 +17,7 @@
>>>   #include "trace.h"
>>>   #include "hw/s390x/s390-pci-bus.h"
>>>   #include "target/s390x/kvm/pv.h"
>>> +#include "hw/s390x/ap-bridge.h"
>>>     /* All I/O instructions but chsc use the s format */
>>>   static uint64_t get_address_from_regs(CPUS390XState *env, uint32_t ipb,
>>> @@ -573,13 +574,19 @@ out:
>>>     static int chsc_sei_nt0_get_event(void *res)
>>>   {
>>> -    /* no events yet */
>>> +    if (s390_has_feat(S390_FEAT_AP)) {
>>> +        return ap_chsc_sei_nt0_get_event(res);
>>> +    }
>>> +
>>>       return 1;
>>>   }
>>>     static int chsc_sei_nt0_have_event(void)
>>>   {
>>> -    /* no events yet */
>>> +    if (s390_has_feat(S390_FEAT_AP)) {
>>> +        return ap_chsc_sei_nt0_have_event();
>>> +    }
>>> +
>>>       return 0;
>>>   }
>>
>>  Hi!
>>
>> This unfortunately fails to link when configuring QEMU with the "-- 
>> without-default-devices" configure switch:
>>
>> /usr/bin/ld: libqemu-s390x-softmmu.a.p/target_s390x_ioinst.c.o: in 
>> function `ioinst_handle_chsc':
>> /tmp/qemu-mini/target/s390x/ioinst.c:587:(.text+0x1ce1): undefined 
>> reference to `ap_chsc_sei_nt0_have_event'
>> /usr/bin/ld: /tmp/qemu-mini/target/s390x/ioinst.c:578:(.text+0x1d1c): 
>> undefined reference to `ap_chsc_sei_nt0_get_event'
>> collect2: error: ld returned 1 exit status
>>
>> I guess you have to rather use some callback mechanism, stubs or #ifdefs 
>> here instead.
>>
>>  Thomas
>>
> Hey Thomas,
> 
> Sorry for the delay. I was trying out some ways to resolve this issue but 
> I'm not sure what I would use for the macro name if I were to
> 
> go the #ifdef route. I had something roughly like this but it wasn't 
> working. Would you have any recommendations?
> 
> static int chsc_sei_nt0_get_event(void *res) { #ifdef HW_S390X_AP_BRIDGE_H 
> if (s390_has_feat(S390_FEAT_AP)) { return ap_chsc_sei_nt0_get_event(res); } 
> #endif return 1; } static int chsc_sei_nt0_have_event(void) { #ifdef 
> HW_S390X_AP_BRIDGE_H if (s390_has_feat(S390_FEAT_AP)) { return 
> ap_chsc_sei_nt0_have_event(); } #endif return 0; }

  Hi,

right, that's the wrong #ifdef that you were trying here.
The problematic function is defined in hw/vfio/ap.c, so have a look into 
hw/vfio/meson.build, and you'll see that it's conditionally included via the 
CONFIG_VFIO_AP switch, so that's what you want here, I think. To be able to 
use it, you likely have to add a:

#include CONFIG_DEVICES

at the beginning of the ioinst.c file. Then you should be able to do:

#ifdef CONFIG_VFIO_AP
     if (s390_has_feat(S390_FEAT_AP)) {
        return ap_chsc_sei_nt0_get_event(res);
     }
#endif

(or whatever the code should look like).

Alternatively, and this might even be the nicer variant, add a file 
hw/vfio/ap-stub.c and include a dummy ap_chsc_sei_nt0_get_event() function 
there. Then in hw/vfio/meson.build add this line:

vfio_ss.add(when: 'CONFIG_VFIO_AP', if_false: files('ap-stub.c'))


  HTH,
   Thomas