[PATCH 0/4] Reduce the use of MAX_VIRT_CPUS in tree

[PATCH 0/4] Reduce the use of MAX_VIRT_CPUS in tree

[Andrew Cooper]

MAX_VIRT_CPUS used to be an appropriate upper bound for vcpus when it was the
length of the fixed-size vcpus array in a struct domain.  When the vcpu array
became dynamically allocated in Xen 4.0, it ceased to be an appropriate bound.

Andrew Cooper (4):
  xen/domain: Introduce domain_max_vcpus() helper and implement per arch
  xen/evtchn: Reduce the size of the poll_mask where possible
  sched/arinc653: Remove MAX_VIRT_CPUS bounds check
  xen/many: Drop redundant MAX_VIRT_CPUS bounds checks

 xen/arch/arm/domain.c        |    5 +++++
 xen/arch/arm/vpsci.c         |    3 ---
 xen/arch/x86/domain.c        |    9 +++++++++
 xen/arch/x86/domctl.c        |    4 ----
 xen/common/compat/domain.c   |    3 ---
 xen/common/domain.c          |    3 ---
 xen/common/domctl.c          |    3 +--
 xen/common/event_channel.c   |    4 ++--
 xen/common/sched_arinc653.c  |    5 ++---
 xen/include/asm-arm/config.h |    1 -
 xen/include/asm-x86/config.h |    3 ---
 xen/include/xen/domain.h     |    3 +++
 12 files changed, 22 insertions(+), 24 deletions(-)

-- 
1.7.10.4

[PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch

[Andrew Cooper]

This allows the common XEN_DOMCTL_max_vcpus handler to loose some x86-specific
architecture knowledge.

It turns out that Xen had the same magic number twice in-tree with different
names (HVM_MAX_VCPUS and MAX_HVM_VCPUS).  This removes all use of
MAX_HVM_VCPUS, and x86 uses HVM_MAX_VCPUS from the public headers.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
CC: Tim Deegan <tim@xen.org>
---
 xen/arch/arm/domain.c        |    5 +++++
 xen/arch/x86/domain.c        |    9 +++++++++
 xen/common/domctl.c          |    3 +--
 xen/include/asm-arm/config.h |    1 -
 xen/include/asm-x86/config.h |    3 ---
 xen/include/xen/domain.h     |    3 +++
 6 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index cfc7ab4..0b09f02 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -400,6 +400,11 @@ void startup_cpu_idle_loop(void)
     reset_stack_and_jump(idle_loop);
 }
 
+unsigned int domain_max_vcpus(const struct domain *d)
+{
+    return MAX_VIRT_CPUS;
+}
+
 struct domain *alloc_domain_struct(void)
 {
     struct domain *d;
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index c8832c6..9d3e01a 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -37,6 +37,7 @@
 #include <xen/wait.h>
 #include <xen/guest_access.h>
 #include <public/sysctl.h>
+#include <public/hvm/hvm_info_table.h>
 #include <asm/regs.h>
 #include <asm/mc146818rtc.h>
 #include <asm/system.h>
@@ -204,6 +205,14 @@ smap_check_policy_t smap_policy_change(struct vcpu *v,
     return old_policy;
 }
 
+unsigned int domain_max_vcpus(const struct domain *d)
+{
+    if ( is_hvm_domain(d) )
+        return HVM_MAX_VCPUS;
+    else
+        return MAX_VIRT_CPUS;
+}
+
 /*
  * The hole may be at or above the 44-bit boundary, so we need to determine
  * the total bit count until reaching 32 significant (not squashed out) bits
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index ee578c0..33ecd45 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -608,8 +608,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
 
         ret = -EINVAL;
         if ( (d == current->domain) || /* no domain_pause() */
-             (max > MAX_VIRT_CPUS) ||
-             (is_hvm_domain(d) && (max > MAX_HVM_VCPUS)) )
+             (max > domain_max_vcpus(d)) )
             break;
 
         /* Until Xenoprof can dynamically grow its vcpu-s array... */
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 1b5a842..3b23e05 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -48,7 +48,6 @@
 #endif
 
 #define MAX_VIRT_CPUS 8
-#define MAX_HVM_VCPUS MAX_VIRT_CPUS
 
 #define asmlinkage /* Nothing needed */
 
diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
index ad52d5b..2fbd68d 100644
--- a/xen/include/asm-x86/config.h
+++ b/xen/include/asm-x86/config.h
@@ -67,9 +67,6 @@
 #define NR_CPUS 256
 #endif
 
-/* Maximum we can support with current vLAPIC ID mapping. */
-#define MAX_HVM_VCPUS 128
-
 /* Linkage for x86 */
 #define __ALIGN .align 16,0x90
 #define __ALIGN_STR ".align 16,0x90"
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 72667da..05ef1ed 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -22,6 +22,9 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info);
  * Arch-specifics.
  */
 
+/* Maximum number of vcpus supported for a specific domain. */
+unsigned int domain_max_vcpus(const struct domain *d);
+
 /* Allocate/free a domain structure. */
 struct domain *alloc_domain_struct(void);
 void free_domain_struct(struct domain *d);
-- 
1.7.10.4

Re: [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch

[Simon Rowe]

On 19/01/15 11:21, Andrew Cooper wrote:
> This allows the common XEN_DOMCTL_max_vcpus handler to loose some x86-specific
> architecture knowledge.
Sp. "lose"
>   
> +unsigned int domain_max_vcpus(const struct domain *d)
> +{
> +    return MAX_VIRT_CPUS;
> +}
> +
>

inline?

Simon

Re: [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch

[Andrew Cooper]

On 19/01/15 12:16, Simon Rowe wrote:
> On 19/01/15 11:21, Andrew Cooper wrote:
>> This allows the common XEN_DOMCTL_max_vcpus handler to loose some
>> x86-specific
>> architecture knowledge.
> Sp. "lose"
>>   +unsigned int domain_max_vcpus(const struct domain *d)
>> +{
>> +    return MAX_VIRT_CPUS;
>> +}
>> +
>>
>
> inline?
>
> Simon
>

I considered that but, while there is an asm-x86/domain.h, there is not
an asm-arm/domain.h which would be the logical location for it to live.

I can introduce an asm-arm/domain.h if the arm maintainers are happy.

~Andrew

Re: [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch

[Ian Campbell]

On Mon, 2015-01-19 at 12:52 +0000, Andrew Cooper wrote:
> I considered that but, while there is an asm-x86/domain.h, there is not
> an asm-arm/domain.h which would be the logical location for it to live.

I think you've just missed it?

$ ls -l xen/include/asm-arm/domain.h
-rw-rw-r-- 1 ianc xendev 6683 Dec 16 12:39 xen/include/asm-arm/domain.h
$

Ian.

Re: [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch

[Andrew Cooper]

On 19/01/15 12:57, Ian Campbell wrote:
> On Mon, 2015-01-19 at 12:52 +0000, Andrew Cooper wrote:
>> I considered that but, while there is an asm-x86/domain.h, there is not
>> an asm-arm/domain.h which would be the logical location for it to live.
> I think you've just missed it?
>
> $ ls -l xen/include/asm-arm/domain.h
> -rw-rw-r-- 1 ianc xendev 6683 Dec 16 12:39 xen/include/asm-arm/domain.h
> $
>
> Ian.
>

D'oh - of course.  It was cscope which told me there wasn't one.

(I really should learn - this isn't the first time)

~Andrew

Re: [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch

[Jan Beulich]

>>> On 19.01.15 at 12:21, <andrew.cooper3@citrix.com> wrote:
> +unsigned int domain_max_vcpus(const struct domain *d)
> +{
> +    if ( is_hvm_domain(d) )
> +        return HVM_MAX_VCPUS;
> +    else
> +        return MAX_VIRT_CPUS;
> +}

Together with making this inline, I think such things are better coded
using the conditional operator (or at the very least without the
redundant "else").

Jan

[PATCH v2 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch

[Andrew Cooper]

This allows the common XEN_DOMCTL_max_vcpus handler to lose some x86-specific
architecture knowledge.

It turns out that Xen had the same magic number twice in-tree with different
names (HVM_MAX_VCPUS and MAX_HVM_VCPUS).  This removes all use of
MAX_HVM_VCPUS, and x86 uses HVM_MAX_VCPUS from the public headers.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
CC: Tim Deegan <tim@xen.org>

---
v2: inline implementations in arch domain.h files

I would really have prefered to have the x86 implementation as a static inline
like the arm side, but because of the inclusion order, is_hvm_domain() is not
available in asm-x86/domain.h  I lack sufficient tuits to untangle this at the
moment.
---
 xen/common/domctl.c          |    3 +--
 xen/include/asm-arm/config.h |    1 -
 xen/include/asm-arm/domain.h |    5 +++++
 xen/include/asm-x86/config.h |    3 ---
 xen/include/asm-x86/domain.h |    3 +++
 5 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index ee578c0..33ecd45 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -608,8 +608,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
 
         ret = -EINVAL;
         if ( (d == current->domain) || /* no domain_pause() */
-             (max > MAX_VIRT_CPUS) ||
-             (is_hvm_domain(d) && (max > MAX_HVM_VCPUS)) )
+             (max > domain_max_vcpus(d)) )
             break;
 
         /* Until Xenoprof can dynamically grow its vcpu-s array... */
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 1b5a842..3b23e05 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -48,7 +48,6 @@
 #endif
 
 #define MAX_VIRT_CPUS 8
-#define MAX_HVM_VCPUS MAX_VIRT_CPUS
 
 #define asmlinkage /* Nothing needed */
 
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 8b7dd85..9018c6a 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -249,6 +249,11 @@ struct arch_vcpu
 void vcpu_show_execution_state(struct vcpu *);
 void vcpu_show_registers(const struct vcpu *);
 
+static inline unsigned int domain_max_vcpus(const struct domain *d)
+{
+    return MAX_VIRT_CPUS;
+}
+
 #endif /* __ASM_DOMAIN_H__ */
 
 /*
diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
index ad52d5b..2fbd68d 100644
--- a/xen/include/asm-x86/config.h
+++ b/xen/include/asm-x86/config.h
@@ -67,9 +67,6 @@
 #define NR_CPUS 256
 #endif
 
-/* Maximum we can support with current vLAPIC ID mapping. */
-#define MAX_HVM_VCPUS 128
-
 /* Linkage for x86 */
 #define __ALIGN .align 16,0x90
 #define __ALIGN_STR ".align 16,0x90"
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 6a77a93..b233fbc 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -9,6 +9,7 @@
 #include <asm/e820.h>
 #include <asm/mce.h>
 #include <public/vcpu.h>
+#include <public/hvm/hvm_info_table.h>
 
 #define has_32bit_shinfo(d)    ((d)->arch.has_32bit_shinfo)
 #define is_pv_32bit_domain(d)  ((d)->arch.is_32bit_pv)
@@ -527,6 +528,8 @@ void domain_cpuid(struct domain *d,
                   unsigned int  *ecx,
                   unsigned int  *edx);
 
+#define domain_max_vcpus(d) (is_hvm_domain(d) ? HVM_MAX_VCPUS : MAX_VIRT_CPUS)
+
 #endif /* __ASM_DOMAIN_H__ */
 
 /*
-- 
1.7.10.4

Re: [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch

[Ian Campbell]

On Mon, 2015-01-19 at 11:21 +0000, Andrew Cooper wrote:
> This allows the common XEN_DOMCTL_max_vcpus handler to loose some x86-specific
> architecture knowledge.
> 
> It turns out that Xen had the same magic number twice in-tree with different
> names (HVM_MAX_VCPUS and MAX_HVM_VCPUS).  This removes all use of
> MAX_HVM_VCPUS, and x86 uses HVM_MAX_VCPUS from the public headers.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>

For ARM:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

I'm assuming Jan will pickup the series for eventual commit, since the
bulk seems to be common and/or x86 related.

[PATCH 2/4] xen/evtchn: Reduce the size of the poll_mask where possible

[Andrew Cooper]

Use domain_max_vcpus(d) in preference to MAX_VIRT_CPUS when allocating the
poll mask.  This allows x86 HVM guests to have a poll mask of 128 bits rather
than 8k bits.

While changing this, use xzalloc_array() in preference to xmalloc_array() to
avoid needing the subsequent call to bitmap_zero().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 xen/common/event_channel.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 7d6de54..1c15569 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -1269,13 +1269,13 @@ int evtchn_init(struct domain *d)
     evtchn_from_port(d, 0)->state = ECS_RESERVED;
 
 #if MAX_VIRT_CPUS > BITS_PER_LONG
-    d->poll_mask = xmalloc_array(unsigned long, BITS_TO_LONGS(MAX_VIRT_CPUS));
+    d->poll_mask = xzalloc_array(
+        unsigned long, BITS_TO_LONGS(domain_max_vcpus(d)));
     if ( !d->poll_mask )
     {
         free_evtchn_bucket(d, d->evtchn);
         return -ENOMEM;
     }
-    bitmap_zero(d->poll_mask, MAX_VIRT_CPUS);
 #endif
 
     return 0;
-- 
1.7.10.4

Re: [PATCH 2/4] xen/evtchn: Reduce the size of the poll_mask where possible

[Jan Beulich]

>>> On 19.01.15 at 12:21, <andrew.cooper3@citrix.com> wrote:
> Use domain_max_vcpus(d) in preference to MAX_VIRT_CPUS when allocating the
> poll mask.  This allows x86 HVM guests to have a poll mask of 128 bits 
> rather
> than 8k bits.
> 
> While changing this, use xzalloc_array() in preference to xmalloc_array() to
> avoid needing the subsequent call to bitmap_zero().
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

[PATCH 3/4] sched/arinc653: Remove MAX_VIRT_CPUS bounds check

[Andrew Cooper]

The arinc653 interface is capable of specifying a domain in the schedule (from
the toolstack) before the domain itself exists, or is present in the cpupool
(The domain is identified by UUID rather than domid). As a result, the
schedule can't be validated at this point.

The vcpu_id from userspace is only ever used to compare against a list of real
vcpus available to the scheduler, which prevents ill-specified vcpus from
actually being scheduled.

Remove the MAX_VIRT_CPUS test, as it is not an appropriate bound for vcpu_id.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Nathan Studer <nate.studer@dornerworks.com>
CC: Robert VanVossen <robert.vanvossen@dornerworks.com>
---
 xen/common/sched_arinc653.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index 5f09ded..819b869 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -246,9 +246,8 @@ arinc653_sched_set(
 
     for ( i = 0; i < schedule->num_sched_entries; i++ )
     {
-        /* Check for a valid VCPU ID and run time. */
-        if ( (schedule->sched_entries[i].vcpu_id >= MAX_VIRT_CPUS)
-             || (schedule->sched_entries[i].runtime <= 0) )
+        /* Check for a valid run time. */
+        if ( schedule->sched_entries[i].runtime <= 0 )
             goto fail;
 
         /* Add this entry's run time to total run time. */
-- 
1.7.10.4

Re: [PATCH 3/4] sched/arinc653: Remove MAX_VIRT_CPUS bounds check

[Jan Beulich]

>>> On 19.01.15 at 12:21, <andrew.cooper3@citrix.com> wrote:
> The arinc653 interface is capable of specifying a domain in the schedule 
> (from
> the toolstack) before the domain itself exists, or is present in the cpupool
> (The domain is identified by UUID rather than domid). As a result, the
> schedule can't be validated at this point.
> 
> The vcpu_id from userspace is only ever used to compare against a list of 
> real
> vcpus available to the scheduler, which prevents ill-specified vcpus from
> actually being scheduled.
> 
> Remove the MAX_VIRT_CPUS test, as it is not an appropriate bound for 
> vcpu_id.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH 3/4] sched/arinc653: Remove MAX_VIRT_CPUS bounds check

[Robert VanVossen]

On 1/20/2015 5:34 AM, Jan Beulich wrote:
>>>> On 19.01.15 at 12:21, <andrew.cooper3@citrix.com> wrote:
>> The arinc653 interface is capable of specifying a domain in the schedule 
>> (from
>> the toolstack) before the domain itself exists, or is present in the cpupool
>> (The domain is identified by UUID rather than domid). As a result, the
>> schedule can't be validated at this point.
>>
>> The vcpu_id from userspace is only ever used to compare against a list of 
>> real
>> vcpus available to the scheduler, which prevents ill-specified vcpus from
>> actually being scheduled.
>>
>> Remove the MAX_VIRT_CPUS test, as it is not an appropriate bound for 
>> vcpu_id.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 

Acked-by Robert VanVossen <robert.vanvossen@dornerworks.com>

[PATCH 4/4] xen/many: Drop redundant MAX_VIRT_CPUS bounds checks

[Andrew Cooper]

In all 4 cases, visible in the context are bounds check against d->max_vcpus.
Domain building will ensure that d->max_vcpus never exceeds an appropriate
bound.  In the x86 case, different types of domains have different maxima for
vcpus, making the checks wrong as opposed to simply redundant.

For vpsci in ARM, 'vcpuid' is an unsigned type so could never be less than 0.

For the common changes to do_{,compat}_vcpu_op(), these changes do result in a
guest visible change, but only in so far as certain invalid vcpu ids will now
fail with -ENOENT rather than -EINVAL.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
CC: Tim Deegan <tim@xen.org>
---
 xen/arch/arm/vpsci.c       |    3 ---
 xen/arch/x86/domctl.c      |    4 ----
 xen/common/compat/domain.c |    3 ---
 xen/common/domain.c        |    3 ---
 4 files changed, 13 deletions(-)

diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index 3f2a482..5d899be 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -37,9 +37,6 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
     else
         vcpuid = target_cpu;
 
-    if ( (vcpuid < 0) || (vcpuid >= MAX_VIRT_CPUS) )
-        return PSCI_INVALID_PARAMETERS;
-
     if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
         return PSCI_INVALID_PARAMETERS;
 
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 82365a4..a1c5db0 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -541,10 +541,6 @@ long arch_do_domctl(
     {
         struct vcpu *v;
 
-        ret = -EINVAL;
-        if ( domctl->u.sendtrigger.vcpu >= MAX_VIRT_CPUS )
-            break;
-
         ret = -ESRCH;
         if ( domctl->u.sendtrigger.vcpu >= d->max_vcpus ||
              (v = d->vcpu[domctl->u.sendtrigger.vcpu]) == NULL )
diff --git a/xen/common/compat/domain.c b/xen/common/compat/domain.c
index 510843d..3ca4ef7 100644
--- a/xen/common/compat/domain.c
+++ b/xen/common/compat/domain.c
@@ -29,9 +29,6 @@ int compat_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) ar
     struct vcpu *v;
     int rc = 0;
 
-    if ( vcpuid >= MAX_VIRT_CPUS )
-        return -EINVAL;
-
     if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
         return -ENOENT;
 
diff --git a/xen/common/domain.c b/xen/common/domain.c
index e02823e..0b05681 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1146,9 +1146,6 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
     struct vcpu_guest_context *ctxt;
     long rc = 0;
 
-    if ( vcpuid >= MAX_VIRT_CPUS )
-        return -EINVAL;
-
     if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
         return -ENOENT;
 
-- 
1.7.10.4

Re: [PATCH 4/4] xen/many: Drop redundant MAX_VIRT_CPUS bounds checks

[Jan Beulich]

>>> On 19.01.15 at 12:21, <andrew.cooper3@citrix.com> wrote:
> In all 4 cases, visible in the context are bounds check against d->max_vcpus.
> Domain building will ensure that d->max_vcpus never exceeds an appropriate
> bound.  In the x86 case, different types of domains have different maxima 
> for
> vcpus, making the checks wrong as opposed to simply redundant.
> 
> For vpsci in ARM, 'vcpuid' is an unsigned type so could never be less than 
> 0.
> 
> For the common changes to do_{,compat}_vcpu_op(), these changes do result in 
> a
> guest visible change, but only in so far as certain invalid vcpu ids will 
> now
> fail with -ENOENT rather than -EINVAL.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH 4/4] xen/many: Drop redundant MAX_VIRT_CPUS bounds checks

[Ian Campbell]

On Mon, 2015-01-19 at 11:21 +0000, Andrew Cooper wrote:
> In all 4 cases, visible in the context are bounds check against d->max_vcpus.
> Domain building will ensure that d->max_vcpus never exceeds an appropriate
> bound.  In the x86 case, different types of domains have different maxima for
> vcpus, making the checks wrong as opposed to simply redundant.
> 
> For vpsci in ARM, 'vcpuid' is an unsigned type so could never be less than 0.
> 
> For the common changes to do_{,compat}_vcpu_op(), these changes do result in a
> guest visible change, but only in so far as certain invalid vcpu ids will now
> fail with -ENOENT rather than -EINVAL.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> CC: Tim Deegan <tim@xen.org>

For ARM: Acked-by: Ian Campbell <ian.campbell@citrix.com>