On Thu, Jul 07, 2016 at 13:47:28 +0100, Daniel P. Berrange wrote:

On Thu, Jul 07, 2016 at 02:37:39PM +0200, Cedric Bosdonnat wrote:

...

Hi Jiri, all,

As I mentioned earlier today on IRC, I have started implementing CPU configuration support for libxl driver.

libxl expects a string definition the state of all the feature (easy to create), but also the numeric values for the CPU family and model. virCPUDef holds the name as a string, but not the numeric values though there are stored in the cpu_map.xml.

Yeah, the family/model values are currently only used for better host CPU detection so they are not exposed outside the cpu driver.

...

For what libxl expects, search for 'cpuid=' in this man page: http://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html

Oh, I was about to suggest the cpuEncode API to translate virCPUDef into CPUID data, but now I see libxl added a new syntax which does not require messing with raw CPUID data, which is nice.

...

Any idea how to get those values from the libxl driver?

I'd suggest just extending the virCPUDef to hold them, and populating it from the cpu_map.xml

This is an option, but if model/family is stored in virCPUDef, which is just an internal representation of the <cpu> XML element, we should probably format and parse the values. But doing so would mean users can set them in domain XML, which I believe is not something we want to allow. So either we will not add any formatting/parsing code for family and model breaking the relation between *Def and XML elements, or we need to come up with a different solution... Another option would be to add a virCPUGetSignature API which would take a virCPUDefPtr and return the corresponding family and model values. Jirka

On Thu, Jul 07, 2016 at 15:02:05 +0100, Daniel P. Berrange wrote:

On Thu, Jul 07, 2016 at 03:46:41PM +0200, Jiri Denemark wrote:

...

On Thu, Jul 07, 2016 at 13:47:28 +0100, Daniel P. Berrange wrote:

...

On Thu, Jul 07, 2016 at 02:37:39PM +0200, Cedric Bosdonnat wrote:

...

Hi Jiri, all,

As I mentioned earlier today on IRC, I have started implementing CPU configuration support for libxl driver.

libxl expects a string definition the state of all the feature (easy to create), but also the numeric values for the CPU family and model. virCPUDef holds the name as a string, but not the numeric values though there are stored in the cpu_map.xml.

Yeah, the family/model values are currently only used for better host CPU detection so they are not exposed outside the cpu driver.

...

...

For what libxl expects, search for 'cpuid=' in this man page: http://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html

Oh, I was about to suggest the cpuEncode API to translate virCPUDef into CPUID data, but now I see libxl added a new syntax which does not require messing with raw CPUID data, which is nice.

...

...

Any idea how to get those values from the libxl driver?

I'd suggest just extending the virCPUDef to hold them, and populating it from the cpu_map.xml

This is an option, but if model/family is stored in virCPUDef, which is just an internal representation of the <cpu> XML element, we should probably format and parse the values. But doing so would mean users can set them in domain XML, which I believe is not something we want to allow. So either we will not add any formatting/parsing code for family and model breaking the relation between *Def and XML elements, or we need to come up with a different solution...

I don't see a problem with allowing the user to set these in the domain XML in general, particularly since Xen is explicitly making them settable by users. The fact that QEMU refuses to allow us to set fine grained CPU model data is just a restriction we have to live with in the QEMU driver.

Well, QEMU allows a user to select a CPU model by name while libxl uses family/model numbers directly. Since our XML definition already includes a model name (which implies the corresponding family/model numbers), adding the raw family/model numbers sounds somewhat redundant (although it provides more control) to me and we need to deal with this redundancy. We'd need to clearly document what happens when the provided family/model numbers do not match the CPU model name (even if the documentation would say the result is undefined and either the CPU model name or family/model numbers may be just ignored depending on the hypervisor). In general, I think we should discourage users from using the family/model numbers directly (unless they know what they are doing) and let libvirt translate the model name into family/model numbers if needed. If we go this way, libxl will be able to use a new virCPUUpdate[1] internal API to fill in the missing family/model numbers in the CPU definition provided in domain XML. [1] it's not even on the mailing list, but you can look at https://gitlab.com/jirkade/libvirt/commits/cpu-probing -- the branch is most likely broken and will not even compile in its current state, but you can get the idea about what it may going to look like once I send it for review on this list. Jirka

Hello Joao! On Thu, 2016-07-07 at 16:20 +0100, Joao Martins wrote:

FYI I too am working on guest <cpu> config work and have an RFC wrt to libvirt host cpu detection. Perhaps we could joint work on this?

I started looking at the host cpu detection yesterday, but it seems the libxl hw_map bits aren't too easy to understand. But sure, if we can join efforts that would be good. I also couldn't find your RFC in the mailing list archives. Do you have code available somewhere? Joining efforts without stepping on each other's toes isn't that easy ;)

As you know, there are two forms to represent this info in xen libxl cpuid: 1) is the xend format which provides the input leaf and output registers values raw CPUID data, and 2) the libxl format. Though while man page depicts what the feature names are - the correspondent input, output registers, and feature names aren't really exported by libxl headers and will have to be replicated in libvirt and adjusted to API changes. So probably xend format would be better suited - which can possible accomodate other leafs than those described by the libxl format? There is also a special case for 'vmx' where we need to set libxl-side nestedhvm attribute to true (on HVM guests).

Sure the xend format would be more flexible, however it's fairly easy to map the libvirt names to the xen ones: those are mostly the same. This also helps checking for the xen unsupported flags. Doing the mapping manually helped me see that libxl has CPU features flags that libvirt doesn't have (maybe we'll have to add them). And anyway, we will have to adjust the libvirt feature set if there are more features supported in future libxl/xen versions as libvirt's <cpu> doesn't hold the register bits but strings.

There is one issue wrt to guest cpu features though: currently Xen and libxl don't provide a way for libxl callers to fetch the cpuid policy that the guest will actually see at boot. So, when applying a cpuid policy the lower toolstack layer (libxc) will validate may possibly disable or change some bits. Some features are incompatible depending on guest type or feature dependencies whether it is a PV or HVM guest or HAP enabled etc, alongside host supported levelling capabilities (e.g. CPUID Faulting support on Intel boxes >= IvyBridge give PV guests CPUID being controlled like HVM guests, or otherwise warn libvirt user if not supported). My point here is that even if we set the CPU features we would be blindly doing so with no assurance that the domain will see those enabled. CPU topology too isn't specifiable under libxl or Xen. But I guess this could be solved as part 2 (which I intend to tackle on) and warn the user when it's not possible to double-check cpu features info - which would be for all supported versions so far.

The warning thing sounds good to me since we don't have a tight control on what the guest will see.

Xen 4.7 had a great deal of improvement in this area, and future versions will have more things coming in. (http://xenbits.xen.org/docs/4.7-testing/features/feature-levelling.html)

Good to see things will improve soon ;) -- Cedric

Hey Joao, On Fri, 2016-07-08 at 16:22 +0100, Joao Martins wrote:

Indeed, libxl hw_caps changes from Xen 4.4-4.6 to Xen 4.7 - and it wasn't somewhat stabilized until the latter, although field is provided from even earlier xen versions. So the quirk there is to handle that, other than that is plain cpuid leaves output.

Hum, hopefully you're much more aware of the libxl things than me ;) I didn't notice the hw_caps changed.

Ah, sure! I don't have a public tree available these days - let me post that shortly on the list for discussion and we could continue from there (will CC you on the series).

Thanks for the patch series. I'll have a look at it.

Hmm, mapping would be fairly easy indeed. I was more concerned about duplication in libxl driver - specially as these feature names in libxl are private (as seen from a library perspective and will have to piggy back what it is on libxl_cpuid.c, even though mnemonics are similar). While having to keep up with changes between libxl and cpu_map.xml in between. But perhaps may be that's less of a concern as cpu_map.xml is made thinking for that sort of adjustments?

For sure using the bits would avoid duplicating the feature strings in the libxl driver, though I find them much more readable... but that point doesn't weight that much in the balance.

The register bits can be fetched as far as I could see from the APIs (see below). I was more thinking along the lines of knowing the register bits from each feature name somehow and just construct the string accordingly (potentially leading to a smaller amounts of code?),

Getting the register bits for each feature name from the cpu_map is something Jiri could surely help us with ;)

and then support xl format whenever everything is exported. Or you think it's a less preferable way to how it should be handled in libvirt? On the other hand the xend syntax might be more cumbersome when setting things that we shouldn't, even though is up to the libxl cpuid policy to validate.

We will still have some sort of prevalidation on the libxl driver side I think, whatever the format we use: this would help error reporting by failing earlier.

Irrespective of the format we decide to use - we need somehow to convert a guest <cpu> definition a complete list of CPUID/featurename data or pre-store id in cpu map definitions in libxl cfg object. We can actually get the CPUID part through cpuGuestData() (the input/ouput registers). Say for example on running on a Broadwell processor and we use this cpu definition (the example doesn't make a lot of sense here, it's just for the purpose of pointing out the issue):

<cpu match='exact'> <model>Nehalem</model> <feature policy='require' name='avx'/> </cpu>

The CPUID registers part coming from cpuGuestData returns me this:

libxlMakeCPU:318 : CPUID[0] input=(EAX=1,ECX=0) :EAX=000306d0, EBX=00000000 ECX=10982201 EDX=078bfbfd libxlMakeCPU:318 : CPUID[1] input=(EAX=7,ECX=0) :EAX=00000000, EBX=00000000 ECX=00000000 EDX=00000000 libxlMakeCPU:318 : CPUID[2] input=(EAX=d,ECX=1) :EAX=00000000, EBX=00000000 ECX=00000000 EDX=00000000 libxlMakeCPU:318 : CPUID[3] input=(EAX=80000001,ECX=0) :EAX=00000000, EBX=00000000 ECX=00000001 EDX=20100800 libxlMakeCPU:318 : CPUID[4] input=(EAX=80000007,ECX=0) :EAX=00000000, EBX=00000000 ECX=00000000 EDX=00000000

So we would need to set too all features corresponding to Nehalem model as described by cpu_map.xml, plus setting up AVX (in the context of this example). libxl keeps no info of features associated with each model/family so there wouldn't be a shortcut akin to QEMU where IIUC libvirt feeds qemu with the expected models and features that it will use. Thoughts?

Not sure what is the best way to do it... may be Jiri knows. -- Cedric