[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Re: [PATCH] Correct SMBIOS handling of multiple tables
From: |
Alex Williamson |
Subject: |
Re: [Qemu-devel] Re: [PATCH] Correct SMBIOS handling of multiple tables |
Date: |
Wed, 27 May 2009 19:38:21 -0600 |
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 <address@hidden>
--
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)