Commit f0a1a061 authored by Claudio Imbrenda's avatar Claudio Imbrenda Committed by Christian Borntraeger
Browse files

KVM: s390: pv: avoid stalls when making pages secure 



Improve make_secure_pte to avoid stalls when the system is heavily
overcommitted. This was especially problematic in kvm_s390_pv_unpack,
because of the loop over all pages that needed unpacking.

Due to the locks being held, it was not possible to simply replace
uv_call with uv_call_sched. A more complex approach was
needed, in which uv_call is replaced with __uv_call, which does not
loop. When the UVC needs to be executed again, -EAGAIN is returned, and
the caller (or its caller) will try again.

When -EAGAIN is returned, the path is the same as when the page is in
writeback (and the writeback check is also performed, which is
harmless).

Fixes: 214d9bbc ("s390/mm: provide memory management functions for protected KVM guests")
Signed-off-by: default avatarClaudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: default avatarJanosch Frank <frankja@linux.ibm.com>
Reviewed-by: default avatarChristian Borntraeger <borntraeger@de.ibm.com>
Link: https://lore.kernel.org/r/20210920132502.36111-5-imbrenda@linux.ibm.com


Signed-off-by: default avatarChristian Borntraeger <borntraeger@de.ibm.com>
parent 1e2aa46d

arch/s390/kernel/uv.c

+23 −6
Original line number Diff line number Diff line
@@ -165,7 +165,7 @@ static int make_secure_pte(pte_t *ptep, unsigned long addr, 
{

	pte_t entry = READ_ONCE(*ptep);

	struct page *page;

	int expected, rc = 0;

	int expected, cc = 0;



	if (!pte_present(entry))

		return -ENXIO;
@@ -181,12 +181,25 @@ static int make_secure_pte(pte_t *ptep, unsigned long addr, 
	if (!page_ref_freeze(page, expected))

		return -EBUSY;

	set_bit(PG_arch_1, &page->flags);

	rc = uv_call(0, (u64)uvcb);

	/*

	 * If the UVC does not succeed or fail immediately, we don't want to

	 * loop for long, or we might get stall notifications.

	 * On the other hand, this is a complex scenario and we are holding a lot of

	 * locks, so we can't easily sleep and reschedule. We try only once,

	 * and if the UVC returned busy or partial completion, we return

	 * -EAGAIN and we let the callers deal with it.

	 */

	cc = __uv_call(0, (u64)uvcb);

	page_ref_unfreeze(page, expected);

	/* Return -ENXIO if the page was not mapped, -EINVAL otherwise */

	if (rc)

		rc = uvcb->rc == 0x10a ? -ENXIO : -EINVAL;

	return rc;

	/*

	 * Return -ENXIO if the page was not mapped, -EINVAL for other errors.

	 * If busy or partially completed, return -EAGAIN.

	 */

	if (cc == UVC_CC_OK)

		return 0;

	else if (cc == UVC_CC_BUSY || cc == UVC_CC_PARTIAL)

		return -EAGAIN;

	return uvcb->rc == 0x10a ? -ENXIO : -EINVAL;

}



/*
@@ -239,6 +252,10 @@ int gmap_make_secure(struct gmap *gmap, unsigned long gaddr, void *uvcb) 
	mmap_read_unlock(gmap->mm);



	if (rc == -EAGAIN) {

		/*

		 * If we are here because the UVC returned busy or partial

		 * completion, this is just a useless check, but it is safe.

		 */

		wait_on_page_writeback(page);

	} else if (rc == -EBUSY) {

		/*

arch/s390/kvm/intercept.c

+5 −0
Original line number Diff line number Diff line
@@ -518,6 +518,11 @@ static int handle_pv_uvc(struct kvm_vcpu *vcpu) 
	 */

	if (rc == -EINVAL)

		return 0;

	/*

	 * If we got -EAGAIN here, we simply return it. It will eventually

	 * get propagated all the way to userspace, which should then try

	 * again.

	 */

	return rc;

}