From: Isaku Yamahata <isaku.yamahata@intel.com>
TDX KVM will use KVM_MEM_ENC_OP. Make struct sev_cmd common both for
vendor backends, SEV and TDX, with rename. Make the struct common uABI for
KVM_MEM_ENC_OP. TDX backend wants to return 64 bit error code instead of
32 bit. To keep ABI for SEV backend, use union to accommodate 64 bit
member. Opportunistically make the implicit padding after id member to
explicit flags member for future use and clarity.
Some data structures for sub-commands could be common. The current
candidate would be KVM_SEV{,_ES}_INIT, KVM_SEV_LAUNCH_FINISH,
KVM_SEV_LAUNCH_UPDATE_VMSA, KVM_SEV_DBG_DECRYPT, and KVM_SEV_DBG_ENCRYPT.
Only compile tested for SEV code.
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
Changes v3 -> v4:
- newly added
---
arch/x86/include/asm/kvm_host.h | 2 +-
arch/x86/include/uapi/asm/kvm.h | 33 ++++++++++++++++
arch/x86/kvm/svm/sev.c | 68 ++++++++++++++++++---------------
arch/x86/kvm/svm/svm.h | 2 +-
arch/x86/kvm/x86.c | 16 +++++++-
5 files changed, 87 insertions(+), 34 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 440a4a13a93f..5ede982442a0 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1710,7 +1710,7 @@ struct kvm_x86_ops {
void (*enable_smi_window)(struct kvm_vcpu *vcpu);
#endif
- int (*mem_enc_ioctl)(struct kvm *kvm, void __user *argp);
+ int (*mem_enc_ioctl)(struct kvm *kvm, struct kvm_mem_enc_cmd *cmd);
int (*mem_enc_register_region)(struct kvm *kvm, struct kvm_enc_region *argp);
int (*mem_enc_unregister_region)(struct kvm *kvm, struct kvm_enc_region *argp);
int (*vm_copy_enc_context_from)(struct kvm *kvm, unsigned int source_fd);
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index aa7a56a47564..32883e520b00 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -562,6 +562,39 @@ struct kvm_pmu_event_filter {
/* x86-specific KVM_EXIT_HYPERCALL flags. */
#define KVM_EXIT_HYPERCALL_LONG_MODE BIT(0)
+struct kvm_mem_enc_cmd {
+ /* sub-command id of KVM_MEM_ENC_OP. */
+ __u32 id;
+ /*
+ * Auxiliary flags for sub-command. If sub-command doesn't use it,
+ * set zero.
+ */
+ __u32 flags;
+ /*
+ * Data for sub-command. An immediate or a pointer to the actual
+ * data in process virtual address. If sub-command doesn't use it,
+ * set zero.
+ */
+ __u64 data;
+ /*
+ * Supplemental error code in the case of error.
+ * SEV error code from the PSP or TDX SEAMCALL status code.
+ * The caller should set zero.
+ */
+ union {
+ struct {
+ __u32 error;
+ /*
+ * KVM_SEV_LAUNCH_START and KVM_SEV_RECEIVE_START
+ * require extra data. Not included in struct
+ * kvm_sev_launch_start or struct kvm_sev_receive_start.
+ */
+ __u32 sev_fd;
+ };
+ __u64 error64;
+ };
+};
+
#define KVM_X86_DEFAULT_VM 0
#define KVM_X86_SW_PROTECTED_VM 1
#define KVM_X86_TDX_VM 2
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 07756b7348ae..94e13bb49c86 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1835,30 +1835,39 @@ int sev_vm_move_enc_context_from(struct kvm *kvm, unsigned int source_fd)
return ret;
}
-int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp)
+int sev_mem_enc_ioctl(struct kvm *kvm, struct kvm_mem_enc_cmd *cmd)
{
- struct kvm_sev_cmd sev_cmd;
+ struct kvm_sev_cmd *sev_cmd = (struct kvm_sev_cmd *)cmd;
int r;
+ /* TODO: replace struct kvm_sev_cmd with kvm_mem_enc_cmd. */
+ BUILD_BUG_ON(sizeof(*sev_cmd) != sizeof(*cmd));
+ BUILD_BUG_ON(offsetof(struct kvm_sev_cmd, id) !=
+ offsetof(struct kvm_mem_enc_cmd, id));
+ BUILD_BUG_ON(sizeof(sev_cmd->id) != sizeof(cmd->id));
+ BUILD_BUG_ON(offsetof(struct kvm_sev_cmd, data) !=
+ offsetof(struct kvm_mem_enc_cmd, data));
+ BUILD_BUG_ON(sizeof(sev_cmd->data) != sizeof(cmd->data));
+ BUILD_BUG_ON(offsetof(struct kvm_sev_cmd, error) !=
+ offsetof(struct kvm_mem_enc_cmd, error));
+ BUILD_BUG_ON(sizeof(sev_cmd->error) != sizeof(cmd->error));
+ BUILD_BUG_ON(offsetof(struct kvm_sev_cmd, sev_fd) !=
+ offsetof(struct kvm_mem_enc_cmd, sev_fd));
+ BUILD_BUG_ON(sizeof(sev_cmd->sev_fd) != sizeof(cmd->sev_fd));
+
if (!sev_enabled)
return -ENOTTY;
- if (!argp)
- return 0;
-
- if (copy_from_user(&sev_cmd, argp, sizeof(struct kvm_sev_cmd)))
- return -EFAULT;
-
mutex_lock(&kvm->lock);
/* Only the enc_context_owner handles some memory enc operations. */
if (is_mirroring_enc_context(kvm) &&
- !is_cmd_allowed_from_mirror(sev_cmd.id)) {
+ !is_cmd_allowed_from_mirror(sev_cmd->id)) {
r = -EINVAL;
goto out;
}
- switch (sev_cmd.id) {
+ switch (sev_cmd->id) {
case KVM_SEV_ES_INIT:
if (!sev_es_enabled) {
r = -ENOTTY;
@@ -1866,67 +1875,64 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp)
}
fallthrough;
case KVM_SEV_INIT:
- r = sev_guest_init(kvm, &sev_cmd);
+ r = sev_guest_init(kvm, sev_cmd);
break;
case KVM_SEV_LAUNCH_START:
- r = sev_launch_start(kvm, &sev_cmd);
+ r = sev_launch_start(kvm, sev_cmd);
break;
case KVM_SEV_LAUNCH_UPDATE_DATA:
- r = sev_launch_update_data(kvm, &sev_cmd);
+ r = sev_launch_update_data(kvm, sev_cmd);
break;
case KVM_SEV_LAUNCH_UPDATE_VMSA:
- r = sev_launch_update_vmsa(kvm, &sev_cmd);
+ r = sev_launch_update_vmsa(kvm, sev_cmd);
break;
case KVM_SEV_LAUNCH_MEASURE:
- r = sev_launch_measure(kvm, &sev_cmd);
+ r = sev_launch_measure(kvm, sev_cmd);
break;
case KVM_SEV_LAUNCH_FINISH:
- r = sev_launch_finish(kvm, &sev_cmd);
+ r = sev_launch_finish(kvm, sev_cmd);
break;
case KVM_SEV_GUEST_STATUS:
- r = sev_guest_status(kvm, &sev_cmd);
+ r = sev_guest_status(kvm, sev_cmd);
break;
case KVM_SEV_DBG_DECRYPT:
- r = sev_dbg_crypt(kvm, &sev_cmd, true);
+ r = sev_dbg_crypt(kvm, sev_cmd, true);
break;
case KVM_SEV_DBG_ENCRYPT:
- r = sev_dbg_crypt(kvm, &sev_cmd, false);
+ r = sev_dbg_crypt(kvm, sev_cmd, false);
break;
case KVM_SEV_LAUNCH_SECRET:
- r = sev_launch_secret(kvm, &sev_cmd);
+ r = sev_launch_secret(kvm, sev_cmd);
break;
case KVM_SEV_GET_ATTESTATION_REPORT:
- r = sev_get_attestation_report(kvm, &sev_cmd);
+ r = sev_get_attestation_report(kvm, sev_cmd);
break;
case KVM_SEV_SEND_START:
- r = sev_send_start(kvm, &sev_cmd);
+ r = sev_send_start(kvm, sev_cmd);
break;
case KVM_SEV_SEND_UPDATE_DATA:
- r = sev_send_update_data(kvm, &sev_cmd);
+ r = sev_send_update_data(kvm, sev_cmd);
break;
case KVM_SEV_SEND_FINISH:
- r = sev_send_finish(kvm, &sev_cmd);
+ r = sev_send_finish(kvm, sev_cmd);
break;
case KVM_SEV_SEND_CANCEL:
- r = sev_send_cancel(kvm, &sev_cmd);
+ r = sev_send_cancel(kvm, sev_cmd);
break;
case KVM_SEV_RECEIVE_START:
- r = sev_receive_start(kvm, &sev_cmd);
+ r = sev_receive_start(kvm, sev_cmd);
break;
case KVM_SEV_RECEIVE_UPDATE_DATA:
- r = sev_receive_update_data(kvm, &sev_cmd);
+ r = sev_receive_update_data(kvm, sev_cmd);
break;
case KVM_SEV_RECEIVE_FINISH:
- r = sev_receive_finish(kvm, &sev_cmd);
+ r = sev_receive_finish(kvm, sev_cmd);
break;
default:
r = -EINVAL;
goto out;
}
- if (copy_to_user(argp, &sev_cmd, sizeof(struct kvm_sev_cmd)))
- r = -EFAULT;
-
out:
mutex_unlock(&kvm->lock);
return r;
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 18af7e712a5a..74ecab20c24b 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -716,7 +716,7 @@ void avic_refresh_virtual_apic_mode(struct kvm_vcpu *vcpu);
extern unsigned int max_sev_asid;
void sev_vm_destroy(struct kvm *kvm);
-int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp);
+int sev_mem_enc_ioctl(struct kvm *kvm, struct kvm_mem_enc_cmd *cmd);
int sev_mem_enc_register_region(struct kvm *kvm,
struct kvm_enc_region *range);
int sev_mem_enc_unregister_region(struct kvm *kvm,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2ae40fa8e178..ab36e8940f1b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7040,11 +7040,25 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
goto out;
}
case KVM_MEMORY_ENCRYPT_OP: {
+ struct kvm_mem_enc_cmd cmd;
+
r = -ENOTTY;
if (!kvm_x86_ops.mem_enc_ioctl)
goto out;
- r = static_call(kvm_x86_mem_enc_ioctl)(kvm, argp);
+ if (!argp) {
+ r = 0;
+ goto out;
+ }
+
+ if (copy_from_user(&cmd, argp, sizeof(cmd))) {
+ r = -EFAULT;
+ goto out;
+ }
+ r = static_call(kvm_x86_mem_enc_ioctl)(kvm, &cmd);
+ if (copy_to_user(argp, &cmd, sizeof(cmd)))
+ r = -EFAULT;
+
break;
}
case KVM_MEMORY_ENCRYPT_REG_REGION: {
--
2.25.1On Thu, Jul 20, 2023, isaku.yamahata@intel.com wrote:
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index aa7a56a47564..32883e520b00 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -562,6 +562,39 @@ struct kvm_pmu_event_filter {
> /* x86-specific KVM_EXIT_HYPERCALL flags. */
> #define KVM_EXIT_HYPERCALL_LONG_MODE BIT(0)
>
> +struct kvm_mem_enc_cmd {
> + /* sub-command id of KVM_MEM_ENC_OP. */
> + __u32 id;
> + /*
> + * Auxiliary flags for sub-command. If sub-command doesn't use it,
> + * set zero.
> + */
> + __u32 flags;
> + /*
> + * Data for sub-command. An immediate or a pointer to the actual
> + * data in process virtual address. If sub-command doesn't use it,
> + * set zero.
> + */
> + __u64 data;
> + /*
> + * Supplemental error code in the case of error.
> + * SEV error code from the PSP or TDX SEAMCALL status code.
> + * The caller should set zero.
> + */
> + union {
> + struct {
> + __u32 error;
> + /*
> + * KVM_SEV_LAUNCH_START and KVM_SEV_RECEIVE_START
> + * require extra data. Not included in struct
> + * kvm_sev_launch_start or struct kvm_sev_receive_start.
> + */
> + __u32 sev_fd;
> + };
> + __u64 error64;
> + };
> +};
Eww. Why not just use an entirely different struct for TDX? I don't see what
benefit this provides other than a warm fuzzy feeling that TDX and SEV share a
struct. Practically speaking, KVM will likely take on more work to forcefully
smush the two together than if they're separate things.
On 7/21/2023 10:51 PM, Sean Christopherson wrote:
> On Thu, Jul 20, 2023, isaku.yamahata@intel.com wrote:
>> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
>> index aa7a56a47564..32883e520b00 100644
>> --- a/arch/x86/include/uapi/asm/kvm.h
>> +++ b/arch/x86/include/uapi/asm/kvm.h
>> @@ -562,6 +562,39 @@ struct kvm_pmu_event_filter {
>> /* x86-specific KVM_EXIT_HYPERCALL flags. */
>> #define KVM_EXIT_HYPERCALL_LONG_MODE BIT(0)
>>
>> +struct kvm_mem_enc_cmd {
>> + /* sub-command id of KVM_MEM_ENC_OP. */
>> + __u32 id;
>> + /*
>> + * Auxiliary flags for sub-command. If sub-command doesn't use it,
>> + * set zero.
>> + */
>> + __u32 flags;
>> + /*
>> + * Data for sub-command. An immediate or a pointer to the actual
>> + * data in process virtual address. If sub-command doesn't use it,
>> + * set zero.
>> + */
>> + __u64 data;
>> + /*
>> + * Supplemental error code in the case of error.
>> + * SEV error code from the PSP or TDX SEAMCALL status code.
>> + * The caller should set zero.
>> + */
>> + union {
>> + struct {
>> + __u32 error;
>> + /*
>> + * KVM_SEV_LAUNCH_START and KVM_SEV_RECEIVE_START
>> + * require extra data. Not included in struct
>> + * kvm_sev_launch_start or struct kvm_sev_receive_start.
>> + */
>> + __u32 sev_fd;
>> + };
>> + __u64 error64;
>> + };
>> +};
>
> Eww. Why not just use an entirely different struct for TDX? I don't see what
> benefit this provides other than a warm fuzzy feeling that TDX and SEV share a
> struct. Practically speaking, KVM will likely take on more work to forcefully
> smush the two together than if they're separate things.
generalizing the struct of KVM_MEM_ENC_OP should be the first step. The
final target should be generalizing a set of commands for confidential
VMs (SEV-* VMs and TDs, maybe even for other arches), e.g., the commands
to create a confidential VM and commands to live migration a
confidential VM.
However, there seems not small divergence between the commands to create
a SEV-* VM and TDX VMs. I'm not sure if it is worth investigating and
pursuing.
On Tue, Jul 25, 2023, Xiaoyao Li wrote:
> On 7/21/2023 10:51 PM, Sean Christopherson wrote:
> > On Thu, Jul 20, 2023, isaku.yamahata@intel.com wrote:
> > > diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> > > index aa7a56a47564..32883e520b00 100644
> > > --- a/arch/x86/include/uapi/asm/kvm.h
> > > +++ b/arch/x86/include/uapi/asm/kvm.h
> > > @@ -562,6 +562,39 @@ struct kvm_pmu_event_filter {
> > > /* x86-specific KVM_EXIT_HYPERCALL flags. */
> > > #define KVM_EXIT_HYPERCALL_LONG_MODE BIT(0)
> > > +struct kvm_mem_enc_cmd {
> > > + /* sub-command id of KVM_MEM_ENC_OP. */
> > > + __u32 id;
> > > + /*
> > > + * Auxiliary flags for sub-command. If sub-command doesn't use it,
> > > + * set zero.
> > > + */
> > > + __u32 flags;
> > > + /*
> > > + * Data for sub-command. An immediate or a pointer to the actual
> > > + * data in process virtual address. If sub-command doesn't use it,
> > > + * set zero.
> > > + */
> > > + __u64 data;
> > > + /*
> > > + * Supplemental error code in the case of error.
> > > + * SEV error code from the PSP or TDX SEAMCALL status code.
> > > + * The caller should set zero.
> > > + */
> > > + union {
> > > + struct {
> > > + __u32 error;
> > > + /*
> > > + * KVM_SEV_LAUNCH_START and KVM_SEV_RECEIVE_START
> > > + * require extra data. Not included in struct
> > > + * kvm_sev_launch_start or struct kvm_sev_receive_start.
> > > + */
> > > + __u32 sev_fd;
> > > + };
> > > + __u64 error64;
> > > + };
> > > +};
> >
> > Eww. Why not just use an entirely different struct for TDX? I don't see what
> > benefit this provides other than a warm fuzzy feeling that TDX and SEV share a
> > struct. Practically speaking, KVM will likely take on more work to forcefully
> > smush the two together than if they're separate things.
>
> generalizing the struct of KVM_MEM_ENC_OP should be the first step.
It's not just the one structure though. The "data" field is a pointer to yet
another layer of commands, and SEV has a rather big pile of those. Making
kvm_mem_enc_cmd common is just putting lipstick on a pig since the vast majority
of the structures associated with the ioctl() would still be vendor specific.
struct kvm_sev_launch_start
struct kvm_sev_launch_update_data
struct kvm_sev_launch_secret
struct kvm_sev_launch_measure
struct kvm_sev_guest_status
struct kvm_sev_dbg
struct kvm_sev_attestation_report
struct kvm_sev_send_start
struct kvm_sev_send_update_data
struct kvm_sev_receive_start
struct kvm_sev_receive_update_data
FWIW, I really dislike KVM's uAPI for KVM_MEM_ENC_OP. The above structures are
all basically copied verbatim from PSP firmware structures, i.e. the commands and
their payloads are tightly coupled to "hardware" and essentially have no abstraction
whatsoever. But that ship has already sailed, and practically speaking trying to
provide a layer of abstraction might not of worked very well anyways.
In other words, unless there's an obvious and easy way path to convergence, I
recommend you don't spend much time/effort on trying to share code with TDX.
On Tue, Jul 25, 2023 at 08:36:09AM -0700,
Sean Christopherson <seanjc@google.com> wrote:
> On Tue, Jul 25, 2023, Xiaoyao Li wrote:
> > On 7/21/2023 10:51 PM, Sean Christopherson wrote:
> > > On Thu, Jul 20, 2023, isaku.yamahata@intel.com wrote:
> > > > diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> > > > index aa7a56a47564..32883e520b00 100644
> > > > --- a/arch/x86/include/uapi/asm/kvm.h
> > > > +++ b/arch/x86/include/uapi/asm/kvm.h
> > > > @@ -562,6 +562,39 @@ struct kvm_pmu_event_filter {
> > > > /* x86-specific KVM_EXIT_HYPERCALL flags. */
> > > > #define KVM_EXIT_HYPERCALL_LONG_MODE BIT(0)
> > > > +struct kvm_mem_enc_cmd {
> > > > + /* sub-command id of KVM_MEM_ENC_OP. */
> > > > + __u32 id;
> > > > + /*
> > > > + * Auxiliary flags for sub-command. If sub-command doesn't use it,
> > > > + * set zero.
> > > > + */
> > > > + __u32 flags;
> > > > + /*
> > > > + * Data for sub-command. An immediate or a pointer to the actual
> > > > + * data in process virtual address. If sub-command doesn't use it,
> > > > + * set zero.
> > > > + */
> > > > + __u64 data;
> > > > + /*
> > > > + * Supplemental error code in the case of error.
> > > > + * SEV error code from the PSP or TDX SEAMCALL status code.
> > > > + * The caller should set zero.
> > > > + */
> > > > + union {
> > > > + struct {
> > > > + __u32 error;
> > > > + /*
> > > > + * KVM_SEV_LAUNCH_START and KVM_SEV_RECEIVE_START
> > > > + * require extra data. Not included in struct
> > > > + * kvm_sev_launch_start or struct kvm_sev_receive_start.
> > > > + */
> > > > + __u32 sev_fd;
> > > > + };
> > > > + __u64 error64;
> > > > + };
> > > > +};
> > >
> > > Eww. Why not just use an entirely different struct for TDX? I don't see what
> > > benefit this provides other than a warm fuzzy feeling that TDX and SEV share a
> > > struct. Practically speaking, KVM will likely take on more work to forcefully
> > > smush the two together than if they're separate things.
> >
> > generalizing the struct of KVM_MEM_ENC_OP should be the first step.
>
> It's not just the one structure though. The "data" field is a pointer to yet
> another layer of commands, and SEV has a rather big pile of those. Making
> kvm_mem_enc_cmd common is just putting lipstick on a pig since the vast majority
> of the structures associated with the ioctl() would still be vendor specific.
> struct kvm_sev_launch_start
> struct kvm_sev_launch_update_data
> struct kvm_sev_launch_secret
> struct kvm_sev_launch_measure
> struct kvm_sev_guest_status
> struct kvm_sev_dbg
> struct kvm_sev_attestation_report
> struct kvm_sev_send_start
> struct kvm_sev_send_update_data
> struct kvm_sev_receive_start
> struct kvm_sev_receive_update_data
>
> FWIW, I really dislike KVM's uAPI for KVM_MEM_ENC_OP. The above structures are
> all basically copied verbatim from PSP firmware structures, i.e. the commands and
> their payloads are tightly coupled to "hardware" and essentially have no abstraction
> whatsoever. But that ship has already sailed, and practically speaking trying to
> provide a layer of abstraction might not of worked very well anyways.
>
> In other words, unless there's an obvious and easy way path to convergence, I
> recommend you don't spend much time/effort on trying to share code with TDX.
I think we can easily unify vcpu initialization, populating/measure initial
memory, completing guest creation, and guest memory access for debug.
KVM_SEV_LAUNCH_UPDATE_VMSA <-> KVM_TDX_INIT_VCPU
KVM_SEV_LAUNCH_UPDATE_DATA and KVM_SEV_LAUNCH_MEASURE <-> KVM_INIT_MEM_REGION
KVM_SEV_LAUNCH_FINISH <-> KVM_TDX_FINALIZE_VM
KVM_SEV_DBG_DECRYPT, KVM_SEV_DBG_ENCRYPT: KVM common API for access protected guest memory
Here's my assessment. For now I don't address migration.
For creating confidential guest:
- Get the capability of underlying platform
KVM_TDX_CAPABILITY: no sev correspondence.
- Initialize VM as confidential VM
struct kvm_sev_launch_start
KVM_SEV{,_ES}_INIT, and KVM_SEV_LAUNCH_START:
KVM_TDX_INIT_VM
They take vendor specific data.
- Initialize vcpu
KVM_SEV_LAUNCH_UPDATE_VMSA: no extra argument
KVM_TDX_INIT_VCPU: no extra argument
- populate initial memory + measurement
KVM_SEV_LAUNCH_UPDATE_DATA and KVM_SEV_LAUNCH_MEASURE,
struct kvm_sev_launch_update_data {
__u64 uaddr;
__u32 len;
};
struct kvm_sev_launch_measure {
__u64 uaddr;
__u32 len;
};
=> GPA is calculated from uaddr.
KVM_INIT_MEM_REGION:
struct kvm_tdx_init_mem_region {
__u64 source_addr; // uaddr
__u64 gpa;
__u64 nr_pages;
};
I think those can same structure. Or prefault or prepopulating
e.g.
struct {
__u64 uaddr;
__u64 gpa;
__u64 len;
#define FLAG_MEASURE BIT(0)
#define FLAG_GPA BIT(1) // GPA is valid or calculated from uaddr
__u64 flags;
};
- Complete initialization. Make the guest ready to run vcpu
KVM_SEV_LAUNCH_FINISH: no argument
KVM_TDX_FINALIZE_VM: no argument
- KVM_SEV_LAUNCH_SECRET: no TDX correspondence
struct kvm_sev_launch_secret
For guest debug
- KVM_SEV_DBG_DECRYPT, KVM_SEV_DBG_ENCRYPT: struct kvm_sev_dbg
This is to read/write guest memory for debug. We can easily have a common
API.
- KVM_SEV_GUEST_STATUS
struct kvm_sev_guest_status
No TDX correspondence
Thanks,
--
Isaku Yamahata <isaku.yamahata@gmail.com>
On Fri, Jul 21, 2023 at 07:51:04AM -0700,
Sean Christopherson <seanjc@google.com> wrote:
> On Thu, Jul 20, 2023, isaku.yamahata@intel.com wrote:
> > diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> > index aa7a56a47564..32883e520b00 100644
> > --- a/arch/x86/include/uapi/asm/kvm.h
> > +++ b/arch/x86/include/uapi/asm/kvm.h
> > @@ -562,6 +562,39 @@ struct kvm_pmu_event_filter {
> > /* x86-specific KVM_EXIT_HYPERCALL flags. */
> > #define KVM_EXIT_HYPERCALL_LONG_MODE BIT(0)
> >
> > +struct kvm_mem_enc_cmd {
> > + /* sub-command id of KVM_MEM_ENC_OP. */
> > + __u32 id;
> > + /*
> > + * Auxiliary flags for sub-command. If sub-command doesn't use it,
> > + * set zero.
> > + */
> > + __u32 flags;
> > + /*
> > + * Data for sub-command. An immediate or a pointer to the actual
> > + * data in process virtual address. If sub-command doesn't use it,
> > + * set zero.
> > + */
> > + __u64 data;
> > + /*
> > + * Supplemental error code in the case of error.
> > + * SEV error code from the PSP or TDX SEAMCALL status code.
> > + * The caller should set zero.
> > + */
> > + union {
> > + struct {
> > + __u32 error;
> > + /*
> > + * KVM_SEV_LAUNCH_START and KVM_SEV_RECEIVE_START
> > + * require extra data. Not included in struct
> > + * kvm_sev_launch_start or struct kvm_sev_receive_start.
> > + */
> > + __u32 sev_fd;
> > + };
> > + __u64 error64;
> > + };
> > +};
>
> Eww. Why not just use an entirely different struct for TDX? I don't see what
> benefit this provides other than a warm fuzzy feeling that TDX and SEV share a
> struct. Practically speaking, KVM will likely take on more work to forcefully
> smush the two together than if they're separate things.
Ok, let's drop this patch. Keep the ABI different for now.
--
Isaku Yamahata <isaku.yamahata@gmail.com>
© 2016 - 2025 Red Hat, Inc.