From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.3231.1686252311727489975 for ; Thu, 08 Jun 2023 12:25:11 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=ga68Ctbp; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [192.168.4.22] (unknown [47.201.8.94]) by linux.microsoft.com (Postfix) with ESMTPSA id B8F7320C145C; Thu, 8 Jun 2023 12:25:09 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com B8F7320C145C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1686252311; bh=SpcR+JCjRD+URcTGv/UmVJ6hYTfEDokf4SCNlR/fVO8=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=ga68Ctbp5S6m6I5x8iTY0ey4vUXvNh3hLC2nOOek7QJbXUGBYOYWpbHgiVriTE7sO C0kHO1wRMeT1ECMOrnozTKwGr3rqxiXlUQ5piq9grxGD6H891ASTZujeET10eftQk/ 6j+Jga4WzMAyiLDv2VfMjMKlU2dpp6VyJsdoL4a8= Message-ID: Date: Thu, 8 Jun 2023 15:25:08 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.11.2 Subject: Re: [edk2-devel] [PATCH 1/2] UefiCpuPkg/CpuMpPei: Print correct buffer size used for page table To: devel@edk2.groups.io, ardb@kernel.org Cc: Ray Ni , Jiewen Yao , Gerd Hoffmann , Taylor Beebe , Oliver Smith-Denny , Dandan Bi , Dun Tan , Liming Gao , "Kinney, Michael D" , Eric Dong , Rahul Kumar , Kun Qin References: <20230608172323.9096-1-ardb@kernel.org> <20230608172323.9096-2-ardb@kernel.org> From: "Michael Kubacki" In-Reply-To: <20230608172323.9096-2-ardb@kernel.org> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Reviewed-by: Michael Kubacki A couple comments below. On 6/8/2023 1:23 PM, Ard Biesheuvel wrote: > The DEBUG print that outputs the base and size of the page table > allocation always prints 0x0 for the size, given that BufferSize will be > updated by PageTableMap () and contain the unused allocation on return. > > So move the DEBUG print right after the allocation. > > Signed-off-by: Ard Biesheuvel > --- > UefiCpuPkg/CpuMpPei/CpuPaging.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c > index b7ddb0005b6fbcac..175e47ccd737a0c1 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c > +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c > @@ -396,6 +396,13 @@ EnablePaePageTable ( > return EFI_OUT_OF_RESOURCES; > > } > > > > + DEBUG (( > > + DEBUG_INFO, > > + "EnablePaePageTable: Created PageTable = 0x%x, BufferSize = %x\n", > > + PageTable, > > + BufferSize > > + )); > > + In the past, a point was made to improve portability between 32-bit and 64-bit architectures by casting UINTN values to UINT64 and then printing them %Lx. If this is a DEBUG only change that might be worth adding as well. In any case, can you please prefix the print specifier for BufferSize with "0x"? > > Status = PageTableMap (&PageTable, PagingPae, Buffer, &BufferSize, 0, SIZE_4GB, &MapAttribute, &MapMask, NULL); > > ASSERT_EFI_ERROR (Status); > > if (EFI_ERROR (Status) || (PageTable == 0)) { > > @@ -417,13 +424,6 @@ EnablePaePageTable ( > // > > AsmWriteCr0 (AsmReadCr0 () | BIT31); > > > > - DEBUG (( > > - DEBUG_INFO, > > - "EnablePaePageTable: Created PageTable = 0x%x, BufferSize = %x\n", > > - PageTable, > > - BufferSize > > - )); > > - > > return Status; > > } > > >