[PATCH v6 40/60] hw/i386: add eoi_intercept_unsupported member to X86MachineState

Xiaoyao Li posted 60 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH v6 40/60] hw/i386: add eoi_intercept_unsupported member to X86MachineState
Posted by Xiaoyao Li 1 year, 3 months ago
Add a new bool member, eoi_intercept_unsupported, to X86MachineState
with default value false. Set true for TDX VM.

Inability to intercept eoi causes impossibility to emulate level
triggered interrupt to be re-injected when level is still kept active.
which affects interrupt controller emulation.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/x86.c         | 1 +
 include/hw/i386/x86.h | 1 +
 target/i386/kvm/tdx.c | 2 ++
 3 files changed, 4 insertions(+)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 01fc5e656272..82faeed24ff9 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -370,6 +370,7 @@ static void x86_machine_initfn(Object *obj)
     x86ms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
     x86ms->bus_lock_ratelimit = 0;
     x86ms->above_4g_mem_start = 4 * GiB;
+    x86ms->eoi_intercept_unsupported = false;
 }
 
 static void x86_machine_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index d43cb3908e65..fd9a30391755 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -73,6 +73,7 @@ struct X86MachineState {
     uint64_t above_4g_mem_start;
 
     /* CPU and apic information: */
+    bool eoi_intercept_unsupported;
     unsigned pci_irq_mask;
     unsigned apic_id_limit;
     uint16_t boot_cpus;
diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
index 9ab4e911f78a..9dcb77e011bd 100644
--- a/target/i386/kvm/tdx.c
+++ b/target/i386/kvm/tdx.c
@@ -388,6 +388,8 @@ static int tdx_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
         return -EOPNOTSUPP;
     }
 
+    x86ms->eoi_intercept_unsupported = true;
+
     /*
      * Set kvm_readonly_mem_allowed to false, because TDX only supports readonly
      * memory for shared memory but not for private memory. Besides, whether a
-- 
2.34.1
Re: [PATCH v6 40/60] hw/i386: add eoi_intercept_unsupported member to X86MachineState
Posted by Igor Mammedov 1 year ago
On Tue,  5 Nov 2024 01:23:48 -0500
Xiaoyao Li <xiaoyao.li@intel.com> wrote:

> Add a new bool member, eoi_intercept_unsupported, to X86MachineState
> with default value false. Set true for TDX VM.

I'd rename it to enable_eoi_intercept, by default set to true for evrything
and make TDX override this to false.
> 
> Inability to intercept eoi causes impossibility to emulate level
> triggered interrupt to be re-injected when level is still kept active.
> which affects interrupt controller emulation.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/i386/x86.c         | 1 +
>  include/hw/i386/x86.h | 1 +
>  target/i386/kvm/tdx.c | 2 ++
>  3 files changed, 4 insertions(+)
> 
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 01fc5e656272..82faeed24ff9 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -370,6 +370,7 @@ static void x86_machine_initfn(Object *obj)
>      x86ms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
>      x86ms->bus_lock_ratelimit = 0;
>      x86ms->above_4g_mem_start = 4 * GiB;
> +    x86ms->eoi_intercept_unsupported = false;
>  }
>  
>  static void x86_machine_class_init(ObjectClass *oc, void *data)
> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> index d43cb3908e65..fd9a30391755 100644
> --- a/include/hw/i386/x86.h
> +++ b/include/hw/i386/x86.h
> @@ -73,6 +73,7 @@ struct X86MachineState {
>      uint64_t above_4g_mem_start;
>  
>      /* CPU and apic information: */
> +    bool eoi_intercept_unsupported;
>      unsigned pci_irq_mask;
>      unsigned apic_id_limit;
>      uint16_t boot_cpus;
> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> index 9ab4e911f78a..9dcb77e011bd 100644
> --- a/target/i386/kvm/tdx.c
> +++ b/target/i386/kvm/tdx.c
> @@ -388,6 +388,8 @@ static int tdx_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>          return -EOPNOTSUPP;
>      }
>  
> +    x86ms->eoi_intercept_unsupported = true;

I don't particulary like accel go to its parent (machine) object and override things there
and that being buried deep inside.

How do you start TDX guest?
Is there a machine property or something like it to enable TDX?

> +
>      /*
>       * Set kvm_readonly_mem_allowed to false, because TDX only supports readonly
>       * memory for shared memory but not for private memory. Besides, whether a
Re: [PATCH v6 40/60] hw/i386: add eoi_intercept_unsupported member to X86MachineState
Posted by Xiaoyao Li 1 year ago
On 1/23/2025 8:41 PM, Igor Mammedov wrote:
> On Tue,  5 Nov 2024 01:23:48 -0500
> Xiaoyao Li <xiaoyao.li@intel.com> wrote:
> 
>> Add a new bool member, eoi_intercept_unsupported, to X86MachineState
>> with default value false. Set true for TDX VM.
> 
> I'd rename it to enable_eoi_intercept, by default set to true for evrything
> and make TDX override this to false.
>>
>> Inability to intercept eoi causes impossibility to emulate level
>> triggered interrupt to be re-injected when level is still kept active.
>> which affects interrupt controller emulation.
>>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>>   hw/i386/x86.c         | 1 +
>>   include/hw/i386/x86.h | 1 +
>>   target/i386/kvm/tdx.c | 2 ++
>>   3 files changed, 4 insertions(+)
>>
>> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
>> index 01fc5e656272..82faeed24ff9 100644
>> --- a/hw/i386/x86.c
>> +++ b/hw/i386/x86.c
>> @@ -370,6 +370,7 @@ static void x86_machine_initfn(Object *obj)
>>       x86ms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
>>       x86ms->bus_lock_ratelimit = 0;
>>       x86ms->above_4g_mem_start = 4 * GiB;
>> +    x86ms->eoi_intercept_unsupported = false;
>>   }
>>   
>>   static void x86_machine_class_init(ObjectClass *oc, void *data)
>> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
>> index d43cb3908e65..fd9a30391755 100644
>> --- a/include/hw/i386/x86.h
>> +++ b/include/hw/i386/x86.h
>> @@ -73,6 +73,7 @@ struct X86MachineState {
>>       uint64_t above_4g_mem_start;
>>   
>>       /* CPU and apic information: */
>> +    bool eoi_intercept_unsupported;
>>       unsigned pci_irq_mask;
>>       unsigned apic_id_limit;
>>       uint16_t boot_cpus;
>> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
>> index 9ab4e911f78a..9dcb77e011bd 100644
>> --- a/target/i386/kvm/tdx.c
>> +++ b/target/i386/kvm/tdx.c
>> @@ -388,6 +388,8 @@ static int tdx_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>>           return -EOPNOTSUPP;
>>       }
>>   
>> +    x86ms->eoi_intercept_unsupported = true;
> 
> I don't particulary like accel go to its parent (machine) object and override things there
> and that being buried deep inside.

I would suggest don't see TDX as accel but see it as a special type of 
x86 machine.

> How do you start TDX guest?
> Is there a machine property or something like it to enable TDX?

via the "confidential-guest-support" property.
This series introduces tdx-guest object and we start a TDX guest by:

$qemu-system-x86_64 -object tdx-guest,id=tdx0 \
     -machine ...,confidential-guest-support=tdx0

>> +
>>       /*
>>        * Set kvm_readonly_mem_allowed to false, because TDX only supports readonly
>>        * memory for shared memory but not for private memory. Besides, whether a
>
Re: [PATCH v6 40/60] hw/i386: add eoi_intercept_unsupported member to X86MachineState
Posted by Igor Mammedov 1 year ago
On Fri, 24 Jan 2025 00:45:50 +0800
Xiaoyao Li <xiaoyao.li@intel.com> wrote:

> On 1/23/2025 8:41 PM, Igor Mammedov wrote:
> > On Tue,  5 Nov 2024 01:23:48 -0500
> > Xiaoyao Li <xiaoyao.li@intel.com> wrote:
> >   
> >> Add a new bool member, eoi_intercept_unsupported, to X86MachineState
> >> with default value false. Set true for TDX VM.  
> > 
> > I'd rename it to enable_eoi_intercept, by default set to true for evrything
> > and make TDX override this to false.  
> >>
> >> Inability to intercept eoi causes impossibility to emulate level
> >> triggered interrupt to be re-injected when level is still kept active.
> >> which affects interrupt controller emulation.
> >>
> >> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> >> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> >> ---
> >>   hw/i386/x86.c         | 1 +
> >>   include/hw/i386/x86.h | 1 +
> >>   target/i386/kvm/tdx.c | 2 ++
> >>   3 files changed, 4 insertions(+)
> >>
> >> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> >> index 01fc5e656272..82faeed24ff9 100644
> >> --- a/hw/i386/x86.c
> >> +++ b/hw/i386/x86.c
> >> @@ -370,6 +370,7 @@ static void x86_machine_initfn(Object *obj)
> >>       x86ms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
> >>       x86ms->bus_lock_ratelimit = 0;
> >>       x86ms->above_4g_mem_start = 4 * GiB;
> >> +    x86ms->eoi_intercept_unsupported = false;
> >>   }
> >>   
> >>   static void x86_machine_class_init(ObjectClass *oc, void *data)
> >> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> >> index d43cb3908e65..fd9a30391755 100644
> >> --- a/include/hw/i386/x86.h
> >> +++ b/include/hw/i386/x86.h
> >> @@ -73,6 +73,7 @@ struct X86MachineState {
> >>       uint64_t above_4g_mem_start;
> >>   
> >>       /* CPU and apic information: */
> >> +    bool eoi_intercept_unsupported;
> >>       unsigned pci_irq_mask;
> >>       unsigned apic_id_limit;
> >>       uint16_t boot_cpus;
> >> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> >> index 9ab4e911f78a..9dcb77e011bd 100644
> >> --- a/target/i386/kvm/tdx.c
> >> +++ b/target/i386/kvm/tdx.c
> >> @@ -388,6 +388,8 @@ static int tdx_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
> >>           return -EOPNOTSUPP;
> >>       }
> >>   
> >> +    x86ms->eoi_intercept_unsupported = true;  
> > 
> > I don't particulary like accel go to its parent (machine) object and override things there
> > and that being buried deep inside.  
> 
> I would suggest don't see TDX as accel but see it as a special type of 
> x86 machine.
> 
> > How do you start TDX guest?
> > Is there a machine property or something like it to enable TDX?  
> 
> via the "confidential-guest-support" property.
> This series introduces tdx-guest object and we start a TDX guest by:
> 
> $qemu-system-x86_64 -object tdx-guest,id=tdx0 \
>      -machine ...,confidential-guest-support=tdx0

then the property setter would be a logical place to set
 eoi_intercept_unsupported = false
when its value is tdx0

> 
> >> +
> >>       /*
> >>        * Set kvm_readonly_mem_allowed to false, because TDX only supports readonly
> >>        * memory for shared memory but not for private memory. Besides, whether a  
> >   
>