| Message ID | 20200224114107.4646-21-borntraeger@de.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: Add support for protected VMs | expand |
On Mon, 24 Feb 2020 06:40:51 -0500 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > From: Claudio Imbrenda <imbrenda@linux.ibm.com> > > The current code tries to first pin shared pages, if that fails (e.g. > because the page is not shared) it will export them. For shared pages > this means that we get a new intercept telling us that the guest is > unsharing that page. We will make the page secure at that point in time > and revoke the host access. This is synchronized with other host events, > e.g. the code will wait until host I/O has finished. > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > Acked-by: David Hildenbrand <david@redhat.com> > [borntraeger@de.ibm.com: patch merging, splitting, fixing] > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > arch/s390/kvm/intercept.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c > index b6b7d4b0e26c..c06599939541 100644 > --- a/arch/s390/kvm/intercept.c > +++ b/arch/s390/kvm/intercept.c > @@ -16,6 +16,7 @@ > #include <asm/asm-offsets.h> > #include <asm/irq.h> > #include <asm/sysinfo.h> > +#include <asm/uv.h> > > #include "kvm-s390.h" > #include "gaccess.h" > @@ -484,12 +485,35 @@ static int handle_pv_sclp(struct kvm_vcpu *vcpu) > return 0; > } > > +static int handle_pv_uvc(struct kvm_vcpu *vcpu) > +{ > + struct uv_cb_share *guest_uvcb = (void *)vcpu->arch.sie_block->sidad; > + struct uv_cb_cts uvcb = { > + .header.cmd = UVC_CMD_UNPIN_PAGE_SHARED, > + .header.len = sizeof(uvcb), > + .guest_handle = kvm_s390_pv_get_handle(vcpu->kvm), > + .gaddr = guest_uvcb->paddr, > + }; > + int rc; > + > + if (guest_uvcb->header.cmd != UVC_CMD_REMOVE_SHARED_ACCESS) { > + WARN_ONCE(1, "Unexpected UVC 0x%x!\n", guest_uvcb->header.cmd); "Unexpected notification intercept for UVC 0x%x" ? > + return 0; > + } > + rc = gmap_make_secure(vcpu->arch.gmap, uvcb.gaddr, &uvcb); > + if (rc == -EINVAL) Add a comment why? > + return 0; > + return rc; > +} > + > static int handle_pv_notification(struct kvm_vcpu *vcpu) > { > if (vcpu->arch.sie_block->ipa == 0xb210) > return handle_pv_spx(vcpu); > if (vcpu->arch.sie_block->ipa == 0xb220) > return handle_pv_sclp(vcpu); > + if (vcpu->arch.sie_block->ipa == 0xb9a4) > + return handle_pv_uvc(vcpu); > > return handle_instruction(vcpu); > }
On 25.02.20 13:18, Cornelia Huck wrote: > On Mon, 24 Feb 2020 06:40:51 -0500 > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > >> From: Claudio Imbrenda <imbrenda@linux.ibm.com> >> >> The current code tries to first pin shared pages, if that fails (e.g. >> because the page is not shared) it will export them. For shared pages >> this means that we get a new intercept telling us that the guest is >> unsharing that page. We will make the page secure at that point in time >> and revoke the host access. This is synchronized with other host events, >> e.g. the code will wait until host I/O has finished. >> >> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> >> Acked-by: David Hildenbrand <david@redhat.com> >> [borntraeger@de.ibm.com: patch merging, splitting, fixing] >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> --- >> arch/s390/kvm/intercept.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c >> index b6b7d4b0e26c..c06599939541 100644 >> --- a/arch/s390/kvm/intercept.c >> +++ b/arch/s390/kvm/intercept.c >> @@ -16,6 +16,7 @@ >> #include <asm/asm-offsets.h> >> #include <asm/irq.h> >> #include <asm/sysinfo.h> >> +#include <asm/uv.h> >> >> #include "kvm-s390.h" >> #include "gaccess.h" >> @@ -484,12 +485,35 @@ static int handle_pv_sclp(struct kvm_vcpu *vcpu) >> return 0; >> } >> >> +static int handle_pv_uvc(struct kvm_vcpu *vcpu) >> +{ >> + struct uv_cb_share *guest_uvcb = (void *)vcpu->arch.sie_block->sidad; >> + struct uv_cb_cts uvcb = { >> + .header.cmd = UVC_CMD_UNPIN_PAGE_SHARED, >> + .header.len = sizeof(uvcb), >> + .guest_handle = kvm_s390_pv_get_handle(vcpu->kvm), >> + .gaddr = guest_uvcb->paddr, >> + }; >> + int rc; >> + >> + if (guest_uvcb->header.cmd != UVC_CMD_REMOVE_SHARED_ACCESS) { >> + WARN_ONCE(1, "Unexpected UVC 0x%x!\n", guest_uvcb->header.cmd); > > "Unexpected notification intercept for UVC 0x%x" ok. > > ? > >> + return 0; >> + } >> + rc = gmap_make_secure(vcpu->arch.gmap, uvcb.gaddr, &uvcb); >> + if (rc == -EINVAL) > > Add a comment why? /* * If the unpin did not succeed, the guest will exit again for the UVC * and we will retry the unpin. */ I will also fixup the misleading patch description: The current code tries to first pin shared pages, if that fails (e.g. because the page is not shared) it will export them. For shared pages this means that we get a new intercept telling us that the guest is unsharing that page. We will unpin the page at that point in time, following the same rules as for make secure. (wait for writeback, no elevated page refs etc).
On Tue, 25 Feb 2020 15:21:42 +0100 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 25.02.20 13:18, Cornelia Huck wrote: > > On Mon, 24 Feb 2020 06:40:51 -0500 > > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > > > >> From: Claudio Imbrenda <imbrenda@linux.ibm.com> > >> > >> The current code tries to first pin shared pages, if that fails (e.g. > >> because the page is not shared) it will export them. For shared pages > >> this means that we get a new intercept telling us that the guest is > >> unsharing that page. We will make the page secure at that point in time > >> and revoke the host access. This is synchronized with other host events, > >> e.g. the code will wait until host I/O has finished. > >> > >> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > >> Acked-by: David Hildenbrand <david@redhat.com> > >> [borntraeger@de.ibm.com: patch merging, splitting, fixing] > >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > >> --- > >> arch/s390/kvm/intercept.c | 24 ++++++++++++++++++++++++ > >> 1 file changed, 24 insertions(+) > I will also fixup the misleading patch description: > > The current code tries to first pin shared pages, if that fails (e.g. > because the page is not shared) it will export them. For shared pages > this means that we get a new intercept telling us that the guest is > unsharing that page. We will unpin the page at that point in time, > following the same rules as for make secure. (wait for writeback, no > elevated page refs etc). I'd suggest: "...as for making a page secure (i.e. waiting for writeback, no elevated page references, etc.)" With the touchups, Reviewed-by: Cornelia Huck <cohuck@redhat.com>
diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c index b6b7d4b0e26c..c06599939541 100644 --- a/arch/s390/kvm/intercept.c +++ b/arch/s390/kvm/intercept.c @@ -16,6 +16,7 @@ #include <asm/asm-offsets.h> #include <asm/irq.h> #include <asm/sysinfo.h> +#include <asm/uv.h> #include "kvm-s390.h" #include "gaccess.h" @@ -484,12 +485,35 @@ static int handle_pv_sclp(struct kvm_vcpu *vcpu) return 0; } +static int handle_pv_uvc(struct kvm_vcpu *vcpu) +{ + struct uv_cb_share *guest_uvcb = (void *)vcpu->arch.sie_block->sidad; + struct uv_cb_cts uvcb = { + .header.cmd = UVC_CMD_UNPIN_PAGE_SHARED, + .header.len = sizeof(uvcb), + .guest_handle = kvm_s390_pv_get_handle(vcpu->kvm), + .gaddr = guest_uvcb->paddr, + }; + int rc; + + if (guest_uvcb->header.cmd != UVC_CMD_REMOVE_SHARED_ACCESS) { + WARN_ONCE(1, "Unexpected UVC 0x%x!\n", guest_uvcb->header.cmd); + return 0; + } + rc = gmap_make_secure(vcpu->arch.gmap, uvcb.gaddr, &uvcb); + if (rc == -EINVAL) + return 0; + return rc; +} + static int handle_pv_notification(struct kvm_vcpu *vcpu) { if (vcpu->arch.sie_block->ipa == 0xb210) return handle_pv_spx(vcpu); if (vcpu->arch.sie_block->ipa == 0xb220) return handle_pv_sclp(vcpu); + if (vcpu->arch.sie_block->ipa == 0xb9a4) + return handle_pv_uvc(vcpu); return handle_instruction(vcpu); }