Re: [Qemu-devel] [PATCH 03/10] hyperv: only add SynIC in compatible configurations

[Eduardo Habkost]

On Fri, Sep 21, 2018 at 11:22:10AM +0300, Roman Kagan wrote:
> Certain configurations do not allow SynIC to be used in QEMU.  In
> particular,
> 
> - when hyperv_vpindex is off, SINT routes can't be used as they refer to
>   the destination vCPU by vp_index
> 
> - older KVM (which doesn't expose KVM_CAP_HYPERV_SYNIC2) zeroes out
>   SynIC message and event pages on every msr load, breaking migration
> 
> OTOH in-KVM users of SynIC -- SynIC timers -- do work in those
> configurations, and we shouldn't stop the guest from using them.
> 
> To cover both scenarios, introduce an X86CPU property that makes CPU
> init code to skip creation of the SynIC object (and thus disables any
> SynIC use in QEMU) but keeps the KVM part of the SynIC working.
> The property is clear by default but is set via compat logic for older
> machine types.
> 
> As a result, when hv_synic and a modern machine type are specified, QEMU
> will refuse to run unless vp_index is on and the kernel is recent
> enough.  OTOH with an older machine type QEMU will run fine with
> hv_synic=on against an older kernel and/or without vp_index enabled but
> will disallow the in-QEMU uses of SynIC (in e.g. VMBus).
> 
> Signed-off-by: Roman Kagan <address@hidden>
> ---
>  include/hw/i386/pc.h |  8 ++++++++
>  target/i386/cpu.h    |  1 +
>  target/i386/cpu.c    |  2 ++
>  target/i386/kvm.c    | 30 ++++++++++++++++++++++--------
>  4 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 6894f37df1..dfe6746692 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -294,6 +294,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>  int e820_get_num_entries(void);
>  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>  
> +#define PC_COMPAT_3_0 \
> +    HW_COMPAT_3_0 \
> +    {\
> +        .driver   = TYPE_X86_CPU,\
> +        .property = "x-hv-synic-kvm-only",\
> +        .value    = "on",\
> +    }

Oops.  This macro does nothing if the 3.1 machine-types aren't
updated to use it.  This patch breaks compatibility on pc-*-3.1.

> [...]

-- 
Eduardo