| Message ID | 20190424160942.13567-9-brijesh.singh@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add AMD SEV guest live migration support | expand |
On 4/24/19 11:10 AM, Singh, Brijesh wrote: > The hypercall can be used by the SEV guest to notify the page encryption This hyercall is used by the SEV guest to notify a change in the page... > 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 the guest pages should be considered encrypted. By default all guest page are considered > > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: "Radim Krčmář" <rkrcmar@redhat.com> > Cc: Joerg Roedel <joro@8bytes.org> > Cc: Borislav Petkov <bp@suse.de> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Cc: x86@kernel.org > Cc: kvm@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > Documentation/virtual/kvm/hypercalls.txt | 14 +++++ > arch/x86/include/asm/kvm_host.h | 2 + > arch/x86/kvm/svm.c | 69 ++++++++++++++++++++++++ > arch/x86/kvm/vmx/vmx.c | 1 + > arch/x86/kvm/x86.c | 5 ++ > include/uapi/linux/kvm_para.h | 1 + > 6 files changed, 92 insertions(+) > > diff --git a/Documentation/virtual/kvm/hypercalls.txt b/Documentation/virtual/kvm/hypercalls.txt > index da24c138c8d1..ecd44e488679 100644 > --- a/Documentation/virtual/kvm/hypercalls.txt > +++ b/Documentation/virtual/kvm/hypercalls.txt > @@ -141,3 +141,17 @@ a0 corresponds to the APIC ID in the third argument (a2), bit 1 > corresponds to the APIC ID a2+1, and so on. > > Returns the number of CPUs to which the IPIs were delivered successfully. > + > +7. 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: set or clear the encryption attribute 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 a9d03af34030..adb0ca035b97 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1196,6 +1196,8 @@ struct kvm_x86_ops { > uint16_t (*nested_get_evmcs_version)(struct kvm_vcpu *vcpu); > > bool (*need_emulation_on_page_fault)(struct kvm_vcpu *vcpu); > + int (*page_enc_status_hc)(struct kvm *kvm, unsigned long gpa, > + unsigned long sz, unsigned long mode); > }; > > struct kvm_arch_async_pf { > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 74b57ab742ad..f024f208b052 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -138,6 +138,8 @@ struct kvm_sev_info { > int fd; /* SEV device fd */ > unsigned long pages_locked; /* Number of pages locked */ > struct list_head regions_list; /* List of registered regions */ > + unsigned long *page_enc_bmap; > + unsigned long page_enc_bmap_size; > }; > > struct kvm_svm { > @@ -1911,6 +1913,8 @@ static void sev_vm_destroy(struct kvm *kvm) > > sev_unbind_asid(kvm, sev->handle); > sev_asid_free(kvm); > + > + kvfree(sev->page_enc_bmap); > } > > static void avic_vm_destroy(struct kvm *kvm) > @@ -7370,6 +7374,69 @@ static int sev_receive_finish(struct kvm *kvm, struct kvm_sev_cmd *argp) > return ret; > } > > +static int sev_resize_page_enc_bitmap(struct kvm *kvm, unsigned long new_size) > +{ > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; > + unsigned long *map; > + unsigned long sz; > + > + if (sev->page_enc_bmap_size >= new_size) > + return 0; > + > + sz = ALIGN(new_size, BITS_PER_LONG) / 8; > + > + if (sz > PAGE_SIZE) > + map = vmalloc(sz); > + else > + map = kmalloc(sz, GFP_KERNEL); Any reason this can't always be vmalloc()? > + > + if (!map) { > + pr_err_once("Failed to allocate decrypted bitmap size %lx\n", sz); > + return 1; Should this be -ENOMEM? > + } > + > + /* mark the page encrypted (by default) */ > + memset(map, 0xff, sz); > + > + bitmap_copy(map, sev->page_enc_bmap, sev->page_enc_bmap_size); > + kvfree(sev->page_enc_bmap); > + > + sev->page_enc_bmap = map; > + sev->page_enc_bmap_size = new_size; > + > + return 0; > +} > + > +static int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa, > + unsigned long npages, unsigned long enc) > +{ > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; > + gfn_t gfn_start, gfn_end; > + int r; int ret; ? "r" at first confused me. > + > + if (!npages) > + return 0; > + > + gfn_start = gpa_to_gfn(gpa); > + gfn_end = gfn_start + npages; > + > + mutex_lock(&kvm->lock); > + > + r = 1; > + if (sev_resize_page_enc_bitmap(kvm, gfn_end)) ret = sev_resize_... if (ret) > + goto unlock; > + > + if (enc) > + __bitmap_set(sev->page_enc_bmap, gfn_start, gfn_end - gfn_start); > + else > + __bitmap_clear(sev->page_enc_bmap, gfn_start, gfn_end - gfn_start); > + > + r = 0; If you do the above, this is not needed. Thanks, Tom > +unlock: > + mutex_unlock(&kvm->lock); > + return r; > +} > + > static int svm_mem_enc_op(struct kvm *kvm, void __user *argp) > { > struct kvm_sev_cmd sev_cmd; > @@ -7711,6 +7778,8 @@ static struct kvm_x86_ops svm_x86_ops __ro_after_init = { > .nested_get_evmcs_version = nested_get_evmcs_version, > > .need_emulation_on_page_fault = svm_need_emulation_on_page_fault, > + > + .page_enc_status_hc = svm_page_enc_status_hc > }; > > static int __init svm_init(void) > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index b4e7d645275a..9c814e560e0f 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -7731,6 +7731,7 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { > .get_vmcs12_pages = NULL, > .nested_enable_evmcs = NULL, > .need_emulation_on_page_fault = vmx_need_emulation_on_page_fault, > + .page_enc_status_hc = NULL, > }; > > static void vmx_cleanup_l1d_flush(void) > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index a0d1fc80ac5a..dea644be5992 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -7141,6 +7141,11 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) > case KVM_HC_SEND_IPI: > ret = kvm_pv_send_ipi(vcpu->kvm, a0, a1, a2, a3, op_64_bit); > break; > + case KVM_HC_PAGE_ENC_STATUS: > + ret = -KVM_ENOSYS; > + if (kvm_x86_ops->page_enc_status_hc) > + ret = kvm_x86_ops->page_enc_status_hc(vcpu->kvm, a0, a1, a2); > + break; > default: > ret = -KVM_ENOSYS; > break; > diff --git a/include/uapi/linux/kvm_para.h b/include/uapi/linux/kvm_para.h > index 6c0ce49931e5..3dc9e579f4f9 100644 > --- a/include/uapi/linux/kvm_para.h > +++ b/include/uapi/linux/kvm_para.h > @@ -28,6 +28,7 @@ > #define KVM_HC_MIPS_CONSOLE_OUTPUT 8 > #define KVM_HC_CLOCK_PAIRING 9 > #define KVM_HC_SEND_IPI 10 > +#define KVM_HC_PAGE_ENC_STATUS 11 > > /* > * hypercalls use architecture specific >
On 4/26/19 4:39 PM, Lendacky, Thomas wrote: > On 4/24/19 11:10 AM, Singh, Brijesh wrote: >> The hypercall can be used by the SEV guest to notify the page encryption > > This hyercall is used by the SEV guest to notify a change in the page... > >> 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 the guest pages should be considered encrypted. > > By default all guest page are considered > >> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: "H. Peter Anvin" <hpa@zytor.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: "Radim Krčmář" <rkrcmar@redhat.com> >> Cc: Joerg Roedel <joro@8bytes.org> >> Cc: Borislav Petkov <bp@suse.de> >> Cc: Tom Lendacky <thomas.lendacky@amd.com> >> Cc: x86@kernel.org >> Cc: kvm@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >> --- >> Documentation/virtual/kvm/hypercalls.txt | 14 +++++ >> arch/x86/include/asm/kvm_host.h | 2 + >> arch/x86/kvm/svm.c | 69 ++++++++++++++++++++++++ >> arch/x86/kvm/vmx/vmx.c | 1 + >> arch/x86/kvm/x86.c | 5 ++ >> include/uapi/linux/kvm_para.h | 1 + >> 6 files changed, 92 insertions(+) >> >> diff --git a/Documentation/virtual/kvm/hypercalls.txt b/Documentation/virtual/kvm/hypercalls.txt >> index da24c138c8d1..ecd44e488679 100644 >> --- a/Documentation/virtual/kvm/hypercalls.txt >> +++ b/Documentation/virtual/kvm/hypercalls.txt >> @@ -141,3 +141,17 @@ a0 corresponds to the APIC ID in the third argument (a2), bit 1 >> corresponds to the APIC ID a2+1, and so on. >> >> Returns the number of CPUs to which the IPIs were delivered successfully. >> + >> +7. 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: set or clear the encryption attribute > > 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 a9d03af34030..adb0ca035b97 100644 >> --- a/arch/x86/include/asm/kvm_host.h >> +++ b/arch/x86/include/asm/kvm_host.h >> @@ -1196,6 +1196,8 @@ struct kvm_x86_ops { >> uint16_t (*nested_get_evmcs_version)(struct kvm_vcpu *vcpu); >> >> bool (*need_emulation_on_page_fault)(struct kvm_vcpu *vcpu); >> + int (*page_enc_status_hc)(struct kvm *kvm, unsigned long gpa, >> + unsigned long sz, unsigned long mode); >> }; >> >> struct kvm_arch_async_pf { >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index 74b57ab742ad..f024f208b052 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -138,6 +138,8 @@ struct kvm_sev_info { >> int fd; /* SEV device fd */ >> unsigned long pages_locked; /* Number of pages locked */ >> struct list_head regions_list; /* List of registered regions */ >> + unsigned long *page_enc_bmap; >> + unsigned long page_enc_bmap_size; >> }; >> >> struct kvm_svm { >> @@ -1911,6 +1913,8 @@ static void sev_vm_destroy(struct kvm *kvm) >> >> sev_unbind_asid(kvm, sev->handle); >> sev_asid_free(kvm); >> + >> + kvfree(sev->page_enc_bmap); >> } >> >> static void avic_vm_destroy(struct kvm *kvm) >> @@ -7370,6 +7374,69 @@ static int sev_receive_finish(struct kvm *kvm, struct kvm_sev_cmd *argp) >> return ret; >> } >> >> +static int sev_resize_page_enc_bitmap(struct kvm *kvm, unsigned long new_size) >> +{ >> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; >> + unsigned long *map; >> + unsigned long sz; >> + >> + if (sev->page_enc_bmap_size >= new_size) >> + return 0; >> + >> + sz = ALIGN(new_size, BITS_PER_LONG) / 8; >> + >> + if (sz > PAGE_SIZE) >> + map = vmalloc(sz); >> + else >> + map = kmalloc(sz, GFP_KERNEL); > > Any reason this can't always be vmalloc()? > Yes, we can use vmalloc() unconditionally. The bitmap size will be mostly greater than PAGE_SIZE hence the above is useless anyway. >> + >> + if (!map) { >> + pr_err_once("Failed to allocate decrypted bitmap size %lx\n", sz); >> + return 1; > > Should this be -ENOMEM? > >> + } >> + >> + /* mark the page encrypted (by default) */ >> + memset(map, 0xff, sz); >> + >> + bitmap_copy(map, sev->page_enc_bmap, sev->page_enc_bmap_size); >> + kvfree(sev->page_enc_bmap); >> + >> + sev->page_enc_bmap = map; >> + sev->page_enc_bmap_size = new_size; >> + >> + return 0; >> +} >> + >> +static int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa, >> + unsigned long npages, unsigned long enc) >> +{ >> + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; >> + gfn_t gfn_start, gfn_end; >> + int r; > > int ret; ? "r" at first confused me. I will fix it in next rev. > >> + >> + if (!npages) >> + return 0; >> + >> + gfn_start = gpa_to_gfn(gpa); >> + gfn_end = gfn_start + npages; >> + >> + mutex_lock(&kvm->lock); >> + >> + r = 1; >> + if (sev_resize_page_enc_bitmap(kvm, gfn_end)) > > ret = sev_resize_... > if (ret) > >> + goto unlock; >> + >> + if (enc) >> + __bitmap_set(sev->page_enc_bmap, gfn_start, gfn_end - gfn_start); >> + else >> + __bitmap_clear(sev->page_enc_bmap, gfn_start, gfn_end - gfn_start); >> + >> + r = 0; > > If you do the above, this is not needed. > Yes agreed. thanks
diff --git a/Documentation/virtual/kvm/hypercalls.txt b/Documentation/virtual/kvm/hypercalls.txt index da24c138c8d1..ecd44e488679 100644 --- a/Documentation/virtual/kvm/hypercalls.txt +++ b/Documentation/virtual/kvm/hypercalls.txt @@ -141,3 +141,17 @@ a0 corresponds to the APIC ID in the third argument (a2), bit 1 corresponds to the APIC ID a2+1, and so on. Returns the number of CPUs to which the IPIs were delivered successfully. + +7. 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: set or clear the 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 a9d03af34030..adb0ca035b97 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1196,6 +1196,8 @@ struct kvm_x86_ops { uint16_t (*nested_get_evmcs_version)(struct kvm_vcpu *vcpu); bool (*need_emulation_on_page_fault)(struct kvm_vcpu *vcpu); + int (*page_enc_status_hc)(struct kvm *kvm, unsigned long gpa, + unsigned long sz, unsigned long mode); }; struct kvm_arch_async_pf { diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 74b57ab742ad..f024f208b052 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -138,6 +138,8 @@ struct kvm_sev_info { int fd; /* SEV device fd */ unsigned long pages_locked; /* Number of pages locked */ struct list_head regions_list; /* List of registered regions */ + unsigned long *page_enc_bmap; + unsigned long page_enc_bmap_size; }; struct kvm_svm { @@ -1911,6 +1913,8 @@ static void sev_vm_destroy(struct kvm *kvm) sev_unbind_asid(kvm, sev->handle); sev_asid_free(kvm); + + kvfree(sev->page_enc_bmap); } static void avic_vm_destroy(struct kvm *kvm) @@ -7370,6 +7374,69 @@ static int sev_receive_finish(struct kvm *kvm, struct kvm_sev_cmd *argp) return ret; } +static int sev_resize_page_enc_bitmap(struct kvm *kvm, unsigned long new_size) +{ + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; + unsigned long *map; + unsigned long sz; + + if (sev->page_enc_bmap_size >= new_size) + return 0; + + sz = ALIGN(new_size, BITS_PER_LONG) / 8; + + if (sz > PAGE_SIZE) + map = vmalloc(sz); + else + map = kmalloc(sz, GFP_KERNEL); + + if (!map) { + pr_err_once("Failed to allocate decrypted bitmap size %lx\n", sz); + return 1; + } + + /* mark the page encrypted (by default) */ + memset(map, 0xff, sz); + + bitmap_copy(map, sev->page_enc_bmap, sev->page_enc_bmap_size); + kvfree(sev->page_enc_bmap); + + sev->page_enc_bmap = map; + sev->page_enc_bmap_size = new_size; + + return 0; +} + +static int svm_page_enc_status_hc(struct kvm *kvm, unsigned long gpa, + unsigned long npages, unsigned long enc) +{ + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; + gfn_t gfn_start, gfn_end; + int r; + + if (!npages) + return 0; + + gfn_start = gpa_to_gfn(gpa); + gfn_end = gfn_start + npages; + + mutex_lock(&kvm->lock); + + r = 1; + if (sev_resize_page_enc_bitmap(kvm, gfn_end)) + goto unlock; + + if (enc) + __bitmap_set(sev->page_enc_bmap, gfn_start, gfn_end - gfn_start); + else + __bitmap_clear(sev->page_enc_bmap, gfn_start, gfn_end - gfn_start); + + r = 0; +unlock: + mutex_unlock(&kvm->lock); + return r; +} + static int svm_mem_enc_op(struct kvm *kvm, void __user *argp) { struct kvm_sev_cmd sev_cmd; @@ -7711,6 +7778,8 @@ static struct kvm_x86_ops svm_x86_ops __ro_after_init = { .nested_get_evmcs_version = nested_get_evmcs_version, .need_emulation_on_page_fault = svm_need_emulation_on_page_fault, + + .page_enc_status_hc = svm_page_enc_status_hc }; static int __init svm_init(void) diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index b4e7d645275a..9c814e560e0f 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -7731,6 +7731,7 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { .get_vmcs12_pages = NULL, .nested_enable_evmcs = NULL, .need_emulation_on_page_fault = vmx_need_emulation_on_page_fault, + .page_enc_status_hc = NULL, }; static void vmx_cleanup_l1d_flush(void) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index a0d1fc80ac5a..dea644be5992 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -7141,6 +7141,11 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) case KVM_HC_SEND_IPI: ret = kvm_pv_send_ipi(vcpu->kvm, a0, a1, a2, a3, op_64_bit); break; + case KVM_HC_PAGE_ENC_STATUS: + ret = -KVM_ENOSYS; + if (kvm_x86_ops->page_enc_status_hc) + ret = kvm_x86_ops->page_enc_status_hc(vcpu->kvm, a0, a1, a2); + break; default: ret = -KVM_ENOSYS; break; diff --git a/include/uapi/linux/kvm_para.h b/include/uapi/linux/kvm_para.h index 6c0ce49931e5..3dc9e579f4f9 100644 --- a/include/uapi/linux/kvm_para.h +++ b/include/uapi/linux/kvm_para.h @@ -28,6 +28,7 @@ #define KVM_HC_MIPS_CONSOLE_OUTPUT 8 #define KVM_HC_CLOCK_PAIRING 9 #define KVM_HC_SEND_IPI 10 +#define KVM_HC_PAGE_ENC_STATUS 11 /* * hypercalls use architecture specific
The hypercall can be used by the SEV guest to notify 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 the guest pages should be considered encrypted. Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: "Radim Krčmář" <rkrcmar@redhat.com> Cc: Joerg Roedel <joro@8bytes.org> Cc: Borislav Petkov <bp@suse.de> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: x86@kernel.org Cc: kvm@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- Documentation/virtual/kvm/hypercalls.txt | 14 +++++ arch/x86/include/asm/kvm_host.h | 2 + arch/x86/kvm/svm.c | 69 ++++++++++++++++++++++++ arch/x86/kvm/vmx/vmx.c | 1 + arch/x86/kvm/x86.c | 5 ++ include/uapi/linux/kvm_para.h | 1 + 6 files changed, 92 insertions(+)