From mboxrd@z Thu Jan 1 00:00:00 1970 From: jeremy.linton@arm.com (Jeremy Linton) Date: Thu, 19 Oct 2017 09:24:12 -0500 Subject: [PATCH v3 1/7] ACPI/PPTT: Add Processor Properties Topology Table parsing In-Reply-To: <2605aa54-95f7-4619-e632-0a95a68e5fb0@caviumnetworks.com> References: <20171012194856.13844-1-jeremy.linton@arm.com> <20171012194856.13844-2-jeremy.linton@arm.com> <38c63cfd-4c13-9461-0d4a-74b0943be0fd@semihalf.com> <2ce92b90-fc2c-3fff-a3e3-b9517057ab2c@arm.com> <4ee4af4d-d987-33f9-4d55-67bfede3ebfa@caviumnetworks.com> <47d8ef93-a3ce-6d40-f8db-2bc7d527ac6f@arm.com> <2605aa54-95f7-4619-e632-0a95a68e5fb0@caviumnetworks.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, On 10/19/2017 12:18 AM, Tomasz Nowicki wrote: > On 18.10.2017 19:30, Jeremy Linton wrote: >> On 10/18/2017 05:24 AM, Tomasz Nowicki wrote: >>> On 18.10.2017 07:39, Tomasz Nowicki wrote: >>>> On 17.10.2017 17:22, Jeremy Linton wrote: >>>>> On 10/17/2017 08:25 AM, Tomasz Nowicki wrote: >>>>>> On 12.10.2017 21:48, Jeremy Linton wrote: (trimming) >>>>>>> +??????????? if (*found != NULL) >>>>>>> +??????????????? pr_err("Found duplicate cache level/type unable >>>>>>> to determine uniqueness\n"); >>> >>> Actually I still see this error messages in my dmesg. It is because >>> the following ThunderX2 per-core L1 and L2 cache hierarchy: >>> >>> Core >>> ??------------------ >>> |????????????????? | >>> | L1i -----??????? | >>> |???????? |??????? | >>> |????????? ----L2? | >>> |???????? |??????? | >>> | L1d -----??????? | >>> |????????????????? | >>> ??------------------ >>> >>> In this case we have two paths which lead to L2 cache and hit above >>> case. Is it really error case? >> >> No, but its not deterministic unless we mark the node, which doesn't >> solve the problem of a table constructed like >> >> L1i->L2 (unified) >> L1d->L2 (unified) >> >> or various other structures which aren't disallowed by the spec and >> have non-deterministic real world meanings, anymore than constructing >> the table like: >> >> L1i >> Lid->L2(unified) >> >> which I tend to prefer because with a structuring like that it can be >> deterministic (and in a way actually represents the non-coherent >> behavior of (most?) ARM64 core's i-caches, as could be argued the >> first example if the allocation policies are varied between the L2 >> nodes). >> >> The really ugly bits here happen if you add another layer: >> >> L1i->L2i-L3 >> L1d------^ >> >> which is why I made that an error message, not including the fact that >> since the levels aren't tagged the numbering and meaning isn't clear. >> >> (the L1i in the above example might be better called an L0i to avoid >> throwing off the reset of the hierarchy numbering, also so it could be >> ignored). >> >> Summary: >> >> I'm not at all happy with this specification's attempt to leave out >> pieces of information which make parsing things more deterministic. In >> this case I'm happy to demote the message level, but not remove it >> entirely but I do think the obvious case you list shouldn't be the >> default one. >> >> Lastly: >> >> I'm assuming the final result is that the table is actually being >> parsed correctly despite the ugly message? > > Indeed, the ThunderX2 PPTT table is being parsed so that topology shown > in lstopo and lscpu is correct. Great. Also, I think this is a better change: if ((*found != NULL) && (*found != cache)) pr_err("Found duplicate cache level/type unable to determine uniqueness\n"); Which if its a duplicate node/type at the given level the message is just suppressed. It will of course still trigger in cases like: L1d->L2 l1i->L2 or other odd cases.