2018-07-30 10:53:14

by Kamlakant Patel

[permalink] [raw]
Subject: [PATCH v3] ipmi: update ssif max_xmit_msg_size limit for multi-part messages

Currently, by setting the xmit_msg_size to 63, the ssif driver never
does a SSIF_MULTI_n_PART, it falls back to only SSIF_MULTI_2_PART.
Due to this all IPMI commands with request size more than 63 bytes
will not work.

As per IPMI spec 2.0 (section 12.15, Table 10), SSIF supports message
length up to 255 bytes. In a multi-part message, the first part must
carry 32 bytes with command 06h. All intermediate ("middle") parts must
carry 32 bytes with command 07h and the number of message data bytes in
the End transaction can range from 1 to 32 bytes with command 08h.

Update ssif max_xmit_msg_size to handle multi-part messages up
to 255 bytes.
Enable command(08h) for End part transaction.

Signed-off-by: Kamlakant Patel <[email protected]>
Suggested-by: Robert Richter <[email protected]>
Reported-by: Karthikeyan M <[email protected]>
---
drivers/char/ipmi/ipmi_ssif.c | 38 +++++++++++++++++++++++++++-----------
1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 18e4650..ada59b7 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -60,6 +60,7 @@
#define SSIF_IPMI_REQUEST 2
#define SSIF_IPMI_MULTI_PART_REQUEST_START 6
#define SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE 7
+#define SSIF_IPMI_MULTI_PART_REQUEST_END 8
#define SSIF_IPMI_RESPONSE 3
#define SSIF_IPMI_MULTI_PART_RESPONSE_MIDDLE 9

@@ -88,6 +89,8 @@
#define SSIF_MSG_JIFFIES ((SSIF_MSG_USEC * 1000) / TICK_NSEC)
#define SSIF_MSG_PART_JIFFIES ((SSIF_MSG_PART_USEC * 1000) / TICK_NSEC)

+#define SSIF_MAX_MSG_LENGTH 255
+
enum ssif_intf_state {
SSIF_NORMAL,
SSIF_GETTING_FLAGS,
@@ -887,28 +890,39 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result,
*/
int left;
unsigned char *data_to_send;
+ int command;

ssif_inc_stat(ssif_info, sent_messages_parts);

left = ssif_info->multi_len - ssif_info->multi_pos;
- if (left > 32)
- left = 32;
+
/* Length byte. */
- ssif_info->multi_data[ssif_info->multi_pos] = left;
+ ssif_info->multi_data[ssif_info->multi_pos] =
+ left > 32 ? 32 : left;
data_to_send = ssif_info->multi_data + ssif_info->multi_pos;
- ssif_info->multi_pos += left;
- if (left < 32)
+ ssif_info->multi_pos += left > 32 ? 32 : left;
+ command = SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE;
+ if (left <= 32) {
/*
* Write is finished. Note that we must end
- * with a write of less than 32 bytes to
- * complete the transaction, even if it is
- * zero bytes.
+ * with a write up to 32 bytes to complete the
+ * transaction, even if it is zero bytes.
+ * The number of message data bytes in the End
+ * transaction can range from 1 to 32 bytes.
+ * As per IPMI spec 2.0(section 12.15,Table 12-10),
+ * the BMC Multi-part Write commands are:
+ * Start-first part - 06h
+ * Middle-part(s) - 07h
+ * End-part - 08h
+ * Update command for END part transaction.
*/
ssif_info->multi_data = NULL;
+ command = SSIF_IPMI_MULTI_PART_REQUEST_END;
+ }

rv = ssif_i2c_send(ssif_info, msg_written_handler,
I2C_SMBUS_WRITE,
- SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
+ command,
data_to_send,
I2C_SMBUS_BLOCK_DATA);
if (rv < 0) {
@@ -1499,9 +1513,11 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
* start and the next message is always going
* to be 1-31 bytes in length. Not ideal, but
* it should work.
+ * As per IPMI spec 2.0, SSIF interface supports
+ * message size up to 255 bytes.
*/
- if (ssif_info->max_xmit_msg_size > 63)
- ssif_info->max_xmit_msg_size = 63;
+ if (ssif_info->max_xmit_msg_size > SSIF_MAX_MSG_LENGTH)
+ ssif_info->max_xmit_msg_size = SSIF_MAX_MSG_LENGTH;
break;

default:
--
2.7.4



2018-07-30 13:00:05

by Corey Minyard

[permalink] [raw]
Subject: Re: [PATCH v3] ipmi: update ssif max_xmit_msg_size limit for multi-part messages

On 07/30/2018 05:51 AM, Kamlakant Patel wrote:
> Currently, by setting the xmit_msg_size to 63, the ssif driver never
> does a SSIF_MULTI_n_PART, it falls back to only SSIF_MULTI_2_PART.
> Due to this all IPMI commands with request size more than 63 bytes
> will not work.
>
> As per IPMI spec 2.0 (section 12.15, Table 10), SSIF supports message
> length up to 255 bytes. In a multi-part message, the first part must
> carry 32 bytes with command 06h. All intermediate ("middle") parts must
> carry 32 bytes with command 07h and the number of message data bytes in
> the End transaction can range from 1 to 32 bytes with command 08h.
>
> Update ssif max_xmit_msg_size to handle multi-part messages up
> to 255 bytes.
> Enable command(08h) for End part transaction.

You don't think it's a good idea to test what the BMC is capable of
doing, per
the patch I sent you earlier?

-corey

> Signed-off-by: Kamlakant Patel <[email protected]>
> Suggested-by: Robert Richter <[email protected]>
> Reported-by: Karthikeyan M <[email protected]>
> ---
> drivers/char/ipmi/ipmi_ssif.c | 38 +++++++++++++++++++++++++++-----------
> 1 file changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
> index 18e4650..ada59b7 100644
> --- a/drivers/char/ipmi/ipmi_ssif.c
> +++ b/drivers/char/ipmi/ipmi_ssif.c
> @@ -60,6 +60,7 @@
> #define SSIF_IPMI_REQUEST 2
> #define SSIF_IPMI_MULTI_PART_REQUEST_START 6
> #define SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE 7
> +#define SSIF_IPMI_MULTI_PART_REQUEST_END 8
> #define SSIF_IPMI_RESPONSE 3
> #define SSIF_IPMI_MULTI_PART_RESPONSE_MIDDLE 9
>
> @@ -88,6 +89,8 @@
> #define SSIF_MSG_JIFFIES ((SSIF_MSG_USEC * 1000) / TICK_NSEC)
> #define SSIF_MSG_PART_JIFFIES ((SSIF_MSG_PART_USEC * 1000) / TICK_NSEC)
>
> +#define SSIF_MAX_MSG_LENGTH 255
> +
> enum ssif_intf_state {
> SSIF_NORMAL,
> SSIF_GETTING_FLAGS,
> @@ -887,28 +890,39 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result,
> */
> int left;
> unsigned char *data_to_send;
> + int command;
>
> ssif_inc_stat(ssif_info, sent_messages_parts);
>
> left = ssif_info->multi_len - ssif_info->multi_pos;
> - if (left > 32)
> - left = 32;
> +
> /* Length byte. */
> - ssif_info->multi_data[ssif_info->multi_pos] = left;
> + ssif_info->multi_data[ssif_info->multi_pos] =
> + left > 32 ? 32 : left;
> data_to_send = ssif_info->multi_data + ssif_info->multi_pos;
> - ssif_info->multi_pos += left;
> - if (left < 32)
> + ssif_info->multi_pos += left > 32 ? 32 : left;
> + command = SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE;
> + if (left <= 32) {
> /*
> * Write is finished. Note that we must end
> - * with a write of less than 32 bytes to
> - * complete the transaction, even if it is
> - * zero bytes.
> + * with a write up to 32 bytes to complete the
> + * transaction, even if it is zero bytes.
> + * The number of message data bytes in the End
> + * transaction can range from 1 to 32 bytes.
> + * As per IPMI spec 2.0(section 12.15,Table 12-10),
> + * the BMC Multi-part Write commands are:
> + * Start-first part - 06h
> + * Middle-part(s) - 07h
> + * End-part - 08h
> + * Update command for END part transaction.
> */
> ssif_info->multi_data = NULL;
> + command = SSIF_IPMI_MULTI_PART_REQUEST_END;
> + }
>
> rv = ssif_i2c_send(ssif_info, msg_written_handler,
> I2C_SMBUS_WRITE,
> - SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
> + command,
> data_to_send,
> I2C_SMBUS_BLOCK_DATA);
> if (rv < 0) {
> @@ -1499,9 +1513,11 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
> * start and the next message is always going
> * to be 1-31 bytes in length. Not ideal, but
> * it should work.
> + * As per IPMI spec 2.0, SSIF interface supports
> + * message size up to 255 bytes.
> */
> - if (ssif_info->max_xmit_msg_size > 63)
> - ssif_info->max_xmit_msg_size = 63;
> + if (ssif_info->max_xmit_msg_size > SSIF_MAX_MSG_LENGTH)
> + ssif_info->max_xmit_msg_size = SSIF_MAX_MSG_LENGTH;
> break;
>
> default:



2018-07-31 07:38:33

by Kamlakant Patel

[permalink] [raw]
Subject: Re: [PATCH v3] ipmi: update ssif max_xmit_msg_size limit for multi-part messages

On Mon, Jul 30, 2018 at 07:56:02AM -0500, Corey Minyard wrote:
Hi Corey,
>
> On 07/30/2018 05:51 AM, Kamlakant Patel wrote:
> >Currently, by setting the xmit_msg_size to 63, the ssif driver never
> >does a SSIF_MULTI_n_PART, it falls back to only SSIF_MULTI_2_PART.
> >Due to this all IPMI commands with request size more than 63 bytes
> >will not work.
> >
> >As per IPMI spec 2.0 (section 12.15, Table 10), SSIF supports message
> >length up to 255 bytes. In a multi-part message, the first part must
> >carry 32 bytes with command 06h. All intermediate ("middle") parts must
> >carry 32 bytes with command 07h and the number of message data bytes in
> >the End transaction can range from 1 to 32 bytes with command 08h.
> >
> >Update ssif max_xmit_msg_size to handle multi-part messages up
> >to 255 bytes.
> >Enable command(08h) for End part transaction.
>
> You don't think it's a good idea to test what the BMC is capable of
> doing, per
> the patch I sent you earlier?
I have not received the patch in unicast to me. If you have sent it to
mailing list I will search and test it. If not, could you please resend
it to me, I will test it and let you know.

Thanks,
Kamlakant Patel
>
> -corey
>
> >Signed-off-by: Kamlakant Patel <[email protected]>
> >Suggested-by: Robert Richter <[email protected]>
> >Reported-by: Karthikeyan M <[email protected]>
> >---
> > drivers/char/ipmi/ipmi_ssif.c | 38 +++++++++++++++++++++++++++-----------
> > 1 file changed, 27 insertions(+), 11 deletions(-)
> >
> >diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
> >index 18e4650..ada59b7 100644
> >--- a/drivers/char/ipmi/ipmi_ssif.c
> >+++ b/drivers/char/ipmi/ipmi_ssif.c
> >@@ -60,6 +60,7 @@
> > #define SSIF_IPMI_REQUEST 2
> > #define SSIF_IPMI_MULTI_PART_REQUEST_START 6
> > #define SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE 7
> >+#define SSIF_IPMI_MULTI_PART_REQUEST_END 8
> > #define SSIF_IPMI_RESPONSE 3
> > #define SSIF_IPMI_MULTI_PART_RESPONSE_MIDDLE 9
> >
> >@@ -88,6 +89,8 @@
> > #define SSIF_MSG_JIFFIES ((SSIF_MSG_USEC * 1000) / TICK_NSEC)
> > #define SSIF_MSG_PART_JIFFIES ((SSIF_MSG_PART_USEC * 1000) / TICK_NSEC)
> >
> >+#define SSIF_MAX_MSG_LENGTH 255
> >+
> > enum ssif_intf_state {
> > SSIF_NORMAL,
> > SSIF_GETTING_FLAGS,
> >@@ -887,28 +890,39 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result,
> > */
> > int left;
> > unsigned char *data_to_send;
> >+ int command;
> >
> > ssif_inc_stat(ssif_info, sent_messages_parts);
> >
> > left = ssif_info->multi_len - ssif_info->multi_pos;
> >- if (left > 32)
> >- left = 32;
> >+
> > /* Length byte. */
> >- ssif_info->multi_data[ssif_info->multi_pos] = left;
> >+ ssif_info->multi_data[ssif_info->multi_pos] =
> >+ left > 32 ? 32 : left;
> > data_to_send = ssif_info->multi_data + ssif_info->multi_pos;
> >- ssif_info->multi_pos += left;
> >- if (left < 32)
> >+ ssif_info->multi_pos += left > 32 ? 32 : left;
> >+ command = SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE;
> >+ if (left <= 32) {
> > /*
> > * Write is finished. Note that we must end
> >- * with a write of less than 32 bytes to
> >- * complete the transaction, even if it is
> >- * zero bytes.
> >+ * with a write up to 32 bytes to complete the
> >+ * transaction, even if it is zero bytes.
> >+ * The number of message data bytes in the End
> >+ * transaction can range from 1 to 32 bytes.
> >+ * As per IPMI spec 2.0(section 12.15,Table 12-10),
> >+ * the BMC Multi-part Write commands are:
> >+ * Start-first part - 06h
> >+ * Middle-part(s) - 07h
> >+ * End-part - 08h
> >+ * Update command for END part transaction.
> > */
> > ssif_info->multi_data = NULL;
> >+ command = SSIF_IPMI_MULTI_PART_REQUEST_END;
> >+ }
> >
> > rv = ssif_i2c_send(ssif_info, msg_written_handler,
> > I2C_SMBUS_WRITE,
> >- SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
> >+ command,
> > data_to_send,
> > I2C_SMBUS_BLOCK_DATA);
> > if (rv < 0) {
> >@@ -1499,9 +1513,11 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
> > * start and the next message is always going
> > * to be 1-31 bytes in length. Not ideal, but
> > * it should work.
> >+ * As per IPMI spec 2.0, SSIF interface supports
> >+ * message size up to 255 bytes.
> > */
> >- if (ssif_info->max_xmit_msg_size > 63)
> >- ssif_info->max_xmit_msg_size = 63;
> >+ if (ssif_info->max_xmit_msg_size > SSIF_MAX_MSG_LENGTH)
> >+ ssif_info->max_xmit_msg_size = SSIF_MAX_MSG_LENGTH;
> > break;
> >
> > default:
>
>