<div>Martin:</div><div><br></div><div>Please hold on merge this first.</div><div><br></div><div>I found we still need some modification.</div>
<div></div>
<blockquote type="cite" style="border-left-style:solid;border-width:1px;margin-left:0px;padding-left:10px;"><span><div><div><blockquote type="cite"><div><span><div><div><div><br></div><div>Again, wrong indentation and unnecessary parentheses.</div><div><br></div><div>Otherwise it looks good, so ACK with those differences and reworded</div><div>commit message. Let me know if you agree and I can push it immediately.</div><div>The suggested diff follows:</div><div><br></div><div>diff --git i/docs/schemas/capability.rng w/docs/schemas/capability.rng</div><div>index 927fc184de41..e5cbfa362ec0 100644</div><div>--- i/docs/schemas/capability.rng</div><div>+++ w/docs/schemas/capability.rng</div><div>@@ -261,13 +261,7 @@</div><div> <attribute name='level'></div><div> <ref name='unsignedInt'/></div><div> </attribute></div><div>- <attribute name='type'></div><div>- <choice></div><div>- <value>both</value></div><div>- <value>code</value></div><div>- <value>data</value></div><div>- </choice></div><div>- </attribute></div><div>+ <ref name='cacheType'/></div><div> <attribute name='size'></div><div> <ref name='unsignedInt'/></div><div> </attribute></div><div>@@ -285,14 +279,8 @@</div><div> <attribute name='unit'></div><div> <ref name='unit'/></div><div> </attribute></div><div>- <attribute name='scope'></div><div>- <choice></div><div>- <value>both</value></div><div>- <value>code</value></div><div>- <value>data</value></div><div>- </choice></div><div>- </attribute></div><div>- <attribute name='max_allocation'></div><div>+ <ref name='cacheType'/></div><div>+ <attribute name='maxAllocs'></div><div> <ref name='unsignedInt'/></div><div> </attribute></div><div> </element></div><div>@@ -302,6 +290,16 @@</div><div> </element></div><div> </define></div><div><br></div><div>+ <define name='cacheType'></div><div>+ <attribute name='type'></div><div>+ <choice></div><div>+ <value>both</value></div><div>+ <value>code</value></div><div>+ <value>data</value></div><div>+ </choice></div><div>+ </attribute></div><div>+ </define></div><div>+</div><div> <define name='guestcaps'></div><div> <element name='guest'></div><div> <ref name='ostype'/></div><div>diff --git i/src/conf/capabilities.c w/src/conf/capabilities.c</div><div>index 8cd2957e9c88..3becc7e18c62 100644</div><div>--- i/src/conf/capabilities.c</div><div>+++ w/src/conf/capabilities.c</div><div>@@ -909,7 +909,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,</div><div> bool min_kilos = !(bank->controls[j]->min % 1024);</div><div> virBufferAsprintf(&controlBuf,</div><div> "<control min='%llu' unit='%s' "</div><div>- "scope='%s' max_allocation='%u'/>\n",</div><div>+ "type='%s' maxAllocs='%u'/>\n",</div><div> bank->controls[j]->min >> (min_kilos * 10),</div><div> min_kilos ? "KiB" : "B",</div><div> virCacheTypeToString(bank->controls[j]->scope),</div><div>@@ -920,12 +920,10 @@ virCapabilitiesFormatCaches(virBufferPtr buf,</div><div> virBufferAddLit(buf, ">\n");</div><div> virBufferAddBuffer(buf, &controlBuf);</div><div> virBufferAddLit(buf, "</bank>\n");</div><div>-</div><div> } else {</div><div> virBufferAddLit(buf, "/>\n");</div><div> }</div><div><br></div><div>- virBufferFreeAndReset(&controlBuf);</div><div> VIR_FREE(cpus_str);</div><div> }</div><div><br></div><div>@@ -1557,16 +1555,19 @@ virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)</div><div> VIR_FREE(ptr);</div><div> }</div><div><br></div><div>-/* test which TYPE of cache control supported</div><div>- * -1: don't support</div><div>- * 0: cat</div><div>- * 1: cdp</div><div>+/*</div><div>+ * This function tests which TYPE of cache control is supported</div><div>+ * Return values are:</div><div>+ * -1: not supported</div><div>+ * 0: CAT</div><div>+ * 1: CDP</div><div> */</div><div> static int</div><div> virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)</div><div> {</div><div> int ret = -1;</div><div> char *path = NULL;</div><div>+</div><div> if (virAsprintf(&path,</div><div> SYSFS_RESCTRL_PATH "/info/L%u",</div><div> bank->level) < 0)</div><div>@@ -1576,7 +1577,10 @@ virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)</div><div> ret = 0;</div><div> } else {</div><div> VIR_FREE(path);</div><div>- /* for CDP enabled case, CODE and DATA will show together */</div><div>+ /*</div><div>+ * If CDP is enabled, there will be both CODE and DATA, but it's enough</div><div>+ * to check one of those only.</div><div>+ */</div><div> if (virAsprintf(&path,</div><div> SYSFS_RESCTRL_PATH "/info/L%uCODE",</div><div> bank->level) < 0)</div><div>@@ -1597,13 +1601,14 @@ virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,</div><div> char *path = NULL;</div><div> char *cbm_mask = NULL;</div><div> char *type_upper = NULL;</div><div>+ unsigned int min_cbm_bits = 0;</div><div> virCapsHostCacheControlPtr control;</div><div><br></div><div> if (VIR_ALLOC(control) < 0)</div><div> goto cleanup;</div><div><br></div><div>- if ((scope > VIR_CACHE_TYPE_BOTH)</div><div>- && (virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0))</div><div>+ if (scope != VIR_CACHE_TYPE_BOTH &&</div><div>+ virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0)</div><div> goto cleanup;</div><div><br></div><div> if (virFileReadValueUint(&control->max_allocation,</div><div>@@ -1619,10 +1624,16 @@ virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,</div><div> type_upper ? type_upper: "") < 0)</div><div> goto cleanup;</div><div><br></div><div>+ if (virFileReadValueUint(&min_cbm_bits,</div><div>+ SYSFS_RESCTRL_PATH "/info/L%u%s/min_cbm_bits",</div><div>+ bank->level,</div><div>+ type_upper ? type_upper : "") < 0)</div><div>+ goto cleanup;</div><div>+</div><div> virStringTrimOptionalNewline(cbm_mask);</div><div><br></div><div> /* cbm_mask: cache bit mask, it's in hex, eg: fffff */</div><div>- control->min = bank->size / (strlen(cbm_mask) * 4);</div><div>+ control->min = min_cbm_bits * bank->size / (strlen(cbm_mask) * 4);</div><div><br></div></div></div></span></div></blockquote></div></div></span></blockquote><div><br></div><div>This could be wrong on new platform in Intel’s SKX CPU after check with platform guys.</div><div><br></div><div>The cbm_mask is “7ff” (11 bits) on SKX. I will refine this by counting the bits.</div><div><br></div><div>We can virFileReadValueString() then convert it to unsigned int, then count the bits of ‘1’.</div><div><br></div><div>thought ? or a new utils function in virfile is required?</div><div><br></div><div><div>I am not sure min_cbm_bits should be used.</div><div><br></div><div>Besides, on old platform (haswell), the min_cbm_bits is 2, but we still can specify a cbm like</div><div>“0x7” ) 3bit while do the cache allocation.</div><div><br></div><div>Later we need this on doing cache allocation(instead of read these value from the host again), we will</div><div>need</div><div>1. total cache size</div><div>2. what the cache size of 1 (CBM) bit stand for, so that we can calculate how many bits we want.</div><div><br></div><div>Maybe we need to add another filed like ’step’ (to indicate 1 bits stand for) ?</div><div><br></div><blockquote type="cite"><span><blockquote type="cite"><div></div></blockquote></span></blockquote></div><blockquote type="cite" style="border-left-style:solid;border-width:1px;margin-left:0px;padding-left:10px;"><span><div><div><blockquote type="cite"><div><span><div><div><div></div><div> control->scope = scope;</div></div></div></span></div></blockquote></div></div></span></blockquote><blockquote type="cite" style="border-left-style:solid;border-width:1px;margin-left:0px;padding-left:10px;"><span><div><div><blockquote type="cite"><div><span><div><div><div><br></div><div>@@ -1732,10 +1743,10 @@ virCapabilitiesInitCaches(virCapsPtr caps)</div><div> VIR_CACHE_TYPE_BOTH) < 0)</div><div> goto cleanup;</div><div> } else if (typeret == 1) {</div><div>- if ((virCapabilitiesGetCacheControl(bank,</div><div>- VIR_CACHE_TYPE_CODE) < 0) ||</div><div>- (virCapabilitiesGetCacheControl(bank,</div><div>- VIR_CACHE_TYPE_DATA) < 0))</div><div>+ if (virCapabilitiesGetCacheControl(bank,</div><div>+ VIR_CACHE_TYPE_CODE) < 0 ||</div><div>+ virCapabilitiesGetCacheControl(bank,</div><div>+ VIR_CACHE_TYPE_DATA) < 0)</div><div> goto cleanup;</div><div> }</div><div><br></div><div>diff --git i/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml w/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml</div><div>index c9f460d8a227..49aa0b98ca88 100644</div><div>--- i/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml</div><div>+++ w/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml</div><div>@@ -42,12 +42,12 @@</div><div> </topology></div><div> <cache></div><div> <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'></div><div>- <control min='768' unit='KiB' scope='code' max_allocation='8'/></div><div>- <control min='768' unit='KiB' scope='data' max_allocation='8'/></div><div>+ <control min='768' unit='KiB' type='code' maxAllocs='8'/></div><div>+ <control min='768' unit='KiB' type='data' maxAllocs='8'/></div><div> </bank></div><div> <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'></div><div>- <control min='768' unit='KiB' scope='code' max_allocation='8'/></div><div>- <control min='768' unit='KiB' scope='data' max_allocation='8'/></div><div>+ <control min='768' unit='KiB' type='code' maxAllocs='8'/></div><div>+ <control min='768' unit='KiB' type='data' maxAllocs='8'/></div><div> </bank></div><div> </cache></div><div> </host></div><div>diff --git i/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml w/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml</div><div>index 04a5eb895727..cb78b4ab788d 100644</div><div>--- i/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml</div><div>+++ w/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml</div><div>@@ -42,10 +42,10 @@</div><div> </topology></div><div> <cache></div><div> <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'></div><div>- <control min='768' unit='KiB' scope='both' max_allocation='4'/></div><div>+ <control min='1536' unit='KiB' type='both' maxAllocs='4'/></div><div> </bank></div><div> <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'></div><div>- <control min='768' unit='KiB' scope='both' max_allocation='4'/></div><div>+ <control min='1536' unit='KiB' type='both' maxAllocs='4'/></div></div></div></span></div></blockquote></div></div></span></blockquote><div> </div><blockquote type="cite" style="border-left-style:solid;border-width:1px;margin-left:0px;padding-left:10px;"><span><div><div><blockquote type="cite"><div><span><div><div><div> </bank></div><div> </cache></div><div> </host></div><div>--</div><div>Martin</div></div></div></span>
</div></blockquote><div>
<br>
</div>
</div></div></span>
</blockquote>
<div>
<br>
</div>