Re: [Freeipmi-devel] Submitting patches

[Albert Chu]

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
> <address@hidden> 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 <address@hidden>
>         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
>         <address@hidden> wrote:
>         >> > Adding in Dann Frazier to thread
>         >>
>         >> Thanks Newell!
>         >>
>         >> > On Wed, Sep 23, 2015 at 1:50 PM, Albert Chu
>         <address@hidden> 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
>         > address@hidden
>         > Computer Scientist
>         > High Performance Systems Division
>         > Lawrence Livermore National Laboratory
>         >
>         >
>         
> 
> 
-- 
Albert Chu
address@hidden
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory