From patchwork Wed Nov 2 16:17:03 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 80506 Delivered-To: patch@linaro.org Received: by 10.140.97.247 with SMTP id m110csp212271qge; Wed, 2 Nov 2016 09:17:05 -0700 (PDT) X-Received: by 10.99.146.8 with SMTP id o8mr6791518pgd.55.1478103425752; Wed, 02 Nov 2016 09:17:05 -0700 (PDT) Return-Path: Received: from ml01.01.org (ml01.01.org. [198.145.21.10]) by mx.google.com with ESMTPS id 145si3840676pgc.315.2016.11.02.09.17.05 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 02 Nov 2016 09:17:05 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) client-ip=198.145.21.10; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 91F9C81D39; Wed, 2 Nov 2016 09:17:03 -0700 (PDT) X-Original-To: edk2-devel@lists.01.org Delivered-To: edk2-devel@lists.01.org Received: from mail-yb0-x22a.google.com (mail-yb0-x22a.google.com [IPv6:2607:f8b0:4002:c09::22a]) (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 2F6B181D36 for ; Wed, 2 Nov 2016 09:17:03 -0700 (PDT) Received: by mail-yb0-x22a.google.com with SMTP id o7so6607940ybb.0 for ; Wed, 02 Nov 2016 09:17:04 -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=k/Ju/hekCVKUELJ6E6WzQ6OruGwZL1TNkaK3SNTcj8w=; b=P2CNBcpI8ZeDzFxZVA02m21qaJn2OWJ4oAzaPIOw4b49KO44wopagGj/qCk60R/rxw V2sKB/X/hjQOii96p06akpU4f/QpLdeRQu+poRdq1sRoNAfjrpMKYAbsYn+7YpF7tWw7 NwHWDrAKPl7AoEW5Sbs+KzCeL9WBlIUUyMb0M= 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=k/Ju/hekCVKUELJ6E6WzQ6OruGwZL1TNkaK3SNTcj8w=; b=mTjqprpIierdkSHf7LrtCENtoF0VSKJmnMMr0i3fV9d1tr/ze8hpDKAvOQrOtS9L7z uXf0QjsArrtbwUF6mn7etnpIYhBmbBHFiAkBnfC7+pi3etGTk6v10ZJlTqJiXuxJb+JQ SMg+7vxcIpHnCqzMQoRjaJ6wzw4ztYu929LbHsUD4J7MbA2MR2XZ7wensegedJKdnqOs 067l5HHBqA/to6nNfdEqzGkC7No208AHAQukv9Rf5sAnrv74mNDGXEQdQJ1YPojXItrY vlHX/3iceiLZ1Byg5m3ZNM8mtSuhcQKsV4Ar19KWExCvH+g0RX07YsPWiedPeSJjltZT Cilg== X-Gm-Message-State: ABUngvcRyphOQOXhkVUzhOTdqexxbGJ4SDsxR0IbXQt4rEvWQv+pPn39fY6FFbWhcHDYI7MaOvw64JHd7+G15lAR X-Received: by 10.36.80.67 with SMTP id m64mr3221162itb.59.1478103423829; Wed, 02 Nov 2016 09:17:03 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.59.147 with HTTP; Wed, 2 Nov 2016 09:17:03 -0700 (PDT) In-Reply-To: <20161102161040.GD27644@bivouac.eciton.net> References: <1477937590-10361-1-git-send-email-ard.biesheuvel@linaro.org> <1477937590-10361-5-git-send-email-ard.biesheuvel@linaro.org> <20161101223227.GP1161@bivouac.eciton.net> <20161102161040.GD27644@bivouac.eciton.net> From: Ard Biesheuvel Date: Wed, 2 Nov 2016 16:17:03 +0000 Message-ID: To: Leif Lindholm Subject: Re: [edk2] [PATCH 4/5] ArmPkg/CpuDxe: set DmaBufferAlignment according to CWG 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: , Cc: edk2-devel-01 Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" On 2 November 2016 at 16:10, Leif Lindholm wrote: > On Wed, Nov 02, 2016 at 01:40:17PM +0000, Ard Biesheuvel wrote: >> On 1 November 2016 at 22:32, Leif Lindholm wrote: >> > On Mon, Oct 31, 2016 at 06:13:09PM +0000, Ard Biesheuvel wrote: >> >> The DmaBufferAlignment currently defaults to 4, which is dangerously >> >> small and may result in lost data on platform that perform non-coherent >> >> DMA. So instead, take the CWG value from the cache info registers. >> >> >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> >> Signed-off-by: Ard Biesheuvel >> >> --- >> >> ArmPkg/Drivers/CpuDxe/CpuDxe.c | 4 +++- >> >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.c b/ArmPkg/Drivers/CpuDxe/CpuDxe.c >> >> index d089cb2d119f..ddc64fd255a0 100644 >> >> --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.c >> >> +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.c >> >> @@ -225,7 +225,7 @@ EFI_CPU_ARCH_PROTOCOL mCpu = { >> >> CpuGetTimerValue, >> >> CpuSetMemoryAttributes, >> >> 0, // NumberOfTimers >> >> - 4, // DmaBufferAlignment >> >> + 2048, // DmaBufferAlignment >> >> }; >> >> >> >> EFI_STATUS >> >> @@ -239,6 +239,8 @@ CpuDxeInitialize ( >> >> >> >> InitializeExceptions (&mCpu); >> >> >> >> + mCpu.DmaBufferAlignment = ArmCacheWritebackGranule (); >> >> + >> > >> > Could we hide the internal structure of mCpu here by moving this to a >> > helper function and calling >> > InitializeDma (&mCpu); >> > (or something)? >> > >> >> We could, but why? The actual struct is defined 10 lines up > > It's just that it's the only place in this function we're prodding the > internals of the object directly. Messes slightly with my zen. > Not a strong opinion, just a preference. > Sure, if you want. In fact, I will break this out as a separate patch, considering that it fixes a serious bug. So you are happy with the patch if I fold the following into it? """ """ _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel Reviewed-by: Leif Lindholm --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.c +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.c @@ -228,6 +228,15 @@ EFI_CPU_ARCH_PROTOCOL mCpu = { 2048, // DmaBufferAlignment }; +STATIC +VOID +InitializeDma ( + IN OUT EFI_CPU_ARCH_PROTOCOL *CpuArchProtocol + ) +{ + CpuArchProtocol->DmaBufferAlignment = ArmCacheWritebackGranule (); +} + EFI_STATUS CpuDxeInitialize ( IN EFI_HANDLE ImageHandle, @@ -239,7 +248,7 @@ CpuDxeInitialize ( InitializeExceptions (&mCpu); - mCpu.DmaBufferAlignment = ArmCacheWritebackGranule (); + InitializeDma (&mCpu); Status = gBS->InstallMultipleProtocolInterfaces ( &mCpuHandle,