XENPF_get_cpuinfo should init the flags output field rather than only
modify it.
XENPF_cpu_online must check for the input CPU number to be in range.
XENPF_cpu_offline must also do that, and should also reject attempts to
offline CPU 0 (this fails in cpu_down() too, but preventing this here
appears more correct given that the code here calls
continue_hypercall_on_cpu(0, ...), which would be flawed if cpu_down()
would ever allow bringing down CPU 0 (and a distinct error code is
easier to deal with when debugging issues).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -449,13 +449,14 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
if ( (g_info->xen_cpuid >= nr_cpu_ids) ||
!cpu_present(g_info->xen_cpuid) )
{
- g_info->flags |= XEN_PCPU_FLAGS_INVALID;
+ g_info->flags = XEN_PCPU_FLAGS_INVALID;
}
else
{
g_info->apic_id = x86_cpu_to_apicid[g_info->xen_cpuid];
g_info->acpi_id = acpi_get_processor_id(g_info->xen_cpuid);
ASSERT(g_info->apic_id != BAD_APICID);
+ g_info->flags = 0;
if (cpu_online(g_info->xen_cpuid))
g_info->flags |= XEN_PCPU_FLAGS_ONLINE;
}
@@ -472,7 +473,7 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
{
int cpu = op->u.cpu_ol.cpuid;
- if ( !cpu_present(cpu) )
+ if ( cpu >= nr_cpu_ids || !cpu_present(cpu) )
{
ret = -EINVAL;
break;
@@ -493,7 +494,13 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
{
int cpu = op->u.cpu_ol.cpuid;
- if ( !cpu_present(cpu) )
+ if ( cpu == 0 )
+ {
+ ret = -EOPNOTSUPP;
+ break;
+ }
+
+ if ( cpu >= nr_cpu_ids || !cpu_present(cpu) )
{
ret = -EINVAL;
break;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2011-Nov-24 16:49 UTC
Re: [PATCH] x86: small fixes to pcpu platform op handling
On 24/11/2011 16:27, "Jan Beulich" <JBeulich@suse.com> wrote:> XENPF_get_cpuinfo should init the flags output field rather than only > modify it. > > XENPF_cpu_online must check for the input CPU number to be in range. > > XENPF_cpu_offline must also do that, and should also reject attempts to > offline CPU 0 (this fails in cpu_down() too, but preventing this here > appears more correct given that the code here calls > continue_hypercall_on_cpu(0, ...), which would be flawed if cpu_down() > would ever allow bringing down CPU 0 (and a distinct error code is > easier to deal with when debugging issues). > > Signed-off-by: Jan Beulich <jbeulich@suse.com>Acked-by: Keir Fraser <keir@xen.org>> --- a/xen/arch/x86/platform_hypercall.c > +++ b/xen/arch/x86/platform_hypercall.c > @@ -449,13 +449,14 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe > if ( (g_info->xen_cpuid >= nr_cpu_ids) || > !cpu_present(g_info->xen_cpuid) ) > { > - g_info->flags |= XEN_PCPU_FLAGS_INVALID; > + g_info->flags = XEN_PCPU_FLAGS_INVALID; > } > else > { > g_info->apic_id = x86_cpu_to_apicid[g_info->xen_cpuid]; > g_info->acpi_id = acpi_get_processor_id(g_info->xen_cpuid); > ASSERT(g_info->apic_id != BAD_APICID); > + g_info->flags = 0; > if (cpu_online(g_info->xen_cpuid)) > g_info->flags |= XEN_PCPU_FLAGS_ONLINE; > } > @@ -472,7 +473,7 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe > { > int cpu = op->u.cpu_ol.cpuid; > > - if ( !cpu_present(cpu) ) > + if ( cpu >= nr_cpu_ids || !cpu_present(cpu) ) > { > ret = -EINVAL; > break; > @@ -493,7 +494,13 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe > { > int cpu = op->u.cpu_ol.cpuid; > > - if ( !cpu_present(cpu) ) > + if ( cpu == 0 ) > + { > + ret = -EOPNOTSUPP; > + break; > + } > + > + if ( cpu >= nr_cpu_ids || !cpu_present(cpu) ) > { > ret = -EINVAL; > break; > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel