| Message ID | 1487148579-7243-19-git-send-email-yi.y.sun@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 15, 2017 at 04:49:33PM +0800, Yi Sun wrote: > This patch implements get HW info flow for L2 CAT including L2 CAT callback > function. > > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> > --- > xen/arch/x86/psr.c | 16 ++++++++++++++++ > xen/arch/x86/sysctl.c | 15 +++++++++++++++ > xen/include/asm-x86/psr.h | 1 + > xen/include/public/sysctl.h | 6 ++++++ > 4 files changed, 38 insertions(+) > > diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c > index 4489637..05100b4 100644 > --- a/xen/arch/x86/psr.c > +++ b/xen/arch/x86/psr.c > @@ -276,6 +276,9 @@ static enum psr_feat_type psr_cbm_type_to_feat_type(enum cbm_type type) > case PSR_CBM_TYPE_L3_CODE: > feat_type = PSR_SOCKET_L3_CDP; > break; > + case PSR_CBM_TYPE_L2: > + feat_type = PSR_SOCKET_L2_CAT; > + break; > default: > feat_type = PSR_SOCKET_UNKNOWN; > break; > @@ -729,8 +732,21 @@ static unsigned int l2_cat_get_cos_max(const struct feat_node *feat) > return feat->info.l2_cat_info.cos_max; > } > > +static bool l2_cat_get_feat_info(const struct feat_node *feat, > + uint32_t data[], uint32_t array_len) > +{ > + if ( !data || 2 > array_len ) > + return false; > + > + data[CBM_LEN] = feat->info.l2_cat_info.cbm_len; > + data[COS_MAX] = feat->info.l2_cat_info.cos_max; > + > + return true; > +} > + > struct feat_ops l2_cat_ops = { > .get_cos_max = l2_cat_get_cos_max, > + .get_feat_info = l2_cat_get_feat_info, > }; > > static void __init parse_psr_bool(char *s, char *value, char *feature, > diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c > index 568bfe9..01cf3b7 100644 > --- a/xen/arch/x86/sysctl.c > +++ b/xen/arch/x86/sysctl.c > @@ -207,6 +207,21 @@ long arch_do_sysctl( > ret = -EFAULT; > break; > } > + case XEN_SYSCTL_PSR_CAT_get_l2_info: > + { > + uint32_t dat[2]; Missing newline, and probalby you want "data" here to match with XEN_SYSCTL_PSR_CAT_get_l3_info? Also I think this 2 should be a constant, and it should be used both here and in l2_cat_get_feat_info. > + ret = psr_get_info(sysctl->u.psr_cat_op.target, > + PSR_CBM_TYPE_L2, dat, 2); ARRAY_SIZE(data) > + if ( ret ) > + break; > + > + sysctl->u.psr_cat_op.u.l2_info.cbm_len = dat[CBM_LEN]; > + sysctl->u.psr_cat_op.u.l2_info.cos_max = dat[COS_MAX]; > + > + if ( !ret && __copy_field_to_guest(u_sysctl, sysctl, u.psr_cat_op) ) > + ret = -EFAULT; > + break; > + } > default: > ret = -EOPNOTSUPP; > break; > diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h > index d7ed012..2e1b3d0 100644 > --- a/xen/include/asm-x86/psr.h > +++ b/xen/include/asm-x86/psr.h > @@ -56,6 +56,7 @@ enum cbm_type { > PSR_CBM_TYPE_L3, > PSR_CBM_TYPE_L3_CODE, > PSR_CBM_TYPE_L3_DATA, > + PSR_CBM_TYPE_L2, > }; > > extern struct psr_cmt *psr_cmt; > diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h > index 00f5e77..cbf5372 100644 > --- a/xen/include/public/sysctl.h > +++ b/xen/include/public/sysctl.h > @@ -744,6 +744,7 @@ typedef struct xen_sysctl_pcitopoinfo xen_sysctl_pcitopoinfo_t; > DEFINE_XEN_GUEST_HANDLE(xen_sysctl_pcitopoinfo_t); > > #define XEN_SYSCTL_PSR_CAT_get_l3_info 0 > +#define XEN_SYSCTL_PSR_CAT_get_l2_info 1 > struct xen_sysctl_psr_cat_op { > uint32_t cmd; /* IN: XEN_SYSCTL_PSR_CAT_* */ > uint32_t target; /* IN */ > @@ -754,6 +755,11 @@ struct xen_sysctl_psr_cat_op { > #define XEN_SYSCTL_PSR_CAT_L3_CDP (1u << 0) > uint32_t flags; /* OUT: CAT flags */ > } l3_info; > + > + struct { > + uint32_t cbm_len; /* OUT: CBM length */ > + uint32_t cos_max; /* OUT: Maximum COS */ > + } l2_info; > } u; > }; > typedef struct xen_sysctl_psr_cat_op xen_sysctl_psr_cat_op_t; > -- > 1.9.1 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel
>>> On 15.02.17 at 09:49, <yi.y.sun@linux.intel.com> wrote: > +static bool l2_cat_get_feat_info(const struct feat_node *feat, > + uint32_t data[], uint32_t array_len) > +{ > + if ( !data || 2 > array_len ) > + return false; > + > + data[CBM_LEN] = feat->info.l2_cat_info.cbm_len; > + data[COS_MAX] = feat->info.l2_cat_info.cos_max; What about PSR_FLAG? Which puts under question the array_len check at the start of this and its sibling functions: If more array entries are provided, they'd all be left uninitialized without the caller having a way to know. Coming back to the data structures being the same for all features, there should presumably be a structure instead of an array be used for communication here, and ... > @@ -754,6 +755,11 @@ struct xen_sysctl_psr_cat_op { > #define XEN_SYSCTL_PSR_CAT_L3_CDP (1u << 0) > uint32_t flags; /* OUT: CAT flags */ > } l3_info; > + > + struct { > + uint32_t cbm_len; /* OUT: CBM length */ > + uint32_t cos_max; /* OUT: Maximum COS */ > + } l2_info; ... this should use the same structure as l3_info (flags being set to zero until a use arises). This would then also allow for more code to be shared instead of duplicated. Jan
On 17-03-09 08:13:59, Jan Beulich wrote: > >>> On 15.02.17 at 09:49, <yi.y.sun@linux.intel.com> wrote: > > +static bool l2_cat_get_feat_info(const struct feat_node *feat, > > + uint32_t data[], uint32_t array_len) > > +{ > > + if ( !data || 2 > array_len ) > > + return false; > > + > > + data[CBM_LEN] = feat->info.l2_cat_info.cbm_len; > > + data[COS_MAX] = feat->info.l2_cat_info.cos_max; > > What about PSR_FLAG? Which puts under question the > array_len check at the start of this and its sibling functions: If > more array entries are provided, they'd all be left uninitialized > without the caller having a way to know. Coming back to the > data structures being the same for all features, there should > presumably be a structure instead of an array be used for > communication here, and ... > > > @@ -754,6 +755,11 @@ struct xen_sysctl_psr_cat_op { > > #define XEN_SYSCTL_PSR_CAT_L3_CDP (1u << 0) > > uint32_t flags; /* OUT: CAT flags */ > > } l3_info; > > + > > + struct { > > + uint32_t cbm_len; /* OUT: CBM length */ > > + uint32_t cos_max; /* OUT: Maximum COS */ > > + } l2_info; > > ... this should use the same structure as l3_info (flags being > set to zero until a use arises). This would then also allow for > more code to be shared instead of duplicated. > Ok, will reuse l3_info for L2 CAT. May modify its name to cat_info. But for MBA or future feature, its info is different with CAT. So, it may not be approriate to use a structure as parameter for 'psr_get_info'. So I would prefer to keep data[]. Of course, I will correct array_len check per Roger's suggestion, like "array_len != PSR_INFO_SIZE". Is that good to you? > Jan
>>> On 10.03.17 at 06:57, <yi.y.sun@linux.intel.com> wrote: > On 17-03-09 08:13:59, Jan Beulich wrote: >> >>> On 15.02.17 at 09:49, <yi.y.sun@linux.intel.com> wrote: >> > +static bool l2_cat_get_feat_info(const struct feat_node *feat, >> > + uint32_t data[], uint32_t array_len) >> > +{ >> > + if ( !data || 2 > array_len ) >> > + return false; >> > + >> > + data[CBM_LEN] = feat->info.l2_cat_info.cbm_len; >> > + data[COS_MAX] = feat->info.l2_cat_info.cos_max; >> >> What about PSR_FLAG? Which puts under question the >> array_len check at the start of this and its sibling functions: If >> more array entries are provided, they'd all be left uninitialized >> without the caller having a way to know. Coming back to the >> data structures being the same for all features, there should >> presumably be a structure instead of an array be used for >> communication here, and ... >> >> > @@ -754,6 +755,11 @@ struct xen_sysctl_psr_cat_op { >> > #define XEN_SYSCTL_PSR_CAT_L3_CDP (1u << 0) >> > uint32_t flags; /* OUT: CAT flags */ >> > } l3_info; >> > + >> > + struct { >> > + uint32_t cbm_len; /* OUT: CBM length */ >> > + uint32_t cos_max; /* OUT: Maximum COS */ >> > + } l2_info; >> >> ... this should use the same structure as l3_info (flags being >> set to zero until a use arises). This would then also allow for >> more code to be shared instead of duplicated. >> > Ok, will reuse l3_info for L2 CAT. May modify its name to cat_info. Of course. > But for MBA or future feature, its info is different with CAT. So, it may > not be approriate to use a structure as parameter for 'psr_get_info'. > So I would prefer to keep data[]. Of course, I will correct array_len check > per Roger's suggestion, like "array_len != PSR_INFO_SIZE". Is that good to > you? Yes, if there is a need for it to be that way down the road, then I'm fine as long as both the risk of array overrun and that of using uninitialized data are minimized as much as possible. Jan
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c index 4489637..05100b4 100644 --- a/xen/arch/x86/psr.c +++ b/xen/arch/x86/psr.c @@ -276,6 +276,9 @@ static enum psr_feat_type psr_cbm_type_to_feat_type(enum cbm_type type) case PSR_CBM_TYPE_L3_CODE: feat_type = PSR_SOCKET_L3_CDP; break; + case PSR_CBM_TYPE_L2: + feat_type = PSR_SOCKET_L2_CAT; + break; default: feat_type = PSR_SOCKET_UNKNOWN; break; @@ -729,8 +732,21 @@ static unsigned int l2_cat_get_cos_max(const struct feat_node *feat) return feat->info.l2_cat_info.cos_max; } +static bool l2_cat_get_feat_info(const struct feat_node *feat, + uint32_t data[], uint32_t array_len) +{ + if ( !data || 2 > array_len ) + return false; + + data[CBM_LEN] = feat->info.l2_cat_info.cbm_len; + data[COS_MAX] = feat->info.l2_cat_info.cos_max; + + return true; +} + struct feat_ops l2_cat_ops = { .get_cos_max = l2_cat_get_cos_max, + .get_feat_info = l2_cat_get_feat_info, }; static void __init parse_psr_bool(char *s, char *value, char *feature, diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c index 568bfe9..01cf3b7 100644 --- a/xen/arch/x86/sysctl.c +++ b/xen/arch/x86/sysctl.c @@ -207,6 +207,21 @@ long arch_do_sysctl( ret = -EFAULT; break; } + case XEN_SYSCTL_PSR_CAT_get_l2_info: + { + uint32_t dat[2]; + ret = psr_get_info(sysctl->u.psr_cat_op.target, + PSR_CBM_TYPE_L2, dat, 2); + if ( ret ) + break; + + sysctl->u.psr_cat_op.u.l2_info.cbm_len = dat[CBM_LEN]; + sysctl->u.psr_cat_op.u.l2_info.cos_max = dat[COS_MAX]; + + if ( !ret && __copy_field_to_guest(u_sysctl, sysctl, u.psr_cat_op) ) + ret = -EFAULT; + break; + } default: ret = -EOPNOTSUPP; break; diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h index d7ed012..2e1b3d0 100644 --- a/xen/include/asm-x86/psr.h +++ b/xen/include/asm-x86/psr.h @@ -56,6 +56,7 @@ enum cbm_type { PSR_CBM_TYPE_L3, PSR_CBM_TYPE_L3_CODE, PSR_CBM_TYPE_L3_DATA, + PSR_CBM_TYPE_L2, }; extern struct psr_cmt *psr_cmt; diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h index 00f5e77..cbf5372 100644 --- a/xen/include/public/sysctl.h +++ b/xen/include/public/sysctl.h @@ -744,6 +744,7 @@ typedef struct xen_sysctl_pcitopoinfo xen_sysctl_pcitopoinfo_t; DEFINE_XEN_GUEST_HANDLE(xen_sysctl_pcitopoinfo_t); #define XEN_SYSCTL_PSR_CAT_get_l3_info 0 +#define XEN_SYSCTL_PSR_CAT_get_l2_info 1 struct xen_sysctl_psr_cat_op { uint32_t cmd; /* IN: XEN_SYSCTL_PSR_CAT_* */ uint32_t target; /* IN */ @@ -754,6 +755,11 @@ struct xen_sysctl_psr_cat_op { #define XEN_SYSCTL_PSR_CAT_L3_CDP (1u << 0) uint32_t flags; /* OUT: CAT flags */ } l3_info; + + struct { + uint32_t cbm_len; /* OUT: CBM length */ + uint32_t cos_max; /* OUT: Maximum COS */ + } l2_info; } u; }; typedef struct xen_sysctl_psr_cat_op xen_sysctl_psr_cat_op_t;
This patch implements get HW info flow for L2 CAT including L2 CAT callback function. Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> --- xen/arch/x86/psr.c | 16 ++++++++++++++++ xen/arch/x86/sysctl.c | 15 +++++++++++++++ xen/include/asm-x86/psr.h | 1 + xen/include/public/sysctl.h | 6 ++++++ 4 files changed, 38 insertions(+)