from:"Corey Minyard"

Re: [PATCH v2 0/4] ipmi: bt-i2c: added IPMI Block Transfer over I2C

2017-08-09 Thread Corey Minyard


On 08/09/2017 08:04 PM, Brendan Higgins wrote:

Perhaps that is some level of abuse, but it's pretty common.  I'm not
against it.

There is standard IPMI firmware NetFN (though no commands defined) that if
you use
the driver automatically goes into "Maintenance mode" and modified the
timeouts
and handling to some extent to help with this.

That is a really good point, I missed that.
...


There are ways to accomplish this that aren't that complex.  You can create
an OEM
command that can query the maximum message size and the ability to do
sequence
numbers in the messages.

If messages larger than 32-bytes are supported, and the host I2C/SMBus
driver
supports it, you could use the standard SSIF SMBus commands to do this, they
have an 8-bit length field.

If sequence numbers are supported, The SSIF could use different SMBus
commands
to do the write and read requests.  Since this is only if you get an OEM
command,
and if you put the sequence numbers at the end where they are easy to add on
the send side, this is a small change to the driver.

What if we just had an OEM command that changed the message structure from
that point on? We could abuse the "maintenance mode" NetFN to get back into
normal SSIF if necessary.


Actually, I wouldn't have a separate "openbmc mode".  I would have 
OpenBMC always

work with standard SSIF, and have separate SMBus commands for messages with
the sequence number and messages larger than 32 bytes.

I've attached a patch with what I would expect the changes to be to the 
host driver.
It doesn't handle multiple outstanding messages, but it shows what 
detection and a

separate SMBus command would look like.


So I think the changes would be small and contained.  I'm actually ok with a
different driver, but I think it would be more valuable to the OpenBMC
project
to have a standardized interface that would work (in a not quite as
efficient
mode) with software that does not use the Linux IPMI driver.

I guess I see the all of my asks as hacky things which we can hopefully remove
at some point. Hopefully, most OpenBMC users won't want or need these things.
...

Regardless of what we do with the "BT-I2C" stuff, I am still interested in
what
you think about this.


I think you are right, it probably belongs some place else.  The way that
makes the most
sense to me would be to have an "ipmi" directory with a "host" and "slave"
side, and since
ipmi is not really a char driver, to move it to the main driver directory.
That might be
fairly disruptive, though.

That was my thinking exactly.


The other option that makes sense to me would be to add a
drivers/char/ipmi_slave directory,
or something like that, and put the slave code there.  That would be less
disruptive.

Right that is the approach I took, except I called it drivers/char/ipmi_bmc.

I originally thought doing the less disruptive thing is best; however, I know
there are also some OpenBMC people who are interested in implementing
IPMB. So maybe now is the time to bite the bullet and create an ipmi
directory under drivers/.


I'm not sure IPMB would make much difference, there's no host side 
change as it's
already supported.  I don't think there would be any significant code 
sharing

between the two.

If there end up being a significant amount of common code, then it would
definitely be worth the effort to move it.


-corey

In summary, I think I can live with making it a mangled form of SSIF, but
I would prefer to put it in its own driver.


You can look at the patch and consider it, and consider that you would 
need to

implement flag and event handling.  On an x86 host there would be SMBIOS
and ACPI stuff to deal with somehow for discovery.  There's probably few 
other

things to deal with.


In any case, I think I would rather focus on the the BMC side IPMI framework
now, since it is a bigger change and would also reduce the work of
implementing a BMC side SSIF driver.

Here is what I propose: we focus on the BMC side IPMI framework RFC that
I sent out the other day:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1463473.html
I will add a change to the BMC side IPMI framework patchset to move all the
IPMI stuff to the new drivers/ipmi directory as discussed and then drop the
patch in that patchset that depends on this patchset.

Let me know what you think


Let's hold off on the new directory, there's probably some convincing of 
the "powers

that be" for that.

I'll look at the patch set tomorrow, unless something critical comes up.

Thanks,

-corey

diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 11237e8..d467b1a 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -60,6 +60,11 @@
 
 #define IPMI_GET_SYSTEM_INTERFACE_CAPABILITIES_CMD	0x57
 
+static u8 openbmc_iana[3] = { 0x10, 0x20, 0x30 };
+#define IPMI_OPENBMC_CAPABILITY_REQUEST_CMD	0x01
+#define SSIF_OPENBMC_REQUEST	0x80
+#define SSIF_OPENBMC_RESPONSE	0x81
+
 #define	SSIF_IPMI_REQUEST			2

Re: [PATCH v2 0/4] ipmi: bt-i2c: added IPMI Block Transfer over I2C

2017-08-08 Thread Corey Minyard


On 08/07/2017 08:25 PM, Brendan Higgins wrote:

On Sat, Aug 5, 2017 at 3:23 PM, Corey Minyard <miny...@acm.org> wrote:

On 08/04/2017 08:18 PM, Brendan Higgins wrote:

This patchset introduces IPMI Block Transfer over I2C (BT-I2C), which has
the
same semantics as IPMI Block Transfer except it done over I2C.

For the OpenBMC people, this is based on an RFC:
https://lists.ozlabs.org/pipermail/openbmc/2016-September/004505.html

The documentation discusses the reason for this in greater detail, suffice
it to
say SSIF cannot be correctly implemented on some naive I2C devices. There
are
some additional reasons why we don't like SSIF, but those are again
covered in
the documentation for all those who are interested.


What you have is not really BT over I2C.  You have just added a sequence
number to the
IPMI messages and dropped the SMBus command.  Other interfaces have sequence
numbers,
too.  Calling it BT is a little over the top.

Fair point, maybe ISIF (I2C System Interface)? I don't have strong feelings
about the name.


I'm not too picky, either, I just didn't want someone to get confused.




Do you really need the performance required by having multiple outstanding
messages?
That adds a lot of complexity, if it's unnecessary it's just a waste.  The
IPMI work on top
of interfaces does not really require that much performance, it's just
reading sensors,
FRU data, and such.  Perhaps you have a reason, but I fail to see
why it's really that big a deal.  The BT interface has this ability, but the
driver does not
take advantage of it and nobody has complained.


Yes, we do have some platforms which only have IPMI as a standard interface
and we are abusing some OEM commands to do some things that we probably
should not do with IPMI like doing firmware updates.


Perhaps that is some level of abuse, but it's pretty common.  I'm not 
against it.


There is standard IPMI firmware NetFN (though no commands defined) that 
if you use
the driver automatically goes into "Maintenance mode" and modified the 
timeouts

and handling to some extent to help with this.


  Also, we have some
commands which take a really long time to complete (> 1s). Admittedly, this is
abusing IPMI to solve problems which should probably be solved elsewhere;
nevertheless, it is a feature we are actually using. And having an option to use
sequence numbers if definitely nice from our perspective.

We will probably want to improve the block transfer driver at some
point as well.


As long as you have a legitimate need, I don't have a problem with 
this.  The upper layer
of the driver can already handle this, there's just a small piece that 
interfaces to
the lower layer that needs to be modified.  And the ability to query the 
maximum

number of outstanding messages from the lower layer.

BT will be harder, the SI interface code would need more significant 
updates.



snip




I would agree that the multi-part messages in SSIF is a big pain and and a
lot of
unnecessary complexity.  I believe it is there to accommodate host hardware
that is
limited.  But SMBus can have 255 byte messages and there's no arbitrary
limit on
I2C.  It is the way of IPMI to support the least common denominator.

Your interface will only work on Linux.  Other OSes (unless they choose to
implement this
driver) will be unable to use your BMC.  Of course there's the NACK issue,
but that's a
big difference, and it would probably still work with existing drivers on
other OSes.

I hope I did not send the message that we are planning on using this instead
of SSIF forever and always. This is intended as an alternative to SSIF for some
systems for which I2C is really the only available hardware interface, but SSIF
is not well suited for said reasons.

It would be great for OpenBMC if someone added support for SSIF, but as far
as I know, no one is asking for it.


I am not convinced that SSIF is not suitable for this.  It is true that 
the lack of NACK
support won't work right now, but for the current SSIF driver that is 
adding a condition
to a single if statement.  I would be happy to have that.  The current 
code returns an

error in this case, but it might be better to just ignore it, anyway.


Plus people may choose to use OpenBMC on things besides the aspeed devices
that
could do a NACK.  That's kind of the point of OpenBMC, isn't it?

Yes, using OpenBMC on other things is part of the point; however, all
of the projects
that I am currently aware of and am trying to support use Aspeed parts
which also
need an option.


My suggestion would be to implement a BMC that supports standard SSIF single
part
messages by default.  That way any existing driver will work with it.  (I
don't know
about the NACK thing, but that's a much smaller issue than a whole new
protocol.)

Coming up with a satisfactory solution to working around systems that cannot
NACK is my primary goal; I don't care about alternatives that do not address
this.


Then use OEM extensions t

Re: [PATCH v2 0/4] ipmi: bt-i2c: added IPMI Block Transfer over I2C

2017-08-05 Thread Corey Minyard


On 08/04/2017 08:18 PM, Brendan Higgins wrote:

This patchset introduces IPMI Block Transfer over I2C (BT-I2C), which has the
same semantics as IPMI Block Transfer except it done over I2C.

For the OpenBMC people, this is based on an RFC:
https://lists.ozlabs.org/pipermail/openbmc/2016-September/004505.html

The documentation discusses the reason for this in greater detail, suffice it to
say SSIF cannot be correctly implemented on some naive I2C devices. There are
some additional reasons why we don't like SSIF, but those are again covered in
the documentation for all those who are interested.


I'm not terribly excited about this.  A few notes:

SMBus alerts are fairly broken in Linux right now.  I have a patch to 
fix this at:

https://github.com/cminyard/linux-ipmi/commit/48136176ce1890f99857c73e0ace5bd8dfb61fbf
I haven't been able to get much traction getting anyone to care.

The lack of a NACK could be worked around fairly easily in the current 
driver.  It looks like you
are just returning a message too short to be valid.  That's easy.  I 
think it's a rather major
deficiency in the hardware to not be able to NACK something, but that is 
what it is.


What you have is not really BT over I2C.  You have just added a sequence 
number to the
IPMI messages and dropped the SMBus command.  Other interfaces have 
sequence numbers,

too.  Calling it BT is a little over the top.

Do you really need the performance required by having multiple 
outstanding messages?
That adds a lot of complexity, if it's unnecessary it's just a waste.  
The IPMI work on top
of interfaces does not really require that much performance, it's just 
reading sensors,

FRU data, and such.  Perhaps you have a reason, but I fail to see
why it's really that big a deal.  The BT interface has this ability, but 
the driver does not

take advantage of it and nobody has complained.

And I don't understand the part about OpenBMC making use of sequence 
numbers.
Why does that matter for this interface?  It's the host side that would 
care about
that, the host would stick the numbers in and the slave would return 
it.  If you are
using sequence numbers in OpenBMC, which sounds quite reasonable, I 
would think
it would be a bad idea to to trust that the host would give you good 
sequence

numbers.

Plus, with multiple outstanding messages, you really need to limit it.  
A particular BMC
may not be able to handle it the full 256, and the ability to have that 
many messages

outstanding is probably not a good thing.

If you really need multiple outstanding messages, the host side IPMI 
message handler
needs to change to allow that.  It's doable, and I know how, I just 
haven't seen the

need.

I would agree that the multi-part messages in SSIF is a big pain and and 
a lot of
unnecessary complexity.  I believe it is there to accommodate host 
hardware that is
limited.  But SMBus can have 255 byte messages and there's no arbitrary 
limit on

I2C.  It is the way of IPMI to support the least common denominator.

Your interface will only work on Linux.  Other OSes (unless they choose 
to implement this
driver) will be unable to use your BMC.  Of course there's the NACK 
issue, but that's a
big difference, and it would probably still work with existing drivers 
on other OSes.
Plus people may choose to use OpenBMC on things besides the aspeed 
devices that

could do a NACK.  That's kind of the point of OpenBMC, isn't it?

My suggestion would be to implement a BMC that supports standard SSIF 
single part
messages by default.  That way any existing driver will work with it.  
(I don't know
about the NACK thing, but that's a much smaller issue than a whole new 
protocol.)
Then use OEM extensions to query things like if it can do messages 
larger than 32

bytes or supports sequence numbers, and how many outstanding messages it
can have, and extend the current SSIF driver.  It wouldn't be very hard.

In my experience, sticking with standards is a good thing, and designing 
your own
protocols is fraught with peril.  I'm not trying to be difficult here, 
but I am against

the proliferation of things that do not need to proliferate :).

-corey



In addition, since I am adding both host side and BMC side support, I figured
that now is a good time to resolve the problem of where to put BMC side IPMI
drivers; right now we have it (there is only one) in drivers/char/ipmi/ with the
rest of the host side IPMI drivers, but I think it makes sense to put all of the
host side IPMI drivers in one directory and all of the BMC side drivers in
another, preferably in a way that does not effect all of the current OpenIPMI
users. I have not created a MAINTAINERS entry for the new directory yet, as I
figured there might be some discussion to be had about it.

I have tested this patchset on the Aspeed 2500 EVB.

Changes since previous update:
   - Cleaned up some documentation.
   - Added patch which moves the Aspeed BT-BMC driver to the new ipmi_bmc
 directory.



--
To

Re: [PATCH v2 01/29] IPMI.txt: standardize document format

2017-06-17 Thread Corey Minyard


On 06/17/2017 10:26 AM, Mauro Carvalho Chehab wrote:

Each text file under Documentation follows a different
format. Some doesn't even have titles!

Change its representation to follow the adopted standard,
using ReST markups for it to be parseable by Sphinx:

- fix document type;
- add missing markups for subitems;
- mark literal blocks;
- add whitespaces and blank lines where needed;
- use bulleted list markups where neded.

Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
---
  Documentation/IPMI.txt | 76 +-
  1 file changed, 44 insertions(+), 32 deletions(-)


This is ok by me.  Nice to have a standard format for this.

Reviewed-by: Corey Minyard <cminy...@mvista.com>


diff --git a/Documentation/IPMI.txt b/Documentation/IPMI.txt
index 6962cab997ef..aa77a25a0940 100644
--- a/Documentation/IPMI.txt
+++ b/Documentation/IPMI.txt
@@ -1,9 +1,8 @@
+=
+The Linux IPMI Driver
+=
  
-  The Linux IPMI Driver

- -
-     Corey Minyard
- <miny...@mvista.com>
-   <miny...@acm.org>
+:Author: Corey Minyard <miny...@mvista.com> / <miny...@acm.org>
  
  The Intelligent Platform Management Interface, or IPMI, is a

  standard for controlling intelligent devices that monitor a system.
@@ -141,7 +140,7 @@ Addressing
  --
  
  The IPMI addressing works much like IP addresses, you have an overlay

-to handle the different address types.  The overlay is:
+to handle the different address types.  The overlay is::
  
struct ipmi_addr

{
@@ -153,7 +152,7 @@ to handle the different address types.  The overlay is:
  The addr_type determines what the address really is.  The driver
  currently understands two different types of addresses.
  
-"System Interface" addresses are defined as:

+"System Interface" addresses are defined as::
  
struct ipmi_system_interface_addr

{
@@ -166,7 +165,7 @@ straight to the BMC on the current card.  The channel must 
be
  IPMI_BMC_CHANNEL.
  
  Messages that are destined to go out on the IPMB bus use the

-IPMI_IPMB_ADDR_TYPE address type.  The format is
+IPMI_IPMB_ADDR_TYPE address type.  The format is::
  
struct ipmi_ipmb_addr

{
@@ -184,16 +183,16 @@ spec.
  Messages
  
  
-Messages are defined as:

+Messages are defined as::
  
-struct ipmi_msg

-{
+  struct ipmi_msg
+  {
unsigned char netfn;
unsigned char lun;
unsigned char cmd;
unsigned char *data;
int   data_len;
-};
+  };
  
  The driver takes care of adding/stripping the header information.  The

  data portion is just the data to be send (do NOT put addressing info
@@ -208,7 +207,7 @@ block of data, even when receiving messages.  Otherwise the 
driver
  will have no place to put the message.
  
  Messages coming up from the message handler in kernelland will come in

-as:
+as::
  
struct ipmi_recv_msg

{
@@ -246,6 +245,7 @@ and the user should not have to care what type of SMI is 
below them.
  
  
  Watching For Interfaces

+^^^
  
  When your code comes up, the IPMI driver may or may not have detected

  if IPMI devices exist.  So you might have to defer your setup until
@@ -256,6 +256,7 @@ and tell you when they come and go.
  
  
  Creating the User

+^
  
  To use the message handler, you must first create a user using

  ipmi_create_user.  The interface number specifies which SMI you want
@@ -272,6 +273,7 @@ closing the device automatically destroys the user.
  
  
  Messaging

+^
  
  To send a message from kernel-land, the ipmi_request_settime() call does

  pretty much all message handling.  Most of the parameter are
@@ -321,6 +323,7 @@ though, since it is tricky to manage your own buffers.
  
  
  Events and Incoming Commands

+
  
  The driver takes care of polling for IPMI events and receiving

  commands (commands are messages that are not responses, they are
@@ -367,7 +370,7 @@ in the system.  It discovers interfaces through a host of 
different
  methods, depending on the system.
  
  You can specify up to four interfaces on the module load line and

-control some module parameters:
+control some module parameters::
  
modprobe ipmi_si.o type=,

 ports=,... addrs=,...
@@ -437,7 +440,7 @@ default is one.  Setting to 0 is useful with the hotmod, 
but is
  obviously only useful for modules.
  
  When compiled into the kernel, the parameters can be specified on the

-kernel command line as:
+kernel command line as::
  
ipmi_si.type=,...

 ipmi_si.ports=,... ipmi_si.addrs=,...
@@ -474,16 +477,22 @@ The driver supports a hot add and remove of interfaces.  
This way,
  interfaces can be added or removed after the kernel is up and running.
  T

Re: [PATCH] Documentation: Fix a typo in IPMI.txt.

2016-12-19 Thread Corey Minyard


On 12/18/2016 01:11 PM, Rami Rosen wrote:

This patch fixes a trivial type in IPMI.txt.


Thanks, queued for next release.

-corey


Signed-off-by: Rami Rosen 
---
  Documentation/IPMI.txt | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/IPMI.txt b/Documentation/IPMI.txt
index 7229230..6962cab 100644
--- a/Documentation/IPMI.txt
+++ b/Documentation/IPMI.txt
@@ -257,7 +257,7 @@ and tell you when they come and go.
  
  Creating the User
  
-To user the message handler, you must first create a user using

+To use the message handler, you must first create a user using
  ipmi_create_user.  The interface number specifies which SMI you want
  to connect to, and you must supply callback functions to be called
  when data comes in.  The callback function can run at interrupt level,



--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/4] ipmi: bt-i2c: added IPMI Block Transfer over I2C

Re: [PATCH v2 0/4] ipmi: bt-i2c: added IPMI Block Transfer over I2C

Re: [PATCH v2 0/4] ipmi: bt-i2c: added IPMI Block Transfer over I2C

Re: [PATCH v2 01/29] IPMI.txt: standardize document format

Re: [PATCH] Documentation: Fix a typo in IPMI.txt.

5 matches

Site Navigation

Mail list logo

Footer information