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: <00b0c70d-eed8-8dc1-8046-25764d991bf4@intel.com>
Date:   Tue, 16 May 2023 17:06:31 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.11.0
Subject: Re: [PATCH v1 7/9] x86/resctrl: Assign HW RMIDs to CPUs for soft RMID
Content-Language: en-US
To:     Peter Newman <peternewman@google.com>
CC:     Fenghua Yu <fenghua.yu@intel.com>, Babu Moger <babu.moger@amd.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, <x86@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Stephane Eranian <eranian@google.com>,
        James Morse <james.morse@arm.com>,
        <linux-kernel@vger.kernel.org>, <linux-kselftest@vger.kernel.org>
References: <20230421141723.2405942-1-peternewman@google.com>
 <20230421141723.2405942-8-peternewman@google.com>
 <3816ccf6-4f74-6406-5ca0-580743efa2a1@intel.com>
 <CALPaoCj76eMTF+VPT8_52_D+fKpWt2ENcyavJ4aooCOo9TYKFw@mail.gmail.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <CALPaoCj76eMTF+VPT8_52_D+fKpWt2ENcyavJ4aooCOo9TYKFw@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RE9HcjBUZWIvNVVjV0JUZWJ1Y2NrRGZMZVZpak5PNkN2T2FMZFRzVzd2blY4?=
 =?utf-8?B?STE5TWFIYS84LzdZbkg0aDhvdytxYy9rdzZ4aThLNWJ4K0ZTMHBMK2FBZDFC?=
 =?utf-8?B?N1VtRHVUR1c4QmNicHZtd2o4MnNrNnRmaUQ5S0FmOWg1VkJadkpCT1E2MUU2?=
 =?utf-8?B?a3hNMXFNVTBZK1UvS3V1M2drNUNwVklDa0FmTGtuREcvdU51YVpCTHJVNENx?=
 =?utf-8?B?d0tsQlYrMHZnZzJBbUNFRVY2ZHJ4R1NpMS91em1Sa2xYay8wRG92V1NyTW1N?=
 =?utf-8?B?QUVSeHhmZ01HbW8vYjVob1pWZUxra2FKelZ1bnZHVHlxSWdwYlpNOVlJNTBN?=
 =?utf-8?B?RkcxelRKdFJOd2VhWGtmZ1FvNmhaeHYyU01QWHh4QzV2QUNLK0t1RFllSEk1?=
 =?utf-8?B?SmlsTmpNYXZ1NjhyZTRCcEd3TzVTRDBNV2xKQzEvdG52MFhNSUY3eHVsNFB4?=
 =?utf-8?B?WHdaUFBYeHF0Mkk3Q0tNd3JkLzdaSVhWSHF0VnB3T2w2aFRhTS9EMENYSkNX?=
 =?utf-8?B?cmxIbDNNdnc5UW5OYVNUaXluN1dmS0hCTkEvWmtaZzhSWDNLS3ZPV3RNN0pE?=
 =?utf-8?B?b3pSUzZ4dWhJWVlsT0JBUEUrY25aTnhHbVBCNld4dllTK1lYOEt5QW5IbmRz?=
 =?utf-8?B?cU1Oc285MGZjRVRIblZ2TmdISDEzdzdLZ3N6ZkZuRDlmMVVRUi85a0NHRS8v?=
 =?utf-8?B?RnBwZ01rZzFXazQ3NEYxd0xaakVCQTQ3RkIzOTA2bm96YmVkRzI2b0Y3bmx5?=
 =?utf-8?B?R1JSZ0Z6RE5ET1BXUEJXQTJyRW95WWJvaGNUV0FYclRFUWpieENwV2w4YjZZ?=
 =?utf-8?B?NTJFQm1PT2E1WVVpOE82NjRTbytoT2luUzVTTWVhdTBUdFRxVDZHUlNrZVpD?=
 =?utf-8?B?WFIrZ2FOOGwzNWRSWVpCSW0xMmVwOVkwTDMyd3k3Z2ZOMkIrWmhOQWwzSk9w?=
 =?utf-8?B?R3p3R2p0dG01ZlBZNTFnWmZxRjVCM1ZCbWV2MkxzT2lTMER0bEcyQjYvb1pZ?=
 =?utf-8?B?WmJzQUtyVVZweWVaQU1qL3ZiYnhsUzZ1dUQ5M1dmdnpWUUtISHRhUTBWQUtY?=
 =?utf-8?B?ZTY1T1VwQUYrNHRYaEtnOTd0dkYyUDBJVEJ6aUFvTzROYVQ2Y0UydW9iZHBn?=
 =?utf-8?B?ZjVqenZJaVZmOEc0ai9vWE5wWW1TZE1MK1ZNWVoraVRCMStxVWI5RnNlaUxB?=
 =?utf-8?B?N2Q0aVczZzhnYTl4THRLcFVpYnMwVWY2S01LbkhpRDl2eW1QNm1WWXk0N1lJ?=
 =?utf-8?B?Q2oyQlYxckN5anBnWnVGQm1FL1Y3SWttcC9lb25mTTh2MWEyRzRJSlh6WXJu?=
 =?utf-8?B?elRsbWF5VWxIRjhiQ2NkNW1lMUFudzZGZk5JY1lSVEhiVTlla1pGeTdMeGxt?=
 =?utf-8?B?M2VHUFRrTjBrRm5hUTdrVzlZUzF2VGdVZllveHJKemdTWU1WMEx6YU5HK29N?=
 =?utf-8?B?ejIyTUJxVk43a0NEVzdmUjlZcnd4cW1YR0lZaWMxTlZpRjZkQ1FZS1ZkTXJZ?=
 =?utf-8?B?Lzd6RWZ4VFFMaUR2MllaNkYrSnZBQVlST2Y5Rm9WOUdScFpjVVJFNUIwMFFp?=
 =?utf-8?B?UWJMZXlSdTR4MmdxZzY5TXJBUWo1RVhVQjBsTWFqUUdYMnpRZ295K01KRWlq?=
 =?utf-8?B?bUg0Mm5kWTI4U2dHRDE2dFVmemtSZkM4U0hRTWU3QzM2dCtzSEMydjFESWkz?=
 =?utf-8?B?bjdDV3Jkb3FJcEhERGIrTm5UTFJRbG5WMVlCVmd1WkN1eHpVOHR3aWFSRklN?=
 =?utf-8?B?UjBnVmh6RDJ5eU5pZVhmZFg0dnBhdlQwWW9CS2s1bW5scFdMTmt4empSMk45?=
 =?utf-8?B?ZEF0M1paMjZKK3Z2dWJLeXNGcktUOGljTGdWQXFyYnhvMXNHQUJVTTgwYzFl?=
 =?utf-8?B?aWgySjBsREZwcGQ2TmduSkNNK3VucmNpOTFGYVpQSkVIN0NYaVFvcUlUTEFI?=
 =?utf-8?B?Rm5idkFhTjFidHV4NXE4bHVZTm9nQU4zZkd2NGJLT2JLdVVFT3JjN3Jyb0Vt?=
 =?utf-8?B?TUp2SllxMVErdmJ2OHdPSEVVYVJXK2ZJVkh0VmdCeFlYMm9rMkJTSEY0Ulhu?=
 =?utf-8?B?cVJpazBtZlZRckpxVWd3MTg3ME8wWHc2bzlRYkZQOFFadTRMcVBvNFQvL0xu?=
 =?utf-8?B?SmI3QU1BSU4wL2xRdEFLNlpJakl1Y0ovank4ZTV0RUVDNkFXbEt3bjBWSG5Q?=
 =?utf-8?B?SlE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8533b289-7258-4152-fe05-08db566a92bf
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 May 2023 00:06:33.2947
 (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: 4URbqEVDBN0iN8hMaKENXvHVttgN6HVCo0x+UgE8b0RxQ9m0ratY5/u7cKJG5beAodOpvGfq2MTacSLPRMxaZyB6sz3RFbimleJb+EoPL9M=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR11MB7911
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.1 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,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED 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 Peter,

On 5/16/2023 7:49 AM, Peter Newman wrote:
> On Thu, May 11, 2023 at 11:40 PM Reinette Chatre
> <reinette.chatre@intel.com> wrote:
>> On 4/21/2023 7:17 AM, Peter Newman wrote:
>>> +     rmid = 0;
>>> +     for_each_cpu(i, &l3ci->shared_cpu_map) {
>>> +             if (i == cpu)
>>> +                     break;
>>> +             rmid++;
>>> +     }
>>> +
>>> +     return rmid;
>>> +}
>>
>> I do not see any impact to the (soft) RMIDs that can be assigned to monitor
>> groups, yet from what I understand a generic "RMID" is used as index to MBM state.
>> Is this correct? A hardware RMID and software RMID would thus share the
>> same MBM state. If this is correct I think we need to work on making
>> the boundaries between hard and soft RMID more clear.
> 
> The only RMID-indexed state used by soft RMIDs right now is
> mbm_state::soft_rmid_bytes. The other aspect of the boundary is
> ensuring that nothing will access the hard RMID-specific state for a
> soft RMID.
> 
> The remainder of the mbm_state is only accessed by the software
> controller, which you suggested that I disable.
> 
> The arch_mbm_state is accessed only through resctrl_arch_rmid_read()
> and resctrl_arch_reset_rmid(), which are called by __mon_event_count()
> or the limbo handler.
> 
> __mon_event_count() is aware of soft RMIDs, so I would just need to
> ensure the software controller is disabled and never put any RMIDs on
> the limbo list. To be safe, I can also add
> WARN_ON_ONCE(rdt_mon_soft_rmid) to the rmid-indexing of the mbm_state
> arrays in the software controller and before the
> resctrl_arch_rmid_read() call in the limbo handler to catch if they're
> ever using soft RMIDs.

I understand and trust that you can ensure that this implementation is
done safely. Please also consider how future changes to resctrl may stumble
if there are not clear boundaries. You may be able to "ensure the software
controller is disabled and never put any RMIDs on the limbo list", but
consider if these rules will be clear to somebody who comes along in a year
or more.

Documenting the data structures with these unique usages will help. 
Specific accessors can sometimes be useful to make it obvious in which state
the data is being accessed and what data can be accessed. Using WARN
as you suggest is a useful tool.

Reinette