Re: [PATCH RFC v2 08/11] KVM: s390: Allow guest-3 cpu add and remove with vsie sigpif

["Christoph Schlameuss" <schlameuss@linux.ibm.com>]

On Tue Nov 11, 2025 at 4:47 PM CET, Janosch Frank wrote:
> On 11/10/25 18:16, Christoph Schlameuss wrote:
>> As we are shadowing the SCA we need to add and remove the pointers to
>> the shadowed control blocks and sca entries whenever the mcn changes.
>> 
>> It is not expected that the mcn changes frequently for a already running
>> guest-3 configuration. So we can simply re-init the ssca whenever the
>> mcn changes.
>> To detect the mcn change we store the expected mcn in the struct
>> vsie_sca when running the ssca init.
>> 
>> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
>> ---
>>   arch/s390/kvm/vsie.c | 20 ++++++++++++++++++--
>>   1 file changed, 18 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>> index 72c794945be916cc107aba74e1609d3b4780d4b9..1e15220e1f1ecfd83b10aa0620ca84ff0ff3c1ac 100644
>> --- a/arch/s390/kvm/vsie.c
>> +++ b/arch/s390/kvm/vsie.c
>> @@ -1926,12 +1926,27 @@ static int init_ssca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, struct
>>   	return PTR_ERR(vsie_page_n);
>>   }
>>   
>> +static bool sca_mcn_equals(struct vsie_sca *sca, u64 *mcn)
>> +{
>> +	bool is_esca = test_bit(VSIE_SCA_ESCA, &sca->flags);
>> +	int i;
>> +
>> +	if (!is_esca)
>> +		return ((struct bsca_block *)phys_to_virt(sca->ssca->osca))->mcn == *mcn;
>> +
>> +	for (i = 0; i < 4; i++)
>> +		if (((struct esca_block *)phys_to_virt(sca->ssca->osca))->mcn[i] != sca->mcn[i])
>> +			return false;
>
> You're reimplementing memcmp(), no?
> Instead of casting which makes the comparison really messy you could use 
> offsetof.
>
> Something like this (+- errors):
>
> void *osca = phys_to_virt(sca->ssca->osca);
> int offset = offsetof(struct bsca_block, mcn);
> int size = 8;
>
> if (test_bit(VSIE_SCA_ESCA, &sca->flags)) {
> 	size = 8 * 4;
> 	offset = offsetof(struct esca_block, mcn);
> }
>
> return !memcmp(osca + offset, mcn, size);
>
>

Yes, that is exactly what it is. Will use your proposal instead.

>> +	return true;
>> +}
>> +
>>   /*
>>    * Shadow the sca on vsie enter.
>>    */
>>   static int shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, struct vsie_sca *sca)
>>   {
>>   	struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
>> +	bool do_init_ssca;
>>   	int rc;
>>   
>>   	vsie_page->sca = sca;
>> @@ -1947,8 +1962,9 @@ static int shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, struct
>>   	if (!use_vsie_sigpif_for(vcpu->kvm, vsie_page))
>>   		return false;
>>   
>> -	/* skip if the guest does not have an usable sca */
>> -	if (!sca->ssca->osca) {
>> +	do_init_ssca = !sca->ssca->osca;
>> +	do_init_ssca = do_init_ssca || !sca_mcn_equals(sca, sca->mcn);
>> +	if (do_init_ssca) {
>>   		rc = init_ssca(vcpu, vsie_page, sca);
>>   		if (rc)
>>   			return rc;
>>