Hi Borislav, On 8/3/2019 2:44 AM, Borislav Petkov wrote: > On Fri, Aug 02, 2019 at 01:11:13PM -0700, Reinette Chatre wrote: >> This patch only makes it possible to determine whether cache is >> inclusive for some x86 platforms while all platforms of all >> architectures are given visibility into this new "inclusive" cache >> information field within the global "struct cacheinfo". > > And this begs the question: do the other architectures even need that > thing exposed? As it is now, this is x86-only so I'm wondering whether > adding all that machinery to the generic struct cacheinfo is even needed > at this point.
I do not know if more users would appear in the future but the goal of this patch is to make this information available to resctrl. Since there are a few ways to do so I really appreciate your guidance on how to do this right. I'd be happy to make needed changes. > TBH, I'd do it differently: read CPUID at init time and cache the > information whether the cache is inclusive locally and be done with it. > It is going to be a single system-wide bit anyway as I'd strongly assume > cache settings like inclusivity should be the same across the whole > system. I've been digesting your comment and tried a few options but I have been unable to come up with something that fulfill all your suggestions - specifically the "single system-wide bit" one. These areas of code are unfamiliar to me so I am not confident what I came up with for other suggestions are the right way either. So far it seems I can do the following: Introduce a new bitfield into struct cpuinfo_x86. There is one existing bitfield in this structure ("initialized") so we could add a new one ("x86_cache_l3_inclusive"?) just before it. With this information included in this struct it becomes available via the global boot_cpu_data, this seems to address the "system-wide bit" but this struct is also maintained for all other CPUs on the system so may not be what you had in mind (not a "single system-wide bit")? If proceeding with inclusion into struct cpuinfo_x86 this new bitfield can be initialized within init_intel_cacheinfo(). There are a few cache properties within cpuinfo_x86 and they are initialized in a variety of paths. init_intel_cacheinfo() is initialized via init_intel() and it already has the needed CPUID information available making initialization here straight forward. Alternatively there is also identify_cpu() that also initializes many cache properties ... but would need some more code to obtain needed values. Finally, if the above is done, the resctrl code could just refer to this new property directly as obtained from boot_cpu_data.x86_cache_l3_inclusive What do you think? > > When the other arches do need it, we can extract that info "up" into the > generic layer. Reinette