diff mbox series

[RFC,03/21] i386/kvm: handle Xen HVM cpuid leaves

Message ID 20221205173137.607044-4-dwmw2@infradead.org
State New
Headers show
Series Xen HVM support under KVM | expand

Commit Message

David Woodhouse Dec. 5, 2022, 5:31 p.m. UTC 
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(+)

Comments

Philippe Mathieu-Daudé Dec. 5, 2022, 9:58 p.m. UTC | #1 
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;
>   };
David Woodhouse Dec. 6, 2022, 12:18 a.m. UTC | #2 
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.
Philippe Mathieu-Daudé Dec. 6, 2022, 7:58 a.m. UTC | #3 
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.
David Woodhouse Dec. 6, 2022, 8:05 a.m. UTC | #4 
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 mbox series

Patch

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