From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x22b.google.com (mail-it0-x22b.google.com [IPv6:2607:f8b0:4001:c0b::22b]) (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 6C12D1A1DEE for ; Thu, 8 Sep 2016 02:38:44 -0700 (PDT) Received: by mail-it0-x22b.google.com with SMTP id e124so4041794ith.0 for ; Thu, 08 Sep 2016 02:38:44 -0700 (PDT) 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=8KppvRLKl1P8LSaW84tFSHV58hvlooq1XQQe6Efvlp8=; b=c0iap/X0o38s6IHGm70cKozd8G5qxN9xaBb9FCw54awQ4hQgfDDOJuG5hQ4zh1cqz3 NnxWEvx4iX6DIVVswIu9F57r+lbR4N1MJ1JiAr6jzLvWgYwe2DGLxYNPCDIhp79uz1hG 6AZPNgTv38wiyg1ZaGHt3dQXwN9xxNbLaPcpE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=8KppvRLKl1P8LSaW84tFSHV58hvlooq1XQQe6Efvlp8=; b=hGFp3equPpnPSo5KpWgOHUq2bSf/S9TvbK+sq/+WdNrMoBxVPogLtfRAJLJVgX6ExE clZ45Dipv/PMAWa3+7NF2FACnCzCN7qfOyn4BdEHUQB0uurHh1XsyjwSsavUNt6AGazC k0SV6Pqk/YGKZC7Z6PMdZKndmckup7rbavkEq8GGTjhgrAXWy2QpW+psk5vsBpBWnPVD Rknxztr0gB8FSqj/n4t2uMeuucBLfY0FdONL/ceK84RuDTWW3GNCfNp0oyhj8dPXLAKm IhfEnoQJt7J61GX5dvvFFNNMI0a7xggJ7zX2fQgTV5U+YhZfWtALXBxPCnQt+ylFcCU0 jYbA== X-Gm-Message-State: AE9vXwP/fQU9QbtUC7cfPtgG8vHbWygFxFmLAK2eF9z7MMMoPWpe5uxkce/fqsZrmkt9t7IkbHYvY0Qea60CtYo6 X-Received: by 10.36.65.2 with SMTP id x2mr13890555ita.78.1473327523719; Thu, 08 Sep 2016 02:38:43 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.204.195 with HTTP; Thu, 8 Sep 2016 02:38:43 -0700 (PDT) In-Reply-To: References: <1473321027-31091-1-git-send-email-ard.biesheuvel@linaro.org> <5c99e1a8-f667-aaab-7214-df94816abd8e@redhat.com> From: Ard Biesheuvel Date: Thu, 8 Sep 2016 10:38:43 +0100 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Leif Lindholm Subject: Re: [PATCH] ArmVirtPkg: restrict mapping attributes of normal memory to EFI_MEMORY_WB X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Sep 2016 09:38:44 -0000 Content-Type: text/plain; charset=UTF-8 On 8 September 2016 at 08:57, Ard Biesheuvel wrote: > On 8 September 2016 at 08:54, Laszlo Ersek wrote: >> On 09/08/16 09:50, Ard Biesheuvel wrote: >>> In general, on an ARM system, mapping normal memory as device memory may >>> have unintended side effects, given that unaligned accesses or loads and >>> stores with special semantics (e.g., load/store exclusive) may fault or >>> may not work as expected. >>> >>> Under KVM, the situation is even worse, since the host may not expect the >>> guest to perform uncached accesses, and so writes to such an uncached >>> region may get lost completely. >>> >>> Since the only safe mapping type under KVM is EFI_MEMORY_WB, remove all >>> other memory type attributes. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Ard Biesheuvel >>> --- >>> ArmVirtPkg/HighMemDxe/HighMemDxe.c | 3 +-- >>> ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c | 3 --- >>> 2 files changed, 1 insertion(+), 5 deletions(-) >>> >>> diff --git a/ArmVirtPkg/HighMemDxe/HighMemDxe.c b/ArmVirtPkg/HighMemDxe/HighMemDxe.c >>> index 7fd7e8e9a539..4d56e6236b54 100644 >>> --- a/ArmVirtPkg/HighMemDxe/HighMemDxe.c >>> +++ b/ArmVirtPkg/HighMemDxe/HighMemDxe.c >>> @@ -78,8 +78,7 @@ InitializeHighMemDxe ( >>> Status = gDS->AddMemorySpace ( >>> EfiGcdMemoryTypeSystemMemory, >>> CurBase, CurSize, >>> - EFI_MEMORY_WB | EFI_MEMORY_WC | >>> - EFI_MEMORY_WT | EFI_MEMORY_UC); >>> + EFI_MEMORY_WB); >>> >>> if (EFI_ERROR (Status)) { >>> DEBUG ((EFI_D_ERROR, >>> diff --git a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c >>> index 251e5314e61d..6f3e54b7afcb 100644 >>> --- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c >>> +++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c >>> @@ -68,9 +68,6 @@ MemoryPeim ( >>> ResourceAttributes = ( >>> EFI_RESOURCE_ATTRIBUTE_PRESENT | >>> EFI_RESOURCE_ATTRIBUTE_INITIALIZED | >>> - EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE | >>> - EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE | >>> - EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE | >>> EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE | >>> EFI_RESOURCE_ATTRIBUTE_TESTED >>> ); >>> >> >> Reviewed-by: Laszlo Ersek >> >> Did you encounter any specific problem with these? >> > > No, but I am looking into using the new optimized BaseMemoryLibOptDxe, > and the AARCH64 version uses DC ZVA instructions for ZeroMem() [i.e., > zero a cacheline, which is much faster than writing zeroes, obviously] > That does not work on uncached memory, and so while I was removing > /that/ attribute from ArmVirtPkg, I realized that WC and WT are not > safe either. > > For bare metal platforms, we will keep WC and WT, but for ArmVirtQemu, > they don't make sense. Not sure about Xen though ... Pushed, thanks.