Christoph Egger
2008-Sep-12 12:31 UTC
[Xen-devel] [PATCH] microcode update support for AMD CPUs
Hi, Attached patch adds microcode update support for AMD CPUs Family10h and Family11h. It is based on a patch for Linux which is on its way for 2.6.28. Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> -- AMD Saxony, Dresden, Germany Operating System Research Center Legal Information: AMD Saxony Limited Liability Company & Co. KG Sitz (Geschäftsanschrift): Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland Registergericht Dresden: HRA 4896 vertretungsberechtigter Komplementär: AMD Saxony LLC (Sitz Wilmington, Delaware, USA) Geschäftsführer der AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2008-Sep-15 14:10 UTC
Re: [Xen-devel] [PATCH] microcode update support for AMD CPUs
Christoph Egger writes ("[Xen-devel] [PATCH] microcode update support for
AMD CPUs"):> + edx = (unsigned int)((unsigned long)(addr >> 32));
This is wrong. addr is an unsigned long, ie 32 bits so you are
shifting by >= the word size, which is not permitted. Helpfully, many
compilers warn about it.
But addr came from this:
addr = (unsigned long)&uci->mc.mc_amd->hdr.data_code;
surely this cannot be right if you''re expecting the address to be
longer than 32 bits in some cases ?
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2008-Sep-15 14:42 UTC
Re: [Xen-devel] [PATCH] microcode update support for AMD CPUs
On 15/9/08 15:10, "Ian Jackson" <Ian.Jackson@eu.citrix.com> wrote:> Christoph Egger writes ("[Xen-devel] [PATCH] microcode update support for AMD > CPUs"): >> + edx = (unsigned int)((unsigned long)(addr >> 32)); > > This is wrong. addr is an unsigned long, ie 32 bits so you are > shifting by >= the word size, which is not permitted. Helpfully, many > compilers warn about it. > > But addr came from this: > addr = (unsigned long)&uci->mc.mc_amd->hdr.data_code; > surely this cannot be right if you''re expecting the address to be > longer than 32 bits in some cases ?The code''s correct when targeting x86_64. It just isn''t careful enough for the i386 target. I''ll check in a fix, since it''s pretty trivial. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger
2008-Sep-15 15:26 UTC
Re: [Xen-devel] [PATCH] microcode update support for AMD CPUs
On Monday 15 September 2008 16:42:17 Keir Fraser wrote:> On 15/9/08 15:10, "Ian Jackson" <Ian.Jackson@eu.citrix.com> wrote: > > Christoph Egger writes ("[Xen-devel] [PATCH] microcode update support for > > AMD > > > > CPUs"): > >> + edx = (unsigned int)((unsigned long)(addr >> 32)); > > > > This is wrong. addr is an unsigned long, ie 32 bits so you are > > shifting by >= the word size, which is not permitted. Helpfully, many > > compilers warn about it. > > > > But addr came from this: > > addr = (unsigned long)&uci->mc.mc_amd->hdr.data_code; > > surely this cannot be right if you''re expecting the address to be > > longer than 32 bits in some cases ? > > The code''s correct when targeting x86_64. It just isn''t careful enough for > the i386 target. I''ll check in a fix, since it''s pretty trivial.Thanks for fixing this. I should have compile-tested for 32bit at least. gcc actually warns with shifting >= count . Christoph -- AMD Saxony, Dresden, Germany Operating System Research Center Legal Information: AMD Saxony Limited Liability Company & Co. KG Sitz (Geschäftsanschrift): Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland Registergericht Dresden: HRA 4896 vertretungsberechtigter Komplementär: AMD Saxony LLC (Sitz Wilmington, Delaware, USA) Geschäftsführer der AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel