Re: [PATCH v5 27/34] KVM: SVM: Add support for booting APs for an SEV-ES guest
On 04/01/21 18:38, Tom Lendacky wrote: Paolo, is this something along the lines of what you were thinking, or am I off base? I created kvm_emulate_ap_reset_hold() to keep the code consolidated and remove the duplication, but can easily make those changes local to sev.c. I'd also like to rename SVM_VMGEXIT_AP_HLT_LOOP to SVM_VMGEXIT_AP_RESET_HOLD to more closely match the GHBC document, but that can be done later (if possible, since it is already part of the uapi include file). Paolo, a quick ping after the holidays as to whether this is the approach you were thinking. I think there are a couple of places in x86.c to update (vcpu_block() and kvm_arch_vcpu_ioctl_get_mpstate()), also. Yes, this is the basic idea. Paolo
Re: [PATCH v5 27/34] KVM: SVM: Add support for booting APs for an SEV-ES guest
On 12/15/20 2:25 PM, Tom Lendacky wrote:
On 12/14/20 1:46 PM, Tom Lendacky wrote:
On 12/14/20 10:03 AM, Paolo Bonzini wrote:
On 10/12/20 18:10, Tom Lendacky wrote:
From: Tom Lendacky
+case SVM_VMGEXIT_AP_HLT_LOOP:
+svm->ap_hlt_loop = true;
This value needs to be communicated to userspace. Let's get this right
from the beginning and use a new KVM_MP_STATE_* value instead (perhaps
reuse KVM_MP_STATE_STOPPED but for x86 #define it as
KVM_MP_STATE_AP_HOLD_RECEIVED?).
Ok, let me look into this.
Paolo, is this something along the lines of what you were thinking, or am
I off base? I created kvm_emulate_ap_reset_hold() to keep the code
consolidated and remove the duplication, but can easily make those changes
local to sev.c. I'd also like to rename SVM_VMGEXIT_AP_HLT_LOOP to
SVM_VMGEXIT_AP_RESET_HOLD to more closely match the GHBC document, but
that can be done later (if possible, since it is already part of the uapi
include file).
Paolo, a quick ping after the holidays as to whether this is the approach
you were thinking. I think there are a couple of places in x86.c to update
(vcpu_block() and kvm_arch_vcpu_ioctl_get_mpstate()), also.
Thanks,
Tom
Thanks,
Tom
---
KVM: SVM: Add support for booting APs for an SEV-ES guest
From: Tom Lendacky
Typically under KVM, an AP is booted using the INIT-SIPI-SIPI sequence,
where the guest vCPU register state is updated and then the vCPU is VMRUN
to begin execution of the AP. For an SEV-ES guest, this won't work because
the guest register state is encrypted.
Following the GHCB specification, the hypervisor must not alter the guest
register state, so KVM must track an AP/vCPU boot. Should the guest want
to park the AP, it must use the AP Reset Hold exit event in place of, for
example, a HLT loop.
First AP boot (first INIT-SIPI-SIPI sequence):
Execute the AP (vCPU) as it was initialized and measured by the SEV-ES
support. It is up to the guest to transfer control of the AP to the
proper location.
Subsequent AP boot:
KVM will expect to receive an AP Reset Hold exit event indicating that
the vCPU is being parked and will require an INIT-SIPI-SIPI sequence to
awaken it. When the AP Reset Hold exit event is received, KVM will place
the vCPU into a simulated HLT mode. Upon receiving the INIT-SIPI-SIPI
sequence, KVM will make the vCPU runnable. It is again up to the guest
to then transfer control of the AP to the proper location.
To differentiate between an actual HLT and an AP Reset Hold, a new MP
state is introduced, KVM_MP_STATE_AP_RESET_HOLD, which the vCPU is
placed in upon receiving the AP Reset Hold exit event. Additionally, to
communicate the AP Reset Hold exit event up to userspace (if needed), a
new exit reason is introduced, KVM_EXIT_AP_RESET_HOLD.
A new x86 ops function is introduced, vcpu_deliver_sipi_vector, in order
to accomplish AP booting. For VMX, vcpu_deliver_sipi_vector is set to the
original SIPI delivery function, kvm_vcpu_deliver_sipi_vector(). SVM adds
a new function that, for non SEV-ES guests, invokes the original SIPI
delivery function, kvm_vcpu_deliver_sipi_vector(), but for SEV-ES guests,
implements the logic above.
Signed-off-by: Tom Lendacky
---
arch/x86/include/asm/kvm_host.h |3 +++
arch/x86/kvm/lapic.c|2 +-
arch/x86/kvm/svm/sev.c | 22 ++
arch/x86/kvm/svm/svm.c | 10 ++
arch/x86/kvm/svm/svm.h |2 ++
arch/x86/kvm/vmx/vmx.c |2 ++
arch/x86/kvm/x86.c | 20 +---
include/uapi/linux/kvm.h|2 ++
8 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 39707e72b062..23d7b203c060 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1287,6 +1287,8 @@ struct kvm_x86_ops {
void (*migrate_timers)(struct kvm_vcpu *vcpu);
void (*msr_filter_changed)(struct kvm_vcpu *vcpu);
int (*complete_emulated_msr)(struct kvm_vcpu *vcpu, int err);
+
+ void (*vcpu_deliver_sipi_vector)(struct kvm_vcpu *vcpu, u8 vector);
};
struct kvm_x86_nested_ops {
@@ -1468,6 +1470,7 @@ int kvm_fast_pio(struct kvm_vcpu *vcpu, int size,
unsigned short port, int in);
int kvm_emulate_cpuid(struct kvm_vcpu *vcpu);
int kvm_emulate_halt(struct kvm_vcpu *vcpu);
int kvm_vcpu_halt(struct kvm_vcpu *vcpu);
+int kvm_emulate_ap_reset_hold(struct kvm_vcpu *vcpu);
int kvm_emulate_wbinvd(struct kvm_vcpu *vcpu);
void kvm_get_segment(struct kvm_vcpu *vcpu, struct kvm_segment *var, int seg);
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 6a87623aa578..a2f08ed777d8 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2898,7 +2898,7 @@ void kvm_apic_accept_events(struct kvm_vcpu *vcpu)
/* evaluate pending_events before reading the vector */
smp_rmb
Re: [PATCH v5 27/34] KVM: SVM: Add support for booting APs for an SEV-ES guest
On 12/14/20 1:46 PM, Tom Lendacky wrote:
> On 12/14/20 10:03 AM, Paolo Bonzini wrote:
>> On 10/12/20 18:10, Tom Lendacky wrote:
>>> From: Tom Lendacky
>>>
>>> +case SVM_VMGEXIT_AP_HLT_LOOP:
>>> +svm->ap_hlt_loop = true;
>>
>> This value needs to be communicated to userspace. Let's get this right
>> from the beginning and use a new KVM_MP_STATE_* value instead (perhaps
>> reuse KVM_MP_STATE_STOPPED but for x86 #define it as
>> KVM_MP_STATE_AP_HOLD_RECEIVED?).
>
> Ok, let me look into this.
Paolo, is this something along the lines of what you were thinking, or am
I off base? I created kvm_emulate_ap_reset_hold() to keep the code
consolidated and remove the duplication, but can easily make those changes
local to sev.c. I'd also like to rename SVM_VMGEXIT_AP_HLT_LOOP to
SVM_VMGEXIT_AP_RESET_HOLD to more closely match the GHBC document, but
that can be done later (if possible, since it is already part of the uapi
include file).
Thanks,
Tom
---
KVM: SVM: Add support for booting APs for an SEV-ES guest
From: Tom Lendacky
Typically under KVM, an AP is booted using the INIT-SIPI-SIPI sequence,
where the guest vCPU register state is updated and then the vCPU is VMRUN
to begin execution of the AP. For an SEV-ES guest, this won't work because
the guest register state is encrypted.
Following the GHCB specification, the hypervisor must not alter the guest
register state, so KVM must track an AP/vCPU boot. Should the guest want
to park the AP, it must use the AP Reset Hold exit event in place of, for
example, a HLT loop.
First AP boot (first INIT-SIPI-SIPI sequence):
Execute the AP (vCPU) as it was initialized and measured by the SEV-ES
support. It is up to the guest to transfer control of the AP to the
proper location.
Subsequent AP boot:
KVM will expect to receive an AP Reset Hold exit event indicating that
the vCPU is being parked and will require an INIT-SIPI-SIPI sequence to
awaken it. When the AP Reset Hold exit event is received, KVM will place
the vCPU into a simulated HLT mode. Upon receiving the INIT-SIPI-SIPI
sequence, KVM will make the vCPU runnable. It is again up to the guest
to then transfer control of the AP to the proper location.
To differentiate between an actual HLT and an AP Reset Hold, a new MP
state is introduced, KVM_MP_STATE_AP_RESET_HOLD, which the vCPU is
placed in upon receiving the AP Reset Hold exit event. Additionally, to
communicate the AP Reset Hold exit event up to userspace (if needed), a
new exit reason is introduced, KVM_EXIT_AP_RESET_HOLD.
A new x86 ops function is introduced, vcpu_deliver_sipi_vector, in order
to accomplish AP booting. For VMX, vcpu_deliver_sipi_vector is set to the
original SIPI delivery function, kvm_vcpu_deliver_sipi_vector(). SVM adds
a new function that, for non SEV-ES guests, invokes the original SIPI
delivery function, kvm_vcpu_deliver_sipi_vector(), but for SEV-ES guests,
implements the logic above.
Signed-off-by: Tom Lendacky
---
arch/x86/include/asm/kvm_host.h |3 +++
arch/x86/kvm/lapic.c|2 +-
arch/x86/kvm/svm/sev.c | 22 ++
arch/x86/kvm/svm/svm.c | 10 ++
arch/x86/kvm/svm/svm.h |2 ++
arch/x86/kvm/vmx/vmx.c |2 ++
arch/x86/kvm/x86.c | 20 +---
include/uapi/linux/kvm.h|2 ++
8 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 39707e72b062..23d7b203c060 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1287,6 +1287,8 @@ struct kvm_x86_ops {
void (*migrate_timers)(struct kvm_vcpu *vcpu);
void (*msr_filter_changed)(struct kvm_vcpu *vcpu);
int (*complete_emulated_msr)(struct kvm_vcpu *vcpu, int err);
+
+ void (*vcpu_deliver_sipi_vector)(struct kvm_vcpu *vcpu, u8 vector);
};
struct kvm_x86_nested_ops {
@@ -1468,6 +1470,7 @@ int kvm_fast_pio(struct kvm_vcpu *vcpu, int size,
unsigned short port, int in);
int kvm_emulate_cpuid(struct kvm_vcpu *vcpu);
int kvm_emulate_halt(struct kvm_vcpu *vcpu);
int kvm_vcpu_halt(struct kvm_vcpu *vcpu);
+int kvm_emulate_ap_reset_hold(struct kvm_vcpu *vcpu);
int kvm_emulate_wbinvd(struct kvm_vcpu *vcpu);
void kvm_get_segment(struct kvm_vcpu *vcpu, struct kvm_segment *var, int seg);
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 6a87623aa578..a2f08ed777d8 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2898,7 +2898,7 @@ void kvm_apic_accept_events(struct kvm_vcpu *vcpu)
/* evaluate pending_events before reading the vector */
smp_rmb();
sipi_vector = apic->sipi_vector;
- kvm_vcpu_deliver_sipi_vector(vcpu, sipi_vector);
+ kvm_x86_ops.vcpu_deliver_sipi_vector(vcpu, sipi_vector);
vcpu->arch.mp_state = KVM_MP_STATE_
Re: [PATCH v5 27/34] KVM: SVM: Add support for booting APs for an SEV-ES guest
On 12/14/20 10:03 AM, Paolo Bonzini wrote:
> On 10/12/20 18:10, Tom Lendacky wrote:
>> From: Tom Lendacky
>>
>> Typically under KVM, an AP is booted using the INIT-SIPI-SIPI sequence,
>> where the guest vCPU register state is updated and then the vCPU is VMRUN
>> to begin execution of the AP. For an SEV-ES guest, this won't work because
>> the guest register state is encrypted.
>>
>> Following the GHCB specification, the hypervisor must not alter the guest
>> register state, so KVM must track an AP/vCPU boot. Should the guest want
>> to park the AP, it must use the AP Reset Hold exit event in place of, for
>> example, a HLT loop.
>>
>> First AP boot (first INIT-SIPI-SIPI sequence):
>> Execute the AP (vCPU) as it was initialized and measured by the SEV-ES
>> support. It is up to the guest to transfer control of the AP to the
>> proper location.
>>
>> Subsequent AP boot:
>> KVM will expect to receive an AP Reset Hold exit event indicating that
>> the vCPU is being parked and will require an INIT-SIPI-SIPI sequence to
>> awaken it. When the AP Reset Hold exit event is received, KVM will place
>> the vCPU into a simulated HLT mode. Upon receiving the INIT-SIPI-SIPI
>> sequence, KVM will make the vCPU runnable. It is again up to the guest
>> to then transfer control of the AP to the proper location.
>>
>> The GHCB specification also requires the hypervisor to save the address of
>> an AP Jump Table so that, for example, vCPUs that have been parked by UEFI
>> can be started by the OS. Provide support for the AP Jump Table set/get
>> exit code.
>>
>> Signed-off-by: Tom Lendacky
>> ---
>> arch/x86/include/asm/kvm_host.h | 2 ++
>> arch/x86/kvm/svm/sev.c | 50 +
>> arch/x86/kvm/svm/svm.c | 7 +
>> arch/x86/kvm/svm/svm.h | 3 ++
>> arch/x86/kvm/x86.c | 9 ++
>> 5 files changed, 71 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h
>> b/arch/x86/include/asm/kvm_host.h
>> index 048b08437c33..60a3b9d33407 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -1286,6 +1286,8 @@ struct kvm_x86_ops {
>> void (*migrate_timers)(struct kvm_vcpu *vcpu);
>> void (*msr_filter_changed)(struct kvm_vcpu *vcpu);
>> +
>> + void (*vcpu_deliver_sipi_vector)(struct kvm_vcpu *vcpu, u8 vector);
>> };
>> struct kvm_x86_nested_ops {
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index a7531de760b5..b47285384b1f 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -17,6 +17,8 @@
>> #include
>> #include
>> +#include
>> +
>> #include "x86.h"
>> #include "svm.h"
>> #include "cpuid.h"
>> @@ -1449,6 +1451,8 @@ static int sev_es_validate_vmgexit(struct vcpu_svm
>> *svm)
>> if (!ghcb_sw_scratch_is_valid(ghcb))
>> goto vmgexit_err;
>> break;
>> + case SVM_VMGEXIT_AP_HLT_LOOP:
>> + case SVM_VMGEXIT_AP_JUMP_TABLE:
>> case SVM_VMGEXIT_UNSUPPORTED_EVENT:
>> break;
>> default:
>> @@ -1770,6 +1774,35 @@ int sev_handle_vmgexit(struct vcpu_svm *svm)
>> control->exit_info_2,
>> svm->ghcb_sa);
>> break;
>> + case SVM_VMGEXIT_AP_HLT_LOOP:
>> + svm->ap_hlt_loop = true;
>
> This value needs to be communicated to userspace. Let's get this right
> from the beginning and use a new KVM_MP_STATE_* value instead (perhaps
> reuse KVM_MP_STATE_STOPPED but for x86 #define it as
> KVM_MP_STATE_AP_HOLD_RECEIVED?).
Ok, let me look into this.
>
>> @@ -68,6 +68,7 @@ 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 */
>> + u64 ap_jump_table; /* SEV-ES AP Jump Table address */
>
> Do you have any plans for migration of this value? How does the guest
> ensure that the hypervisor does not screw with it?
I'll be sure that this is part of the SEV-ES live migration support.
For SEV-ES, we can't guarantee that the hypervisor doesn't screw with it.
This is something that SEV-SNP will be able to address.
Thanks,
Tom
>
> Paolo
>
Re: [PATCH v5 27/34] KVM: SVM: Add support for booting APs for an SEV-ES guest
On 10/12/20 18:10, Tom Lendacky wrote:
@@ -10144,6 +10144,15 @@ void kvm_vcpu_deliver_sipi_vector(struct kvm_vcpu
*vcpu, u8 vector)
{
struct kvm_segment cs;
+ /*
+* Guests with protected state can't have their state altered by KVM,
+* call the vcpu_deliver_sipi_vector() x86 op for processing.
+*/
+ if (vcpu->arch.guest_state_protected) {
+ kvm_x86_ops.vcpu_deliver_sipi_vector(vcpu, vector);
+ return;
+ }
+
Also, I don't mind that you just call
kvm_x86_ops.vcpu_deliver_sipi_vector from lapic.c, and make VMX just do
.vcpu_deliver_sipi_vector = kvm_vcpu_deliver_sipi_vector,
(SVM would do it if !guest_state_protected). This matches more or less
how I redid the MSR part.
Paolo
Re: [PATCH v5 27/34] KVM: SVM: Add support for booting APs for an SEV-ES guest
On 10/12/20 18:10, Tom Lendacky wrote:
From: Tom Lendacky
Typically under KVM, an AP is booted using the INIT-SIPI-SIPI sequence,
where the guest vCPU register state is updated and then the vCPU is VMRUN
to begin execution of the AP. For an SEV-ES guest, this won't work because
the guest register state is encrypted.
Following the GHCB specification, the hypervisor must not alter the guest
register state, so KVM must track an AP/vCPU boot. Should the guest want
to park the AP, it must use the AP Reset Hold exit event in place of, for
example, a HLT loop.
First AP boot (first INIT-SIPI-SIPI sequence):
Execute the AP (vCPU) as it was initialized and measured by the SEV-ES
support. It is up to the guest to transfer control of the AP to the
proper location.
Subsequent AP boot:
KVM will expect to receive an AP Reset Hold exit event indicating that
the vCPU is being parked and will require an INIT-SIPI-SIPI sequence to
awaken it. When the AP Reset Hold exit event is received, KVM will place
the vCPU into a simulated HLT mode. Upon receiving the INIT-SIPI-SIPI
sequence, KVM will make the vCPU runnable. It is again up to the guest
to then transfer control of the AP to the proper location.
The GHCB specification also requires the hypervisor to save the address of
an AP Jump Table so that, for example, vCPUs that have been parked by UEFI
can be started by the OS. Provide support for the AP Jump Table set/get
exit code.
Signed-off-by: Tom Lendacky
---
arch/x86/include/asm/kvm_host.h | 2 ++
arch/x86/kvm/svm/sev.c | 50 +
arch/x86/kvm/svm/svm.c | 7 +
arch/x86/kvm/svm/svm.h | 3 ++
arch/x86/kvm/x86.c | 9 ++
5 files changed, 71 insertions(+)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 048b08437c33..60a3b9d33407 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1286,6 +1286,8 @@ struct kvm_x86_ops {
void (*migrate_timers)(struct kvm_vcpu *vcpu);
void (*msr_filter_changed)(struct kvm_vcpu *vcpu);
+
+ void (*vcpu_deliver_sipi_vector)(struct kvm_vcpu *vcpu, u8 vector);
};
struct kvm_x86_nested_ops {
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index a7531de760b5..b47285384b1f 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -17,6 +17,8 @@
#include
#include
+#include
+
#include "x86.h"
#include "svm.h"
#include "cpuid.h"
@@ -1449,6 +1451,8 @@ static int sev_es_validate_vmgexit(struct vcpu_svm *svm)
if (!ghcb_sw_scratch_is_valid(ghcb))
goto vmgexit_err;
break;
+ case SVM_VMGEXIT_AP_HLT_LOOP:
+ case SVM_VMGEXIT_AP_JUMP_TABLE:
case SVM_VMGEXIT_UNSUPPORTED_EVENT:
break;
default:
@@ -1770,6 +1774,35 @@ int sev_handle_vmgexit(struct vcpu_svm *svm)
control->exit_info_2,
svm->ghcb_sa);
break;
+ case SVM_VMGEXIT_AP_HLT_LOOP:
+ svm->ap_hlt_loop = true;
This value needs to be communicated to userspace. Let's get this right
from the beginning and use a new KVM_MP_STATE_* value instead (perhaps
reuse KVM_MP_STATE_STOPPED but for x86 #define it as
KVM_MP_STATE_AP_HOLD_RECEIVED?).
@@ -68,6 +68,7 @@ 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 */
+ u64 ap_jump_table; /* SEV-ES AP Jump Table address */
Do you have any plans for migration of this value? How does the guest
ensure that the hypervisor does not screw with it?
Paolo
+ ret = kvm_emulate_halt(&svm->vcpu);
+ break;
+ case SVM_VMGEXIT_AP_JUMP_TABLE: {
+ struct kvm_sev_info *sev = &to_kvm_svm(svm->vcpu.kvm)->sev_info;
+
+ switch (control->exit_info_1) {
+ case 0:
+ /* Set AP jump table address */
+ sev->ap_jump_table = control->exit_info_2;
+ break;
+ case 1:
+ /* Get AP jump table address */
+ ghcb_set_sw_exit_info_2(ghcb, sev->ap_jump_table);
+ break;
+ default:
+ pr_err("svm: vmgexit: unsupported AP jump table request -
exit_info_1=%#llx\n",
+ control->exit_info_1);
+ ghcb_set_sw_exit_info_1(ghcb, 1);
+ ghcb_set_sw_exit_info_2(ghcb,
+ X86_TRAP_UD |
+ SVM_EVTINJ_TYPE_EXEPT |
+ SVM_EVTINJ_VALID);
+ }
+
+
