Re: [RFC][Patch] IBM Real-Time "SMI Free" mode driver -v5

[Vernon Mauery]

On 24-Sep-2010 02:20 PM, Randy Dunlap wrote:
> On Fri, 24 Sep 2010 14:06:42 -0700 Vernon Mauery wrote:
>
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index cff7cc2..5fba368 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -590,4 +590,20 @@ config INTEL_IPS
> >  	  functionality.  If in doubt, say Y here; it will only load on
> >  	  supported platforms.
> >
> > +config IBM_RTL
> > +	tristate "Device driver to enable PRTL support"
> > +	depends on X86 && PCI
> > +	---help---
> > +	  Enable support for IBM Premium Real Time Mode (PRTM).
> > +	  This module will allow you the enter and exit PRTM in the BIOS via
> > +	  sysfs on platforms that support this feature.  System in PRTM will
> > +	  not recive CPU-generated SMIs for recoverable errors.  Use of this
>
> 	      receive (again/still)

Ack. Again.

> > +	  feature without proper support may void your hardware warranty.
> > +
> > +	  If the proper BIOS support is found the driver will load and create
> > +	  /sys/devices/system/ibm_rtl/.  The "state" variable will indicate
> > +	  whether or not the BIOS is in PRTM.
[snip]
> > +#include <linux/delay.h>
> > +#include <linux/module.h>
> > +#include <linux/io.h>
> > +#include <linux/sysdev.h>
> > +#include <linux/dmi.h>
> > +#include <linux/mutex.h>
> > +#include <asm/bios_ebda.h>
> > +
> > +static int force;
>
> s/int/bool/
>
> > +module_param(force, bool, 0);
> > +MODULE_PARM_DESC(force, "Force driver load, ignore DMI data");
> > +
> > +static int debug;
>
> s/int/bool/

Ack.

> > +module_param(debug, bool, 0644);
> > +MODULE_PARM_DESC(debug, "Show debug output");
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Keith Mannthey <kmmanth@xxxxxxxxxx>");
> > +MODULE_AUTHOR("Vernon Mauery <vernux@xxxxxxxxxx>");
> > +
> > +#define RTL_ADDR_TYPE_IO    1
> > +#define RTL_ADDR_TYPE_MMIO  2
> > +
> > +#define RTL_CMD_ENTER_PRTM  1
> > +#define RTL_CMD_EXIT_PRTM   2
> > +
> > +/* The RTL table as presented by the EBDA: */
> > +struct ibm_rtl_table {
> > +	char signature[5];
>
> Question:  are all 5 bytes of signature[] used?  It looks like the search code
> only checks for 32 bits of signature (and the last byte can be garbage?).

Technically, the 5 bytes are supposed to be "_RTL_", but it is a lot 
easier to just check _RTL since we can do that with a single read.  I 
suppose I could do a 64-bit read and mask the bits that matter.  Do you 
have an opinion on the matter?

> > +	u8 version;
> > +	u8 rt_status;
> > +	u8 command;
> > +	u8 command_status;
> > +	u8 cmd_address_type;
> > +	u8 cmd_granularity;
> > +	u8 cmd_offset;
> > +	u16 reserve1;
> > +	u32 cmd_port_address; /* platform dependent address */
> > +	u32 cmd_port_value;   /* platform dependent value */
> > +} __attribute__((packed));
> > +
> > +#define RTL_SIGNATURE (('L'<<24)|('T'<<16)|('R'<<8)|'_')
> > +
> > +#define RTL_DEBUG(A, ...) do { \
> > +	if (debug) \
> > +		pr_info("ibm-rtl: " A, ##__VA_ARGS__ ); \
> > +} while (0)
> > +
> > +static DEFINE_MUTEX(rtl_lock);
> > +static struct ibm_rtl_table __iomem *rtl_table;
> > +static void __iomem *ebda_map;
> > +static void __iomem *rtl_cmd_addr;
> > +static u8 rtl_cmd_type;
> > +static u8 rtl_cmd_width;
>
> ...
>
> > +static int __init ibm_rtl_init(void) {
> > +	unsigned long ebda_addr, ebda_size;
> > +	unsigned int ebda_kb;
> > +	int ret = -ENODEV, i;
> > +
> > +	if (force)
> > +		pr_warning("ibm-rtl: module loaded by force!\n");
>
> Please drop the '!'.

Ack.

--Vernon
--
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/