[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Freeipmi-devel] Submitting patches
|
From: |
Albert Chu |
|
Subject: |
Re: [Freeipmi-devel] Submitting patches |
|
Date: |
Tue, 29 Sep 2015 14:44:03 -0700 |
Thanks. It'll be in the next release of FreeIPMI.
Al
On Tue, 2015-09-29 at 14:09 -0700, Newell Jensen wrote:
> FYI,
>
> This was tested on arm64 and x86 systems.
>
> On Tue, Sep 29, 2015 at 2:08 PM, Newell Jensen
> <address@hidden> wrote:
> 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 <address@hidden>
> 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
> <address@hidden> 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
> > >> <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
> > >
> > >
> --
> 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