All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yazen Ghannam <yazen.ghannam@amd.com>
To: <mingo@kernel.org>, <tglx@linutronix.de>, <bp@suse.de>,
	<hpa@zytor.com>, <linux-kernel@vger.kernel.org>
Cc: <linux-tip-commits@vger.kernel.org>
Subject: Re: [tip:ras/core] x86/RAS: Simplify SMCA HWID descriptor struct
Date: Thu, 10 Nov 2016 12:50:04 -0500	[thread overview]
Message-ID: <20161110175004.lji5k53ccli7gjig@yaz-fedora.dyhomenet> (raw)
In-Reply-To: <tip-1ce9cd7f9f0b71af7c496b816734bc2dc699363a@git.kernel.org>

>  static void get_smca_bank_info(unsigned int bank)
>  {
>  	unsigned int i, hwid_mcatype, cpu = smp_processor_id();
> -	struct smca_hwid_mcatype *type;
> +	struct smca_hwid *s_hwid;
>  	u32 high, instance_id;
> -	u16 hwid, mcatype;
>  
>  	/* Collect bank_info using CPU 0 for now. */
>  	if (cpu)
> @@ -162,14 +157,13 @@ static void get_smca_bank_info(unsigned int bank)
>  		return;
>  	}
>  
> -	hwid = high & MCI_IPID_HWID;
> -	mcatype = (high & MCI_IPID_MCATYPE) >> 16;
> -	hwid_mcatype = HWID_MCATYPE(hwid, mcatype);
> +	hwid_mcatype = HWID_MCATYPE(high & MCI_IPID_HWID,
> +				    (high & MCI_IPID_MCATYPE) >> 16);
>

Sorry for catching this late, but it seems this change doesn't compile
correctly. This causes the value of hwid_mcatype to be incorrect, so we
will never match a bank to its type.

I see this with GCC 4.8.5 and 5.4.0. 

There are no warnings or issues when building or booting just
that the behavior is incorrect. 

Disassembly of above change:
      db:       8b 45 e0                mov    -0x20(%rbp),%eax
      de:       41 89 c4                mov    %eax,%r12d
      e1:       25 00 00 ff 0f          and    $0xfff0000,%eax
      e6:       41 c1 ec 10             shr    $0x10,%r12d
      ea:       41 09 c4                or     %eax,%r12d

Disassembly of original code:
     286:       8b 45 d0                mov    -0x30(%rbp),%eax
     289:       41 89 c5                mov    %eax,%r13d
     28c:       c1 e8 10                shr    $0x10,%eax
     28f:       41 81 e5 ff 0f 00 00    and    $0xfff,%r13d
     296:       41 c1 e5 10             shl    $0x10,%r13d
     29a:       41 09 c5                or     %eax,%r13d

Adding extra parentheses in HWID_MCATYPE() gives the same assembly as the
original code and fixes the behavior.

> +	hwid_mcatype = HWID_MCATYPE((high & MCI_IPID_HWID)),
> +				    ((high & MCI_IPID_MCATYPE) >> 16));

Thanks,
Yazen

  reply	other threads:[~2016-11-10 19:21 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-03 12:55 [PATCH 1/3] x86/RAS: Simplify SMCA bank descriptor struct Borislav Petkov
2016-11-03 12:55 ` [PATCH 2/3] x86/RAS: Simplify SMCA HWID " Borislav Petkov
2016-11-08 16:20   ` [tip:ras/core] " tip-bot for Borislav Petkov
2016-11-10 17:50     ` Yazen Ghannam [this message]
2016-11-10 17:57       ` Borislav Petkov
2016-11-10 19:53         ` Thomas Gleixner
2016-11-10 20:12           ` Yazen Ghannam
2016-11-03 12:55 ` [PATCH 3/3] x86/RAS: Rename smca_bank_names to smca_names Borislav Petkov
2016-11-08 16:21   ` [tip:ras/core] " tip-bot for Borislav Petkov
2016-11-04 14:44 ` [PATCH 1/3] x86/RAS: Simplify SMCA bank descriptor struct Yazen Ghannam
2016-11-04 15:23   ` Borislav Petkov
2016-11-08 16:21     ` [tip:ras/core] x86/RAS: Hide SMCA bank names tip-bot for Borislav Petkov
2016-11-08 16:20 ` [tip:ras/core] x86/RAS: Simplify SMCA bank descriptor struct tip-bot for Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161110175004.lji5k53ccli7gjig@yaz-fedora.dyhomenet \
    --to=yazen.ghannam@amd.com \
    --cc=bp@suse.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.