| Message ID | 1477937590-10361-5-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
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.biesheuvel@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)? > Status = gBS->InstallMultipleProtocolInterfaces ( > &mCpuHandle, > &gEfiCpuArchProtocolGuid, &mCpu, > -- > 2.7.4 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 1 November 2016 at 22:32, Leif Lindholm <leif.lindholm@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.biesheuvel@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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, Nov 02, 2016 at 01:40:17PM +0000, Ard Biesheuvel wrote: > On 1 November 2016 at 22:32, Leif Lindholm <leif.lindholm@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.biesheuvel@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. / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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 (); + Status = gBS->InstallMultipleProtocolInterfaces ( &mCpuHandle, &gEfiCpuArchProtocolGuid, &mCpu,
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.biesheuvel@linaro.org> --- ArmPkg/Drivers/CpuDxe/CpuDxe.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel