Messages in this thread | | | Date | Tue, 7 Jun 2022 08:51:35 -0700 | Subject | Re: [PATCH v4 15/21] x86/resctrl: Abstract __rmid_read() | From | Reinette Chatre <> |
| |
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
| |