Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d8f0224a-6cb4-eeb4-59cc-39d6ce4fba5a@intel.com>
Date:   Tue, 23 Aug 2022 10:20:20 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.12.0
Subject: Re: [PATCH v5 00/21] x86/resctrl: Make resctrl_arch_rmid_read()
 return values in bytes
Content-Language: en-US
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>,
        <lcherian@marvell.com>, <bobo.shaobowang@huawei.com>,
        <tan.shaopeng@fujitsu.com>, Jamie Iles <quic_jiles@quicinc.com>,
        Cristian Marussi <cristian.marussi@arm.com>,
        "Xin Hao" <xhao@linux.alibaba.com>, <xingxin.hx@openanolis.org>,
        <baolin.wang@linux.alibaba.com>
References: <20220622164629.20795-1-james.morse@arm.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20220622164629.20795-1-james.morse@arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OUIwdWxaNFU0QlBCUjBsaW1IR3B2dXIraFJJSDNJLzllTFNEQzZjTHA5MVVE?=
 =?utf-8?B?RDViSXVNRWFZeFdzWDZrWmM5b2ZaazZxUitkUm45d2NUMG1lVDVhZ3ovcnhn?=
 =?utf-8?B?U0hsMEFjeUZKUkVSWVEzQnF5bFUzSU15SE5haXFuMHFkdHk2QU9XYnB2ZEFo?=
 =?utf-8?B?NUhCYnVqMHBVdDUwUE1oWllpWnJrQzQwV09WMjNiTThDSU9paEE5bzQzbTBX?=
 =?utf-8?B?UUUxU3pBcnJzRW4yRDIzMm1wWEJxUUFycXN3bVdnZEhlNHNRbHJ5cnp4V2or?=
 =?utf-8?B?ZHErNE1tVjZ3ZElpNmNiM1VudXlMRzdQTGFtSkpqRUtYMTEzQjNhWnk0bDIx?=
 =?utf-8?B?M2E3TlIwNXlPTnlMZkdRV1VVanc5bGs3YWZPaSszWFVlM3p1TnZSQ2tDcmRr?=
 =?utf-8?B?V212cWZobFZ1RlcvL0o0Q2hXKzBBMWxwdllQWC96ZSs5a1llcWs2a3A2VUlO?=
 =?utf-8?B?YTllYm0wVGhBb201NjhPUlcwRGdjQ0t5WUljNDdad3d1TnBySzNONmFiQkJE?=
 =?utf-8?B?bmN3ejBPR25YM1h5Y25ZekluTFc3VUVRZ1NycW5QUDVGUDFaQ0xveDQ2dld0?=
 =?utf-8?B?L1JsVnRoUGZVdzdEbnpVY0ZhM2JTdU42Y2RqK2RDcnlBcFNDdVFaVmlOdkJ6?=
 =?utf-8?B?cnhOL1AzTi8xMjNKY1ljMkFoSkhPNlAzK1dBT2JaQm04MmVrNEhGRVk3b2hW?=
 =?utf-8?B?Ky94eFQ2bUdGYmhPdGlrbUxxM2Z2U0tBZjgwdHkrVDdGSjArUXViS0E3ckRk?=
 =?utf-8?B?TEtjYWgxKytSelYvQU8xMmZlaS9IRUJkZ2dWMStzNVZOY0VSQWlpdHAxMSsv?=
 =?utf-8?B?MWFvSmFBdlI2VEdkeVI3QXNJQ043TXhFTnhhc2VUL0FaZjRGLyt4UDhpWmpy?=
 =?utf-8?B?QVYvYkM2OTNjVSs2QktSSUhSMjI1MzlQY3JTbC9IcHYwR0VTRE9qTGJUZklh?=
 =?utf-8?B?Qmg3UXhXTGtQOUVEYXl6T3hJSXduTXQ2Q3FCK05PYlhzL1dXdkRqTG5ZOURj?=
 =?utf-8?B?OFpyTGFJcnFCd3RNRy9Xd01oK1I4R2Iya1E5d1R3VVRheDNLL0FnN2pOUHlv?=
 =?utf-8?B?K2M2akRLTFJIUk5Gdzd0VDA4RmZJU1RaOUZjT1ZwRDltZUkzdFlPb0x2RXlR?=
 =?utf-8?B?blNUM1JtZVR2YkhkSkpTUFFaQmt2VlhaeEdZdFc3ckQ5RDdCV05Ba2VBR0pl?=
 =?utf-8?B?YTkvOExOZTJmS01ZUTc0S1BiN2dGcUNlSXRlL2JROWV0SnBpZ1ptdE1FNXpt?=
 =?utf-8?B?c3p0WStMQnFIdkgzRGVieWVIdlIySlVwL3JIZTgxcE1IWkltbXU4d2xmQUZK?=
 =?utf-8?B?QlI5OE92ME1zeHl0YnF5YkRNc244Myt3dVUzeko2TmxpOWtTcGRINTlPTkhV?=
 =?utf-8?B?YUJCVVFRSmtMN3N1aHB0YW1wbmthR0F4dkZ5TGF0aVBxSExKZHlCL1N1ci9P?=
 =?utf-8?B?WFY2cWEvNkRCV3d0MUZDUC9nK2YyRG1tbDFNdG0rbzNTRDREQnBSdnQwb0U3?=
 =?utf-8?B?ZnhsMmhGc1VaakZIdE9DeDV0d1JURGhxQUV0c21Ba2g1RUl0amFCcXBhZC9l?=
 =?utf-8?B?V0hQV1dlbTllOFhYUkF4Rnl0V3ZPTmZ4Ym1kTHNIa1ZqSS9odGQwdUdvWER2?=
 =?utf-8?B?czIycmhpRHJLMEdQUytwaXFLbjErVldsNS8vUmFTU3VVZEJvcFRMV3NJT2sv?=
 =?utf-8?B?SkxPbDJuWmFaODJPaVNpM1lZU2ZoWFhIc3Q4T2ZqLzJnMnR3dVdFZVdHUWZn?=
 =?utf-8?B?U1VMK1NpYVVTb295TlEwSnN0WlVFUEF5VVM5cmtjelNkUE1mL1N1MjVCaGIw?=
 =?utf-8?B?emVWMmZRd0V5T1N5T2I5bDdqYjEyeTBFeGsyMkpReVFPKzZic1B4VGFwQ2Iy?=
 =?utf-8?B?Mk4wbUJkQTlKTXF1SDFzWWI0ZjVTbmVCRDlTazAwRE5CNVhxeVFXR0pramFs?=
 =?utf-8?B?VlZPVWp0S1IxNGlHTjg1RHJJRE9HMW4wa0Z0UWVlZlZHS0Q0d2FWTnkzdnNl?=
 =?utf-8?B?SG1qRmV1YlhDcHVpZnYxMHM5K2tRWFB1aEdVTkFaWDdnNk9vYkVXemtFSXJv?=
 =?utf-8?B?K3RQcnVxMXhta3dZNFAwc2syRS9RRnlrYmlZWDhZL1R0QUtIWmF0R0FudXFx?=
 =?utf-8?B?Wk5WaUhYNWxnK0UvWWRJbG5jYXBZREJRQ2RmUjloNTJxQk51OUdtRkdCTlRV?=
 =?utf-8?B?NVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: a6502a2e-852f-4fe8-9939-08da852bc360
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Aug 2022 17:20:23.6509
 (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: DLMPhg8n2wSQd7XhlzZ/J6IbJgNvEidVr/8oWcC72mb/RybeHO7GUnmoXK2a0yKktfR3RKeBj/i9KgbILAH6DCgSL5YwHg1cIJeIoDVJYf4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR11MB3088
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi James,

On 6/22/2022 9:46 AM, James Morse wrote:
> The aim of this series is to insert a split between the parts of the monitor
> code that the architecture must implement, and those that are part of the
> resctrl filesystem. The eventual aim is to move all filesystem parts out
> to live in /fs/resctrl, so that resctrl can be wired up for MPAM.
> 
> What's MPAM? See the cover letter of a previous series. [1]
> 
> The series adds domain online/offline callbacks to allow the filesystem to
> manage some of its structures itself, then moves all the 'mba_sc' behaviour
> to be part of the filesystem.
> This means another architecture doesn't need to provide an mbps_val array.
> As its all software, the resctrl filesystem should be able to do this without
> any help from the architecture code.
> 
> Finally __rmid_read() is refactored to be the API call that the architecture
> provides to read a counter value. All the hardware specific overflow detection,
> scaling and value correction should occur behind this helper.
> 

Thank you for your patience as I was offline for a while. 

This series looks good to me. I have one remaining comment that I provided
in reply to "[07/21] x86/resctrl: Abstract and use supports_mba_mbps()" where
it seems to me that an existing issue could easily be addressed in the new
function. 

I do not have tests for the software controller and only did basic sanity
checks. It would be great if the folks using this feature could test this
series.

Thank you very much. From my side it looks good:

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette