Received-SPF: pass (google.com: domain of linux-kernel+bounces-195885-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <66468aa6-e0a2-4c41-99b1-741f574c5fc1@intel.com>
Date: Thu, 30 May 2024 13:27:31 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v19 18/20] x86/resctrl: Enable RMID shared RMID mode on
 Sub-NUMA Cluster (SNC) systems
To: Tony Luck <tony.luck@intel.com>, Fenghua Yu <fenghua.yu@intel.com>, Maciej
 Wieczor-Retman <maciej.wieczor-retman@intel.com>, Peter Newman
	<peternewman@google.com>, James Morse <james.morse@arm.com>, Babu Moger
	<babu.moger@amd.com>, Drew Fustini <dfustini@baylibre.com>, Dave Martin
	<Dave.Martin@arm.com>
CC: <x86@kernel.org>, <linux-kernel@vger.kernel.org>,
	<patches@lists.linux.dev>
References: <20240528222006.58283-1-tony.luck@intel.com>
 <20240528222006.58283-19-tony.luck@intel.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20240528222006.58283-19-tony.luck@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
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?N0J3UUFGM0p4R3E5QWxqdmtOeXd0YmxYVHVPSE1mM3h2UWgzWmpUN0ZTL2NG?=
 =?utf-8?B?N2M0ZTZCYjFONE52YjArOVVOWWx1TzBTM3lnZ1NPdmZad3plTmF2WU1WNUZM?=
 =?utf-8?B?TjF6anZ2MnlyaHhqOVV6M24xQm5pRHRKdXl3clluV3U3UWhoY1l4WmFaNXd5?=
 =?utf-8?B?V2pxMFJHUkRkeDJIaFFjRS9HUEJ4dDFKOFNCSVR5R1FhQlpqMk1KMFJNTXdW?=
 =?utf-8?B?TlVsdGZ2Z3pJeEFwbGRsbWtieS9uckdGa0Z0ZzA2Z245MWRXYXl6Q2x6WG5C?=
 =?utf-8?B?QWZHTHRZUjBRcWJ1RHNLNEFsUVF6Mks2UGFIU2I1STJBMXZNbkt5aVk1QzJo?=
 =?utf-8?B?RERCWDNydEw2bDdxbmZRTmhNMFFmdkhPS09zTXBSR09YdGRkbmFiMGphYzNp?=
 =?utf-8?B?Ni9rZkxDOEdtK0Njdm10NVF5VXd2NXFYZ0FTcnArNStld2VGQy9sclQrL2Zv?=
 =?utf-8?B?N0xybkp3UEV0RlZ4eFU5Y2VOcFFSL0oxbDZsMnlIWVR3dmw4WGQxcW94MlNN?=
 =?utf-8?B?cWJVeDVROUg2ODZ2cFBKcG1KTUxQS2QzYXIxMjE0RGF3UzlhbGVza29XYXdF?=
 =?utf-8?B?ZjVZQnFmTnRqRVB1dFgxbnlXS0Znb1BDeFI3ZnlWWElITE9CemRLdTR5RmVl?=
 =?utf-8?B?dUd1QzlFN3V4RDJiVFpyWEpZOUxVblZkUVJvUm1USnZ3TUgyMUFzMlR1VlhN?=
 =?utf-8?B?MXdNNUNZSU1KakliRkJ1SjNtZ0JnaU1rT0ZMZ291K0wzQ3pKdkRkdmRWd0VN?=
 =?utf-8?B?Si9pajllUkZKR21uZU1ZYWJ2ZVJLWE9VS2ZjVlp0WTJ4TmZjbHJZOUgrRW5G?=
 =?utf-8?B?T3MrZnVVa3RzRnB1cy9yUUhBaXlYc3dSY2VydkwzaUZTdXFQVFhzZmg4VEFq?=
 =?utf-8?B?NnI5ai9UUWE4OGlScFlzTm9FcXEzRWQ3eWd1MlpDU3JHZlVBZzFod0swTWdL?=
 =?utf-8?B?TnpEMmU5ODZBcWV0L0lDSFdDZWdodmEyNHZhNW8rOEhOL3Y2N1A5ajg2K25K?=
 =?utf-8?B?dU85VGNlSHBxZTlSV1R5ZnZwUnB5UTFBMUFCVEZhR1NCUnpERHF5alkxeTVV?=
 =?utf-8?B?eXEwYWNiNUwvdkhQSGFESHVwdGtHdXhKbTdjZmZ0bnlRdzZHQW9jWGZOcnpl?=
 =?utf-8?B?ck5QblF3eCtDd2wxVE11WGNja1lzWUJYZlpKc2Y3eDYrY1RQZnhkVjhUdmUz?=
 =?utf-8?B?ekxmNDdBUmhhZW1Rd1VjVzU0YVFqSy84L3FTa1hxVzM0NlpjWjJxUHl2TGVJ?=
 =?utf-8?B?bnpocEx4Q0YvN0FJbFFTNmRxeGZwZzF1c2NqS0NrbSt2K3o1RkhHV1BDUjJL?=
 =?utf-8?B?UXpPYVljSzgwZkdRaXJPSlBBS0tTODFlNTBocU1KM0FlUWgrR1VkcWo2cnVu?=
 =?utf-8?B?SjdGWjViZk1PcTZMRFFyZzNVa1BrSm5QSnNycndPK0t3WTF0SE42WG1CZmx5?=
 =?utf-8?B?SHBZL2tkemp0WGZDNEowbXY2QmRmSnI5Qmowei9aSWlFbWM1YXUxTXpmRjU0?=
 =?utf-8?B?R0tBRVkvN2UzdDNHVEU3SDh0OUo0c3dYbjJjSHo5U3JwTHJsWWFLQnVtRjQv?=
 =?utf-8?B?L2xGS0x2Um50QUkrQlJxTkdYWGZ5ZUlVamUvVTBCMFlWc3ZjUGx4cDlFQXlK?=
 =?utf-8?B?c0EvZHBKbEFOcEtySzJTM2l1Ti82NW1ZVDZTU3hLQzBKc2FqL2ZPZU5OUllZ?=
 =?utf-8?B?dVlzQnB3NDFDVG9Dc29sdWd6QkRaRDdybGhZeFlsZzA4VTAvcmdsT3pBVFFl?=
 =?utf-8?B?amcyVldEQ29XSlpNeFA5UXppZ0hySWpaK3FNQzVpUTRtRmpzRXRXY1RvRkI1?=
 =?utf-8?B?WGVienVFS2V6b1hPRlFlcHl2aXBrNytXNmNiU0tDc24ycDg5cXp4WHo2Q2RS?=
 =?utf-8?B?Q0VpMDcxN3FVRVlINHZLSVNRb0VDMGtSSlA5ZU5RZTVmSy9vUEh3ME45RFVE?=
 =?utf-8?B?MSthWmxoR2hvT2N6dVpDbDNoTWpITVg2ci8zWXIxQ205U1VhQVNGUzkzY0Fn?=
 =?utf-8?B?UzJpOHk0MmUrMXJmRldqNGxTUDhxQk9CSTlmZHlFVTgwZHJnckYydFhJUWU0?=
 =?utf-8?B?V0d3a0FYZTFyRDZlQXFadWhaSlFIVE93VlVTcHNySUJRZWk5SmFtaGM0Wmxp?=
 =?utf-8?B?S2FSdy9GVUsraktaL1JPV1g3Qkc5V0JubUhjMEtYRWJJRHcybHd4ZEZTTXI5?=
 =?utf-8?B?NEE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5d5c0946-4dd8-4e3c-4f83-08dc80e6f036
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 May 2024 20:27:34.1808
 (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: SADCpj9dp8HjMWR515OnKLwFZh4w4KS10wocqmoPIc+R7wrdQN0vToSZAatfQlE88xVXIpk9HKE1aewkc4n5HBln/ZvVEk7YrdEzuaL4+EU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7317
X-OriginatorOrg: intel.com

Hi Tony,

On 5/28/24 3:20 PM, Tony Luck wrote:
> There is an MSR which configures how RMIDs are distributed across SNC
> nodes. When SNC is enabled bit 0 of this MSR must be cleared.

Missing explanation why bit 0 of this MSR must be cleared.

> 
> Add an architecture specific hook into domain_add_cpu_mon() to call
> a function to set the MSR.

Can this explain more than what the code already tells us?

> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>   arch/x86/include/asm/msr-index.h       |  1 +
>   arch/x86/kernel/cpu/resctrl/internal.h |  2 ++
>   arch/x86/kernel/cpu/resctrl/core.c     |  2 ++
>   arch/x86/kernel/cpu/resctrl/monitor.c  | 26 ++++++++++++++++++++++++++
>   4 files changed, 31 insertions(+)
> 
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index e022e6eb766c..3cb8dd6311c3 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -1164,6 +1164,7 @@
>   #define MSR_IA32_QM_CTR			0xc8e
>   #define MSR_IA32_PQR_ASSOC		0xc8f
>   #define MSR_IA32_L3_CBM_BASE		0xc90
> +#define MSR_RMID_SNC_CONFIG		0xca0
>   #define MSR_IA32_L2_CBM_BASE		0xd10
>   #define MSR_IA32_MBA_THRTL_BASE		0xd50
>   
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 7957fc38b71c..08520321f5d0 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -532,6 +532,8 @@ static inline bool resctrl_arch_get_cdp_enabled(enum resctrl_res_level l)
>   
>   int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable);
>   
> +void arch_mon_domain_online(struct rdt_resource *r, struct rdt_mon_domain *d);

I expect that this function will be called from the already
architecture specific code so needing an architecture specific hook as the changelog
mentions does not seem appropriate.

Reinette