Messages in this thread | | | Date | Mon, 26 Sep 2022 17:33:13 +0200 | Subject | Re: [PATCH 1/3] xen/pv: allow pmu msr accesses to cause GP | From | Juergen Gross <> |
| |
On 26.09.22 17:29, Jan Beulich wrote: > On 26.09.2022 16:18, Juergen Gross wrote: >> Today pmu_msr_read() and pmu_msr_write() fall back to the safe variants >> of read/write MSR in case the MSR access isn't emulated via Xen. Allow >> the caller to select the potentially faulting variant by passing NULL >> for the error pointer. > > Maybe make this "the sole caller" or some such? Because if there were > multiple, they might easily disagree on what the best meaning of passing > NULL is.
Okay.
> >> --- a/arch/x86/xen/pmu.c >> +++ b/arch/x86/xen/pmu.c >> @@ -293,22 +293,24 @@ static bool xen_amd_pmu_emulate(unsigned int msr, u64 *val, bool is_read) >> bool pmu_msr_read(unsigned int msr, uint64_t *val, int *err) >> { >> if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) { >> - if (is_amd_pmu_msr(msr)) { >> - if (!xen_amd_pmu_emulate(msr, val, 1)) >> - *val = native_read_msr_safe(msr, err); >> - return true; >> + if (!is_amd_pmu_msr(msr)) >> + return false; >> + if (!xen_amd_pmu_emulate(msr, val, 1)) { >> + *val = err ? native_read_msr_safe(msr, err) >> + : native_read_msr(msr); >> } >> + return true; > > Minor remark: Fold this and ... > >> } else { >> int type, index; >> >> - if (is_intel_pmu_msr(msr, &type, &index)) { >> - if (!xen_intel_pmu_emulate(msr, val, type, index, 1)) >> - *val = native_read_msr_safe(msr, err); >> - return true; >> + if (!is_intel_pmu_msr(msr, &type, &index)) >> + return false; >> + if (!xen_intel_pmu_emulate(msr, val, type, index, 1)) { >> + *val = err ? native_read_msr_safe(msr, err) >> + : native_read_msr(msr); >> } >> + return true; > > ... this by moving them ... > >> } >> - >> - return false; >> } > > ... above here? You might even de-duplicate the native_read_msr{,_safe}() > invocations by moving them out of the if/else ...
Oh, nice idea!
Juergen
[unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature] | |