Re: [PATCH RFC v2 10/11] KVM: s390: Add VSIE shadow configuration

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

On Thu Nov 20, 2025 at 12:02 PM CET, Janosch Frank wrote:
> On 11/10/25 18:16, Christoph Schlameuss wrote:
>> Introduce two new module parameters allowing to keep more shadow
>> structures
>> 
>> * vsie_shadow_scb_max
>>    Override the maximum number of VSIE control blocks / vsie_pages to
>>    shadow in guest-1. KVM will use the maximum of the current number of
>>    vCPUs and a maximum of 256 or this value if it is lower.
>>    This is the number of guest-3 control blocks / CPUs to keep shadowed
>>    to minimize the repeated shadowing effort.
>
> KVM will either use this value or the number of current VCPUs. Either 
> way the number will be capped to 256.
>

>> 
>> * vsie_shadow_sca_max
>>    Override the maximum number of VSIE system control areas to
>>    shadow in guest-1. KVM will use a minimum of the current number of
>>    vCPUs and a maximum of 256 or this value if it is lower.
>>    This is the number of guest-3 system control areas / VMs to keep
>>    shadowed to minimize repeated shadowing effort.
>> 
>> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
> Except for the current implementation with arrays, nothing is limiting 
> us from going over 256 in the future by changing the code. I'm not sure 
> if I ever want to see such an environment in practice though.
>

Even if we would implement that I would not expect to see any improvement from
that without SIGPI. It would be interesting if a crazy over committed system
would even benefit from that or not. That would mainly bring down the not
running SCB shadow and SCA shadow re-init effort.

>>   arch/s390/kvm/vsie.c | 18 +++++++++++++++---
>>   1 file changed, 15 insertions(+), 3 deletions(-)
>> 
>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>> index b69ef763b55296875522f2e63169446b5e2d5053..cd114df5e119bd289d14037d1f1c5bfe148cf5c7 100644
>> --- a/arch/s390/kvm/vsie.c
>> +++ b/arch/s390/kvm/vsie.c
>> @@ -98,9 +98,19 @@ struct vsie_sca {
>>   	struct vsie_page	*pages[KVM_S390_MAX_VSIE_VCPUS];
>>   };
>>   
>> +/* maximum vsie shadow scb */
>> +unsigned int vsie_shadow_scb_max;
>
> Don't we need to initialize the variables or mark them static so they are 0?
>

Yes, initializing to 1.

Interestingly I at least not notice this acting up weirdly.

>> +module_param(vsie_shadow_scb_max, uint, 0644);
>> +MODULE_PARM_DESC(vsie_shadow_scb_max, "Maximum number of VSIE shadow control blocks to keep. Values smaller number vcpus uses number of vcpus; maximum 256");
>> +
>> +/* maximum vsie shadow sca */
>> +unsigned int vsie_shadow_sca_max;
>> +module_param(vsie_shadow_sca_max, uint, 0644);
>> +MODULE_PARM_DESC(vsie_shadow_sca_max, "Maximum number of VSIE shadow system control areas to keep. Values smaller number of vcpus uses number of vcpus; 0 to disable sca shadowing; maximum 256");
>> +
>>   static inline bool use_vsie_sigpif(struct kvm *kvm)
>>   {
>> -	return kvm->arch.use_vsie_sigpif;
>> +	return kvm->arch.use_vsie_sigpif && vsie_shadow_sca_max;
>
> This functions as the enablement of vsie_sigpif?
> Is there a reason why we don't want this enabled per default?
>

Yes, setting both values to default to 1 seems most logical to me.

I am setting this in my testing so I did actually not notice I set the default
to disable here. Thanks.

>>   }
>>   
>>   static inline bool use_vsie_sigpif_for(struct kvm *kvm, struct vsie_page *vsie_page)
>> @@ -907,7 +917,8 @@ static struct vsie_sca *get_vsie_sca(struct kvm_vcpu *vcpu, struct vsie_page *vs
>>   	 * We want at least #online_vcpus shadows, so every VCPU can execute the
>>   	 * VSIE in parallel. (Worst case all single core VMs.)
>>   	 */
>> -	max_sca = MIN(atomic_read(&kvm->online_vcpus), KVM_S390_MAX_VSIE_VCPUS);
>> +	max_sca = MIN(MAX(atomic_read(&kvm->online_vcpus), vsie_shadow_sca_max),
>> +		      KVM_S390_MAX_VSIE_VCPUS);
>>   	if (kvm->arch.vsie.sca_count < max_sca) {
>>   		BUILD_BUG_ON(sizeof(struct vsie_sca) > PAGE_SIZE);
>>   		sca_new = (void *)__get_free_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);
>> @@ -1782,7 +1793,8 @@ static struct vsie_page *get_vsie_page(struct kvm_vcpu *vcpu, unsigned long addr
>>   		put_vsie_page(vsie_page);
>>   	}
>>   
>> -	max_vsie_page = MIN(atomic_read(&kvm->online_vcpus), KVM_S390_MAX_VSIE_VCPUS);
>> +	max_vsie_page = MIN(MAX(atomic_read(&kvm->online_vcpus), vsie_shadow_scb_max),
>> +			    KVM_S390_MAX_VSIE_VCPUS);
>>   
>>   	/* allocate new vsie_page - we will likely need it */
>>   	if (addr || kvm->arch.vsie.page_count < max_vsie_page) {
>>