Message ID | 20250124132048.3229049-52-xiaoyao.li@intel.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | QEMU TDX support | expand |
On Fri, Jan 24, 2025 at 2:40 PM Xiaoyao Li <xiaoyao.li@intel.com> wrote: > > For TDX guest, the phys_bits is not configurable and can only be > host/native value. > > Validate phys_bits inside tdx_check_features(). Hi Xiaoyao, to avoid qemu-kvm: TDX requires guest CPU physical bits (48) to match host CPU physical bits (52) I need options like -cpu host,phys-bits=52,guest-phys-bits=52,host-phys-bits-limit=52,-kvm-asyncpf-int to start a TDX guest, is that intentional? Thanks, Paolo > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> > --- > target/i386/host-cpu.c | 2 +- > target/i386/host-cpu.h | 1 + > target/i386/kvm/tdx.c | 8 ++++++++ > 3 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c > index 3e4e85e729c8..8a15af458b05 100644 > --- a/target/i386/host-cpu.c > +++ b/target/i386/host-cpu.c > @@ -15,7 +15,7 @@ > #include "system/system.h" > > /* Note: Only safe for use on x86(-64) hosts */ > -static uint32_t host_cpu_phys_bits(void) > +uint32_t host_cpu_phys_bits(void) > { > uint32_t eax; > uint32_t host_phys_bits; > diff --git a/target/i386/host-cpu.h b/target/i386/host-cpu.h > index 6a9bc918baa4..b97ec01c9bec 100644 > --- a/target/i386/host-cpu.h > +++ b/target/i386/host-cpu.h > @@ -10,6 +10,7 @@ > #ifndef HOST_CPU_H > #define HOST_CPU_H > > +uint32_t host_cpu_phys_bits(void); > void host_cpu_instance_init(X86CPU *cpu); > void host_cpu_max_instance_init(X86CPU *cpu); > bool host_cpu_realizefn(CPUState *cs, Error **errp); > diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c > index bb75eb06dad9..c906a76c4c0e 100644 > --- a/target/i386/kvm/tdx.c > +++ b/target/i386/kvm/tdx.c > @@ -24,6 +24,7 @@ > > #include "cpu.h" > #include "cpu-internal.h" > +#include "host-cpu.h" > #include "hw/i386/e820_memory_layout.h" > #include "hw/i386/x86.h" > #include "hw/i386/tdvf.h" > @@ -838,6 +839,13 @@ static int tdx_check_features(X86ConfidentialGuest *cg, CPUState *cs) > return -1; > } > > + if (cpu->phys_bits != host_cpu_phys_bits()) { > + error_report("TDX requires guest CPU physical bits (%u) " > + "to match host CPU physical bits (%u)", > + cpu->phys_bits, host_cpu_phys_bits()); > + exit(1); > + } > + > return 0; > } > > -- > 2.34.1 >
On 2/1/2025 2:27 AM, Paolo Bonzini wrote: > On Fri, Jan 24, 2025 at 2:40 PM Xiaoyao Li <xiaoyao.li@intel.com> wrote: >> >> For TDX guest, the phys_bits is not configurable and can only be >> host/native value. >> >> Validate phys_bits inside tdx_check_features(). > > Hi Xiaoyao, > > to avoid > > qemu-kvm: TDX requires guest CPU physical bits (48) to match host CPU > physical bits (52) > > I need options like > > -cpu host,phys-bits=52,guest-phys-bits=52,host-phys-bits-limit=52,-kvm-asyncpf-int > > to start a TDX guest, is that intentional? "-cpu host" should be sufficient and should not hit the error. why did you get "guest CPU physical bits (48)"? > Thanks, > > Paolo > >> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> >> --- >> target/i386/host-cpu.c | 2 +- >> target/i386/host-cpu.h | 1 + >> target/i386/kvm/tdx.c | 8 ++++++++ >> 3 files changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c >> index 3e4e85e729c8..8a15af458b05 100644 >> --- a/target/i386/host-cpu.c >> +++ b/target/i386/host-cpu.c >> @@ -15,7 +15,7 @@ >> #include "system/system.h" >> >> /* Note: Only safe for use on x86(-64) hosts */ >> -static uint32_t host_cpu_phys_bits(void) >> +uint32_t host_cpu_phys_bits(void) >> { >> uint32_t eax; >> uint32_t host_phys_bits; >> diff --git a/target/i386/host-cpu.h b/target/i386/host-cpu.h >> index 6a9bc918baa4..b97ec01c9bec 100644 >> --- a/target/i386/host-cpu.h >> +++ b/target/i386/host-cpu.h >> @@ -10,6 +10,7 @@ >> #ifndef HOST_CPU_H >> #define HOST_CPU_H >> >> +uint32_t host_cpu_phys_bits(void); >> void host_cpu_instance_init(X86CPU *cpu); >> void host_cpu_max_instance_init(X86CPU *cpu); >> bool host_cpu_realizefn(CPUState *cs, Error **errp); >> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c >> index bb75eb06dad9..c906a76c4c0e 100644 >> --- a/target/i386/kvm/tdx.c >> +++ b/target/i386/kvm/tdx.c >> @@ -24,6 +24,7 @@ >> >> #include "cpu.h" >> #include "cpu-internal.h" >> +#include "host-cpu.h" >> #include "hw/i386/e820_memory_layout.h" >> #include "hw/i386/x86.h" >> #include "hw/i386/tdvf.h" >> @@ -838,6 +839,13 @@ static int tdx_check_features(X86ConfidentialGuest *cg, CPUState *cs) >> return -1; >> } >> >> + if (cpu->phys_bits != host_cpu_phys_bits()) { >> + error_report("TDX requires guest CPU physical bits (%u) " >> + "to match host CPU physical bits (%u)", >> + cpu->phys_bits, host_cpu_phys_bits()); >> + exit(1); >> + } >> + >> return 0; >> } >> >> -- >> 2.34.1 >> > >
On 2/2/25 15:39, Xiaoyao Li wrote: > On 2/1/2025 2:27 AM, Paolo Bonzini wrote: >> On Fri, Jan 24, 2025 at 2:40 PM Xiaoyao Li <xiaoyao.li@intel.com> wrote: >>> >>> For TDX guest, the phys_bits is not configurable and can only be >>> host/native value. >>> >>> Validate phys_bits inside tdx_check_features(). >> >> Hi Xiaoyao, >> >> to avoid >> >> qemu-kvm: TDX requires guest CPU physical bits (48) to match host CPU >> physical bits (52) >> >> I need options like >> >> -cpu host,phys-bits=52,guest-phys-bits=52,host-phys-bits-limit=52,- >> kvm-asyncpf-int >> >> to start a TDX guest, is that intentional? > > "-cpu host" should be sufficient and should not hit the error. > > why did you get "guest CPU physical bits (48)"? Nevermind - I got it from the RHEL machine types. The fix (which does not have to be included upstream, though I would not complain) is diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c index e0ab41bcb7b..7aca8219405 100644 --- a/target/i386/kvm/tdx.c +++ b/target/i386/kvm/tdx.c @@ -444,6 +444,7 @@ static void tdx_cpu_instance_init(X86ConfidentialGuest *cg, CPUState *cpu) X86CPU *x86cpu = X86_CPU(cpu); object_property_set_bool(OBJECT(cpu), "pmu", false, &error_abort); + object_property_set_bool(OBJECT(cpu), "host-phys-bits-limit", 0, &error_abort); /* invtsc is fixed1 for TD guest */ object_property_set_bool(OBJECT(cpu), "invtsc", true, &error_abort); but it also needs the patch at https://lore.kernel.org/qemu-devel/20250203114132.259155-1-pbonzini@redhat.com/T/. With these two changes, QEMU accepts "-cpu host" just fine. Paolo
diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c index 3e4e85e729c8..8a15af458b05 100644 --- a/target/i386/host-cpu.c +++ b/target/i386/host-cpu.c @@ -15,7 +15,7 @@ #include "system/system.h" /* Note: Only safe for use on x86(-64) hosts */ -static uint32_t host_cpu_phys_bits(void) +uint32_t host_cpu_phys_bits(void) { uint32_t eax; uint32_t host_phys_bits; diff --git a/target/i386/host-cpu.h b/target/i386/host-cpu.h index 6a9bc918baa4..b97ec01c9bec 100644 --- a/target/i386/host-cpu.h +++ b/target/i386/host-cpu.h @@ -10,6 +10,7 @@ #ifndef HOST_CPU_H #define HOST_CPU_H +uint32_t host_cpu_phys_bits(void); void host_cpu_instance_init(X86CPU *cpu); void host_cpu_max_instance_init(X86CPU *cpu); bool host_cpu_realizefn(CPUState *cs, Error **errp); diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c index bb75eb06dad9..c906a76c4c0e 100644 --- a/target/i386/kvm/tdx.c +++ b/target/i386/kvm/tdx.c @@ -24,6 +24,7 @@ #include "cpu.h" #include "cpu-internal.h" +#include "host-cpu.h" #include "hw/i386/e820_memory_layout.h" #include "hw/i386/x86.h" #include "hw/i386/tdvf.h" @@ -838,6 +839,13 @@ static int tdx_check_features(X86ConfidentialGuest *cg, CPUState *cs) return -1; } + if (cpu->phys_bits != host_cpu_phys_bits()) { + error_report("TDX requires guest CPU physical bits (%u) " + "to match host CPU physical bits (%u)", + cpu->phys_bits, host_cpu_phys_bits()); + exit(1); + } + return 0; }
For TDX guest, the phys_bits is not configurable and can only be host/native value. Validate phys_bits inside tdx_check_features(). Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> --- target/i386/host-cpu.c | 2 +- target/i386/host-cpu.h | 1 + target/i386/kvm/tdx.c | 8 ++++++++ 3 files changed, 10 insertions(+), 1 deletion(-)