Re: [PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
On Tue, Apr 06, 2021 at 03:48:20PM +, Sean Christopherson wrote:
> On Mon, Apr 05, 2021, Ashish Kalra wrote:
> > From: Ashish Kalra
>
> ...
>
> > diff --git a/arch/x86/include/asm/kvm_host.h
> > b/arch/x86/include/asm/kvm_host.h
> > index 3768819693e5..78284ebbbee7 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1352,6 +1352,8 @@ struct kvm_x86_ops {
> > int (*complete_emulated_msr)(struct kvm_vcpu *vcpu, int err);
> >
> > void (*vcpu_deliver_sipi_vector)(struct kvm_vcpu *vcpu, u8 vector);
> > + int (*page_enc_status_hc)(struct kvm_vcpu *vcpu, unsigned long gpa,
> > + unsigned long sz, unsigned long mode);
> > };
> >
> > struct kvm_x86_nested_ops {
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index c9795a22e502..fb3a315e5827 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -1544,6 +1544,67 @@ static int sev_receive_finish(struct kvm *kvm,
> > struct kvm_sev_cmd *argp)
> > return ret;
> > }
> >
> > +static int sev_complete_userspace_page_enc_status_hc(struct kvm_vcpu *vcpu)
> > +{
> > + vcpu->run->exit_reason = 0;
> > + kvm_rax_write(vcpu, vcpu->run->dma_sharing.ret);
> > + ++vcpu->stat.hypercalls;
> > + return kvm_skip_emulated_instruction(vcpu);
> > +}
> > +
> > +int svm_page_enc_status_hc(struct kvm_vcpu *vcpu, unsigned long gpa,
> > + unsigned long npages, unsigned long enc)
> > +{
> > + kvm_pfn_t pfn_start, pfn_end;
> > + struct kvm *kvm = vcpu->kvm;
> > + gfn_t gfn_start, gfn_end;
> > +
> > + if (!sev_guest(kvm))
> > + return -EINVAL;
> > +
> > + if (!npages)
> > + return 0;
>
> Parth of me thinks passing a zero size should be an error not a nop. Either
> way
> works, just feels a bit weird to allow this to be a nop.
>
> > +
> > + gfn_start = gpa_to_gfn(gpa);
>
> This should check that @gpa is aligned.
>
> > + gfn_end = gfn_start + npages;
> > +
> > + /* out of bound access error check */
> > + if (gfn_end <= gfn_start)
> > + return -EINVAL;
> > +
> > + /* lets make sure that gpa exist in our memslot */
> > + pfn_start = gfn_to_pfn(kvm, gfn_start);
> > + pfn_end = gfn_to_pfn(kvm, gfn_end);
> > +
> > + if (is_error_noslot_pfn(pfn_start) && !is_noslot_pfn(pfn_start)) {
> > + /*
> > +* Allow guest MMIO range(s) to be added
> > +* to the shared pages list.
> > +*/
> > + return -EINVAL;
> > + }
> > +
> > + if (is_error_noslot_pfn(pfn_end) && !is_noslot_pfn(pfn_end)) {
> > + /*
> > +* Allow guest MMIO range(s) to be added
> > +* to the shared pages list.
> > +*/
> > + return -EINVAL;
> > + }
>
> I don't think KVM should do any checks beyond gfn_end <= gfn_start. Just punt
> to userspace and give userspace full say over what is/isn't legal.
>
> > +
> > + if (enc)
> > + vcpu->run->exit_reason = KVM_EXIT_DMA_UNSHARE;
> > + else
> > + vcpu->run->exit_reason = KVM_EXIT_DMA_SHARE;
>
> Use a single exit and pass "enc" via kvm_run. I also strongly dislike "DMA",
> there's no guarantee the guest is sharing memory for DMA.
>
> I think we can usurp KVM_EXIT_HYPERCALL for this? E.g.
>
> vcpu->run->exit_reason= KVM_EXIT_HYPERCALL;
> vcpu->run->hypercall.nr = KVM_HC_PAGE_ENC_STATUS;
> vcpu->run->hypercall.args[0] = gfn_start << PAGE_SHIFT;
> vcpu->run->hypercall.args[1] = npages * PAGE_SIZE;
> vcpu->run->hypercall.args[2] = enc;
> vcpu->run->hypercall.longmode = is_64_bit_mode(vcpu);
>
> > +
> > + vcpu->run->dma_sharing.addr = gfn_start;
>
> Addresses and pfns are not the same thing. If you're passing the size in
> bytes,
> then it's probably best to pass the gpa, not the gfn. Same for the params
> from
> the guest, they should be in the same "domain".
>
> > + vcpu->run->dma_sharing.len = npages * PAGE_SIZE;
> > + vcpu->arch.complete_userspace_io =
> > + sev_complete_userspace_page_enc_status_hc;
>
> I vote to drop the "userspace" part, it's already quite verbose.
>
> vcpu->arch.complete_userspace_io = sev_complete_page_enc_status_hc;
>
> > +
> > + return 0;
> > +}
> > +
>
> ..
>
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index f7d12fca397b..ef5c77d59651 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -8273,6 +8273,18 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
> > kvm_sched_yield(vcpu->kvm, a0);
> > ret = 0;
> > break;
> > + case KVM_HC_PAGE_ENC_STATUS: {
> > + int r;
> > +
> > + ret = -KVM_ENOSYS;
> > + if (kvm_x86_ops.page_enc_status_hc) {
> > + r = kvm_x86_ops.page_enc_status_hc(vcpu, a0, a1, a2);
>
> Use static_call().
>
> > + if (r >= 0)
> > +
Re: [PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
On Tue, Apr 06, 2021, Steve Rutherford wrote:
> On Tue, Apr 6, 2021 at 9:08 AM Ashish Kalra wrote:
> > I see the following in Documentation/virt/kvm/api.rst :
> > ..
> > ..
> > /* KVM_EXIT_HYPERCALL */
> > struct {
> > __u64 nr;
> > __u64 args[6];
> > __u64 ret;
> > __u32 longmode;
> > __u32 pad;
> > } hypercall;
> >
> > Unused. This was once used for 'hypercall to userspace'. To implement
> > such functionality, use KVM_EXIT_IO (x86) or KVM_EXIT_MMIO (all except
> > s390).
> >
> > This mentions this exitcode to be unused and implementing this
> > functionality using KVM_EXIT_IO for x86?
>
> I suspect this description is historical. It was originally from 2009.
> KVM_EXIT_IO is used for IO port reads/writes.
The purpose of the comment is to discourage use of hypercalls for things that
can instead be done via port I/O and/or MMIO. The biggest advantage is that KVM
doesn't need to act as an intermediary; userspace can define whatever paravirt
shenanigans it so desires and KVM naturally handles the I/O accesses.
MMIO in particular also allows for finer granularity of permissions in the
guest,
e.g. the guest kernel can expose the address to a userspace application to allow
said application to make "hypercalls". Port I/O technically has similar
properties, but the I/O bitmaps are heinous.
For this particular case, because we want to make a _KVM_ hypercall that just
happens to get punted to userspace, and because there is no known or projected
use case for letting guest userspace control memory encryption it makes sense to
use a "real" hypercall.
> Personally, I would prefer to stay the course and use a name similar
> to KVM_EXIT_DMA_SHARE, say KVM_EXIT_MEM_SHARE and
> KVM_EXIT_MEM_UNSHARE. These just seem very clear, which I appreciate.
> Reusing hypercall would work, but shoehorning this into
> KVM_EXIT_HYPERCALL when we don't have generic hypercall exits feels a
> bit off in my mind. Note: that preference isn't particularly strong.
I'm not against adding a new exit type, I'm against adding _two_ new exit types.
I also don't like using "(UN)SHARE" because there may be future use cases where
the hypercall isn't used to "share' memory, but to inform the host of a change
in state. I don't have a concrete example, but it's not completely absurd to
think that there might be a scenario where a guest has the ability to use
multiple
keys and needs to communicate key usage to the host. Linux already has horrific
(IMO) names for describing encrypted vs. "other" memory, I'd strongly prefer to
avoid adding yet another potentially wrong name to the mix.
Re: [PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
On Tue, Apr 6, 2021 at 9:08 AM Ashish Kalra wrote:
>
> On Tue, Apr 06, 2021 at 03:48:20PM +, Sean Christopherson wrote:
> > On Mon, Apr 05, 2021, Ashish Kalra wrote:
> > > From: Ashish Kalra
> >
> > ...
> >
> > > diff --git a/arch/x86/include/asm/kvm_host.h
> > > b/arch/x86/include/asm/kvm_host.h
> > > index 3768819693e5..78284ebbbee7 100644
> > > --- a/arch/x86/include/asm/kvm_host.h
> > > +++ b/arch/x86/include/asm/kvm_host.h
> > > @@ -1352,6 +1352,8 @@ struct kvm_x86_ops {
> > > int (*complete_emulated_msr)(struct kvm_vcpu *vcpu, int err);
> > >
> > > void (*vcpu_deliver_sipi_vector)(struct kvm_vcpu *vcpu, u8 vector);
> > > + int (*page_enc_status_hc)(struct kvm_vcpu *vcpu, unsigned long gpa,
> > > + unsigned long sz, unsigned long mode);
> > > };
> > >
> > > struct kvm_x86_nested_ops {
> > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > > index c9795a22e502..fb3a315e5827 100644
> > > --- a/arch/x86/kvm/svm/sev.c
> > > +++ b/arch/x86/kvm/svm/sev.c
> > > @@ -1544,6 +1544,67 @@ static int sev_receive_finish(struct kvm *kvm,
> > > struct kvm_sev_cmd *argp)
> > > return ret;
> > > }
> > >
> > > +static int sev_complete_userspace_page_enc_status_hc(struct kvm_vcpu
> > > *vcpu)
> > > +{
> > > + vcpu->run->exit_reason = 0;
> > > + kvm_rax_write(vcpu, vcpu->run->dma_sharing.ret);
> > > + ++vcpu->stat.hypercalls;
> > > + return kvm_skip_emulated_instruction(vcpu);
> > > +}
> > > +
> > > +int svm_page_enc_status_hc(struct kvm_vcpu *vcpu, unsigned long gpa,
> > > + unsigned long npages, unsigned long enc)
> > > +{
> > > + kvm_pfn_t pfn_start, pfn_end;
> > > + struct kvm *kvm = vcpu->kvm;
> > > + gfn_t gfn_start, gfn_end;
> > > +
> > > + if (!sev_guest(kvm))
> > > + return -EINVAL;
> > > +
> > > + if (!npages)
> > > + return 0;
> >
> > Parth of me thinks passing a zero size should be an error not a nop.
> > Either way
> > works, just feels a bit weird to allow this to be a nop.
> >
> > > +
> > > + gfn_start = gpa_to_gfn(gpa);
> >
> > This should check that @gpa is aligned.
> >
> > > + gfn_end = gfn_start + npages;
> > > +
> > > + /* out of bound access error check */
> > > + if (gfn_end <= gfn_start)
> > > + return -EINVAL;
> > > +
> > > + /* lets make sure that gpa exist in our memslot */
> > > + pfn_start = gfn_to_pfn(kvm, gfn_start);
> > > + pfn_end = gfn_to_pfn(kvm, gfn_end);
> > > +
> > > + if (is_error_noslot_pfn(pfn_start) && !is_noslot_pfn(pfn_start)) {
> > > + /*
> > > +* Allow guest MMIO range(s) to be added
> > > +* to the shared pages list.
> > > +*/
> > > + return -EINVAL;
> > > + }
> > > +
> > > + if (is_error_noslot_pfn(pfn_end) && !is_noslot_pfn(pfn_end)) {
> > > + /*
> > > +* Allow guest MMIO range(s) to be added
> > > +* to the shared pages list.
> > > +*/
> > > + return -EINVAL;
> > > + }
> >
> > I don't think KVM should do any checks beyond gfn_end <= gfn_start. Just
> > punt
> > to userspace and give userspace full say over what is/isn't legal.
> >
> > > +
> > > + if (enc)
> > > + vcpu->run->exit_reason = KVM_EXIT_DMA_UNSHARE;
> > > + else
> > > + vcpu->run->exit_reason = KVM_EXIT_DMA_SHARE;
> >
> > Use a single exit and pass "enc" via kvm_run. I also strongly dislike
> > "DMA",
> > there's no guarantee the guest is sharing memory for DMA.
> >
> > I think we can usurp KVM_EXIT_HYPERCALL for this? E.g.
> >
>
> I see the following in Documentation/virt/kvm/api.rst :
> ..
> ..
> /* KVM_EXIT_HYPERCALL */
> struct {
> __u64 nr;
> __u64 args[6];
> __u64 ret;
> __u32 longmode;
> __u32 pad;
> } hypercall;
>
> Unused. This was once used for 'hypercall to userspace'. To implement
> such functionality, use KVM_EXIT_IO (x86) or KVM_EXIT_MMIO (all except s390).
>
> This mentions this exitcode to be unused and implementing this
> functionality using KVM_EXIT_IO for x86?
I suspect this description is historical. It was originally from 2009.
KVM_EXIT_IO is used for IO port reads/writes.
Personally, I would prefer to stay the course and use a name similar
to KVM_EXIT_DMA_SHARE, say KVM_EXIT_MEM_SHARE and
KVM_EXIT_MEM_UNSHARE. These just seem very clear, which I appreciate.
Reusing hypercall would work, but shoehorning this into
KVM_EXIT_HYPERCALL when we don't have generic hypercall exits feels a
bit off in my mind. Note: that preference isn't particularly strong.
Steve
>
> Thanks,
> Ashish
>
> > vcpu->run->exit_reason= KVM_EXIT_HYPERCALL;
> > vcpu->run->hypercall.nr = KVM_HC_PAGE_ENC_STATUS;
> > vcpu->run->hypercall.args[0] = gfn_start << PAGE_SHIFT;
> > vcpu->run->hypercall.args[1] = npages * PAGE_SIZE;
> >
Re: [PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
On Tue, Apr 06, 2021 at 06:22:48AM +, Ashish Kalra wrote:
> On Mon, Apr 05, 2021 at 01:42:42PM -0700, Steve Rutherford wrote:
> > On Mon, Apr 5, 2021 at 7:28 AM Ashish Kalra wrote:
> > >
> > > From: Ashish Kalra
> > >
> > > This hypercall is used by the SEV guest to notify a change in the page
> > > encryption status to the hypervisor. The hypercall should be invoked
> > > only when the encryption attribute is changed from encrypted -> decrypted
> > > and vice versa. By default all guest pages are considered encrypted.
> > >
> > > The hypercall exits to userspace to manage the guest shared regions and
> > > integrate with the userspace VMM's migration code.
> > >
> > > The patch integrates and extends DMA_SHARE/UNSHARE hypercall to
> > > userspace exit functionality (arm64-specific) patch from Marc Zyngier,
> > > to avoid arch-specific stuff and have a common interface
> > > from the guest back to the VMM and sharing of the host handling of the
> > > hypercall to support use case for a guest to share memory with a host.
> > >
> > > Cc: Thomas Gleixner
> > > Cc: Ingo Molnar
> > > Cc: "H. Peter Anvin"
> > > Cc: Paolo Bonzini
> > > Cc: Joerg Roedel
> > > Cc: Borislav Petkov
> > > Cc: Tom Lendacky
> > > Cc: x...@kernel.org
> > > Cc: k...@vger.kernel.org
> > > Cc: linux-kernel@vger.kernel.org
> > > Signed-off-by: Brijesh Singh
> > > Signed-off-by: Ashish Kalra
> > > ---
> > > Documentation/virt/kvm/api.rst| 18
> > > Documentation/virt/kvm/hypercalls.rst | 15 +++
> > > arch/x86/include/asm/kvm_host.h | 2 +
> > > arch/x86/kvm/svm/sev.c| 61 +++
> > > arch/x86/kvm/svm/svm.c| 2 +
> > > arch/x86/kvm/svm/svm.h| 2 +
> > > arch/x86/kvm/vmx/vmx.c| 1 +
> > > arch/x86/kvm/x86.c| 12 ++
> > > include/uapi/linux/kvm.h | 8
> > > include/uapi/linux/kvm_para.h | 1 +
> > > 10 files changed, 122 insertions(+)
> > >
> > > diff --git a/Documentation/virt/kvm/api.rst
> > > b/Documentation/virt/kvm/api.rst
> > > index 307f2fcf1b02..52bd7e475fd6 100644
> > > --- a/Documentation/virt/kvm/api.rst
> > > +++ b/Documentation/virt/kvm/api.rst
> > > @@ -5475,6 +5475,24 @@ Valid values for 'type' are:
> > > Userspace is expected to place the hypercall result into the
> > > appropriate
> > > field before invoking KVM_RUN again.
> > >
> > > +::
> > > +
> > > + /* KVM_EXIT_DMA_SHARE / KVM_EXIT_DMA_UNSHARE */
> > > + struct {
> > > + __u64 addr;
> > > + __u64 len;
> > > + __u64 ret;
> > > + } dma_sharing;
> > > +
> > > +This defines a common interface from the guest back to the KVM to support
> > > +use case for a guest to share memory with a host.
> > > +
> > > +The addr and len fields define the starting address and length of the
> > > +shared memory region.
> > > +
> > > +Userspace is expected to place the hypercall result into the "ret" field
> > > +before invoking KVM_RUN again.
> > > +
> > > ::
> > >
> > > /* Fix the size of the union. */
> > > diff --git a/Documentation/virt/kvm/hypercalls.rst
> > > b/Documentation/virt/kvm/hypercalls.rst
> > > index ed4fddd364ea..7aff0cebab7c 100644
> > > --- a/Documentation/virt/kvm/hypercalls.rst
> > > +++ b/Documentation/virt/kvm/hypercalls.rst
> > > @@ -169,3 +169,18 @@ a0: destination APIC ID
> > >
> > > :Usage example: When sending a call-function IPI-many to vCPUs, yield if
> > > any of the IPI target vCPUs was preempted.
> > > +
> > > +
> > > +8. KVM_HC_PAGE_ENC_STATUS
> > > +-
> > > +:Architecture: x86
> > > +:Status: active
> > > +:Purpose: Notify the encryption status changes in guest page table (SEV
> > > guest)
> > > +
> > > +a0: the guest physical address of the start page
> > > +a1: the number of pages
> > > +a2: encryption attribute
> > > +
> > > + Where:
> > > + * 1: Encryption attribute is set
> > > + * 0: Encryption attribute is cleared
> > > diff --git a/arch/x86/include/asm/kvm_host.h
> > > b/arch/x86/include/asm/kvm_host.h
> > > index 3768819693e5..78284ebbbee7 100644
> > > --- a/arch/x86/include/asm/kvm_host.h
> > > +++ b/arch/x86/include/asm/kvm_host.h
> > > @@ -1352,6 +1352,8 @@ struct kvm_x86_ops {
> > > int (*complete_emulated_msr)(struct kvm_vcpu *vcpu, int err);
> > >
> > > void (*vcpu_deliver_sipi_vector)(struct kvm_vcpu *vcpu, u8
> > > vector);
> > > + int (*page_enc_status_hc)(struct kvm_vcpu *vcpu, unsigned long
> > > gpa,
> > > + unsigned long sz, unsigned long mode);
> > > };
> > >
> > > struct kvm_x86_nested_ops {
> > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > > index c9795a22e502..fb3a315e5827 100644
> > > --- a/arch/x86/kvm/svm/sev.c
> > > +++ b/arch/x86/kvm/svm/sev.c
> > > @@ -1544,6 +1544,67 @@ static
Re: [PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
On Tue, Apr 06, 2021 at 03:48:20PM +, Sean Christopherson wrote:
> On Mon, Apr 05, 2021, Ashish Kalra wrote:
> > From: Ashish Kalra
>
> ...
>
> > diff --git a/arch/x86/include/asm/kvm_host.h
> > b/arch/x86/include/asm/kvm_host.h
> > index 3768819693e5..78284ebbbee7 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1352,6 +1352,8 @@ struct kvm_x86_ops {
> > int (*complete_emulated_msr)(struct kvm_vcpu *vcpu, int err);
> >
> > void (*vcpu_deliver_sipi_vector)(struct kvm_vcpu *vcpu, u8 vector);
> > + int (*page_enc_status_hc)(struct kvm_vcpu *vcpu, unsigned long gpa,
> > + unsigned long sz, unsigned long mode);
> > };
> >
> > struct kvm_x86_nested_ops {
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index c9795a22e502..fb3a315e5827 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -1544,6 +1544,67 @@ static int sev_receive_finish(struct kvm *kvm,
> > struct kvm_sev_cmd *argp)
> > return ret;
> > }
> >
> > +static int sev_complete_userspace_page_enc_status_hc(struct kvm_vcpu *vcpu)
> > +{
> > + vcpu->run->exit_reason = 0;
> > + kvm_rax_write(vcpu, vcpu->run->dma_sharing.ret);
> > + ++vcpu->stat.hypercalls;
> > + return kvm_skip_emulated_instruction(vcpu);
> > +}
> > +
> > +int svm_page_enc_status_hc(struct kvm_vcpu *vcpu, unsigned long gpa,
> > + unsigned long npages, unsigned long enc)
> > +{
> > + kvm_pfn_t pfn_start, pfn_end;
> > + struct kvm *kvm = vcpu->kvm;
> > + gfn_t gfn_start, gfn_end;
> > +
> > + if (!sev_guest(kvm))
> > + return -EINVAL;
> > +
> > + if (!npages)
> > + return 0;
>
> Parth of me thinks passing a zero size should be an error not a nop. Either
> way
> works, just feels a bit weird to allow this to be a nop.
>
> > +
> > + gfn_start = gpa_to_gfn(gpa);
>
> This should check that @gpa is aligned.
>
> > + gfn_end = gfn_start + npages;
> > +
> > + /* out of bound access error check */
> > + if (gfn_end <= gfn_start)
> > + return -EINVAL;
> > +
> > + /* lets make sure that gpa exist in our memslot */
> > + pfn_start = gfn_to_pfn(kvm, gfn_start);
> > + pfn_end = gfn_to_pfn(kvm, gfn_end);
> > +
> > + if (is_error_noslot_pfn(pfn_start) && !is_noslot_pfn(pfn_start)) {
> > + /*
> > +* Allow guest MMIO range(s) to be added
> > +* to the shared pages list.
> > +*/
> > + return -EINVAL;
> > + }
> > +
> > + if (is_error_noslot_pfn(pfn_end) && !is_noslot_pfn(pfn_end)) {
> > + /*
> > +* Allow guest MMIO range(s) to be added
> > +* to the shared pages list.
> > +*/
> > + return -EINVAL;
> > + }
>
> I don't think KVM should do any checks beyond gfn_end <= gfn_start. Just punt
> to userspace and give userspace full say over what is/isn't legal.
>
> > +
> > + if (enc)
> > + vcpu->run->exit_reason = KVM_EXIT_DMA_UNSHARE;
> > + else
> > + vcpu->run->exit_reason = KVM_EXIT_DMA_SHARE;
>
> Use a single exit and pass "enc" via kvm_run. I also strongly dislike "DMA",
> there's no guarantee the guest is sharing memory for DMA.
>
> I think we can usurp KVM_EXIT_HYPERCALL for this? E.g.
>
I see the following in Documentation/virt/kvm/api.rst :
..
..
/* KVM_EXIT_HYPERCALL */
struct {
__u64 nr;
__u64 args[6];
__u64 ret;
__u32 longmode;
__u32 pad;
} hypercall;
Unused. This was once used for 'hypercall to userspace'. To implement
such functionality, use KVM_EXIT_IO (x86) or KVM_EXIT_MMIO (all except s390).
This mentions this exitcode to be unused and implementing this
functionality using KVM_EXIT_IO for x86?
Thanks,
Ashish
> vcpu->run->exit_reason= KVM_EXIT_HYPERCALL;
> vcpu->run->hypercall.nr = KVM_HC_PAGE_ENC_STATUS;
> vcpu->run->hypercall.args[0] = gfn_start << PAGE_SHIFT;
> vcpu->run->hypercall.args[1] = npages * PAGE_SIZE;
> vcpu->run->hypercall.args[2] = enc;
> vcpu->run->hypercall.longmode = is_64_bit_mode(vcpu);
>
> > +
> > + vcpu->run->dma_sharing.addr = gfn_start;
>
> Addresses and pfns are not the same thing. If you're passing the size in
> bytes,
> then it's probably best to pass the gpa, not the gfn. Same for the params
> from
> the guest, they should be in the same "domain".
>
> > + vcpu->run->dma_sharing.len = npages * PAGE_SIZE;
> > + vcpu->arch.complete_userspace_io =
> > + sev_complete_userspace_page_enc_status_hc;
>
> I vote to drop the "userspace" part, it's already quite verbose.
>
> vcpu->arch.complete_userspace_io = sev_complete_page_enc_status_hc;
>
> > +
> > + return 0;
> > +}
> > +
>
> ..
>
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index
Re: [PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
On Mon, Apr 05, 2021, Ashish Kalra wrote:
> From: Ashish Kalra
...
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 3768819693e5..78284ebbbee7 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1352,6 +1352,8 @@ struct kvm_x86_ops {
> int (*complete_emulated_msr)(struct kvm_vcpu *vcpu, int err);
>
> void (*vcpu_deliver_sipi_vector)(struct kvm_vcpu *vcpu, u8 vector);
> + int (*page_enc_status_hc)(struct kvm_vcpu *vcpu, unsigned long gpa,
> + unsigned long sz, unsigned long mode);
> };
>
> struct kvm_x86_nested_ops {
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index c9795a22e502..fb3a315e5827 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1544,6 +1544,67 @@ static int sev_receive_finish(struct kvm *kvm, struct
> kvm_sev_cmd *argp)
> return ret;
> }
>
> +static int sev_complete_userspace_page_enc_status_hc(struct kvm_vcpu *vcpu)
> +{
> + vcpu->run->exit_reason = 0;
> + kvm_rax_write(vcpu, vcpu->run->dma_sharing.ret);
> + ++vcpu->stat.hypercalls;
> + return kvm_skip_emulated_instruction(vcpu);
> +}
> +
> +int svm_page_enc_status_hc(struct kvm_vcpu *vcpu, unsigned long gpa,
> +unsigned long npages, unsigned long enc)
> +{
> + kvm_pfn_t pfn_start, pfn_end;
> + struct kvm *kvm = vcpu->kvm;
> + gfn_t gfn_start, gfn_end;
> +
> + if (!sev_guest(kvm))
> + return -EINVAL;
> +
> + if (!npages)
> + return 0;
Parth of me thinks passing a zero size should be an error not a nop. Either way
works, just feels a bit weird to allow this to be a nop.
> +
> + gfn_start = gpa_to_gfn(gpa);
This should check that @gpa is aligned.
> + gfn_end = gfn_start + npages;
> +
> + /* out of bound access error check */
> + if (gfn_end <= gfn_start)
> + return -EINVAL;
> +
> + /* lets make sure that gpa exist in our memslot */
> + pfn_start = gfn_to_pfn(kvm, gfn_start);
> + pfn_end = gfn_to_pfn(kvm, gfn_end);
> +
> + if (is_error_noslot_pfn(pfn_start) && !is_noslot_pfn(pfn_start)) {
> + /*
> + * Allow guest MMIO range(s) to be added
> + * to the shared pages list.
> + */
> + return -EINVAL;
> + }
> +
> + if (is_error_noslot_pfn(pfn_end) && !is_noslot_pfn(pfn_end)) {
> + /*
> + * Allow guest MMIO range(s) to be added
> + * to the shared pages list.
> + */
> + return -EINVAL;
> + }
I don't think KVM should do any checks beyond gfn_end <= gfn_start. Just punt
to userspace and give userspace full say over what is/isn't legal.
> +
> + if (enc)
> + vcpu->run->exit_reason = KVM_EXIT_DMA_UNSHARE;
> + else
> + vcpu->run->exit_reason = KVM_EXIT_DMA_SHARE;
Use a single exit and pass "enc" via kvm_run. I also strongly dislike "DMA",
there's no guarantee the guest is sharing memory for DMA.
I think we can usurp KVM_EXIT_HYPERCALL for this? E.g.
vcpu->run->exit_reason= KVM_EXIT_HYPERCALL;
vcpu->run->hypercall.nr = KVM_HC_PAGE_ENC_STATUS;
vcpu->run->hypercall.args[0] = gfn_start << PAGE_SHIFT;
vcpu->run->hypercall.args[1] = npages * PAGE_SIZE;
vcpu->run->hypercall.args[2] = enc;
vcpu->run->hypercall.longmode = is_64_bit_mode(vcpu);
> +
> + vcpu->run->dma_sharing.addr = gfn_start;
Addresses and pfns are not the same thing. If you're passing the size in bytes,
then it's probably best to pass the gpa, not the gfn. Same for the params from
the guest, they should be in the same "domain".
> + vcpu->run->dma_sharing.len = npages * PAGE_SIZE;
> + vcpu->arch.complete_userspace_io =
> + sev_complete_userspace_page_enc_status_hc;
I vote to drop the "userspace" part, it's already quite verbose.
vcpu->arch.complete_userspace_io = sev_complete_page_enc_status_hc;
> +
> + return 0;
> +}
> +
..
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index f7d12fca397b..ef5c77d59651 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8273,6 +8273,18 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
> kvm_sched_yield(vcpu->kvm, a0);
> ret = 0;
> break;
> + case KVM_HC_PAGE_ENC_STATUS: {
> + int r;
> +
> + ret = -KVM_ENOSYS;
> + if (kvm_x86_ops.page_enc_status_hc) {
> + r = kvm_x86_ops.page_enc_status_hc(vcpu, a0, a1, a2);
Use static_call().
> + if (r >= 0)
> + return r;
> + ret = r;
> + }
Hmm, an alternative to adding a kvm_x86_ops hook would be to tag the VM as
supporting/allowing the hypercall. That would clean up this code, ensure VMX
and SVM don't end up creating a
Re: [PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
On Tue, Apr 06, 2021, Sean Christopherson wrote:
> On Tue, Apr 06, 2021, Ashish Kalra wrote:
> > On Mon, Apr 05, 2021 at 01:42:42PM -0700, Steve Rutherford wrote:
> > > On Mon, Apr 5, 2021 at 7:28 AM Ashish Kalra wrote:
> > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > > index f7d12fca397b..ef5c77d59651 100644
> > > > --- a/arch/x86/kvm/x86.c
> > > > +++ b/arch/x86/kvm/x86.c
> > > > @@ -8273,6 +8273,18 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
> > > > kvm_sched_yield(vcpu->kvm, a0);
> > > > ret = 0;
> > > > break;
> > > > + case KVM_HC_PAGE_ENC_STATUS: {
> > > > + int r;
> > > > +
> > > > + ret = -KVM_ENOSYS;
> > > > + if (kvm_x86_ops.page_enc_status_hc) {
> > > > + r = kvm_x86_ops.page_enc_status_hc(vcpu, a0,
> > > > a1, a2);
> > > > + if (r >= 0)
> > > > + return r;
> > > > + ret = r;
> > > Style nit: Why not just set ret, and return ret if ret >=0?
> > >
> >
> > But ret is "unsigned long", if i set ret and return, then i will return to
> > guest
> > even in case of error above ?
>
> As proposed, svm_page_enc_status_hc() already hooks complete_userspace_io(),
> so
> this could be hoisted out of the switch statement.
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 16fb39503296..794dde3adfab 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8261,6 +8261,10 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
> goto out;
> }
>
> + /* comment goes here */
> + if (nr == KVM_HC_PAGE_ENC_STATUS && kvm_x86_ops.page_enc_status_hc)
> + return static_call(kvm_x86_page_enc_status_hc(vcpu, a0, a1,
> a2));
Gah, the SEV implementation can also return -EINVAL, and that should fail the
hypercall, not kill the guest. Normally we try to avoid output params, but
in this case it might be less ugly to do:
case KVM_HC_PAGE_ENC_STATUS: {
if (!kvm_x86_ops.page_enc_status_hc)
break;
if (!static_call(kvm_x86_page_enc_status_hc(vcpu, a0,
a1,
a2, ));
return 0;
break;
> +
> ret = -KVM_ENOSYS;
>
> switch (nr) {
>
Re: [PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
On Tue, Apr 06, 2021, Ashish Kalra wrote:
> On Mon, Apr 05, 2021 at 01:42:42PM -0700, Steve Rutherford wrote:
> > On Mon, Apr 5, 2021 at 7:28 AM Ashish Kalra wrote:
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index f7d12fca397b..ef5c77d59651 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -8273,6 +8273,18 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
> > > kvm_sched_yield(vcpu->kvm, a0);
> > > ret = 0;
> > > break;
> > > + case KVM_HC_PAGE_ENC_STATUS: {
> > > + int r;
> > > +
> > > + ret = -KVM_ENOSYS;
> > > + if (kvm_x86_ops.page_enc_status_hc) {
> > > + r = kvm_x86_ops.page_enc_status_hc(vcpu, a0, a1,
> > > a2);
> > > + if (r >= 0)
> > > + return r;
> > > + ret = r;
> > Style nit: Why not just set ret, and return ret if ret >=0?
> >
>
> But ret is "unsigned long", if i set ret and return, then i will return to
> guest
> even in case of error above ?
As proposed, svm_page_enc_status_hc() already hooks complete_userspace_io(), so
this could be hoisted out of the switch statement.
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 16fb39503296..794dde3adfab 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8261,6 +8261,10 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
goto out;
}
+ /* comment goes here */
+ if (nr == KVM_HC_PAGE_ENC_STATUS && kvm_x86_ops.page_enc_status_hc)
+ return static_call(kvm_x86_page_enc_status_hc(vcpu, a0, a1,
a2));
+
ret = -KVM_ENOSYS;
switch (nr) {
Re: [PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
On Mon, Apr 05, 2021 at 01:42:42PM -0700, Steve Rutherford wrote:
> On Mon, Apr 5, 2021 at 7:28 AM Ashish Kalra wrote:
> >
> > From: Ashish Kalra
> >
> > This hypercall is used by the SEV guest to notify a change in the page
> > encryption status to the hypervisor. The hypercall should be invoked
> > only when the encryption attribute is changed from encrypted -> decrypted
> > and vice versa. By default all guest pages are considered encrypted.
> >
> > The hypercall exits to userspace to manage the guest shared regions and
> > integrate with the userspace VMM's migration code.
> >
> > The patch integrates and extends DMA_SHARE/UNSHARE hypercall to
> > userspace exit functionality (arm64-specific) patch from Marc Zyngier,
> > to avoid arch-specific stuff and have a common interface
> > from the guest back to the VMM and sharing of the host handling of the
> > hypercall to support use case for a guest to share memory with a host.
> >
> > Cc: Thomas Gleixner
> > Cc: Ingo Molnar
> > Cc: "H. Peter Anvin"
> > Cc: Paolo Bonzini
> > Cc: Joerg Roedel
> > Cc: Borislav Petkov
> > Cc: Tom Lendacky
> > Cc: x...@kernel.org
> > Cc: k...@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Brijesh Singh
> > Signed-off-by: Ashish Kalra
> > ---
> > Documentation/virt/kvm/api.rst| 18
> > Documentation/virt/kvm/hypercalls.rst | 15 +++
> > arch/x86/include/asm/kvm_host.h | 2 +
> > arch/x86/kvm/svm/sev.c| 61 +++
> > arch/x86/kvm/svm/svm.c| 2 +
> > arch/x86/kvm/svm/svm.h| 2 +
> > arch/x86/kvm/vmx/vmx.c| 1 +
> > arch/x86/kvm/x86.c| 12 ++
> > include/uapi/linux/kvm.h | 8
> > include/uapi/linux/kvm_para.h | 1 +
> > 10 files changed, 122 insertions(+)
> >
> > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> > index 307f2fcf1b02..52bd7e475fd6 100644
> > --- a/Documentation/virt/kvm/api.rst
> > +++ b/Documentation/virt/kvm/api.rst
> > @@ -5475,6 +5475,24 @@ Valid values for 'type' are:
> > Userspace is expected to place the hypercall result into the
> > appropriate
> > field before invoking KVM_RUN again.
> >
> > +::
> > +
> > + /* KVM_EXIT_DMA_SHARE / KVM_EXIT_DMA_UNSHARE */
> > + struct {
> > + __u64 addr;
> > + __u64 len;
> > + __u64 ret;
> > + } dma_sharing;
> > +
> > +This defines a common interface from the guest back to the KVM to support
> > +use case for a guest to share memory with a host.
> > +
> > +The addr and len fields define the starting address and length of the
> > +shared memory region.
> > +
> > +Userspace is expected to place the hypercall result into the "ret" field
> > +before invoking KVM_RUN again.
> > +
> > ::
> >
> > /* Fix the size of the union. */
> > diff --git a/Documentation/virt/kvm/hypercalls.rst
> > b/Documentation/virt/kvm/hypercalls.rst
> > index ed4fddd364ea..7aff0cebab7c 100644
> > --- a/Documentation/virt/kvm/hypercalls.rst
> > +++ b/Documentation/virt/kvm/hypercalls.rst
> > @@ -169,3 +169,18 @@ a0: destination APIC ID
> >
> > :Usage example: When sending a call-function IPI-many to vCPUs, yield if
> > any of the IPI target vCPUs was preempted.
> > +
> > +
> > +8. KVM_HC_PAGE_ENC_STATUS
> > +-
> > +:Architecture: x86
> > +:Status: active
> > +:Purpose: Notify the encryption status changes in guest page table (SEV
> > guest)
> > +
> > +a0: the guest physical address of the start page
> > +a1: the number of pages
> > +a2: encryption attribute
> > +
> > + Where:
> > + * 1: Encryption attribute is set
> > + * 0: Encryption attribute is cleared
> > diff --git a/arch/x86/include/asm/kvm_host.h
> > b/arch/x86/include/asm/kvm_host.h
> > index 3768819693e5..78284ebbbee7 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1352,6 +1352,8 @@ struct kvm_x86_ops {
> > int (*complete_emulated_msr)(struct kvm_vcpu *vcpu, int err);
> >
> > void (*vcpu_deliver_sipi_vector)(struct kvm_vcpu *vcpu, u8 vector);
> > + int (*page_enc_status_hc)(struct kvm_vcpu *vcpu, unsigned long gpa,
> > + unsigned long sz, unsigned long mode);
> > };
> >
> > struct kvm_x86_nested_ops {
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index c9795a22e502..fb3a315e5827 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -1544,6 +1544,67 @@ static int sev_receive_finish(struct kvm *kvm,
> > struct kvm_sev_cmd *argp)
> > return ret;
> > }
> >
> > +static int sev_complete_userspace_page_enc_status_hc(struct kvm_vcpu *vcpu)
> > +{
> > + vcpu->run->exit_reason = 0;
> I don't believe you need to clear exit_reason: it's universally set on exit.
>
>
Re: [PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
On Mon, Apr 5, 2021 at 7:28 AM Ashish Kalra wrote:
>
> From: Ashish Kalra
>
> This hypercall is used by the SEV guest to notify a change in the page
> encryption status to the hypervisor. The hypercall should be invoked
> only when the encryption attribute is changed from encrypted -> decrypted
> and vice versa. By default all guest pages are considered encrypted.
>
> The hypercall exits to userspace to manage the guest shared regions and
> integrate with the userspace VMM's migration code.
>
> The patch integrates and extends DMA_SHARE/UNSHARE hypercall to
> userspace exit functionality (arm64-specific) patch from Marc Zyngier,
> to avoid arch-specific stuff and have a common interface
> from the guest back to the VMM and sharing of the host handling of the
> hypercall to support use case for a guest to share memory with a host.
>
> Cc: Thomas Gleixner
> Cc: Ingo Molnar
> Cc: "H. Peter Anvin"
> Cc: Paolo Bonzini
> Cc: Joerg Roedel
> Cc: Borislav Petkov
> Cc: Tom Lendacky
> Cc: x...@kernel.org
> Cc: k...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Brijesh Singh
> Signed-off-by: Ashish Kalra
> ---
> Documentation/virt/kvm/api.rst| 18
> Documentation/virt/kvm/hypercalls.rst | 15 +++
> arch/x86/include/asm/kvm_host.h | 2 +
> arch/x86/kvm/svm/sev.c| 61 +++
> arch/x86/kvm/svm/svm.c| 2 +
> arch/x86/kvm/svm/svm.h| 2 +
> arch/x86/kvm/vmx/vmx.c| 1 +
> arch/x86/kvm/x86.c| 12 ++
> include/uapi/linux/kvm.h | 8
> include/uapi/linux/kvm_para.h | 1 +
> 10 files changed, 122 insertions(+)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 307f2fcf1b02..52bd7e475fd6 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -5475,6 +5475,24 @@ Valid values for 'type' are:
> Userspace is expected to place the hypercall result into the appropriate
> field before invoking KVM_RUN again.
>
> +::
> +
> + /* KVM_EXIT_DMA_SHARE / KVM_EXIT_DMA_UNSHARE */
> + struct {
> + __u64 addr;
> + __u64 len;
> + __u64 ret;
> + } dma_sharing;
> +
> +This defines a common interface from the guest back to the KVM to support
> +use case for a guest to share memory with a host.
> +
> +The addr and len fields define the starting address and length of the
> +shared memory region.
> +
> +Userspace is expected to place the hypercall result into the "ret" field
> +before invoking KVM_RUN again.
> +
> ::
>
> /* Fix the size of the union. */
> diff --git a/Documentation/virt/kvm/hypercalls.rst
> b/Documentation/virt/kvm/hypercalls.rst
> index ed4fddd364ea..7aff0cebab7c 100644
> --- a/Documentation/virt/kvm/hypercalls.rst
> +++ b/Documentation/virt/kvm/hypercalls.rst
> @@ -169,3 +169,18 @@ a0: destination APIC ID
>
> :Usage example: When sending a call-function IPI-many to vCPUs, yield if
> any of the IPI target vCPUs was preempted.
> +
> +
> +8. KVM_HC_PAGE_ENC_STATUS
> +-
> +:Architecture: x86
> +:Status: active
> +:Purpose: Notify the encryption status changes in guest page table (SEV
> guest)
> +
> +a0: the guest physical address of the start page
> +a1: the number of pages
> +a2: encryption attribute
> +
> + Where:
> + * 1: Encryption attribute is set
> + * 0: Encryption attribute is cleared
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 3768819693e5..78284ebbbee7 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1352,6 +1352,8 @@ struct kvm_x86_ops {
> int (*complete_emulated_msr)(struct kvm_vcpu *vcpu, int err);
>
> void (*vcpu_deliver_sipi_vector)(struct kvm_vcpu *vcpu, u8 vector);
> + int (*page_enc_status_hc)(struct kvm_vcpu *vcpu, unsigned long gpa,
> + unsigned long sz, unsigned long mode);
> };
>
> struct kvm_x86_nested_ops {
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index c9795a22e502..fb3a315e5827 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1544,6 +1544,67 @@ static int sev_receive_finish(struct kvm *kvm, struct
> kvm_sev_cmd *argp)
> return ret;
> }
>
> +static int sev_complete_userspace_page_enc_status_hc(struct kvm_vcpu *vcpu)
> +{
> + vcpu->run->exit_reason = 0;
I don't believe you need to clear exit_reason: it's universally set on exit.
> + kvm_rax_write(vcpu, vcpu->run->dma_sharing.ret);
> + ++vcpu->stat.hypercalls;
> + return kvm_skip_emulated_instruction(vcpu);
> +}
> +
> +int svm_page_enc_status_hc(struct kvm_vcpu *vcpu, unsigned long gpa,
> + unsigned long npages, unsigned long enc)
> +{
> + kvm_pfn_t
[PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
From: Ashish Kalra
This hypercall is used by the SEV guest to notify a change in the page
encryption status to the hypervisor. The hypercall should be invoked
only when the encryption attribute is changed from encrypted -> decrypted
and vice versa. By default all guest pages are considered encrypted.
The hypercall exits to userspace to manage the guest shared regions and
integrate with the userspace VMM's migration code.
The patch integrates and extends DMA_SHARE/UNSHARE hypercall to
userspace exit functionality (arm64-specific) patch from Marc Zyngier,
to avoid arch-specific stuff and have a common interface
from the guest back to the VMM and sharing of the host handling of the
hypercall to support use case for a guest to share memory with a host.
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: "H. Peter Anvin"
Cc: Paolo Bonzini
Cc: Joerg Roedel
Cc: Borislav Petkov
Cc: Tom Lendacky
Cc: x...@kernel.org
Cc: k...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Brijesh Singh
Signed-off-by: Ashish Kalra
---
Documentation/virt/kvm/api.rst| 18
Documentation/virt/kvm/hypercalls.rst | 15 +++
arch/x86/include/asm/kvm_host.h | 2 +
arch/x86/kvm/svm/sev.c| 61 +++
arch/x86/kvm/svm/svm.c| 2 +
arch/x86/kvm/svm/svm.h| 2 +
arch/x86/kvm/vmx/vmx.c| 1 +
arch/x86/kvm/x86.c| 12 ++
include/uapi/linux/kvm.h | 8
include/uapi/linux/kvm_para.h | 1 +
10 files changed, 122 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 307f2fcf1b02..52bd7e475fd6 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -5475,6 +5475,24 @@ Valid values for 'type' are:
Userspace is expected to place the hypercall result into the appropriate
field before invoking KVM_RUN again.
+::
+
+ /* KVM_EXIT_DMA_SHARE / KVM_EXIT_DMA_UNSHARE */
+ struct {
+ __u64 addr;
+ __u64 len;
+ __u64 ret;
+ } dma_sharing;
+
+This defines a common interface from the guest back to the KVM to support
+use case for a guest to share memory with a host.
+
+The addr and len fields define the starting address and length of the
+shared memory region.
+
+Userspace is expected to place the hypercall result into the "ret" field
+before invoking KVM_RUN again.
+
::
/* Fix the size of the union. */
diff --git a/Documentation/virt/kvm/hypercalls.rst
b/Documentation/virt/kvm/hypercalls.rst
index ed4fddd364ea..7aff0cebab7c 100644
--- a/Documentation/virt/kvm/hypercalls.rst
+++ b/Documentation/virt/kvm/hypercalls.rst
@@ -169,3 +169,18 @@ a0: destination APIC ID
:Usage example: When sending a call-function IPI-many to vCPUs, yield if
any of the IPI target vCPUs was preempted.
+
+
+8. KVM_HC_PAGE_ENC_STATUS
+-
+:Architecture: x86
+:Status: active
+:Purpose: Notify the encryption status changes in guest page table (SEV guest)
+
+a0: the guest physical address of the start page
+a1: the number of pages
+a2: encryption attribute
+
+ Where:
+ * 1: Encryption attribute is set
+ * 0: Encryption attribute is cleared
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 3768819693e5..78284ebbbee7 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1352,6 +1352,8 @@ struct kvm_x86_ops {
int (*complete_emulated_msr)(struct kvm_vcpu *vcpu, int err);
void (*vcpu_deliver_sipi_vector)(struct kvm_vcpu *vcpu, u8 vector);
+ int (*page_enc_status_hc)(struct kvm_vcpu *vcpu, unsigned long gpa,
+ unsigned long sz, unsigned long mode);
};
struct kvm_x86_nested_ops {
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index c9795a22e502..fb3a315e5827 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1544,6 +1544,67 @@ static int sev_receive_finish(struct kvm *kvm, struct
kvm_sev_cmd *argp)
return ret;
}
+static int sev_complete_userspace_page_enc_status_hc(struct kvm_vcpu *vcpu)
+{
+ vcpu->run->exit_reason = 0;
+ kvm_rax_write(vcpu, vcpu->run->dma_sharing.ret);
+ ++vcpu->stat.hypercalls;
+ return kvm_skip_emulated_instruction(vcpu);
+}
+
+int svm_page_enc_status_hc(struct kvm_vcpu *vcpu, unsigned long gpa,
+ unsigned long npages, unsigned long enc)
+{
+ kvm_pfn_t pfn_start, pfn_end;
+ struct kvm *kvm = vcpu->kvm;
+ gfn_t gfn_start, gfn_end;
+
+ if (!sev_guest(kvm))
+ return -EINVAL;
+
+ if (!npages)
+ return 0;
+
+ gfn_start = gpa_to_gfn(gpa);
+ gfn_end = gfn_start + npages;
+
+ /* out of bound access error check */
+ if (gfn_end <= gfn_start)
+
