* [PATCH 0/4] Reduce the use of MAX_VIRT_CPUS in tree
@ 2015-01-19 11:21 Andrew Cooper
2015-01-19 11:21 ` [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch Andrew Cooper
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Andrew Cooper @ 2015-01-19 11:21 UTC (permalink / raw)
To: Xen-devel
Cc: Keir Fraser, Ian Campbell, Andrew Cooper, Ian Jackson, Tim Deegan,
Jan Beulich
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch
2015-01-19 11:21 [PATCH 0/4] Reduce the use of MAX_VIRT_CPUS in tree Andrew Cooper
@ 2015-01-19 11:21 ` Andrew Cooper
2015-01-19 12:16 ` Simon Rowe
` (2 more replies)
2015-01-19 11:21 ` [PATCH 2/4] xen/evtchn: Reduce the size of the poll_mask where possible Andrew Cooper
` (2 subsequent siblings)
3 siblings, 3 replies; 17+ messages in thread
From: Andrew Cooper @ 2015-01-19 11:21 UTC (permalink / raw)
To: Xen-devel
Cc: Keir Fraser, Ian Campbell, Andrew Cooper, Tim Deegan,
Stefano Stabellini, Jan Beulich
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/4] xen/evtchn: Reduce the size of the poll_mask where possible
2015-01-19 11:21 [PATCH 0/4] Reduce the use of MAX_VIRT_CPUS in tree Andrew Cooper
2015-01-19 11:21 ` [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch Andrew Cooper
@ 2015-01-19 11:21 ` Andrew Cooper
2015-01-20 10:28 ` Jan Beulich
2015-01-19 11:21 ` [PATCH 3/4] sched/arinc653: Remove MAX_VIRT_CPUS bounds check Andrew Cooper
2015-01-19 11:21 ` [PATCH 4/4] xen/many: Drop redundant MAX_VIRT_CPUS bounds checks Andrew Cooper
3 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2015-01-19 11:21 UTC (permalink / raw)
To: Xen-devel
Cc: Keir Fraser, Ian Campbell, Andrew Cooper, Ian Jackson, Tim Deegan,
Jan Beulich
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/4] sched/arinc653: Remove MAX_VIRT_CPUS bounds check
2015-01-19 11:21 [PATCH 0/4] Reduce the use of MAX_VIRT_CPUS in tree Andrew Cooper
2015-01-19 11:21 ` [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch Andrew Cooper
2015-01-19 11:21 ` [PATCH 2/4] xen/evtchn: Reduce the size of the poll_mask where possible Andrew Cooper
@ 2015-01-19 11:21 ` Andrew Cooper
2015-01-20 10:34 ` Jan Beulich
2015-01-19 11:21 ` [PATCH 4/4] xen/many: Drop redundant MAX_VIRT_CPUS bounds checks Andrew Cooper
3 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2015-01-19 11:21 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Keir Fraser, Nathan Studer, Jan Beulich,
Robert VanVossen
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/4] xen/many: Drop redundant MAX_VIRT_CPUS bounds checks
2015-01-19 11:21 [PATCH 0/4] Reduce the use of MAX_VIRT_CPUS in tree Andrew Cooper
` (2 preceding siblings ...)
2015-01-19 11:21 ` [PATCH 3/4] sched/arinc653: Remove MAX_VIRT_CPUS bounds check Andrew Cooper
@ 2015-01-19 11:21 ` Andrew Cooper
2015-01-20 10:36 ` Jan Beulich
2015-01-20 11:06 ` Ian Campbell
3 siblings, 2 replies; 17+ messages in thread
From: Andrew Cooper @ 2015-01-19 11:21 UTC (permalink / raw)
To: Xen-devel
Cc: Keir Fraser, Ian Campbell, Andrew Cooper, Tim Deegan,
Stefano Stabellini, Jan Beulich
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch
2015-01-19 11:21 ` [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch Andrew Cooper
@ 2015-01-19 12:16 ` Simon Rowe
2015-01-19 12:52 ` Andrew Cooper
2015-01-20 10:27 ` Jan Beulich
2015-01-20 11:05 ` [PATCH " Ian Campbell
2 siblings, 1 reply; 17+ messages in thread
From: Simon Rowe @ 2015-01-19 12:16 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew 'e152a' Cooper
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch
2015-01-19 12:16 ` Simon Rowe
@ 2015-01-19 12:52 ` Andrew Cooper
2015-01-19 12:57 ` Ian Campbell
0 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2015-01-19 12:52 UTC (permalink / raw)
To: Simon Rowe, xen-devel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch
2015-01-19 12:52 ` Andrew Cooper
@ 2015-01-19 12:57 ` Ian Campbell
2015-01-19 12:59 ` Andrew Cooper
0 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2015-01-19 12:57 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Simon Rowe, xen-devel
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch
2015-01-19 12:57 ` Ian Campbell
@ 2015-01-19 12:59 ` Andrew Cooper
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2015-01-19 12:59 UTC (permalink / raw)
To: Ian Campbell; +Cc: Simon Rowe, xen-devel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch
2015-01-19 11:21 ` [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch Andrew Cooper
2015-01-19 12:16 ` Simon Rowe
@ 2015-01-20 10:27 ` Jan Beulich
2015-01-20 14:06 ` [PATCH v2 " Andrew Cooper
2015-01-20 11:05 ` [PATCH " Ian Campbell
2 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2015-01-20 10:27 UTC (permalink / raw)
To: Andrew Cooper
Cc: Keir Fraser, Tim Deegan, Stefano Stabellini, Ian Campbell,
Xen-devel
>>> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] xen/evtchn: Reduce the size of the poll_mask where possible
2015-01-19 11:21 ` [PATCH 2/4] xen/evtchn: Reduce the size of the poll_mask where possible Andrew Cooper
@ 2015-01-20 10:28 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2015-01-20 10:28 UTC (permalink / raw)
To: Andrew Cooper
Cc: Keir Fraser, Tim Deegan, Ian Jackson, Ian Campbell, Xen-devel
>>> 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>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] sched/arinc653: Remove MAX_VIRT_CPUS bounds check
2015-01-19 11:21 ` [PATCH 3/4] sched/arinc653: Remove MAX_VIRT_CPUS bounds check Andrew Cooper
@ 2015-01-20 10:34 ` Jan Beulich
2015-01-20 14:20 ` Robert VanVossen
0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2015-01-20 10:34 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Keir Fraser, Nathan Studer, Robert VanVossen, Xen-devel
>>> 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>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] xen/many: Drop redundant MAX_VIRT_CPUS bounds checks
2015-01-19 11:21 ` [PATCH 4/4] xen/many: Drop redundant MAX_VIRT_CPUS bounds checks Andrew Cooper
@ 2015-01-20 10:36 ` Jan Beulich
2015-01-20 11:06 ` Ian Campbell
1 sibling, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2015-01-20 10:36 UTC (permalink / raw)
To: Andrew Cooper
Cc: Keir Fraser, Tim Deegan, Stefano Stabellini, Ian Campbell,
Xen-devel
>>> 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>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch
2015-01-19 11:21 ` [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch Andrew Cooper
2015-01-19 12:16 ` Simon Rowe
2015-01-20 10:27 ` Jan Beulich
@ 2015-01-20 11:05 ` Ian Campbell
2 siblings, 0 replies; 17+ messages in thread
From: Ian Campbell @ 2015-01-20 11:05 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Tim Deegan, Keir Fraser, Jan Beulich,
Xen-devel
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] xen/many: Drop redundant MAX_VIRT_CPUS bounds checks
2015-01-19 11:21 ` [PATCH 4/4] xen/many: Drop redundant MAX_VIRT_CPUS bounds checks Andrew Cooper
2015-01-20 10:36 ` Jan Beulich
@ 2015-01-20 11:06 ` Ian Campbell
1 sibling, 0 replies; 17+ messages in thread
From: Ian Campbell @ 2015-01-20 11:06 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Tim Deegan, Keir Fraser, Jan Beulich,
Xen-devel
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>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch
2015-01-20 10:27 ` Jan Beulich
@ 2015-01-20 14:06 ` Andrew Cooper
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2015-01-20 14:06 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Stefano Stabellini, Keir Fraser, Jan Beulich,
Tim Deegan
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] sched/arinc653: Remove MAX_VIRT_CPUS bounds check
2015-01-20 10:34 ` Jan Beulich
@ 2015-01-20 14:20 ` Robert VanVossen
0 siblings, 0 replies; 17+ messages in thread
From: Robert VanVossen @ 2015-01-20 14:20 UTC (permalink / raw)
To: Jan Beulich, Andrew Cooper; +Cc: Keir Fraser, Nathan Studer, Xen-devel
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>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-01-20 14:20 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-19 11:21 [PATCH 0/4] Reduce the use of MAX_VIRT_CPUS in tree Andrew Cooper
2015-01-19 11:21 ` [PATCH 1/4] xen/domain: Introduce domain_max_vcpus() helper and implement per arch Andrew Cooper
2015-01-19 12:16 ` Simon Rowe
2015-01-19 12:52 ` Andrew Cooper
2015-01-19 12:57 ` Ian Campbell
2015-01-19 12:59 ` Andrew Cooper
2015-01-20 10:27 ` Jan Beulich
2015-01-20 14:06 ` [PATCH v2 " Andrew Cooper
2015-01-20 11:05 ` [PATCH " Ian Campbell
2015-01-19 11:21 ` [PATCH 2/4] xen/evtchn: Reduce the size of the poll_mask where possible Andrew Cooper
2015-01-20 10:28 ` Jan Beulich
2015-01-19 11:21 ` [PATCH 3/4] sched/arinc653: Remove MAX_VIRT_CPUS bounds check Andrew Cooper
2015-01-20 10:34 ` Jan Beulich
2015-01-20 14:20 ` Robert VanVossen
2015-01-19 11:21 ` [PATCH 4/4] xen/many: Drop redundant MAX_VIRT_CPUS bounds checks Andrew Cooper
2015-01-20 10:36 ` Jan Beulich
2015-01-20 11:06 ` Ian Campbell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.