linux-kernel - Re: [RFC PATCH mpam mpam/snapshot/v6.12-rc1 v3 0/5] arm_mpam: Introduce the Narrow-PARTID feature for MPAM driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z3bCxn4L0SsxIMw8@e133380.arm.com>
Date: Thu, 2 Jan 2025 16:45:58 +0000
From: Dave Martin <Dave.Martin@....com>
To: Zeng Heng <zengheng4@...wei.com>
Cc: james.morse@....com, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, jonathan.cameron@...wei.com,
	xiexiuqi@...wei.com,
	"Wangshaobo (bobo)" <bobo.shaobowang@...wei.com>
Subject: Re: [RFC PATCH mpam mpam/snapshot/v6.12-rc1 v3 0/5] arm_mpam:
 Introduce the Narrow-PARTID feature for MPAM driver

On Fri, Dec 20, 2024 at 06:59:31PM +0800, Zeng Heng wrote:
> 
> 
> On 2024/12/13 0:17, Dave Martin wrote:
> > Hi,
> > 
> > On Sat, Dec 07, 2024 at 05:21:31PM +0800, Zeng Heng wrote:

[...]

> > > For a mixture of MSCs system, for MSCs that do not support narrow-partid,
> > > we use the PARTIDs exceeding the number of closids as reqPARTIDs for
> > > expanding the monitoring groups.
> > 
> > Mixed systems still seem not to be handled completely by this series?
> > 
> > You do cope with the scenario where some MSCs support Narrowing and
> > some do not, but there is still the problem of incompatible controls.
> > 
> > If a non-Narrowing MSC has controls that are not of the "partition
> > bitmap" type, then splitting a resctrl control group across multiple
> > PARTIDs is going to change the hardware regulation behaviour.  There
> > does not seem to be any way to work around this be programming
> > different control values (for example).  So, there may be over- or
> > under-allocation of resources compared with what the user requests in
> > resctrlfs.
> > 
> > So, I think there is still a need to check which controls are present,
> > and either disable the use of non-identity intPARTID<->reqPARTID
> > mappings if incompatible controls are present (or don't expose those
> > controls to resctrl).
> > 
> > (If you were not trying to address this issue yet then that is not a
> > problem for an RFC, but it is best to be clear about the
> > limitations...)
> > 
> 
> In the v3, the limitation you mentioned has not yet been handled.
> V3 has not yet determined how to define this limitation well. Additionally,
> for v3, I was still uncertain whether a large-scale
> refactoring was still necessary at that time.
> 
> In fact, I take this limitation very seriously, and I will try to define
> the limitation in next version, explaining it separately as a distinct
> patch.

Fair enough.  I just wanted to make sure that this issue is not missed
(since this is where a lot of the complexity comes from!)


[...]

> > This approach looks reasonable overall, and in this version the changes
> > do seem to be better localised in the mpam_resctrl.c glue code now.
> > 
> > I had also been working on a similar approach, so I have posted it for
> > comparison [1] -- though the two approaches are doing pretty much the
> > same thing, some details differ.
> > 
> > (Note, I have not addressed PARTID Narrowing at all yet; however,
> > I think more thought is needed for that.)
> > 
> 
> Yes, localize the narrow-partid feature within mpam_resctrl.c file is
> the biggest restructuring improvement in this version. Of course, thanks
> for your meticulous review and insightful comments.
> 
> In the meanwhile, I have roughly read through the patch set you sent
> out, especially patch 4 (arm_mpam: Introduce flexible CLOSID/RMID
> translation). If I have any comments worth discussing, I will try to
> share them.

I would appreciate that, thanks!

> 
> > 
> > Note: Are there bisection issues with some of the patches in the
> > series?  It looks like not all of the ID conversions are applied in the
> > same patch, so I'm wondering whether strange behaviour may be seen at
> > the intermediate commits.
> > 
> 
> The original intention of splitting the patch set was to facilitate review.
> I will merge the part of patch 5 into patch 2 and consider
> making the patch set be friendly for bisection issues.
> 
> 
> Best regards,
> Zeng Heng

Understood.

James may have a view on how important bisectability is, since this is
not upstream code -- but it is probably a good idea to maintain full
bisectability if at all possible.

Cheers
---Dave