Re: [PATCH v7 14/21] KVM: s390: vsie: Add helper to pin and unpin multiple guest pages

[Janosch Frank <frankja@linux.ibm.com>]

On 9/18/26 5:09 PM, Christoph Schlameuss wrote:
> To handle vSIE configurations with an ssca we need to pin and unpin
> multiple consecutive guest-2 pages in guest-1. As these might not be
> consecutive in guest-1 it is necessary to iterate over all pages and
> store guest and host addresses for later use.
> 
> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
> ---
>   arch/s390/kvm/s390/vsie.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 45 insertions(+)
> 
> diff --git a/arch/s390/kvm/s390/vsie.c b/arch/s390/kvm/s390/vsie.c
> index 1b88470efd4a..fa43b6c46bbb 100644
> --- a/arch/s390/kvm/s390/vsie.c
> +++ b/arch/s390/kvm/s390/vsie.c
> @@ -71,6 +71,11 @@ static_assert(sizeof(struct vsie_page) == PAGE_SIZE);
>   static_assert(offsetof(struct vsie_page, mcck_info) == offsetof(struct sie_page, mcck_info));
>   static_assert(IS_ALIGNED(offsetof(struct vsie_page, crycb), 8));
>   
> +struct kvm_address_pair {
> +	gpa_t gpa;
> +	hpa_t hpa;
> +};
> +
>   static inline bool sie_uses_esca(struct kvm_s390_sie_block *scb)
>   {
>   	return (scb->ecb2 & ECB2_ESCA);
> @@ -251,6 +256,46 @@ static void unpin_guest_page(struct kvm *kvm, gpa_t gpa, hpa_t hpa)
>   	mark_page_dirty(kvm, gpa_to_gfn(gpa));
>   }
>   
> +/* unpin multiple guest pages pinned with pin_guest_pages() */
> +static void unpin_guest_pages(struct kvm *kvm, struct kvm_address_pair *addr, unsigned int nr_pages)
> +{
> +	int i;
> +
> +	for (i = 0; i < nr_pages; i++) {
> +		unpin_guest_page(kvm, addr[i].gpa, addr[i].hpa);
> +		addr[i].gpa = 0;
> +		addr[i].hpa = 0;
> +	}
> +}
> +
> +/*
> + * pin nr_pages consecutive guest pages
> + */

Why is this multi line?

> +static int pin_guest_pages(struct kvm *kvm, gpa_t gpa, unsigned int nr_pages,
> +			   struct kvm_address_pair *addr, unsigned int *nr_pages_pinned)
> +{

s/addr/addrs/ ?
Same for unpin?

> +	int rc = 0, i;
> +	hpa_t hpa;
> +
> +	gpa = gpa & PAGE_MASK;
> +
> +	/* the guest pages may not be mapped continuously, so pin each page */
> +	for (i = 0; i < nr_pages; i++) {
> +		rc = pin_guest_page(kvm, gpa + PAGE_SIZE * i, &hpa);
> +		if (rc)
> +			goto err;

There's no reason for the goto, is there?
If not, please inline the err label.


Apart from these nits the patch looks fine.

> +		addr[i].gpa = gpa + PAGE_SIZE * i;
> +		addr[i].hpa = hpa;
> +	}
> +	*nr_pages_pinned = i;
> +	return rc;
> +
> +err:
> +	unpin_guest_pages(kvm, addr, i);
> +	*nr_pages_pinned = 0;
> +	return rc;
> +}
> +
>   /* copy the updated intervention request bits into the shadow scb */
>   static void update_intervention_requests(struct vsie_page *vsie_page)
>   {
>