On 2 November 2016 at 16:21, Leif Lindholm <leif.lindh...@linaro.org> wrote:
> On Wed, Nov 02, 2016 at 04:17:03PM +0000, Ard Biesheuvel wrote:
>> On 2 November 2016 at 16:10, Leif Lindholm <leif.lindh...@linaro.org> wrote:
>> > On Wed, Nov 02, 2016 at 01:40:17PM +0000, Ard Biesheuvel wrote:
>> >> On 1 November 2016 at 22:32, Leif Lindholm <leif.lindh...@linaro.org>
>> >> 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 <ard.biesheu...@linaro.org>
>> >> >> ---
>> >> >> 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.
>
> Agreed.
>
>> So you are happy with the patch if I fold the following into it?
>
> Super happy - thanks!
> Reviewed-by: Leif Lindholm <leif.lindh...@linaro.org>
>
OK, pushed.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
