Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management BaseDriver (dcdbas) with sysfs support

From: Michael E Brown
Date: Tue Aug 16 2005 - 01:11:09 EST


On Tue, 2005-08-16 at 01:36 -0400, Valdis.Kletnieks@xxxxxx wrote:
> On Mon, 15 Aug 2005 23:58:43 CDT, Michael E Brown said:
>
> > No, this is an _EXCELLENT_ reason why _LESS_ of this should be in the
> > kernel. Why should we have to duplicate a _TON_ of code inside the
> > kernel to figure out which platform we are on, and then look up in a
> > table which method to use for that platform? We have a _MUCH_ nicer
> > programming environment available to us in userspace where we can use
> > things like libsmbios to look up the platform type, then look in an
> > easily-updateable text file which smi type to use. In general, plugging
> > the wrong value here is a no-op.
>
> You'll still need to do some *very* basic checking - there's fairly
> scary-looking 'outb' call in callintf_smi() and host_control_smi() that seems to
> be totally too trusting that The Right Thing is located at address CMOS_BASE_PORT:

Ok, very nice. Finally some actual code review, thanks. :-)

>
> + for (index = PE1300_CMOS_CMD_STRUCT_PTR;
> + index < (PE1300_CMOS_CMD_STRUCT_PTR + 4);
> + index++) {
> + outb(index,
> + (CMOS_BASE_PORT + CMOS_PAGE2_INDEX_PORT_PIIX4));
> + outb(*data++,
> + (CMOS_BASE_PORT + CMOS_PAGE2_DATA_PORT_PIIX4));
> + }
>
> This Dell C840 has an 845, not a PIIX. What just got toasted if this driver
> gets called?

These are all just standard CMOS port numbers that pretty much every
chipset uses to access CMOS.

If you have not got a PE1300, the worst that happens is you have some
random bits scribbled into your CMOS. Nothing that that "cmos clear"
jumper won't fix. :-)

Seriously, this file is meant to be activated by a userspace program
that looks up the system ID in the appropriate table and writes the
correct value into the driver. I'm not sure there is much more to be
said than "don't do that." The official Dell management stack will
always ensure that this is set correctly. If you don't use the official
Dell stack, libsmbios is available, and we would be happy to make the
appropriate tables available there. If you don't use either of these and
go fiddling with this, I'm not sure there is much more to be done.

>
> Can we have a check that the machine is (a) a Dell and (b) has a PIIX and (c) the
> PIIX has a functional SMI behind it, before we start doing outb() calls?
>
>

I'll have to defer to Doug on this. It may be possible to arrange this.
--
Michael


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/