Re: [PATCH v7 12/21] KVM: s390: vsie: Add helper unshadow_intercept()

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

On Wed Sep 23, 2026 at 4:04 PM CEST, Janosch Frank wrote:
> On 9/18/26 5:09 PM, Christoph Schlameuss wrote:
>> Extract helper method from existing code to reuse in new code.
>> 
>> unshadow_intercept() acts like unshadow_scb() but only copies intercept
>> information. Used where the rest of the scb is not yet shadowed.
>> 
>> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
>> ---
>>   arch/s390/kvm/s390/vsie.c | 28 ++++++++++++++++++++--------
>>   1 file changed, 20 insertions(+), 8 deletions(-)
>> 
>> diff --git a/arch/s390/kvm/s390/vsie.c b/arch/s390/kvm/s390/vsie.c
>> index 95baac9a1ccb..c42e2df4c0ab 100644
>> --- a/arch/s390/kvm/s390/vsie.c
>> +++ b/arch/s390/kvm/s390/vsie.c
>> @@ -565,18 +565,27 @@ static void prepare_ibc(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>   	}
>>   }
>>   
>> -/* unshadow the scb, copying parameters back to the real scb */
>> -static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>> +/*
>> + * Only write the interception fields back; avoid clobbering the timer and
>> + * guest-state fields in scb_o.
>> + */
>> +static inline void unshadow_intercept(struct kvm_s390_sie_block *scb_o,
>> +				      struct kvm_s390_sie_block *scb_s)
>>   {
>> -	struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
>> -	struct kvm_s390_sie_block *scb_o = vsie_page->scb_o;
>> -
>> -	/* interception */
>>   	scb_o->icptcode = scb_s->icptcode;
>>   	scb_o->icptstatus = scb_s->icptstatus;
>>   	scb_o->ipa = scb_s->ipa;
>>   	scb_o->ipb = scb_s->ipb;
>>   	scb_o->gbea = scb_s->gbea;
> I have the feeling that we make things difficult for ourselves by not 
> writing SIE entry validities into the scb_o directly. And I think you 
> somewhat recognized that problem since you chose to write the validity 
> to scb_o and not scb_s in _shadow_sca_cpu() but I have yet to fully read 
> that patch.
>
> For 99% of the validities we've never even entered SIE so we copy data 
> twice for absolutely no reason other than symmetry. out_unshadow in 
> kvm_s390_handle_vsie() isn't really necessary, if pin_blocks() fails we 
> can jump to unpin_scb() if we write the validity to scb_o.
>
> Maybe we could move (un)shadow_scb() into vsie_run() and be done with it.
>
> I have some more reading to do.

The problem I am addressing here is that we can encounter validity problems on
the primary/entering or secondary cpus that we do not actively run yet.
Since this would typically happen when we are shadowing the sca (directed to by
the first scb of that configuration) we need to write the validity somewhere
before potentially even having properly shadowed any scb.

I don't think I am a huge fan of the idea of pulling (un)shadow_scb() into
vsie_run(), as that would mean to also pull in (un)pin_blocks() or risk lots of
potential TOUTOC problems. We have to shadow before checking to avoid them.

Getting rid of the symmetry and writing to scb_o and failing from pin_blocks()
to out_put would be fine IMO. The counter argument here clearly is that this is
complication for the hopefully rather cold error path.

Cheers,
Christoph