From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1M9UZm-0007CY-Sr for qemu-devel@nongnu.org; Wed, 27 May 2009 21:38:34 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1M9UZi-0007CM-DG for qemu-devel@nongnu.org; Wed, 27 May 2009 21:38:34 -0400 Received: from [199.232.76.173] (port=38039 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1M9UZi-0007CJ-1w for qemu-devel@nongnu.org; Wed, 27 May 2009 21:38:30 -0400 Received: from g1t0028.austin.hp.com ([15.216.28.35]:35297) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1M9UZh-0003Gg-C4 for qemu-devel@nongnu.org; Wed, 27 May 2009 21:38:29 -0400 Subject: Re: [Qemu-devel] Re: [PATCH] Correct SMBIOS handling of multiple tables From: Alex Williamson In-Reply-To: <4A1DD659.7090604@us.ibm.com> References: <1243460525-25570-1-git-send-email-eak@us.ibm.com> <1243466249.20591.64.camel@lappy> <4A1DD659.7090604@us.ibm.com> Content-Type: text/plain Date: Wed, 27 May 2009 19:38:21 -0600 Message-Id: <1243474701.15223.12.camel@2710p.home> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Beth Kon Cc: bochs-developers@lists.sourceforge.net, qemu-devel@nongnu.org On Wed, 2009-05-27 at 20:10 -0400, Beth Kon wrote: > Without this patch, the current code has a problem even in the case of > default tables (nothing specified on command line). Following the code > path for add_struct(4, p, cpu_num) (for example) add_struct is called > once for each cpu. add_struct calls smbios_load_external, and the first > thing that does is check used_bitmap. The first pass through for table 4 > works. But if the vm is smp, the second pass through doesn't work > because used_bitmap reports that a table 4 entry was already created, so > smbios_load_external returns a 1 and add_struct becomes a noop in > effect. I think this patch does what you intended, to override default > creation with external creation if specified on the command line, and to > get only one complete set of tables for each type. Oh, I see the problem now, thanks for the explanation. I don't think that's the right fix though. If we remove the duplicate check at the start of smbios_load_external() and the user passes a binary, we might add it more than once. For instance the case where the user specifies '-smp 2' and passes 2 type 4 entries, they would end up with 4 type 4 entries. Don't we just need to only mark the bitmap if we load something externally? Something like this: Signed-off-by: Alex Williamson -- diff --git a/bios/rombios32.c b/bios/rombios32.c index f861f81..c869798 100644 --- a/bios/rombios32.c +++ b/bios/rombios32.c @@ -2554,13 +2554,14 @@ smbios_load_external(int type, char **p, unsigned *nr_structs, *max_struct_size = *p - (char *)header; } - /* Mark that we've reported on this type */ - used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f)); + if (start != *p) { + /* Mark that we've reported on this type */ + used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f)); + return 1; + } - return (start != *p); -#else /* !BX_QEMU */ +#endif /* !BX_QEMU */ return 0; -#endif } void smbios_init(void)