From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Bill Rieske" Subject: Re: [PATCH] bios: resolve memory device roll over reporting issues with >32G guests Date: Mon, 10 Nov 2008 09:31:16 -0700 Message-ID: <4917FF62.860B.008B.3@novell.com> References: <491456C7.860B.008B.3@novell.com> <4916E275.2000802@redhat.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__Part9CB4C0C4.1__=" Cc: To: Return-path: Received: from lucius.provo.novell.com ([137.65.248.127]:35191 "EHLO lucius.provo.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753163AbYKJQbT (ORCPT ); Mon, 10 Nov 2008 11:31:19 -0500 In-Reply-To: <4916E275.2000802@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is a MIME message. If you are reading this text, you may want to consider changing to a mail reader or gateway that understands how to properly handle MIME multipart messages. --=__Part9CB4C0C4.1__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline > Instead of this, can you add an snprintf() (in a separate patch) and = use=20 > it? There's already a vsnprintf() so all the heavy machinery is in = place. Nice catch much cleaner The field within the Memory Device type 17 is only a word with the MSB = being=20 used to report MB/KB. Thereby, a guest with 32G and greater would = report=20 incorrect memory device information rolling over to 0. This presents more than one memory device and associated memory structures= =20 if the memory is larger than 16G Signed-off-by: Bill Rieske --=__Part9CB4C0C4.1__= Content-Type: text/plain; name="smbios-large-mem.diff" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="smbios-large-mem.diff" diff --git a/bios/rombios32.c b/bios/rombios32.c=0Aindex a91b155..5442e70 = 100755=0A--- a/bios/rombios32.c=0A+++ b/bios/rombios32.c=0A@@ -370,6 = +370,17 @@ int vsnprintf(char *buf, int buflen, const char *fmt, va_list = args)=0A return buf - buf0;=0A }=0A =0A+int snprintf(char * buf, = size_t size, const char *fmt, ...)=0A+{=0A+ va_list args;=0A+ = int i;=0A+=0A+ va_start(args, fmt);=0A+ i=3Dvsnprintf(buf,size,fmt,= args);=0A+ va_end(args);=0A+ return i;=0A+}=0A+=0A void = bios_printf(int flags, const char *fmt, ...)=0A {=0A va_list ap;=0A@@ = -1914,7 +1925,7 @@ smbios_type_4_init(void *start, unsigned int cpu_number)= =0A =0A /* Type 16 -- Physical Memory Array */=0A static void *=0A-smbios_t= ype_16_init(void *start, uint32_t memsize)=0A+smbios_type_16_init(void = *start, uint32_t memsize, int nr_mem_devs)=0A {=0A struct smbios_type_1= 6 *p =3D (struct smbios_type_16*)start;=0A =0A@@ -1927,7 +1938,7 @@ = smbios_type_16_init(void *start, uint32_t memsize)=0A p->error_correcti= on =3D 0x01; /* other */=0A p->maximum_capacity =3D memsize * 1024;=0A = p->memory_error_information_handle =3D 0xfffe; /* none provided */=0A- = p->number_of_memory_devices =3D 1;=0A+ p->number_of_memory_devices = =3D nr_mem_devs;=0A =0A start +=3D sizeof(struct smbios_type_16);=0A = *((uint16_t *)start) =3D 0;=0A@@ -1937,20 +1948,19 @@ smbios_type_16_init= (void *start, uint32_t memsize)=0A =0A /* Type 17 -- Memory Device */=0A = static void *=0A-smbios_type_17_init(void *start, uint32_t memory_size_mb)= =0A+smbios_type_17_init(void *start, uint32_t memory_size_mb, int = instance)=0A {=0A struct smbios_type_17 *p =3D (struct smbios_type_17 = *)start;=0A =0A p->header.type =3D 17;=0A p->header.length =3D = sizeof(struct smbios_type_17);=0A- p->header.handle =3D 0x1100;=0A+ = p->header.handle =3D 0x1100 + instance;=0A =0A p->physical_memory_array= _handle =3D 0x1000;=0A p->total_width =3D 64;=0A p->data_width =3D = 64;=0A- /* truncate memory_size_mb to 16 bits and clear most significant= =0A- bit [indicates size in MB] */=0A- p->size =3D (uint16_t) = memory_size_mb & 0x7fff;=0A+/* TODO: should assert in case something is = wrong ASSERT((memory_size_mb & ~0x7fff) =3D=3D 0); */ =0A+ p->size = =3D memory_size_mb;=0A p->form_factor =3D 0x09; /* DIMM */=0A = p->device_set =3D 0;=0A p->device_locator_str =3D 1;=0A@@ -1959,8 = +1969,8 @@ smbios_type_17_init(void *start, uint32_t memory_size_mb)=0A = p->type_detail =3D 0;=0A =0A start +=3D sizeof(struct smbios_type_17);= =0A- memcpy((char *)start, "DIMM 1", 7);=0A- start +=3D 7;=0A+ = snprintf(start, 8, "DIMM %d", instance); =0A+ start +=3D strlen(start) = + 1;=0A *((uint8_t *)start) =3D 0;=0A =0A return start+1;=0A@@ = -1968,16 +1978,16 @@ smbios_type_17_init(void *start, uint32_t memory_size_= mb)=0A =0A /* Type 19 -- Memory Array Mapped Address */=0A static void = *=0A-smbios_type_19_init(void *start, uint32_t memory_size_mb)=0A+smbios_ty= pe_19_init(void *start, uint32_t memory_size_mb, int instance)=0A {=0A = struct smbios_type_19 *p =3D (struct smbios_type_19 *)start;=0A =0A = p->header.type =3D 19;=0A p->header.length =3D sizeof(struct smbios_typ= e_19);=0A- p->header.handle =3D 0x1300;=0A+ p->header.handle =3D = 0x1300 + instance;=0A =0A- p->starting_address =3D 0;=0A- p->ending_a= ddress =3D (memory_size_mb * 1024) - 1;=0A+ p->starting_address =3D = instance << 24;=0A+ p->ending_address =3D p->starting_address + = (memory_size_mb << 10) - 1;=0A p->memory_array_handle =3D 0x1000;=0A = p->partition_width =3D 1;=0A =0A@@ -1989,18 +1999,18 @@ smbios_type_19_in= it(void *start, uint32_t memory_size_mb)=0A =0A /* Type 20 -- Memory = Device Mapped Address */=0A static void *=0A-smbios_type_20_init(void = *start, uint32_t memory_size_mb)=0A+smbios_type_20_init(void *start, = uint32_t memory_size_mb, int instance)=0A {=0A struct smbios_type_20 = *p =3D (struct smbios_type_20 *)start;=0A =0A p->header.type =3D = 20;=0A p->header.length =3D sizeof(struct smbios_type_20);=0A- = p->header.handle =3D 0x1400;=0A+ p->header.handle =3D 0x1400 + = instance;=0A =0A- p->starting_address =3D 0;=0A- p->ending_address = =3D (memory_size_mb * 1024) - 1;=0A- p->memory_device_handle =3D = 0x1100;=0A- p->memory_array_mapped_address_handle =3D 0x1300;=0A+ = p->starting_address =3D instance << 24;=0A+ p->ending_address =3D = p->starting_address + (memory_size_mb << 10) - 1;=0A+ p->memory_device_h= andle =3D 0x1100 + instance;=0A+ p->memory_array_mapped_address_handle = =3D 0x1300 + instance;=0A p->partition_row_position =3D 1;=0A = p->interleave_position =3D 0;=0A p->interleaved_data_depth =3D 0;=0A@@ = -2051,6 +2061,7 @@ void smbios_init(void)=0A char *start, *p, *q;=0A = int memsize =3D (ram_end =3D=3D ram_size) ? ram_size / (1024 * 1024) = :=0A (ram_end - (1ull << 32) + ram_size) / (1024 * = 1024);=0A+ int i, nr_mem_devs;=0A =0A #ifdef BX_USE_EBDA_TABLES=0A = ebda_cur_addr =3D align(ebda_cur_addr, 16);=0A@@ -2062,23 +2073,32 @@ void = smbios_init(void)=0A =0A p =3D (char *)start + sizeof(struct = smbios_entry_point);=0A =0A-#define add_struct(fn) { \=0A+#define = add_struct(fn) do{ \=0A q =3D (fn); \=0A nr_structs++; \=0A if = ((q - p) > max_struct_size) \=0A max_struct_size =3D q - p; \=0A = p =3D q; \=0A-}=0A+}while (0)=0A =0A add_struct(smbios_type_0_init(p)= );=0A add_struct(smbios_type_1_init(p));=0A add_struct(smbios_type_= 3_init(p));=0A for (cpu_num =3D 1; cpu_num <=3D smp_cpus; cpu_num++)=0A= add_struct(smbios_type_4_init(p, cpu_num));=0A- add_struct(smbi= os_type_16_init(p, memsize));=0A- add_struct(smbios_type_17_init(p, = memsize));=0A- add_struct(smbios_type_19_init(p, ram_end / (1024 * = 1024)));=0A- add_struct(smbios_type_20_init(p, ram_end / (1024 * = 1024)));=0A+=0A+ /* Each 'memory device' covers up to 16GB of address = space. */=0A+ nr_mem_devs =3D (memsize + 0x3fff) >> 14;=0A+ = add_struct(smbios_type_16_init(p, memsize, nr_mem_devs));=0A+ for ( i = =3D 0; i < nr_mem_devs; i++ )=0A+ {=0A+ uint32_t dev_memsize =3D = ((i =3D=3D (nr_mem_devs - 1))=0A+ ? = (memsize & 0x3fff) : 0x4000);=0A+ add_struct(smbios_type_17_init(p, = dev_memsize, i));=0A+ add_struct(smbios_type_19_init(p, dev_memsize,= i));=0A+ add_struct(smbios_type_20_init(p, dev_memsize, i));=0A+ = }=0A+=0A add_struct(smbios_type_32_init(p));=0A add_struct(smbios_= type_127_init(p));=0A =0A --=__Part9CB4C0C4.1__=--