Received-SPF: pass (google.com: domain of linux-kernel+bounces-25034-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <3bd2a660-82fd-51fe-f7cd-492908d90ca7@amd.com>
Date: Fri, 12 Jan 2024 14:38:22 -0600
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Reply-To: babu.moger@amd.com
Subject: Re: [PATCH v4 2/2] x86/resctrl: Read supported bandwidth sources
 using CPUID command
Content-Language: en-US
To: Reinette Chatre <reinette.chatre@intel.com>,
 Babu Moger <babu.moger@amd.com>, corbet@lwn.net, fenghua.yu@intel.com,
 tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
 dave.hansen@linux.intel.com
Cc: x86@kernel.org, hpa@zytor.com, paulmck@kernel.org, rdunlap@infradead.org,
 tj@kernel.org, peterz@infradead.org, seanjc@google.com,
 kim.phillips@amd.com, jmattson@google.com, ilpo.jarvinen@linux.intel.com,
 jithu.joseph@intel.com, kan.liang@linux.intel.com, nikunj@amd.com,
 daniel.sneddon@linux.intel.com, pbonzini@redhat.com,
 rick.p.edgecombe@intel.com, rppt@kernel.org,
 maciej.wieczor-retman@intel.com, linux-doc@vger.kernel.org,
 linux-kernel@vger.kernel.org, eranian@google.com, peternewman@google.com,
 dhagiani@amd.com
References: <20231201005720.235639-1-babu.moger@amd.com>
 <ba1305bf2a29ce2667d01cf997718126a7ea607b.1705009003.git.babu.moger@amd.com>
 <4c5bb5ac-1e84-4021-845d-cafea5a1fbde@intel.com>
From: "Moger, Babu" <bmoger@amd.com>
In-Reply-To: <4c5bb5ac-1e84-4021-845d-cafea5a1fbde@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?N3BNODAzdHNVMzIrbnJQc2JIQjZoeTRnTXNoMnd2QUJUOXU5Nk5HSVk3eDJp?=
 =?utf-8?B?aHBST1o5QkV6eVNBeU8wN3Bway9HVWNQYUNrUHplZnpCeWxHMHhVQ0xyRGtw?=
 =?utf-8?B?WUZwb0FRclJ3d2lMYkU5RVd2UjRXQWNWS01oZFM5RUVabU83eVByU3dEQ1du?=
 =?utf-8?B?WWdSZ2dWMG5PNFFvZnIzak5USmZ5QTg4SVFVNWNHa1FldS9SaVJnZXBnUjB6?=
 =?utf-8?B?Q0RGelZLZ3Q4aEl3MlhsYUFsbUdleVovQ0JJcndkODk5aWhVbXRtb1dGS3JV?=
 =?utf-8?B?YmhTaVNJelE2dS9TUkxFOW1jd0FiRnAwQythd2ZNNk9VN2Y2Ymo5UFRoT1F2?=
 =?utf-8?B?dHc4Q0FWL1F0bkVZVzBnbnFtMFBiakFWMmhUS21Wc3V3d0p5YzBaTStqYk5r?=
 =?utf-8?B?clU3elliUVNQNXJsK0F2L3N6cWdnUmo3SWsyNHAwMkRDZzdHdmJuUTl6ZzJ4?=
 =?utf-8?B?NTNwUTEvWEZCaVhMU1hjWTc4YVhFS0Y2VThlNEVEOElOK0hnN0hkQ2tQMkUv?=
 =?utf-8?B?ckxGMXJ2Z2dVVjV2ZGNwalJST0FwVm9EMjdsOTBxeGhzeHJldWRRSVpXd1VP?=
 =?utf-8?B?SU9lNjZlUVlaVUJHelFxNFdTNU15WWllbnZ4RWhEbHVwOWpsMkppTXRtWm9q?=
 =?utf-8?B?Z0poakt6amcyYlBMZmJ1MFhmanFiQ25oZzVlenNYU0tyYmZCalBreFhrbWkz?=
 =?utf-8?B?RFJUaVFYNmp4NXJNL0NlRkRIWkJRSVEybC8rUVBZOVBueHpPVlJicGJVdUZS?=
 =?utf-8?B?TDB4c3MvZmVqcXA3RVJCUFpzMmdiMTFzanl2cjdOL3A2OFhkS3BkeWJYRGVD?=
 =?utf-8?B?TVNoejZLNCtCT3I2a0N4QVAvcmMrcEVkZVFxVzBJOTJpTkxOSUZPVEU4RDBL?=
 =?utf-8?B?WTBYVTgxbjM3V3JGMW84d3BLK3YxNDJaZVBGRXhNclI0RStLZHFwNFVOMEcz?=
 =?utf-8?B?blUyY3dJeEY3bWVUeVRheWVtNmJ6V0lVYVNLL1dZSFNrdS9Bb0NLdEg3KzhO?=
 =?utf-8?B?Qng1UmxGRFcrTVh2NzEydCt4WWNHM0tWUlZoVkpmbXg5cDFwSWlibzVYR1JJ?=
 =?utf-8?B?bk0wMTAzNWVrMW14U2xLdUgvWGVEeEJUN1Q5U2tWUFBSWk1peFRMRzROY2lk?=
 =?utf-8?B?M2VVNFgzM1UyREhBYi9uUXpnTlpUajY3TEsxV1M5MXl3QzQ2SFJFZ0MrQXhT?=
 =?utf-8?B?dmlRSzQwWEx4a1ZRQWg5S3BSMFBEVUpZSUVNcDhpNHVGc2tLek1DQkFpTGZX?=
 =?utf-8?B?ZmRKdU5oSzhlZ01yQ2VHckRyd1FEdHNmRjEwNVZQUjRXYnhGby9HYnlsTFlF?=
 =?utf-8?B?VXhRZ3pEVU1qSWF0WVZxdkdUL0RhbGg5Nkl2Zjhkc0hWL3BSZmp5S3orUGRz?=
 =?utf-8?B?d00zdnBMT2h5ZERKL01uL09tRWNOeHZLRGRsQ2lDRlNHUnEzOUdlN2dlZDBv?=
 =?utf-8?B?V25sZzdvdkxpN0c4ajNEbW9DMDdPVkJNN0xTQWpwMGRpVGtEdTJzd2ZQNzdz?=
 =?utf-8?B?VlZ2WUFMR1Avb2xsZi9MTjJ2VXhya2dZUkY2ZWpQTjBYL04wVW1QdTVWOGNy?=
 =?utf-8?B?UVE2NFFVc1ZlNU5lcnFnZFl1MUdialRScEtxTkVTZWZsMEUrLytZbWZTcTNl?=
 =?utf-8?B?SmJjWDFyekwxeXAybkhFS3BFU25JelgzWS9UdC9ZdXJkeTNsTnRwMkZWcEVm?=
 =?utf-8?B?SzZiUzF4c0wrMHZqQzEzRnZPTis5emRsdkFnc3pWZnlmOWpCdmowZC92MktO?=
 =?utf-8?B?ZGNnajFTRDZmc1I5U2JVMmtFM1J0bWxqWjVBVXB6LzhTaUpVSG83RnBySHZq?=
 =?utf-8?B?Q1U3MGRtclkrdlIrVmRFS1Y1UHFlNWVMeHlnMkN0NjBvc2RxcUVjczYvQWgx?=
 =?utf-8?B?WVo4emtDOWdtenVoWGEwalIyNFZOMUpBZVhYdHRWdU84empNVjN2SDdub01K?=
 =?utf-8?B?OWJSbTNnVzJVWllSUEQ5cDREUmlKU2FEbEtsN2RremdoL3p4TzFYcThoYmZ3?=
 =?utf-8?B?S0lpeFh3eVIxMmVCUUFYZUxCU3BiSElkejFGcS95K2Y3VTViTkQ4OW41TE0z?=
 =?utf-8?B?QnZQMWp0U2ZqdmpPTzlTRERiMTF2dVJGaGtBRW9yNVNwNnRrVnNSRHNQZ054?=
 =?utf-8?Q?g/kUWZ5A7G5R61jVogQj3nLmx?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: faf8bb09-4fea-4279-b9c8-08dc13ae6e4c
X-MS-Exchange-CrossTenant-AuthSource: MW3PR12MB4553.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Jan 2024 20:38:27.6826
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: s3xeYlf8is9SbYM6rabzJygRx/4Am+xgXmvcQYQFC4hv/t8iH9nm9ELPRjLR/ds0
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR12MB4460

Hi  Reinette,

On 1/12/2024 1:02 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 1/11/2024 1:36 PM, Babu Moger wrote:
>
>> @@ -1686,6 +1681,13 @@ static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
>>   		return -EINVAL;
>>   	}
>>   
>> +	/* mon_config cannot be more than the supported set of events */
> copy&paste error? There is no mon_config in this function.
Yea. it should be mbm_cfg_mask.  Will fix it.
>
> (copy&paste difficulties reminds me of [1])
>
>> +	if ((val & hw_res->mbm_cfg_mask) != val) {
>> +		rdt_last_cmd_printf("Invalid event configuration: The maximum valid "
>> +				    "bitmask is 0x%02x\n", hw_res->mbm_cfg_mask);
> checkpatch.pl should have warned about this split of text across two lines.
> Logging functions and single strings are allowed to exceed the max line length.
> If you just merge the two lines then checkpatch.pl may still warn for resctrl strings
> but that is because it does not recognize rdt_last_cmd_printf() as a logging function.
>
> You can also just shorten the string so this patch passes the checkpatch.pl check.
> For example,
> "Invalid event configuration: maximum valid mask is 0x%02x\n"
> or
> "Invalid event configuration: maximum is 0x%02x\n"
> or ?

Yes. Checkpatch reported error when I split the text.

How about this?. Checkpatch is happy.

rdt_last_cmd_printf("Invalid event configuration: max valid mask is 
0x%02x\n",
+                                   hw_res->mbm_cfg_mask);

>
> Reinette
>
> [1] https://lore.kernel.org/lkml/cc273d98-d73c-49bd-8799-b119966e226c@amd.com/

We spent quite a bit of time on this earlier. Yea. It did not go the way 
I wanted it. Now, I needed to get back to higher priority tasks.  Will 
pick it up once, I am done with current priorities.

Thanks

Babu