Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1365400 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [REGRESSION] firmware: dmi_scan: add SBMIOS entry and DMI tables Paul Menzel <paulepanter@users.sourceforge.net> - 2016-03-28 11:30 +0200
    Re: [REGRESSION] firmware: dmi_scan: add SBMIOS entry and DMI tables "ivan.khoronzhuk" <ivan.khoronzhuk@linaro.org> - 2016-03-28 14:50 +0200
      Re: [REGRESSION] firmware: dmi_scan: add SBMIOS entry and DMI tables Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> - 2016-03-28 15:20 +0200

#1365400 — [REGRESSION] firmware: dmi_scan: add SBMIOS entry and DMI tables

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [next] | [standalone]

#1365470

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.
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
>

[toc] | [prev] | [next] | [standalone]

#1365483

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web