You mean ASSERT (((UINTN) InternalData & (*Size - 1)) == 0)? Do you want to use ReadUnaligned/WriteUnaligned for all the UINT16*/UINT32*/UINT64* data pointers even they are assured to be aligned? I prefer to use ASSERT() if I must select one.
Thanks, Star -----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] Sent: Wednesday, May 6, 2015 4:19 PM To: Zeng, Star Cc: edk2-devel@lists.sourceforge.net; Laszlo Ersek; Feng, Bob C Subject: Re: [edk2] [PATCH] MdeModulePkg: avoid unaligned writes in PcdDxe driver On 6 May 2015 at 10:13, Zeng, Star <star.z...@intel.com> wrote: > Yes of course, it relies on the correctness of the BaseTools. Even the > whole BIOS image relies on the correctness of the BaseTools. ^_^ > ... which is exactly why there are ASSERT()s all over the place, isn't it? > The ASSERT() you mean is like below? I admit it is an approach to ensure the > correctness of BaseTools. > ASSERT (((UINTN) InternalData & (sizeof (UINT16) - 1)) == > 0); > No we need (*Size - 1) not a constant. > But the ASSERT check maybe a little redundant as it will be in every 16/32/64 > PcdSet. Do you think it could be a little more efficient to add ASSERT check > only for the start pointer of uninitialized data(must be 8bytes aligned) as > we found the bug only in the uninitialized data? you can reference the > analysis result about the root cause of the bug in the email I attached in > the previous email thread. > Well, the most efficient would be not to ASSERT() at all but use ReadUnaligned/WriteUnaligned, since those will be implemented according to the capabilities of the architecture, i.e., it may simply be an unaligned load/store, but in ARM's case, it may perform the access byte by byte (depending on which base architecture version is being targeted) I still think we need to approach this as two separate problems, and fix this one by using the unaligned accessors. That way, it is independent of whether/how the BaseTools get fixed. -- Ard. ------------------------------------------------------------------------------ One dashboard for servers and applications across Physical-Virtual-Cloud Widest out-of-the-box monitoring support with 50+ applications Performance metrics, stats and reports that give you Actionable Insights Deep dive visibility with transaction tracing using APM Insight. http://ad.doubleclick.net/ddm/clk/290420510;117567292;y _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
