linux-kernel - Re: [PATCH v4 15/21] x86/resctrl: Abstract __rmid

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 for Android: free password hash cracker in your pocket

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

Message-ID: <e07c8739-5c8a-8fe9-039c-3a08ce873026@intel.com>
Date:   Tue, 7 Jun 2022 08:51:35 -0700
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     James Morse <james.morse@....com>, <x86@...nel.org>,
        <linux-kernel@...r.kernel.org>
CC:     Fenghua Yu <fenghua.yu@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        H Peter Anvin <hpa@...or.com>,
        Babu Moger <Babu.Moger@....com>,
        <shameerali.kolothum.thodi@...wei.com>,
        D Scott Phillips OS <scott@...amperecomputing.com>,
        <lcherian@...vell.com>, <bobo.shaobowang@...wei.com>,
        <tan.shaopeng@...itsu.com>, Jamie Iles <quic_jiles@...cinc.com>,
        Cristian Marussi <cristian.marussi@....com>,
        "Xin Hao" <xhao@...ux.alibaba.com>, <xingxin.hx@...nanolis.org>,
        <baolin.wang@...ux.alibaba.com>
Subject: Re: [PATCH v4 15/21] x86/resctrl: Abstract __rmid_read()

Hi James,

On 6/7/2022 5:07 AM, James Morse wrote:
> On 17/05/2022 22:23, Reinette Chatre wrote:
>> On 4/12/2022 5:44 AM, James Morse wrote:
>>
>>> @@ -180,14 +180,24 @@ static u64 __rmid_read(u32 rmid, enum resctrl_event_id eventid)
>>>  	 * are error bits.
>>>  	 */
>>>  	wrmsr(MSR_IA32_QM_EVTSEL, eventid, rmid);
>>> -	rdmsrl(MSR_IA32_QM_CTR, val);
>>> +	rdmsrl(MSR_IA32_QM_CTR, msr_val);
>>>  
>>> -	return val;
>>> +	if (msr_val & RMID_VAL_ERROR)
>>> +		return -EIO;
>>> +	if (msr_val & RMID_VAL_UNAVAIL)
>>> +		return -EINVAL;
>>> +
>>> +	*val = msr_val;
>>> +
>>> +	return 0;
>>>  }
>>>  
>>
>> In above EIO is used to represent RMID_VAL_ERROR ...
>>
>>> @@ -343,7 +355,7 @@ static u64 __mon_event_count(u32 rmid, struct rmid_read *rr)
>>>  		 * Code would never reach here because an invalid
>>>  		 * event id would fail the __rmid_read.
> 
> (I'll fix this comment)
> 
>>>  		 */
>>> -		return RMID_VAL_ERROR;
>>> +		return -EINVAL;
>>>  	}
>>>  
>>>  	if (rr->first) {
>>
>> I understand it can be seen as a symbolic change but could
>> RMID_VAL_ERROR consistently be associated with the same error?
> 
> This one isn't really RMID_VAL_ERROR - it was never read from the hardware, this was an
> invalid argument supplied by the caller.
> 
> You can only hit this if resctrl_arch_rmid_read() doesn't read RMID_VAL_ERROR from the
> hardware, because the hardware supports the event, but its an invalid argument as far as
> this code is concerned.
> 
> I'd prefer to avoid EIO as the error was not reported from hardware - its only reachable
> if the hardware does support the event!

ok, yes, that is fair. I believe no functional change is intended with this change so
please do highlight any such change(s).

Reinette