Re: [PATCH] x86/AMD: Apply workaround for AMD F16h Erratum792

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 05/02/2014 20:59, Aravind Gopalakrishnan wrote:
> Workaround for the Erratum will be in BIOSes spun only after
> Jan 2014 onwards. But initial production parts shipped in 2013
> itself. Since there is a coverage hole, we should carry this fix
> in software in case BIOS does not do the right thing or someone
> is using old BIOS.
>
> Refer to Revision Guide for AMD F16h models 00h-0fh, document 51810
> Rev. 3.04, November2013 for details on the Erratum.
>
> Tested the patch on Fam16h server platform and it works fine.
>
> Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
> Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>
> ---
>  xen/arch/x86/cpu/amd.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
> index 3307141..f2780c4 100644
> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -369,6 +369,7 @@ static void __devinit init_amd(struct cpuinfo_x86 *c)
>  	u32 l, h;
>  
>  	unsigned long long value;
> +	u32 pci_val;

Please move this to the scope created by the lower hunk.

>  
>  	/* Disable TLB flush filter by setting HWCR.FFDIS on K8
>  	 * bit 6 of msr C001_0015
> @@ -477,6 +478,35 @@ static void __devinit init_amd(struct cpuinfo_x86 *c)
>  		       " all your (PV) guest kernels. ***\n");
>  
>  	if (c->x86 == 0x16 && c->x86_model <= 0xf) {
> +        /*
> +         * Apply workaround for erratum 792
> +         * Description:
> +         * Processor does not ensure DRAM scrub read/write sequence
> +         * is atomic wrt accesses to CC6 save state area. Therefore
> +         * if a concurrent scrub read/write access is to same address
> +         * the entry may appear as if it is not written. This quirk
> +         * applies to Fam16h models 00h-0Fh
> +         *
> +         * See "Revision Guide" for AMD F16h models 00h-0fh,
> +         * document 51810 rev. 3.04, Nov 2013
> +         *
> +         * Equivalent Linux patch link:
> +         * http://marc.info/?l=linux-kernel&m=139066012217149&w=2
> +         */
> +        if (smp_processor_id() == 0) {
> +            pci_val = pci_conf_read32(0, 0, 0x18, 0x3, 0x58);
> +            if (pci_val & 0x1f) {
> +                pci_val &= ~(0x1f);

0x1f is by default an int, so you use a u suffix to make it u32 to use
as a mask.  Brackets are not really needed.

> +                pci_conf_write32(0, 0, 0x18, 0x3, 0x58, pci_val);
> +            }
> +
> +            pci_val = pci_conf_read32(0, 0, 0x18, 0x3, 0x5c);
> +            if (pci_val & 0x1) {
> +                pci_val &= ~(0x1);
> +                pci_conf_write32(0, 0, 0x18, 0x3, 0x5c, pci_val);
> +            }
> +        }
> +

Indentation needs some work.  This file derives from Linux so uses tabs
(at 8 spaces wide), rather than the Xen style of 4 spaces.

~Andrew

>  		rdmsrl(MSR_AMD64_LS_CFG, value);
>  		if (!(value & (1 << 15))) {
>  			static bool_t warned;