As far as I know, there is no any BlockIo producers bouncing I/O. Just DiskIo producers have such behavior.
Jeremy may mix them up. -----Original Message----- From: Laszlo Ersek [mailto:ler...@redhat.com] Sent: Monday, February 15, 2016 19:46 To: Tian, Feng; Ryan Harkin; Jeremy Linton Cc: edk2-devel@lists.01.org; Leif Lindholm Subject: Re: [edk2] [PATCH] MdeModulePkg, AtaBusDxe: Bounce buffer IO operations if unaligned On 02/15/16 03:18, Tian, Feng wrote: > 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." Not wishing to influence the discussion, just out of curiosity: Jeremy mentions "numerous other BlockIo protocol providers in edk2 bounce IO operations rather than simply allowing them to fail" -- can we see some examples? I wonder if, upon seeing that code, we could use "git blame" to find out *why* those workarounds had been introduced. Thanks Laszlo > > 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/MdeModu > lePkg/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/MdeModu > lePkg/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/ref > s/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 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
