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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <63845ed3-f09e-43b2-5ded-394bbbd874c4@arm.com>
Date:   Tue, 7 Jun 2022 13:07:40 +0100
From:   James Morse <james.morse@....com>
To:     Reinette Chatre <reinette.chatre@...el.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 Reinette,

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!


Thanks,

James