[libvirt] [PATCH v3.1 3/6] libxl: add support for CPUID features policy

[Marek Marczykowski-Górecki]

On Wed, Jan 03, 2018 at 05:00:10PM -0700, Jim Fehlig wrote:
> On 12/19/2017 06:19 AM, Joao Martins wrote:
> > On 12/13/2017 07:09 PM, Marek Marczykowski-Górecki wrote:
> > > +/*
> > > + * Translate CPU feature name from libvirt to libxl (from_libxl=false) or from
> > > + * libxl to libvirt (from_libxl=true).
> > > + */
> > > +const char *
> > > +xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
> > > +{
> > > +    static const char *translation_table[][2] = {
> > > +        /* libvirt name, libxl name */
> > > +        { "cx16", "cmpxchg16" },
> > > +        { "cid", "cntxid" },
> > > +        { "ds_cpl", "dscpl" },
> > > +        { "pclmuldq", "pclmulqdq" },
> > > +        { "pni", "sse3" },
> > > +        { "ht", "htt" },
> > > +        { "pn", "psn" },
> > > +        { "clflush", "clfsh" },
> > > +        { "sep", "sysenter" },
> > > +        { "cx8", "cmpxchg8" },
> > > +        { "nodeid_msr", "nodeid" },
> > > +        { "cr8legacy", "altmovcr8" },
> > > +        { "lahf_lm", "lahfsahf" },
> > > +        { "cmp_legacy", "cmplegacy" },
> > > +        { "fxsr_opt", "ffxsr" },
> > > +        { "pdpe1gb", "page1gb" },
> > > +    };
> > > +    size_t i;
> > > +
> > > +    for (i = 0; i < ARRAY_CARDINALITY(translation_table); i++)
> > > +        if (STREQ(translation_table[i][from_libxl], feature_name))
> > > +            return translation_table[i][!from_libxl];
> > > +    return feature_name;
> > > +}
> > > +
> > 
> > Cc'ing Jim as he may have some thoughts on the direction of this.
> > 
> > What happens if the admin decides to change /usr/share/libvirt/cpu_map.xml?
> 
> Is changing existing content likely/practical?
> 
> > Also you can also add new leafs/features to cpu_map.xml
> 
> Right. And I suppose the table in libxl_cpuid.c can grow too. And in cases
> where they differ we'd need to update the static table, which we'll probably
> only remember to do when receiving bug reports. So I like the idea of making
> this more dynamic, but I apparently don't have enough brain power today to
> understand your suggestion :-).
>
> > Maybe an idea instead is to have a table with all leafs that libxl has hardcoded
> > (i.e. cpuid_flags array on
> > http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl_cpuid.c;hb=HEAD#l92).
> 
> Where would this table reside? In src/cpu/?
> 
> > Then adding a new cpu driver routine to look for cpuid data based on feature
> > name (and the reverse too). Finally you would populate this translation table at
> > driver load time, or you could even get away without a translation table (but
> > would be a little more inefficient?).
> 
> I'm having difficulty understanding how this provides a dynamic solution.
> Wouldn't the table you mention above need extended anytime the hardcoded one
> in libxl was extended? Which would probably only happen after receiving a
> bug report? :-)

Probably... And worse thing about such table is it needs to contain all
entries from said libxl_cpuid.c. My solution require only listing
entries with mismatching names.

Alternative would be to not use "new libxl syntax", but "old xend
syntax" (which is still supported by libxl). The later allow to list
specific bits instead of feature names. That was implemented in v1 of
this patch series[1]... The problem with that approach is currently libvirt
does not expose API for lookup of individual features, but only to
construct full CPU and then enumerate its CPUID. That means it isn't
feasible for the current approach (mode='host-passthrough', then
enable/disable individual features). See discussion on v1.
Adding such API to libvirt cpu driver is beyond my knowledge of libvirt
code and available time :/

> Hmm, the more I think about it, the more I convince myself that knowing
> libvirt and libxl use different names for a CPU feature is static
> information. I'm afraid I don't have any better ideas beyond Marek's neat
> trick.
> 
> Regards,
> Jim

[1] https://www.redhat.com/archives/libvir-list/2017-June/msg01292.html

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180104/40ab9fbc/attachment-0001.sig>