[v7] QEMU TDX support

[PATCH v7 38/52] i386/apic: Skip kvm_apic_put() for TDX

Posted by Xiaoyao Li 2 months, 1 week ago

KVM neithers allow writing to MSR_IA32_APICBASE for TDs, nor allow for
KVM_SET_LAPIC[*].

Note, KVM_GET_LAPIC is also disallowed for TDX. It is called in the path

  do_kvm_cpu_synchronize_state()
  -> kvm_arch_get_registers()
     -> kvm_get_apic()

and it's already disllowed for confidential guest through
guest_state_protected.

[*] https://lore.kernel.org/all/Z3w4Ku4Jq0CrtXne@google.com/

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 hw/i386/kvm/apic.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 757510600098..a1850524a67f 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -17,6 +17,7 @@
 #include "system/hw_accel.h"
 #include "system/kvm.h"
 #include "kvm/kvm_i386.h"
+#include "kvm/tdx.h"
 
 static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
                                     int reg_id, uint32_t val)
@@ -141,6 +142,10 @@ static void kvm_apic_put(CPUState *cs, run_on_cpu_data data)
     struct kvm_lapic_state kapic;
     int ret;
 
+    if(is_tdx_vm()) {
+        return;
+    }
+
     kvm_put_apicbase(s->cpu, s->apicbase);
     kvm_put_apic_state(s, &kapic);
 
-- 
2.34.1

Re: [PATCH v7 38/52] i386/apic: Skip kvm_apic_put() for TDX

Posted by Francesco Lavra 1 month ago

On Fri, 2025-01-24 at 08:20 -0500, Xiaoyao Li wrote:
> KVM neithers allow writing to MSR_IA32_APICBASE for TDs, nor allow
> for
> KVM_SET_LAPIC[*].
> 
> Note, KVM_GET_LAPIC is also disallowed for TDX. It is called in the
> path
> 
>   do_kvm_cpu_synchronize_state()
>   -> kvm_arch_get_registers()
>      -> kvm_get_apic()
> 
> and it's already disllowed for confidential guest through
> guest_state_protected.
> 
> [*] https://lore.kernel.org/all/Z3w4Ku4Jq0CrtXne@google.com/
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>  hw/i386/kvm/apic.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
> index 757510600098..a1850524a67f 100644
> --- a/hw/i386/kvm/apic.c
> +++ b/hw/i386/kvm/apic.c
> @@ -17,6 +17,7 @@
>  #include "system/hw_accel.h"
>  #include "system/kvm.h"
>  #include "kvm/kvm_i386.h"
> +#include "kvm/tdx.h"
>  
>  static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
>                                      int reg_id, uint32_t val)
> @@ -141,6 +142,10 @@ static void kvm_apic_put(CPUState *cs,
> run_on_cpu_data data)
>      struct kvm_lapic_state kapic;
>      int ret;
>  
> +    if(is_tdx_vm()) {

Missing space between if and (.
scripts/checkpatch.pl would have caught this.

Re: [PATCH v7 38/52] i386/apic: Skip kvm_apic_put() for TDX

Posted by Xiaoyao Li 1 month ago

On 2/28/2025 12:57 AM, Francesco Lavra wrote:
> On Fri, 2025-01-24 at 08:20 -0500, Xiaoyao Li wrote:
>> KVM neithers allow writing to MSR_IA32_APICBASE for TDs, nor allow
>> for
>> KVM_SET_LAPIC[*].
>>
>> Note, KVM_GET_LAPIC is also disallowed for TDX. It is called in the
>> path
>>
>>    do_kvm_cpu_synchronize_state()
>>    -> kvm_arch_get_registers()
>>       -> kvm_get_apic()
>>
>> and it's already disllowed for confidential guest through
>> guest_state_protected.
>>
>> [*] https://lore.kernel.org/all/Z3w4Ku4Jq0CrtXne@google.com/
>>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>>   hw/i386/kvm/apic.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
>> index 757510600098..a1850524a67f 100644
>> --- a/hw/i386/kvm/apic.c
>> +++ b/hw/i386/kvm/apic.c
>> @@ -17,6 +17,7 @@
>>   #include "system/hw_accel.h"
>>   #include "system/kvm.h"
>>   #include "kvm/kvm_i386.h"
>> +#include "kvm/tdx.h"
>>   
>>   static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
>>                                       int reg_id, uint32_t val)
>> @@ -141,6 +142,10 @@ static void kvm_apic_put(CPUState *cs,
>> run_on_cpu_data data)
>>       struct kvm_lapic_state kapic;
>>       int ret;
>>   
>> +    if(is_tdx_vm()) {
> 
> Missing space between if and (.
> scripts/checkpatch.pl would have caught this.

Me to be blamed that don't use checkpatch.pl everytime before post.