linux-kernel - Re: [PATCH v2 07/10] nvme-pci: Use PCI p2pmem subsystem to manage the CMB

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: <416d8d27-200f-befb-1c30-14544fffcba0@deltatee.com>
Date:   Mon, 5 Mar 2018 18:14:02 -0700
From:   Logan Gunthorpe <logang@...tatee.com>
To:     Oliver <oohall@...il.com>
Cc:     Keith Busch <keith.busch@...el.com>, linux-kernel@...r.kernel.org,
        linux-pci@...r.kernel.org, linux-nvme@...ts.infradead.org,
        linux-rdma@...r.kernel.org,
        "linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
        linux-block@...r.kernel.org, Jens Axboe <axboe@...nel.dk>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Alex Williamson <alex.williamson@...hat.com>,
        Jérôme Glisse <jglisse@...hat.com>,
        Jason Gunthorpe <jgg@...lanox.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Max Gurtovoy <maxg@...lanox.com>,
        Christoph Hellwig <hch@....de>
Subject: Re: [PATCH v2 07/10] nvme-pci: Use PCI p2pmem subsystem to manage the
 CMB

On 05/03/18 05:49 PM, Oliver wrote:
> It's in arch/powerpc/kernel/io.c as _memcpy_toio() and it has two full barriers!
> 
> Awesome!
> 
> Our io.h indicates that our iomem accessors are designed to provide x86ish
> strong ordering of accesses to MMIO space. The git log indicates
> arch/powerpc/kernel/io.c has barely been touched in the last decade so
> odds are most of that code was written in the elder days when people
> were less aware of ordering issues. It might just be overly conservative
> by today's standards, but maybe not (see below).

Yes, that seems overly conservative.

> (I'm not going to suggest ditching the lwsync trick. mpe is not going
> to take that patch
> without a really good reason)

Well, that's pretty gross. Is this not exactly the situation mmiowb() is 
meant to solve? See [1].

Though, you're right in principle. Even if power was similar to other 
systems in this way, it's still a risk that if these pages get passed 
somewhere in the kernel that uses a spin lock like that without an 
mmiowb() call, then it's going to have a bug. For now, the risk is 
pretty low as we know exactly where all the p2pmem pages will be used 
but if it gets into other places, all bets are off. I did do some work 
trying to make a safe version of io-pages and also trying to change from 
pages to pfn_t in large areas but neither approach seemed likely to get 
any traction in the community, at least not in the near term.

Logan

[1] ACQUIRES VS I/O ACCESSES in 
https://www.kernel.org/doc/Documentation/memory-barriers.txt