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

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

[Aravind Gopalakrishnan]

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;
 
 	/* 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);
+                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);
+            }
+        }
+
 		rdmsrl(MSR_AMD64_LS_CFG, value);
 		if (!(value & (1 << 15))) {
 			static bool_t warned;
-- 
1.8.1.2

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

[Andrew Cooper]

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;

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

[Aravind Gopalakrishnan]

On Wed, Feb 05, 2014 at 08:54:42PM +0000, Andrew Cooper wrote:
> On 05/02/2014 20:59, Aravind Gopalakrishnan wrote:
> >  	unsigned long long value;
> > +	u32 pci_val;
> 
> Please move this to the scope created by the lower hunk.
> 

Done

> > +            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.
> 

Done

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

Done;
Sending out changes in V2.

Thanks,
Aravind.

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

[Jan Beulich]

>>> On 05.02.14 at 21:54, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 05/02/2014 20:59, Aravind Gopalakrishnan wrote:
>> +        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.

This seems pretty pointless a request - conversion from signed to
unsigned types is well defined, even more so when the converted
value is representable by both.

Jan