Re: [libvirt] [PATCH] libxl: support <hap> feature

On 02/04/2016 01:41 AM, Jim Fehlig wrote: > On 02/03/2016 02:20 PM, Joao Martins wrote: >> On 02/03/2016 04:12 AM, Jim Fehlig wrote: >>> Even though the libxl driver advertises <hap> in capabilities, >>> it is ignored when set in domXML. Enable hap in the >>> libxl_domain_create_info struct when <hap> is specified in domXML. >>> >>> The xm/xl <-> domXML parser/formatter already supports hap but >>> lacked a test with hap enabled. Add a hap test. > [...] >> FWIW looks good and also: >> >> Tested-by: Joao Martins <joao.m.martins(a)oracle.com&gt; > Thanks for reviewing/testing the patch! But after playing with the patch a bit > more, I'm considering dropping it :-/. > > Before the patch, <hap> was ignored by the libxl driver, which left the 'hap' > field of libxl_domain_create_info struct unset (= LIBXL__DEFBOOL_DEFAULT). For > HVM guests, libxl will enable hap when libxl_domain_create_info.hap == > LIBXL__DEFBOOL_DEFAULT. And using hap is good, since using shadow page table is > less efficient. The downside is there is no way to turn hap off. > > After the patch it is possible to turn hap on and off via presence or absence of > <hap>. However, there is a *lot* of existing config without <hap> that currently > reaps the benefits of hap nonetheless. After patch, that config would require > adding <hap>, else shadow page table will be used. (Note that in xm and xl > config, hap is enabled in the absence of 'hap='.) > > I'm starting to question whether this feature should even be exposed. I think > the only use case is debugging. E.g. turn hap off if suspected hardware bug. > Otherwise you would certainly want the hypervisor to use hap if it is able to do > so. What do you think? > Hm, good point. There is another field for VIR_TRISTATE which represents when it's absent (VIR_TRISTATE_SWITCH_ABSENT). So perhaps we would leave the default (= LIBXL_DEFBOOL_DEFAULT) if it's absent, and only set hap if the user explicitly switches on/off?

I am not sure if the usecase is just debug, but given there are certain performance differences compared to shadow paging (depending on the HVM guest workload) it might be nice to allow the administrator to choose it.