Hi, Jeremy Sorry for missing this mail.
I would *REJECT* this patch as it should be grub2's bug in which it passes down an unaligned buffer. Accroding to UEFI spec , it clearly described that the IoAlign should be met by the BlockIo's caller "IoAlign Supplies the alignment requirement for any buffer used in a data transfer. IoAlign values of 0 and 1 mean that the buffer can be placed anywhere in memory. Otherwise, IoAlign must be a power of 2, and the requirement is that the start address of a buffer must be evenly divisible by IoAlign with no remainder." And another proof is the return status described in BlockIo.ReadBlocks() "EFI_INVALID_PARAMETER The read request contains LBAs that are not valid, or the buffer is not on proper alignment." Thanks Feng -----Original Message----- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ryan Harkin Sent: Tuesday, February 09, 2016 21:23 To: Jeremy Linton Cc: edk2-devel@lists.01.org; Leif Lindholm Subject: Re: [edk2] [PATCH] MdeModulePkg, AtaBusDxe: Bounce buffer IO operations if unaligned Hi Jeremy, On 15 January 2016 at 15:20, Jeremy Linton <jeremy.lin...@arm.com> wrote: > BlockIo Protocol devices specify a buffer alignment requirement in > their media descriptors. This should be honored by users of the protocol. > In cases where the user of the protocol (grub2 in this case) fail to > correctly align their buffers, IO failures occur. Apparently this has > been known for a while as numerous other BlockIo protocol providers in > edk2 bounce IO operations rather than simply allowing them to fail. > > This patch adds bounce buffer logic to the AtaBusDxe. This logic is > only triggered if the buffers are not correctly aligned. It assures > that a wide range of adapters work regardless of buggy EFI applications. > You are missing this line: Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jeremy Linton <jeremy.lin...@arm.com> > --- > MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.c | 26 > ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.c > b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.c > index e7449f9..8cb08ce 100644 > --- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.c > +++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.c > @@ -1038,6 +1038,7 @@ BlockIoReadWrite ( > UINTN BlockSize; > UINTN NumberOfBlocks; > UINTN IoAlign; > + VOID *IoBounceBuffer; > > if (IsBlockIo2) { > Media = ((EFI_BLOCK_IO2_PROTOCOL *) This)->Media; > @@ -1078,7 +1079,18 @@ BlockIoReadWrite ( > > IoAlign = Media->IoAlign; > if (IoAlign > 0 && (((UINTN) Buffer & (IoAlign - 1)) != 0)) { > - return EFI_INVALID_PARAMETER; > + IoBounceBuffer=AllocateAlignedBuffer(AtaDevice,BufferSize); > + if (IoBounceBuffer == NULL) { > + //both bad parameter and out of resources.. > + return EFI_INVALID_PARAMETER; > + } > + > + if (IsWrite) { > + CopyMem(IoBounceBuffer,Buffer,BufferSize); > + } > + } > + else { > + IoBounceBuffer=Buffer; > } > > OldTpl = gBS->RaiseTPL (TPL_CALLBACK); @@ -1086,10 +1098,20 @@ > BlockIoReadWrite ( > // > // Invoke low level AtaDevice Access Routine. > // > - Status = AccessAtaDevice (AtaDevice, Buffer, Lba, NumberOfBlocks, > IsWrite, Token); > + Status = AccessAtaDevice (AtaDevice, IoBounceBuffer, Lba, > + NumberOfBlocks, IsWrite, Token); > > gBS->RestoreTPL (OldTpl); > > + // were we using a bounce buffer? > + if (IoAlign > 0 && (((UINTN) Buffer & (IoAlign - 1)) != 0)) { > + if (!IsWrite) { > + // for reads copy the data back to the original buffer > + CopyMem(Buffer,IoBounceBuffer,BufferSize); > + } > + FreeAlignedBuffer(IoBounceBuffer,BufferSize); > + } > + > return Status; > } > > -- > 2.4.3 > > Unfortunately, when I booted with this patch on top of my tree [1] the board crashes when booting from PXE when I have a SATA hard drive attached. If I disconnect the drive, it boots fine. Here's the trace: Booting EFI Network Synchronous Exception at 0x00000000F8B6C058 /working/platforms/uefi/edk2/Build/ArmJuno/DEBUG_GCC49/AARCH64/MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe/DEBUG/Udp4Dxe.dll loaded at 0x00000000F8B65000 X0 0xAFAFAFAFAFAFAFAF X1 0x00000000000F4240 X2 0x0000000000000000 X3 0x0000000000000000 X4 0x00000000FCFC68A0 X5 0x00000000F8B68E4C X6 0x00000000FD007AA8 X7 0x00000000FD02F120 X8 0x00000000FE331780 X9 0x0000004100000000 X10 0x0000000000000092 X11 0x0000000000000083 X12 0x0000000070FFE07A X13 0x0000000000000000 X14 0x0000000000000000 X15 0x0000000000000000 X16 0x00000000FEFFFBF0 X17 0x0000000000000000 X18 0x0000000000000000 X19 0x00000000FCFC68AC X20 0x0000000000000000 X21 0x0000000000000000 X22 0x0000000000000000 X23 0x0000000000000000 X24 0x0000000000000000 X25 0x0000000000000000 X26 0x0000000000000000 X27 0x0000000000000000 X28 0x0000000000000000 FP 0x0000000000000000 LR 0x00000000F8B6BF24 V0 0xF7FFFFFFFEBFFFBF FFFFFFFB77FFFFFF V1 0xFFFF9FFDFFFEFFFE FCFFFFFBAFFFBFDF V2 0xB77FE7EFFFFFFFFF FBFFFFF1BFFFEFF7 V3 0xFFBDFFFBDFBBFFFD FFFFFFF5FFFDFF73 V4 0x7BF7FBFFFBFFFFFB F77FFFFBFDFFFFF7 V5 0xFFFBFFFF7DEDF7FF FFFFFFFFFFFFFFFF V6 0xFABAFFFE57BFFFFF 7F7F9FFFDFFFFF7F V7 0xBBFFFCFBFFFFFFBD EFFFBFFDFFFFFFFF V8 0x0000000000000000 FFFF7FFFF5FFFFFF V9 0x0000000000000000 F7FFFFFFFFFFFBFE V10 0x0000000000000000 7FFBFEFEFFFFFFFF V11 0x0000000000000000 F3FFBFFFFFFBFFFF V12 0x0000000000000000 FEFF7DFFBFFFFFFF V13 0x0000000000000000 BFFEFEFEFFFFFFFF V14 0x0000000000000000 FBFCFFFF7FFFFBFF V15 0x0000000000000000 FFFEFFFFFFFF7FFF V16 0xFFEF7EFFFFFFF7FF F7FFFBFFFFF7FFFF V17 0xFDFEFDFFFFFFD5FF FFFFFFFFFFFDFFFE V18 0xEFFFFFFFFFFFFFFF DBFFFF7DFFFFFBFE V19 0xFFFDFFBBFFFFE7FF FFFDFFFFFFF7FBBE V20 0x7FFD5F7FFFFFFFFF FFFFFBFF7FD7FDFF V21 0xFFFFFFFBFFFFFFF7 FFFBFEFFFEFDFDB5 V22 0x7FFBF7F6FFFEFFFF FF7DDFFA06A10300 V23 0xFFDFEFFFB7FFFFFF 7F377EFEBFBFFEFF V24 0xFFDFFFFFFFFFFFFF 7DEE7FF7FFFFDFEF V25 0xFFFFFFFFDFFF7FFF FFFBFFFFFEDFF7FF V26 0xDFFFFFFEFFFFFFFF FFFFBFDF7FFFFFBF V27 0xFDFBFFBFFFFFFFFF FFC7F7EFFFFFFDFF V28 0xFF7FEFFF7FDF6BFF CFFE7EFFFFFDFFDF V29 0xF7FFBF77EFFFFFFF BFFFFF9F71024505 V30 0xFF77FDBDF7FFFFFF DFFEEEFBBB6FFFFF V31 0xFFFFFFF7FFFFFFBF F7FFF77FB6FF7FFF SP 0x00000000FEFFF150 ELR 0x00000000F8B6C058 SPSR 0x20000309 FPSR 0x00000000 ESR 0x96000004 FAR 0xAFAFAFAFAFAFAFAF ESR : EC 0x25 IL 0x1 ISS 0x00000004 Data abort: Translation fault, zeroth level ASSERT [ArmCpuDxe] /working/platforms/uefi/edk2/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c(186): ((BOOLEAN)(0==1)) If I unplug the network cable so PXE fails, it attempts to boot from HDD instead and I get another trace: Booting EFI Hard Drive Synchronous Exception at 0x00000000FE3177C0 /working/platforms/uefi/edk2/Build/ArmJuno/DEBUG_GCC49/AARCH64/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll loaded at 0x00000000FE305000 X0 0x00000000FE3323C8 X1 0x00000000AFAFAFAF X2 0x0000000000000000 X3 0x00000000000007E0 X4 0x00000000018D0D10 X5 0x0000000000000000 X6 0x0000000000000000 X7 0x0000000000000020 X8 0x00000000FE331780 X9 0x0000004100000000 X10 0x0000000000000092 X11 0x0000000000000083 X12 0x0000000070FFE07A X13 0x0000000000000000 X14 0x0000000000000000 X15 0x0000000000000000 X16 0x00000000FEFFFBF0 X17 0x0000000000000000 X18 0x0000000000000000 X19 0x00000000FE32517C X20 0x0000000000010000 X21 0x00000000FD89C018 X22 0x0000000000000000 X23 0x0000000000A03800 X24 0x00000000FC6FE018 X25 0x0000000000000000 X26 0x0000000000000000 X27 0x0000000000000010 X28 0x0000000000000001 FP 0x0000000000000000 LR 0x00000000FE317768 V0 0xF7FFFFFFF6BFFFA7 7FFFFEBA77FFFFFF V1 0xFFFFBFFDFFFEFFFE FC7FFFFBAFFFBFDF V2 0x97FFE7EFFFFFFFFF FBFFBFF1BFFFEFFF V3 0xFFBDFFFBCFBBFFED FFF9FFE5FEF9EF73 V4 0x7BF77BFFDBFFFFFB F77FFFF9FDFFAFF7 V5 0xFFFBFFFF7DE9F77F FFFFFF5FFFFFFFFF V6 0xB8DAFFFF57BFFFDF 7F5FDFFFFFFFFF7F V7 0xBAFFFCFBF7FFFE3D EFFDBFFDFFFFFFFF V8 0x0000000000000000 FFFF7CFFF5FFFF7F V9 0x0000000000000000 F7FDFFFFFFFFFBFE V10 0x0000000000000000 7FFBFEFAF7FFFFFF V11 0x0000000000000000 F3FFB2FFFFFBFDFF V12 0x0000000000000000 EFFF7DFF9FFFBFFF V13 0x0000000000000000 BFFEFEFEFFFFFFFF V14 0x0000000000000000 FBECFFFF7FFFFBFF V15 0x0000000000000000 FFFEFEBFFFFF7FFF V16 0xFFEF7EBFFFFFF7FF B5FFFBFFFFE7F7FB V17 0xFDFED9FFBFFFD5FF FFFFFFFFFFFDFFFF V18 0xFFFFFFFFFFFFFFFF DFF7FF7DFF7FFBFE V19 0xFFEDFFFBFFFFE7FF DFFDFFFFFFF7FBBE V20 0x7FFD5F7F5FFFFFFF BFFFF9FF7FD7FCFB V21 0xFFFFFFFDFFFFFFF7 FFF3FEFFFFFDFDF5 V22 0x7BFFF7D6FFFEFFFF FF7DDFFA06A10100 V23 0xFFDFEFFFF7FFFFFF 7F277EFEBFBFFEFF V24 0xFFDFFFFEFFFFFFFF 7D6E7FF7FFFFDFEF V25 0xFFD7FFFFDFFF7FFF FFFBFFFFFEDFF7FF V26 0x97FFFFDFFEFFFFFF FFFFBFDF7FFEFFBF V27 0xFDFAFFBFFFFFFFFF FDC7F7FF7FFFF5FF V28 0xFD7FE7FE7FDFEBFF CDFE7E7FFFFDFFDF V29 0xF7EFAF66EFFFFFFF BFFFFF9F71024505 V30 0x7F77FDBCF7FDFFFF DFFECEFABB6FFFFF V31 0xFFDEFFF7FFFFFFBF F7BFF77FF6F77FFF SP 0x00000000FEFFE8B0 ELR 0x00000000FE3177C0 SPSR 0x30000309 FPSR 0x00000000 ESR 0x96000005 FAR 0x000000025D928326 ESR : EC 0x25 IL 0x1 ISS 0x00000005 Data abort: Translation fault, first level ASSERT [ArmCpuDxe] /working/platforms/uefi/edk2/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c(186): ((BOOLEAN)(0==1)) [1] https://git.linaro.org/landing-teams/working/arm/edk2.git/shortlog/refs/tags/armlt-20160209 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
