From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1M9TAL-0006p6-Hz for qemu-devel@nongnu.org; Wed, 27 May 2009 20:08:13 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1M9TAH-0006ou-RC for qemu-devel@nongnu.org; Wed, 27 May 2009 20:08:13 -0400 Received: from [199.232.76.173] (port=43936 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1M9TAH-0006or-EY for qemu-devel@nongnu.org; Wed, 27 May 2009 20:08:09 -0400 Received: from e38.co.us.ibm.com ([32.97.110.159]:56305) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1M9TAG-0005pl-Ua for qemu-devel@nongnu.org; Wed, 27 May 2009 20:08:09 -0400 Received: from d03relay02.boulder.ibm.com (d03relay02.boulder.ibm.com [9.17.195.227]) by e38.co.us.ibm.com (8.13.1/8.13.1) with ESMTP id n4S05Fj1032199 for ; Wed, 27 May 2009 18:05:15 -0600 Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d03relay02.boulder.ibm.com (8.13.8/8.13.8/NCO v9.2) with ESMTP id n4S085BW250904 for ; Wed, 27 May 2009 18:08:05 -0600 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n4S084Uo031739 for ; Wed, 27 May 2009 18:08:05 -0600 Message-ID: <4A1DD659.7090604@us.ibm.com> Date: Wed, 27 May 2009 20:10:01 -0400 From: Beth Kon MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH] Correct SMBIOS handling of multiple tables References: <1243460525-25570-1-git-send-email-eak@us.ibm.com> <1243466249.20591.64.camel@lappy> In-Reply-To: <1243466249.20591.64.camel@lappy> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: bochs-developers@lists.sourceforge.net, qemu-devel@nongnu.org Alex Williamson wrote: > On Wed, 2009-05-27 at 17:42 -0400, Beth Kon wrote: > >> The current code prevents multiple entries of the same type table, as required, >> for example, by table type 4. >> > > Hi Beth, > > Are you trying to add multiple type 4 entries from a single type 4 > binary passed in via -smbios file=? ex: > > qemu -smp 2 -smbios file=type4.bin > > My intention was that once the user overrides an entry with a binary > image, they're responsible for providing all of the entries of that > type. That means for a 2-way guest, if you want to override the type 4 > entry with a binary, you need to specify it twice on the command line. > The code below would allow you to load a type 4 entry for each vCPU > while only specifying one on the command line, but what if you want 2 > slightly different entries? We have no way of associating a provided > entry to the one needed at table creating time, which is why we load > them all on the first pass and ignore requests to add the same type > again later. Thanks, > > Alex > Hi Alex. No, I wasn't trying to allow only one table to be specified on the command line for multiple same-type tables. The user is still responsible for providing all the entries. 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. > > >> diff --git a/bios/rombios32.c b/bios/rombios32.c >> index f861f81..43aa065 100644 >> --- a/bios/rombios32.c >> +++ b/bios/rombios32.c >> @@ -2146,6 +2146,10 @@ struct smbios_table { >> #define SMBIOS_FIELD_ENTRY 0 >> #define SMBIOS_TABLE_ENTRY 1 >> >> +#ifdef BX_QEMU >> + static uint64_t smbios_used_bitmap[4] = { 0 }; >> +#endif >> + >> static size_t >> smbios_load_field(int type, size_t offset, void *addr) >> { >> @@ -2496,14 +2500,9 @@ smbios_load_external(int type, char **p, unsigned *nr_structs, >> unsigned *max_struct_size) >> { >> #ifdef BX_QEMU >> - static uint64_t used_bitmap[4] = { 0 }; >> char *start = *p; >> int i; >> >> - /* Check if we've already reported these tables */ >> - if (used_bitmap[(type >> 6) & 0x3] & (1ULL << (type & 0x3f))) >> - return 1; >> - >> /* Don't introduce spurious end markers */ >> if (type == 127) >> return 0; >> @@ -2555,7 +2554,7 @@ smbios_load_external(int type, char **p, unsigned *nr_structs, >> } >> >> /* Mark that we've reported on this type */ >> - used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f)); >> + smbios_used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f)); >> >> return (start != *p); >> #else /* !BX_QEMU */ >> @@ -2612,6 +2611,9 @@ void smbios_init(void) >> add_struct(32, p); >> /* Add any remaining provided entries before the end marker */ >> for (i = 0; i < 256; i++) >> + /* Check if we've already reported these tables */ >> + if (smbios_used_bitmap[(i >> 6) & 0x3] & (1ULL << (i & 0x3f))) >> + continue; >> smbios_load_external(i, &p, &nr_structs, &max_struct_size); >> add_struct(127, p); >> >> >> > > > > >