linux-kernel - Re: [PATCH v3 1/2] firmware: google: Expose CBMEM over sysfs

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAODwPW8Koy1BvKGJU6PKexYx+PNE+WY7+m69gcxT689vBy+AoQ@mail.gmail.com>
Date:   Mon, 9 Dec 2019 22:54:23 -0800
From:   Julius Werner <jwerner@...omium.org>
To:     Patrick Rudolph <patrick.rudolph@...ements.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Allison Randal <allison@...utok.net>,
        Alexios Zavras <alexios.zavras@...el.com>,
        Stephen Boyd <swboyd@...omium.org>,
        Samuel Holland <samuel@...lland.org>,
        Julius Werner <jwerner@...omium.org>
Subject: Re: [PATCH v3 1/2] firmware: google: Expose CBMEM over sysfs

> +static int cbmem_probe(struct coreboot_device *cdev)
> +{
> +       struct device *dev = &cdev->dev;
> +       struct cb_priv *priv;
> +       int err;
> +
> +       priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       memcpy(&priv->entry, &cdev->cbmem_entry, sizeof(priv->entry));
> +
> +       priv->remap = memremap(priv->entry.address,
> +                              priv->entry.entry_size, MEMREMAP_WB);

We've just been discussing some problems with CBMEM areas and memory
mapping types in Chrome OS. CBMEM is not guaranteed to be page-aligned
(at least not the "small" entries), but the kernel can only assign
memory attributes for a page at a time (and refuses to map the same
area twice with two different memory types, for good reason). So if
CBMEM entries sharing a page are mapped as writeback by one driver but
uncached by the other, things break.

There are some CBMEM entries that need to be mapped uncached (e.g. the
ACPI UCSI table, which isn't even handled by anything using this CBMEM
code) and others for which it would make more sense (e.g. the memory
console, where firmware may add more lines at runtime), but I don't
think there are any regions that really *need* to be writeback. None
of the stuff accessing these areas should access them often enough
that caching matters, and I think it's generally more common to map
firmware memory areas as uncached anyway. So how about we standardize
on mapping it all uncached to avoid any attribute clashes? (That would
mean changing the existing VPD and memconsole drivers to use
ioremap(), too.)