On 16/07/18 13:04, Jan Beulich wrote:
>>>> On 13.07.18 at 22:03, <andrew.coop...@citrix.com> wrote:
>> --- a/xen/arch/x86/sysctl.c
>> +++ b/xen/arch/x86/sysctl.c
>> @@ -31,6 +31,33 @@
>> #include <asm/psr.h>
>> #include <asm/cpuid.h>
>>
>> +const struct cpu_policy system_policies[] = {
> By the end of the series the array remains unused outside this
> source file. I'd appreciate if it was made extern only when actually
> needed, not the least because ...
>
>> + [ XEN_SYSCTL_cpu_policy_raw ] = {
>> + &raw_cpuid_policy,
>> + &raw_msr_policy,
>> + },
>> + [ XEN_SYSCTL_cpu_policy_host ] = {
>> + &host_cpuid_policy,
>> + &host_msr_policy,
>> + },
>> + [ XEN_SYSCTL_cpu_policy_pv_max ] = {
>> + &pv_max_cpuid_policy,
>> + &pv_max_msr_policy,
>> + },
>> + [ XEN_SYSCTL_cpu_policy_hvm_max ] = {
>> + &hvm_max_cpuid_policy,
>> + &hvm_max_msr_policy,
>> + },
>> + [ XEN_SYSCTL_cpu_policy_pv_default ] = {
>> + &pv_max_cpuid_policy,
>> + &pv_max_msr_policy,
>> + },
>> + [ XEN_SYSCTL_cpu_policy_hvm_default ] = {
>> + &hvm_max_cpuid_policy,
>> + &hvm_max_msr_policy,
>> + },
>> +};
> ... this does not make obvious (without consulting sysctl.h) that
> there are now holes (and hence hidden NULL pointers); this is
> perhaps already undesirable with the user of this array that the
> next patch adds.
What holes? There shouldn't be any, and gdb confirms my expectations:
(gdb) p/x system_policies
$1 = {{cpuid = 0xffff82d080474a80, msr = 0xffff82d080475960}, {cpuid =
0xffff82d080474340, msr = 0xffff82d08047595c}, {cpuid =
0xffff82d080473c00, msr = 0xffff82d080475954}, {cpuid =
0xffff82d0804734c0, msr = 0xffff82d080475958}, {cpuid =
0xffff82d080473c00, msr = 0xffff82d080475954}, {cpuid =
0xffff82d0804734c0, msr = 0xffff82d080475958}}
>
> With "static" added and the "extern" dropped from the header
> Acked-by: Jan Beulich <jbeul...@suse.com>
I'm not going to waste even more time by committing something which is
wrong, and having to undo it again in a later patch.
The user, DOMCTL_set_cpu_policy, is deferred from this series because it
is still under development, but there is absolutely no question that
this array needs to be externally accessible.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
