On Tue, 03 Jun 2025 18:35:42 +0200
Christoph Schlameuss <schlameuss@xxxxxxxxxxxxx> wrote:
[...]
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 60c360c18690f6b94e8483dab2c25f016451204b..95a876ff7aca9c632c3e361275da6781ec070c07 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -51,21 +51,11 @@ static int sca_ext_call_pending(struct kvm_vcpu *vcpu, int *src_id)
>
> BUG_ON(!kvm_s390_use_sca_entries());
> read_lock(&vcpu->kvm->arch.sca_lock);
> - if (vcpu->kvm->arch.use_esca) {
> - struct esca_block *sca = vcpu->kvm->arch.sca;
> - union esca_sigp_ctrl sigp_ctrl =
> - sca->cpu[vcpu->vcpu_id].sigp_ctrl;
> + struct esca_block *sca = vcpu->kvm->arch.sca;
> + union esca_sigp_ctrl sigp_ctrl = sca->cpu[vcpu->vcpu_id].sigp_ctrl;
those variables can now be declared at the top of the function, no need
to declare them in the middle.
>
> - c = sigp_ctrl.c;
> - scn = sigp_ctrl.scn;
> - } else {
> - struct bsca_block *sca = vcpu->kvm->arch.sca;
> - union bsca_sigp_ctrl sigp_ctrl =
> - sca->cpu[vcpu->vcpu_id].sigp_ctrl;
> -
> - c = sigp_ctrl.c;
> - scn = sigp_ctrl.scn;
> - }
> + c = sigp_ctrl.c;
> + scn = sigp_ctrl.scn;
> read_unlock(&vcpu->kvm->arch.sca_lock);
>
> if (src_id)
> @@ -80,33 +70,17 @@ static int sca_inject_ext_call(struct kvm_vcpu *vcpu, int src_id)
>
> BUG_ON(!kvm_s390_use_sca_entries());
> read_lock(&vcpu->kvm->arch.sca_lock);
> - if (vcpu->kvm->arch.use_esca) {
> - struct esca_block *sca = vcpu->kvm->arch.sca;
> - union esca_sigp_ctrl *sigp_ctrl =
> - &(sca->cpu[vcpu->vcpu_id].sigp_ctrl);
> - union esca_sigp_ctrl new_val = {0}, old_val;
> -
> - old_val = READ_ONCE(*sigp_ctrl);
> - new_val.scn = src_id;
> - new_val.c = 1;
> - old_val.c = 0;
> -
> - expect = old_val.value;
> - rc = cmpxchg(&sigp_ctrl->value, old_val.value, new_val.value);
> - } else {
> - struct bsca_block *sca = vcpu->kvm->arch.sca;
> - union bsca_sigp_ctrl *sigp_ctrl =
> - &(sca->cpu[vcpu->vcpu_id].sigp_ctrl);
> - union bsca_sigp_ctrl new_val = {0}, old_val;
> + struct esca_block *sca = vcpu->kvm->arch.sca;
> + union esca_sigp_ctrl *sigp_ctrl = &sca->cpu[vcpu->vcpu_id].sigp_ctrl;
> + union esca_sigp_ctrl new_val = {0}, old_val;
..same here..
also, since you're touching this anyway, can you rewrite the
declaration so that the initialisation is at the end?
union esca_sigp_ctrl old_val, new_val = {0};
>
> - old_val = READ_ONCE(*sigp_ctrl);
> - new_val.scn = src_id;
> - new_val.c = 1;
> - old_val.c = 0;
> + old_val = READ_ONCE(*sigp_ctrl);
> + new_val.scn = src_id;
> + new_val.c = 1;
> + old_val.c = 0;
>
> - expect = old_val.value;
> - rc = cmpxchg(&sigp_ctrl->value, old_val.value, new_val.value);
> - }
> + expect = old_val.value;
> + rc = cmpxchg(&sigp_ctrl->value, old_val.value, new_val.value);
> read_unlock(&vcpu->kvm->arch.sca_lock);
>
> if (rc != expect) {
> @@ -123,19 +97,10 @@ static void sca_clear_ext_call(struct kvm_vcpu *vcpu)
> return;
> kvm_s390_clear_cpuflags(vcpu, CPUSTAT_ECALL_PEND);
> read_lock(&vcpu->kvm->arch.sca_lock);
> - if (vcpu->kvm->arch.use_esca) {
> - struct esca_block *sca = vcpu->kvm->arch.sca;
> - union esca_sigp_ctrl *sigp_ctrl =
> - &(sca->cpu[vcpu->vcpu_id].sigp_ctrl);
> + struct esca_block *sca = vcpu->kvm->arch.sca;
> + union esca_sigp_ctrl *sigp_ctrl = &sca->cpu[vcpu->vcpu_id].sigp_ctrl;
..and here..
>
> - WRITE_ONCE(sigp_ctrl->value, 0);
> - } else {
> - struct bsca_block *sca = vcpu->kvm->arch.sca;
> - union bsca_sigp_ctrl *sigp_ctrl =
> - &(sca->cpu[vcpu->vcpu_id].sigp_ctrl);
> -
> - WRITE_ONCE(sigp_ctrl->value, 0);
> - }
> + WRITE_ONCE(sigp_ctrl->value, 0);
> read_unlock(&vcpu->kvm->arch.sca_lock);
> }
>
[...]
> @@ -3573,105 +3548,23 @@ static void sca_add_vcpu(struct kvm_vcpu *vcpu)
> return;
> }
> read_lock(&vcpu->kvm->arch.sca_lock);
> - if (vcpu->kvm->arch.use_esca) {
> - struct esca_block *sca = vcpu->kvm->arch.sca;
> - phys_addr_t sca_phys = virt_to_phys(sca);
> -
> - sca->cpu[vcpu->vcpu_id].sda = virt_to_phys(vcpu->arch.sie_block);
> - vcpu->arch.sie_block->scaoh = sca_phys >> 32;
> - vcpu->arch.sie_block->scaol = sca_phys & ESCA_SCAOL_MASK;
> - vcpu->arch.sie_block->ecb2 |= ECB2_ESCA;
> - set_bit_inv(vcpu->vcpu_id, (unsigned long *) sca->mcn);
> - } else {
> - struct bsca_block *sca = vcpu->kvm->arch.sca;
> - phys_addr_t sca_phys = virt_to_phys(sca);
> -
> - sca->cpu[vcpu->vcpu_id].sda = virt_to_phys(vcpu->arch.sie_block);
> - vcpu->arch.sie_block->scaoh = sca_phys >> 32;
> - vcpu->arch.sie_block->scaol = sca_phys;
> - set_bit_inv(vcpu->vcpu_id, (unsigned long *) &sca->mcn);
> - }
> + struct esca_block *sca = vcpu->kvm->arch.sca;
> + phys_addr_t sca_phys = virt_to_phys(sca);
..and here
> +
> + sca->cpu[vcpu->vcpu_id].sda = virt_to_phys(vcpu->arch.sie_block);
> + vcpu->arch.sie_block->scaoh = sca_phys >> 32;
> + vcpu->arch.sie_block->scaol = sca_phys & ESCA_SCAOL_MASK;
> + vcpu->arch.sie_block->ecb2 |= ECB2_ESCA;
> + set_bit_inv(vcpu->vcpu_id, (unsigned long *)sca->mcn);
> read_unlock(&vcpu->kvm->arch.sca_lock);
> }
>
[...]
