Thanks for the response
>The ipmi_serial_alert() function should be just discarding the
character because either it
>is the alert char, or it is some unexpected character (the reason for
the "concern" comment
>is because the baseboard management controller, BMC, has violated the
specification if it
>is sending anything other than the alert char at this time).
So, what we want to have happen is for ipmi_serial_event() to return
IDLE if the character is not the alert character and ATN if it is the
alert character.
>> Since the ipmi_serial_alert() routine doesn't do anything with the
return
>> value of ipmi_serial_read_data() the alert_char gets effectively
lost.
>
>It is recognized because the return value is set to "SI_SM_ATTN"
In the original version of the code I have, since
ipmi_serial_read_data() doesn't put the character in the enc_read_msg[]
buffer, the alert chracter really wasn't getting recognized since the
check for it was in the enc_read_msg[] buffer:
static int
ipmi_serial_alert(struct si_sm_data *serial) {
int error;
ipmi_serial_read_data(serial,&error);
if(error)
return 0;
if(serial->enc_read_msg[serial->read_pos] !=
serial->codec->alert_char)
return 1;
return 0;
}
So, it does sound like ipmi_serial_alert() should be checking the return
value of ipmi_serial_read_data():
static int
ipmi_serial_alert(struct si_sm_data *serial)
{
unsigned char byte;
int error;
byte = ipmi_serial_read_data(serial,&error);
if(error)
return 0;
if(byte == serial->codec->alert_char)
return 1;
return 0;
}
Thanks for the response it was helpful.
Do you know of any pointers to any documentation that may describe the
state machine and how the code is supposed to move from one state to the
next ?
Thanks
Chris Kottaridis
Senior Engineer
Wind River Systems
719-522-9786
-----Original Message-----
From: dagriego [mailto:[EMAIL PROTECTED]
Sent: Tuesday, July 17, 2007 11:13 PM
To: Kottaridis, Chris
Cc: [email protected]
Subject: Re: [Openipmi-developer] Serial alert missing
Sorry Chris I've been on vacation last week an only got a chance to look
at this today :-)
The ipmi_serial_alert() function should be just discarding the character
because either it is the alert char, or it is some unexpected character
(the reason for the "concern" comment is because the baseboard
management controller, BMC, has violated the specification if it is
sending anything other than the alert char at this time).
See further comments below:
On 7/11/07, Kottaridis, Chris <[EMAIL PROTECTED]> wrote:
>
>
> I am having a problem with the IPMI serial alert mechanism. I am
> running on a 2.6.10 linux kernel.
>
> The report I got goes like this;
>
> >it was discovered that the reason we were not following down the ipmi
> >alert code path was due to the msg type being set incorrectly. It
> >seems that the
> msg type being
> >sent by the IPMC is correct and being received by the IPMI serial
> >interface
> driver is 0x7. Somewhere
> >in the msg passing to the ipmi_serial_alert() function the msg type
> >is set
> to 0x5b which is incorrect.
>
> I am a bit unfamilair with the IPMI protocols and would appreciate any
> help or advice on this issue.
>
> In looking at the ipmi_serial_alert() code it does seem that indeed
> the character is probably getting lost:
>
>
> static int
> ipmi_serial_alert(struct si_sm_data *serial) {
> int error;
> ipmi_serial_read_data(serial,&error);
> if(error)
> return 0;
> if(serial->enc_read_msg[serial->read_pos] !=
> serial->codec->alert_char)
> return 1;
> return 0;
> }
>
>
> The ipmi_serial_read_data() routine does NOT move the character from
> the tty buffer to the enc_read_msg[] buffer, but just returns the next
> character in the tty buffer:
It is suppose to discard the character because it is unexpected. No
need to copy it to the buffer.
>
> static unsigned char
> ipmi_serial_read_data(struct si_sm_data *serial,int *error) {
> unsigned char retval;
> struct tty_struct *tty = serial->port->info->tty;
> unsigned long flags;
> *error = 0;
> retval = tty->read_buf[tty->read_tail];
> spin_lock_irqsave(&tty->read_lock, flags);
> tty->read_tail = (tty->read_tail + 1) & (N_TTY_BUF_SIZE-1);
> tty->read_cnt -= 1;
> spin_unlock_irqrestore(&tty->read_lock, flags);
> return retval;
> }
>
>
> Since the ipmi_serial_alert() routine doesn't do anything with the
return
> value of ipmi_serial_read_data() the alert_char gets effectively lost.
It is recognized because the return value is set to "SI_SM_ATTN"
>
> There is a different routine called ipmi_serial_read_next_byte that
does
> move data from the tty buffer into the enc_read_msg[] buffer. You can
see
> that if there is room in the enc_read_msg[] buffer that it uses
> ipmi_serial_read_data() and saves the return value into the
enc_read_msg[]
> buffer:
>
> } else {
> serial->enc_read_msg[serial->read_pos] =
> ipmi_serial_read_data(serial,&error);
>
>
> It then starts processing that character comparing it against various
> control characters. The alert_char is one of them it looks for:
>
> /* See if this is an alert char */
> if((serial->codec->alert_char) &&
>
> (serial->enc_read_msg[serial->read_pos] ==
>
> serial->codec->alert_char)){
> atomic_set(&serial->alert,1);
> serial->rx_timeout -= time;
> return;
> }
This is needed because we can not process the alert until after the
current message is complete.
>
> Notice that if the character just read is the alert_char it sets the
> serial->alert flag and then returns WITHOUT incrementing the value
> serial->readpos. Since read_pos was not incremented the alert_char is
> effectively not part of the enc_read_msg[] buffer.
>
> So, I think there are two things wrong with ipmi_serial_alert(). First
it
> uses ipmi_serial_read_data() which effectively throws the alert_char
away.
> Secondly, it looks for the alert_char to be in the enc_read_msg[]
buffer. It
> so happens that if ipmi_serial_read_next_byte() is called the
character at
> read_pos in the enc_read_msg[] buffer is the alert_char. But, if the
> character is not the alert_char, or one of the other control
characters,
> then read_pos will be incremented and we'll be looking at some random
value
> located where the next incoming character is to be placed. So, we'll
be
> making the decision as to wether the alert_char was sent off of some
random
> bogus data, if we were to just use ipmi_serial_read_next_byte()
instead of
> ipmi_serial_read_data().
>
> It seems the ipmi_serial_alert() routine should look more like this:
>
> static int
> ipmi_serial_alert(struct si_sm_data *serial)
> {
> int error;
> ipmi_serial_read_next_byte(serial,&error);
> if(error)
> return 0;
> /* check if the alert character was sent */
> if(atomic_read(serial->alert) == 1) {
> /* Reset alert if it was set so we don't think we got
two
> alert characters */
> atomic_set(&serial->alert, 0);
> return 1;
> }
> return 0;
> }
>
>
> This assumes that if the character is not the alert character we want
it to
> go into the enc_read_msg[]. If we should just toss the value in the
tty
> buffer if it's not the alert character then we can do this:
>
> static int
> ipmi_serial_alert(struct si_sm_data *serial)
> {
> unsigned char byte;
> int error;
> byte = ipmi_serial_read_data(serial,&error);
> if(error)
> return 0;
> if(byte == serial->codec->alert_char)
> return 1;
> return 0;
> }
>
>
>
> Now, ipmi_serial_alert() only gets called from ipmi_serial_event() if
the
> state machine is either SERIAL_IDLE or SERIAL_ASYNC_MSG and if
queue_msg
> field isn't set:
>
> switch (serial->state) {
> case SERIAL_IDLE:
> case SERIAL_ASYNC_MSG:
>
> if(atomic_read(&serial->alert)){
> atomic_set(&serial->alert,0);
> return SI_SM_ATTN;
> }
> if
> (ipmi_serial_check_rx_ready(serial,(SERIAL_IDLE?0:time)))
> {
> /* Something in RX FIFO */
> if(serial->codec->queue_msg){
> /* BMC does not support msg queue
> * Must queue on host
> */
> do{
>
> ipmi_serial_read_next_byte(serial,time);
>
> if(serial->end_of_rx){
>
> serial->codec->queue_msg(
>
> serial->codec_data,
>
> serial->enc_read_msg,
>
> serial->read_pos);
>
> ipmi_serial_clear_transaction(serial);
> return
> SI_SM_ATTN;
> }
>
> }while(ipmi_serial_check_rx_ready(serial,0));
> serial->state = SERIAL_ASYNC_MSG;
> return SI_SM_CALL_WITH_DELAY;
> }else{
> /* if there is an attention charactor
we
> * should check for it
> here, otherwise
> * anything at the serial
> port should be of
> * some concern
> */
> if(serial->codec->alert_char){
>
> if(!ipmi_serial_alert(serial))
>
> return SI_SM_IDLE;
> return SI_SM_ATTN;
> }
> return SI_SM_IDLE;
> }
>
> I am not real sure what the serial->codec->queue_msg is getting set to
nor
> the comment about "BMC" there. I also am not real sure about the
comment
> that if the character is not the alert_char there should be "some
concern".
> If this means we should only expect an alert_char then I guess I'd go
for
> the second implementation.
BMC = baseboard management controller
One of the supported BMCs did not follow the specification and could
not queue messages in the BMC from other devices such as the shelf
controller, so we provided this workaround to allow these messages to
be queued on the host and simulate receiving the alert char.
>
> I am not sure wether we should preserve any character that
> ipmi_serial_alert() might find in the tty buffer that isn't the
alert_char
> or not.
These characters are unexpected, so currently they are discarded.
>
> Any advice or suggestions would be appreciated.
>
Hope that clarifies things for you.
-David
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Openipmi-developer mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
