[PATCH 0/3] TDX: Fix and enhancement

[PATCH 0/3] TDX: Fix and enhancement

[Xiaoyao Li]

Patch 1 and patch 3 are the left review feedback of TDX QEMU v9 series.

Patch 2 is caught during preparing patch 3. 

Xiaoyao Li (3):
  i386/cpu: Rename enable_cpuid_0x1f to force_cpuid_0x1f
  i386/tdx: Fix the typo of the comment of struct TdxGuest
  i386/tdx: Clarify the error message of
    mrconfigid/mrowner/mrownerconfig

 target/i386/cpu.h     |  4 ++--
 target/i386/kvm/tdx.c | 14 ++++++++++----
 target/i386/kvm/tdx.h |  6 +++---
 3 files changed, 15 insertions(+), 9 deletions(-)


base-commit: 6322b753f798337835e205b6d805356bea582c86
-- 
2.43.0

[PATCH 1/3] i386/cpu: Rename enable_cpuid_0x1f to force_cpuid_0x1f

[Xiaoyao Li]

The name of "enable_cpuid_0x1f" isn't right to its behavior because the
leaf 0x1f can be enabled even when "enable_cpuid_0x1f" is false.

Rename it to "force_cpuid_0x1f" to better reflect its behavior.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
Suggested by Igor at https://lore.kernel.org/qemu-devel/20250513144515.37615651@imammedo.users.ipa.redhat.com/
---
 target/i386/cpu.h     | 4 ++--
 target/i386/kvm/tdx.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 1146465c8c62..aaef6a2a6767 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2270,7 +2270,7 @@ struct ArchCPU {
     bool enable_cpuid_0xb;
 
     /* Force to enable cpuid 0x1f */
-    bool enable_cpuid_0x1f;
+    bool force_cpuid_0x1f;
 
     /* Enable auto level-increase for all CPUID leaves */
     bool full_cpuid_auto_level;
@@ -2540,7 +2540,7 @@ void mark_forced_on_features(X86CPU *cpu, FeatureWord w, uint64_t mask,
 
 static inline bool x86_has_cpuid_0x1f(X86CPU *cpu)
 {
-    return cpu->enable_cpuid_0x1f ||
+    return cpu->force_cpuid_0x1f ||
            x86_has_extended_topo(cpu->env.avail_cpu_topo);
 }
 
diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
index 0a21ae555c5c..17e671f1710f 100644
--- a/target/i386/kvm/tdx.c
+++ b/target/i386/kvm/tdx.c
@@ -746,7 +746,7 @@ static void tdx_cpu_instance_init(X86ConfidentialGuest *cg, CPUState *cpu)
     /* invtsc is fixed1 for TD guest */
     object_property_set_bool(OBJECT(cpu), "invtsc", true, &error_abort);
 
-    x86cpu->enable_cpuid_0x1f = true;
+    x86cpu->force_cpuid_0x1f = true;
 }
 
 static uint32_t tdx_adjust_cpuid_features(X86ConfidentialGuest *cg,
-- 
2.43.0

[PATCH 2/3] i386/tdx: Fix the typo of the comment of struct TdxGuest

[Xiaoyao Li]

Change sha348 to sha384.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 target/i386/kvm/tdx.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/i386/kvm/tdx.h b/target/i386/kvm/tdx.h
index 04b5afe199f9..8dd66e90149b 100644
--- a/target/i386/kvm/tdx.h
+++ b/target/i386/kvm/tdx.h
@@ -40,9 +40,9 @@ typedef struct TdxGuest {
     bool initialized;
     uint64_t attributes;    /* TD attributes */
     uint64_t xfam;
-    char *mrconfigid;       /* base64 encoded sha348 digest */
-    char *mrowner;          /* base64 encoded sha348 digest */
-    char *mrownerconfig;    /* base64 encoded sha348 digest */
+    char *mrconfigid;       /* base64 encoded sha384 digest */
+    char *mrowner;          /* base64 encoded sha384 digest */
+    char *mrownerconfig;    /* base64 encoded sha384 digest */
 
     MemoryRegion *tdvf_mr;
     TdxFirmware tdvf;
-- 
2.43.0

[PATCH 3/3] i386/tdx: Clarify the error message of mrconfigid/mrowner/mrownerconfig

[Xiaoyao Li]

The error message is misleading - we successfully decoded the data,
the decoded data was simply with the wrong length.

Change the error message to show it is an length check failure with both
the received and expected values.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
Suggested by Daniel at https://lore.kernel.org/qemu-devel/aBzT3TrdldaN-uqx@redhat.com/
---
 target/i386/kvm/tdx.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
index 17e671f1710f..4172bbd23340 100644
--- a/target/i386/kvm/tdx.c
+++ b/target/i386/kvm/tdx.c
@@ -1026,7 +1026,9 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
             return -1;
         }
         if (data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
-            error_setg(errp, "TDX: failed to decode mrconfigid");
+            error_setg(errp, "TDX 'mrconfigid' sha384 digest was %ld bytes, "
+                             "expected %d bytes", data_len,
+                             QCRYPTO_HASH_DIGEST_LEN_SHA384);
             return -1;
         }
         memcpy(init_vm->mrconfigid, data, data_len);
@@ -1039,7 +1041,9 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
             return -1;
         }
         if (data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
-            error_setg(errp, "TDX: failed to decode mrowner");
+            error_setg(errp, "TDX 'mrowner' sha384 digest was %ld bytes, "
+                             "expected %d bytes", data_len,
+                             QCRYPTO_HASH_DIGEST_LEN_SHA384);
             return -1;
         }
         memcpy(init_vm->mrowner, data, data_len);
@@ -1052,7 +1056,9 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
             return -1;
         }
         if (data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
-            error_setg(errp, "TDX: failed to decode mrownerconfig");
+            error_setg(errp, "TDX 'mrownerconfig' sha384 digest was %ld bytes, "
+                             "expected %d bytes", data_len,
+                             QCRYPTO_HASH_DIGEST_LEN_SHA384);
             return -1;
         }
         memcpy(init_vm->mrownerconfig, data, data_len);
-- 
2.43.0

Re: [PATCH 3/3] i386/tdx: Clarify the error message of mrconfigid/mrowner/mrownerconfig

[Daniel P. Berrangé]

On Tue, Jun 03, 2025 at 01:03:05AM -0400, Xiaoyao Li wrote:
> The error message is misleading - we successfully decoded the data,
> the decoded data was simply with the wrong length.
> 
> Change the error message to show it is an length check failure with both
> the received and expected values.
> 
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> Suggested by Daniel at https://lore.kernel.org/qemu-devel/aBzT3TrdldaN-uqx@redhat.com/
> ---
>  target/i386/kvm/tdx.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 2/3] i386/tdx: Fix the typo of the comment of struct TdxGuest

[Daniel P. Berrangé]

On Tue, Jun 03, 2025 at 01:03:04AM -0400, Xiaoyao Li wrote:
> Change sha348 to sha384.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>  target/i386/kvm/tdx.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 1/3] i386/cpu: Rename enable_cpuid_0x1f to force_cpuid_0x1f

[Daniel P. Berrangé]

On Tue, Jun 03, 2025 at 01:03:03AM -0400, Xiaoyao Li wrote:
> The name of "enable_cpuid_0x1f" isn't right to its behavior because the
> leaf 0x1f can be enabled even when "enable_cpuid_0x1f" is false.
> 
> Rename it to "force_cpuid_0x1f" to better reflect its behavior.
> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> Suggested by Igor at https://lore.kernel.org/qemu-devel/20250513144515.37615651@imammedo.users.ipa.redhat.com/
> ---
>  target/i386/cpu.h     | 4 ++--
>  target/i386/kvm/tdx.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 1/3] i386/cpu: Rename enable_cpuid_0x1f to force_cpuid_0x1f

[Igor Mammedov]

On Tue,  3 Jun 2025 01:03:03 -0400
Xiaoyao Li <xiaoyao.li@intel.com> wrote:

> The name of "enable_cpuid_0x1f" isn't right to its behavior because the
> leaf 0x1f can be enabled even when "enable_cpuid_0x1f" is false.
> 
> Rename it to "force_cpuid_0x1f" to better reflect its behavior.
> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> Suggested by Igor at https://lore.kernel.org/qemu-devel/20250513144515.37615651@imammedo.users.ipa.redhat.com/
> ---
>  target/i386/cpu.h     | 4 ++--
>  target/i386/kvm/tdx.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 1146465c8c62..aaef6a2a6767 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -2270,7 +2270,7 @@ struct ArchCPU {
>      bool enable_cpuid_0xb;
>  
>      /* Force to enable cpuid 0x1f */
> -    bool enable_cpuid_0x1f;
> +    bool force_cpuid_0x1f;
>  
>      /* Enable auto level-increase for all CPUID leaves */
>      bool full_cpuid_auto_level;
> @@ -2540,7 +2540,7 @@ void mark_forced_on_features(X86CPU *cpu, FeatureWord w, uint64_t mask,
>  
>  static inline bool x86_has_cpuid_0x1f(X86CPU *cpu)
>  {
> -    return cpu->enable_cpuid_0x1f ||
> +    return cpu->force_cpuid_0x1f ||
>             x86_has_extended_topo(cpu->env.avail_cpu_topo);
>  }
>  
> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> index 0a21ae555c5c..17e671f1710f 100644
> --- a/target/i386/kvm/tdx.c
> +++ b/target/i386/kvm/tdx.c
> @@ -746,7 +746,7 @@ static void tdx_cpu_instance_init(X86ConfidentialGuest *cg, CPUState *cpu)
>      /* invtsc is fixed1 for TD guest */
>      object_property_set_bool(OBJECT(cpu), "invtsc", true, &error_abort);
>  
> -    x86cpu->enable_cpuid_0x1f = true;
> +    x86cpu->force_cpuid_0x1f = true;
>  }
>  
>  static uint32_t tdx_adjust_cpuid_features(X86ConfidentialGuest *cg,

Re: [PATCH 2/3] i386/tdx: Fix the typo of the comment of struct TdxGuest

[Igor Mammedov]

On Tue,  3 Jun 2025 01:03:04 -0400
Xiaoyao Li <xiaoyao.li@intel.com> wrote:

> Change sha348 to sha384.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  target/i386/kvm/tdx.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/i386/kvm/tdx.h b/target/i386/kvm/tdx.h
> index 04b5afe199f9..8dd66e90149b 100644
> --- a/target/i386/kvm/tdx.h
> +++ b/target/i386/kvm/tdx.h
> @@ -40,9 +40,9 @@ typedef struct TdxGuest {
>      bool initialized;
>      uint64_t attributes;    /* TD attributes */
>      uint64_t xfam;
> -    char *mrconfigid;       /* base64 encoded sha348 digest */
> -    char *mrowner;          /* base64 encoded sha348 digest */
> -    char *mrownerconfig;    /* base64 encoded sha348 digest */
> +    char *mrconfigid;       /* base64 encoded sha384 digest */
> +    char *mrowner;          /* base64 encoded sha384 digest */
> +    char *mrownerconfig;    /* base64 encoded sha384 digest */
>  
>      MemoryRegion *tdvf_mr;
>      TdxFirmware tdvf;

Re: [PATCH 3/3] i386/tdx: Clarify the error message of mrconfigid/mrowner/mrownerconfig

[Igor Mammedov]

On Tue,  3 Jun 2025 01:03:05 -0400
Xiaoyao Li <xiaoyao.li@intel.com> wrote:

> The error message is misleading - we successfully decoded the data,
> the decoded data was simply with the wrong length.
> 
> Change the error message to show it is an length check failure with both
> the received and expected values.
> 
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> Suggested by Daniel at https://lore.kernel.org/qemu-devel/aBzT3TrdldaN-uqx@redhat.com/
> ---
>  target/i386/kvm/tdx.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> index 17e671f1710f..4172bbd23340 100644
> --- a/target/i386/kvm/tdx.c
> +++ b/target/i386/kvm/tdx.c
> @@ -1026,7 +1026,9 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
>              return -1;
>          }
>          if (data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
> -            error_setg(errp, "TDX: failed to decode mrconfigid");
> +            error_setg(errp, "TDX 'mrconfigid' sha384 digest was %ld bytes, "
> +                             "expected %d bytes", data_len,
> +                             QCRYPTO_HASH_DIGEST_LEN_SHA384);
>              return -1;
>          }
>          memcpy(init_vm->mrconfigid, data, data_len);
> @@ -1039,7 +1041,9 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
>              return -1;
>          }
>          if (data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
> -            error_setg(errp, "TDX: failed to decode mrowner");
> +            error_setg(errp, "TDX 'mrowner' sha384 digest was %ld bytes, "
> +                             "expected %d bytes", data_len,
> +                             QCRYPTO_HASH_DIGEST_LEN_SHA384);
>              return -1;
>          }
>          memcpy(init_vm->mrowner, data, data_len);
> @@ -1052,7 +1056,9 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
>              return -1;
>          }
>          if (data_len != QCRYPTO_HASH_DIGEST_LEN_SHA384) {
> -            error_setg(errp, "TDX: failed to decode mrownerconfig");
> +            error_setg(errp, "TDX 'mrownerconfig' sha384 digest was %ld bytes, "
> +                             "expected %d bytes", data_len,
> +                             QCRYPTO_HASH_DIGEST_LEN_SHA384);
>              return -1;
>          }
>          memcpy(init_vm->mrownerconfig, data, data_len);

Re: [PATCH 1/3] i386/cpu: Rename enable_cpuid_0x1f to force_cpuid_0x1f

[Zhao Liu]

On Tue, Jun 03, 2025 at 01:03:03AM -0400, Xiaoyao Li wrote:
> Date: Tue,  3 Jun 2025 01:03:03 -0400
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> Subject: [PATCH 1/3] i386/cpu: Rename enable_cpuid_0x1f to force_cpuid_0x1f
> X-Mailer: git-send-email 2.43.0
> 
> The name of "enable_cpuid_0x1f" isn't right to its behavior because the
> leaf 0x1f can be enabled even when "enable_cpuid_0x1f" is false.
> 
> Rename it to "force_cpuid_0x1f" to better reflect its behavior.
> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> Suggested by Igor at https://lore.kernel.org/qemu-devel/20250513144515.37615651@imammedo.users.ipa.redhat.com/
> ---
>  target/i386/cpu.h     | 4 ++--
>  target/i386/kvm/tdx.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 2/3] i386/tdx: Fix the typo of the comment of struct TdxGuest

[Zhao Liu]

On Tue, Jun 03, 2025 at 01:03:04AM -0400, Xiaoyao Li wrote:
> Date: Tue,  3 Jun 2025 01:03:04 -0400
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> Subject: [PATCH 2/3] i386/tdx: Fix the typo of the comment of struct
>  TdxGuest
> X-Mailer: git-send-email 2.43.0
> 
> Change sha348 to sha384.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>  target/i386/kvm/tdx.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 3/3] i386/tdx: Clarify the error message of mrconfigid/mrowner/mrownerconfig

[Zhao Liu]

On Tue, Jun 03, 2025 at 01:03:05AM -0400, Xiaoyao Li wrote:
> Date: Tue,  3 Jun 2025 01:03:05 -0400
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> Subject: [PATCH 3/3] i386/tdx: Clarify the error message of
>  mrconfigid/mrowner/mrownerconfig
> X-Mailer: git-send-email 2.43.0
> 
> The error message is misleading - we successfully decoded the data,
> the decoded data was simply with the wrong length.
> 
> Change the error message to show it is an length check failure with both
> the received and expected values.
> 
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> Suggested by Daniel at https://lore.kernel.org/qemu-devel/aBzT3TrdldaN-uqx@redhat.com/
> ---
>  target/i386/kvm/tdx.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 0/3] TDX: Fix and enhancement

[Paolo Bonzini]

Queued, thanks.

Paolo