linux-kernel - RE: [PATCH v18 06/17] x86/resctrl: Introduce snc_nodes_per_l3

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]

Date: Thu, 23 May 2024 23:18:53 +0000
From: "Luck, Tony" <tony.luck@...el.com>
To: "Chatre, Reinette" <reinette.chatre@...el.com>, "Yu, Fenghua"
	<fenghua.yu@...el.com>, "Wieczor-Retman, Maciej"
	<maciej.wieczor-retman@...el.com>, Peter Newman <peternewman@...gle.com>,
	James Morse <james.morse@....com>, Babu Moger <babu.moger@....com>, "Drew
 Fustini" <dfustini@...libre.com>, Dave Martin <Dave.Martin@....com>
CC: "x86@...nel.org" <x86@...nel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "patches@...ts.linux.dev"
	<patches@...ts.linux.dev>
Subject: RE: [PATCH v18 06/17] x86/resctrl: Introduce snc_nodes_per_l3_cache

> > So (in arch/x86/kernel/cpu/resctrl/monitor.c)
> >
> > static int snc_nodes_per_l3_cache = 1;
> > 
> > Set and use only in this (arch specific) file.
>
> Since this series initializes this value in
> arch/x86/kernel/cpu/resctrl/core.c it is not clear to
> me from just this one line how you envision the changes.

v18 did the initialization in core.c. But since SNC is all about monitor
features it looks more logical to do this here:

resctrl_late_init()
    get_rdt_resources()
	get_rdt_mon_resources()
	    rdt_get_mon_l3_config()  <<<< Do SNC enumeration here
	    

> Just to be clear ... when I refer to arch specific and
> filesystem code I am considering how this series integrates with [1]
> since that is the direction resctrl is headed in.
> Being "arch specific" thus does not require that it be moved into
> monitor.c - it could be added to arch/x86/kernel/cpu/resctrl/internal.h
> where it can remain after the fs/arch split.

The logical place to convert from logical RMID to physical RMID looks
to be in __rmid_read(). Just need to pass in the domain pointer (from
both resctrl_arch_reset_rmid() and resctrl_arch_rmid_read().
>
> It will be very helpful if you view your series while taking
> [1] into account. For example, when looking at [1] you will see that
> mon_event_count() and __mon_event_count() are resctrl filesystem
> functions. When you consider that it should be clear that adding
> an arch specific get_node_rmid() between these functions will make
> the arch/fs split more difficult.

I'll try to keep that in mind as I rework my series. In v18 my "sum" code
went into __mon_event_count().  I'll see if I can push that down another
level.

-Tony