| Message ID | 20241105062408.3533704-60-xiaoyao.li@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | QEMU TDX support | expand |
On 11/5/24 07:24, Xiaoyao Li wrote: > Otherwise, it gets warnings like below when number of vcpus > 1: > > warning: TDX enforces set the feature: CPUID.01H:EDX.ht [bit 28] > > This is because x86_confidential_guest_check_features() checks > env->features[] instead of the cpuid date set up by cpu_x86_cpuid() > > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> > --- > target/i386/cpu.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 472ab206d8fe..214a1b00a815 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -6571,7 +6571,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > *edx = env->features[FEAT_1_EDX]; > if (threads_per_pkg > 1) { > *ebx |= threads_per_pkg << 16; > - *edx |= CPUID_HT; > } > if (!cpu->enable_pmu) { > *ecx &= ~CPUID_EXT_PDCM; > @@ -7784,6 +7783,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > Error *local_err = NULL; > unsigned requested_lbr_fmt; > > + qemu_early_init_vcpu(cs); > + > #if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY) > /* Use pc-relative instructions in system-mode */ > tcg_cflags_set(cs, CF_PCREL); > @@ -7851,6 +7852,14 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > } > } > > + /* > + * It needs to called after feature filter because KVM doesn't report HT > + * as supported Does it, since kvm_arch_get_supported_cpuid() has the following line? if (function == 1 && reg == R_EDX) { ... /* KVM never reports CPUID_HT but QEMU can support when vcpus > 1 */ ret |= CPUID_HT; ? Paolo > + */ > + if (cs->nr_cores * cs->nr_threads > 1) { > + env->features[FEAT_1_EDX] |= CPUID_HT; > + }
On 11/5/2024 5:12 PM, Paolo Bonzini wrote: > On 11/5/24 07:24, Xiaoyao Li wrote: >> Otherwise, it gets warnings like below when number of vcpus > 1: >> >> warning: TDX enforces set the feature: CPUID.01H:EDX.ht [bit 28] >> >> This is because x86_confidential_guest_check_features() checks >> env->features[] instead of the cpuid date set up by cpu_x86_cpuid() >> >> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> >> --- >> target/i386/cpu.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >> index 472ab206d8fe..214a1b00a815 100644 >> --- a/target/i386/cpu.c >> +++ b/target/i386/cpu.c >> @@ -6571,7 +6571,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t >> index, uint32_t count, >> *edx = env->features[FEAT_1_EDX]; >> if (threads_per_pkg > 1) { >> *ebx |= threads_per_pkg << 16; >> - *edx |= CPUID_HT; >> } >> if (!cpu->enable_pmu) { >> *ecx &= ~CPUID_EXT_PDCM; >> @@ -7784,6 +7783,8 @@ static void x86_cpu_realizefn(DeviceState *dev, >> Error **errp) >> Error *local_err = NULL; >> unsigned requested_lbr_fmt; >> + qemu_early_init_vcpu(cs); >> + >> #if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY) >> /* Use pc-relative instructions in system-mode */ >> tcg_cflags_set(cs, CF_PCREL); >> @@ -7851,6 +7852,14 @@ static void x86_cpu_realizefn(DeviceState *dev, >> Error **errp) >> } >> } >> + /* >> + * It needs to called after feature filter because KVM doesn't >> report HT >> + * as supported > > Does it, since kvm_arch_get_supported_cpuid() has the following line? > > if (function == 1 && reg == R_EDX) { > ... > /* KVM never reports CPUID_HT but QEMU can support when vcpus > > 1 */ > ret |= CPUID_HT; > > ? It seems I mixed it up with no_autoenable_flags. /faceplam CPUID_HT doesn't get enabled by x86_cpu_expand_features() for "-cpu host/max". It won't be filtered by x86_cpu_filter_features() either because QEMU sets it in kvm_arch_get_supported_cpuid(). yes, the comment is wrong and comment needs to be dropped. The code can be move up to just below x86_cpu_expand_features() or inside it? > Paolo > >> + */ >> + if (cs->nr_cores * cs->nr_threads > 1) { >> + env->features[FEAT_1_EDX] |= CPUID_HT; >> + } >
On Tue, Nov 5, 2024 at 10:33 AM Xiaoyao Li <xiaoyao.li@intel.com> wrote: > > On 11/5/2024 5:12 PM, Paolo Bonzini wrote: > > On 11/5/24 07:24, Xiaoyao Li wrote: > >> Otherwise, it gets warnings like below when number of vcpus > 1: > >> > >> warning: TDX enforces set the feature: CPUID.01H:EDX.ht [bit 28] > >> > >> This is because x86_confidential_guest_check_features() checks > >> env->features[] instead of the cpuid date set up by cpu_x86_cpuid() > >> > > It seems I mixed it up with no_autoenable_flags. /faceplam > > CPUID_HT doesn't get enabled by x86_cpu_expand_features() for "-cpu > host/max". It won't be filtered by x86_cpu_filter_features() either > because QEMU sets it in kvm_arch_get_supported_cpuid(). > > yes, the comment is wrong and comment needs to be dropped. The code can > be move up to just below x86_cpu_expand_features() or inside it? Inside it seems okay, and you can then remove it from cpu_x86_cpuid(). However, let's also add qemu_early_init_vcpu() to the realize function from all targets, and remove MachineState *ms = MACHINE(qdev_get_machine()); cpu->nr_cores = machine_topo_get_cores_per_socket(ms); cpu->nr_threads = ms->smp.threads; from qemu_init_vcpu(). You can resend patches 58 and 59 separately from the TDX series. Paolo
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 472ab206d8fe..214a1b00a815 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -6571,7 +6571,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *edx = env->features[FEAT_1_EDX]; if (threads_per_pkg > 1) { *ebx |= threads_per_pkg << 16; - *edx |= CPUID_HT; } if (!cpu->enable_pmu) { *ecx &= ~CPUID_EXT_PDCM; @@ -7784,6 +7783,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) Error *local_err = NULL; unsigned requested_lbr_fmt; + qemu_early_init_vcpu(cs); + #if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY) /* Use pc-relative instructions in system-mode */ tcg_cflags_set(cs, CF_PCREL); @@ -7851,6 +7852,14 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) } } + /* + * It needs to called after feature filter because KVM doesn't report HT + * as supported + */ + if (cs->nr_cores * cs->nr_threads > 1) { + env->features[FEAT_1_EDX] |= CPUID_HT; + } + /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on * CPUID[1].EDX. */
Otherwise, it gets warnings like below when number of vcpus > 1: warning: TDX enforces set the feature: CPUID.01H:EDX.ht [bit 28] This is because x86_confidential_guest_check_features() checks env->features[] instead of the cpuid date set up by cpu_x86_cpuid() Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> --- target/i386/cpu.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)