Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v1 06/20] x86/resctrl: Switch over to the resctrl mbps_val
 list
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>,
        Jamie Iles <jamie@nuviainc.com>,
        "D Scott Phillips OS" <scott@os.amperecomputing.com>,
        <lcherian@marvell.com>, <bobo.shaobowang@huawei.com>
References: <20210729223610.29373-1-james.morse@arm.com>
 <20210729223610.29373-7-james.morse@arm.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
Message-ID: <5be446b6-d6f6-dacb-4368-7c1e7aedc6cf@intel.com>
Date:   Wed, 1 Sep 2021 14:25:26 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Firefox/78.0 Thunderbird/78.13.0
In-Reply-To: <20210729223610.29373-7-james.morse@arm.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MzlMdlRYNno1REloSE8xa0hnbzMxeHVJVnhka0tiUDhORzZhZHEwZzg3Ujg3?=
 =?utf-8?B?TnBRdG9NSWtkZ2UvNmpjVFh1SXMrVnRxdEJhQUhycWk0RXlhK2ZGYklUenFM?=
 =?utf-8?B?VGJlQWRQay91L250U1VXakFlblFaUzZZZUQzanR6MTA1NXdqZVV4RnArdkg3?=
 =?utf-8?B?N1N5ZGNLZkdBcEoreVBGbmI5Vk54TkVKV05OVGQvMlNVbnNSa25XSkdVZmxU?=
 =?utf-8?B?bmVsYjFDSUxHRnRkVUxjenRjYThOb0R5YVRwZWQ3bXE4ZTYzTk4zVlpXMjcv?=
 =?utf-8?B?WlRqa1drcWpOM05XTXlQOU96dUROU0E0R2w4MStkdERKWFk5dlBQVzJHcVEv?=
 =?utf-8?B?ck5yNGVmSk5IRjFPV1UzUDBPNHpYMmw5L3dEMUlycWt0cXVQN29heGVmWE1o?=
 =?utf-8?B?N0NXME9kQkdRSUx3cE1HbjVrbGkzV1l6YkpRc2cvaWtiSGYxMURtd1Z1MHk0?=
 =?utf-8?B?QTczbHdlTTZac0luc1dqNlB3b05FUE0zWWtlL2JaQVlpM0hZNm40M0dGcEFr?=
 =?utf-8?B?Y1ZoSzEyVlh3ZnVuejFPbEhEaUNuczVjR1c2NndoeHZrVHFYbTR0K1VmTXV2?=
 =?utf-8?B?OGl5Ym42VlllSGQ4UDdBdmhLWFdhZllMZkw0QWh2b1pVSnUzNUVkTlR4UXp0?=
 =?utf-8?B?cWNPRC9zSUx0c1VOYTg1Wks5YkttcTF3aUJrZ1Y5ZWJnVkFlNE16djhPc2lW?=
 =?utf-8?B?VzkyWThlVWF0R3RQR2VidlQzdFlxWXVuZERSdHFpSHhTcS9IaGoyUWREN3dv?=
 =?utf-8?B?U3RVVFBRZml0Ym5KSXlnUWNmSzE0QmxJNXNCeEg1S3E0UmplbExvMnFtN1JO?=
 =?utf-8?B?Q21OblB6QWtsMFFzeS9rL0I2dHdOODd3ckpGeklCamxKZFRQODRxM2RrcnlQ?=
 =?utf-8?B?Rlk1OXhUKzV2QzR5UDhkU3AwYkRDa25sYTRxVklzUDZtalNsaW1rbkF5WGcw?=
 =?utf-8?B?UXNPdmV1VmlhNGExc212Rnl5K1lIMmpGcG9zQVhCWmhnQlNrK1dEbXRWcU9k?=
 =?utf-8?B?VUdsK3hLb0k5NmovbDBlczdGVUg3WUUreDJKMThDUDM2eW5qeWhwNk9HU2Uw?=
 =?utf-8?B?czdGTmFwQ1JaOVVqeVJJdzRnUnRtcitJM0tNQ0ROKzVQSGNDU2VUb0tCNlFC?=
 =?utf-8?B?c3gxakw2TmIrRE9wWTNFeCtJaFJKMlI0dDdoN0Q3RFNtWDhXZE4rQ3ZTR2gz?=
 =?utf-8?B?Um9VUHBBMWx4RzRlOFFpYTlIL29ma1JORDJSclpQdlFTWk1XL1d1WElrYW44?=
 =?utf-8?B?dzFSR1pjNkwrNng1TDBvVTJ3WU1ieDNsaVZuTzhmUFJrTHhlNnVTSllIbVEx?=
 =?utf-8?B?dWVYL3lkekc1aUxHdlViczlpZk5BYVZiem0vQld5WTdqbVRVcEQ0TzZCTzNC?=
 =?utf-8?B?eC9pRlc0djAxbmJsbTZwTWpKUjV4V1lTYlZqTGRESmsrUkpTc2tDeTJIVVVl?=
 =?utf-8?B?SmllRVBoM0JCTVZMRkxTQ3A1RkkxWStWZDcyOHlwUVJ0TGw5WnJocVdPUXF4?=
 =?utf-8?B?UHZhRUhvT2psWnozLzhIWGhINnIvRWZMWnBSN2RwTmhyVDh3QXRXejU5em9V?=
 =?utf-8?B?djU3NXFRQXdSbHBSMGFqNWNQZ1ZvVXlWaWdFQ2lhTmxEQkJuRVd0MmVVUCt2?=
 =?utf-8?B?R21DeHUxZ2d3ZkQ2Z1g0eE5QZVhmTHlBekVaNThDUHV4ZXdMc1ltQjFLVlVP?=
 =?utf-8?B?bkFhQ2Z0S3R4MVMvdTJWejVaejcvdll4WlhQSnFUZTZOeGd2Mk9VdTBqQkI4?=
 =?utf-8?Q?37gaPq20dntzk7S10HfnH8KzbXkorDhZHG7n46/?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8d8f7b57-cf37-46c2-099d-08d96d8f06dd
X-MS-Exchange-CrossTenant-AuthSource: BN0PR11MB5744.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Sep 2021 21:25:31.3541
 (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: DV8TTK/ilR/mxZ8JR/e4+X919pPHnBtkXb4GjGiff++NDYma5bmhl4lAw8vFU7iB/D+ucuccXBnzcOpXjYIxVk+4y/Muiw0gRWX02QyHt8A=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR1101MB2258
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi James,

On 7/29/2021 3:35 PM, James Morse wrote:
> Updates to resctrl's software controller follow the same path as
> other configuration updates, but they don't modify the hardware state.
> rdtgroup_schemata_write() uses parse_line() and the resource's
> ctrlval_parse function to stage the configuration.
> resctrl_arch_update_domains() then updates the mbps_val[] array
> instead, and resctrl_arch_update_domains() skips the rdt_ctrl_update()
> call that would update hardware.
> 
> This complicates the interface between resctrl's filesystem parts
> and architecture specific code. It should be possible for mba_sc
> to be completely implemented by the filesystem parts of resctrl. This
> would allow it to work on a second architecture with no additional code.
> 
> Change parse_bw() to write the configuration value directly to the
> mba_sc[] array in the domain structure. Change rdtgroup_schemata_write()
> to skip the call to resctrl_arch_update_domains(), meaning all the
> mba_sc specific code in resctrl_arch_update_domains() can be removed.
> On the read-side, show_doms() and update_mba_bw() are changed to read
> the mba_sc[] array from the domain structure. With this,
> resctrl_arch_get_config() no longer needs to consider mba_sc resources.
> 
> Change parse_bw() to write these values directly, meaning
> rdtgroup_schemata_write() never needs to call update_domains()
> for mba_sc resources.

The above paragraph seems to contain duplicate information from the 
paragraph that precedes it.

> 
> Get show_doms() to test is_mba_sc() and retrieve the value
> directly, instead of using get_config() for the hardware value.
> 
> This means the arch code's resctrl_arch_get_config() and
> resctrl_arch_update_domains() no longer need to be aware of
> mba_sc, and we can get rid of the update_mba_bw() code that
> reaches into the hw_dom to get the msr value.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---

...
> @@ -406,6 +406,14 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
>   
>   	list_for_each_entry(s, &resctrl_schema_all, list) {
>   		r = s->res;
> +
> +		/*
> +		 * Writes to mba_sc resources update the software controller,
> +		 * not the control msr.
> +		 */
> +		if (is_mba_sc(r))
> +			continue;
> +

A few resources can be updated in a single write to the schemata file. 
It is thus possible to update the cache allocation resource as well as 
memory bandwidth allocation in a single write. As I understand this 
change in this scenario all configuration updates will be skipped, not 
just the memory bandwidth allocation ones.

>   		ret = resctrl_arch_update_domains(r, rdtgrp->closid);
>   		if (ret)
>   			goto out;

Reinette