linux-kernel - Re: [PATCH] x86/AMD: Apply erratum 688 on machines without a BIOS fix

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 22 Oct 2016 01:01:43 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     sonofagun@...nmailbox.org
Cc:     linux-kernel@...r.kernel.org, Nikos Barkas <levelwol@...il.com>
Subject: Re: [PATCH] x86/AMD: Apply erratum 688 on machines without a BIOS fix

On Sat, Oct 22, 2016 at 12:51:32AM +0300, sonofagun@...nmailbox.org wrote:
> Thank you for your time! I have chosen reply to list and all recipients, it
> must work now.

Yes, exactly what I had in mind.

> My brother rejected the proposed patch because it does not provide
> equivalent functionality with the original.
> 
> Our initial patch would fix 3 broken models and 1 working model. Your patch
> will only work for 1 model. Only machines having our APU will be fixed. All
> B0 APUs will be unpatched. This is not right. Check the revision guide to
> verify that.

Right you are: I read too much into the description of bit 2 of
D18F4x164. Of course we want to apply that fix to to ON-Bs too.

> To avoid unneeded complexity we propose this patch as V2, do you agree?
> 
> +#define MSR_AMD64_IC_CFG	0xC0011021
> +
> +static void init_amd_on(struct cpuinfo_x86 *c)
> +{
> +	/*
> +	 * Apply erratum 688 fix so machines without a BIOS
> +	 * fix work.
> +	 */
> +
> +	u32 val = pci_read_config(0, 0x18, 0x4, 0x164);
> +
> +	if (!(val & BIT(2))) {
> +		msr_set_bit(MSR_AMD64_IC_CFG, 3);
> +		msr_set_bit(MSR_AMD64_IC_CFG, 14);

Yes, that should work fine.

Btw, there's missing a closing } for the if-test here.

> +}
>  static void init_amd_bd(struct cpuinfo_x86 *c)
>  {
>  	u64 value;
> @@ -738,6 +750,7 @@ static void init_amd(struct cpuinfo_x86
>  	case 0xf:  init_amd_k8(c); break;
>  	case 0x10: init_amd_gh(c); break;
>  	case 0x12: init_amd_ln(c); break;
> +	case 0x14: init_amd_on(c); break;
>  	case 0x15: init_amd_bd(c); break;
>  	}
> 
> Please advice to proceed!

Right, please send a tested version of the above with the explanation
text from your initial submission.

Thanks.

> erratum 721 :-(

Hmm, interesting.

Do you have a way to trigger that one?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.