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

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...). 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? :-)

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