From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::241; helo=mail-io0-x241.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x241.google.com (mail-io0-x241.google.com [IPv6:2607:f8b0:4001:c06::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A0380222CF1BE for ; Wed, 10 Jan 2018 03:57:05 -0800 (PST) Received: by mail-io0-x241.google.com with SMTP id d11so1912909iog.5 for ; Wed, 10 Jan 2018 04:02:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=3P/0LH4+JfTlQlMT/7oyU+03onjV1yrohUuUFuQzX4M=; b=cIhuv5KujRtL5PyXx7dW0rx3VdFeEKceUG0SVIlRBknAHvUTXPTGmmcg8rUUgZLDxG Bqaxs33fTErRB4LnoJ+chBkRt8Sg+SwqcO5E276UR8IBC45eJNOi5N56h2pvHZa4xaI8 LT/dnoL6Spd/po7t/jvCOlfQzDqQio2+ZjUEg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=3P/0LH4+JfTlQlMT/7oyU+03onjV1yrohUuUFuQzX4M=; b=l2VX17ibMMIRE5NECtd/NgbsljsUi82PliMBIHYwxNxyi+5wdbAjyuOzn+yhcH8wpX mljDWq0XE9R1K2eV4XQ4uajtEBvUrZL+uGw7mJWbunCd7wt7Ei4AiDxsR7Eoe3ku7R5v DqEu+zsGEoNoOMUMv8Jk7Jp0fjZ7Jq65+8XXlYClqEmYhmCeA/iLC3PE1lwrvhLCdNYd rsUwIIzZWcvgCi7JF8i0CPJkKsZY/SicEDZNdXEuRihgYiDhNqRbNnYOR1HAsxQvXicC r8BvJo2xfe/b6xjtL6y1oEL3htjRCEe3cnBq3xhlmAtiK8cQeDQamtUbyTYIxNBZSbmM uwpQ== X-Gm-Message-State: AKwxytdJxzA3SFGpnbxOQa+Eh0l7g0sUdxJXDZulkOy7jYx3vsGOOspP IWK/RoH43BcTlt/a5kEshNqRPcVWJZA2H28BpzYBnrB5 X-Google-Smtp-Source: ACJfBouDi+wNkqCWqO3XDqe2K6zzgfCWwnd4TQqe3h2pAsDQA0fkvdjAEIxK24olgHu+TEfy8RBWt+2VeIMxYEtMZsY= X-Received: by 10.107.135.205 with SMTP id r74mr830051ioi.248.1515585736463; Wed, 10 Jan 2018 04:02:16 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.37.197 with HTTP; Wed, 10 Jan 2018 04:02:15 -0800 (PST) In-Reply-To: References: <20171222190821.12440-1-evan.lloyd@arm.com> <20171222190821.12440-19-evan.lloyd@arm.com> From: Ard Biesheuvel Date: Wed, 10 Jan 2018 12:02:15 +0000 Message-ID: To: Alexei Fedorov Cc: Evan Lloyd , "Matteo.Carlini@arm.com@arm.com" <"Matteo.Carlini@arm.com"@arm.com>, "leif.lindholm@linaro.org@arm.com" <"leif.lindholm@linaro.org"@arm.com>, "nd@arm.com@arm.com" <"nd@arm.com"@arm.com>, "edk2-devel@lists.01.org" , Arvind Chauhan , "ard.biesheuvel@linaro.org@arm.com" <"ard.biesheuvel@linaro.org"@arm.com>, Thomas Abraham Subject: Re: [PATCH edk2-platforms v2 18/18] ARM/JunoPkg: Add HDLCD platform library X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Jan 2018 11:57:06 -0000 Content-Type: text/plain; charset="UTF-8" On 10 January 2018 at 11:45, Alexei Fedorov wrote: > Hi Ard, > > > You wrote > > "Well, the fact that you need to override the BaseMemoryLib > implementation is telling. This means the region is treated as memory > in the code, but mapped with device semantics. So this is not about > performance, but about the face that the UEFI spec stipulates that > unaligned access to memory are allowed on AArch64, and so if the > contents of these regions are dereferenced as memory, they should be > mapped as memory. If they are mapped as device, you should only use > MmioRead32/MmioWrite32 accessors." > > > In that case could you comment on the following commits: > > "The BaseMemoryLib has switch to use BaseMemoryLibOptDxe at OPP, but the > flash module is device attributes and have to be alignment accessed. so we > change the flash related drivers to use generic BaseMemoryLib which is > alignment access. " > > > Hisilicon/D02: flash related drivers switch to use generic BaseMemoryLib: > https://git.linaro.org/uefi/OpenPlatformPkg.git/commit/Platforms/Hisilicon/D02/Pv660D02.dsc?id=e3c520596d9ebdc525f284a9da95f080af815ec9 > > > Hisilicon/D03: flash related drivers switch to use generic BaseMemoryLib: > https://git.linaro.org/uefi/OpenPlatformPkg.git/commit/Platforms/Hisilicon/D03/D03.dsc?id=337713801cceb684326bfde22975310ca1bd0cc0 > > > Hisilicon/D05: flash related drivers switch to use generic BaseMemoryLib: > https://git.linaro.org/uefi/OpenPlatformPkg.git/commit/Platforms/Hisilicon/D05/D05.dsc?id=0749464022407f11c0c6a46cb8eb293fc74ef236 > NOR flash can only be mapped as device memory, because it switches between array mode and command mode, and in the latter mode, we require device semantics. Ideally, we should be able to switch between cacheable and device mappings in this case (because we also switch between modes), but this is not possible when running under the OS, and VariableRuntimeDxe is a DXE_RUNTIME_DRIVER type module. In general, memory should not be mapped with device semantics and vice versa. NOR flash is arguably a corner case, especially because the generic variable driver is shared with x86 which doesn't care about any of this, and so the code is quite sloppy when it comes to dereferencing pointers that may point outside of memory.