Re: [PATCH RFC v2 01/11] KVM: s390: Add SCAO read and write helpers

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Tue, 11 Nov 2025 15:37:44 +0100
"Christoph Schlameuss" <schlameuss@linux.ibm.com> wrote:

> On Tue Nov 11, 2025 at 2:45 PM CET, Claudio Imbrenda wrote:
> > On Mon, 10 Nov 2025 18:16:41 +0100
> > Christoph Schlameuss <schlameuss@linux.ibm.com> wrote:
> >  
> >> Introduce some small helper functions to get and set the system control
> >> area origin address from the SIE control block.
> >> 
> >> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
> >> ---
> >>  arch/s390/kvm/vsie.c | 29 +++++++++++++++++++++--------
> >>  1 file changed, 21 insertions(+), 8 deletions(-)
> >> 
> >> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> >> index 347268f89f2f186bea623a3adff7376cabc305b2..ced2ca4ce5b584403d900ed11cb064919feda8e9 100644
> >> --- a/arch/s390/kvm/vsie.c
> >> +++ b/arch/s390/kvm/vsie.c
> >> @@ -123,6 +123,23 @@ static int prefix_is_mapped(struct vsie_page *vsie_page)
> >>  	return !(atomic_read(&vsie_page->scb_s.prog20) & PROG_REQUEST);
> >>  }
> >>  
> >> +static gpa_t read_scao(struct kvm *kvm, struct kvm_s390_sie_block *scb)
> >> +{
> >> +	gpa_t sca;  
> >
> > is it, though?
> >  
> >> +
> >> +	sca = READ_ONCE(scb->scaol) & ~0xfUL;
> >> +	if (test_kvm_cpu_feat(kvm, KVM_S390_VM_CPU_FEAT_64BSCAO))
> >> +		sca |= (u64)READ_ONCE(scb->scaoh) << 32;  
> >
> > this feels more like an hpa_t, which is what you also use in the
> > function below
> >  
> 
> It actually can be either. Without vsie sigp this is a gpa for reading and
> writing. With vsie sigp this is a gpa when reading and a hpa when writing

this is a little confusing, maybe add an explanation

> it. It might be best to not imply anything here but just use "unsigned long"
> for these functions.

yes

> 
> >> +
> >> +	return sca;
> >> +}
> >> +
> >> +static void write_scao(struct kvm_s390_sie_block *scb, hpa_t hpa)
> >> +{
> >> +	scb->scaoh = (u32)((u64)hpa >> 32);
> >> +	scb->scaol = (u32)(u64)hpa;
> >> +}
> >> +
> >>  /* copy the updated intervention request bits into the shadow scb */
> >>  static void update_intervention_requests(struct vsie_page *vsie_page)
> >>  {
> >> @@ -714,12 +731,11 @@ static void unpin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
> >>  	struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
> >>  	hpa_t hpa;
> >>  
> >> -	hpa = (u64) scb_s->scaoh << 32 | scb_s->scaol;
> >> +	hpa = read_scao(vcpu->kvm, scb_s);
> >>  	if (hpa) {
> >>  		unpin_guest_page(vcpu->kvm, vsie_page->sca_gpa, hpa);
> >>  		vsie_page->sca_gpa = 0;
> >> -		scb_s->scaol = 0;
> >> -		scb_s->scaoh = 0;
> >> +		write_scao(scb_s, 0);
> >>  	}
> >>  
> >>  	hpa = scb_s->itdba;
> >> @@ -773,9 +789,7 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
> >>  	gpa_t gpa;
> >>  	int rc = 0;
> >>  
> >> -	gpa = READ_ONCE(scb_o->scaol) & ~0xfUL;
> >> -	if (test_kvm_cpu_feat(vcpu->kvm, KVM_S390_VM_CPU_FEAT_64BSCAO))
> >> -		gpa |= (u64) READ_ONCE(scb_o->scaoh) << 32;
> >> +	gpa = read_scao(vcpu->kvm, scb_o);
> >>  	if (gpa) {
> >>  		if (gpa < 2 * PAGE_SIZE)
> >>  			rc = set_validity_icpt(scb_s, 0x0038U);
> >> @@ -792,8 +806,7 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
> >>  		if (rc)
> >>  			goto unpin;
> >>  		vsie_page->sca_gpa = gpa;
> >> -		scb_s->scaoh = (u32)((u64)hpa >> 32);
> >> -		scb_s->scaol = (u32)(u64)hpa;
> >> +		write_scao(scb_s, hpa);
> >>  	}
> >>  
> >>  	gpa = READ_ONCE(scb_o->itdba) & ~0xffUL;
> >>   
>