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

[Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>]

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