[libvirt] [PATCHv2 11/11] qemu_driver: BaselineHypervisorCPU supports S390 using QEMU/QMP
Chris Venteicher
cventeic at redhat.com
Sat Jul 21 04:05:25 UTC 2018
Quoting David Hildenbrand (2018-07-18 02:26:24)
> On 18.07.2018 00:39, Collin Walling wrote:
> > On 07/17/2018 05:01 PM, David Hildenbrand wrote:
> >> On 13.07.2018 18:00, Jiri Denemark wrote:
> >>> On Mon, Jul 09, 2018 at 22:56:55 -0500, Chris Venteicher wrote:
> >>>> Transient S390 configurations require using QEMU to compute CPU Model
> >>>> Baseline and to do CPU Feature Expansion.
> >>>>
> >>>> Start and use a single QEMU instance to do both the baseline and
> >>>> expansion transactions required by BaselineHypervisorCPU.
> >>>>
> >>>> CPU Feature Expansion uses true / false to indicate if property is/isn't
> >>>> included in model. Baseline only returns property list where all
> >>>> enumerated properties are included.
> >>>
> >>> So are you saying on s390 there's no chance there would be a CPU model
> >>> with some feature which is included in the CPU model disabled for some
> >>> reason? Sounds too good to be true :-) (This is the question I referred
> >>> to in one of my replies to the other patches.)
> >>
> >> Giving some background information: When we expand/baseline CPU models,
> >> we always expand them to the "-base" variants of our CPU models, which
> >> contain some set of features we expect to be around in all sane
> >> configurations ("minimal feature set").
> >>
> >> It is very unlikely that we ever have something like
> >> "z14-base,featx=off", but it could happen
> >> - When using an emulator (TCG)
> >> - When running nested and the guest hypervisor is started with such a
> >> strange CPU model
> >> - When something in the HW is very wrong or eventually removed in the
> >> future (unlikely but possible)
> >>
> >> On some very weird inputs to a baseline request, such a strange model
> >> can also be the result. But it is very unusual.
> >>
> >> I assume something like "baseline z14-base,featx=off with z14-base" will
> >> result in "z14-base,featx=off", too.
> >>
> >>
> >
> > That's correct. A CPU model with a feature disabled that is baseline with a CPU
> > model with that same feature enabled will omit that feature in the QMP response.
> >
> > e.g. if z14-base has features x, y, z then
> >
> > "baseline z14-base,featx=off with z14-base" will result in "z14-base,featy=on,featz=on"
I am runing tests on both S390 and X86 (hacked QEMU to enable baseline).
I don't see a "false" property in the baseline response in any of the logs.
I did try to slip a "zpci":false into the query-cpu-model-baseline but I still
don't get a false in the response.
Here is the request/response for reference.
{"execute":"query-cpu-model-baseline",
"arguments":{"modela":{"name":"z14"},
"modelb":{"name":"z13","props":{"msa5":true,"exrl":true,"zpci":false}}},
"id":"libvirt-2"}
{"return": {"model": {"name": "z13-base","props": {"aen": true, "aefsi": true,
"msa5": true, "msa4": true, "msa3": true, "msa2": true, "msa1": true, "sthyi":
true, "edat": true, "ri": true, "edat2": true, "vx": true, "ipter": true,
"esop": true, "cte": true, "sea_esop2": true, "te": true, "cmm": true}}}, "id":
"libvirt-2"}
> Usually we try to not chose a model with stripped off base features ("we
> try to produce a model that looks sane"), but instead fallback to some
> very ancient CPU model. E.g.
>
> { "execute": "query-cpu-model-baseline", "arguments" : { "modela": {
> "name": "z14-base", "props": {"msa" : false}}, "modelb": { "name": "z14"}} }
>
> -> {"return": {"model": {"name": "z800-base", "props": {"etf2": true,
> "ldisp": true}}}}
>
> We might want to change that behavior in the future however (or maybe it
> already is like this for some corner cases) - assume some base feature
> gets dropped by HW in a new CPU generation. We don't always want to
> fallback to a z900 or so when baselining. So one should assume that we
> can have disabled features here.
>
> Especially as there is a BUG in QEMU I'll have to fix:
>
> { "execute": "query-cpu-model-baseline", "arguments" : { "modela": {
> "name": "z14-base", "props": {"esan3" : false}}, "modelb": { "name":
> "z14"}} }
>
> -> Segmentation fault
>
> This would have to produce a model with esan3 disabled (very very
> unlikely to ever happen in real life :) )
>
> The result should be something like {"model": {"name": "z900-base",
> "props": {"esan3": false}}} or an error that they cannot be baselined.
>
Seems like were saying I should be filtering out or otherwise property excluding
any false properties that are returned. Please correct if I have this wrong.
I currently do filtering / exclusion on the result of expansion but seems like I
should be doing filtering on baseline output too if we don't do the expansion
just in case baseline would return a false property for some reason.
>
> >
> > Since baseline will just report a base cpu and its minimal feature set... I wonder if it
> > makes sense for libvirt to just report the features resulting from the baseline command
> > instead of later calling expansion?
>
> Yes it does and the docs say:
>
> "Baseline two CPU models, creating a compatible third model. The created
> model will always be a static, migration-safe CPU model (see "static"
> CPU model expansion for details)"
>
Here is my understanding:
1) query-cpu-model-baseline will only return migratable properties.
2) query-cpu-model-expansion on S390 only returns migratable properties.
In fact, here is what happens if you ask S390 for non-migratable features too:
{"execute":"query-cpu-model-expansion",
"arguments":{"type":"static","model":{"name":"host","props":{"migratable":false}}},"id":"libvirt-45"}
Line [{"id": "libvirt-45", "error": {"class": "GenericError", "desc": "Property '.migratable' not found"}}]
3) There seem to be two usecases for expansion
A/ Enumrate migratable properties in the baseline model
B/ Enumerate both migratable and nonmigratable props in baseline model.
B/ Doesn't work on S390 but A/ does. Here is what A/ looks like:
{"execute":"query-cpu-model-baseline",
"arguments":{"modela":{"name":"z13-base"},"modelb":{"name":"z13-base"}},
"id":"libvirt-4"}
Line [{"return": {"model": {"name": "z13-base"}}, "id": "libvirt-4"}]
***
{"execute":"query-cpu-model-expansion",
"arguments":{"type":"full","model":{"name":"z13-base"}},"id":"libvirt-5"}
Line [{"return": {"model": {"name": "z13-base", "props": {"pfmfi": false,
"exrl": true, "stfle45": true, "cmma": false, "dateh2": true, "aen": false,
"gen13ptff": true, "dateh": true, "cmmnt": false, "iacc2": true, "parseh": true,
"csst": true, "idter": false, "idtes": true, "msa": true, "aefsi": false,
"hpma2": false, "csst2": true, "csske": true, "mepoch": false, "msa8": false,
"msa7": false, "msa6": false, "msa5": false, "msa4": false, "msa3": false,
"msa2": false, "msa1": false, "sthyi": false, "stckf": true, "stfle": true,
"edat": false, "etf3": true, "etf2": true, "hfpm": true, "ri": false, "edat2":
false, "hfpue": true, "dfp": true, "mvcos": true, "sprogp": true, "sigpif":
false, "ldisphp": true, "vx": false, "ipter": false, "emon": true, "cei": false,
"cmpsceh": true, "ginste": true, "dfppc": true, "dfpzc": true, "dfphp": true,
"stfle49": true, "mepochptff": false, "opc": false, "asnlxr": true, "gpereh":
false, "minste2": false, "vxeh": false, "vxpd": false, "esop": false, "ectg":
true, "ib": false, "siif": false, "tsi": false, "tpei": false, "esan3": true,
"fpe": true, "ibs": false, "zarch": true, "stfle53": true, "sief2": false,
"eimm": true, "iep": false, "irbm": false, "srs": true, "kss": false, "cte":
false, "ais": false, "fpseh": true, "ltlbc": true, "ldisp": true, "bpb": false,
"64bscao": false, "ctop": false, "gs": false, "sema": false, "etf3eh": true,
"etf2eh": true, "eec": false, "ppa15": false, "zpci": false, "nonqks": true,
"sea_esop2": false, "pfpo": true, "te": false, "cmm": false, "tods": true,
"plo": true, "gsls": false, "skey": false}}}, "id": "libvirt-5"}]
> >
> >>>
> >>> Most of the code you added in this patch is indented by three spaces
> >>> while we use four spaces in libvirt.
> >>>
> >>>> ---
> >>>> src/qemu/qemu_driver.c | 74 +++++++++++++++++++++++++++++++++++++-----
> >>>> 1 file changed, 65 insertions(+), 9 deletions(-)
> >>>>
> >>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> >>>> index 9a35e04a85..6c6107f077 100644
> >>>> --- a/src/qemu/qemu_driver.c
> >>>> +++ b/src/qemu/qemu_driver.c
> >>>> @@ -13400,10 +13400,13 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
> >>>> virArch arch;
> >>>> virDomainVirtType virttype;
> >>>> virDomainCapsCPUModelsPtr cpuModels;
> >>>> - bool migratable;
> >>>> + bool migratable_only;
> >>>
> >>> Why? The bool says the user specified
> >>> VIR_CONNECT_BASELINE_CPU_MIGRATABLE which means they want a migratable
> >>> CPU back. What does the "_only" part mean? This API does not return
> >>> several CPUs, it only returns a single one and it's either migratable or
> >>> not.
> >>>
> >>>> virCPUDefPtr cpu = NULL;
> >>>> char *cpustr = NULL;
> >>>> char **features = NULL;
> >>>> + virQEMUCapsInitQMPCommandPtr cmd = NULL;
> >>>> + bool forceTCG = false;
> >>>> + qemuMonitorCPUModelInfoPtr modelInfo = NULL;
> >>>>
> >>>> virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES |
> >>>> VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL);
> >>>> @@ -13411,8 +13414,6 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
> >>>> if (virConnectBaselineHypervisorCPUEnsureACL(conn) < 0)
> >>>> goto cleanup;
> >>>>
> >>>> - migratable = !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE);
> >>>> -
> >>>> if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_AUTO)))
> >>>> goto cleanup;
> >>>>
> >>>> @@ -13425,6 +13426,19 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
> >>>> if (!qemuCaps)
> >>>> goto cleanup;
> >>>>
> >>>> + /* QEMU can enumerate non-migratable cpu model features for some archs like x86
> >>>> + * migratable_only == true: ask for and include only migratable features
> >>>> + * migratable_only == false: ask for and include all features
> >>>> + */
> >>>> + migratable_only = !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE);
> >>>> +
> >>>> + if (ARCH_IS_S390(arch)) {
> >>>> + /* QEMU for S390 arch only enumerates migratable features
> >>>> + * No reason to explicitly ask QEMU for or include non-migratable features
> >>>> + */
> >>>> + migratable_only = true;
> >>>> + }
> >>>> +
> >>>
> >>> And what if they come up with some features which are not migratable in
> >>> the future? I don't think there's any reason for this API to mess with
> >>> the value. The code should just provide the same CPU in both cases for
> >>> s390.
> >>
> >> s390x usually only provides features if they are migratable. Could it
> >> happen it the future that we have something that cannot be migrated?
> >> Possible but very very unlikely. We cared about migration (even for
> >> nested support) right from the beginning. As of now, we have no features
> >> that are supported by QEMU that cannot be migrated - in contrast to e.g.
> >> x86 (e.g. VMX/SVM). Every new feature has to be whitelisted in QEMU -
> >> and we only allow to do so if migration is in place for it.
> >>
A problem here is if I set migratable_only (or migratable) to false then
property "migratable":false is added to query-cpu-model-expansion.
If X86 && model "name":"host" (limted of names) you get a successfull result.
For all other archs and specific model names like (z13, IvyBridge) you get this:
Line [{"id": "libvirt-45","error": {"class": "GenericError", "desc": "Property '.migratable' not found"}}]
So unless something changes on the QEMU side you get nothing if you try to get
query-cpu-model-expansion to enumerate non-migratable features outside the X86 +
name: host type of usecases.
> >
> > You're a gold mine on this kind of information. I may have to pester you about some
> > CPU model related stuff in the future :)
>
> Sure, just CC or ask me and I'm happy to help.
>
>
> --
>
> Thanks,
>
> David / dhildenb
More information about the libvir-list
mailing list