linux-kernel - Re: [PATCH 5/7] x86/resctrl: Add interface to enable/disable SDCIAE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9d52bac8-5e4c-4bd2-b01d-c78d4b7274d6@intel.com>
Date: Wed, 16 Oct 2024 08:53:50 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: <babu.moger@....com>, <corbet@....net>, <tglx@...utronix.de>,
	<mingo@...hat.com>, <bp@...en8.de>, <dave.hansen@...ux.intel.com>,
	<x86@...nel.org>
CC: <fenghua.yu@...el.com>, <hpa@...or.com>, <paulmck@...nel.org>,
	<thuth@...hat.com>, <xiongwei.song@...driver.com>, <ardb@...nel.org>,
	<pawan.kumar.gupta@...ux.intel.com>, <daniel.sneddon@...ux.intel.com>,
	<sandipan.das@....com>, <kai.huang@...el.com>, <peterz@...radead.org>,
	<kan.liang@...ux.intel.com>, <pbonzini@...hat.com>, <xin3.li@...el.com>,
	<ebiggers@...gle.com>, <alexandre.chartre@...cle.com>, <perry.yuan@....com>,
	<tan.shaopeng@...itsu.com>, <james.morse@....com>, <tony.luck@...el.com>,
	<maciej.wieczor-retman@...el.com>, <linux-doc@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <peternewman@...gle.com>,
	<eranian@...gle.com>
Subject: Re: [PATCH 5/7] x86/resctrl: Add interface to enable/disable SDCIAE

Hi Babu,

On 10/15/24 12:25 PM, Moger, Babu wrote:
> Hi Reinette,
> 
> Noticed I didn't respond to this comment.
> 
> On 9/19/24 10:35, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 9/18/24 1:10 PM, Moger, Babu wrote:
>>> On 9/13/24 15:51, Reinette Chatre wrote:
>>>> On 8/16/24 9:16 AM, Babu Moger wrote:
>>
>> ...
>>
>>>>> +        if (enable) {
>>>>> +            ret = closid_alloc_sdciae(r);
>>>>> +            if (ret < 0) {
>>>>> +                rdt_last_cmd_puts("SDCIAE CLOSID is not available\n");
>>>>> +                goto out_sdciae;
>>>>> +            }
>>>>> +        } else {
>>>>> +            sdciae_closid = get_sdciae_closid(r);
>>>>> +            closid_free(sdciae_closid);
>>>>> +        }
>>>>
>>>>
>>>>> +
>>>>> +        ret = resctrl_arch_set_sdciae_enabled(RDT_RESOURCE_L3, enable);
>>>>
>>>> I assume that once SDCIAE is enabled the I/O traffic will start flowing to
>>>> whatever
>>>> was the last CBM of the max CLOSID? Is this intended or should there be
>>>> some default
>>>> CBM that this feature should start with?
>>>
>>> It will start with whatever the last CBM for max CLOSID.
>>
>> This seems arbitrary based on whatever allocation the previous resource group
>> using the CLOSID has. When a new resource group is created resctrl ensures
>> that it is created with all usable allocations, see rdtgroup_init_cat().
> 
> Checked again with with the team here. When SDCIAE is enabled, it uses the
> value in L3QosAllocMask15 (value in L3_MASK_15 MSR).  Enabling SDCIAE does
> not change the value of L3QosAllocMask15.

I see the issue as similar to how resource group allocations are managed.
Just like resctrl ensures that when a new resource group is created, it is done
with maximum allocations that the resource group may use ... not the allocations
left over from the previous resource group that used those MSRs.

I understand that the hardware uses L3QosAllocMask15 and does not change
L3QosAllocMask15 when SDCIAE is enabled, but resctrl is in a position to initialize
those registers at the time when SDCIAE is initialized to create a sane default
allocation, not an allocation of whatever happened to be in MSR at that time.

>> Letting cache injection start with whatever allocation remnant programmed
>> in a register does not seem ideal. What if, for example, after that resource
>> group was removed, a new exclusive resource group was created that overlaps
>> with that allocation? 
> 
> In that case. it will share the bit mask with the exclusive group. We may
> need to add a text about it.

No. This can be avoided entirely when resctrl initializes the MSR to a sane
default, no?

Reinette