linux-kernel - Re: [PATCH v11 10/23] x86/resctrl: Remove MSR reading of event configuration value

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] [day] [month] [year] [list]

Message-ID: <0fc8dbd4-07d8-40bd-8eec-402b48762807@zytor.com>
Date: Fri, 7 Feb 2025 02:07:19 -0800
From: Xin Li <xin@...or.com>
To: Reinette Chatre <reinette.chatre@...el.com>,
        Babu Moger <babu.moger@....com>, corbet@....net, tglx@...utronix.de,
        mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
        tony.luck@...el.com, peternewman@...gle.com
Cc: x86@...nel.org, hpa@...or.com, paulmck@...nel.org,
        akpm@...ux-foundation.org, thuth@...hat.com, rostedt@...dmis.org,
        xiongwei.song@...driver.com, pawan.kumar.gupta@...ux.intel.com,
        daniel.sneddon@...ux.intel.com, jpoimboe@...nel.org,
        perry.yuan@....com, sandipan.das@....com, kai.huang@...el.com,
        xiaoyao.li@...el.com, seanjc@...gle.com, xin3.li@...el.com,
        andrew.cooper3@...rix.com, ebiggers@...gle.com,
        mario.limonciello@....com, james.morse@....com,
        tan.shaopeng@...itsu.com, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, maciej.wieczor-retman@...el.com,
        eranian@...gle.com
Subject: Re: [PATCH v11 10/23] x86/resctrl: Remove MSR reading of event
 configuration value

On 2/6/2025 8:17 AM, Reinette Chatre wrote:
>>> +    wrmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, 0);
>> This is the existing code, however it would be better to use wrmsrl()
>> when the higher 32-bit are all 0s:
>>
>>      wrmsrl(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config);
>>
> Could you please elaborate what makes this change better?

In short, it takes one less argument, and doesn't pass an argument of 0.

The longer story is that hpa and I are refactoring the MSR access APIs
to accommodate the immediate form of MSR access instructions.  And we
are not happy about that there are too many MSR access APIs and their
uses are *random*.  The native wrmsr() and wrmsrl() are essentially the 
same and the only difference is that wrmsr() passes a 64-bit value to be
written into a MSR in *2* u32 arguments.  But we already have struct msr
defined in asm/shared/msr.h as:
	struct msr {
         	union {
                 	struct {
                         	u32 l;
	                        u32 h;
         	        };
                 	u64 q;
	        };
	};

it's more natural to do the same job with this data structure in most
cases.  And we want to remove wrmsr() and only keep wrmsrl(), thus a
developer won't have to figure out which one is better to use :-P.

For that to happen, one cleanup is to replace wrmsr(msr, low, 0) with
wrmsrl(msr, low) (low is automatically converted to u64 from u32).

However, I'm fine if Babu wants to keep it as-is.

Thanks!
     Xin