diff mbox series

[v6,14/14] KVM: x86: Add kexec support for SEV Live Migration.

Message ID 0caf809845d2fdb1a1ec17955826df9777f502fb.1585548051.git.ashish.kalra@amd.com (mailing list archive)
State New, archived
Headers show
Series Add AMD SEV guest live migration support | expand

Commit Message

Kalra, Ashish March 30, 2020, 6:23 a.m. UTC
From: Ashish Kalra <ashish.kalra@amd.com>

Reset the host's page encryption bitmap related to kernel
specific page encryption status settings before we load a
new kernel by kexec. We cannot reset the complete
page encryption bitmap here as we need to retain the
UEFI/OVMF firmware specific settings.

Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 arch/x86/kernel/kvm.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Brijesh Singh March 30, 2020, 4 p.m. UTC | #1
On 3/30/20 1:23 AM, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
>
> Reset the host's page encryption bitmap related to kernel
> specific page encryption status settings before we load a
> new kernel by kexec. We cannot reset the complete
> page encryption bitmap here as we need to retain the
> UEFI/OVMF firmware specific settings.
>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  arch/x86/kernel/kvm.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 8fcee0b45231..ba6cce3c84af 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -34,6 +34,7 @@
>  #include <asm/hypervisor.h>
>  #include <asm/tlb.h>
>  #include <asm/cpuidle_haltpoll.h>
> +#include <asm/e820/api.h>
>  
>  static int kvmapf = 1;
>  
> @@ -357,6 +358,33 @@ static void kvm_pv_guest_cpu_reboot(void *unused)
>  	 */
>  	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
>  		wrmsrl(MSR_KVM_PV_EOI_EN, 0);
> +	/*
> +	 * Reset the host's page encryption bitmap related to kernel
> +	 * specific page encryption status settings before we load a
> +	 * new kernel by kexec. NOTE: We cannot reset the complete
> +	 * page encryption bitmap here as we need to retain the
> +	 * UEFI/OVMF firmware specific settings.
> +	 */
> +	if (kvm_para_has_feature(KVM_FEATURE_SEV_LIVE_MIGRATION) &&
> +		(smp_processor_id() == 0)) {


In patch 13/14, the KVM_FEATURE_SEV_LIVE_MIGRATION is set
unconditionally and because of that now the below code will be executed
on non-SEV guest. IMO, this feature must be cleared for non-SEV guest to
avoid making unnecessary hypercall's.


> +		unsigned long nr_pages;
> +		int i;
> +
> +		for (i = 0; i < e820_table->nr_entries; i++) {
> +			struct e820_entry *entry = &e820_table->entries[i];
> +			unsigned long start_pfn, end_pfn;
> +
> +			if (entry->type != E820_TYPE_RAM)
> +				continue;
> +
> +			start_pfn = entry->addr >> PAGE_SHIFT;
> +			end_pfn = (entry->addr + entry->size) >> PAGE_SHIFT;
> +			nr_pages = DIV_ROUND_UP(entry->size, PAGE_SIZE);
> +
> +			kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS,
> +				entry->addr, nr_pages, 1);
> +		}
> +	}
>  	kvm_pv_disable_apf();
>  	kvm_disable_steal_time();
>  }
Kalra, Ashish March 30, 2020, 4:45 p.m. UTC | #2
Hello Brijesh,

On Mon, Mar 30, 2020 at 11:00:14AM -0500, Brijesh Singh wrote:
> 
> On 3/30/20 1:23 AM, Ashish Kalra wrote:
> > From: Ashish Kalra <ashish.kalra@amd.com>
> >
> > Reset the host's page encryption bitmap related to kernel
> > specific page encryption status settings before we load a
> > new kernel by kexec. We cannot reset the complete
> > page encryption bitmap here as we need to retain the
> > UEFI/OVMF firmware specific settings.
> >
> > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> > ---
> >  arch/x86/kernel/kvm.c | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> > index 8fcee0b45231..ba6cce3c84af 100644
> > --- a/arch/x86/kernel/kvm.c
> > +++ b/arch/x86/kernel/kvm.c
> > @@ -34,6 +34,7 @@
> >  #include <asm/hypervisor.h>
> >  #include <asm/tlb.h>
> >  #include <asm/cpuidle_haltpoll.h>
> > +#include <asm/e820/api.h>
> >  
> >  static int kvmapf = 1;
> >  
> > @@ -357,6 +358,33 @@ static void kvm_pv_guest_cpu_reboot(void *unused)
> >  	 */
> >  	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
> >  		wrmsrl(MSR_KVM_PV_EOI_EN, 0);
> > +	/*
> > +	 * Reset the host's page encryption bitmap related to kernel
> > +	 * specific page encryption status settings before we load a
> > +	 * new kernel by kexec. NOTE: We cannot reset the complete
> > +	 * page encryption bitmap here as we need to retain the
> > +	 * UEFI/OVMF firmware specific settings.
> > +	 */
> > +	if (kvm_para_has_feature(KVM_FEATURE_SEV_LIVE_MIGRATION) &&
> > +		(smp_processor_id() == 0)) {
> 
> 
> In patch 13/14, the KVM_FEATURE_SEV_LIVE_MIGRATION is set
> unconditionally and because of that now the below code will be executed
> on non-SEV guest. IMO, this feature must be cleared for non-SEV guest to
> avoid making unnecessary hypercall's.
> 
>

I will additionally add a sev_active() check here to ensure that we don't make the unnecassary hypercalls on non-SEV guests.

> > +		unsigned long nr_pages;
> > +		int i;
> > +
> > +		for (i = 0; i < e820_table->nr_entries; i++) {
> > +			struct e820_entry *entry = &e820_table->entries[i];
> > +			unsigned long start_pfn, end_pfn;
> > +
> > +			if (entry->type != E820_TYPE_RAM)
> > +				continue;
> > +
> > +			start_pfn = entry->addr >> PAGE_SHIFT;
> > +			end_pfn = (entry->addr + entry->size) >> PAGE_SHIFT;
> > +			nr_pages = DIV_ROUND_UP(entry->size, PAGE_SIZE);
> > +
> > +			kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS,
> > +				entry->addr, nr_pages, 1);
> > +		}
> > +	}
> >  	kvm_pv_disable_apf();
> >  	kvm_disable_steal_time();
> >  }

Thanks,
Ashish
Brijesh Singh March 31, 2020, 2:26 p.m. UTC | #3
On 3/30/20 11:45 AM, Ashish Kalra wrote:
> Hello Brijesh,
>
> On Mon, Mar 30, 2020 at 11:00:14AM -0500, Brijesh Singh wrote:
>> On 3/30/20 1:23 AM, Ashish Kalra wrote:
>>> From: Ashish Kalra <ashish.kalra@amd.com>
>>>
>>> Reset the host's page encryption bitmap related to kernel
>>> specific page encryption status settings before we load a
>>> new kernel by kexec. We cannot reset the complete
>>> page encryption bitmap here as we need to retain the
>>> UEFI/OVMF firmware specific settings.
>>>
>>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>>> ---
>>>  arch/x86/kernel/kvm.c | 28 ++++++++++++++++++++++++++++
>>>  1 file changed, 28 insertions(+)
>>>
>>> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
>>> index 8fcee0b45231..ba6cce3c84af 100644
>>> --- a/arch/x86/kernel/kvm.c
>>> +++ b/arch/x86/kernel/kvm.c
>>> @@ -34,6 +34,7 @@
>>>  #include <asm/hypervisor.h>
>>>  #include <asm/tlb.h>
>>>  #include <asm/cpuidle_haltpoll.h>
>>> +#include <asm/e820/api.h>
>>>  
>>>  static int kvmapf = 1;
>>>  
>>> @@ -357,6 +358,33 @@ static void kvm_pv_guest_cpu_reboot(void *unused)
>>>  	 */
>>>  	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
>>>  		wrmsrl(MSR_KVM_PV_EOI_EN, 0);
>>> +	/*
>>> +	 * Reset the host's page encryption bitmap related to kernel
>>> +	 * specific page encryption status settings before we load a
>>> +	 * new kernel by kexec. NOTE: We cannot reset the complete
>>> +	 * page encryption bitmap here as we need to retain the
>>> +	 * UEFI/OVMF firmware specific settings.
>>> +	 */
>>> +	if (kvm_para_has_feature(KVM_FEATURE_SEV_LIVE_MIGRATION) &&
>>> +		(smp_processor_id() == 0)) {
>>
>> In patch 13/14, the KVM_FEATURE_SEV_LIVE_MIGRATION is set
>> unconditionally and because of that now the below code will be executed
>> on non-SEV guest. IMO, this feature must be cleared for non-SEV guest to
>> avoid making unnecessary hypercall's.
>>
>>
> I will additionally add a sev_active() check here to ensure that we don't make the unnecassary hypercalls on non-SEV guests.


IMO, instead of using the sev_active() we should make sure that the
feature is not enabled when SEV is not active.


>>> +		unsigned long nr_pages;
>>> +		int i;
>>> +
>>> +		for (i = 0; i < e820_table->nr_entries; i++) {
>>> +			struct e820_entry *entry = &e820_table->entries[i];
>>> +			unsigned long start_pfn, end_pfn;
>>> +
>>> +			if (entry->type != E820_TYPE_RAM)
>>> +				continue;
>>> +
>>> +			start_pfn = entry->addr >> PAGE_SHIFT;
>>> +			end_pfn = (entry->addr + entry->size) >> PAGE_SHIFT;
>>> +			nr_pages = DIV_ROUND_UP(entry->size, PAGE_SIZE);
>>> +
>>> +			kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS,
>>> +				entry->addr, nr_pages, 1);
>>> +		}
>>> +	}
>>>  	kvm_pv_disable_apf();
>>>  	kvm_disable_steal_time();
>>>  }
> Thanks,
> Ashish
Kalra, Ashish April 2, 2020, 11:34 p.m. UTC | #4
Hello Brijesh,

On Tue, Mar 31, 2020 at 09:26:26AM -0500, Brijesh Singh wrote:
> 
> On 3/30/20 11:45 AM, Ashish Kalra wrote:
> > Hello Brijesh,
> >
> > On Mon, Mar 30, 2020 at 11:00:14AM -0500, Brijesh Singh wrote:
> >> On 3/30/20 1:23 AM, Ashish Kalra wrote:
> >>> From: Ashish Kalra <ashish.kalra@amd.com>
> >>>
> >>> Reset the host's page encryption bitmap related to kernel
> >>> specific page encryption status settings before we load a
> >>> new kernel by kexec. We cannot reset the complete
> >>> page encryption bitmap here as we need to retain the
> >>> UEFI/OVMF firmware specific settings.
> >>>
> >>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> >>> ---
> >>>  arch/x86/kernel/kvm.c | 28 ++++++++++++++++++++++++++++
> >>>  1 file changed, 28 insertions(+)
> >>>
> >>> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> >>> index 8fcee0b45231..ba6cce3c84af 100644
> >>> --- a/arch/x86/kernel/kvm.c
> >>> +++ b/arch/x86/kernel/kvm.c
> >>> @@ -34,6 +34,7 @@
> >>>  #include <asm/hypervisor.h>
> >>>  #include <asm/tlb.h>
> >>>  #include <asm/cpuidle_haltpoll.h>
> >>> +#include <asm/e820/api.h>
> >>>  
> >>>  static int kvmapf = 1;
> >>>  
> >>> @@ -357,6 +358,33 @@ static void kvm_pv_guest_cpu_reboot(void *unused)
> >>>  	 */
> >>>  	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
> >>>  		wrmsrl(MSR_KVM_PV_EOI_EN, 0);
> >>> +	/*
> >>> +	 * Reset the host's page encryption bitmap related to kernel
> >>> +	 * specific page encryption status settings before we load a
> >>> +	 * new kernel by kexec. NOTE: We cannot reset the complete
> >>> +	 * page encryption bitmap here as we need to retain the
> >>> +	 * UEFI/OVMF firmware specific settings.
> >>> +	 */
> >>> +	if (kvm_para_has_feature(KVM_FEATURE_SEV_LIVE_MIGRATION) &&
> >>> +		(smp_processor_id() == 0)) {
> >>
> >> In patch 13/14, the KVM_FEATURE_SEV_LIVE_MIGRATION is set
> >> unconditionally and because of that now the below code will be executed
> >> on non-SEV guest. IMO, this feature must be cleared for non-SEV guest to
> >> avoid making unnecessary hypercall's.
> >>
> >>
> > I will additionally add a sev_active() check here to ensure that we don't make the unnecassary hypercalls on non-SEV guests.
> 
> 
> IMO, instead of using the sev_active() we should make sure that the
> feature is not enabled when SEV is not active.
> 

Yes, now the KVM_FEATURE_SEV_LIVE_MIGRATION feature is enabled
dynamically in svm_cpuid_update() after it gets called from
svm_launch_finish(), which ensures that it only gets set when a SEV
guest is active.

Thanks,
Ashish

> 
> >>> +		unsigned long nr_pages;
> >>> +		int i;
> >>> +
> >>> +		for (i = 0; i < e820_table->nr_entries; i++) {
> >>> +			struct e820_entry *entry = &e820_table->entries[i];
> >>> +			unsigned long start_pfn, end_pfn;
> >>> +
> >>> +			if (entry->type != E820_TYPE_RAM)
> >>> +				continue;
> >>> +
> >>> +			start_pfn = entry->addr >> PAGE_SHIFT;
> >>> +			end_pfn = (entry->addr + entry->size) >> PAGE_SHIFT;
> >>> +			nr_pages = DIV_ROUND_UP(entry->size, PAGE_SIZE);
> >>> +
> >>> +			kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS,
> >>> +				entry->addr, nr_pages, 1);
> >>> +		}
> >>> +	}
> >>>  	kvm_pv_disable_apf();
> >>>  	kvm_disable_steal_time();
> >>>  }
> > Thanks,
> > Ashish
Dave Young April 3, 2020, 12:57 p.m. UTC | #5
Ccing kexec list.

Ashish, could you cc kexec list if you repost later?
On 03/30/20 at 06:23am, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Reset the host's page encryption bitmap related to kernel
> specific page encryption status settings before we load a
> new kernel by kexec. We cannot reset the complete
> page encryption bitmap here as we need to retain the
> UEFI/OVMF firmware specific settings.
> 
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  arch/x86/kernel/kvm.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 8fcee0b45231..ba6cce3c84af 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -34,6 +34,7 @@
>  #include <asm/hypervisor.h>
>  #include <asm/tlb.h>
>  #include <asm/cpuidle_haltpoll.h>
> +#include <asm/e820/api.h>
>  
>  static int kvmapf = 1;
>  
> @@ -357,6 +358,33 @@ static void kvm_pv_guest_cpu_reboot(void *unused)
>  	 */
>  	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
>  		wrmsrl(MSR_KVM_PV_EOI_EN, 0);
> +	/*
> +	 * Reset the host's page encryption bitmap related to kernel
> +	 * specific page encryption status settings before we load a
> +	 * new kernel by kexec. NOTE: We cannot reset the complete
> +	 * page encryption bitmap here as we need to retain the
> +	 * UEFI/OVMF firmware specific settings.
> +	 */
> +	if (kvm_para_has_feature(KVM_FEATURE_SEV_LIVE_MIGRATION) &&
> +		(smp_processor_id() == 0)) {
> +		unsigned long nr_pages;
> +		int i;
> +
> +		for (i = 0; i < e820_table->nr_entries; i++) {
> +			struct e820_entry *entry = &e820_table->entries[i];
> +			unsigned long start_pfn, end_pfn;
> +
> +			if (entry->type != E820_TYPE_RAM)
> +				continue;
> +
> +			start_pfn = entry->addr >> PAGE_SHIFT;
> +			end_pfn = (entry->addr + entry->size) >> PAGE_SHIFT;
> +			nr_pages = DIV_ROUND_UP(entry->size, PAGE_SIZE);
> +
> +			kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS,
> +				entry->addr, nr_pages, 1);
> +		}
> +	}
>  	kvm_pv_disable_apf();
>  	kvm_disable_steal_time();
>  }
> -- 
> 2.17.1
>
Krish Sadhukhan April 4, 2020, 12:55 a.m. UTC | #6
On 3/29/20 11:23 PM, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
>
> Reset the host's page encryption bitmap related to kernel
> specific page encryption status settings before we load a
> new kernel by kexec. We cannot reset the complete
> page encryption bitmap here as we need to retain the
> UEFI/OVMF firmware specific settings.


Can the commit message mention why host page encryption needs to be 
reset ? Since the theme of these patches is guest migration in-SEV 
context, it might be useful to mention why the host context comes in here.

>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>   arch/x86/kernel/kvm.c | 28 ++++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
>
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 8fcee0b45231..ba6cce3c84af 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -34,6 +34,7 @@
>   #include <asm/hypervisor.h>
>   #include <asm/tlb.h>
>   #include <asm/cpuidle_haltpoll.h>
> +#include <asm/e820/api.h>
>   
>   static int kvmapf = 1;
>   
> @@ -357,6 +358,33 @@ static void kvm_pv_guest_cpu_reboot(void *unused)
>   	 */
>   	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
>   		wrmsrl(MSR_KVM_PV_EOI_EN, 0);
> +	/*
> +	 * Reset the host's page encryption bitmap related to kernel
> +	 * specific page encryption status settings before we load a
> +	 * new kernel by kexec. NOTE: We cannot reset the complete
> +	 * page encryption bitmap here as we need to retain the
> +	 * UEFI/OVMF firmware specific settings.
> +	 */
> +	if (kvm_para_has_feature(KVM_FEATURE_SEV_LIVE_MIGRATION) &&
> +		(smp_processor_id() == 0)) {
> +		unsigned long nr_pages;
> +		int i;
> +
> +		for (i = 0; i < e820_table->nr_entries; i++) {
> +			struct e820_entry *entry = &e820_table->entries[i];
> +			unsigned long start_pfn, end_pfn;
> +
> +			if (entry->type != E820_TYPE_RAM)
> +				continue;
> +
> +			start_pfn = entry->addr >> PAGE_SHIFT;
> +			end_pfn = (entry->addr + entry->size) >> PAGE_SHIFT;
> +			nr_pages = DIV_ROUND_UP(entry->size, PAGE_SIZE);
> +
> +			kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS,
> +				entry->addr, nr_pages, 1);
> +		}
> +	}
>   	kvm_pv_disable_apf();
>   	kvm_disable_steal_time();
>   }
Kalra, Ashish April 4, 2020, 9:57 p.m. UTC | #7
The host's page encryption bitmap is maintained for the guest to keep the encrypted/decrypted state 
of the guest pages, therefore we need to explicitly mark all shared pages as encrypted again before
rebooting into the new guest kernel.

On Fri, Apr 03, 2020 at 05:55:52PM -0700, Krish Sadhukhan wrote:
> 
> On 3/29/20 11:23 PM, Ashish Kalra wrote:
> > From: Ashish Kalra <ashish.kalra@amd.com>
> > 
> > Reset the host's page encryption bitmap related to kernel
> > specific page encryption status settings before we load a
> > new kernel by kexec. We cannot reset the complete
> > page encryption bitmap here as we need to retain the
> > UEFI/OVMF firmware specific settings.
> 
> 
> Can the commit message mention why host page encryption needs to be reset ?
> Since the theme of these patches is guest migration in-SEV context, it might
> be useful to mention why the host context comes in here.
> 
> > 
> > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> > ---
> >   arch/x86/kernel/kvm.c | 28 ++++++++++++++++++++++++++++
> >   1 file changed, 28 insertions(+)
> > 
> > diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> > index 8fcee0b45231..ba6cce3c84af 100644
> > --- a/arch/x86/kernel/kvm.c
> > +++ b/arch/x86/kernel/kvm.c
> > @@ -34,6 +34,7 @@
> >   #include <asm/hypervisor.h>
> >   #include <asm/tlb.h>
> >   #include <asm/cpuidle_haltpoll.h>
> > +#include <asm/e820/api.h>
> >   static int kvmapf = 1;
> > @@ -357,6 +358,33 @@ static void kvm_pv_guest_cpu_reboot(void *unused)
> >   	 */
> >   	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
> >   		wrmsrl(MSR_KVM_PV_EOI_EN, 0);
> > +	/*
> > +	 * Reset the host's page encryption bitmap related to kernel
> > +	 * specific page encryption status settings before we load a
> > +	 * new kernel by kexec. NOTE: We cannot reset the complete
> > +	 * page encryption bitmap here as we need to retain the
> > +	 * UEFI/OVMF firmware specific settings.
> > +	 */
> > +	if (kvm_para_has_feature(KVM_FEATURE_SEV_LIVE_MIGRATION) &&
> > +		(smp_processor_id() == 0)) {
> > +		unsigned long nr_pages;
> > +		int i;
> > +
> > +		for (i = 0; i < e820_table->nr_entries; i++) {
> > +			struct e820_entry *entry = &e820_table->entries[i];
> > +			unsigned long start_pfn, end_pfn;
> > +
> > +			if (entry->type != E820_TYPE_RAM)
> > +				continue;
> > +
> > +			start_pfn = entry->addr >> PAGE_SHIFT;
> > +			end_pfn = (entry->addr + entry->size) >> PAGE_SHIFT;
> > +			nr_pages = DIV_ROUND_UP(entry->size, PAGE_SIZE);
> > +
> > +			kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS,
> > +				entry->addr, nr_pages, 1);
> > +		}
> > +	}
> >   	kvm_pv_disable_apf();
> >   	kvm_disable_steal_time();
> >   }
Krish Sadhukhan April 6, 2020, 6:37 p.m. UTC | #8
On 4/4/20 2:57 PM, Ashish Kalra wrote:
> The host's page encryption bitmap is maintained for the guest to keep the encrypted/decrypted state
> of the guest pages, therefore we need to explicitly mark all shared pages as encrypted again before
> rebooting into the new guest kernel.
>
> On Fri, Apr 03, 2020 at 05:55:52PM -0700, Krish Sadhukhan wrote:
>> On 3/29/20 11:23 PM, Ashish Kalra wrote:
>>> From: Ashish Kalra <ashish.kalra@amd.com>
>>>
>>> Reset the host's page encryption bitmap related to kernel
>>> specific page encryption status settings before we load a
>>> new kernel by kexec. We cannot reset the complete
>>> page encryption bitmap here as we need to retain the
>>> UEFI/OVMF firmware specific settings.
>>
>> Can the commit message mention why host page encryption needs to be reset ?
>> Since the theme of these patches is guest migration in-SEV context, it might
>> be useful to mention why the host context comes in here.
>>
>>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>>> ---
>>>    arch/x86/kernel/kvm.c | 28 ++++++++++++++++++++++++++++
>>>    1 file changed, 28 insertions(+)
>>>
>>> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
>>> index 8fcee0b45231..ba6cce3c84af 100644
>>> --- a/arch/x86/kernel/kvm.c
>>> +++ b/arch/x86/kernel/kvm.c
>>> @@ -34,6 +34,7 @@
>>>    #include <asm/hypervisor.h>
>>>    #include <asm/tlb.h>
>>>    #include <asm/cpuidle_haltpoll.h>
>>> +#include <asm/e820/api.h>
>>>    static int kvmapf = 1;
>>> @@ -357,6 +358,33 @@ static void kvm_pv_guest_cpu_reboot(void *unused)
>>>    	 */
>>>    	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
>>>    		wrmsrl(MSR_KVM_PV_EOI_EN, 0);
>>> +	/*
>>> +	 * Reset the host's page encryption bitmap related to kernel
>>> +	 * specific page encryption status settings before we load a
>>> +	 * new kernel by kexec. NOTE: We cannot reset the complete
>>> +	 * page encryption bitmap here as we need to retain the
>>> +	 * UEFI/OVMF firmware specific settings.
>>> +	 */
>>> +	if (kvm_para_has_feature(KVM_FEATURE_SEV_LIVE_MIGRATION) &&
>>> +		(smp_processor_id() == 0)) {
>>> +		unsigned long nr_pages;
>>> +		int i;
>>> +
>>> +		for (i = 0; i < e820_table->nr_entries; i++) {
>>> +			struct e820_entry *entry = &e820_table->entries[i];
>>> +			unsigned long start_pfn, end_pfn;
>>> +
>>> +			if (entry->type != E820_TYPE_RAM)
>>> +				continue;
>>> +
>>> +			start_pfn = entry->addr >> PAGE_SHIFT;
>>> +			end_pfn = (entry->addr + entry->size) >> PAGE_SHIFT;
>>> +			nr_pages = DIV_ROUND_UP(entry->size, PAGE_SIZE);
>>> +
>>> +			kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS,
>>> +				entry->addr, nr_pages, 1);
>>> +		}
>>> +	}
>>>    	kvm_pv_disable_apf();
>>>    	kvm_disable_steal_time();
>>>    }

Thanks for the explanation. It will certainly help one understand the 
context better if you add it to the commit message.

Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
diff mbox series

Patch

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 8fcee0b45231..ba6cce3c84af 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -34,6 +34,7 @@ 
 #include <asm/hypervisor.h>
 #include <asm/tlb.h>
 #include <asm/cpuidle_haltpoll.h>
+#include <asm/e820/api.h>
 
 static int kvmapf = 1;
 
@@ -357,6 +358,33 @@  static void kvm_pv_guest_cpu_reboot(void *unused)
 	 */
 	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
 		wrmsrl(MSR_KVM_PV_EOI_EN, 0);
+	/*
+	 * Reset the host's page encryption bitmap related to kernel
+	 * specific page encryption status settings before we load a
+	 * new kernel by kexec. NOTE: We cannot reset the complete
+	 * page encryption bitmap here as we need to retain the
+	 * UEFI/OVMF firmware specific settings.
+	 */
+	if (kvm_para_has_feature(KVM_FEATURE_SEV_LIVE_MIGRATION) &&
+		(smp_processor_id() == 0)) {
+		unsigned long nr_pages;
+		int i;
+
+		for (i = 0; i < e820_table->nr_entries; i++) {
+			struct e820_entry *entry = &e820_table->entries[i];
+			unsigned long start_pfn, end_pfn;
+
+			if (entry->type != E820_TYPE_RAM)
+				continue;
+
+			start_pfn = entry->addr >> PAGE_SHIFT;
+			end_pfn = (entry->addr + entry->size) >> PAGE_SHIFT;
+			nr_pages = DIV_ROUND_UP(entry->size, PAGE_SIZE);
+
+			kvm_sev_hypercall3(KVM_HC_PAGE_ENC_STATUS,
+				entry->addr, nr_pages, 1);
+		}
+	}
 	kvm_pv_disable_apf();
 	kvm_disable_steal_time();
 }