Re: [Xen-devel] [PATCH 3/6] x86/vpmu: Remove core2_no_vpmu_ops

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.11.16 at 13:31, <andrew.cooper3@xxxxxxxxxx> wrote:
> core2_no_vpmu_ops exists solely to work around the default-leaking of 
> CPUID/MSR
> values in Xen.
> 
> With CPUID handling removed from arch_vpmu_ops, the RDMSR handling is the last
> remaining hook.  Since core2_no_vpmu_ops's introduction in c/s 25250ed7 "vpmu
> intel: Add cpuid handling when vpmu disabled", a lot of work has been done and
> the nop path in vpmu_do_msr() now suffices.
> 
> vpmu_do_msr() also falls into the nop path for un-configured or unprivileged
> domains, which enables the removal the duplicate logic in priv_op_read_msr().
> 
> Finally, make all arch_vpmu_ops structures const as they are never modified,
> and make them static as they are not referred to externally.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Fundamentally the patch is fine and could have my ack, but I expect
it to change should patch 2 be deferred. Furthermore ...

> --- a/xen/arch/x86/cpu/vpmu_amd.c
> +++ b/xen/arch/x86/cpu/vpmu_amd.c
> @@ -488,7 +488,7 @@ static void amd_vpmu_dump(const struct vcpu *v)
>      }
>  }
>  
> -struct arch_vpmu_ops amd_vpmu_ops = {
> +const static struct arch_vpmu_ops amd_vpmu_ops = {

... the conventional ordering is storage class first; I'd find it especially
confusing if someone didn't put "typedef" first, and "typedef" too is a
storage class specifier.

Jan


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