| Message ID | 1490015485-53685-21-git-send-email-chenhui.sun@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | D02/D03 platforms bug fix | expand |
On Mon, Mar 20, 2017 at 09:11:24PM +0800, Chenhui Sun wrote: > EFI_BLOCK_IO_PROTOCOL.ReadBlocks - ReadBlocks() returns valid parameter More description please. What problem does this patch solve? > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: wangljjun <wanglijun@huawei.com> > Signed-off-by: Heyi Guo <heyi.guo@linaro.org> > Signed-off-by: Yi Li <phoenix.liyi@huawei.com> > --- > Chips/Hisilicon/Drivers/FlashFvbDxe/FlashBlockIoDxe.c | 12 ++++++++++-- > Chips/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.c | 2 +- > Drivers/Block/ramdisk/ramdisk.c | 18 +++++++++++------- > 3 files changed, 22 insertions(+), 10 deletions(-) > > diff --git a/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashBlockIoDxe.c b/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashBlockIoDxe.c > index 91c0733..5d5f3fa 100644 > --- a/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashBlockIoDxe.c > +++ b/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashBlockIoDxe.c > @@ -29,10 +29,14 @@ FlashBlockIoReadBlocks ( > OUT VOID* Buffer > ) > { > - FLASH_INSTANCE* Instance; > - EFI_STATUS Status; > + FLASH_INSTANCE* Instance; > + EFI_STATUS Status; > + EFI_BLOCK_IO_MEDIA *Media; > + UINTN IoAlign; > > Instance = INSTANCE_FROM_BLKIO_THIS(This); > + Media = This->Media; > + IoAlign = Media->IoAlign; > > DEBUG ((EFI_D_INFO, "FlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer)); > > @@ -44,6 +48,10 @@ FlashBlockIoReadBlocks ( > { > Status = EFI_MEDIA_CHANGED; > } > + else if (IoAlign > 0 && (((UINTN) Buffer & (IoAlign - 1)) != 0)) > + { Existing code is incorrectly formatted. No need to fix for this patch (I should have caught that on first review), but please ensure lines you add or modify are of form: if () { } else if () { } else { } > + Status = EFI_INVALID_PARAMETER; > + } > else > { > Status = FlashReadBlocks (Instance, Lba, BufferSizeInBytes, Buffer); > diff --git a/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.c b/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.c > index d118a90..c719d53 100644 > --- a/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.c > +++ b/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.c > @@ -1125,7 +1125,7 @@ FlashReadBlocks ( > // We must have some bytes to read > if (BufferSizeInBytes == 0) > { > - return EFI_BAD_BUFFER_SIZE; > + return EFI_SUCCESS; Is this to make a read of size 0 succeed? > } > > // The size of the buffer must be a multiple of the block size > diff --git a/Drivers/Block/ramdisk/ramdisk.c b/Drivers/Block/ramdisk/ramdisk.c > index bd2607d..14c2f0e 100644 > --- a/Drivers/Block/ramdisk/ramdisk.c > +++ b/Drivers/Block/ramdisk/ramdisk.c > @@ -481,14 +481,18 @@ STATIC EFI_STATUS RamDiskReadBlocks( > > Media = This->Media; > > - if(BufferSize % Media->BlockSize != 0) In this situation, I would prefer if you could leave this line badly formatted. The diff is made much more difficult to read by your correction of the coding style. > + if (Media->MediaId != MediaId) { > + return EFI_MEDIA_CHANGED; > + } > + if (BufferSize % Media->BlockSize != 0) { > return EFI_BAD_BUFFER_SIZE; > - > - if(LBA > Media->LastBlock) > - return EFI_DEVICE_ERROR; > - > - if(LBA + BufferSize / Media->BlockSize - 1 > Media->LastBlock) > - return EFI_DEVICE_ERROR; > + } > + if (BufferSize == 0) { > + return EFI_SUCCESS; > + } > + if (LBA + BufferSize / Media->BlockSize - 1 > Media->LastBlock) { > + return EFI_INVALID_PARAMETER; > + } > > RamDiskDev = RAM_DISK_FROM_THIS(This); > RamDiskLBA = RamDiskDev->Start + MultU64x32(LBA,Media->BlockSize); > -- > 1.9.1 >
diff --git a/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashBlockIoDxe.c b/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashBlockIoDxe.c index 91c0733..5d5f3fa 100644 --- a/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashBlockIoDxe.c +++ b/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashBlockIoDxe.c @@ -29,10 +29,14 @@ FlashBlockIoReadBlocks ( OUT VOID* Buffer ) { - FLASH_INSTANCE* Instance; - EFI_STATUS Status; + FLASH_INSTANCE* Instance; + EFI_STATUS Status; + EFI_BLOCK_IO_MEDIA *Media; + UINTN IoAlign; Instance = INSTANCE_FROM_BLKIO_THIS(This); + Media = This->Media; + IoAlign = Media->IoAlign; DEBUG ((EFI_D_INFO, "FlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer)); @@ -44,6 +48,10 @@ FlashBlockIoReadBlocks ( { Status = EFI_MEDIA_CHANGED; } + else if (IoAlign > 0 && (((UINTN) Buffer & (IoAlign - 1)) != 0)) + { + Status = EFI_INVALID_PARAMETER; + } else { Status = FlashReadBlocks (Instance, Lba, BufferSizeInBytes, Buffer); diff --git a/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.c b/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.c index d118a90..c719d53 100644 --- a/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.c +++ b/Chips/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.c @@ -1125,7 +1125,7 @@ FlashReadBlocks ( // We must have some bytes to read if (BufferSizeInBytes == 0) { - return EFI_BAD_BUFFER_SIZE; + return EFI_SUCCESS; } // The size of the buffer must be a multiple of the block size diff --git a/Drivers/Block/ramdisk/ramdisk.c b/Drivers/Block/ramdisk/ramdisk.c index bd2607d..14c2f0e 100644 --- a/Drivers/Block/ramdisk/ramdisk.c +++ b/Drivers/Block/ramdisk/ramdisk.c @@ -481,14 +481,18 @@ STATIC EFI_STATUS RamDiskReadBlocks( Media = This->Media; - if(BufferSize % Media->BlockSize != 0) + if (Media->MediaId != MediaId) { + return EFI_MEDIA_CHANGED; + } + if (BufferSize % Media->BlockSize != 0) { return EFI_BAD_BUFFER_SIZE; - - if(LBA > Media->LastBlock) - return EFI_DEVICE_ERROR; - - if(LBA + BufferSize / Media->BlockSize - 1 > Media->LastBlock) - return EFI_DEVICE_ERROR; + } + if (BufferSize == 0) { + return EFI_SUCCESS; + } + if (LBA + BufferSize / Media->BlockSize - 1 > Media->LastBlock) { + return EFI_INVALID_PARAMETER; + } RamDiskDev = RAM_DISK_FROM_THIS(This); RamDiskLBA = RamDiskDev->Start + MultU64x32(LBA,Media->BlockSize);