Hi Dave, On 27/08/2025 12:01, Dave Martin wrote: > <super-pedantic mode enabled> Uh oh! > (Since this likely be people's go-to patch for understanding what MPAM > is, it is probably worth going the extra mile.) > > On Fri, Aug 22, 2025 at 03:29:48PM +0000, James Morse wrote: >> The bulk of the MPAM driver lives outside the arch code because it >> largely manages MMIO devices that generate interrupts. The driver >> needs a Kconfig symbol to enable it, as MPAM is only found on arm64 > > Prefer -> "[...] to enable it. As MPAM is only [...]" > >> platforms, that is where the Kconfig option makes the most sense. > > It could be clearer what "where" refers to, here. Sure, > Maybe reword from ", that is [...]" -> ", the arm64 tree is the most > natural home for the Kconfig option." > > (Or something like that.) Sure, >> This Kconfig option will later be used by the arch code to enable >> or disable the MPAM context-switch code, and registering the CPUs > > Nit: "registering" -> "to register" > >> properties with the MPAM driver. > > Nit: "CPUs properties" -> "properties of CPUs" ? > > (Maybe there was just a missed apostrophe, but it may be more readable > here if written out longhand.) Done, it just takes one person to think its clearer! >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index e9bbfacc35a6..658e47fc0c5a 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -2060,6 +2060,23 @@ config ARM64_TLB_RANGE >> ARMv8.4-TLBI provides TLBI invalidation instruction that apply to a >> range of input addresses. >> >> +config ARM64_MPAM >> + bool "Enable support for MPAM" >> + help > > <pedantic mode on> > >> + Memory Partitioning and Monitoring is an optional extension >> + that allows the CPUs to mark load and store transactions with > > Nit: "memory transactions" ? Sure, > (I'm wondering whether there are some transactions such as atomic > exchanges that are not neatly characterised as "load" or "store". > Possibly MPAM labels some transactions that really are neither.) Equally instruction fetch and possibly even CMOs get these labels. I wanted something other than 'transactions' so it wasn't confused with transactional memory - and traffic seemed to vauge. I don't think anyone expects a formal definition in the Kconfig text... >> + labels for partition-id and performance-monitoring-group. > > Nit: the hyphenation suggests that these are known terms (in this > specific, hyphenated, form) with specific definitions somewhere. > I don't think that this is the case? At least, I have not seen the > terms presented in this way anywhere else. > > Also, the partition ID is itself a label, so "label for partition-id" > is a tautology. > > How about: > > --8<-- > > Memory System Resource Partitioning and Monitoring (MPAM) is an > optional extension to the Arm architecture that allows each > transaction issued to the memory system to be labelled with a > Partition identifier (PARTID) and Performance Monitoring Group > identifier (PMG). > > -->8-- Done, > (Yes, that really seems to be what MPAM stands for in the published > specs. That's quite a mounthful, and news to me... I can't say I paid > much attention to the document titles beyond "MPAM"!) > >> + System components, such as the caches, can use the partition-id >> + to apply a performance policy. MPAM monitors can use the > > What is a "performance policy"? A bunch of controls, the value of which reflect some kind of policy. > The MPAM specs talk about resource controls; it's probably best to > stick to the same terminology. > >> + partition-id and performance-monitoring-group to measure the >> + cache occupancy or data throughput. > > So, how about something like: > > --8<-- > > Memory system components, such as the caches, can be configured with > policies to control how much of various physical resources (such as > memory bandwidth or cache memory) the transactions labelled with each > PARTID can consume. Depending on the capabilities of the hardware, > the PARTID and PMG can also be used as filtering criteria to measure > the memory system resource consumption of different parts of a > workload. > > -->8-- Done, > (Where "Memory system components" is used in a generic sense and so not > capitalised.) (I can't wait for the Memory System Component on the Memory Side Cache!) >> + >> + Use of this extension requires CPU support, support in the >> + memory system components (MSC), and a description from firmware > > But here, we are explicitly using an architectural term now, so > > "Memory System Components" (MSC) > > makes sense. > >> + of where the MSC are in the address space. > > Prefer "MSCs" ? (Not everyone agrees about whether TLAs are > pluralisable but it is easier on the reader if "are" has an obviously > plural noun to bind to.) Sure, Thanks, James
