>>> On 16.12.15 at 22:24, <andrew.coop...@citrix.com> wrote:
> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -300,6 +300,9 @@ static void __init noinline amd_init_levelling(void)
> cpumask_defaults._6c &= (~0ULL << 32);
> cpumask_defaults._6c |= ecx;
> }
> +
> + if (levelling_caps)
> + ctxt_switch_levelling = amd_ctxt_switch_levelling;
> }
Indentation.
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -86,6 +86,13 @@ static const struct cpu_dev default_cpu = {
> };
> static const struct cpu_dev *this_cpu = &default_cpu;
>
> +void default_ctxt_switch_levelling(const struct domain *nextd)
static
> +{
> + /* Nop */
> +}
> +void (*ctxt_switch_levelling)(const struct domain *nextd) __read_mostly =
> + default_ctxt_switch_levelling;
While current and past gcc may accept (and honor) this placement of
the __read_mostly annotation, I think it is wrong from a strict language
syntax pov. Imo it instead ought to be
void (*__read_mostly ctxt_switch_levelling)(const struct domain *nextd) =
Also - indentation again.
> @@ -145,6 +145,13 @@ void intel_ctxt_switch_levelling(const struct domain
> *nextd)
> struct cpumasks *these_masks = &this_cpu(cpumasks);
> const struct cpumasks *masks = &cpumask_defaults;
>
> + if (cpu_has_cpuid_faulting) {
> + set_cpuid_faulting(nextd && is_pv_domain(nextd) &&
> + !is_control_domain(nextd) &&
> + !is_hardware_domain(nextd));
> + return;
> + }
Considering that you don't even probe the masking MSRs, this seems
inconsistent with your "always level the entire system" choice. As said
I'm opposed to that (i.e. the code here is fine), but at the very least
things ought to be consistent.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
