Al,
Here is the new patch. A public bug at:
https://bugs.launchpad.net/ubuntu/+source/freeipmi/+bug/1499838
Thanks,
Newell
On Thu, Sep 24, 2015 at 5:25 PM, Albert Chu <ch...@llnl.gov> wrote:
> I have never run on an arm machine (let alone FreeIPMI on an arm
> machine), so I'm sort of going off of your guy's testing +
> judgement :-)
>
> Just let me know what you think would be best.
>
> Al
>
> On Thu, 2015-09-24 at 17:00 -0600, Dann Frazier wrote:
> > Thanks Al!
> >
> > However, looking closer at the code, I wonder if the proposed fix
> > isn't too heavy handed. It looks like there is support for locating
> > the smbios table address from the efi systable in
> > ipmi-locate-dmidecode.c. I don't know of any reason accessing the
> > memory region found here would be harmful (assuming firmware isn't
> > lying!), and I'd hate to break support for systems that support this
> > interface.
> >
> > The real problem is falling back to the legacy address (0xf0000) -
> > indeed, that's what Newell's strace output shows we are doing when the
> > fault is triggered.
> >
> > Newell - can you try just disabling that?
> >
> > On Thu, Sep 24, 2015 at 2:40 PM, Albert Chu <ch...@llnl.gov> wrote:
> > > Thanks, applied.
> > >
> > > Al
> > >
> > > On Thu, 2015-09-24 at 12:28 -0700, Newell Jensen wrote:
> > >> Al,
> > >>
> > >>
> > >> Here is my patch (which I have also attached):
> > >>
> > >>
> > >> Index: ipmi-locate/ipmi-locate.c
> > >> ===================================================================
> > >> --- ipmi-locate/ipmi-locate.c (revision 10210)
> > >> +++ ipmi-locate/ipmi-locate.c (working copy)
> > >> @@ -537,9 +537,13 @@
> > >> exit (EXIT_FAILURE);
> > >> }
> > >>
> > >> +#ifndef __arm__
> > >> +#ifndef __aarch64__
> > >> dmidecode_probe_display (ctx);
> > >> smbios_probe_display (ctx);
> > >> acpi_probe_display (ctx);
> > >> +#endif
> > >> +#endif
> > >> pci_probe_display (ctx);
> > >> if (cmd_args.defaults)
> > >> defaults_display (ctx);
> > >>
> > >>
> > >>
> > >> On Thu, Sep 24, 2015 at 10:08 AM, Dann Frazier
> > >> <dann.fraz...@canonical.com> wrote:
> > >> Hi Al,
> > >> There's no urgency for a formal release. We're past feature
> > >> freeze
> > >> for 15.10, so we couldn't pull in a new upstream release
> > >> anyway.
> > >> However, we can cherry pick it as a bug fix. Though, as Newell
> > >> just
> > >> mentioned to me, we probably want to rework the patch so apply
> > >> to
> > >> ARM32 as well.
> > >>
> > >> On Thu, Sep 24, 2015 at 10:59 AM, Albert Chu <ch...@llnl.gov>
> > >> wrote:
> > >> > Hi Dann,
> > >> >
> > >> > Thanks for the additional info, I wasn't aware of it.
> > >> >
> > >> > In that case, I think we can use the patch.
> > >> >
> > >> > What's the urgency on a new release w/ this patch?
> > >> >
> > >> > Al
> > >> >
> > >> > On Thu, 2015-09-24 at 10:28 -0600, Dann Frazier wrote:
> > >> >> On Thu, Sep 24, 2015 at 9:29 AM, Newell Jensen
> > >> <newell.jen...@gmail.com> wrote:
> > >> >> > Adding in Dann Frazier to thread
> > >> >>
> > >> >> Thanks Newell!
> > >> >>
> > >> >> > On Wed, Sep 23, 2015 at 1:50 PM, Albert Chu
> > >> <ch...@llnl.gov> wrote:
> > >> >> >>
> > >> >> >> Hi Newell,
> > >> >> >>
> > >> >> >> Hmmm, I'm not sure if your patch is the right approach.
> > >> While there may
> > >> >> >> be a problem w/ /dev/mem on your system, it may not be
> > >> something that
> > >> >> >> exists on all systems. Or if it's a bug, it may be
> > >> fixed in the future.
> > >> >> >> So just removing the probes on arm64 may not be the best
> > >> choice.
> > >> >>
> > >> >> While SMBIOS tables will certainly exist on ARM systems,
> > >> they should
> > >> >> be described properly by firmware (e.g. EFI) and exposed by
> > >> the kernel
> > >> >> at /sys/firmware/dmi/tables/. My understanding is that
> > >> poking in
> > >> >> /dev/mem for the table at legacy addresses on ARM will
> > >> always be bad
> > >> >> because IO memory is memory mapped. This has been a problem
> > >> for every
> > >> >> ARM server platform I've seen (that is, every one supported
> > >> by
> > >> >> Ubuntu).
> > >> >>
> > >> >> lshw had this same issue, and we resolved it by dropping
> > >> the /dev/mem
> > >> >> probing for ARM (and other platforms), while still
> > >> retaining
> > >> >> non-legacy SMBIOS access:
> > >> >> http://www.ezix.org/project/ticket/628
> > >> >>
> > >> >> >> Perhaps a better option would be to create a set of
> > >> options in
> > >> >> >> ipmi-locate to limit what probes to do? That way (if
> > >> you're scripting
> > >> >> >> this), you can avoid the probing of known problem areas?
> > >> >> >>
> > >> >> >> It probably wouldn't be hard to add a bunch of the
> > >> options. Do you
> > >> >> >> think that'd suffice?
> > >> >>
> > >> >> That would be sufficient for the specific use case Newell
> > >> and I are
> > >> >> working in the MAAS project - that is, we could pass
> > >> different
> > >> >> parameters depending on the architecture. However, it would
> > >> still
> > >> >> remain an issue for other users of ipmi-locate, who would
> > >> need to know
> > >> >> that a special argument needs to be passed if they are on
> > >> ARM.
> > >> >>
> > >> >> -dann
> > >> > --
> > >> > Albert Chu
> > >> > ch...@llnl.gov
> > >> > Computer Scientist
> > >> > High Performance Systems Division
> > >> > Lawrence Livermore National Laboratory
> > >> >
> > >> >
> > >>
> > >>
> > >>
> > > --
> > > Albert Chu
> > > ch...@llnl.gov
> > > Computer Scientist
> > > High Performance Systems Division
> > > Lawrence Livermore National Laboratory
> > >
> > >
> --
> Albert Chu
> ch...@llnl.gov
> Computer Scientist
> High Performance Systems Division
> Lawrence Livermore National Laboratory
>
>
>
Index: ipmi-locate/ipmi-locate.c
===================================================================
--- ipmi-locate/ipmi-locate.c (revision 10215)
+++ ipmi-locate/ipmi-locate.c (working copy)
@@ -537,13 +537,9 @@
exit (EXIT_FAILURE);
}
-#ifndef __arm__
-#ifndef __aarch64__
dmidecode_probe_display (ctx);
smbios_probe_display (ctx);
acpi_probe_display (ctx);
-#endif
-#endif
pci_probe_display (ctx);
if (cmd_args.defaults)
defaults_display (ctx);
Index: libfreeipmi/locate/ipmi-locate-acpi-spmi.c
===================================================================
--- libfreeipmi/locate/ipmi-locate-acpi-spmi.c (revision 10215)
+++ libfreeipmi/locate/ipmi-locate-acpi-spmi.c (working copy)
@@ -1483,6 +1483,9 @@
uint64_t val;
int rv = -1;
+#if defined(__arm__) || defined(__aarch64__)
+ return (-1);
+#else
if (!ctx || ctx->magic != IPMI_LOCATE_CTX_MAGIC)
{
ERR_TRACE (ipmi_locate_ctx_errormsg (ctx), ipmi_locate_ctx_errnum (ctx));
@@ -1666,4 +1669,5 @@
fiid_obj_destroy (obj_acpi_table_hdr);
fiid_obj_destroy (obj_acpi_spmi_table_descriptor);
return (rv);
+#endif /* defined(__arm__) || defined(__aarch64__) */
}
Index: libfreeipmi/locate/ipmi-locate-dmidecode.c
===================================================================
--- libfreeipmi/locate/ipmi-locate-dmidecode.c (revision 10215)
+++ libfreeipmi/locate/ipmi-locate-dmidecode.c (working copy)
@@ -513,6 +513,7 @@
free (buf);
}
+#if !defined(__arm__) && !defined(__aarch64__)
else
{
if (!(buf = _mem_chunk (ctx, 0xF0000, 0x10000, DEFAULT_MEM_DEV)))
@@ -549,6 +550,7 @@
free (buf);
}
+#endif
if (found)
{
Index: libfreeipmi/locate/ipmi-locate-smbios.c
===================================================================
--- libfreeipmi/locate/ipmi-locate-smbios.c (revision 10215)
+++ libfreeipmi/locate/ipmi-locate-smbios.c (working copy)
@@ -434,6 +434,9 @@
uint64_t strobed;
struct ipmi_locate_info linfo;
+#if defined(__arm__) || defined(__aarch64__)
+ return (-1);
+#else
if (!ctx || ctx->magic != IPMI_LOCATE_CTX_MAGIC)
{
ERR_TRACE (ipmi_locate_ctx_errormsg (ctx), ipmi_locate_ctx_errnum (ctx));
@@ -513,4 +516,5 @@
cleanup:
free (bufp);
return (-1);
+#endif
}
_______________________________________________
Freeipmi-devel mailing list
Freeipmi-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/freeipmi-devel
