Hi Shameer, On 21/06/2019 16:57, Shameerali Kolothum Thodi wrote: >> -----Original Message----- >> From: James Morse [mailto:james.morse@xxxxxxx] >> Subject: Re: [RFC PATCH 2/2] ACPI / PPTT: cacheinfo: Label caches based on >> fw_token >>> and noted that >>> on our HiSilicon platform all the L3 cache were labeled with the same Id. >> Debugging> revealed that the above leaf node check was removed in this >> branch[2] which makes >>> the min_physid calculation going wrong. >>> Just wondering is there any particular reason >>> for removing the check or the branch is not carrying the latest patch? >> >> Nope, that's a bug. >> >> Jeremy Linton's review feedback[0] was that that PROCESSOR_ID_VALID flag >> can't be relied >> on. It looks like I over-zealously removed the whole if(), and this doesn't cause a >> problem with my pptt so I didn't notice. >> >> I've fixed it locally, I've also pushed a fix to those branches, but it will get folded >> in >> next time I push a branch. > > Thanks for that. > > Apart from the above, I have come across few other issues as well and had some > temporary fixes to the branch here[0]. This is encountered while trying to get the > resctrl fs mounted and attempted a cqm test run using resctrl_tests tool. Thanks! I haven't run that on the model yet as I want it to get the monitors working first. If you are seeing bugs in that monitor code, beware you're the only person to ever run it! > The fixes may not be proper ones, but I think it will give you an idea. Please take a > look and let me know your thoughts. {,!} exposed_mon_capable, yup that's a typo. the evt_list being uninitialised is because that code has never run, as noted in the KNOWN_ISSUES, (The model doesn't expose have any of the mpam counters...) Issues around component->resource mapping will disappear as I've re-written all that to fix issues around picking the same resource twice. The domid bitfield not being big enough for the width of the cacheinfo id field looks like a bug in the existing resctrl code. Could you spin that as a patch against mainline? It won't affect any x86 system, but I don't want to 'fix' anything as part of the mpam support. We almost certainly need to compress the cache-id numbers down to {0,1,2} if only so we haven't filled all the exposed bits on day-1. (so it might not matter for arm64 either...) Thanks, James
