From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lf1-f49.google.com (mail-lf1-f49.google.com [209.85.167.49]) by mx.groups.io with SMTP id smtpd.web11.5493.1628058252047570068 for ; Tue, 03 Aug 2021 23:24:12 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@google.com header.s=20161025 header.b=iI1lbjM4; spf=pass (domain: google.com, ip: 209.85.167.49, mailfrom: chengchieh@google.com) Received: by mail-lf1-f49.google.com with SMTP id t9so2622103lfc.6 for ; Tue, 03 Aug 2021 23:24:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=OuSM10FUDRjovEAxQ3U4C9zssMrk+7T6EaAoVRLbZsc=; b=iI1lbjM4HJYj5o2vpkP2P5Qr79EsnckNMnOm3dIM8LrtYHok/CvSuhwEcePeAmuH6s 4rXEKJlucRPvzcXk+TW12HvkPy7Jc4D080JDGBpDk4gmNJ1r1pbDXroFhbF/4WlTcqxj XN554IYAJetRWL0H5CBh+WXrSyZ/D+sUjRh5a4AjpVYo45wH/f4l6stZDm7qRu/MzP1Q Vm94z7AGZZhXYFcfT/TVdcZgjmD7XdCTdo27jBfk6NwFJrRCjP0ZPVRmJqwwTX/LhC3g tvwMrQ4X1LvszBIUD+GeElofpJK3p6LSEDcujOkzPzmMosDNBTcGlr8R/GAcj2dB7cMl oorA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=OuSM10FUDRjovEAxQ3U4C9zssMrk+7T6EaAoVRLbZsc=; b=rV3AYuZThbDEkI4wWHEEQQqwgr1qMKB5qPgDb2xrjsvv0Qy2cV77ALoK6Ug2N1J3pY XnR4Pdoui4gihr5HHrYIP/DOlofuhEapeyJE0kMolNf9KWqqqOFAZxRWxra1dty8X1Ei cjo2D/rEb/0/lYVFkBJ0/sRTQKPqVZGAVrkZwqAhrYefOEqgGmPxr69ar3lAG0qAna9x LtXJGIJi/FjWOJsBnUFLRhzZZ6QBCdQzC7QtfCA4GPATRadHsW5o8LgvzNwY5g7AqjZE IBn26/ZxZIjwF9Xew2qmcp3bUup0pwARK9HKraezSnha+i/dNO51TIljNyF8GYUT9XZT UUEQ== X-Gm-Message-State: AOAM533Tbl2/YBMlTqye78OPfzDmsx7tyPyNMUS3zAlNe/YxdLPnOdWh Cwu/wSG1khLJ65e78Futdb4faidvkE6DiAex/hFp2g== X-Google-Smtp-Source: ABdhPJwL022/jb0Cja+jniM62In9dAED0RZvvn/nQoYYjwwfJhYSqg/ua27OqJ7q+L49D4rzkYNRH9CtiroqNUv8bgA= X-Received: by 2002:ac2:50c2:: with SMTP id h2mr12288575lfm.332.1628058250074; Tue, 03 Aug 2021 23:24:10 -0700 (PDT) MIME-Version: 1.0 References: <20210721132328.1415485-1-chengchieh@google.com> <20210721132328.1415485-5-chengchieh@google.com> In-Reply-To: From: Cheng-Chieh Huang Date: Wed, 4 Aug 2021 14:23:58 +0800 Message-ID: Subject: Re: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data To: "Dong, Guo" Cc: "devel@edk2.groups.io" Content-Type: multipart/alternative; boundary="000000000000abc7c405c8b5d971" --000000000000abc7c405c8b5d971 Content-Type: text/plain; charset="UTF-8" Hi Guo, Thanks for the review. This is a quick fix for today's PlatformHookLib design. PlatformHookLib (probably via PlatformBootManagerLib) will be linked by BDS and potentially called again in Windows bootloader. If we did not reserve this range of memory, system will crash the system when it was called. This can be reproduced if we boot Windows 2018. I also did not think reserving this memory til runtime is a good idea, but I am not sure why we want to link PlatformHookLib to PlatformBootManagerLib. I tried to remove PlatformHookLib in PlatformBootManagerLib and it seems to work as well. maybe we should take this approach. what do you think? -- Cheng-Chieh On Wed, Aug 4, 2021 at 10:44 AM Dong, Guo wrote: > > why build runtime memory allocation hob here? > The PayloadEntry should already got the information and build a new HOB > list to DXE core, will anyone access these region late? > If yes, maybe you need add LINUXBOOT_PAYLOAD flag for this code, and > update commit message on this. > > Thanks, > Guo > > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of > Cheng-Chieh Huang via groups.io > Sent: Wednesday, July 21, 2021 6:23 AM > To: devel@edk2.groups.io > Cc: Cheng-Chieh Huang > Subject: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload > config in runtime services data > > Signed-off-by: Cheng-Chieh Huang > --- > UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c > b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c > index ae16f25c7c0e..70afbf83ed4a 100644 > --- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c > +++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c > @@ -517,6 +517,8 @@ BuildGenericHob ( > > // The UEFI payload FV > BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase), PcdGet32 > (PcdPayloadFdMemSize), EfiBootServicesData); > + // The UEFI payload config FV > + BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase) - SIZE_64KB, > SIZE_64KB, EfiRuntimeServicesData); > > // > // Build CPU memory space and IO space hob > -- > 2.32.0.402.g57bb445576-goog > > > > > > > --000000000000abc7c405c8b5d971 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi=C2=A0Guo,

Thanks for the = review. This is a quick fix for today's PlatformHookLib=C2=A0design. Pl= atformHookLib (probably via PlatformBootManagerLib) will be linked by BDS a= nd potentially called again in Windows bootloader. If we did not reserve th= is range of memory, system will crash the system when it was called. This c= an be reproduced if we boot Windows 2018. I also did not think reserving th= is memory til runtime is a good idea, but I am not sure why we want to link= PlatformHookLib to PlatformBootManagerLib. I tried to remove PlatformHookL= ib in PlatformBootManagerLib and it seems to work as well. maybe we should = take this approach. what do you think?

--
Cheng-Chieh

On Wed, Aug 4, 2021 at 10:44 AM Dong, Guo <guo.dong@intel.com> wr= ote:

why build runtime memory allocation hob here?
The PayloadEntry should already got the information and build a new HOB li= st to DXE core, will anyone access these region late?
If yes, maybe you need add LINUXBOOT_PAYLOAD flag for this code, and updat= e commit message on this.

Thanks,
Guo

-----Original Message-----
From: devel@edk2= .groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.g= roups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload confi= g in runtime services data

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
=C2=A0UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 2 ++
=C2=A01 file changed, 2 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c b/UefiPayl= oadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index ae16f25c7c0e..70afbf83ed4a 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -517,6 +517,8 @@ BuildGenericHob (

=C2=A0 =C2=A0// The UEFI payload FV
=C2=A0 =C2=A0BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase), Pcd= Get32 (PcdPayloadFdMemSize), EfiBootServicesData);
+=C2=A0 // The UEFI payload config FV
+=C2=A0 BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase) - SIZE_64= KB, SIZE_64KB, EfiRuntimeServicesData);

=C2=A0 =C2=A0//
=C2=A0 =C2=A0// Build CPU memory space and IO space hob
--
2.32.0.402.g57bb445576-goog






--000000000000abc7c405c8b5d971--