From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x232.google.com (mail-wm0-x232.google.com [IPv6:2a00:1450:400c:c09::232]) (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 0B9E581C8A for ; Wed, 2 Nov 2016 09:10:44 -0700 (PDT) Received: by mail-wm0-x232.google.com with SMTP id p190so278561485wmp.1 for ; Wed, 02 Nov 2016 09:10:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=uW3xPAobGKgwy+gFtLNO474dbmTgwLHxDyAsNdbbAQI=; b=Mgtixx22XO76iwnncIf2uGUuI1Q5A3cET4ONWbiB+ijpKtRtT1/KQihaj3ZoFo1mQT /2/+V9g2S46Gg54IMV3a+dwazFFME6DXplLnZdD+Ew38kyQjcQrqkRc/eWanFK7H9HVc ZH81RCyD8TIUHBtPpkjo1k0UQ9HO/5S5mid4k= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=uW3xPAobGKgwy+gFtLNO474dbmTgwLHxDyAsNdbbAQI=; b=EB6IPsVPAV+4qb/auSROnE2rymY6rFzQSigc+RoJNYeJVtniaHin87Wx1gPiIAxQBL h9jD0N3QhqDXfqK88sQJ5N44sEID1vImPUFsOLouGLrBk9immEXqhWWTJ68OWQ8AeEtt 9CD2+y4QX4+1dI0enDwggFYolhi4rrz152NgjT47/8VoJ55kaJ212tXExj8zEllJiqAJ qUjDKpR7HDrx967enZ+DHy9ravuj+io/xg4vJ5nkjhxpcpXikq2SkWDUDKJnUzf/6yIc SL5qZz8QCj1xil4jF987v72DlLVQrI4bPyQZc2mZxPNNZu8Luvh3x5mUqS8q1dDsA4aQ Uvmg== X-Gm-Message-State: ABUngvfhcOaHK3PKHKnkMktiyyGZsUQSm1PnIqYMCPSgLlTyUpDXLQVJNR6pKc21fRztJGu/ X-Received: by 10.194.223.97 with SMTP id qt1mr3442022wjc.33.1478103043429; Wed, 02 Nov 2016 09:10:43 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id hy10sm3594601wjb.10.2016.11.02.09.10.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 02 Nov 2016 09:10:42 -0700 (PDT) Date: Wed, 2 Nov 2016 16:10:41 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel-01 Message-ID: <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> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [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: , X-List-Received-Date: Wed, 02 Nov 2016 16:10:44 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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. / Leif