Re: [PATCH v2 05/16] x86/mce/amd: Clean up SMCA configuration

[Borislav Petkov]

On Thu, Apr 04, 2024 at 10:13:48AM -0500, Yazen Ghannam wrote:
> +	/*
> +	 * OS is required to set the MCAX enable bit to acknowledge that it is
> +	 * now using the new MSR ranges and new registers under each
> +	 * bank. It also means that the OS will configure deferred
> +	 * errors in the new MCA_CONFIG register. If the bit is not set,
> +	 * uncorrectable errors will cause a system panic.
> +	 */
> +	mca_config |= FIELD_PREP(CFG_MCAX_EN, 0x1);

Can we please drop this cryptic crap?

	mca_config |= SMCA_MCI_CONFIG_MCAX_ENABLE;

if I trust the PPR.

> -		this_cpu_ptr(mce_banks_array)[bank].lsb_in_status = !!(low & BIT(8));
> +	/*
> +	 * SMCA sets the Deferred Error Interrupt type per bank.
> +	 *
> +	 * MCA_CONFIG[DeferredIntTypeSupported] is bit 5, and tells us
> +	 * if the DeferredIntType bit field is available.
> +	 *
> +	 * MCA_CONFIG[DeferredIntType] is bits [38:37]. OS should set
> +	 * this to 0x1 to enable APIC based interrupt. First, check that
> +	 * no interrupt has been set.
> +	 */
> +	if (FIELD_GET(CFG_DFR_INT_SUPP, mca_config) && !FIELD_GET(CFG_DFR_INT_TYPE, mca_config))
> +		mca_config |= FIELD_PREP(CFG_DFR_INT_TYPE, INTR_TYPE_APIC);

Ditto:

	if (mca_config & CFG_DFR_INT_SUPP &&
	  !(mca_config & CFG_DFR_INT_TYPE))
		mca_config |= CFG_DFR_INT_TYPE | CFG_INTR_TYPE_APIC;

Plain and simple. Not this unreadable mess.

And use proper prefixes with the register name in them:

SMCA_MCI_CONFIG_

or so, for example.

>  
> -		wrmsr(smca_config, low, high);
> -	}
> +	if (FIELD_GET(CFG_LSB_IN_STATUS, mca_config))
> +		this_cpu_ptr(mce_banks_array)[bank].lsb_in_status = true;
> +
> +	wrmsrl(MSR_AMD64_SMCA_MCx_CONFIG(bank), mca_config);
> +}
> +
> +static void smca_configure_old(unsigned int bank, unsigned int cpu)

Yah, at the end of the patchset there should be patches which remove all
the _old things. Not in a different patchset. You can split things
accordingly.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette