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.
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: "Radim Krčmář" <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Brijesh Singh <[email protected]>
---
Documentation/virtual/kvm/hypercalls.txt | 14 +++++
arch/x86/include/asm/kvm_host.h | 2 +
arch/x86/kvm/svm.c | 70 ++++++++++++++++++++++++
arch/x86/kvm/vmx/vmx.c | 1 +
arch/x86/kvm/x86.c | 5 ++
include/uapi/linux/kvm_para.h | 1 +
6 files changed, 93 insertions(+)
diff --git a/Documentation/virtual/kvm/hypercalls.txt b/Documentation/virtual/kvm/hypercalls.txt
index da24c138c8d1..94f0611f4d88 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: 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 26d1eb83f72a..b463a81dc176 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1199,6 +1199,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 3089942f6630..431718309359 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -135,6 +135,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 {
@@ -1910,6 +1912,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)
@@ -2084,6 +2088,7 @@ static void avic_set_running(struct kvm_vcpu *vcpu, bool is_run)
static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
{
+ struct kvm_sev_info *sev = &to_kvm_svm(vcpu->kvm)->sev_info;
struct vcpu_svm *svm = to_svm(vcpu);
u32 dummy;
u32 eax = 1;
@@ -2105,6 +2110,12 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
if (kvm_vcpu_apicv_active(vcpu) && !init_event)
avic_update_vapic_bar(svm, APIC_DEFAULT_PHYS_BASE);
+
+ /* reset the page encryption bitmap */
+ if (sev_guest(vcpu->kvm)) {
+ kvfree(sev->page_enc_bmap);
+ sev->page_enc_bmap_size = 0;
+ }
}
static int avic_init_vcpu(struct vcpu_svm *svm)
@@ -7357,6 +7368,63 @@ 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;
+
+ map = vmalloc(sz);
+ if (!map) {
+ pr_err_once("Failed to allocate decrypted bitmap size %lx\n", sz);
+ return -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 ret;
+
+ if (!npages)
+ return 0;
+
+ gfn_start = gpa_to_gfn(gpa);
+ gfn_end = gfn_start + npages;
+
+ mutex_lock(&kvm->lock);
+ ret = sev_resize_page_enc_bitmap(kvm, gfn_end);
+ 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);
+
+unlock:
+ mutex_unlock(&kvm->lock);
+ return ret;
+}
+
static int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
{
struct kvm_sev_cmd sev_cmd;
@@ -7698,6 +7766,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 d98eac371c0a..78f8a93fc6dd 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7724,6 +7724,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 63bb1ee8258e..6baf48ec0ed4 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7219,6 +7219,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
--
2.17.1
On Wed, 10 Jul 2019, Singh, Brijesh wrote:
> diff --git a/Documentation/virtual/kvm/hypercalls.txt b/Documentation/virtual/kvm/hypercalls.txt
> index da24c138c8d1..94f0611f4d88 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: 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 26d1eb83f72a..b463a81dc176 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1199,6 +1199,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 3089942f6630..431718309359 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -135,6 +135,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 {
> @@ -1910,6 +1912,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)
Adding Cfir who flagged this kvfree().
Other freeing of sev->page_enc_bmap in this patch also set
sev->page_enc_bmap_size to 0 and neither set sev->page_enc_bmap to NULL
after freeing it.
For extra safety, is it possible to sev->page_enc_bmap = NULL anytime the
bitmap is kvfreed?
> @@ -2084,6 +2088,7 @@ static void avic_set_running(struct kvm_vcpu *vcpu, bool is_run)
>
> static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> {
> + struct kvm_sev_info *sev = &to_kvm_svm(vcpu->kvm)->sev_info;
> struct vcpu_svm *svm = to_svm(vcpu);
> u32 dummy;
> u32 eax = 1;
> @@ -2105,6 +2110,12 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>
> if (kvm_vcpu_apicv_active(vcpu) && !init_event)
> avic_update_vapic_bar(svm, APIC_DEFAULT_PHYS_BASE);
> +
> + /* reset the page encryption bitmap */
> + if (sev_guest(vcpu->kvm)) {
> + kvfree(sev->page_enc_bmap);
> + sev->page_enc_bmap_size = 0;
> + }
> }
>
> static int avic_init_vcpu(struct vcpu_svm *svm)
What is protecting sev->page_enc_bmap and sev->page_enc_bmap_size in calls
to svm_vcpu_reset()?
In addition, it seems that svm_page_enc_status_hc() accepts 'gpa',
'npages', 'enc' directly from the guest, and so these can take
arbitrary values. A very large 'npages' could lead to an int overflow
in 'gfn_end = gfn_start + npages', making gfn_end < gfn_start. This
could an OOB access in the bitmap. Concrete example: gfn_start = 2,
npages = -1, gfn_end = 2+(-1) = 1, sev_resize_page_enc_bitmap
allocates a bitmap for a single page (new_size=1), __bitmap_set access
offset gfn_end - gfn_start = -1.
On Sun, Jul 21, 2019 at 1:57 PM David Rientjes <[email protected]> wrote:
>
> On Wed, 10 Jul 2019, Singh, Brijesh wrote:
>
> > diff --git a/Documentation/virtual/kvm/hypercalls.txt b/Documentation/virtual/kvm/hypercalls.txt
> > index da24c138c8d1..94f0611f4d88 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: 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 26d1eb83f72a..b463a81dc176 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1199,6 +1199,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 3089942f6630..431718309359 100644
> > --- a/arch/x86/kvm/svm.c
> > +++ b/arch/x86/kvm/svm.c
> > @@ -135,6 +135,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 {
> > @@ -1910,6 +1912,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)
>
> Adding Cfir who flagged this kvfree().
>
> Other freeing of sev->page_enc_bmap in this patch also set
> sev->page_enc_bmap_size to 0 and neither set sev->page_enc_bmap to NULL
> after freeing it.
>
> For extra safety, is it possible to sev->page_enc_bmap = NULL anytime the
> bitmap is kvfreed?
>
> > @@ -2084,6 +2088,7 @@ static void avic_set_running(struct kvm_vcpu *vcpu, bool is_run)
> >
> > static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> > {
> > + struct kvm_sev_info *sev = &to_kvm_svm(vcpu->kvm)->sev_info;
> > struct vcpu_svm *svm = to_svm(vcpu);
> > u32 dummy;
> > u32 eax = 1;
> > @@ -2105,6 +2110,12 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> >
> > if (kvm_vcpu_apicv_active(vcpu) && !init_event)
> > avic_update_vapic_bar(svm, APIC_DEFAULT_PHYS_BASE);
> > +
> > + /* reset the page encryption bitmap */
> > + if (sev_guest(vcpu->kvm)) {
> > + kvfree(sev->page_enc_bmap);
> > + sev->page_enc_bmap_size = 0;
> > + }
> > }
> >
> > static int avic_init_vcpu(struct vcpu_svm *svm)
>
> What is protecting sev->page_enc_bmap and sev->page_enc_bmap_size in calls
> to svm_vcpu_reset()?
On 7/21/19 3:57 PM, David Rientjes wrote:
> On Wed, 10 Jul 2019, Singh, Brijesh wrote:
>
>> diff --git a/Documentation/virtual/kvm/hypercalls.txt b/Documentation/virtual/kvm/hypercalls.txt
>> index da24c138c8d1..94f0611f4d88 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: 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 26d1eb83f72a..b463a81dc176 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -1199,6 +1199,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 3089942f6630..431718309359 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -135,6 +135,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 {
>> @@ -1910,6 +1912,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)
>
> Adding Cfir who flagged this kvfree().
>
> Other freeing of sev->page_enc_bmap in this patch also set
> sev->page_enc_bmap_size to 0 and neither set sev->page_enc_bmap to NULL
> after freeing it.
>
> For extra safety, is it possible to sev->page_enc_bmap = NULL anytime the
> bitmap is kvfreed?
>
Good catch, I'll fix it in next rev.
>> @@ -2084,6 +2088,7 @@ static void avic_set_running(struct kvm_vcpu *vcpu, bool is_run)
>>
>> static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>> {
>> + struct kvm_sev_info *sev = &to_kvm_svm(vcpu->kvm)->sev_info;
>> struct vcpu_svm *svm = to_svm(vcpu);
>> u32 dummy;
>> u32 eax = 1;
>> @@ -2105,6 +2110,12 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>>
>> if (kvm_vcpu_apicv_active(vcpu) && !init_event)
>> avic_update_vapic_bar(svm, APIC_DEFAULT_PHYS_BASE);
>> +
>> + /* reset the page encryption bitmap */
>> + if (sev_guest(vcpu->kvm)) {
>> + kvfree(sev->page_enc_bmap);
>> + sev->page_enc_bmap_size = 0;
>> + }
>> }
>>
>> static int avic_init_vcpu(struct vcpu_svm *svm)
>
> What is protecting sev->page_enc_bmap and sev->page_enc_bmap_size in calls
> to svm_vcpu_reset()?
>
Yes, it need to be protected with vm lock. I will fix it in next rev.
Additionally, I think what I have here is wrong, we need to reset the
bitmap only when bsp is getting reset.
-Brijesh
On 7/22/19 12:12 PM, Cfir Cohen wrote:
> In addition, it seems that svm_page_enc_status_hc() accepts 'gpa',
> 'npages', 'enc' directly from the guest, and so these can take
> arbitrary values. A very large 'npages' could lead to an int overflow
> in 'gfn_end = gfn_start + npages', making gfn_end < gfn_start. This
> could an OOB access in the bitmap. Concrete example: gfn_start = 2,
> npages = -1, gfn_end = 2+(-1) = 1, sev_resize_page_enc_bitmap
> allocates a bitmap for a single page (new_size=1), __bitmap_set access
> offset gfn_end - gfn_start = -1.
>
Good point. I will add a check for it, something like
if (gfn_end <= gfn_start)
return -EINVAL;
>
> On Sun, Jul 21, 2019 at 1:57 PM David Rientjes <[email protected]> wrote:
>>
>> On Wed, 10 Jul 2019, Singh, Brijesh wrote:
>>
>>> diff --git a/Documentation/virtual/kvm/hypercalls.txt b/Documentation/virtual/kvm/hypercalls.txt
>>> index da24c138c8d1..94f0611f4d88 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: 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 26d1eb83f72a..b463a81dc176 100644
>>> --- a/arch/x86/include/asm/kvm_host.h
>>> +++ b/arch/x86/include/asm/kvm_host.h
>>> @@ -1199,6 +1199,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 3089942f6630..431718309359 100644
>>> --- a/arch/x86/kvm/svm.c
>>> +++ b/arch/x86/kvm/svm.c
>>> @@ -135,6 +135,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 {
>>> @@ -1910,6 +1912,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)
>>
>> Adding Cfir who flagged this kvfree().
>>
>> Other freeing of sev->page_enc_bmap in this patch also set
>> sev->page_enc_bmap_size to 0 and neither set sev->page_enc_bmap to NULL
>> after freeing it.
>>
>> For extra safety, is it possible to sev->page_enc_bmap = NULL anytime the
>> bitmap is kvfreed?
>>
>>> @@ -2084,6 +2088,7 @@ static void avic_set_running(struct kvm_vcpu *vcpu, bool is_run)
>>>
>>> static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>>> {
>>> + struct kvm_sev_info *sev = &to_kvm_svm(vcpu->kvm)->sev_info;
>>> struct vcpu_svm *svm = to_svm(vcpu);
>>> u32 dummy;
>>> u32 eax = 1;
>>> @@ -2105,6 +2110,12 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>>>
>>> if (kvm_vcpu_apicv_active(vcpu) && !init_event)
>>> avic_update_vapic_bar(svm, APIC_DEFAULT_PHYS_BASE);
>>> +
>>> + /* reset the page encryption bitmap */
>>> + if (sev_guest(vcpu->kvm)) {
>>> + kvfree(sev->page_enc_bmap);
>>> + sev->page_enc_bmap_size = 0;
>>> + }
>>> }
>>>
>>> static int avic_init_vcpu(struct vcpu_svm *svm)
>>
>> What is protecting sev->page_enc_bmap and sev->page_enc_bmap_size in calls
>> to svm_vcpu_reset()?
> struct kvm_arch_async_pf {
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 3089942f6630..431718309359 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -135,6 +135,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;
> };
>
Just a high level question. Would it be better for these bitmaps to
live in kvm_memory_slot and the ioctl to be take a memslot instead of
a GPA + length? The c-bit status bitmap will probably need to be
checked at when checking the dirty log and KVM_GET_DIRTY_LOG
operations on memslots.