[lm-sensors] Super-IO locking

[lm-sensors] Super-IO locking

[Jim Cromie]

Jean and friends,

SuperIO devices typically hide many functional units behind 2 io-bus 
addresses.
These various units/devices will obviously have separate drivers to 
control them,
leading to the potential that 2 drivers will clash over the 'port'.

As I see it, we need a place to put a lock for the sio port, ideally without
creating a dependency of one driver on another. That said, its seems a 
bit like
overkill to create a 3rd module which merely holds the lock that both 
drivers use,
and therefore depend upon.  IOW, this replaces one dependency for another.

OTOH, an sio-lock manager which provides a lock for any sio port user
(that uses the helper) could be justified.  There are a bunch of SuperIO
units in the hwmon/* world, so this seems like the right place to find
potential module clients.

Any comments ?

[lm-sensors] Super-IO locking

[Jean Delvare]

Hi Jim,

> SuperIO devices typically hide many functional units behind 2 io-bus 
> addresses.  These various units/devices will obviously have separate
> drivers to control them, leading to the potential that 2 drivers
> will clash over the 'port'.

This is true. There have been a proposal from Evgeniy Poliakov in year
2004 for a new superio subsystem which would have addressed the issue.
Back then, it looked awfully complex to me. It seems to have vanished
since, so the problem is still there.

> As I see it, we need a place to put a lock for the sio port, ideally
> without creating a dependency of one driver on another. That said,
> it seems a bit like overkill to create a 3rd module which merely
> holds the lock that both drivers use, and therefore depend upon.
> IOW, this replaces one dependency for another.

Direct dependencies aren't realistic, as you may have more than two
drivers interested in one given Super-I/O device. Also, two devices
from the same family may require a different set of drivers to cover
all the functions, so you may end up with a complex network of
dependencies. The only realistic solution is a centralized one, where
one dedicated piece of code controls the access to the I/O ports, and
other drivers kindly ask for the permission when they need to access it.

> OTOH, an sio-lock manager which provides a lock for any sio port user
> (that uses the helper) could be justified.  There are a bunch of SuperIO
> units in the hwmon/* world, so this seems like the right place to find
> potential module clients.
>
> Any comments ?

So far, we have been limiting the possible problems for hardware
monitoring drivers by only accessing the Super-I/O ports during module
initialization. This has some limitations (like VID pins being read
only once, or some runtime configuration changes being impossible) but
is sufficient in most cases.

Implementing a centralized superio controller is not trivial. There are
decisions to be made. Do we want a complete subsystem, or a simple
helper? Do we want to provide a complete API to the specific LPC
registers (logical device selection, activation register, I/O base
registers, interrupt configuration, etc)? Or do we simply want a
generic I/O ports multiplexer, with a very simple interface? Or both?
Most Super-I/O chips follow the Intel LPC standard, living at 0x2e or
0x4e, but I think some live at strange addresses, and/or do not follow
the regular register mapping. How do we handle these?

The detection part won't be easy either. While answering the question
"is this Super-I/O device present at address X" is pretty simple
(that's what individual drivers do at the moment), pre-detecting
Super-I/O devices promises to be a difficult task. What ports do we
want to probe? Given that probing these ports means writing to them, we
better be careful. Also, each Super-I/O chip may have a different
"access key", so we'd need to try all known keys. Once we're in, the
different chips use different registers for identification. Register
0x20 is the most commonly used, but some chips use other registers in
the 0x21-0x2f range for a complete identification.

Now, if anyone wants to take his/her chance with an experimental
implementation, I'll be happy to review it. I'm fine with a simple
implementation local to hardware monitoring drivers to start with.

Thanks,


-- 
Jean Delvare

[lm-sensors] Super-IO locking

[Jim Cromie]

Jean Delvare wrote:

>Hi Jim,
>
>  
>
>>SuperIO devices typically hide many functional units behind 2 io-bus 
>>addresses.  These various units/devices will obviously have separate
>>drivers to control them, leading to the potential that 2 drivers
>>will clash over the 'port'.
>>    
>>
>
>This is true. There have been a proposal from Evgeniy Poliakov in year
>2004 for a new superio subsystem which would have addressed the issue.
>Back then, it looked awfully complex to me. It seems to have vanished
>since, so the problem is still there.
>
>  
>
>>As I see it, we need a place to put a lock for the sio port, ideally
>>without creating a dependency of one driver on another. That said,
>>it seems a bit like overkill to create a 3rd module which merely
>>holds the lock that both drivers use, and therefore depend upon.
>>IOW, this replaces one dependency for another.
>>    
>>
>
>Direct dependencies aren't realistic, as you may have more than two
>drivers interested in one given Super-I/O device. Also, two devices
>from the same family may require a different set of drivers to cover
>all the functions, so you may end up with a complex network of
>dependencies. The only realistic solution is a centralized one, where
>one dedicated piece of code controls the access to the I/O ports, and
>other drivers kindly ask for the permission when they need to access it.
>
>  
>
>>OTOH, an sio-lock manager which provides a lock for any sio port user
>>(that uses the helper) could be justified.  There are a bunch of SuperIO
>>units in the hwmon/* world, so this seems like the right place to find
>>potential module clients.
>>
>>Any comments ?
>>    
>>
>
>So far, we have been limiting the possible problems for hardware
>monitoring drivers by only accessing the Super-I/O ports during module
>initialization. This has some limitations (like VID pins being read
>only once, or some runtime configuration changes being impossible) but
>is sufficient in most cases.
>
>Implementing a centralized superio controller is not trivial. There are
>decisions to be made. Do we want a complete subsystem, or a simple
>helper? Do we want to provide a complete API to the specific LPC
>registers (logical device selection, activation register, I/O base
>registers, interrupt configuration, etc)? Or do we simply want a
>generic I/O ports multiplexer, with a very simple interface? Or both?
>Most Super-I/O chips follow the Intel LPC standard, living at 0x2e or
>0x4e, but I think some live at strange addresses, and/or do not follow
>the regular register mapping. How do we handle these?
>
>The detection part won't be easy either. While answering the question
>"is this Super-I/O device present at address X" is pretty simple
>(that's what individual drivers do at the moment), pre-detecting
>Super-I/O devices promises to be a difficult task. What ports do we
>want to probe? Given that probing these ports means writing to them, we
>better be careful. Also, each Super-I/O chip may have a different
>"access key", so we'd need to try all known keys. Once we're in, the
>different chips use different registers for identification. Register
>0x20 is the most commonly used, but some chips use other registers in
>the 0x21-0x2f range for a complete identification.
>
>Now, if anyone wants to take his/her chance with an experimental
>implementation, I'll be happy to review it. I'm fine with a simple
>implementation local to hardware monitoring drivers to start with.
>
>Thanks,
>
>
>  
>
Curiously, I have one ;-)  Its *unready* for serious review, but I can 
outline it.
I went for a simple helper approach:

- provides an array of lock-structs (mod-param sets size), which are 
handed out upon request.
- does no sio-lock search, expects drivers to request them specifically.
- but it does simplify the request. (implements the boilerplate)
- reserves locks for 1st requester of a (sio-port,device-id) tuple.
- 2nd requester shares that lock.
- ie its a singleton

- doesnt attempt to stop anti-social behavior (I wouldnt know how)
- drivers are expected to do their own lock/unlock-ing
    driver chooses  efficiency vs long-lock-time (maybe debug-mutexes 
will help here)

- lock-struct is a lock, sio-addr, dev_id, and (TBD) cur_cmd, cur_val
- returning a lock-struct gives some type-based protection against 
inadvertent misuse.
- sio_lock(), sio_unlock()  further distinguish it.
- but user can still reach inside the lock-struct to the lock itself
- currently semaphore based, will be mutex.

Also provides sio_inb() and sio_outb(), which are correlates of s
uperio_inb() and superio_outb().  Currently, these functions attempt to 
avoid
unnecessary outb()s to sioaddr (the command address), but thats likely 
to be a
rarely used code-path, and I plan to rip it out, and maybe revisit it later.


So, the API:

+struct superio_gate {
+       struct semaphore sema;  /* lock is here, for no-offset cast */
+       int sioaddr;            /* this port's cmd-address */
+       int cur_cmd;            /* of cmd-addr, avoid slow io-access */
+       int cur_val;            /* of data-addr, avoid slow io-access */
+};
+
+struct superio_gate* get_sio_gate(int sioaddr, int devid_addr, int 
devid_val);
+void sio_lock(struct superio_gate*);
+void sio_unlock(struct superio_gate*);
+int  sio_inb(struct superio_gate*, int reg);
+void sio_outb(struct superio_gate*, int reg, int val );
+void sio_exit(struct superio_gate*);


users will do:
  struct superio_gate* gate = get_sio_gate( sioaddr, devid_addr, 
wanted_id );

and then either
    sio_lock(gate);
or
    lock_mutex(&gate->sema);

Once users have taken the lock, they can use

    superio_inb(gate->sioaddr);
    superio_inb(sioaddr);               // if they know which was found

    sio_inb(gate);            //
    inb(..)

wrt get_sio_gate(),  Im thinking about  get_oneof_sio_gates(), which would
accept null-terminated arrays for each arg (maybe not devid_addr, since
thats typically fixed across a family of chips - and the drivers that 
can operate them),
and do the looping so clients dont have to.


So, with all that said, you can choose to defer a detailed review, since 
Ill be
cleaning it up a bunch.  I suspect we could chew on the API itself for a 
while 1st.

tia
jimc
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: diff.siolocks-alpha1
Url: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20060121/987a91f4/diff-0001.pl