Re: [Xen-devel] [PATCH v11 1/2] x86/viridian: Re-purpose the HVM parameter to be a feature mask

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-09-22 at 17:48 +0100, Ian Jackson wrote:
> Paul Durrant writes ("[PATCH v11 1/2] x86/viridian: Re-purpose the HVM 
> parameter to be a feature mask"):
> > The following commits introduced the time reference counter MSR and
> > TSC/APIC frequency MSRs into the viridian feature set respectively:
> 
> The libxl and libxc parts of this, and the corresponding docs, look
> good to me, apart from one part that I would like Ian C to comment on
> - see below.
> 
> If you had separated that out into a patch of its own, I would have
> given a tools ack to this one :-).

AFAICT only the max_val is actually used, min_val I suppose is just for
completeness.

Can we not just either include an explicit MAX case in the enum itself
or provide a libxl function to allocate a suitably sized bitmap like we
do with libxl_cpu_bitmap_alloc (which lets us keep all this internal so
we don't need to think about the API implications too hard).

> +libxl_viridian_enlightenment = Enumeration("viridian_enlightenment",
> [
> +    (0, "base"),
> +    (1, "freq"),
> +    ])

Does this need to be kept in sync with the hvm.h values?.

> +/* Base+Freq viridian feature sets:
> + *
> + * - Hypercall MSRs (HV_X64_MSR_GUEST_OS_ID and HV_X64_MSR_HYPERCALL)
> + * - APIC access MSRs (HV_X64_MSR_EOI, HV_X64_MSR_ICR and HV_X64_MSR_TPR)
> + * - Virtual Processor index MSR (HV_X64_MSR_VP_INDEX)
> + * - Timer frequency MSRs (HV_X64_MSR_TSC_FREQUENCY and
> + *   HV_X64_MSR_APIC_FREQUENCY)
> + */
> +#define _HVMPV_base_freq 0
> +#define HVMPV_base_freq  (1 << _HVMPV_base_freq)
> +
> +/* Feature set modifications */
> +
> +/* Disable timer frequency MSRs (HV_X64_MSR_TSC_FREQUENCY and
> + * HV_X64_MSR_APIC_FREQUENCY).
> + * This modification restores the viridian feature set to the
> + * original 'base' set exposed in releases prior to Xen 4.4.
> + */
> +#define _HVMPV_no_freq 1
> +#define HVMPV_no_freq  (1 << _HVMPV_no_freq)
> +
> +#define HVMPV_feature_mask (HVMPV_base_freq|HVMPV_no_freq)

Given that these have no backwards compatiblity reqs can't we avoid the
negative feature at the hypercall and DTRT in libxl?

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel