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

Igor Mammedov
2025-01-24T13:01:06Z

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
> >
>