When running the Linux Kernel, on some systems that doesn't have any DMI
table (like a Xen domU), some dmi_* calls can generates Warnings like :
>/ WARNING: at /usr/src/linux-2.6.29.1/drivers/firmware/dmi_scan.c:425/
>/ dmi_matches+0x7e/0x80()/
>/ dmi check: not initialized yet/
Some users reported this error :
http://lists.xensource.com/archives/html/xen-users/2009-04/msg00128.html
https://qa.mandriva.com/show_bug.cgi?id=54775
When the kernel is compiled with CONFIG_DMI, dmi_check_system(),
dmi_first_match(), dmi_name_in_vendors(), dmi_find_device(),
dmi_get_date(), dmi_match() calls doesn't check the status of the
dmi_available variable.
When this functions are called and if no valid dmi table has been found,
this pretty simple patch just return the default values returned when
CONFIG_DMI isn't set.
This patch applies to the lastest git tree.
I'm CCing the x86 maintainers as I can't find any maintainer of
drivers/firmware/dmi.
Signed-off-by: Erwan Velu <[email protected]>
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 938100f..ea8b433 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -457,6 +457,9 @@ int dmi_check_system(const struct dmi_system_id *list)
int count = 0;
const struct dmi_system_id *d;
+ if (!dmi_available)
+ return 0;
+
for (d = list; d->ident; d++)
if (dmi_matches(d)) {
count++;
@@ -484,6 +487,9 @@ const struct dmi_system_id *dmi_first_match(const
struct dmi_system_id *list)
{
const struct dmi_system_id *d;
+ if (!dmi_available)
+ return NULL;
+
for (d = list; d->ident; d++)
if (dmi_matches(d))
return d;
@@ -501,6 +507,9 @@ EXPORT_SYMBOL(dmi_first_match);
*/
const char *dmi_get_system_info(int field)
{
+ if (!dmi_available)
+ return NULL;
+
return dmi_ident[field];
}
EXPORT_SYMBOL(dmi_get_system_info);
@@ -512,6 +521,10 @@ EXPORT_SYMBOL(dmi_get_system_info);
int dmi_name_in_serial(const char *str)
{
int f = DMI_PRODUCT_SERIAL;
+
+ if (!dmi_available)
+ return 0;
+
if (dmi_ident[f] && strstr(dmi_ident[f], str))
return 1;
return 0;
@@ -527,6 +540,10 @@ int dmi_name_in_vendors(const char *str)
DMI_PRODUCT_NAME, DMI_PRODUCT_VERSION, DMI_BOARD_VENDOR,
DMI_BOARD_NAME, DMI_BOARD_VERSION, DMI_NONE };
int i;
+
+ if (!dmi_available)
+ return 0;
+
for (i = 0; fields[i] != DMI_NONE; i++) {
int f = fields[i];
if (dmi_ident[f] && strstr(dmi_ident[f], str))
@@ -554,6 +571,9 @@ const struct dmi_device * dmi_find_device(int type,
const char *name,
const struct list_head *head = from ? &from->list : &dmi_devices;
struct list_head *d;
+ if (!dmi_available)
+ return NULL;
+
for(d = head->next; d != &dmi_devices; d = d->next) {
const struct dmi_device *dev =
list_entry(d, struct dmi_device, list);
@@ -592,6 +612,16 @@ bool dmi_get_date(int field, int *yearp, int
*monthp, int *dayp)
const char *s, *y;
char *e;
+ if (!dmi_available) {
+ if (yearp)
+ *yearp = 0;
+ if (monthp)
+ *monthp = 0;
+ if (dayp)
+ *dayp = 0;
+ return false;
+ }
+
s = dmi_get_system_info(field);
exists = s;
if (!exists)
@@ -676,6 +706,9 @@ bool dmi_match(enum dmi_field f, const char *str)
{
const char *info = dmi_get_system_info(f);
+ if (!dmi_available)
+ return false;
+
if (info == NULL || str == NULL)
return info == str;
On Fri, 2009-10-23 at 11:34 +0200, Erwan Velu wrote:
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 938100f..ea8b433 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -457,6 +457,9 @@ int dmi_check_system(const struct dmi_system_id
> *list)
> int count = 0;
> const struct dmi_system_id *d;
>
> + if (!dmi_available)
> + return 0;
> +
> for (d = list; d->ident; d++)
> if (dmi_matches(d)) {
> count++;
It looks like all the tabs have been removed from the code. Did you copy
and paste the patch content into this email? Often times that will
remove the tabs.
Daniel
Daniel Walker a ?crit :
> It looks like all the tabs have been removed from the code. Did you copy
> and paste the patch content into this email? Often times that will
> remove the tabs
The original patch is here : http://konilope.linuxeries.org/dmi_scan.patch
Hope it's better.
On Fri, 2009-10-23 at 12:34 +0200, Erwan Velu wrote:
> Daniel Walker a écrit :
> > It looks like all the tabs have been removed from the code. Did you copy
> > and paste the patch content into this email? Often times that will
> > remove the tabs
> The original patch is here : http://konilope.linuxeries.org/dmi_scan.patch
>
> Hope it's better.
Yeah that one looks better.. Usually email clients have a command line
"Insert File" which you can use to include the patch without removing
the tabs. Just a note for the next patch you send, although you might
want to resubmit this one ..
Daniel
Daniel Walker a écrit :
> Yeah that one looks better.. Usually email clients have a command line
> "Insert File" which you can use to include the patch without removing
> the tabs. Just a note for the next patch you send, although you might
> want to resubmit this one ..
>
Thx for your comments, that was my first submission. Will do it better
next time.
* Erwan Velu <[email protected]> wrote:
> Daniel Walker a ?crit :
>> Yeah that one looks better.. Usually email clients have a command line
>> "Insert File" which you can use to include the patch without removing
>> the tabs. Just a note for the next patch you send, although you might
>> want to resubmit this one ..
>>
>
> Thx for your comments, that was my first submission. Will do it better
> next time.
See Documentation/email-clients.txt for a detailed list of email clients
and how to submit patches using them.
Ingo
* Erwan Velu <[email protected]> wrote:
> When running the Linux Kernel, on some systems that doesn't have any DMI
> table (like a Xen domU), some dmi_* calls can generates Warnings like :
>
>> / WARNING: at /usr/src/linux-2.6.29.1/drivers/firmware/dmi_scan.c:425/
>> / dmi_matches+0x7e/0x80()/
>> / dmi check: not initialized yet/
Empty DMI tables are common. What is not common is to call dmi_matches()
before the DMI strings code has initialized.
> Some users reported this error :
> http://lists.xensource.com/archives/html/xen-users/2009-04/msg00128.html
> https://qa.mandriva.com/show_bug.cgi?id=54775
Looks like a Xen bug. DMI matching functions should be called after that
code has initialized. The warning was added to catch such early calls.
Your patch works around that bug and the warning.
Ingo
Ingo Molnar a ?crit :
> Looks like a Xen bug. DMI matching functions should be called after that
> code has initialized. The warning was added to catch such early calls.
>
> Your patch works around that bug and the warning.
>
So I think you'll drop it and we have to make xen people fix it ?
On Fri, 2009-10-23 at 13:49 +0200, Erwan Velu wrote:
> Ingo Molnar a écrit :
> > Looks like a Xen bug. DMI matching functions should be called after that
> > code has initialized. The warning was added to catch such early calls.
> >
> > Your patch works around that bug and the warning.
> >
> So I think you'll drop it and we have to make xen people fix it ?
It's your defect, so you can still try to fix it (unless the "xen
people" or someone else beats you to it.)
It looks like on a normal system dmi_scan_machine() gets called very
early in setup_arch() arch/x86/kernel/setup.c . A possible good fix
might be to add a dmi_disable() into the dmi driver that just shuts off
dmi, and run that in xen_arch_setup() in arch/x86/xen/setup.c .
Daniel
Daniel Walker a écrit :
> [...]
> It's your defect, so you can still try to fix it (unless the "xen
> people" or someone else beats you to it.)
>
> It looks like on a normal system dmi_scan_machine() gets called very
> early in setup_arch() arch/x86/kernel/setup.c . A possible good fix
> might be to add a dmi_disable() into the dmi driver that just shuts off
> dmi, and run that in xen_arch_setup() in arch/x86/xen/setup.c
Could it make sense having this patch (I can work on it) while keeping
my previous patch ?
Does it make sense keeping the default return value I've been adding
when no dmi table is found ?
On Fri, 2009-10-23 at 17:03 +0200, Erwan Velu wrote:
> Daniel Walker a écrit :
> > [...]
> > It's your defect, so you can still try to fix it (unless the "xen
> > people" or someone else beats you to it.)
> >
> > It looks like on a normal system dmi_scan_machine() gets called very
> > early in setup_arch() arch/x86/kernel/setup.c . A possible good fix
> > might be to add a dmi_disable() into the dmi driver that just shuts off
> > dmi, and run that in xen_arch_setup() in arch/x86/xen/setup.c
> Could it make sense having this patch (I can work on it) while keeping
> my previous patch ?
> Does it make sense keeping the default return value I've been adding
> when no dmi table is found ?
Ingo mentioned that the returning mechanism your adding was left out
intentionally to catch this error, so I don't think your original patch
could be included ..
Daniel
* Daniel Walker <[email protected]> wrote:
> On Fri, 2009-10-23 at 17:03 +0200, Erwan Velu wrote:
> > Daniel Walker a ?crit :
> > > [...]
> > > It's your defect, so you can still try to fix it (unless the "xen
> > > people" or someone else beats you to it.)
> > >
> > > It looks like on a normal system dmi_scan_machine() gets called very
> > > early in setup_arch() arch/x86/kernel/setup.c . A possible good fix
> > > might be to add a dmi_disable() into the dmi driver that just shuts off
> > > dmi, and run that in xen_arch_setup() in arch/x86/xen/setup.c
> > Could it make sense having this patch (I can work on it) while keeping
> > my previous patch ?
> > Does it make sense keeping the default return value I've been adding
> > when no dmi table is found ?
>
> Ingo mentioned that the returning mechanism your adding was left out
> intentionally to catch this error, so I don't think your original
> patch could be included ..
Yes. That mechanism found a real bug here.
Calling the DMI code too early (when the strings are still empty) can
cause silent failures: we wont crash but we might miss to act on DMI
quirks.
Ingo
On 10/23/09 02:34, Erwan Velu wrote:
> When running the Linux Kernel, on some systems that doesn't have any
> DMI table (like a Xen domU), some dmi_* calls can generates Warnings
> like :
>
>> / WARNING: at /usr/src/linux-2.6.29.1/drivers/firmware/dmi_scan.c:425/
>> / dmi_matches+0x7e/0x80()/
>> / dmi check: not initialized yet/
>
> Some users reported this error :
> http://lists.xensource.com/archives/html/xen-users/2009-04/msg00128.html
> https://qa.mandriva.com/show_bug.cgi?id=54775
I don't think either of those reports are for kernels with upstream Xen
support. Novell reimplements their own Xen support in their kernels; I
don't know whether Mandriva repackages the Novell kernel or not, but
some distros do.
> When the kernel is compiled with CONFIG_DMI, dmi_check_system(),
> dmi_first_match(), dmi_name_in_vendors(), dmi_find_device(),
> dmi_get_date(), dmi_match() calls doesn't check the status of the
> dmi_available variable.
>
> When this functions are called and if no valid dmi table has been
> found, this pretty simple patch just return the default values
> returned when CONFIG_DMI isn't set.
>
> This patch applies to the lastest git tree.
> I'm CCing the x86 maintainers as I can't find any maintainer of
> drivers/firmware/dmi.
This doesn't make any sense to me. dmi_scan_machine() should be called
in the normal place under Xen, so it will initialize the DMI subsystem.
There won't be any DMI table in domU, but that's OK.
Please include a full boot log showing the problem. "DMI not present or
invalid" should always be present in the kernel log.
J
On 10/23/09 08:30, Ingo Molnar wrote:
>> Ingo mentioned that the returning mechanism your adding was left out
>> intentionally to catch this error, so I don't think your original
>> patch could be included ..
>>
> Yes. That mechanism found a real bug here.
>
> Calling the DMI code too early (when the strings are still empty) can
> cause silent failures: we wont crash but we might miss to act on DMI
> quirks.
>
Yes. There's nothing preventing the DMI subsystem from being
initialized under Xen; in fact we rely on it in a dom0 kernel (which
does have access to the DMI tables). I don't know what the underlying
bug in the original report is, but there's more to it than failing to
init DMI.
J
* Jeremy Fitzhardinge <[email protected]> wrote:
> On 10/23/09 08:30, Ingo Molnar wrote:
> >> Ingo mentioned that the returning mechanism your adding was left out
> >> intentionally to catch this error, so I don't think your original
> >> patch could be included ..
> >>
> > Yes. That mechanism found a real bug here.
> >
> > Calling the DMI code too early (when the strings are still empty)
> > can cause silent failures: we wont crash but we might miss to act on
> > DMI quirks.
>
> Yes. There's nothing preventing the DMI subsystem from being
> initialized under Xen; in fact we rely on it in a dom0 kernel (which
> does have access to the DMI tables). I don't know what the underlying
> bug in the original report is, but there's more to it than failing to
> init DMI.
yeah. It's probably some init ordering problem - some version of Xen
calling into the DMI code too early. It probably doesnt even matter in
practice as we rarely rely on DMI details in Xen guests, right?
Ingo
On 10/23/09 10:04, Ingo Molnar wrote:
>> Yes. There's nothing preventing the DMI subsystem from being
>> initialized under Xen; in fact we rely on it in a dom0 kernel (which
>> does have access to the DMI tables). I don't know what the underlying
>> bug in the original report is, but there's more to it than failing to
>> init DMI.
>>
> yeah. It's probably some init ordering problem - some version of Xen
> calling into the DMI code too early. It probably doesnt even matter in
> practice as we rarely rely on DMI details in Xen guests, right?
>
The backtraces in the original report showed that the messages were
coming from DMI calls in the PCI init path or i8042_init. PCI shouldn't
be being called early, and i8042_init is just a normal module_init.
DomU has no DMI (we just put all zeros into the ISA window to catch any
probes), and nothing Xen-specific has any DMI dependencies. So there
should be no cause to make any premature dmi calls.
It looks like setup_arch() is being missed, but its hard to see how we'd
get very far in that case...
Also, the referenced reports are for distro kernels which may not
contain mainline Xen in the first place (Novell have their own massive
Xen patch stack they dump onto the kernels before shipping, so Xen bug
reports against SuSE kernels don't have any relevance to mainline). I'd
like to see a boot log of a mainline kernel showing the problem.
J