Received-SPF: pass (google.com: domain of linux-kernel+bounces-136211-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <fb4abe11-5859-4a53-a4db-bb4e59a571c6@intel.com>
Date: Mon, 8 Apr 2024 20:41:04 -0700
User-Agent: Mozilla Thunderbird
From: Reinette Chatre <reinette.chatre@intel.com>
Subject: Re: [PATCH v1 29/31] fs/resctrl: Add boiler plate for external
 resctrl code
To: James Morse <james.morse@arm.com>, <x86@kernel.org>,
	<linux-kernel@vger.kernel.org>
CC: Fenghua Yu <fenghua.yu@intel.com>, Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, H Peter Anvin
	<hpa@zytor.com>, Babu Moger <Babu.Moger@amd.com>,
	<shameerali.kolothum.thodi@huawei.com>, D Scott Phillips OS
	<scott@os.amperecomputing.com>, <carl@os.amperecomputing.com>,
	<lcherian@marvell.com>, <bobo.shaobowang@huawei.com>,
	<tan.shaopeng@fujitsu.com>, <baolin.wang@linux.alibaba.com>, Jamie Iles
	<quic_jiles@quicinc.com>, Xin Hao <xhao@linux.alibaba.com>,
	<peternewman@google.com>, <dfustini@baylibre.com>, <amitsinght@marvell.com>,
	David Hildenbrand <david@redhat.com>, Rex Nie <rex.nie@jaguarmicro.com>,
	"Dave Martin" <dave.martin@arm.com>
References: <20240321165106.31602-1-james.morse@arm.com>
 <20240321165106.31602-30-james.morse@arm.com>
Content-Language: en-US
In-Reply-To: <20240321165106.31602-30-james.morse@arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SVU3SlZRTGs4Qzc0U1kxQWlCTFVBOEc3cm11VGFtZUVVQTh2QU02YXhsYVBH?=
 =?utf-8?B?cW1VYkxtS0ZhcjRyOXJNdkJvQWlOWnVzNGNkcnZibzR0dnhFR0Y2WExiaDA5?=
 =?utf-8?B?Z3dFT0RVbVBmUWVmZEE2a09xZmdzMy9ZRlhtUGJyV29QSjk1eTVEZHVwL2w5?=
 =?utf-8?B?Y0tVRDZkZXJwQ3I1Z0M1b3E0akJOeFoxTFFSSXYvbjA2aDFXN0ppcm5Td3Ev?=
 =?utf-8?B?K0hTL2VOMW1BQktlbnNreTBMc3ZmL1AvY0pzaUl4dDRMbVZ3a3huOXNIYjNQ?=
 =?utf-8?B?SklySCt0Z1Q3QkJUUkJzMHVKYjI5YVdpR2RhOFk2UGhXVk1TUkF1aG1BRXV6?=
 =?utf-8?B?MENkTWYweldGalU5SEhNUnhldi94M0QvbnNCUThySVA2Tk1sWTFoR2lKdVJF?=
 =?utf-8?B?Vi9CNU1CZG9OL3NUQnVFbERQYVNJZEFyaTRFQlVSTXA1eEs3VXRnUE9BTzVj?=
 =?utf-8?B?QjJhRmsxUVdEc1doemdjQTNQNTNoUXRnZ3VUZGJtVzZ5MGo3SU5ZbnM0OEg5?=
 =?utf-8?B?YVJXUmJBTnh4R09MQUprVkFtQlg1cS9GZkhBTE1FcDlDMjBTRG5hMEwrVzBh?=
 =?utf-8?B?MjFHcEJ1OEtuMjNOcHhkUVo4cEltNnlhM3JKNFhRVVRlcmU4U25QMGNJbmxV?=
 =?utf-8?B?Sjl0bU5JV3JxVkcvUUpBSnlXSTBwU0RkMnJ3Q0hDWld1bDk1akJMcVNKVmx2?=
 =?utf-8?B?cEdYR2VUZS9IdUI5dk44R0JJNUtIeGFDcWFFcnhyd0lzVElCbFErNDduQkxu?=
 =?utf-8?B?RnVSVXN0dkh1UXlqUExnM2tpUTdhOHFzKzcvdHVYN0h2NUo4dkx2bS8xc3Rv?=
 =?utf-8?B?RlpJNmNsb1pjc0tVWXlEV3NXZ1pTMVRUeTdHNElUTXZqVmhqWit0OGtQRmFw?=
 =?utf-8?B?ZzQvQjNZUTZyaGpFVUFNNU51STFFUEhhVitqYVVtQ2tMaVpaQ2RBOUMxMm1L?=
 =?utf-8?B?SFpuNGtaQWpqbmVCbFZwRC9pd2dtTWpDTzZyb2JWdDBFdjZIQkZtN3Z6cy9Z?=
 =?utf-8?B?NmhRNklwQTQycWpNZlZyb05vcUhmSElDcy9rWWRGQlNGVWNUbVg0NTU4YXF0?=
 =?utf-8?B?dnp4RUdDb0piQzJTVkxrR0NnNVJrWmEvOVdJZXNQQ1pnaWZ0MVYzVkJudlRp?=
 =?utf-8?B?V3p6d0hZKzdsT2E4dHBBNGR4SVJUeldOT2Rkak93UEJDeXl6aTZWVlBGUFVr?=
 =?utf-8?B?S1FBSStGdVFBbU1rNjBQTzhrMVNrcWxMWkRrT2VJeGJ6UUc1MmZESWxHeDVT?=
 =?utf-8?B?ZWxKTW1CRFhtWGN4d01YS3BlK1hQL1lRWWZ6elI5NmRHeVBUMjBDcGI2TXJE?=
 =?utf-8?B?VjBKanJpYmNEMm0xMnJ2K1Y2VVU1dkVxcTFLT2VPdVlJYkhVVzdDNHVTZjRw?=
 =?utf-8?B?OWhNdnVrUFZpS2VzbElGQzhBL1NQRlF2N0hValJyWXJkMDlyQ1A1ckIrRmkw?=
 =?utf-8?B?bm5KUjE5Q0U5VEdEQWFyZE1iOFNGcFM5MUVsbXVUSjdZQXpBTzVOV3pxcnFV?=
 =?utf-8?B?cldCc24xaGFwZk1iQUtIbUdoTHAwRXlwZmF5aDFkcDJWUjZjZldxSkwvaTNa?=
 =?utf-8?B?dTVuNk1sUkhKN2Y2U0U4Q0Q3Ykk0QUlMM3NZWlpJREQxdGF1bndkTFZYVEYy?=
 =?utf-8?B?ZDk3bFpPRU9hcUdLK2dPYnducnlRZkhrUGVPM2FJa2g1NUZ3YnZrSHBoOUZa?=
 =?utf-8?B?YUhiNklJcmExK3Uya1BCcVFhenNEOE9KVkhmUHhFSTdLY0xZMHB6cmk1WWI2?=
 =?utf-8?B?VGpsOXc4TGhKRE9hOURaUG50REU1SGV1Y3hka2gvOGltQ0g3UWM1Qjl5R2tz?=
 =?utf-8?B?SHoxQ3Ura01SR0JpZURHRjFkZWpra0piVTY0bmt1ZGhmOGNPSGtodVVzL0hC?=
 =?utf-8?B?enFQWllJdS9hb2FBcFFLcFBOZ3RHRTd6cFlHS1QwMHBrQXZOVm5JOHVRWEZY?=
 =?utf-8?B?ZVYxeDYwZVcrdThaK3ovOHpPc3hXeXhHeXdOczNBZkEyVUQ0d25YSW9uVFRn?=
 =?utf-8?B?UlRCWDBHNjVrVjRHNGMzcGlPcHNJdDB2bUdHMjhnUjZtVVBmMzZ6UFhuK3pB?=
 =?utf-8?B?QkZPNGxRa05wT1JrbU9ORzJnKzY0RTRnNXpCeUhYbzREeWJ2US9hcDVSWUtr?=
 =?utf-8?B?ZUFONUc2ZnNWWXgwSytJOVVzSTE1Y1VIamNCWkV1bmRDVUd3TEJxSkxPNnBi?=
 =?utf-8?B?ZlE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: c2cbb1a4-51e3-45b6-e27e-08dc5846e38f
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Apr 2024 03:41:07.0042
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Htputo1AOS3vKTnYaXjQCN8zd/JwpOYpggjrhcK52Jh0ttZvg7pHLL4hbxI+o06PH1h3uJbK0PeL6DFyY7TF8vJprLZOJoDdd4FyTV1dCBo=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4666
X-OriginatorOrg: intel.com

Hi James,

On 3/21/2024 9:51 AM, James Morse wrote:
> Add Makefile and Kconfig for fs/resctrl. Add ARCH_HAS_CPU_RESCTRL
> for the common parts of the resctrl interface and make X86_CPU_RESCTRL
> depend on this.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>  MAINTAINERS              |  1 +
>  arch/Kconfig             |  8 ++++++++
>  arch/x86/Kconfig         | 10 +++-------
>  fs/Kconfig               |  1 +
>  fs/Makefile              |  1 +
>  fs/resctrl/Kconfig       | 23 +++++++++++++++++++++++
>  fs/resctrl/Makefile      |  3 +++
>  fs/resctrl/ctrlmondata.c |  0
>  fs/resctrl/internal.h    |  0
>  fs/resctrl/monitor.c     |  0
>  fs/resctrl/psuedo_lock.c |  0
>  fs/resctrl/rdtgroup.c    |  0
>  include/linux/resctrl.h  |  4 ++++
>  13 files changed, 44 insertions(+), 7 deletions(-)
>  create mode 100644 fs/resctrl/Kconfig
>  create mode 100644 fs/resctrl/Makefile
>  create mode 100644 fs/resctrl/ctrlmondata.c
>  create mode 100644 fs/resctrl/internal.h
>  create mode 100644 fs/resctrl/monitor.c
>  create mode 100644 fs/resctrl/psuedo_lock.c
>  create mode 100644 fs/resctrl/rdtgroup.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5621dd823e79..c49090e9c777 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18543,6 +18543,7 @@ S:	Supported
>  F:	Documentation/arch/x86/resctrl*
>  F:	arch/x86/include/asm/resctrl.h
>  F:	arch/x86/kernel/cpu/resctrl/
> +F:	fs/resctrl/
>  F:	include/linux/resctrl*.h
>  F:	tools/testing/selftests/resctrl/
>  
> diff --git a/arch/Kconfig b/arch/Kconfig
> index fd18b7db2c77..131d874d6738 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -1406,6 +1406,14 @@ config STRICT_MODULE_RWX
>  config ARCH_HAS_PHYS_TO_DMA
>  	bool
>  
> +config ARCH_HAS_CPU_RESCTRL
> +	bool
> +	help
> +	  The 'resctrl' filesystem allows CPU controls of shared resources
> +	  such as caches and memory bandwidth to be configured. An architecture
> +	  selects this if it provides the arch-specific hooks for the filesystem
> +	  and needs the per-task CLOSID/RMID properties.

Should it mention monitoring capabilities?

> +
>  config HAVE_ARCH_COMPILER_H
>  	bool
>  	help
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index e071e564452e..cb043543f088 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -479,8 +479,10 @@ config GOLDFISH
>  config X86_CPU_RESCTRL
>  	bool "x86 CPU resource control support"
>  	depends on X86 && (CPU_SUP_INTEL || CPU_SUP_AMD)
> +	depends on MISC_FILESYSTEMS
>  	select KERNFS

Do both X86_CPU_RESCTRL and RESCTRL_FS need to select KERNFS?

> -	select PROC_CPU_RESCTRL		if PROC_FS
> +	select ARCH_HAS_CPU_RESCTRL
> +	select RESCTRL_FS
>  	select RESCTRL_FS_PSEUDO_LOCK
>  	help
>  	  Enable x86 CPU resource control support.
> @@ -507,12 +509,6 @@ config X86_FRED
>  	  ring transitions and exception/interrupt handling if the
>  	  system supports.
>  
> -config RESCTRL_FS_PSEUDO_LOCK
> -	bool
> -	help
> -	  Software mechanism to pin data in a cache portion using
> -	  micro-architecture specific knowledge.
> -
>  if X86_32
>  config X86_BIGSMP
>  	bool "Support for big SMP systems with more than 8 CPUs"
> diff --git a/fs/Kconfig b/fs/Kconfig
> index a46b0cbc4d8f..d8a36383b6dc 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -331,6 +331,7 @@ source "fs/omfs/Kconfig"
>  source "fs/hpfs/Kconfig"
>  source "fs/qnx4/Kconfig"
>  source "fs/qnx6/Kconfig"
> +source "fs/resctrl/Kconfig"
>  source "fs/romfs/Kconfig"
>  source "fs/pstore/Kconfig"
>  source "fs/sysv/Kconfig"
> diff --git a/fs/Makefile b/fs/Makefile
> index 6ecc9b0a53f2..da6e2d028722 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -129,3 +129,4 @@ obj-$(CONFIG_EFIVAR_FS)		+= efivarfs/
>  obj-$(CONFIG_EROFS_FS)		+= erofs/
>  obj-$(CONFIG_VBOXSF_FS)		+= vboxsf/
>  obj-$(CONFIG_ZONEFS_FS)		+= zonefs/
> +obj-$(CONFIG_RESCTRL_FS)	+= resctrl/
> diff --git a/fs/resctrl/Kconfig b/fs/resctrl/Kconfig
> new file mode 100644
> index 000000000000..36a1ddbe6c21
> --- /dev/null
> +++ b/fs/resctrl/Kconfig

Could you please review the contents of this file for
appropriate line length and consistent tab usage?

> @@ -0,0 +1,23 @@
> +config RESCTRL_FS
> +	bool "CPU Resource Control Filesystem (resctrl)"
> +	depends on ARCH_HAS_CPU_RESCTRL
> +	select KERNFS
> +	select PROC_CPU_RESCTRL               if PROC_FS
> +	help
> +	  Resctrl is a filesystem interface
> +	  to control allocation and
> +	  monitoring of system resources
> +	  used by the CPUs.
> +
> +config RESCTRL_FS_PSEUDO_LOCK
> +	bool
> +	help
> +          Software mechanism to pin data in a cache portion using
> +          micro-architecture specific knowledge.
> +
> +config RESCTRL_RMID_DEPENDS_ON_CLOSID
> +	bool
> +	help
> +	  Enable by the architecture when the RMID values depend on the CLOSID.
> +	  This causes the closid allocator to search for CLOSID with clean
> +	  RMID.
> diff --git a/fs/resctrl/Makefile b/fs/resctrl/Makefile
> new file mode 100644
> index 000000000000..10fcfb0fdb10
> --- /dev/null
> +++ b/fs/resctrl/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_RESCTRL_FS)		+= rdtgroup.o ctrlmondata.o monitor.o
> +obj-$(CONFIG_RESCTRL_FS_PSEUDO_LOCK)	+= psuedo_lock.o
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/fs/resctrl/psuedo_lock.c b/fs/resctrl/psuedo_lock.c

pseudo_lock.c

> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 5da55e58f229..f786ffceeda3 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -8,6 +8,10 @@
>  #include <linux/pid.h>
>  #include <linux/resctrl_types.h>
>  
> +#ifdef CONFIG_ARCH_HAS_CPU_RESCTRL
> +#include <asm/resctrl.h>
> +#endif
> +
>  /* CLOSID, RMID value used by the default control group */
>  #define RESCTRL_RESERVED_CLOSID		0
>  #define RESCTRL_RESERVED_RMID		0

Reinette