From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id DFE52D813CE for ; Mon, 24 Jul 2023 07:34:00 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=je/az0wVYq58BnH1fANuStn6mcrYvfMjhUDRnehTgS8=; c=relaxed/simple; d=groups.io; h=X-Received:X-Received:X-Received:X-Received:X-Received:X-Gm-Message-State:X-Google-Smtp-Source:X-Received:MIME-Version:References:In-Reply-To:From:Date:X-Gmail-Original-Message-ID:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1690184039; v=1; b=BRRjpLUAMhVzye5E8eOQGntP0WEpkb47i656QyHPkgr8HpiJ1oBnUy+mBhyzK6JFW622ThbZ ldKlprCQhJdwOs9CiFrVxq76HDeGmNn//xU37v2m5bd57ucTxmn38JF9jX+EJcBmKzdagnuyBEn fzEwYe0vZq9vXzoR5EuVYEJk= X-Received: by 127.0.0.2 with SMTP id VQ6aYY7687511xCISsB5mHrI; Mon, 24 Jul 2023 00:33:59 -0700 X-Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web10.42592.1690184038500770626 for ; Mon, 24 Jul 2023 00:33:58 -0700 X-Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id D847260F64 for ; Mon, 24 Jul 2023 07:33:57 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id 46532C433C9 for ; Mon, 24 Jul 2023 07:33:57 +0000 (UTC) X-Received: by mail-lj1-f179.google.com with SMTP id 38308e7fff4ca-2b6fdaf6eefso57433701fa.0 for ; Mon, 24 Jul 2023 00:33:57 -0700 (PDT) X-Gm-Message-State: Q7Rvii7NghxFFaw4NLUBPrHgx7686176AA= X-Google-Smtp-Source: APBJJlEruyOVnBDOnPriJkng2D9/fyjYEKN1FpRbleK9CmIG0QPhxtX9aqwgosM3/1lBvE1IkuHWgZvr9+qD2Oz7lso= X-Received: by 2002:a2e:b61b:0:b0:2b6:ec2b:659 with SMTP id r27-20020a2eb61b000000b002b6ec2b0659mr4741268ljn.17.1690184035215; Mon, 24 Jul 2023 00:33:55 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: "Ard Biesheuvel" Date: Mon, 24 Jul 2023 09:33:43 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH] MdeModulePkg: AllocatePages for TranslateBmpToGopBlt To: "S, Ashraf Ali" Cc: "devel@edk2.groups.io" , "Ck, Chitralekha" , "Ni, Ray" , "Gao, Zhichao" , "Duggapu, Chinni B" Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=BRRjpLUA; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none) On Mon, 24 Jul 2023 at 06:56, S, Ashraf Ali 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 On Behalf Of Ard Biesheuvel > Sent: Friday, July 21, 2023 10:49 PM > To: devel@edk2.groups.io; Ck, Chitralekha > Cc: Ni, Ray ; Gao, Zhichao ; S, Ashraf Ali ; Duggapu, Chinni B > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg: AllocatePages for TranslateBmpToGopBlt > > On Fri, 21 Jul 2023 at 17:38, chitralekha ck 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 > > Cc: Zhichao Gao > > Cc: Ashraf Ali S > > Cc: Chinni B Duggapu > > Signed-off-by: chitralekha ck > > --- > > .../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] -=-=-=-=-=-=-=-=-=-=-=-