Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210726100026.12538-1-lorenzo.pieralisi@arm.com>
 <20210802152359.12623-2-lorenzo.pieralisi@arm.com> <YRKtEDycefrZLB3X@infradead.org>
In-Reply-To: <YRKtEDycefrZLB3X@infradead.org>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Wed, 11 Aug 2021 12:40:28 +0200
Message-ID: <CAMj1kXEB1CFj1svCWu7yOoUi_OkEqYEUQnB_XWOd3gD+ejO_6w@mail.gmail.com>
Subject: Re: [PATCH v2 1/3] ACPI: osl: Add __force attribute in
 acpi_os_map_iomem() cast
To:     Christoph Hellwig <hch@infradead.org>
Cc:     Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Veronika kabatova <vkabatov@redhat.com>,
        Robin Murphy <robin.murphy@arm.com>,
        Will Deacon <will@kernel.org>,
        Hanjun Guo <guohanjun@huawei.com>,
        Sudeep Holla <sudeep.holla@arm.com>,
        Catalin Marinas <catalin.marinas@arm.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, 10 Aug 2021 at 18:46, Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Aug 02, 2021 at 04:23:57PM +0100, Lorenzo Pieralisi wrote:
> > Add a __force attribute to the void* cast in acpi_os_map_iomem()
> > to prevent sparse warnings.
>
> Err, no.  These annotation are there for a reason and need to
> be propagated instead.  And independent of that a __force cast
> without a comment explaining it is a complete no-go.

The whole problem we are solving here is that ACPI, being based on
x86, conflates MMIO mappings with memory mappings, and has been using
the same underlying infrastructure for either. On arm64, this is not
sufficient, given that the semantics of uncached memory vs device are
different (the former permits unaligned accesses and clear cacheline
instructions, but the latter doesn't). A recent optimization applied
to memcpy() on arm64 (which now relies more on unaligned accesses for
performance) has uncovered an issue where firmware tables being mapped
non-cacheable by the ACPI core will end up using device mappings,
which causes memcpy() to choke on their contents.

So propagating the annotation makes no sense, as we are creating a
memory mapping using the iomem primitive. I wouldn't object to a
comment being added, but I think the context should have been obvious
to anyone who had bothered to look at the entire series.