Comment 61 for bug 1828495

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

@Christian,

I'm flagging the libvirt SRUs as Opinion. So far I have done quite a lot cherry-picks and backports identifying all the structures that have changed and I'm not currently seeing how this can be done as a SRU (even for Disco). There are quite a few data model changes to how libvirt was acquiring CPU features (and, now, missing features) that simply wont fit into SRU rules.

The base need for the features would be:

commit c145b660b8225f73db16660461077ef931730939
Author: Jiri Denemark <email address hidden>
Date: Fri Jun 7 09:07:10 2019

    cpu_conf: Introduce virCPUDefFilterFeatures

    This new internal API can be used for in place filtering of CPU features
    in virCPUDef.

    Signed-off-by: Jiri Denemark <email address hidden>
    Reviewed-by: Ján Tomko <email address hidden>

----

commit 2a4c23210674b453f91569f0f4b9fd5ebe8d7906
Author: Jiri Denemark <email address hidden>
Date: Mon Jun 10 11:46:10 2019

    qemu: Probe for max-x86_64-cpu type

    We will use it to check whether QEMU supports a specific CPU property.

    Signed-off-by: Jiri Denemark <email address hidden>
    Reviewed-by: Ján Tomko <email address hidden>

----

commit 0d254bce4ec6fd62c0277d24e28bf018a4c6cb37
Author: Jiri Denemark <email address hidden>
Date: Mon Jun 10 11:49:22 2019

    qemu: Probe for "unavailable-features" CPU property

    It is similar to "filtered-features" property, which reports CPUID bits
    corresponding to disabled features, but more general. The
    "unavailable-features" property supports both CPUID and MSR features by
    listing their names.

    Signed-off-by: Jiri Denemark <email address hidden>
    Reviewed-by: Ján Tomko <email address hidden>

----

commit 4e6f58b8d55d44fa9f80736b2745b44710f6e25a
Author: Jiri Denemark <email address hidden>
Date: Wed Jun 19 14:01:30 2019

    conf: Introduce virCPUDefCheckFeatures

    This API can be used to check whether a CPU definition contains features
    matching a given filter.

    Signed-off-by: Jiri Denemark <email address hidden>
    Reviewed-by: Ján Tomko <email address hidden>

----
commit b8e086a570b14b1f83fc07e25df6da758abe7706
Author: Jiri Denemark <email address hidden>
Date: Wed Jun 19 16:58:01 2019

    cpu_x86: Turn virCPUx86DataIteratorInit into a function

    Until now, this was a macro usable for direct initialization when a
    variable is defined. Turning the macro into a function makes it more
    general.

    Signed-off-by: Jiri Denemark <email address hidden>
    Reviewed-by: Ján Tomko <email address hidden>

< compiles good >

Then we would need the backport of:

cpu_x86: Introduce virCPUx86DataItem container struct

The following patches introduce CPU features read from MSR in addition
to those queried via CPUID instruction. Let's introduce a container
struct which will be able to describe either feature type.

Signed-off-by: Jiri Denemark <email address hidden>
Reviewed-by: Ján Tomko <email address hidden>

AND all the ones bellow:

fcf4846a6b cpu_x86: Add support for storing MSR features in CPU map
370177e2f6 cpu_x86: Store virCPUx86DataItem content in union
10b80165db cpu_x86: Make x86cpuidMatch more general
2eea67a98e cpu_x86: Make x86cpuidMatchMasked more general
da1efddfa6 cpu_x86: Make x86cpuidAndBits more general
4e3cab2d00 cpu_x86: Make x86cpuidClearBits more general
9c6f00fc33 cpu_x86: Make x86cpuidSetBits more general
559ccd7815 cpu_x86: Introduce virCPUx86DataCmp
0fdc0ad84c cpu_x86: Simplify x86DataAdd
3eff71a2d5 cpu_x86: Rename virCPUx86VendorToCPUID
8f1a8ce397 cpu_x86: Rename virCPUx86DataAddCPUID
ce42042577 cpu_x86: Rename virCPUx86DataAddCPUIDInt
95accfa7fa cpu_x86: Rename virCPUx86CPUIDSorter
609f467f13 cpu_x86: Rename x86DataCpuid
5655b83139 cpu_x86: Rename x86DataCpuidNext function
6c22b329d5 cpu_x86: Rename virCPUx86DataItem variables
c02d70d52e cpu_x86: Rename virCPUx86Vendor.cpuid

AT LEAST to compile the virCPUx86DataItem container feature.

----

Continuing, we would, still, need:

commit bcfed7f1c84cbff21d129a79cbd675b0cd51613c
Author: Jiri Denemark <email address hidden>
Date: Wed Jun 19 16:59:12 2019

    cpu_x86: Introduce virCPUx86FeatureFilter*MSR

    This functions may be used as a virCPUDefFeatureFilter callbacks for
    virCPUDefCheckFeatures, virCPUDefFilerFeatures, and similar functions to
    select (virCPUx86FeatureFilterSelectMSR) or drop
    (virCPUx86FeatureFilterDropMSR) features reported via MSR.

    Signed-off-by: Jiri Denemark <email address hidden>
    Reviewed-by: Ján Tomko <email address hidden>

commit 56b254dccc96b7339494812c9df07ccf6af3da95
Author: Jiri Denemark <email address hidden>
Date: Fri Mar 22 12:52:21 2019

    cpu_x86: Read CPU features from IA32_ARCH_CAPABILITIES MSR

    This is used by the host capabilities code to construct host CPU
    definition.

    Signed-off-by: Jiri Denemark <email address hidden>
    Reviewed-by: Ján Tomko <email address hidden>

commit c8ec678fd9d97189667c0121f48a220dd26856b7
Author: Jiri Denemark <email address hidden>
Date: Thu Mar 14 11:44:38 2019

    cpu_map: Introduce IA32_ARCH_CAPABILITIES MSR features

    Signed-off-by: Jiri Denemark <email address hidden>
    Reviewed-by: Ján Tomko <email address hidden>

commit 8eb4a89f5f7973f50aa8b6fa0b1a45b825dda208
Author: Jiri Denemark <email address hidden>
Date: Wed Jun 19 16:59:49 2019

    qemu: Forbid MSR features with old QEMU

    Without "unavailable-features" CPU property we cannot properly detect
    whether a specific MSR feature we asked for (either explicitly or
    implicitly via a CPU model) was disabled by QEMU for some reason.
    Because this could break migration, snapshots, and save/restore
    operaions, it's better to just forbid any use of MSR features with QEMU
    which lacks "unavailable-features" CPU property.

    Signed-off-by: Jiri Denemark <email address hidden>
    Reviewed-by: Ján Tomko <email address hidden>

commit 2674d00ed484091faf2b6e6b1efe58ee9a72b96b
Author: Jiri Denemark <email address hidden>
Date: Wed Jun 19 17:22:09 2019

    qemu: Drop MSR features from host-model with old QEMU

    With QEMU versions which lack "unavailable-features" we use CPUID based
    detection of features which were enabled or disabled once QEMU starts.
    Thus using MSR features with host-model would result in all of them
    being marked as disabled in the active domain definition even though
    QEMU did not actually disable them.

    Let's make sure we add MSR features to host-model only when
    "unavailable-features" property is supported by QEMU.

    Signed-off-by: Jiri Denemark <email address hidden>
    Reviewed-by: Ján Tomko <email address hidden>

and possibly many others as dependencies to these last.

----

There are some optional things like:

commit 5030a7450b0f0117a7903303572c6bda6c012327
Author: Jiri Denemark <email address hidden>
Date: Fri Jun 7 10:00:28 2019

    qemu_command: Use canonical names of CPU features

    When building QEMU command line, we should use the preferred spelling of
    each CPU feature without relying on compatibility aliases (which may be
    removed at some point).

    The "unavailable-features" CPU property is used as a witness for the
    correct names of the features in our translation table.

    Signed-off-by: Jiri Denemark <email address hidden>
    Reviewed-by: Ján Tomko <email address hidden>

which will change the canonical names for the CPU features (which is something used by the commits that will create the arch_capabilities and unavailable-features features).

----

So, IMO, we should keep QEMU cmdline XML arguments for the mitigation features and tell users to use *at least* Eoan's libvirt for the arch-capabilities feature in libvirt. Orelse we would have to either backport tons of libvirt patches OR workaround this by creating or own version, which is a no-no for SRUs.

Any objection ?

PS: We still need to document the arch-capabilities and all the mitigations it advertises (and features enabled/disabled in cmdline amd, possibly, now, in libvirt as cmdline xml extra arguments).