On 15.08.2025 22:41, Andrew Cooper wrote:
> --- a/xen/arch/x86/nmi.c
> +++ b/xen/arch/x86/nmi.c
> @@ -218,16 +218,16 @@ void disable_lapic_nmi_watchdog(void)
> return;
> switch (boot_cpu_data.x86_vendor) {
> case X86_VENDOR_AMD:
> - wrmsr(MSR_K7_EVNTSEL0, 0, 0);
> + wrmsrns(MSR_K7_EVNTSEL0, 0);
Since you switch to non-serializing here, ...
> @@ -308,11 +308,11 @@ static void setup_k7_watchdog(void)
> | K7_EVNTSEL_USR
> | K7_NMI_EVENT;
>
> - wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
> + wrmsr(MSR_K7_EVNTSEL0, evntsel);
> write_watchdog_counter("K7_PERFCTR0");
> apic_write(APIC_LVTPC, APIC_DM_NMI);
> evntsel |= K7_EVNTSEL_ENABLE;
> - wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
> + wrmsr(MSR_K7_EVNTSEL0, evntsel);
> }
... why not also here?
> --- a/xen/arch/x86/oprofile/op_model_athlon.c
> +++ b/xen/arch/x86/oprofile/op_model_athlon.c
> @@ -34,7 +34,7 @@
> #define MAX_COUNTERS FAM15H_NUM_COUNTERS
>
> #define CTR_READ(msr_content,msrs,c) do {rdmsrl(msrs->counters[(c)].addr,
> (msr_content));} while (0)
> -#define CTR_WRITE(l,msrs,c) do {wrmsr(msrs->counters[(c)].addr, -(unsigned
> int)(l), -1);} while (0)
> +#define CTR_WRITE(l,msrs,c) do { wrmsr(msrs->counters[(c)].addr, -l); }
> while (0)
This isn't obviously correct (as in: no functional change): The macro is,
for example, passed reset_value[] contents, which is of type unsigned long.
Quite possible that the original code was wrong, though.
In any event l wants parenthesizing.
Jan
