Message ID | 1458336385-2606-2-git-send-email-lichong659@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
>>> On 18.03.16 at 22:26, <lichong659@gmail.com> wrote: > Add XEN_DOMCTL_SCHEDOP_getvcpuinfo and _putvcpuinfo hypercalls > to independently get and set the scheduling parameters of each > vCPU of a domain > > Signed-off-by: Chong Li <chong.li@wustl.edu> > Signed-off-by: Meng Xu <mengxu@cis.upenn.edu> > Signed-off-by: Sisu Xi <xisisu@gmail.com> > > --- > Changes on PATCH v7: > 1) A bug in case XEN_DOMCTL_SCHEDOP_getinfo (in Xen 4.6) is fixed: > The default PERIOD or BUDGET should be divided by MICROSECS(1), > before returned to upper caller. Seems like there's still some misunderstanding here: Anything past the first --- won't make it into the repo, yet the description of what bug you fix should end up there. > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -338,24 +338,64 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t); > #define XEN_SCHEDULER_ARINC653 7 > #define XEN_SCHEDULER_RTDS 8 > > -/* Set or get info? */ > +typedef struct xen_domctl_sched_credit { > + uint16_t weight; > + uint16_t cap; > +} xen_domctl_sched_credit_t; > + > +typedef struct xen_domctl_sched_credit2 { > + uint16_t weight; > +} xen_domctl_sched_credit2_t; > + > +typedef struct xen_domctl_sched_rtds { > + uint32_t period; > + uint32_t budget; > +} xen_domctl_sched_rtds_t; > + > +typedef struct xen_domctl_schedparam_vcpu { > + union { > + xen_domctl_sched_credit_t credit; > + xen_domctl_sched_credit2_t credit2; > + xen_domctl_sched_rtds_t rtds; > + } u; > + uint32_t vcpuid; > + uint16_t padding[2]; So why uint16_t[2] instead of just uint32_t? And what's the padding needed for in the first place? Also, while for a domctl it's not as strictly needed as for other hypercalls, checking that all padding fields are zero would still seem to be rather desirable. Jan
On Mon, Mar 21, 2016 at 8:35 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 18.03.16 at 22:26, <lichong659@gmail.com> wrote: >> Add XEN_DOMCTL_SCHEDOP_getvcpuinfo and _putvcpuinfo hypercalls >> to independently get and set the scheduling parameters of each >> vCPU of a domain >> >> Signed-off-by: Chong Li <chong.li@wustl.edu> >> Signed-off-by: Meng Xu <mengxu@cis.upenn.edu> >> Signed-off-by: Sisu Xi <xisisu@gmail.com> >> >> --- >> Changes on PATCH v7: >> 1) A bug in case XEN_DOMCTL_SCHEDOP_getinfo (in Xen 4.6) is fixed: >> The default PERIOD or BUDGET should be divided by MICROSECS(1), >> before returned to upper caller. > > Seems like there's still some misunderstanding here: Anything > past the first --- won't make it into the repo, yet the description > of what bug you fix should end up there. I see. Do I put the "bug fix" before "Signed-off-by ..." or after it? > >> --- a/xen/include/public/domctl.h >> +++ b/xen/include/public/domctl.h >> @@ -338,24 +338,64 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t); >> #define XEN_SCHEDULER_ARINC653 7 >> #define XEN_SCHEDULER_RTDS 8 >> >> -/* Set or get info? */ >> +typedef struct xen_domctl_sched_credit { >> + uint16_t weight; >> + uint16_t cap; >> +} xen_domctl_sched_credit_t; >> + >> +typedef struct xen_domctl_sched_credit2 { >> + uint16_t weight; >> +} xen_domctl_sched_credit2_t; >> + >> +typedef struct xen_domctl_sched_rtds { >> + uint32_t period; >> + uint32_t budget; >> +} xen_domctl_sched_rtds_t; >> + >> +typedef struct xen_domctl_schedparam_vcpu { >> + union { >> + xen_domctl_sched_credit_t credit; >> + xen_domctl_sched_credit2_t credit2; >> + xen_domctl_sched_rtds_t rtds; >> + } u; >> + uint32_t vcpuid; >> + uint16_t padding[2]; > > So why uint16_t[2] instead of just uint32_t? And what's the > padding needed for in the first place? You're right. It's better to use uint32_t, which pads (the struct) to the 64-bit boundary. > Also, while for a domctl it's > not as strictly needed as for other hypercalls, checking that all > padding fields are zero would still seem to be rather desirable. > Do you mean: + case XEN_DOMCTL_SCHEDOP_getvcpuinfo: + case XEN_DOMCTL_SCHEDOP_putvcpuinfo: + if ( op->u.v.padding !=0 ) + { + rc = -EINVAL; + break; + } + while ( index < op->u.v.nr_vcpus ) + { + if ( copy_from_guest_offset(&local_sched, + op->u.v.vcpus, index, 1) ) + { + rc = -EFAULT; + break; + } + if ( local_sched.padding != 0 ) + { + rc = -EINVAL; + break; + } + if ( local_sched.vcpuid >= d->max_vcpus || + d->vcpu[local_sched.vcpuid] == NULL ) + { + rc = -EINVAL; + break; + } ? Thanks, Chong
On 21/03/16 15:18, Chong Li wrote: > On Mon, Mar 21, 2016 at 8:35 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>>> On 18.03.16 at 22:26, <lichong659@gmail.com> wrote: >>> Add XEN_DOMCTL_SCHEDOP_getvcpuinfo and _putvcpuinfo hypercalls >>> to independently get and set the scheduling parameters of each >>> vCPU of a domain >>> >>> Signed-off-by: Chong Li <chong.li@wustl.edu> >>> Signed-off-by: Meng Xu <mengxu@cis.upenn.edu> >>> Signed-off-by: Sisu Xi <xisisu@gmail.com> >>> >>> --- >>> Changes on PATCH v7: >>> 1) A bug in case XEN_DOMCTL_SCHEDOP_getinfo (in Xen 4.6) is fixed: >>> The default PERIOD or BUDGET should be divided by MICROSECS(1), >>> before returned to upper caller. >> >> Seems like there's still some misunderstanding here: Anything >> past the first --- won't make it into the repo, yet the description >> of what bug you fix should end up there. > > I see. Do I put the "bug fix" before "Signed-off-by ..." or after it? You need to put everything important for understanding this changeset in the changset description. So in this case, you'd probably add a paragraph like this: "Also fix a bug in XEN_DOMCTL_SCHEDOP_getinfo, where PERIOD and BUDGET are not divided by MICROSECS(1) before being returned to the caller." (And that would be before the SoB.) -George
>>> On 21.03.16 at 16:18, <lichong659@gmail.com> wrote: > On Mon, Mar 21, 2016 at 8:35 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>>> On 18.03.16 at 22:26, <lichong659@gmail.com> wrote: >>> --- a/xen/include/public/domctl.h >>> +++ b/xen/include/public/domctl.h >>> @@ -338,24 +338,64 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t); >>> #define XEN_SCHEDULER_ARINC653 7 >>> #define XEN_SCHEDULER_RTDS 8 >>> >>> -/* Set or get info? */ >>> +typedef struct xen_domctl_sched_credit { >>> + uint16_t weight; >>> + uint16_t cap; >>> +} xen_domctl_sched_credit_t; >>> + >>> +typedef struct xen_domctl_sched_credit2 { >>> + uint16_t weight; >>> +} xen_domctl_sched_credit2_t; >>> + >>> +typedef struct xen_domctl_sched_rtds { >>> + uint32_t period; >>> + uint32_t budget; >>> +} xen_domctl_sched_rtds_t; >>> + >>> +typedef struct xen_domctl_schedparam_vcpu { >>> + union { >>> + xen_domctl_sched_credit_t credit; >>> + xen_domctl_sched_credit2_t credit2; >>> + xen_domctl_sched_rtds_t rtds; >>> + } u; >>> + uint32_t vcpuid; >>> + uint16_t padding[2]; >> >> So why uint16_t[2] instead of just uint32_t? And what's the >> padding needed for in the first place? > > You're right. It's better to use uint32_t, which pads (the struct) to > the 64-bit boundary. Which doesn't answer the "why in the first place" part - I don't see why structure size needs to be a multiple of 64 bits. >> Also, while for a domctl it's >> not as strictly needed as for other hypercalls, checking that all >> padding fields are zero would still seem to be rather desirable. >> > > Do you mean: > > + case XEN_DOMCTL_SCHEDOP_getvcpuinfo: > + case XEN_DOMCTL_SCHEDOP_putvcpuinfo: > + if ( op->u.v.padding !=0 ) > + { > + rc = -EINVAL; > + break; > + } > + while ( index < op->u.v.nr_vcpus ) > + { > + if ( copy_from_guest_offset(&local_sched, > + op->u.v.vcpus, index, 1) ) > + { > + rc = -EFAULT; > + break; > + } > + if ( local_sched.padding != 0 ) > + { > + rc = -EINVAL; > + break; > + } > + if ( local_sched.vcpuid >= d->max_vcpus || > + d->vcpu[local_sched.vcpuid] == NULL ) > + { > + rc = -EINVAL; > + break; > + } > > ? Yes, something along those lines. Jan
On Mon, Mar 21, 2016 at 10:49 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 21.03.16 at 16:18, <lichong659@gmail.com> wrote: >> On Mon, Mar 21, 2016 at 8:35 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>>>> On 18.03.16 at 22:26, <lichong659@gmail.com> wrote: >>>> --- a/xen/include/public/domctl.h >>>> +++ b/xen/include/public/domctl.h >>>> @@ -338,24 +338,64 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t); >>>> #define XEN_SCHEDULER_ARINC653 7 >>>> #define XEN_SCHEDULER_RTDS 8 >>>> >>>> -/* Set or get info? */ >>>> +typedef struct xen_domctl_sched_credit { >>>> + uint16_t weight; >>>> + uint16_t cap; >>>> +} xen_domctl_sched_credit_t; >>>> + >>>> +typedef struct xen_domctl_sched_credit2 { >>>> + uint16_t weight; >>>> +} xen_domctl_sched_credit2_t; >>>> + >>>> +typedef struct xen_domctl_sched_rtds { >>>> + uint32_t period; >>>> + uint32_t budget; >>>> +} xen_domctl_sched_rtds_t; >>>> + >>>> +typedef struct xen_domctl_schedparam_vcpu { >>>> + union { >>>> + xen_domctl_sched_credit_t credit; >>>> + xen_domctl_sched_credit2_t credit2; >>>> + xen_domctl_sched_rtds_t rtds; >>>> + } u; >>>> + uint32_t vcpuid; >>>> + uint16_t padding[2]; >>> >>> So why uint16_t[2] instead of just uint32_t? And what's the >>> padding needed for in the first place? >> >> You're right. It's better to use uint32_t, which pads (the struct) to >> the 64-bit boundary. > > Which doesn't answer the "why in the first place" part - I don't > see why structure size needs to be a multiple of 64 bits. > In this patch post, http://lists.xen.org/archives/html/xen-devel/2015-07/msg02334.html you had a comment about the structure size, which I think you mean the struct size should be a multiple of 64 bits. Chong
>>> On 21.03.16 at 17:03, <lichong659@gmail.com> wrote: > On Mon, Mar 21, 2016 at 10:49 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>>> On 21.03.16 at 16:18, <lichong659@gmail.com> wrote: >>> On Mon, Mar 21, 2016 at 8:35 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>>>>> On 18.03.16 at 22:26, <lichong659@gmail.com> wrote: >>>>> --- a/xen/include/public/domctl.h >>>>> +++ b/xen/include/public/domctl.h >>>>> @@ -338,24 +338,64 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t); >>>>> #define XEN_SCHEDULER_ARINC653 7 >>>>> #define XEN_SCHEDULER_RTDS 8 >>>>> >>>>> -/* Set or get info? */ >>>>> +typedef struct xen_domctl_sched_credit { >>>>> + uint16_t weight; >>>>> + uint16_t cap; >>>>> +} xen_domctl_sched_credit_t; >>>>> + >>>>> +typedef struct xen_domctl_sched_credit2 { >>>>> + uint16_t weight; >>>>> +} xen_domctl_sched_credit2_t; >>>>> + >>>>> +typedef struct xen_domctl_sched_rtds { >>>>> + uint32_t period; >>>>> + uint32_t budget; >>>>> +} xen_domctl_sched_rtds_t; >>>>> + >>>>> +typedef struct xen_domctl_schedparam_vcpu { >>>>> + union { >>>>> + xen_domctl_sched_credit_t credit; >>>>> + xen_domctl_sched_credit2_t credit2; >>>>> + xen_domctl_sched_rtds_t rtds; >>>>> + } u; >>>>> + uint32_t vcpuid; >>>>> + uint16_t padding[2]; >>>> >>>> So why uint16_t[2] instead of just uint32_t? And what's the >>>> padding needed for in the first place? >>> >>> You're right. It's better to use uint32_t, which pads (the struct) to >>> the 64-bit boundary. >> >> Which doesn't answer the "why in the first place" part - I don't >> see why structure size needs to be a multiple of 64 bits. >> > In this patch post, > > http://lists.xen.org/archives/html/xen-devel/2015-07/msg02334.html > > you had a comment about the structure size, which I think you mean > the struct size should be a multiple of 64 bits. Looks like I had got mislead by there being struct xen_domctl_sched_sedf, but it not being part of the union. I'm sorry for that. Jan
On Mon, Mar 21, 2016 at 11:25 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 21.03.16 at 17:03, <lichong659@gmail.com> wrote: >> On Mon, Mar 21, 2016 at 10:49 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>>>> On 21.03.16 at 16:18, <lichong659@gmail.com> wrote: >>>> On Mon, Mar 21, 2016 at 8:35 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>>>>>> On 18.03.16 at 22:26, <lichong659@gmail.com> wrote: >>>>>> --- a/xen/include/public/domctl.h >>>>>> +++ b/xen/include/public/domctl.h >>>>>> @@ -338,24 +338,64 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t); >>>>>> #define XEN_SCHEDULER_ARINC653 7 >>>>>> #define XEN_SCHEDULER_RTDS 8 >>>>>> >>>>>> -/* Set or get info? */ >>>>>> +typedef struct xen_domctl_sched_credit { >>>>>> + uint16_t weight; >>>>>> + uint16_t cap; >>>>>> +} xen_domctl_sched_credit_t; >>>>>> + >>>>>> +typedef struct xen_domctl_sched_credit2 { >>>>>> + uint16_t weight; >>>>>> +} xen_domctl_sched_credit2_t; >>>>>> + >>>>>> +typedef struct xen_domctl_sched_rtds { >>>>>> + uint32_t period; >>>>>> + uint32_t budget; >>>>>> +} xen_domctl_sched_rtds_t; >>>>>> + >>>>>> +typedef struct xen_domctl_schedparam_vcpu { >>>>>> + union { >>>>>> + xen_domctl_sched_credit_t credit; >>>>>> + xen_domctl_sched_credit2_t credit2; >>>>>> + xen_domctl_sched_rtds_t rtds; >>>>>> + } u; >>>>>> + uint32_t vcpuid; >>>>>> + uint16_t padding[2]; >>>>> >>>>> So why uint16_t[2] instead of just uint32_t? And what's the >>>>> padding needed for in the first place? >>>> >>>> You're right. It's better to use uint32_t, which pads (the struct) to >>>> the 64-bit boundary. >>> >>> Which doesn't answer the "why in the first place" part - I don't >>> see why structure size needs to be a multiple of 64 bits. >>> >> In this patch post, >> >> http://lists.xen.org/archives/html/xen-devel/2015-07/msg02334.html >> >> you had a comment about the structure size, which I think you mean >> the struct size should be a multiple of 64 bits. > > Looks like I had got mislead by there being struct > xen_domctl_sched_sedf, but it not being part of the union. I'm > sorry for that. > Ok. I've already fixed all problems pointed out by George and Jan. Dario and Meng, do you have any other comments on this patch? Chong
On Fri, 2016-03-25 at 13:07 -0500, Chong Li wrote: > Ok. I've already fixed all problems pointed out by George and Jan. > > Dario and Meng, do you have any other comments on this patch? > Hey, Sorry for the delay... busy, and then public holidays... I'll look at this series later today. Regards, Dario
On Fri, 2016-03-25 at 13:07 -0500, Chong Li wrote: > Ok. I've already fixed all problems pointed out by George and Jan. > > Dario and Meng, do you have any other comments on this patch? > Ok, I've looked at the patch and no, I don't have any further comments on this patch (and it looks to me you addressed all the comments I made on v7). So this one looks fine, with others' comments addressed, of course. Thanks and Regards, Dario
On Tue, Mar 29, 2016 at 06:59:15PM +0200, Dario Faggioli wrote: > On Fri, 2016-03-25 at 13:07 -0500, Chong Li wrote: > > Ok. I've already fixed all problems pointed out by George and Jan. > > > > Dario and Meng, do you have any other comments on this patch? > > > Ok, I've looked at the patch and no, I don't have any further comments > on this patch (and it looks to me you addressed all the comments I made > on v7). So this one looks fine, with others' comments addressed, of > course. Sounds like an Reviewed-by? > > Thanks and Regards, > Dario > -- > <<This happens because I choose it to happen!>> (Raistlin Majere) > ----------------------------------------------------------------- > Dario Faggioli, Ph.D, http://about.me/dario.faggioli > Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On Tue, 2016-03-29 at 13:13 -0400, Konrad Rzeszutek Wilk wrote: > On Tue, Mar 29, 2016 at 06:59:15PM +0200, Dario Faggioli wrote: > > On Fri, 2016-03-25 at 13:07 -0500, Chong Li wrote: > > > Ok. I've already fixed all problems pointed out by George and > > > Jan. > > > > > > Dario and Meng, do you have any other comments on this patch? > > > > > Ok, I've looked at the patch and no, I don't have any further > > comments > > on this patch (and it looks to me you addressed all the comments I > > made > > on v7). So this one looks fine, with others' comments addressed, of > > course. > Sounds like an Reviewed-by? > It does sounds *like* it, but since this is being re-sent anyway, I'll take the chance to check the new version --just to be sure-- and provide it at that point/time, in all its glory. :-D Regards, Dario
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c index 0dce790..b504702 100644 --- a/xen/common/sched_credit.c +++ b/xen/common/sched_credit.c @@ -1054,15 +1054,13 @@ csched_dom_cntl( * lock. Runq lock not needed anywhere in here. */ spin_lock_irqsave(&prv->lock, flags); - if ( op->cmd == XEN_DOMCTL_SCHEDOP_getinfo ) + switch ( op->cmd ) { + case XEN_DOMCTL_SCHEDOP_getinfo: op->u.credit.weight = sdom->weight; op->u.credit.cap = sdom->cap; - } - else - { - ASSERT(op->cmd == XEN_DOMCTL_SCHEDOP_putinfo); - + break; + case XEN_DOMCTL_SCHEDOP_putinfo: if ( op->u.credit.weight != 0 ) { if ( !list_empty(&sdom->active_sdom_elem) ) @@ -1075,7 +1073,9 @@ csched_dom_cntl( if ( op->u.credit.cap != (uint16_t)~0U ) sdom->cap = op->u.credit.cap; - + break; + default: + return -EINVAL; } spin_unlock_irqrestore(&prv->lock, flags); diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c index 3c49ffa..c2dcf0f 100644 --- a/xen/common/sched_credit2.c +++ b/xen/common/sched_credit2.c @@ -1421,14 +1421,12 @@ csched2_dom_cntl( * runq lock to update csvcs. */ spin_lock_irqsave(&prv->lock, flags); - if ( op->cmd == XEN_DOMCTL_SCHEDOP_getinfo ) + switch ( op->cmd ) { + case XEN_DOMCTL_SCHEDOP_getinfo: op->u.credit2.weight = sdom->weight; - } - else - { - ASSERT(op->cmd == XEN_DOMCTL_SCHEDOP_putinfo); - + break; + case XEN_DOMCTL_SCHEDOP_putinfo: if ( op->u.credit2.weight != 0 ) { struct vcpu *v; @@ -1457,6 +1455,9 @@ csched2_dom_cntl( vcpu_schedule_unlock(lock, svc->vcpu); } } + break; + default: + return -EINVAL; } spin_unlock_irqrestore(&prv->lock, flags); diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c index 3f1d047..b640e59 100644 --- a/xen/common/sched_rt.c +++ b/xen/common/sched_rt.c @@ -86,6 +86,22 @@ #define RTDS_DEFAULT_PERIOD (MICROSECS(10000)) #define RTDS_DEFAULT_BUDGET (MICROSECS(4000)) +/* + * Max period: max delta of time type, because period is added to the time + * a vcpu activates, so this must not overflow. + * Min period: 10 us, considering the scheduling overhead (when period is + * too low, scheduling is invoked too frequently, causing high overhead). + */ +#define RTDS_MAX_PERIOD (STIME_DELTA_MAX) +#define RTDS_MIN_PERIOD (MICROSECS(10)) + +/* + * Min budget: 10 us, considering the scheduling overhead (when budget is + * consumed too fast, scheduling is invoked too frequently, causing + * high overhead). + */ +#define RTDS_MIN_BUDGET (MICROSECS(10)) + #define UPDATE_LIMIT_SHIFT 10 #define MAX_SCHEDULE (MILLISECS(1)) /* @@ -1129,24 +1145,16 @@ rt_dom_cntl( struct vcpu *v; unsigned long flags; int rc = 0; + xen_domctl_schedparam_vcpu_t local_sched; + s_time_t period, budget; + uint32_t index = 0; switch ( op->cmd ) { case XEN_DOMCTL_SCHEDOP_getinfo: - if ( d->max_vcpus > 0 ) - { - spin_lock_irqsave(&prv->lock, flags); - svc = rt_vcpu(d->vcpu[0]); - op->u.rtds.period = svc->period / MICROSECS(1); - op->u.rtds.budget = svc->budget / MICROSECS(1); - spin_unlock_irqrestore(&prv->lock, flags); - } - else - { - /* If we don't have vcpus yet, let's just return the defaults. */ - op->u.rtds.period = RTDS_DEFAULT_PERIOD; - op->u.rtds.budget = RTDS_DEFAULT_BUDGET; - } + /* Return the default parameters. */ + op->u.rtds.period = RTDS_DEFAULT_PERIOD / MICROSECS(1); + op->u.rtds.budget = RTDS_DEFAULT_BUDGET / MICROSECS(1); break; case XEN_DOMCTL_SCHEDOP_putinfo: if ( op->u.rtds.period == 0 || op->u.rtds.budget == 0 ) @@ -1163,6 +1171,63 @@ rt_dom_cntl( } spin_unlock_irqrestore(&prv->lock, flags); break; + case XEN_DOMCTL_SCHEDOP_getvcpuinfo: + case XEN_DOMCTL_SCHEDOP_putvcpuinfo: + while ( index < op->u.v.nr_vcpus ) + { + if ( copy_from_guest_offset(&local_sched, + op->u.v.vcpus, index, 1) ) + { + rc = -EFAULT; + break; + } + if ( local_sched.vcpuid >= d->max_vcpus || + d->vcpu[local_sched.vcpuid] == NULL ) + { + rc = -EINVAL; + break; + } + + if ( op->cmd == XEN_DOMCTL_SCHEDOP_getvcpuinfo ) + { + spin_lock_irqsave(&prv->lock, flags); + svc = rt_vcpu(d->vcpu[local_sched.vcpuid]); + local_sched.u.rtds.budget = svc->budget / MICROSECS(1); + local_sched.u.rtds.period = svc->period / MICROSECS(1); + spin_unlock_irqrestore(&prv->lock, flags); + + if ( copy_to_guest_offset(op->u.v.vcpus, index, + &local_sched, 1) ) + { + rc = -EFAULT; + break; + } + } + else + { + period = MICROSECS(local_sched.u.rtds.period); + budget = MICROSECS(local_sched.u.rtds.budget); + if ( period > RTDS_MAX_PERIOD || budget < RTDS_MIN_BUDGET || + budget > period || period < RTDS_MIN_PERIOD ) + { + rc = -EINVAL; + break; + } + + spin_lock_irqsave(&prv->lock, flags); + svc = rt_vcpu(d->vcpu[local_sched.vcpuid]); + svc->period = period; + svc->budget = budget; + spin_unlock_irqrestore(&prv->lock, flags); + } + /* Process a most 64 vCPUs without checking for preemptions. */ + if ( (++index > 63) && hypercall_preempt_check() ) + break; + } + if ( !rc ) + /* notify upper caller how many vcpus have been processed. */ + op->u.v.nr_vcpus = index; + break; } return rc; diff --git a/xen/common/schedule.c b/xen/common/schedule.c index c195129..cd25e08 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -1148,11 +1148,20 @@ long sched_adjust(struct domain *d, struct xen_domctl_scheduler_op *op) if ( ret ) return ret; - if ( (op->sched_id != DOM2OP(d)->sched_id) || - ((op->cmd != XEN_DOMCTL_SCHEDOP_putinfo) && - (op->cmd != XEN_DOMCTL_SCHEDOP_getinfo)) ) + if ( op->sched_id != DOM2OP(d)->sched_id ) return -EINVAL; + switch ( op->cmd ) + { + case XEN_DOMCTL_SCHEDOP_putinfo: + case XEN_DOMCTL_SCHEDOP_getinfo: + case XEN_DOMCTL_SCHEDOP_putvcpuinfo: + case XEN_DOMCTL_SCHEDOP_getvcpuinfo: + break; + default: + return -EINVAL; + } + /* NB: the pluggable scheduler code needs to take care * of locking by itself. */ if ( (ret = SCHED_OP(DOM2OP(d), adjust, d, op)) == 0 ) diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index 7a56b3f..24675f5 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -338,24 +338,64 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t); #define XEN_SCHEDULER_ARINC653 7 #define XEN_SCHEDULER_RTDS 8 -/* Set or get info? */ +typedef struct xen_domctl_sched_credit { + uint16_t weight; + uint16_t cap; +} xen_domctl_sched_credit_t; + +typedef struct xen_domctl_sched_credit2 { + uint16_t weight; +} xen_domctl_sched_credit2_t; + +typedef struct xen_domctl_sched_rtds { + uint32_t period; + uint32_t budget; +} xen_domctl_sched_rtds_t; + +typedef struct xen_domctl_schedparam_vcpu { + union { + xen_domctl_sched_credit_t credit; + xen_domctl_sched_credit2_t credit2; + xen_domctl_sched_rtds_t rtds; + } u; + uint32_t vcpuid; + uint16_t padding[2]; +} xen_domctl_schedparam_vcpu_t; +DEFINE_XEN_GUEST_HANDLE(xen_domctl_schedparam_vcpu_t); + +/* + * Set or get info? + * For schedulers supporting per-vcpu settings (e.g., RTDS): + * XEN_DOMCTL_SCHEDOP_putinfo sets params for all vcpus; + * XEN_DOMCTL_SCHEDOP_getinfo gets default params; + * XEN_DOMCTL_SCHEDOP_put(get)vcpuinfo sets (gets) params of vcpus; + * + * For schedulers not supporting per-vcpu settings: + * XEN_DOMCTL_SCHEDOP_putinfo sets params for all vcpus; + * XEN_DOMCTL_SCHEDOP_getinfo gets domain-wise params; + * XEN_DOMCTL_SCHEDOP_put(get)vcpuinfo returns error; + */ #define XEN_DOMCTL_SCHEDOP_putinfo 0 #define XEN_DOMCTL_SCHEDOP_getinfo 1 +#define XEN_DOMCTL_SCHEDOP_putvcpuinfo 2 +#define XEN_DOMCTL_SCHEDOP_getvcpuinfo 3 struct xen_domctl_scheduler_op { uint32_t sched_id; /* XEN_SCHEDULER_* */ uint32_t cmd; /* XEN_DOMCTL_SCHEDOP_* */ + /* IN/OUT */ union { - struct xen_domctl_sched_credit { - uint16_t weight; - uint16_t cap; - } credit; - struct xen_domctl_sched_credit2 { - uint16_t weight; - } credit2; - struct xen_domctl_sched_rtds { - uint32_t period; - uint32_t budget; - } rtds; + xen_domctl_sched_credit_t credit; + xen_domctl_sched_credit2_t credit2; + xen_domctl_sched_rtds_t rtds; + struct { + XEN_GUEST_HANDLE_64(xen_domctl_schedparam_vcpu_t) vcpus; + /* + * IN: Number of elements in vcpus array. + * OUT: Number of processed elements of vcpus array. + */ + uint32_t nr_vcpus; + uint32_t padding; + } v; } u; }; typedef struct xen_domctl_scheduler_op xen_domctl_scheduler_op_t;