Re: [Freeipmi-devel] [PATCH 0/5] Parse SPMI tables using sysfs

[Albert Chu]

Hey Dann,

Code looks good.  One thing I noticed.  On one of my systems I have:

# ls /sys/firmware/acpi/tables/ | grep SPMI
SPMI1
SPMI2

So I'm thinking a similar "table instance" (like w/ the /dev/mem) needs
to be supported?  I hacked up something and it seems on one of my
systems there is IPMI info in the SPMI1 path (also verifies the core
code of your patches works!).

But I guess on your systems there is only a
/sys/firmware/acpi/tables/SPMI path?

In my hack I passed table_instance into ipmi_acpi_get_table_sysfs and
adjusted the path accordingly.  Perhaps to support both path types, if
table_instance == 0, try both "SPMI" and "SPMI0"?  Sort of hackish, but
probably ok.

I'm more than happy to apply my fix after yours, but since you have way
more systems with this support, I think it'd be better to come from
your side and me to just make sure it works on the one random node I
have that works with this (I tried a few other nodes and there was no
IPMI info in sysfs SPMI).

While hacking on this, I noticed a corner case in the acpi code.  I
pushed a fix upstream that's independent of your code.  But you should
probably pull it down to make sure things still work.

Al

On Tue, 2018-07-31 at 11:42 -0600, dann frazier wrote:
> I ran into an ARM server that describes its system interface only via
> the SPMI ACPI table and not via SMBIOS. freeipmi has SPMI support,
> but
> its implementation accesses ACPI tables using /dev/mem, which isn't
> safe to do on ARM. In fact, this code is #ifdef'd out for ARM
> platforms.
> 
> This series teaches freeipmi how to parse ACPI tables out of sysfs,
> keeping the fallback /dev/mem implementation as a fallback. It also
> fixes a couple of apparent bugs in the SPMI parsing itself.
> 
> Tested on a HiSilicon D06 ARM Server, and 4 x86 servers:
> HP ProLiant DL165 G7
> HP ProLiant DL385 G7
> QuantaGrid D52B
> Supermicro Super Server
> 
> I've actually not been able to get the existing /dev/mem code to work
> on any platform I've tried myself, so I was unable to regression test
> it.
> 
> dann frazier (5):
>   Don't try to separate the header from the ACPI table data
>   Split RSDT/XSDT parsing into new function
>   Add support for parsing SPMI table exposed via sysfs
>   Allow sysfs SPMI parsing on ARM platforms
>   Correct order of bytes in specification_revision field of ACPI SPMI
>     table
> 
>  libfreeipmi/locate/ipmi-locate-acpi-spmi.c | 240 ++++++++++++++++---
> --
>  1 file changed, 184 insertions(+), 56 deletions(-)
> 
-- 
Albert Chu
address@hidden
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory