Re: [PATCH 1/2] hyperv: Support hv-emsr-bitmap enlightenment

On Thu, Aug 15, 2024 at 03:57:35PM +0200, Tim Wiederhake wrote: > qemu supports this enlightenment since version 7.10. > > From the qemu commit: >    The newly introduced enlightenment allow L0 (KVM) and L1 (Hyper- > V) >    hypervisors to collaborate to avoid unnecessary updates to L2 >    MSR-Bitmap upon vmexits. > > Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; > --- > docs/formatdomain.rst                              | 2 ++ > src/conf/domain_conf.c                             | 4 ++++ > src/conf/domain_conf.h                             | 1 + > src/conf/schemas/domaincommon.rng                  | 5 +++++ > src/cpu/cpu_x86.c                                  | 4 ++++ > src/cpu/cpu_x86_data.h                             | 1 + > src/qemu/qemu_command.c                            | 5 +++++ > src/qemu/qemu_process.c                            | 1 + > tests/domaincapsdata/qemu_8.0.0-q35.x86_64.xml     | 1 + > tests/domaincapsdata/qemu_8.0.0-tcg.x86_64.xml     | 1 + > tests/domaincapsdata/qemu_8.0.0.x86_64.xml         | 1 + > tests/domaincapsdata/qemu_8.1.0-q35.x86_64.xml     | 1 + > tests/domaincapsdata/qemu_8.1.0-tcg.x86_64.xml     | 1 + > tests/domaincapsdata/qemu_8.1.0.x86_64.xml         | 1 + > tests/domaincapsdata/qemu_8.2.0-q35.x86_64.xml     | 1 + > tests/domaincapsdata/qemu_8.2.0-tcg.x86_64.xml     | 1 + > tests/domaincapsdata/qemu_8.2.0.x86_64.xml         | 1 + > tests/domaincapsdata/qemu_9.0.0-q35.x86_64.xml     | 1 + > tests/domaincapsdata/qemu_9.0.0-tcg.x86_64.xml     | 1 + > tests/domaincapsdata/qemu_9.0.0.x86_64.xml         | 1 + > tests/domaincapsdata/qemu_9.1.0-q35.x86_64.xml     | 1 + > tests/domaincapsdata/qemu_9.1.0-tcg.x86_64.xml     | 1 + > tests/domaincapsdata/qemu_9.1.0.x86_64.xml         | 1 + > tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml   | 1 + > tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml   | 1 + > tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml   | 1 + > tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml   | 1 + > tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml   | 1 + > tests/qemuxmlconfdata/hyperv-off.x86_64-latest.xml | 1 + > tests/qemuxmlconfdata/hyperv-off.xml               | 1 + > tests/qemuxmlconfdata/hyperv.x86_64-latest.args    | 2 +- > tests/qemuxmlconfdata/hyperv.x86_64-latest.xml     | 1 + > tests/qemuxmlconfdata/hyperv.xml                   | 1 + > 33 files changed, 48 insertions(+), 1 deletion(-) > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst > index e1e219bcb5..0cf3dc9deb 100644 > --- a/docs/formatdomain.rst > +++ b/docs/formatdomain.rst > @@ -1989,6 +1989,7 @@ Hypervisors may allow certain CPU / machine > features to be toggled on/off. >        <tlbflush state='on'/> >        <ipi state='on'/> >        <evmcs state='on'/> > +       <emsr_bitmap state='on'/> >      </hyperv> >      <kvm> >        <hidden state='on'/> > @@ -2076,6 +2077,7 @@ are: >    ipi             Enable PV IPI > support                                                  on, > off                                      :since:`4.10.0 (QEMU 3.1)` >    evmcs           Enable Enlightened > VMCS                                                on, > off                                      :since:`4.10.0 (QEMU 3.1)` >    avic            Enable use Hyper-V SynIC with hardware > APICv/AVIC                      on, > off                                      :since:`8.10.0 (QEMU 6.2)` > +   emsr_bitmap     Avoid unnecessary updates to L2 MSR Bitmap upon > vmexits.               on, off                                      > :since:`10.7.0 (QEMU 7.1)` >    =============== > =================================================================== > === ============================================ > ======================================================= > >    :since:`Since 8.0.0`, the hypervisor can be configured further > by setting > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index d950921667..c88e885d07 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -211,6 +211,7 @@ VIR_ENUM_IMPL(virDomainHyperv, >               "ipi", >               "evmcs", >               "avic", > +              "emsr_bitmap", > ); > > VIR_ENUM_IMPL(virDomainKVM, > @@ -16574,6 +16575,7 @@ > virDomainFeaturesHyperVDefParse(virDomainDef *def, >         case VIR_DOMAIN_HYPERV_IPI: >         case VIR_DOMAIN_HYPERV_EVMCS: >         case VIR_DOMAIN_HYPERV_AVIC: > +        case VIR_DOMAIN_HYPERV_EMSR_BITMAP: >             break; > >         case VIR_DOMAIN_HYPERV_STIMER: > @@ -21020,6 +21022,7 @@ > virDomainDefFeaturesCheckABIStability(virDomainDef *src, >             case VIR_DOMAIN_HYPERV_IPI: >             case VIR_DOMAIN_HYPERV_EVMCS: >             case VIR_DOMAIN_HYPERV_AVIC: > +            case VIR_DOMAIN_HYPERV_EMSR_BITMAP: >                 if (src->hyperv_features[i] != dst- > >hyperv_features[i]) { >                     virReportError(VIR_ERR_CONFIG_UNSUPPORTED, >                                    _("State of HyperV enlightenment > feature '%1$s' differs: source: '%2$s', destination: '%3$s'"), > @@ -27761,6 +27764,7 @@ virDomainDefFormatFeatures(virBuffer *buf, >                 case VIR_DOMAIN_HYPERV_IPI: >                 case VIR_DOMAIN_HYPERV_EVMCS: >                 case VIR_DOMAIN_HYPERV_AVIC: > +                case VIR_DOMAIN_HYPERV_EMSR_BITMAP: >                     virBufferAddLit(&childBuf, "/>\n"); >                     break; > > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index eae621f900..332bcd0cf9 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -2203,6 +2203,7 @@ typedef enum { >     VIR_DOMAIN_HYPERV_IPI, >     VIR_DOMAIN_HYPERV_EVMCS, >     VIR_DOMAIN_HYPERV_AVIC, > +    VIR_DOMAIN_HYPERV_EMSR_BITMAP, > >     VIR_DOMAIN_HYPERV_LAST > } virDomainHyperv; > diff --git a/src/conf/schemas/domaincommon.rng > b/src/conf/schemas/domaincommon.rng > index 05ba697924..6f81fc94c2 100644 > --- a/src/conf/schemas/domaincommon.rng > +++ b/src/conf/schemas/domaincommon.rng > @@ -7962,6 +7962,11 @@ >                 <ref name="featurestate"/> >               </element> >             </optional> > +            <optional> > +              <element name="emsr_bitmap"> > +                <ref name="featurestate"/> > +              </element> > +            </optional> >           </interleave> >         </group> >       </choice> > diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c > index fcbce0ec46..139ac48234 100644 > --- a/src/cpu/cpu_x86.c > +++ b/src/cpu/cpu_x86.c > @@ -107,6 +107,9 @@ KVM_FEATURE_DEF(VIR_CPU_x86_HV_IPI, > KVM_FEATURE_DEF(VIR_CPU_x86_HV_EVMCS, >                 0x40000004, 0x00004000, 0x0); > > +KVM_FEATURE_DEF(VIR_CPU_x86_HV_EMSR_BITMAP, > +                0x4000000A, 0x00080000, 0x0); > + > static virCPUx86Feature x86_kvm_features[] = > { >     KVM_FEATURE(VIR_CPU_x86_KVM_PV_UNHALT), > @@ -124,6 +127,7 @@ static virCPUx86Feature x86_kvm_features[] = >     KVM_FEATURE(VIR_CPU_x86_HV_IPI), >     KVM_FEATURE(VIR_CPU_x86_HV_EVMCS), >     KVM_FEATURE(VIR_CPU_x86_HV_STIMER_DIRECT), > +    KVM_FEATURE(VIR_CPU_x86_HV_EMSR_BITMAP), > }; > > typedef struct _virCPUx86Signature virCPUx86Signature; > diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h > index d1ca63db6d..414643fe8d 100644 > --- a/src/cpu/cpu_x86_data.h > +++ b/src/cpu/cpu_x86_data.h > @@ -62,6 +62,7 @@ struct _virCPUx86MSR { > #define VIR_CPU_x86_HV_IPI       "hv-ipi" > #define VIR_CPU_x86_HV_EVMCS     "hv-evmcs" > #define VIR_CPU_x86_HV_AVIC      "hv-avic" > +#define VIR_CPU_x86_HV_EMSR_BITMAP "hv-emsr_bitmap" > > /* Hyper-V Synthetic Timer option */ > #define VIR_CPU_x86_HV_STIMER_DIRECT "hv-stimer-direct" > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 28914c9c34..cac6fc57a5 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -6523,6 +6523,11 @@ qemuBuildCpuCommandLine(virCommand *cmd, >                                       def->hyperv_vendor_id); >                 break; > > +            case VIR_DOMAIN_HYPERV_EMSR_BITMAP: > +                if (def->hyperv_features[i] == > VIR_TRISTATE_SWITCH_ON) > +                    virBufferAsprintf(&buf, ",%s=on", "hv-emsr- > bitmap"); > +                break; > + What is the reason for us to only include the "on" state?  Is it because qemu guarantees the default is (and always will be) off?  If that is true, then Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com&gt;

If not, then we should adapt this and all of the above to something like: if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ABSENT) {      virBufferAsprintf(&buf,                        ",%s=%s",                        "hv-emsr-bitmap",                        virTristateSwitchTypeToString(def- >hyperv_features[i])); } or no?