Re: [PATCH v5 17/21] KVM: s390: vsie: Allow guest-3 cpu add and remove with ssca

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Shadow SCA does not clear entries for CPUs that are removed from the configuration, leading to incorrect SIGP delivery.
--

commit c4d994f0fee41044da7d25bf24f02291a68cd80a
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>

KVM: s390: vsie: Allow guest-3 cpu add and remove with ssca

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 /
processors in the configuration changes.

> diff --git a/arch/s390/kvm/s390/vsie.c b/arch/s390/kvm/s390/vsie.c
> index 4bd11eaa7d81d..6e42b00206bd9 100644
> --- a/arch/s390/kvm/s390/vsie.c
> +++ b/arch/s390/kvm/s390/vsie.c

[ ... ]

> @@ -2126,17 +2163,17 @@ static int _shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
>  	bool is_esca = sie_uses_esca(vsie_page->scb_o);
>  	unsigned int cpu_nr, cpu_slots;
>  	hpa_t sca_o_entry_hpa;
> -	unsigned long *mcn;
>  	int rc;
>  
>  	if (is_esca)
> -		mcn = phys_to_virt(sca_o_hpa(vsie_sca)) + offsetof(struct esca_block, mcn);
> +		set_bit(VSIE_SCA_ESCA, &vsie_sca->flags);
>  	else
> -		mcn = phys_to_virt(sca_o_hpa(vsie_sca)) + offsetof(struct bsca_block, mcn);
> +		clear_bit(VSIE_SCA_ESCA, &vsie_sca->flags);
> +	sca_mcn_copy(vsie_sca, phys_to_virt(sca_o_hpa(vsie_sca)));
>  
>  	/* pin and make shadow for ALL scb in the sca */
>  	cpu_slots = is_esca ? KVM_S390_MAX_VSIE_VCPUS : KVM_S390_BSCA_CPU_SLOTS;
> -	for_each_set_bit_inv(cpu_nr, mcn, cpu_slots) {
> +	for_each_set_bit_inv(cpu_nr, (unsigned long *)&vsie_sca->mcn, cpu_slots) {

[Severity: High]
Does this code skip clearing entries for CPUs that are removed from the
configuration?

When guest-2 removes a CPU, its bit is cleared in the original SCA's mcn.
Then KVM detects the mcn change and calls _shadow_sca().

Since this loop copies the new mcn and iterates only over the set bits using
for_each_set_bit_inv(), won't the ssca.cpu entries for the removed CPUs be
skipped and never zeroed out?

Because the hardware SSCA structure lacks an mcn mask and relies on ssda == 0
to identify non-existent CPUs, could the hardware continue to use the stale,
non-zero shadow pointers and process SIGPs for removed CPUs instead of
returning CC 3 (Not Operational)?

>  		rc = get_sca_entry_addr(vcpu->kvm, vsie_sca, cpu_nr, NULL, &sca_o_entry_hpa);
>  		if (rc)
>  			break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-vsie-sigpi-v5-0-cf471edb46b2@linux.ibm.com?part=17