| Message ID | 20221205173137.607044-4-dwmw2@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Xen HVM support under KVM | expand |
Hi David, On 5/12/22 18:31, David Woodhouse wrote: > From: Joao Martins <joao.m.martins@oracle.com> > > Introduce support for emulating CPUID for Xen HVM guests via > xen, xen_vapic as changeable params. > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > [dwmw2: Obtain xen_version from machine property] > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > target/i386/cpu.c | 2 ++ > target/i386/cpu.h | 3 ++ > target/i386/kvm/kvm.c | 72 +++++++++++++++++++++++++++++++++++++++++++ > target/i386/xen.h | 8 +++++ > 4 files changed, 85 insertions(+) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 22b681ca37..45aa9e40a5 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -7069,6 +7069,8 @@ static Property x86_cpu_properties[] = { > * own cache information (see x86_cpu_load_def()). > */ > DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, true), > + DEFINE_PROP_BOOL("xen", X86CPU, xen, false), Maybe name it 'xen-hvm'? > + DEFINE_PROP_BOOL("xen-vapic", X86CPU, xen_vapic, false), What happens if we use -cpu host,-kvm,+xen,-xen-vapic ? Is -cpu host,-kvm,-xen,+xen-vapic meaningful? Otherwise we need to error out (eventually displaying some hint). > > /* > * From "Requirements for Implementing the Microsoft > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index d4bc19577a..5ddd14467e 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -1964,6 +1964,9 @@ struct ArchCPU { > int32_t thread_id; > > int32_t hv_max_vps; > + > + bool xen; > + bool xen_vapic; > };
On Mon, 2022-12-05 at 22:58 +0100, Philippe Mathieu-Daudé wrote: > > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > > index 22b681ca37..45aa9e40a5 100644 > > --- a/target/i386/cpu.c > > +++ b/target/i386/cpu.c > > @@ -7069,6 +7069,8 @@ static Property x86_cpu_properties[] = { > > * own cache information (see x86_cpu_load_def()). > > */ > > DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, true), > > + DEFINE_PROP_BOOL("xen", X86CPU, xen, false), > > Maybe name it 'xen-hvm'? I think I'd prefer it to go away completely. If the *machine* has the Xen feature enabled (which I've made implicit in the 'xen-version' property), perhaps we should *always* disable 'expose_kvm' and enable the Xen CPUID leaves instead? > > + DEFINE_PROP_BOOL("xen-vapic", X86CPU, xen_vapic, false), > > What happens if we use -cpu host,-kvm,+xen,-xen-vapic ? That's sane; it does the Xen CPUID thing but doesn't advertise the vAPIC feature in the Xen CPUID leaves. > Is -cpu host,-kvm,-xen,+xen-vapic meaningful? Otherwise we need to error > out (eventually displaying some hint). Indeed it isn't meaningful, and should cause an error.
On 6/12/22 01:18, David Woodhouse wrote: > On Mon, 2022-12-05 at 22:58 +0100, Philippe Mathieu-Daudé wrote: >>> >>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >>> index 22b681ca37..45aa9e40a5 100644 >>> --- a/target/i386/cpu.c >>> +++ b/target/i386/cpu.c >>> @@ -7069,6 +7069,8 @@ static Property x86_cpu_properties[] = { >>> * own cache information (see x86_cpu_load_def()). >>> */ >>> DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, true), >>> + DEFINE_PROP_BOOL("xen", X86CPU, xen, false), >> >> Maybe name it 'xen-hvm'? > > I think I'd prefer it to go away completely. If the *machine* has the > Xen feature enabled (which I've made implicit in the 'xen-version' > property), perhaps we should *always* disable 'expose_kvm' and enable > the Xen CPUID leaves instead? It would be silly to run a non-Xen guest on the Xen machine, so it is not a bad idea :) >>> + DEFINE_PROP_BOOL("xen-vapic", X86CPU, xen_vapic, false), >> >> What happens if we use -cpu host,-kvm,+xen,-xen-vapic ? > > That's sane; it does the Xen CPUID thing but doesn't advertise the > vAPIC feature in the Xen CPUID leaves. In which case we don't want to use the vAPIC? Thanks, Phil.
On Tue, 2022-12-06 at 08:58 +0100, Philippe Mathieu-Daudé wrote: > On 6/12/22 01:18, David Woodhouse wrote: > > On Mon, 2022-12-05 at 22:58 +0100, Philippe Mathieu-Daudé wrote: > > > > > > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > > > > index 22b681ca37..45aa9e40a5 100644 > > > > --- a/target/i386/cpu.c > > > > +++ b/target/i386/cpu.c > > > > @@ -7069,6 +7069,8 @@ static Property x86_cpu_properties[] = { > > > > * own cache information (see x86_cpu_load_def()). > > > > */ > > > > DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, true), > > > > + DEFINE_PROP_BOOL("xen", X86CPU, xen, false), > > > > > > Maybe name it 'xen-hvm'? > > > > I think I'd prefer it to go away completely. If the *machine* has the > > Xen feature enabled (which I've made implicit in the 'xen-version' > > property), perhaps we should *always* disable 'expose_kvm' and enable > > the Xen CPUID leaves instead? > > It would be silly to run a non-Xen guest on the Xen machine, so it is > not a bad idea :) Right. I just wasn't sure how to do it as *default* but still let it be overridden. I may just hard-code it. > > > > + DEFINE_PROP_BOOL("xen-vapic", X86CPU, xen_vapic, false), > > > > > > What happens if we use -cpu host,-kvm,+xen,-xen-vapic ? > > > > That's sane; it does the Xen CPUID thing but doesn't advertise the > > vAPIC feature in the Xen CPUID leaves. > > In which case we don't want to use the vAPIC? Indeed. In that case we won't advertise vAPIC to the guest, and then the guest will use PIRQ to route MSIs to event channels instead of directly to guest vCPUs.
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 22b681ca37..45aa9e40a5 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -7069,6 +7069,8 @@ static Property x86_cpu_properties[] = { * own cache information (see x86_cpu_load_def()). */ DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, true), + DEFINE_PROP_BOOL("xen", X86CPU, xen, false), + DEFINE_PROP_BOOL("xen-vapic", X86CPU, xen_vapic, false), /* * From "Requirements for Implementing the Microsoft diff --git a/target/i386/cpu.h b/target/i386/cpu.h index d4bc19577a..5ddd14467e 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1964,6 +1964,9 @@ struct ArchCPU { int32_t thread_id; int32_t hv_max_vps; + + bool xen; + bool xen_vapic; }; diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index ff3ea245cf..4b21d03250 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -22,6 +22,7 @@ #include <linux/kvm.h> #include "standard-headers/asm-x86/kvm_para.h" +#include "standard-headers/xen/arch-x86/cpuid.h" #include "cpu.h" #include "host-cpu.h" @@ -34,6 +35,7 @@ #include "xen.h" #include "hyperv.h" #include "hyperv-proto.h" +#include "xen.h" #include "exec/gdbstub.h" #include "qemu/host-utils.h" @@ -775,6 +777,12 @@ static inline bool freq_within_bounds(int freq, int target_freq) return false; } + +static bool xen_enabled_on_kvm(X86CPU *cpu) +{ + return cpu->xen; +} + static int kvm_arch_set_tsc_khz(CPUState *cs) { X86CPU *cpu = X86_CPU(cs); @@ -1800,6 +1808,70 @@ int kvm_arch_init_vcpu(CPUState *cs) has_msr_hv_hypercall = true; } + if (xen_enabled_on_kvm(cpu) && kvm_base == XEN_CPUID_SIGNATURE) { + struct kvm_cpuid_entry2 *xen_max_leaf; + MachineState *ms = MACHINE(qdev_get_machine()); + uint32_t xen_version = object_property_get_int(OBJECT(ms), "xen-version", &error_abort); + + memcpy(signature, "XenVMMXenVMM", 12); + + xen_max_leaf = c = &cpuid_data.entries[cpuid_i++]; + c->function = XEN_CPUID_SIGNATURE; + c->eax = XEN_CPUID_TIME; + c->ebx = signature[0]; + c->ecx = signature[1]; + c->edx = signature[2]; + + c = &cpuid_data.entries[cpuid_i++]; + c->function = XEN_CPUID_VENDOR; + c->eax = xen_version; + c->ebx = 0; + c->ecx = 0; + c->edx = 0; + + c = &cpuid_data.entries[cpuid_i++]; + c->function = XEN_CPUID_HVM_MSR; + /* Number of hypercall-transfer pages */ + c->eax = 1; + /* Hypercall MSR base address */ + c->ebx = XEN_HYPERCALL_MSR; + c->ecx = 0; + c->edx = 0; + + c = &cpuid_data.entries[cpuid_i++]; + c->function = XEN_CPUID_TIME; + c->eax = ((!!tsc_is_stable_and_known(env) << 1) | + (!!(env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_RDTSCP) << 2)); + /* default=0 (emulate if necessary) */ + c->ebx = 0; + /* guest tsc frequency */ + c->ecx = env->user_tsc_khz; + /* guest tsc incarnation (migration count) */ + c->edx = 0; + + c = &cpuid_data.entries[cpuid_i++]; + c->function = XEN_CPUID_HVM; + xen_max_leaf->eax = XEN_CPUID_HVM; + if (xen_version >= XEN_VERSION(4,5)) { + c->function = XEN_CPUID_HVM; + + if (cpu->xen_vapic) { + c->eax |= XEN_HVM_CPUID_APIC_ACCESS_VIRT; + c->eax |= XEN_HVM_CPUID_X2APIC_VIRT; + } + + c->eax |= XEN_HVM_CPUID_IOMMU_MAPPINGS; + + if (xen_version >= XEN_VERSION(4,6)) { + c->eax |= XEN_HVM_CPUID_VCPU_ID_PRESENT; + c->ebx = cs->cpu_index; + } + } + + kvm_base = KVM_CPUID_SIGNATURE_NEXT; + } + + if (cpu->expose_kvm) { memcpy(signature, "KVMKVMKVM\0\0\0", 12); c = &cpuid_data.entries[cpuid_i++]; diff --git a/target/i386/xen.h b/target/i386/xen.h index 6c4f3b7822..d4903ecfa1 100644 --- a/target/i386/xen.h +++ b/target/i386/xen.h @@ -14,6 +14,14 @@ #define XEN_HYPERCALL_MSR 0x40000000 +#define XEN_CPUID_SIGNATURE 0x40000000 +#define XEN_CPUID_VENDOR 0x40000001 +#define XEN_CPUID_HVM_MSR 0x40000002 +#define XEN_CPUID_TIME 0x40000003 +#define XEN_CPUID_HVM 0x40000004 + +#define XEN_VERSION(maj, min) ((maj) << 16 | (min)) + int kvm_xen_init(KVMState *s, uint32_t xen_version); #endif /* QEMU_I386_XEN_H */