Re: [libvirt] [PATCH 03/10] conf: Add CMT capability to host
> -Original Message- > From: John Ferlan [mailto:jfer...@redhat.com] > Sent: Saturday, September 8, 2018 1:11 AM > To: Wang, Huaqiang ; libvir-list@redhat.com > Cc: Feng, Shaohe ; Niu, Bing ; > Ding, Jian-feng ; Zang, Rui > Subject: Re: [libvirt] [PATCH 03/10] conf: Add CMT capability to host > > > > On 09/07/2018 04:37 AM, Wang, Huaqiang wrote: > > > > > >> -Original Message- > >> From: John Ferlan [mailto:jfer...@redhat.com] > >> Sent: Wednesday, September 5, 2018 7:59 PM > >> To: Wang, Huaqiang ; libvir-list@redhat.com > >> Cc: Feng, Shaohe ; Niu, Bing > >> ; Ding, Jian-feng ; > >> Zang, Rui > >> Subject: Re: [libvirt] [PATCH 03/10] conf: Add CMT capability to host > >> > >> > >> > >> On 08/27/2018 07:23 AM, Wang Huaqiang wrote: > >>> CMT capability for each cache bank, includes -. Maximum CMT > >>> monitoring groups(sharing with MBM) could be created, > >>>which reflects the maximum hardware RMID count. > >>> -. 'cache threshold'. > >>> -. Statistical information of last level cache, the actual cache > >>>occupancy. > >>> > >>> cache is splitted into 'bank's, each bank MAY have different cache > >>> configuration, report cache monitoring capability in unit of cache bank. > >>> > >>> cache monitor capability is shown as below: > >>> > >>> > >>> > >>> > >>> >>> maxAllocs='8'/> > >>> + > >>> + > >>> + > >>> > >>> > >>> > >>> >>> maxAllocs='8'/> > >>> + > >>> + > >>> + > >>> > >>> > >>> > >>> > >>> Signed-off-by: Wang Huaqiang > >>> --- > >>> docs/schemas/capability.rng | 28 > >>> src/conf/capabilities.c | 17 + > >>> 2 files changed, 45 insertions(+) > >>> > >> > >> This output would be combined with part of existing patch2. > >> > > > > Will be combined in next version patch. > > > >>> diff --git a/docs/schemas/capability.rng > >>> b/docs/schemas/capability.rng index d61515c..67498f1 100644 > >>> --- a/docs/schemas/capability.rng > >>> +++ b/docs/schemas/capability.rng > >>> @@ -314,6 +314,24 @@ > >>> > >>> > >>> > >>> + > >>> + > >>> + > >>> + > >>> + > >>> + > >>> + > >>> + > >>> + > >>> + > >>> + > >>> + > >>> + > >>> + > >>> + > >>> + > >>> + > >>> + > >>> > >>> > >>> > >>> @@ -329,6 +347,16 @@ > >>> > >>> > >>> > >>> + > >>> + > >>> + > >>> +llc_occupancy > >>> +mbm_total_bytes > >>> +mbm_local_bytes > >> > >> So these are the only 3 values you'll ever expect? Probably not a > >> good idea to list them like this or the current algorithm is overkill > >> looking for > prefixed "llc_" > >> and "mbm_" values. > >> > >> If "llc_somethingnew" shows up some day, then the schema is invalidated. > >> > > > > Disagree. > > I don't think the schema will be invalidated when new "llc_somethingnew" > > comes. Libvirt only recognize these three feature names. > > Hmm... maybe not clear enough - see, virt-xml-validate. > > Using limits the choices or allowed values to that known list. > So if some kernel some day adds llc_somethingnew, but the libvirt rng file for > the customer isn't updated to include that, then the XML doesn't validate. > > Trying to think of something else similar that exists today, but nothing > springs to > mind. > Align with you. Let's pass through all
Re: [libvirt] [PATCH 03/10] conf: Add CMT capability to host
On 09/07/2018 04:37 AM, Wang, Huaqiang wrote: > > >> -Original Message- >> From: John Ferlan [mailto:jfer...@redhat.com] >> Sent: Wednesday, September 5, 2018 7:59 PM >> To: Wang, Huaqiang ; libvir-list@redhat.com >> Cc: Feng, Shaohe ; Niu, Bing ; >> Ding, Jian-feng ; Zang, Rui >> Subject: Re: [libvirt] [PATCH 03/10] conf: Add CMT capability to host >> >> >> >> On 08/27/2018 07:23 AM, Wang Huaqiang wrote: >>> CMT capability for each cache bank, includes -. Maximum CMT monitoring >>> groups(sharing with MBM) could be created, >>>which reflects the maximum hardware RMID count. >>> -. 'cache threshold'. >>> -. Statistical information of last level cache, the actual cache >>>occupancy. >>> >>> cache is splitted into 'bank's, each bank MAY have different cache >>> configuration, report cache monitoring capability in unit of cache bank. >>> >>> cache monitor capability is shown as below: >>> >>> >>> >>> >>> >> maxAllocs='8'/> >>> + >>> + >>> + >>> >>> >>> >>> >> maxAllocs='8'/> >>> + >>> + >>> + >>> >>> >>> >>> >>> Signed-off-by: Wang Huaqiang >>> --- >>> docs/schemas/capability.rng | 28 >>> src/conf/capabilities.c | 17 + >>> 2 files changed, 45 insertions(+) >>> >> >> This output would be combined with part of existing patch2. >> > > Will be combined in next version patch. > >>> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng >>> index d61515c..67498f1 100644 >>> --- a/docs/schemas/capability.rng >>> +++ b/docs/schemas/capability.rng >>> @@ -314,6 +314,24 @@ >>> >>> >>> >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> >>> >>> >>> @@ -329,6 +347,16 @@ >>> >>> >>> >>> + >>> + >>> + >>> +llc_occupancy >>> +mbm_total_bytes >>> +mbm_local_bytes >> >> So these are the only 3 values you'll ever expect? Probably not a good idea >> to >> list them like this or the current algorithm is overkill looking for >> prefixed "llc_" >> and "mbm_" values. >> >> If "llc_somethingnew" shows up some day, then the schema is invalidated. >> > > Disagree. > I don't think the schema will be invalidated when new "llc_somethingnew" > comes. Libvirt only recognize these three feature names. Hmm... maybe not clear enough - see, virt-xml-validate. Using limits the choices or allowed values to that known list. So if some kernel some day adds llc_somethingnew, but the libvirt rng file for the customer isn't updated to include that, then the XML doesn't validate. Trying to think of something else similar that exists today, but nothing springs to mind. > > If a new hardware feature name comes, take your example, the > 'llc_somethingnew', > then without a function enabling, should we let it be shown here? > I think properly no. It makes sense to say libvirt only supports the enabled > hardware feature, not all hardware features. To get the new 'llc_somethingnew' > supported here, you need to make changes here and submit the patch. > >> If all you're supporting or care about is the 3 values, then each should be >> fetched >> separately. Just the names make me wonder if they come with some associated >> value that would be in some file. Perhaps answered in later patches. > > Only cares about the 3 values. Will apply more strict name rule to them in > source > code in next version patch. > > " Just the names make me wonder if they come with some associated > value that would be in some file. Perhaps answered in later patches." -- &g
Re: [libvirt] [PATCH 03/10] conf: Add CMT capability to host
> -Original Message-
> From: John Ferlan [mailto:jfer...@redhat.com]
> Sent: Wednesday, September 5, 2018 7:59 PM
> To: Wang, Huaqiang ; libvir-list@redhat.com
> Cc: Feng, Shaohe ; Niu, Bing ;
> Ding, Jian-feng ; Zang, Rui
> Subject: Re: [libvirt] [PATCH 03/10] conf: Add CMT capability to host
>
>
>
> On 08/27/2018 07:23 AM, Wang Huaqiang wrote:
> > CMT capability for each cache bank, includes -. Maximum CMT monitoring
> > groups(sharing with MBM) could be created,
> >which reflects the maximum hardware RMID count.
> > -. 'cache threshold'.
> > -. Statistical information of last level cache, the actual cache
> >occupancy.
> >
> > cache is splitted into 'bank's, each bank MAY have different cache
> > configuration, report cache monitoring capability in unit of cache bank.
> >
> > cache monitor capability is shown as below:
> >
> >
> >
> >
> > > maxAllocs='8'/>
> > +
> > +
> > +
> >
> >
> >
> > > maxAllocs='8'/>
> > +
> > +
> > +
> >
> >
> >
> >
> > Signed-off-by: Wang Huaqiang
> > ---
> > docs/schemas/capability.rng | 28
> > src/conf/capabilities.c | 17 +
> > 2 files changed, 45 insertions(+)
> >
>
> This output would be combined with part of existing patch2.
>
Will be combined in next version patch.
> > diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
> > index d61515c..67498f1 100644
> > --- a/docs/schemas/capability.rng
> > +++ b/docs/schemas/capability.rng
> > @@ -314,6 +314,24 @@
> >
> >
> >
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> >
> >
> >
> > @@ -329,6 +347,16 @@
> >
> >
> >
> > +
> > +
> > +
> > +llc_occupancy
> > +mbm_total_bytes
> > +mbm_local_bytes
>
> So these are the only 3 values you'll ever expect? Probably not a good idea
> to
> list them like this or the current algorithm is overkill looking for prefixed
> "llc_"
> and "mbm_" values.
>
> If "llc_somethingnew" shows up some day, then the schema is invalidated.
>
Disagree.
I don't think the schema will be invalidated when new "llc_somethingnew"
comes. Libvirt only recognize these three feature names.
If a new hardware feature name comes, take your example, the 'llc_somethingnew',
then without a function enabling, should we let it be shown here?
I think properly no. It makes sense to say libvirt only supports the enabled
hardware feature, not all hardware features. To get the new 'llc_somethingnew'
supported here, you need to make changes here and submit the patch.
> If all you're supporting or care about is the 3 values, then each should be
> fetched
> separately. Just the names make me wonder if they come with some associated
> value that would be in some file. Perhaps answered in later patches.
Only cares about the 3 values. Will apply more strict name rule to them in
source
code in next version patch.
" Just the names make me wonder if they come with some associated
value that would be in some file. Perhaps answered in later patches." -- not
understand.
> > +
> > +
> > +
> > +
> >
> >
> >
> > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index
> > 5280348..7932088 100644
> > --- a/src/conf/capabilities.c
> > +++ b/src/conf/capabilities.c
> > @@ -942,6 +942,23 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> >controls->max_allocation);
> > }
> >
> > +if (bank->monitor &&
> > +bank->monitor->nfeatures) {
> > +virBufferAsprintf(,
> > +
Re: [libvirt] [PATCH 03/10] conf: Add CMT capability to host
On 08/27/2018 07:23 AM, Wang Huaqiang wrote:
> CMT capability for each cache bank, includes
> -. Maximum CMT monitoring groups(sharing with MBM) could be created,
>which reflects the maximum hardware RMID count.
> -. 'cache threshold'.
> -. Statistical information of last level cache, the actual cache
>occupancy.
>
> cache is splitted into 'bank's, each bank MAY have different cache
> configuration, report cache monitoring capability in unit of cache bank.
>
> cache monitor capability is shown as below:
>
>
>
>
>
> +
> +
> +
>
>
>
>
> +
> +
> +
>
>
>
>
> Signed-off-by: Wang Huaqiang
> ---
> docs/schemas/capability.rng | 28
> src/conf/capabilities.c | 17 +
> 2 files changed, 45 insertions(+)
>
This output would be combined with part of existing patch2.
> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
> index d61515c..67498f1 100644
> --- a/docs/schemas/capability.rng
> +++ b/docs/schemas/capability.rng
> @@ -314,6 +314,24 @@
>
>
>
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
>
>
>
> @@ -329,6 +347,16 @@
>
>
>
> +
> +
> +
> +llc_occupancy
> +mbm_total_bytes
> +mbm_local_bytes
So these are the only 3 values you'll ever expect? Probably not a good
idea to list them like this or the current algorithm is overkill looking
for prefixed "llc_" and "mbm_" values.
If "llc_somethingnew" shows up some day, then the schema is invalidated.
If all you're supporting or care about is the 3 values, then each should
be fetched separately. Just the names make me wonder if they come with
some associated value that would be in some file. Perhaps answered in
later patches.
> +
> +
> +
> +
>
>
>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 5280348..7932088 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -942,6 +942,23 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
>controls->max_allocation);
> }
>
> +if (bank->monitor &&
> +bank->monitor->nfeatures) {
> +virBufferAsprintf(,
> + " + "maxAllocs='%u'>\n",
> + bank->monitor->cache_threshold,
> + bank->monitor->max_allocation);
> +for (j = 0; j < bank->monitor->nfeatures; j++) {
> +virBufferAdjustIndent(, 2);
> +virBufferAsprintf(,
> + "\n",
> + bank->monitor->features[j]);
> +virBufferAdjustIndent(, -2);
> +}
> +virBufferAddLit(, "\n");
> +}
> +
Not clear this data is at the right level, still.
John
> if (virBufferCheckError() < 0)
> return -1;
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
