| Message ID | 1504603957-5389-7-git-send-email-yi.y.sun@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Sep 05, 2017 at 05:32:28PM +0800, Yi Sun wrote: > This patch implements get value domctl interface for MBA. > > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > --- > v3: > - change 'PSR_VAL_TYPE_MBA' to 'PSR_TYPE_MBA_THRTL'. > (suggested by Roger Pau Monné) > --- > xen/arch/x86/domctl.c | 7 +++++++ > xen/include/public/domctl.h | 1 + > 2 files changed, 8 insertions(+) > > diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c > index 696eff2..7902af7 100644 > --- a/xen/arch/x86/domctl.c > +++ b/xen/arch/x86/domctl.c > @@ -1496,6 +1496,13 @@ long arch_do_domctl( > copyback = true; > break; > > + case XEN_DOMCTL_PSR_ALLOC_GET_MBA_THRTL: > + ret = psr_get_val(d, domctl->u.psr_alloc.target, > + &val32, PSR_TYPE_MBA_THRTL); > + domctl->u.psr_alloc.data = val32; Hm, why does psr_get_val take a uint32_t * instead of a uint64_t *? So that you can directly pass &domctl->u.psr_alloc.data. Or the other way around, why is domctl->u.psr_alloc.data a uint64_t instead of a uint32_t? Thanks, Roger.
On 17-09-19 10:15:42, Roger Pau Monn� wrote: > On Tue, Sep 05, 2017 at 05:32:28PM +0800, Yi Sun wrote: > > diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c > > index 696eff2..7902af7 100644 > > --- a/xen/arch/x86/domctl.c > > +++ b/xen/arch/x86/domctl.c > > @@ -1496,6 +1496,13 @@ long arch_do_domctl( > > copyback = true; > > break; > > > > + case XEN_DOMCTL_PSR_ALLOC_GET_MBA_THRTL: > > + ret = psr_get_val(d, domctl->u.psr_alloc.target, > > + &val32, PSR_TYPE_MBA_THRTL); > > + domctl->u.psr_alloc.data = val32; > > Hm, why does psr_get_val take a uint32_t * instead of a uint64_t *? So > that you can directly pass &domctl->u.psr_alloc.data. > > Or the other way around, why is domctl->u.psr_alloc.data a uint64_t > instead of a uint32_t? > There is a historical reason. The COS MSR is 64bit. So, the original codes in L3 CAT (submitted years ago) used uint64_t. But during L2 CAT review, per Jan's comment, the uint64_t is not necessary in psr.c. So, we convert it to uint32_t in psr.c and make the codes you see here. > Thanks, Roger.
On Wed, Sep 20, 2017 at 01:09:06PM +0800, Yi Sun wrote: > On 17-09-19 10:15:42, Roger Pau Monn� wrote: > > On Tue, Sep 05, 2017 at 05:32:28PM +0800, Yi Sun wrote: > > > diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c > > > index 696eff2..7902af7 100644 > > > --- a/xen/arch/x86/domctl.c > > > +++ b/xen/arch/x86/domctl.c > > > @@ -1496,6 +1496,13 @@ long arch_do_domctl( > > > copyback = true; > > > break; > > > > > > + case XEN_DOMCTL_PSR_ALLOC_GET_MBA_THRTL: > > > + ret = psr_get_val(d, domctl->u.psr_alloc.target, > > > + &val32, PSR_TYPE_MBA_THRTL); > > > + domctl->u.psr_alloc.data = val32; > > > > Hm, why does psr_get_val take a uint32_t * instead of a uint64_t *? So > > that you can directly pass &domctl->u.psr_alloc.data. > > > > Or the other way around, why is domctl->u.psr_alloc.data a uint64_t > > instead of a uint32_t? > > > There is a historical reason. The COS MSR is 64bit. So, the original codes > in L3 CAT (submitted years ago) used uint64_t. > > But during L2 CAT review, per Jan's comment, the uint64_t is not necessary > in psr.c. So, we convert it to uint32_t in psr.c and make the codes you see > here. Since this is a DOMCTL, you can change the type of the structure to be an uint32_t, so that you can pass it directly (unless I'm missing something else that requires this to be uint64_t). Roger.
On 17-09-20 09:43:40, Roger Pau Monn� wrote: > On Wed, Sep 20, 2017 at 01:09:06PM +0800, Yi Sun wrote: > > On 17-09-19 10:15:42, Roger Pau Monn� wrote: > > > On Tue, Sep 05, 2017 at 05:32:28PM +0800, Yi Sun wrote: > > > > diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c > > > > index 696eff2..7902af7 100644 > > > > --- a/xen/arch/x86/domctl.c > > > > +++ b/xen/arch/x86/domctl.c > > > > @@ -1496,6 +1496,13 @@ long arch_do_domctl( > > > > copyback = true; > > > > break; > > > > > > > > + case XEN_DOMCTL_PSR_ALLOC_GET_MBA_THRTL: > > > > + ret = psr_get_val(d, domctl->u.psr_alloc.target, > > > > + &val32, PSR_TYPE_MBA_THRTL); > > > > + domctl->u.psr_alloc.data = val32; > > > > > > Hm, why does psr_get_val take a uint32_t * instead of a uint64_t *? So > > > that you can directly pass &domctl->u.psr_alloc.data. > > > > > > Or the other way around, why is domctl->u.psr_alloc.data a uint64_t > > > instead of a uint32_t? > > > > > There is a historical reason. The COS MSR is 64bit. So, the original codes > > in L3 CAT (submitted years ago) used uint64_t. > > > > But during L2 CAT review, per Jan's comment, the uint64_t is not necessary > > in psr.c. So, we convert it to uint32_t in psr.c and make the codes you see > > here. > > Since this is a DOMCTL, you can change the type of the structure to be > an uint32_t, so that you can pass it directly (unless I'm missing > something else that requires this to be uint64_t). > The tools layer implementation uses uint64_t according to SDM definition. So, we have to do a convertion here or in tools/. Or, we need modify interfaces in tools layer. As CAT interfaces in tools layer have been released and it is a natural choice to make it same as SDM, I think should keep uint64_t in tools at least. If so, we must do the convertion somewhere. Then, I think it is not so necessary to change current codes. Is that good to you? > Roger.
On Wed, Sep 20, 2017 at 05:22:22PM +0800, Yi Sun wrote: > On 17-09-20 09:43:40, Roger Pau Monn� wrote: > > On Wed, Sep 20, 2017 at 01:09:06PM +0800, Yi Sun wrote: > > > On 17-09-19 10:15:42, Roger Pau Monn� wrote: > > > > On Tue, Sep 05, 2017 at 05:32:28PM +0800, Yi Sun wrote: > > > > > diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c > > > > > index 696eff2..7902af7 100644 > > > > > --- a/xen/arch/x86/domctl.c > > > > > +++ b/xen/arch/x86/domctl.c > > > > > @@ -1496,6 +1496,13 @@ long arch_do_domctl( > > > > > copyback = true; > > > > > break; > > > > > > > > > > + case XEN_DOMCTL_PSR_ALLOC_GET_MBA_THRTL: > > > > > + ret = psr_get_val(d, domctl->u.psr_alloc.target, > > > > > + &val32, PSR_TYPE_MBA_THRTL); > > > > > + domctl->u.psr_alloc.data = val32; > > > > > > > > Hm, why does psr_get_val take a uint32_t * instead of a uint64_t *? So > > > > that you can directly pass &domctl->u.psr_alloc.data. > > > > > > > > Or the other way around, why is domctl->u.psr_alloc.data a uint64_t > > > > instead of a uint32_t? > > > > > > > There is a historical reason. The COS MSR is 64bit. So, the original codes > > > in L3 CAT (submitted years ago) used uint64_t. > > > > > > But during L2 CAT review, per Jan's comment, the uint64_t is not necessary > > > in psr.c. So, we convert it to uint32_t in psr.c and make the codes you see > > > here. > > > > Since this is a DOMCTL, you can change the type of the structure to be > > an uint32_t, so that you can pass it directly (unless I'm missing > > something else that requires this to be uint64_t). > > > The tools layer implementation uses uint64_t according to SDM definition. So, > we have to do a convertion here or in tools/. Or, we need modify interfaces in > tools layer. > > As CAT interfaces in tools layer have been released and it is a natural choice > to make it same as SDM, I think should keep uint64_t in tools at least. If so, > we must do the convertion somewhere. Then, I think it is not so necessary to > change current codes. Is that good to you? You can change libxc interfaces at will, you need to pay more attention if you change libxl. This is just FYI. I'm not suggesting you have to change the interfaces.
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index 696eff2..7902af7 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -1496,6 +1496,13 @@ long arch_do_domctl( copyback = true; break; + case XEN_DOMCTL_PSR_ALLOC_GET_MBA_THRTL: + ret = psr_get_val(d, domctl->u.psr_alloc.target, + &val32, PSR_TYPE_MBA_THRTL); + domctl->u.psr_alloc.data = val32; + copyback = true; + break; + default: ret = -EOPNOTSUPP; break; diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index a953157..8be38cc 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -1144,6 +1144,7 @@ struct xen_domctl_psr_alloc { #define XEN_DOMCTL_PSR_ALLOC_GET_L3_DATA 5 #define XEN_DOMCTL_PSR_ALLOC_SET_L2_CBM 6 #define XEN_DOMCTL_PSR_ALLOC_GET_L2_CBM 7 +#define XEN_DOMCTL_PSR_ALLOC_GET_MBA_THRTL 9 uint32_t cmd; /* IN: XEN_DOMCTL_PSR_CAT_OP_* */ uint32_t target; /* IN */ uint64_t data; /* IN/OUT */