From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-x22c.google.com (mail-wr0-x22c.google.com [IPv6:2a00:1450:400c:c0c::22c]) (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 1BD6321967BF9 for ; Tue, 13 Jun 2017 02:17:49 -0700 (PDT) Received: by mail-wr0-x22c.google.com with SMTP id q97so133502959wrb.2 for ; Tue, 13 Jun 2017 02:19:03 -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=8Zd7adcAcPYpqnncKk7x7JFYsOp7TDRT+B1PCFCnsdo=; b=X3v7XrAvTreVxk+IZ8HQnnc2n22LAprwswn5frG5AlX7nBgi7xYcVkhp2J4pBS54p8 v4CIGQln5CTnhMh+1YL+9c1ztmTreVVAEE66dbMmGBkKBTPev18VaOd0L1CiwiikCONb 2GGcL3FeOYwHTvt9cyAkKyMdsFW4V+oTq16vQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=8Zd7adcAcPYpqnncKk7x7JFYsOp7TDRT+B1PCFCnsdo=; b=hgNXAcJTsJ5kt0zjLnb+OMl5vEa+gf3suHNPnscTHnH2lmrUNjQR8ROSvippMzb0gF c69TubMEHxb89YDAhh0Lv0swlJURhLrlpDVlocJKm3wcKMS09Swr9plAozuFfBQfPyik r35rJ8zK4yS3WkB55Wk4NlQTt4x9l2xCKx0vR3/1FIxJQFPSrGrb95Fre4Ifz0L93t8j 0fbTFuSRV31v1j47RsRvKJ677Rt0cC4VMC7+U9V5Pe+APBz65PBwvNlT6Ge+TmzCsWfI XqQl2ttMOz0Eo/tA/X814xFf0QCKMWsheJsfwa6e+xN8lPP2f8RGOCZsFkdGkAXOq4o5 zC5A== X-Gm-Message-State: AKS2vOyFcIVCPfKlTUUcdBzDXmroJT7yi19UqhQH6cag3Jj9JAa/g28Y aHZhah7rkHJTlp6X X-Received: by 10.28.167.135 with SMTP id q129mr1918392wme.73.1497345541715; Tue, 13 Jun 2017 02:19:01 -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 e77sm2955779wma.32.2017.06.13.02.19.00 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 13 Jun 2017 02:19:01 -0700 (PDT) Date: Tue, 13 Jun 2017 10:18:59 +0100 From: Leif Lindholm To: Jun Nie Cc: Ard Biesheuvel , Haojian Zhuang , edk2-devel@lists.01.org, Shawn Guo , Jason Liu Message-ID: <20170613091859.GO26676@bivouac.eciton.net> References: <1497232768-8993-1-git-send-email-jun.nie@linaro.org> <20170612155315.GK26676@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [PATCH v2] EmbeddedPkg/MmcDxe: Add alignment for ECSD data X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 13 Jun 2017 09:17:49 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Jun 13, 2017 at 10:14:34AM +0800, Jun Nie wrote: > 2017-06-12 23:53 GMT+08:00 Leif Lindholm : > > On Mon, Jun 12, 2017 at 09:59:28AM +0800, Jun Nie wrote: > >> Add alignment for ECSD data for DMA access. Otherwise > >> the data is corrupted on Sanechips platform. > > > > I never did see a reply to my proposed solution, and the below is not > > it. Can you explain why you prefer this one? > > Sorry, just see your email because that thread is not highlighted for > new email in gmail for unknown reason. > I have concern that "UINT64 VENDOR_SPECIFIC_FIELD[8]" cannot secure > the ECSD alignment because it is not the first member. It being the first member or not has no relevance. By my count, it starts at offset 64, with all preceding entries being UINT8. > Changing the first member to "UINT64 RESERVED_1[2]" shall secure the > alignment. Relying on a reserved field for setting alignment constraints risks having to find an alternative method at some point in the future, so I agree this is not a preferred solution. > But I preferred Pad method. It is more readable if all ECSD member > are UINT8 type. I confess am not familiar enough with eMMC to make a clean call here, but if real alignment constrains exist, using UINT8 is actively misleading, not to mention incorrect. If there is no real alignment constraint, this is not the code that needs fixing. > It is also more clear to add alignment info in > CARD_INFO, just before ECSD member. Why is it more clear to apply alignment constraints at point of use instead of point of definition? Is there any chance you can share the EFI_MMC_HOST_PROTOCOL implementation that triggers this error? / Leif > I do not get point of Andrew, maybe he share the same concern. > > Jun > > > > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Jun Nie > >> --- > >> EmbeddedPkg/Universal/MmcDxe/Mmc.h | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h b/EmbeddedPkg/Universal/MmcDxe/Mmc.h > >> index 8a7d5a3..6e3ab17 100644 > >> --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h > >> +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h > >> @@ -319,6 +319,7 @@ typedef struct { > >> OCR OCRData; > >> CID CIDData; > >> CSD CSDData; > >> + UINT64 Pad; // For 8 bytes alignment of ECSDData > >> ECSD ECSDData; // MMC V4 extended card specific > >> } CARD_INFO; > >> > >> -- > >> 1.9.1 > >>