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: <9158a0d87f6493977455179202cd86165437f5f6.camel@9elements.com>
Date:   Fri, 20 Dec 2019 08:12:22 +0100
From:   patrick.rudolph@...ements.com
To:     Julius Werner <jwerner@...omium.org>
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>
Subject: Re: [PATCH v3 1/2] firmware: google: Expose CBMEM over sysfs

On Mon, 2019-12-09 at 22:54 -0800, Julius Werner wrote:
> > +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.)

I wasn't aware that CBMEM is used for DMA as there's no such concept in
coreboot yet. For me it looks like the UCSI is regular DRAM mapped as
WB accessed by the ACPI interpreter.
I'll prepare a new patch-set using ioremap in all drivers that access
CBMEM.