linux-kernel - Re: Re: [PATCH V2 2/4] misc/pvpanic: add MMIO support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a0Y7en4Aa_-ho7fdp17SsG=XnTePhTeqxFmr5PE4LvHeA@mail.gmail.com>
Date:   Fri, 26 Oct 2018 09:11:01 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     peng.hao2@....com.cn
Cc:     andy.shevchenko@...il.com, robh+dt@...nel.org,
        mark.rutland@....com, gregkh@...uxfoundation.org,
        andy@...radead.org, dvhart@...radead.org,
        linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        hutao@...fujitsu.com
Subject: Re: Re: [PATCH V2 2/4] misc/pvpanic: add MMIO support

On 10/26/18, peng.hao2@....com.cn <peng.hao2@....com.cn> wrote:
>>
>> On some architectures (e.g. arm64), it's preferable to use MMIO, since
>> this can be used standalone. Add MMIO support to the pvpanic driver.
>>
>>
>>No global variables, please.

There obviously has to be at least one global variable here, since the
caller of panic() has no context about the device that provides
the MMIO register. But I agree that using ioreadxx() here would
be nice to simplify the code, and it avoids having separate variables
for pio and mmio mode.

>>If your switch to ioreadxx()iowritexx() it would really help you here.
>>
> I can use iowrite8, but here:
>  if (base)
>       iounmap(base);
> I found the nearest API pci_iounmap, but it's a bit strange to see how this
> function is
> used here from the name of the function.
> And I do not confirm that this API is suitable for all architectures.

>From reading the driver, I think you either come from a caller that
always uses PIO or from one that always uses MMIO, and they
are paired with probe/remove, so you can assign the global
__iomem pointer using either ioport_map() or ioremap(), and
undo that using ioport_unmap/iounmap respectively, while the
user just uses iowrite8() unconditionally. This is allowed by the
iowritexx() interfaces.

Alternatively, using devm_ioremap_resource() should work for
either pio or mmio, but it requires a device pointer, which in turn
means you have to slightly rewrite the ACPI portion to use a
platform_driver instead.

      Arnd