* [edk2-devel] [PATCH] MdeModulePkg: AllocatePages for TranslateBmpToGopBlt @ 2023-07-21 12:12 chitralekha ck 2023-07-21 17:18 ` Ard Biesheuvel 2023-07-24 1:05 ` 回复: " gaoliming via groups.io 0 siblings, 2 replies; 5+ messages in thread From: chitralekha ck @ 2023-07-21 12:12 UTC (permalink / raw) To: devel; +Cc: chitralekha ck, Ray Ni, Zhichao Gao, Ashraf Ali S, Chinni B Duggapu https://bugzilla.tianocore.org/show_bug.cgi?id=4507 AllocatePool limits to allocate memory of 64 KB at most. if the the image size is higher than that it will fail to allocate. change the function debug string to __func__ Cc: Ray Ni <ray.ni@intel.com> Cc: Zhichao Gao <zhichao.gao@intel.com> Cc: Ashraf Ali S <ashraf.ali.s@intel.com> Cc: Chinni B Duggapu <chinni.b.duggapu@intel.com> Signed-off-by: chitralekha ck <chitralekha.ck@intel.com> --- .../Library/BaseBmpSupportLib/BmpSupportLib.c | 58 +++++++++++-------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c b/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c index c5e885d7a6..d0833a721f 100644 --- a/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c +++ b/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c @@ -52,7 +52,7 @@ const BMP_IMAGE_HEADER mBmpImageHeaderTemplate = { /** Translate a *.BMP graphics image to a GOP blt buffer. If a NULL Blt buffer is passed in a GopBlt buffer will be allocated by this routine using - EFI_BOOT_SERVICES.AllocatePool(). If a GopBlt buffer is passed in it will be + EFI_BOOT_SERVICES.AllocatePages(). If a GopBlt buffer is passed in it will be used if it is big enough. @param[in] BmpImage Pointer to BMP file. @@ -113,14 +113,14 @@ TranslateBmpToGopBlt ( } if (BmpImageSize < sizeof (BMP_IMAGE_HEADER)) { - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: BmpImageSize too small\n")); + DEBUG ((DEBUG_ERROR, "%a: BmpImageSize too small\n", __func__)); return RETURN_UNSUPPORTED; } BmpHeader = (BMP_IMAGE_HEADER *)BmpImage; if ((BmpHeader->CharB != 'B') || (BmpHeader->CharM != 'M')) { - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: BmpHeader->Char fields incorrect\n")); + DEBUG ((DEBUG_ERROR, "%a: BmpHeader->Char fields incorrect\n", __func__)); return RETURN_UNSUPPORTED; } @@ -128,12 +128,12 @@ TranslateBmpToGopBlt ( // Doesn't support compress. // if (BmpHeader->CompressionType != 0) { - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: Compression Type unsupported.\n")); + DEBUG ((DEBUG_ERROR, "%a: Compression Type unsupported\n", __func__)); return RETURN_UNSUPPORTED; } if ((BmpHeader->PixelHeight == 0) || (BmpHeader->PixelWidth == 0)) { - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: BmpHeader->PixelHeight or BmpHeader->PixelWidth is 0.\n")); + DEBUG ((DEBUG_ERROR, "%a: BmpHeader PixelHeight or PixelWidth is 0\n", __func__)); return RETURN_UNSUPPORTED; } @@ -144,7 +144,8 @@ TranslateBmpToGopBlt ( if (BmpHeader->HeaderSize != sizeof (BMP_IMAGE_HEADER) - OFFSET_OF (BMP_IMAGE_HEADER, HeaderSize)) { DEBUG (( DEBUG_ERROR, - "TranslateBmpToGopBlt: BmpHeader->Headership is not as expected. Headersize is 0x%x\n", + "%a: BmpHeader->Headership is not as expected. Headersize is 0x%x\n", + __func__, BmpHeader->HeaderSize )); return RETURN_UNSUPPORTED; @@ -161,7 +162,8 @@ TranslateBmpToGopBlt ( if (EFI_ERROR (Status)) { DEBUG (( DEBUG_ERROR, - "TranslateBmpToGopBlt: invalid BmpImage... PixelWidth:0x%x BitPerPixel:0x%x\n", + "%a: invalid BmpImage. PixelWidth:0x%x BitPerPixel:0x%x\n", + __func__, BmpHeader->PixelWidth, BmpHeader->BitPerPixel )); @@ -172,7 +174,8 @@ TranslateBmpToGopBlt ( if (EFI_ERROR (Status)) { DEBUG (( DEBUG_ERROR, - "TranslateBmpToGopBlt: invalid BmpImage... DataSizePerLine:0x%x\n", + "%a: invalid BmpImage. DataSizePerLine:0x%x\n", + __func__, DataSizePerLine )); @@ -189,7 +192,8 @@ TranslateBmpToGopBlt ( if (EFI_ERROR (Status)) { DEBUG (( DEBUG_ERROR, - "TranslateBmpToGopBlt: invalid BmpImage... DataSizePerLine:0x%x PixelHeight:0x%x\n", + "%a: invalid BmpImage. DataSizePerLine:0x%x PixelHeight:0x%x\n", + __func__, DataSizePerLine, BmpHeader->PixelHeight )); @@ -206,7 +210,8 @@ TranslateBmpToGopBlt ( if (EFI_ERROR (Status)) { DEBUG (( DEBUG_ERROR, - "TranslateBmpToGopBlt: invalid BmpImage... PixelHeight:0x%x DataSizePerLine:0x%x\n", + "%a: invalid BmpImage. PixelHeight:0x%x DataSizePerLine:0x%x\n", + __func__, BmpHeader->PixelHeight, DataSizePerLine )); @@ -218,11 +223,11 @@ TranslateBmpToGopBlt ( (BmpHeader->Size < BmpHeader->ImageOffset) || (BmpHeader->Size - BmpHeader->ImageOffset != DataSize)) { - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: invalid BmpImage... \n")); - DEBUG ((DEBUG_ERROR, " BmpHeader->Size: 0x%x\n", BmpHeader->Size)); - DEBUG ((DEBUG_ERROR, " BmpHeader->ImageOffset: 0x%x\n", BmpHeader->ImageOffset)); - DEBUG ((DEBUG_ERROR, " BmpImageSize: 0x%lx\n", (UINTN)BmpImageSize)); - DEBUG ((DEBUG_ERROR, " DataSize: 0x%lx\n", (UINTN)DataSize)); + DEBUG ((DEBUG_ERROR, "%a: invalid BmpImage... \n", __func__)); + DEBUG ((DEBUG_ERROR, " Size: 0x%x\n", BmpHeader->Size)); + DEBUG ((DEBUG_ERROR, " ImageOffset: 0x%x\n", BmpHeader->ImageOffset)); + DEBUG ((DEBUG_ERROR, " BmpImageSize: 0x%lx\n", (UINTN)BmpImageSize)); + DEBUG ((DEBUG_ERROR, " DataSize: 0x%lx\n", (UINTN)DataSize)); return RETURN_UNSUPPORTED; } @@ -279,7 +284,8 @@ TranslateBmpToGopBlt ( if (EFI_ERROR (Status)) { DEBUG (( DEBUG_ERROR, - "TranslateBmpToGopBlt: invalid BltBuffer needed size... PixelWidth:0x%x PixelHeight:0x%x\n", + "%a: invalid BltBuffer needed size. PixelWidth:0x%x PixelHeight:0x%x\n", + __func__, BmpHeader->PixelWidth, BmpHeader->PixelHeight )); @@ -297,7 +303,8 @@ TranslateBmpToGopBlt ( if (EFI_ERROR (Status)) { DEBUG (( DEBUG_ERROR, - "TranslateBmpToGopBlt: invalid BltBuffer needed size... PixelWidth x PixelHeight:0x%x struct size:0x%x\n", + "%a: invalid BltBuffer needed size. PixelWidth x PixelHeight:0x%x struct size:0x%x\n", + __func__, Temp, sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL) )); @@ -312,7 +319,7 @@ TranslateBmpToGopBlt ( // DEBUG ((DEBUG_INFO, "Bmp Support: Allocating 0x%X bytes of memory\n", BltBufferSize)); *GopBltSize = (UINTN)BltBufferSize; - *GopBlt = AllocatePool (*GopBltSize); + *GopBlt = AllocatePages (*GopBltSize); IsAllocated = TRUE; if (*GopBlt == NULL) { return RETURN_OUT_OF_RESOURCES; @@ -329,13 +336,14 @@ TranslateBmpToGopBlt ( *PixelWidth = BmpHeader->PixelWidth; *PixelHeight = BmpHeader->PixelHeight; - DEBUG ((DEBUG_INFO, "BmpHeader->ImageOffset 0x%X\n", BmpHeader->ImageOffset)); - DEBUG ((DEBUG_INFO, "BmpHeader->PixelWidth 0x%X\n", BmpHeader->PixelWidth)); - DEBUG ((DEBUG_INFO, "BmpHeader->PixelHeight 0x%X\n", BmpHeader->PixelHeight)); - DEBUG ((DEBUG_INFO, "BmpHeader->BitPerPixel 0x%X\n", BmpHeader->BitPerPixel)); - DEBUG ((DEBUG_INFO, "BmpHeader->ImageSize 0x%X\n", BmpHeader->ImageSize)); - DEBUG ((DEBUG_INFO, "BmpHeader->HeaderSize 0x%X\n", BmpHeader->HeaderSize)); - DEBUG ((DEBUG_INFO, "BmpHeader->Size 0x%X\n", BmpHeader->Size)); + DEBUG ((DEBUG_INFO, "BmpHeader :\n")); + DEBUG ((DEBUG_INFO, " ImageOffset 0x%X\n", BmpHeader->ImageOffset)); + DEBUG ((DEBUG_INFO, " PixelWidth 0x%X\n", BmpHeader->PixelWidth)); + DEBUG ((DEBUG_INFO, " PixelHeight 0x%X\n", BmpHeader->PixelHeight)); + DEBUG ((DEBUG_INFO, " BitPerPixel 0x%X\n", BmpHeader->BitPerPixel)); + DEBUG ((DEBUG_INFO, " ImageSize 0x%X\n", BmpHeader->ImageSize)); + DEBUG ((DEBUG_INFO, " HeaderSize 0x%X\n", BmpHeader->HeaderSize)); + DEBUG ((DEBUG_INFO, " Size 0x%X\n", BmpHeader->Size)); // // Translate image from BMP to Blt buffer format -- 2.38.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107128): https://edk2.groups.io/g/devel/message/107128 Mute This Topic: https://groups.io/mt/100278877/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg: AllocatePages for TranslateBmpToGopBlt 2023-07-21 12:12 [edk2-devel] [PATCH] MdeModulePkg: AllocatePages for TranslateBmpToGopBlt chitralekha ck @ 2023-07-21 17:18 ` Ard Biesheuvel 2023-07-24 4:56 ` Ashraf Ali S 2023-07-24 1:05 ` 回复: " gaoliming via groups.io 1 sibling, 1 reply; 5+ messages in thread From: Ard Biesheuvel @ 2023-07-21 17:18 UTC (permalink / raw) To: devel, chitralekha.ck; +Cc: Ray Ni, Zhichao Gao, Ashraf Ali S, Chinni B Duggapu On Fri, 21 Jul 2023 at 17:38, chitralekha ck <chitralekha.ck@intel.com> wrote: > > https://bugzilla.tianocore.org/show_bug.cgi?id=4507 > AllocatePool limits to allocate memory of 64 KB at most. What is the basis for this observation? In DXE, pool allocations are unbounded afaik. > if the the image size is higher than that it will fail to allocate. > change the function debug string to __func__ > Please don't mix unrelated changes in the same patch. > Cc: Ray Ni <ray.ni@intel.com> > Cc: Zhichao Gao <zhichao.gao@intel.com> > Cc: Ashraf Ali S <ashraf.ali.s@intel.com> > Cc: Chinni B Duggapu <chinni.b.duggapu@intel.com> > Signed-off-by: chitralekha ck <chitralekha.ck@intel.com> > --- > .../Library/BaseBmpSupportLib/BmpSupportLib.c | 58 +++++++++++-------- > 1 file changed, 33 insertions(+), 25 deletions(-) > > diff --git a/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c b/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > index c5e885d7a6..d0833a721f 100644 > --- a/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > +++ b/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > @@ -52,7 +52,7 @@ const BMP_IMAGE_HEADER mBmpImageHeaderTemplate = { > /** > Translate a *.BMP graphics image to a GOP blt buffer. If a NULL Blt buffer > is passed in a GopBlt buffer will be allocated by this routine using > - EFI_BOOT_SERVICES.AllocatePool(). If a GopBlt buffer is passed in it will be > + EFI_BOOT_SERVICES.AllocatePages(). If a GopBlt buffer is passed in it will be > used if it is big enough. > > @param[in] BmpImage Pointer to BMP file. > @@ -113,14 +113,14 @@ TranslateBmpToGopBlt ( > } > > if (BmpImageSize < sizeof (BMP_IMAGE_HEADER)) { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: BmpImageSize too small\n")); > + DEBUG ((DEBUG_ERROR, "%a: BmpImageSize too small\n", __func__)); > return RETURN_UNSUPPORTED; > } > > BmpHeader = (BMP_IMAGE_HEADER *)BmpImage; > > if ((BmpHeader->CharB != 'B') || (BmpHeader->CharM != 'M')) { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: BmpHeader->Char fields incorrect\n")); > + DEBUG ((DEBUG_ERROR, "%a: BmpHeader->Char fields incorrect\n", __func__)); > return RETURN_UNSUPPORTED; > } > > @@ -128,12 +128,12 @@ TranslateBmpToGopBlt ( > // Doesn't support compress. > // > if (BmpHeader->CompressionType != 0) { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: Compression Type unsupported.\n")); > + DEBUG ((DEBUG_ERROR, "%a: Compression Type unsupported\n", __func__)); > return RETURN_UNSUPPORTED; > } > > if ((BmpHeader->PixelHeight == 0) || (BmpHeader->PixelWidth == 0)) { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: BmpHeader->PixelHeight or BmpHeader->PixelWidth is 0.\n")); > + DEBUG ((DEBUG_ERROR, "%a: BmpHeader PixelHeight or PixelWidth is 0\n", __func__)); > return RETURN_UNSUPPORTED; > } > > @@ -144,7 +144,8 @@ TranslateBmpToGopBlt ( > if (BmpHeader->HeaderSize != sizeof (BMP_IMAGE_HEADER) - OFFSET_OF (BMP_IMAGE_HEADER, HeaderSize)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: BmpHeader->Headership is not as expected. Headersize is 0x%x\n", > + "%a: BmpHeader->Headership is not as expected. Headersize is 0x%x\n", > + __func__, > BmpHeader->HeaderSize > )); > return RETURN_UNSUPPORTED; > @@ -161,7 +162,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BmpImage... PixelWidth:0x%x BitPerPixel:0x%x\n", > + "%a: invalid BmpImage. PixelWidth:0x%x BitPerPixel:0x%x\n", > + __func__, > BmpHeader->PixelWidth, > BmpHeader->BitPerPixel > )); > @@ -172,7 +174,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BmpImage... DataSizePerLine:0x%x\n", > + "%a: invalid BmpImage. DataSizePerLine:0x%x\n", > + __func__, > DataSizePerLine > )); > > @@ -189,7 +192,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BmpImage... DataSizePerLine:0x%x PixelHeight:0x%x\n", > + "%a: invalid BmpImage. DataSizePerLine:0x%x PixelHeight:0x%x\n", > + __func__, > DataSizePerLine, > BmpHeader->PixelHeight > )); > @@ -206,7 +210,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BmpImage... PixelHeight:0x%x DataSizePerLine:0x%x\n", > + "%a: invalid BmpImage. PixelHeight:0x%x DataSizePerLine:0x%x\n", > + __func__, > BmpHeader->PixelHeight, > DataSizePerLine > )); > @@ -218,11 +223,11 @@ TranslateBmpToGopBlt ( > (BmpHeader->Size < BmpHeader->ImageOffset) || > (BmpHeader->Size - BmpHeader->ImageOffset != DataSize)) > { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: invalid BmpImage... \n")); > - DEBUG ((DEBUG_ERROR, " BmpHeader->Size: 0x%x\n", BmpHeader->Size)); > - DEBUG ((DEBUG_ERROR, " BmpHeader->ImageOffset: 0x%x\n", BmpHeader->ImageOffset)); > - DEBUG ((DEBUG_ERROR, " BmpImageSize: 0x%lx\n", (UINTN)BmpImageSize)); > - DEBUG ((DEBUG_ERROR, " DataSize: 0x%lx\n", (UINTN)DataSize)); > + DEBUG ((DEBUG_ERROR, "%a: invalid BmpImage... \n", __func__)); > + DEBUG ((DEBUG_ERROR, " Size: 0x%x\n", BmpHeader->Size)); > + DEBUG ((DEBUG_ERROR, " ImageOffset: 0x%x\n", BmpHeader->ImageOffset)); > + DEBUG ((DEBUG_ERROR, " BmpImageSize: 0x%lx\n", (UINTN)BmpImageSize)); > + DEBUG ((DEBUG_ERROR, " DataSize: 0x%lx\n", (UINTN)DataSize)); > > return RETURN_UNSUPPORTED; > } > @@ -279,7 +284,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BltBuffer needed size... PixelWidth:0x%x PixelHeight:0x%x\n", > + "%a: invalid BltBuffer needed size. PixelWidth:0x%x PixelHeight:0x%x\n", > + __func__, > BmpHeader->PixelWidth, > BmpHeader->PixelHeight > )); > @@ -297,7 +303,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BltBuffer needed size... PixelWidth x PixelHeight:0x%x struct size:0x%x\n", > + "%a: invalid BltBuffer needed size. PixelWidth x PixelHeight:0x%x struct size:0x%x\n", > + __func__, > Temp, > sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL) > )); > @@ -312,7 +319,7 @@ TranslateBmpToGopBlt ( > // > DEBUG ((DEBUG_INFO, "Bmp Support: Allocating 0x%X bytes of memory\n", BltBufferSize)); > *GopBltSize = (UINTN)BltBufferSize; > - *GopBlt = AllocatePool (*GopBltSize); > + *GopBlt = AllocatePages (*GopBltSize); > IsAllocated = TRUE; > if (*GopBlt == NULL) { > return RETURN_OUT_OF_RESOURCES; > @@ -329,13 +336,14 @@ TranslateBmpToGopBlt ( > > *PixelWidth = BmpHeader->PixelWidth; > *PixelHeight = BmpHeader->PixelHeight; > - DEBUG ((DEBUG_INFO, "BmpHeader->ImageOffset 0x%X\n", BmpHeader->ImageOffset)); > - DEBUG ((DEBUG_INFO, "BmpHeader->PixelWidth 0x%X\n", BmpHeader->PixelWidth)); > - DEBUG ((DEBUG_INFO, "BmpHeader->PixelHeight 0x%X\n", BmpHeader->PixelHeight)); > - DEBUG ((DEBUG_INFO, "BmpHeader->BitPerPixel 0x%X\n", BmpHeader->BitPerPixel)); > - DEBUG ((DEBUG_INFO, "BmpHeader->ImageSize 0x%X\n", BmpHeader->ImageSize)); > - DEBUG ((DEBUG_INFO, "BmpHeader->HeaderSize 0x%X\n", BmpHeader->HeaderSize)); > - DEBUG ((DEBUG_INFO, "BmpHeader->Size 0x%X\n", BmpHeader->Size)); > + DEBUG ((DEBUG_INFO, "BmpHeader :\n")); > + DEBUG ((DEBUG_INFO, " ImageOffset 0x%X\n", BmpHeader->ImageOffset)); > + DEBUG ((DEBUG_INFO, " PixelWidth 0x%X\n", BmpHeader->PixelWidth)); > + DEBUG ((DEBUG_INFO, " PixelHeight 0x%X\n", BmpHeader->PixelHeight)); > + DEBUG ((DEBUG_INFO, " BitPerPixel 0x%X\n", BmpHeader->BitPerPixel)); > + DEBUG ((DEBUG_INFO, " ImageSize 0x%X\n", BmpHeader->ImageSize)); > + DEBUG ((DEBUG_INFO, " HeaderSize 0x%X\n", BmpHeader->HeaderSize)); > + DEBUG ((DEBUG_INFO, " Size 0x%X\n", BmpHeader->Size)); > > // > // Translate image from BMP to Blt buffer format > -- > 2.38.1.windows.1 > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107133): https://edk2.groups.io/g/devel/message/107133 Mute This Topic: https://groups.io/mt/100278877/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg: AllocatePages for TranslateBmpToGopBlt 2023-07-21 17:18 ` Ard Biesheuvel @ 2023-07-24 4:56 ` Ashraf Ali S 2023-07-24 7:33 ` Ard Biesheuvel 0 siblings, 1 reply; 5+ messages in thread From: Ashraf Ali S @ 2023-07-24 4:56 UTC (permalink / raw) To: devel@edk2.groups.io, ardb@kernel.org, Ck, Chitralekha Cc: Ni, Ray, Gao, Zhichao, Duggapu, Chinni B Hi., The observation is based on if the Library consumed in the PEI Phase (PostMem). Thanks., S, Ashraf Ali -----Original Message----- From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel Sent: Friday, July 21, 2023 10:49 PM To: devel@edk2.groups.io; Ck, Chitralekha <chitralekha.ck@intel.com> Cc: Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg: AllocatePages for TranslateBmpToGopBlt On Fri, 21 Jul 2023 at 17:38, chitralekha ck <chitralekha.ck@intel.com> wrote: > > https://bugzilla.tianocore.org/show_bug.cgi?id=4507 > AllocatePool limits to allocate memory of 64 KB at most. What is the basis for this observation? In DXE, pool allocations are unbounded afaik. > if the the image size is higher than that it will fail to allocate. > change the function debug string to __func__ > Please don't mix unrelated changes in the same patch. > Cc: Ray Ni <ray.ni@intel.com> > Cc: Zhichao Gao <zhichao.gao@intel.com> > Cc: Ashraf Ali S <ashraf.ali.s@intel.com> > Cc: Chinni B Duggapu <chinni.b.duggapu@intel.com> > Signed-off-by: chitralekha ck <chitralekha.ck@intel.com> > --- > .../Library/BaseBmpSupportLib/BmpSupportLib.c | 58 > +++++++++++-------- > 1 file changed, 33 insertions(+), 25 deletions(-) > > diff --git a/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > b/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > index c5e885d7a6..d0833a721f 100644 > --- a/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > +++ b/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > @@ -52,7 +52,7 @@ const BMP_IMAGE_HEADER mBmpImageHeaderTemplate = { > /** > Translate a *.BMP graphics image to a GOP blt buffer. If a NULL Blt buffer > is passed in a GopBlt buffer will be allocated by this routine > using > - EFI_BOOT_SERVICES.AllocatePool(). If a GopBlt buffer is passed in > it will be > + EFI_BOOT_SERVICES.AllocatePages(). If a GopBlt buffer is passed in > + it will be > used if it is big enough. > > @param[in] BmpImage Pointer to BMP file. > @@ -113,14 +113,14 @@ TranslateBmpToGopBlt ( > } > > if (BmpImageSize < sizeof (BMP_IMAGE_HEADER)) { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: BmpImageSize too small\n")); > + DEBUG ((DEBUG_ERROR, "%a: BmpImageSize too small\n", __func__)); > return RETURN_UNSUPPORTED; > } > > BmpHeader = (BMP_IMAGE_HEADER *)BmpImage; > > if ((BmpHeader->CharB != 'B') || (BmpHeader->CharM != 'M')) { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: BmpHeader->Char fields incorrect\n")); > + DEBUG ((DEBUG_ERROR, "%a: BmpHeader->Char fields incorrect\n", > + __func__)); > return RETURN_UNSUPPORTED; > } > > @@ -128,12 +128,12 @@ TranslateBmpToGopBlt ( > // Doesn't support compress. > // > if (BmpHeader->CompressionType != 0) { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: Compression Type unsupported.\n")); > + DEBUG ((DEBUG_ERROR, "%a: Compression Type unsupported\n", > + __func__)); > return RETURN_UNSUPPORTED; > } > > if ((BmpHeader->PixelHeight == 0) || (BmpHeader->PixelWidth == 0)) { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: BmpHeader->PixelHeight or BmpHeader->PixelWidth is 0.\n")); > + DEBUG ((DEBUG_ERROR, "%a: BmpHeader PixelHeight or PixelWidth is > + 0\n", __func__)); > return RETURN_UNSUPPORTED; > } > > @@ -144,7 +144,8 @@ TranslateBmpToGopBlt ( > if (BmpHeader->HeaderSize != sizeof (BMP_IMAGE_HEADER) - OFFSET_OF (BMP_IMAGE_HEADER, HeaderSize)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: BmpHeader->Headership is not as expected. Headersize is 0x%x\n", > + "%a: BmpHeader->Headership is not as expected. Headersize is 0x%x\n", > + __func__, > BmpHeader->HeaderSize > )); > return RETURN_UNSUPPORTED; > @@ -161,7 +162,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BmpImage... PixelWidth:0x%x BitPerPixel:0x%x\n", > + "%a: invalid BmpImage. PixelWidth:0x%x BitPerPixel:0x%x\n", > + __func__, > BmpHeader->PixelWidth, > BmpHeader->BitPerPixel > )); > @@ -172,7 +174,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BmpImage... DataSizePerLine:0x%x\n", > + "%a: invalid BmpImage. DataSizePerLine:0x%x\n", > + __func__, > DataSizePerLine > )); > > @@ -189,7 +192,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BmpImage... DataSizePerLine:0x%x PixelHeight:0x%x\n", > + "%a: invalid BmpImage. DataSizePerLine:0x%x PixelHeight:0x%x\n", > + __func__, > DataSizePerLine, > BmpHeader->PixelHeight > )); > @@ -206,7 +210,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BmpImage... PixelHeight:0x%x DataSizePerLine:0x%x\n", > + "%a: invalid BmpImage. PixelHeight:0x%x DataSizePerLine:0x%x\n", > + __func__, > BmpHeader->PixelHeight, > DataSizePerLine > )); > @@ -218,11 +223,11 @@ TranslateBmpToGopBlt ( > (BmpHeader->Size < BmpHeader->ImageOffset) || > (BmpHeader->Size - BmpHeader->ImageOffset != DataSize)) > { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: invalid BmpImage... \n")); > - DEBUG ((DEBUG_ERROR, " BmpHeader->Size: 0x%x\n", BmpHeader->Size)); > - DEBUG ((DEBUG_ERROR, " BmpHeader->ImageOffset: 0x%x\n", BmpHeader->ImageOffset)); > - DEBUG ((DEBUG_ERROR, " BmpImageSize: 0x%lx\n", (UINTN)BmpImageSize)); > - DEBUG ((DEBUG_ERROR, " DataSize: 0x%lx\n", (UINTN)DataSize)); > + DEBUG ((DEBUG_ERROR, "%a: invalid BmpImage... \n", __func__)); > + DEBUG ((DEBUG_ERROR, " Size: 0x%x\n", BmpHeader->Size)); > + DEBUG ((DEBUG_ERROR, " ImageOffset: 0x%x\n", BmpHeader->ImageOffset)); > + DEBUG ((DEBUG_ERROR, " BmpImageSize: 0x%lx\n", (UINTN)BmpImageSize)); > + DEBUG ((DEBUG_ERROR, " DataSize: 0x%lx\n", (UINTN)DataSize)); > > return RETURN_UNSUPPORTED; > } > @@ -279,7 +284,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BltBuffer needed size... PixelWidth:0x%x PixelHeight:0x%x\n", > + "%a: invalid BltBuffer needed size. PixelWidth:0x%x PixelHeight:0x%x\n", > + __func__, > BmpHeader->PixelWidth, > BmpHeader->PixelHeight > )); > @@ -297,7 +303,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BltBuffer needed size... PixelWidth x PixelHeight:0x%x struct size:0x%x\n", > + "%a: invalid BltBuffer needed size. PixelWidth x PixelHeight:0x%x struct size:0x%x\n", > + __func__, > Temp, > sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL) > )); > @@ -312,7 +319,7 @@ TranslateBmpToGopBlt ( > // > DEBUG ((DEBUG_INFO, "Bmp Support: Allocating 0x%X bytes of memory\n", BltBufferSize)); > *GopBltSize = (UINTN)BltBufferSize; > - *GopBlt = AllocatePool (*GopBltSize); > + *GopBlt = AllocatePages (*GopBltSize); > IsAllocated = TRUE; > if (*GopBlt == NULL) { > return RETURN_OUT_OF_RESOURCES; @@ -329,13 +336,14 @@ > TranslateBmpToGopBlt ( > > *PixelWidth = BmpHeader->PixelWidth; > *PixelHeight = BmpHeader->PixelHeight; > - DEBUG ((DEBUG_INFO, "BmpHeader->ImageOffset 0x%X\n", > BmpHeader->ImageOffset)); > - DEBUG ((DEBUG_INFO, "BmpHeader->PixelWidth 0x%X\n", > BmpHeader->PixelWidth)); > - DEBUG ((DEBUG_INFO, "BmpHeader->PixelHeight 0x%X\n", > BmpHeader->PixelHeight)); > - DEBUG ((DEBUG_INFO, "BmpHeader->BitPerPixel 0x%X\n", > BmpHeader->BitPerPixel)); > - DEBUG ((DEBUG_INFO, "BmpHeader->ImageSize 0x%X\n", > BmpHeader->ImageSize)); > - DEBUG ((DEBUG_INFO, "BmpHeader->HeaderSize 0x%X\n", > BmpHeader->HeaderSize)); > - DEBUG ((DEBUG_INFO, "BmpHeader->Size 0x%X\n", BmpHeader->Size)); > + DEBUG ((DEBUG_INFO, "BmpHeader :\n")); DEBUG ((DEBUG_INFO, " > + ImageOffset 0x%X\n", BmpHeader->ImageOffset)); DEBUG ((DEBUG_INFO, > + " PixelWidth 0x%X\n", BmpHeader->PixelWidth)); DEBUG ((DEBUG_INFO, > + " PixelHeight 0x%X\n", BmpHeader->PixelHeight)); DEBUG > + ((DEBUG_INFO, " BitPerPixel 0x%X\n", BmpHeader->BitPerPixel)); > + DEBUG ((DEBUG_INFO, " ImageSize 0x%X\n", BmpHeader->ImageSize)); > + DEBUG ((DEBUG_INFO, " HeaderSize 0x%X\n", BmpHeader->HeaderSize)); > + DEBUG ((DEBUG_INFO, " Size 0x%X\n", BmpHeader->Size)); > > // > // Translate image from BMP to Blt buffer format > -- > 2.38.1.windows.1 > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107157): https://edk2.groups.io/g/devel/message/107157 Mute This Topic: https://groups.io/mt/100278877/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg: AllocatePages for TranslateBmpToGopBlt 2023-07-24 4:56 ` Ashraf Ali S @ 2023-07-24 7:33 ` Ard Biesheuvel 0 siblings, 0 replies; 5+ messages in thread From: Ard Biesheuvel @ 2023-07-24 7:33 UTC (permalink / raw) To: S, Ashraf Ali Cc: devel@edk2.groups.io, Ck, Chitralekha, Ni, Ray, Gao, Zhichao, Duggapu, Chinni B On Mon, 24 Jul 2023 at 06:56, S, Ashraf Ali <ashraf.ali.s@intel.com> wrote: > > Hi., > > The observation is based on if the Library consumed in the PEI Phase (PostMem). > In that case, please fix the comment that refers to EFI_BOOT_SERVICES.AllocatePool/Pages() directly, and instead, update the comment to reflect that AllocatePool() is being avoided due to its 64k allocation size limit when the library is incorporated into a PEI component. Thanks, > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel > Sent: Friday, July 21, 2023 10:49 PM > To: devel@edk2.groups.io; Ck, Chitralekha <chitralekha.ck@intel.com> > Cc: Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com> > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg: AllocatePages for TranslateBmpToGopBlt > > On Fri, 21 Jul 2023 at 17:38, chitralekha ck <chitralekha.ck@intel.com> wrote: > > > > https://bugzilla.tianocore.org/show_bug.cgi?id=4507 > > AllocatePool limits to allocate memory of 64 KB at most. > > What is the basis for this observation? In DXE, pool allocations are unbounded afaik. > > > > > if the the image size is higher than that it will fail to allocate. > > change the function debug string to __func__ > > > > Please don't mix unrelated changes in the same patch. > > > Cc: Ray Ni <ray.ni@intel.com> > > Cc: Zhichao Gao <zhichao.gao@intel.com> > > Cc: Ashraf Ali S <ashraf.ali.s@intel.com> > > Cc: Chinni B Duggapu <chinni.b.duggapu@intel.com> > > Signed-off-by: chitralekha ck <chitralekha.ck@intel.com> > > --- > > .../Library/BaseBmpSupportLib/BmpSupportLib.c | 58 > > +++++++++++-------- > > 1 file changed, 33 insertions(+), 25 deletions(-) > > > > diff --git a/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > > b/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > > index c5e885d7a6..d0833a721f 100644 > > --- a/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > > +++ b/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > > @@ -52,7 +52,7 @@ const BMP_IMAGE_HEADER mBmpImageHeaderTemplate = { > > /** > > Translate a *.BMP graphics image to a GOP blt buffer. If a NULL Blt buffer > > is passed in a GopBlt buffer will be allocated by this routine > > using > > - EFI_BOOT_SERVICES.AllocatePool(). If a GopBlt buffer is passed in > > it will be > > + EFI_BOOT_SERVICES.AllocatePages(). If a GopBlt buffer is passed in > > + it will be > > used if it is big enough. > > > > @param[in] BmpImage Pointer to BMP file. > > @@ -113,14 +113,14 @@ TranslateBmpToGopBlt ( > > } > > > > if (BmpImageSize < sizeof (BMP_IMAGE_HEADER)) { > > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: BmpImageSize too small\n")); > > + DEBUG ((DEBUG_ERROR, "%a: BmpImageSize too small\n", __func__)); > > return RETURN_UNSUPPORTED; > > } > > > > BmpHeader = (BMP_IMAGE_HEADER *)BmpImage; > > > > if ((BmpHeader->CharB != 'B') || (BmpHeader->CharM != 'M')) { > > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: BmpHeader->Char fields incorrect\n")); > > + DEBUG ((DEBUG_ERROR, "%a: BmpHeader->Char fields incorrect\n", > > + __func__)); > > return RETURN_UNSUPPORTED; > > } > > > > @@ -128,12 +128,12 @@ TranslateBmpToGopBlt ( > > // Doesn't support compress. > > // > > if (BmpHeader->CompressionType != 0) { > > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: Compression Type unsupported.\n")); > > + DEBUG ((DEBUG_ERROR, "%a: Compression Type unsupported\n", > > + __func__)); > > return RETURN_UNSUPPORTED; > > } > > > > if ((BmpHeader->PixelHeight == 0) || (BmpHeader->PixelWidth == 0)) { > > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: BmpHeader->PixelHeight or BmpHeader->PixelWidth is 0.\n")); > > + DEBUG ((DEBUG_ERROR, "%a: BmpHeader PixelHeight or PixelWidth is > > + 0\n", __func__)); > > return RETURN_UNSUPPORTED; > > } > > > > @@ -144,7 +144,8 @@ TranslateBmpToGopBlt ( > > if (BmpHeader->HeaderSize != sizeof (BMP_IMAGE_HEADER) - OFFSET_OF (BMP_IMAGE_HEADER, HeaderSize)) { > > DEBUG (( > > DEBUG_ERROR, > > - "TranslateBmpToGopBlt: BmpHeader->Headership is not as expected. Headersize is 0x%x\n", > > + "%a: BmpHeader->Headership is not as expected. Headersize is 0x%x\n", > > + __func__, > > BmpHeader->HeaderSize > > )); > > return RETURN_UNSUPPORTED; > > @@ -161,7 +162,8 @@ TranslateBmpToGopBlt ( > > if (EFI_ERROR (Status)) { > > DEBUG (( > > DEBUG_ERROR, > > - "TranslateBmpToGopBlt: invalid BmpImage... PixelWidth:0x%x BitPerPixel:0x%x\n", > > + "%a: invalid BmpImage. PixelWidth:0x%x BitPerPixel:0x%x\n", > > + __func__, > > BmpHeader->PixelWidth, > > BmpHeader->BitPerPixel > > )); > > @@ -172,7 +174,8 @@ TranslateBmpToGopBlt ( > > if (EFI_ERROR (Status)) { > > DEBUG (( > > DEBUG_ERROR, > > - "TranslateBmpToGopBlt: invalid BmpImage... DataSizePerLine:0x%x\n", > > + "%a: invalid BmpImage. DataSizePerLine:0x%x\n", > > + __func__, > > DataSizePerLine > > )); > > > > @@ -189,7 +192,8 @@ TranslateBmpToGopBlt ( > > if (EFI_ERROR (Status)) { > > DEBUG (( > > DEBUG_ERROR, > > - "TranslateBmpToGopBlt: invalid BmpImage... DataSizePerLine:0x%x PixelHeight:0x%x\n", > > + "%a: invalid BmpImage. DataSizePerLine:0x%x PixelHeight:0x%x\n", > > + __func__, > > DataSizePerLine, > > BmpHeader->PixelHeight > > )); > > @@ -206,7 +210,8 @@ TranslateBmpToGopBlt ( > > if (EFI_ERROR (Status)) { > > DEBUG (( > > DEBUG_ERROR, > > - "TranslateBmpToGopBlt: invalid BmpImage... PixelHeight:0x%x DataSizePerLine:0x%x\n", > > + "%a: invalid BmpImage. PixelHeight:0x%x DataSizePerLine:0x%x\n", > > + __func__, > > BmpHeader->PixelHeight, > > DataSizePerLine > > )); > > @@ -218,11 +223,11 @@ TranslateBmpToGopBlt ( > > (BmpHeader->Size < BmpHeader->ImageOffset) || > > (BmpHeader->Size - BmpHeader->ImageOffset != DataSize)) > > { > > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: invalid BmpImage... \n")); > > - DEBUG ((DEBUG_ERROR, " BmpHeader->Size: 0x%x\n", BmpHeader->Size)); > > - DEBUG ((DEBUG_ERROR, " BmpHeader->ImageOffset: 0x%x\n", BmpHeader->ImageOffset)); > > - DEBUG ((DEBUG_ERROR, " BmpImageSize: 0x%lx\n", (UINTN)BmpImageSize)); > > - DEBUG ((DEBUG_ERROR, " DataSize: 0x%lx\n", (UINTN)DataSize)); > > + DEBUG ((DEBUG_ERROR, "%a: invalid BmpImage... \n", __func__)); > > + DEBUG ((DEBUG_ERROR, " Size: 0x%x\n", BmpHeader->Size)); > > + DEBUG ((DEBUG_ERROR, " ImageOffset: 0x%x\n", BmpHeader->ImageOffset)); > > + DEBUG ((DEBUG_ERROR, " BmpImageSize: 0x%lx\n", (UINTN)BmpImageSize)); > > + DEBUG ((DEBUG_ERROR, " DataSize: 0x%lx\n", (UINTN)DataSize)); > > > > return RETURN_UNSUPPORTED; > > } > > @@ -279,7 +284,8 @@ TranslateBmpToGopBlt ( > > if (EFI_ERROR (Status)) { > > DEBUG (( > > DEBUG_ERROR, > > - "TranslateBmpToGopBlt: invalid BltBuffer needed size... PixelWidth:0x%x PixelHeight:0x%x\n", > > + "%a: invalid BltBuffer needed size. PixelWidth:0x%x PixelHeight:0x%x\n", > > + __func__, > > BmpHeader->PixelWidth, > > BmpHeader->PixelHeight > > )); > > @@ -297,7 +303,8 @@ TranslateBmpToGopBlt ( > > if (EFI_ERROR (Status)) { > > DEBUG (( > > DEBUG_ERROR, > > - "TranslateBmpToGopBlt: invalid BltBuffer needed size... PixelWidth x PixelHeight:0x%x struct size:0x%x\n", > > + "%a: invalid BltBuffer needed size. PixelWidth x PixelHeight:0x%x struct size:0x%x\n", > > + __func__, > > Temp, > > sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL) > > )); > > @@ -312,7 +319,7 @@ TranslateBmpToGopBlt ( > > // > > DEBUG ((DEBUG_INFO, "Bmp Support: Allocating 0x%X bytes of memory\n", BltBufferSize)); > > *GopBltSize = (UINTN)BltBufferSize; > > - *GopBlt = AllocatePool (*GopBltSize); > > + *GopBlt = AllocatePages (*GopBltSize); > > IsAllocated = TRUE; > > if (*GopBlt == NULL) { > > return RETURN_OUT_OF_RESOURCES; @@ -329,13 +336,14 @@ > > TranslateBmpToGopBlt ( > > > > *PixelWidth = BmpHeader->PixelWidth; > > *PixelHeight = BmpHeader->PixelHeight; > > - DEBUG ((DEBUG_INFO, "BmpHeader->ImageOffset 0x%X\n", > > BmpHeader->ImageOffset)); > > - DEBUG ((DEBUG_INFO, "BmpHeader->PixelWidth 0x%X\n", > > BmpHeader->PixelWidth)); > > - DEBUG ((DEBUG_INFO, "BmpHeader->PixelHeight 0x%X\n", > > BmpHeader->PixelHeight)); > > - DEBUG ((DEBUG_INFO, "BmpHeader->BitPerPixel 0x%X\n", > > BmpHeader->BitPerPixel)); > > - DEBUG ((DEBUG_INFO, "BmpHeader->ImageSize 0x%X\n", > > BmpHeader->ImageSize)); > > - DEBUG ((DEBUG_INFO, "BmpHeader->HeaderSize 0x%X\n", > > BmpHeader->HeaderSize)); > > - DEBUG ((DEBUG_INFO, "BmpHeader->Size 0x%X\n", BmpHeader->Size)); > > + DEBUG ((DEBUG_INFO, "BmpHeader :\n")); DEBUG ((DEBUG_INFO, " > > + ImageOffset 0x%X\n", BmpHeader->ImageOffset)); DEBUG ((DEBUG_INFO, > > + " PixelWidth 0x%X\n", BmpHeader->PixelWidth)); DEBUG ((DEBUG_INFO, > > + " PixelHeight 0x%X\n", BmpHeader->PixelHeight)); DEBUG > > + ((DEBUG_INFO, " BitPerPixel 0x%X\n", BmpHeader->BitPerPixel)); > > + DEBUG ((DEBUG_INFO, " ImageSize 0x%X\n", BmpHeader->ImageSize)); > > + DEBUG ((DEBUG_INFO, " HeaderSize 0x%X\n", BmpHeader->HeaderSize)); > > + DEBUG ((DEBUG_INFO, " Size 0x%X\n", BmpHeader->Size)); > > > > // > > // Translate image from BMP to Blt buffer format > > -- > > 2.38.1.windows.1 > > > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107158): https://edk2.groups.io/g/devel/message/107158 Mute This Topic: https://groups.io/mt/100278877/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 5+ messages in thread
* 回复: [edk2-devel] [PATCH] MdeModulePkg: AllocatePages for TranslateBmpToGopBlt 2023-07-21 12:12 [edk2-devel] [PATCH] MdeModulePkg: AllocatePages for TranslateBmpToGopBlt chitralekha ck 2023-07-21 17:18 ` Ard Biesheuvel @ 2023-07-24 1:05 ` gaoliming via groups.io 1 sibling, 0 replies; 5+ messages in thread From: gaoliming via groups.io @ 2023-07-24 1:05 UTC (permalink / raw) To: devel, chitralekha.ck Cc: 'Ray Ni', 'Zhichao Gao', 'Ashraf Ali S', 'Chinni B Duggapu' Chitralekha: I understand this limitation is PEI version AllocatePool. Do you meet with the usage that TranslateBmpToGopBlt is consumed in PEI phase? Thanks Liming > -----邮件原件----- > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 chitralekha > ck > 发送时间: 2023年7月21日 20:13 > 收件人: devel@edk2.groups.io > 抄送: chitralekha ck <chitralekha.ck@intel.com>; Ray Ni <ray.ni@intel.com>; > Zhichao Gao <zhichao.gao@intel.com>; Ashraf Ali S <ashraf.ali.s@intel.com>; > Chinni B Duggapu <chinni.b.duggapu@intel.com> > 主题: [edk2-devel] [PATCH] MdeModulePkg: AllocatePages for > TranslateBmpToGopBlt > > https://bugzilla.tianocore.org/show_bug.cgi?id=4507 > AllocatePool limits to allocate memory of 64 KB at most. > if the the image size is higher than that it will fail to allocate. > change the function debug string to __func__ > > Cc: Ray Ni <ray.ni@intel.com> > Cc: Zhichao Gao <zhichao.gao@intel.com> > Cc: Ashraf Ali S <ashraf.ali.s@intel.com> > Cc: Chinni B Duggapu <chinni.b.duggapu@intel.com> > Signed-off-by: chitralekha ck <chitralekha.ck@intel.com> > --- > .../Library/BaseBmpSupportLib/BmpSupportLib.c | 58 +++++++++++-------- > 1 file changed, 33 insertions(+), 25 deletions(-) > > diff --git a/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > b/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > index c5e885d7a6..d0833a721f 100644 > --- a/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > +++ b/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c > @@ -52,7 +52,7 @@ const BMP_IMAGE_HEADER > mBmpImageHeaderTemplate = { > /** > Translate a *.BMP graphics image to a GOP blt buffer. If a NULL Blt buffer > is passed in a GopBlt buffer will be allocated by this routine using > - EFI_BOOT_SERVICES.AllocatePool(). If a GopBlt buffer is passed in it will be > + EFI_BOOT_SERVICES.AllocatePages(). If a GopBlt buffer is passed in it will > be > used if it is big enough. > > @param[in] BmpImage Pointer to BMP file. > @@ -113,14 +113,14 @@ TranslateBmpToGopBlt ( > } > > if (BmpImageSize < sizeof (BMP_IMAGE_HEADER)) { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: BmpImageSize too > small\n")); > + DEBUG ((DEBUG_ERROR, "%a: BmpImageSize too small\n", __func__)); > return RETURN_UNSUPPORTED; > } > > BmpHeader = (BMP_IMAGE_HEADER *)BmpImage; > > if ((BmpHeader->CharB != 'B') || (BmpHeader->CharM != 'M')) { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: BmpHeader->Char > fields incorrect\n")); > + DEBUG ((DEBUG_ERROR, "%a: BmpHeader->Char fields incorrect\n", > __func__)); > return RETURN_UNSUPPORTED; > } > > @@ -128,12 +128,12 @@ TranslateBmpToGopBlt ( > // Doesn't support compress. > // > if (BmpHeader->CompressionType != 0) { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: Compression Type > unsupported.\n")); > + DEBUG ((DEBUG_ERROR, "%a: Compression Type unsupported\n", > __func__)); > return RETURN_UNSUPPORTED; > } > > if ((BmpHeader->PixelHeight == 0) || (BmpHeader->PixelWidth == 0)) { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: > BmpHeader->PixelHeight or BmpHeader->PixelWidth is 0.\n")); > + DEBUG ((DEBUG_ERROR, "%a: BmpHeader PixelHeight or PixelWidth is > 0\n", __func__)); > return RETURN_UNSUPPORTED; > } > > @@ -144,7 +144,8 @@ TranslateBmpToGopBlt ( > if (BmpHeader->HeaderSize != sizeof (BMP_IMAGE_HEADER) - > OFFSET_OF (BMP_IMAGE_HEADER, HeaderSize)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: BmpHeader->Headership is not as expected. > Headersize is 0x%x\n", > + "%a: BmpHeader->Headership is not as expected. Headersize is > 0x%x\n", > + __func__, > BmpHeader->HeaderSize > )); > return RETURN_UNSUPPORTED; > @@ -161,7 +162,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BmpImage... PixelWidth:0x%x > BitPerPixel:0x%x\n", > + "%a: invalid BmpImage. PixelWidth:0x%x BitPerPixel:0x%x\n", > + __func__, > BmpHeader->PixelWidth, > BmpHeader->BitPerPixel > )); > @@ -172,7 +174,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BmpImage... > DataSizePerLine:0x%x\n", > + "%a: invalid BmpImage. DataSizePerLine:0x%x\n", > + __func__, > DataSizePerLine > )); > > @@ -189,7 +192,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BmpImage... DataSizePerLine:0x%x > PixelHeight:0x%x\n", > + "%a: invalid BmpImage. DataSizePerLine:0x%x PixelHeight:0x%x\n", > + __func__, > DataSizePerLine, > BmpHeader->PixelHeight > )); > @@ -206,7 +210,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BmpImage... PixelHeight:0x%x > DataSizePerLine:0x%x\n", > + "%a: invalid BmpImage. PixelHeight:0x%x DataSizePerLine:0x%x\n", > + __func__, > BmpHeader->PixelHeight, > DataSizePerLine > )); > @@ -218,11 +223,11 @@ TranslateBmpToGopBlt ( > (BmpHeader->Size < BmpHeader->ImageOffset) || > (BmpHeader->Size - BmpHeader->ImageOffset != DataSize)) > { > - DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: invalid BmpImage... > \n")); > - DEBUG ((DEBUG_ERROR, " BmpHeader->Size: 0x%x\n", > BmpHeader->Size)); > - DEBUG ((DEBUG_ERROR, " BmpHeader->ImageOffset: 0x%x\n", > BmpHeader->ImageOffset)); > - DEBUG ((DEBUG_ERROR, " BmpImageSize: 0x%lx\n", > (UINTN)BmpImageSize)); > - DEBUG ((DEBUG_ERROR, " DataSize: 0x%lx\n", (UINTN)DataSize)); > + DEBUG ((DEBUG_ERROR, "%a: invalid BmpImage... \n", __func__)); > + DEBUG ((DEBUG_ERROR, " Size: 0x%x\n", BmpHeader->Size)); > + DEBUG ((DEBUG_ERROR, " ImageOffset: 0x%x\n", > BmpHeader->ImageOffset)); > + DEBUG ((DEBUG_ERROR, " BmpImageSize: 0x%lx\n", > (UINTN)BmpImageSize)); > + DEBUG ((DEBUG_ERROR, " DataSize: 0x%lx\n", (UINTN)DataSize)); > > return RETURN_UNSUPPORTED; > } > @@ -279,7 +284,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BltBuffer needed size... > PixelWidth:0x%x PixelHeight:0x%x\n", > + "%a: invalid BltBuffer needed size. PixelWidth:0x%x > PixelHeight:0x%x\n", > + __func__, > BmpHeader->PixelWidth, > BmpHeader->PixelHeight > )); > @@ -297,7 +303,8 @@ TranslateBmpToGopBlt ( > if (EFI_ERROR (Status)) { > DEBUG (( > DEBUG_ERROR, > - "TranslateBmpToGopBlt: invalid BltBuffer needed size... PixelWidth x > PixelHeight:0x%x struct size:0x%x\n", > + "%a: invalid BltBuffer needed size. PixelWidth x PixelHeight:0x%x > struct size:0x%x\n", > + __func__, > Temp, > sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL) > )); > @@ -312,7 +319,7 @@ TranslateBmpToGopBlt ( > // > DEBUG ((DEBUG_INFO, "Bmp Support: Allocating 0x%X bytes of > memory\n", BltBufferSize)); > *GopBltSize = (UINTN)BltBufferSize; > - *GopBlt = AllocatePool (*GopBltSize); > + *GopBlt = AllocatePages (*GopBltSize); > IsAllocated = TRUE; > if (*GopBlt == NULL) { > return RETURN_OUT_OF_RESOURCES; > @@ -329,13 +336,14 @@ TranslateBmpToGopBlt ( > > *PixelWidth = BmpHeader->PixelWidth; > *PixelHeight = BmpHeader->PixelHeight; > - DEBUG ((DEBUG_INFO, "BmpHeader->ImageOffset 0x%X\n", > BmpHeader->ImageOffset)); > - DEBUG ((DEBUG_INFO, "BmpHeader->PixelWidth 0x%X\n", > BmpHeader->PixelWidth)); > - DEBUG ((DEBUG_INFO, "BmpHeader->PixelHeight 0x%X\n", > BmpHeader->PixelHeight)); > - DEBUG ((DEBUG_INFO, "BmpHeader->BitPerPixel 0x%X\n", > BmpHeader->BitPerPixel)); > - DEBUG ((DEBUG_INFO, "BmpHeader->ImageSize 0x%X\n", > BmpHeader->ImageSize)); > - DEBUG ((DEBUG_INFO, "BmpHeader->HeaderSize 0x%X\n", > BmpHeader->HeaderSize)); > - DEBUG ((DEBUG_INFO, "BmpHeader->Size 0x%X\n", BmpHeader->Size)); > + DEBUG ((DEBUG_INFO, "BmpHeader :\n")); > + DEBUG ((DEBUG_INFO, " ImageOffset 0x%X\n", > BmpHeader->ImageOffset)); > + DEBUG ((DEBUG_INFO, " PixelWidth 0x%X\n", BmpHeader->PixelWidth)); > + DEBUG ((DEBUG_INFO, " PixelHeight 0x%X\n", BmpHeader->PixelHeight)); > + DEBUG ((DEBUG_INFO, " BitPerPixel 0x%X\n", BmpHeader->BitPerPixel)); > + DEBUG ((DEBUG_INFO, " ImageSize 0x%X\n", BmpHeader->ImageSize)); > + DEBUG ((DEBUG_INFO, " HeaderSize 0x%X\n", > BmpHeader->HeaderSize)); > + DEBUG ((DEBUG_INFO, " Size 0x%X\n", BmpHeader->Size)); > > // > // Translate image from BMP to Blt buffer format > -- > 2.38.1.windows.1 > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107146): https://edk2.groups.io/g/devel/message/107146 Mute This Topic: https://groups.io/mt/100321390/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-07-24 7:34 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-07-21 12:12 [edk2-devel] [PATCH] MdeModulePkg: AllocatePages for TranslateBmpToGopBlt chitralekha ck 2023-07-21 17:18 ` Ard Biesheuvel 2023-07-24 4:56 ` Ashraf Ali S 2023-07-24 7:33 ` Ard Biesheuvel 2023-07-24 1:05 ` 回复: " gaoliming via groups.io
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox