Re: [REGRESSION] firmware: dmi_scan: add SBMIOS entry and DMI tables

[Ivan Khoronzhuk]

On 28.03.16 15:44, ivan.khoronzhuk wrote:
> Hi Paul,
>
> The dmi_remap() is arch dependent function and for mainline used as ioremap_cache for x86, arm..
> And only for ia64 as ioremap (where it's same as ioremap_cache). I'm talking about k4.5.
k4.5 -> v4.6-rc1.

> It's rather bug of dmi_remap than the patch which just use it.
>
> The only reason why the bug wasn't found earlier it was unmapped back at init, but it
> doesn't mean it cannot be used after init, which can lead to strange behavior in future.
> If it should be ioremap_cache(), it's better to change dmi_remap() for your arch.
>
> Oh, yes, k4.2 is using the ioremap instead of ioremap_cache().
> But seems it's currently solved with:
> commit ce1143aa60273220a9f89012f2aaaed04f97e9a2
> "x86/dmi: Switch dmi_remap() from ioremap() [uncached] to ioremap_cache()"
> So, probably, it should be back ported.
>
> On 28.03.16 11:44, Paul Menzel wrote:
> > Dear Ivan, dear Jeann,
> >
> >
> > There is an unwanted regression due to commit d7f96f97 (firmware:
> > dmi_scan: add SBMIOS entry and DMI tables).
> >
> > Since Linux kernel 4.2 the utility `cbmem`, used to access information
> > stored in memory, from the coreboot project [1] does not work anymore
> > on a lot of systems as reported in corebootâs issue tracker as ticket
> > #33 [2].
> >
> > ```
> > Failed to mmap /dev/mem: Resource temporarily unavailable
> > ```
> >
> > Aaron Durbin analyzed on the coreboot mailing list [3]:
> >
> > > > 3) Why is that range set as uncached-minus?  Would write-back work?
> > >
> > > Please see this thread:
> > > http://www.coreboot.org/pipermail/coreboot/2015-September/080381.html
> > >
> > > The actual issue stems from
> > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/firmware/dmi_scan.c?id=d7f96f97c4031fa4ffdb7801f9aae23e96170a6f
> > > which maintains a persistent mapping of smbios tables. It uses
> > > dmi_remap() which is '#define dmi_remap ioremap' which is where the
> > > uncached-minus PAT entry comes from. It should be using the same
> > > mechanism as  the ACPI table mappings which uses ioremap_cache().
> >
> > Itâd be great, if the commit could be reverted, or the code be changed
> > in a way that `cbmem` still works.
> >
> > If I should report this issue somewhere else, please tell me too, and
> > Iâll do my best to follow up there.
> >
> >
> > Thanks,
> >
> > Paul
> >
> >
> > [1] https://www.coreboot.org
> > [2] https://ticket.coreboot.org/issues/33
> > [3] https://www.coreboot.org/pipermail/coreboot/2015-October/080568.html

--
Regards,
Ivan Khoronzhuk