From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lj1-f172.google.com (mail-lj1-f172.google.com [209.85.208.172]) by mx.groups.io with SMTP id smtpd.web11.19699.1628758324658658881 for ; Thu, 12 Aug 2021 01:52:05 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@google.com header.s=20161025 header.b=LKxODp7w; spf=pass (domain: google.com, ip: 209.85.208.172, mailfrom: chengchieh@google.com) Received: by mail-lj1-f172.google.com with SMTP id h11so9469903ljo.12 for ; Thu, 12 Aug 2021 01:52:04 -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=zoT7m93wQRIKLuUzghVbziNHTQ6edYPy+AJncUCR2sc=; b=LKxODp7wP4jxd7JtK6W0nraAMBzd6efw2HKTpnGlTG+UdXvE4UFCcNJmXkpwrfq4Kc aB8UO5fbFe++xgDZqucxMdiK1CBMidz8tpVL06jrOF+nIpvX+JrWbfq3cANif56M0HQQ vdr2ckM1SdtPElxDPs5R4QzIL2vz7290xgsDNa9xg60RQEc72skYILzjnqk8YAAIcb+J qf+Hiv2YhVbPX7mop6Kov48WJXlBUwU1O9Ms93kjka894jNPx3TWrunpgRFJssUIg6DU PbwgBwtfD4S5SXt6JNG7d7Elj8aROmxQrO+TFciBxIrN2GZ4pTCe8qNNf0JIqpD4Gb11 6EYA== 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=zoT7m93wQRIKLuUzghVbziNHTQ6edYPy+AJncUCR2sc=; b=P3kjyiwrq9wA8lIcVtgIUrtmsW/2cHHkNNGTHNfR0X0qJoYhMlHNk041vAgefSQ/Dc aSm4mqIJzdr6tl7D5I6/gG+kgHKm78VJDbljP1G8vclvMYgC5z8GjJ9VtOlGdsVQKDDF cx2FBGOKjHVseSlwGp8um77XeLBo9NO73geCrCdhpWtvXqR5R3cZYm6GHyAOLzVOvKE+ tZYjrwwHq3+iHYTXoGrpEBa1lVYBMv6XynDM2hm9iPrRKQESLENFbniA+cWSy1WG3Abv 5X2L/C1QPVg2fUoFemGouue+l72IUe3Co6xUjw9fKm0iRZT9hj9Os04fxMXT4Org7UmW LyQA== X-Gm-Message-State: AOAM531NqS2zT8GJnRl9E2v776+cOesB05ZiPeFgs8i4ZCCWrVVcCFDV dqauSJcTX2sToHkvZgMDNgcxKokcXlZPRc9wA8m0mA== X-Google-Smtp-Source: ABdhPJy3XxSmU313PBytEK+gmXU3Tkkrr/Z48Oh2XLTSUSPEkCwF44UCjREvIEeToFkH0hWU6280jNXyj4Whq2iCtpQ= X-Received: by 2002:a2e:9d43:: with SMTP id y3mr2162795ljj.85.1628758322363; Thu, 12 Aug 2021 01:52:02 -0700 (PDT) MIME-Version: 1.0 References: <20210807145106.2236803-1-chengchieh@google.com> In-Reply-To: From: Cheng-Chieh Huang Date: Thu, 12 Aug 2021 16:51:51 +0800 Message-ID: Subject: Re: [edk2-devel] [PATCH v2 0/4] UefiPayloadPkg: LinuxBoot Support in UefiPayload To: "Ni, Ray" Cc: "devel@edk2.groups.io" , "Schaefer, Daniel" , Trammell Hudson , "Ma, Maurice" , "Dong, Guo" , "You, Benjamin" Content-Type: multipart/alternative; boundary="0000000000003b77b105c958d93b" --0000000000003b77b105c958d93b Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Ray, 1. UefiPayloadPkg: Add LINUXBOOT payload target a. Why is IO/MEM align in AdjustRootBridgeResource() skipped for Linux Payload? -> This alignment is too conservative and causes the system to run out of IO resources. AFAIK, we do not need this in UEFI OS and our use case involves larger systems (let's say server) . b. can you please run ECC to make sure the C source code complains to EDKII coding standard? -> sure will do. 2. UefiPayloadPkg: Use legacy timer in Linuxboot payload a. Can you kindly explain the reason of HPET timer failure? Is this patch a temporary workaround? -> This is taken from OvmfPkg's Xen package. Current Hpet driver does not work well after we hand over control to OS. Yes we can consider this as a workaround. This should not affect functionality because we will go back to the regular TSC/HPET timer in linux after booting to the next OS. 4. UefiPayloadPkg: Reserve Payload config in runtime services data a. Why? -> Currently, due to some design issue, this Hook will also be called in BDS. Dong said he will work on a fix for this and he suggested that I gate it LINUXBOOT_PAYLOAD and he will clean it up in his fix. For more details, please see earlier mail thread titled [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data > By the way, can you please add me to the CC list since I am the maintainer of UefiPayloadPkg (newly become so you might not notice that=F0= =9F=98=8A) next time you send updates? Sure. -- Cheng-Chieh On Thu, Aug 12, 2021 at 12:49 PM Ni, Ray wrote: > I will leave all comments to the patches here since I have difficulty to > find individual patches without these patches sending to me directly. > > In general, the commit messages are too short to explain the > background/reason of the code change. Can you please put more explanation > in the commit message? > > 1. UefiPayloadPkg: Add LINUXBOOT payload target > a. Why is IO/MEM align in AdjustRootBridgeResource() skipped for Linux > Payload? > b. can you please run ECC to make sure the C source code complains to > EDKII coding standard? > > 2. UefiPayloadPkg: Use legacy timer in Linuxboot payload > a. Can you kindly explain the reason of HPET timer failure? Is this > patch a temporary workaround? > > 3. UefiPayloadPkg: Update maximum logic processor to 256 > a. It still has the limitation of 256 threads. I am ok with your code > change. > > 4. UefiPayloadPkg: Reserve Payload config in runtime services data > a. Why? > > > By the way, can you please add me to the CC list since I am the maintaine= r > of UefiPayloadPkg (newly become so you might not notice that=F0=9F=98=8A)= next time > you send updates? > > Thanks, > ray > > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of > Cheng-Chieh Huang via groups.io > Sent: Saturday, August 7, 2021 10:51 PM > To: devel@edk2.groups.io > Cc: Cheng-Chieh Huang ; Schaefer, Daniel < > daniel.schaefer@hpe.com>; Trammell Hudson ; Ma, Maurice = < > maurice.ma@intel.com>; Dong, Guo ; You, Benjamin < > benjamin.you@intel.com> > Subject: [edk2-devel] [PATCH v2 0/4] UefiPayloadPkg: LinuxBoot Support in > UefiPayload > > These are necessary patches to Support LinuxBoot in UefiPayload. > With these paches, we can boot to ESXi and Windows from a linux in QEMU. > > This is second parse. In addition to fixing reviwer's suggestions, I > removed the following CLs. > * Add DISABLE_MMX_SSE to avoid generating floating points operation > -> will send a seperate patch to add these flags to BaseTools > > * LinuxBoot: use a text format for the configuration block. > -> will work with Trammell Hudson to cover this patch to EDK2 style. > > LinuxBoot README: > https://github.com/linuxboot/edk2/blob/uefipayload/UefiPayloadPkg/README.= md > > v2 PR to tianocore: > https://github.com/tianocore/edk2/pull/1873 > > Cheng-Chieh Huang (4): > UefiPayloadPkg: Add LINUXBOOT payload target > UefiPayloadPkg: Use legacy timer in Linuxboot payload > UefiPayloadPkg: Update maximum logic processor to 256 > UefiPayloadPkg: Reserve Payload config in runtime services data > > UefiPayloadPkg/UefiPayloadPkg.dsc | 24 ++- > UefiPayloadPkg/UefiPayloadPkg.fdf | 5 + > UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf | 39 +++= ++ > UefiPayloadPkg/Library/LbParseLib/Linuxboot.h | 47 +++= ++ > UefiPayloadPkg/Library/LbParseLib/LbParseLib.c | 182 > ++++++++++++++++++++ > UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c | 6 +- > UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 4 + > 7 files changed, 299 insertions(+), 8 deletions(-) create mode 100644 > UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf > create mode 100644 UefiPayloadPkg/Library/LbParseLib/Linuxboot.h > create mode 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.c > > Cc: Cheng-Chieh Huang > Cc: Daniel Schaefer > Cc: Trammell Hudson > Cc: Maurice Ma > Cc: Guo Dong > Cc: Benjamin You > > > -- > 2.32.0.605.g8dce9f2422-goog > > > >=20 > > > --0000000000003b77b105c958d93b Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi=C2=A0Ray,

1. UefiPa= yloadPkg: Add LINUXBOOT payload target
=C2=A0 a. Why is IO/MEM align in = AdjustRootBridgeResource() skipped for Linux Payload?
-> This = alignment is too conservative and causes the system to run out of IO resour= ces. AFAIK, we do not need this in UEFI OS and our use case involves=C2=A0l= arger systems (let's say server) .
=C2=A0 b. can you please run ECC = to make sure the C source code complains to EDKII coding standard?
-> sure will=C2=A0do.

2. UefiPayloadPkg: Use = legacy timer in Linuxboot payload
=C2=A0 a. Can you kindly explain the r= eason of HPET timer failure? Is this patch a temporary workaround?
=C2=A0->=C2=A0 This is taken from OvmfPkg's Xen package. Curre= nt Hpet driver does not work well after we hand over control to OS. Yes we = can consider this as a workaround. This should not affect functionality bec= ause we will go back to the regular TSC/HPET timer in linux after booting t= o the next=C2=A0OS.

4. UefiPayloadPkg: Reserve Pay= load=C2=A0config=C2=A0in=C2=A0runtime=C2=A0= services=C2=A0data
=C2=A0 a. Why?
-> Cu= rrently, due to some design issue, this Hook will also be called in BDS. Do= ng said he will work on a fix for this and he suggested that I=C2=A0gate=C2= =A0it LINUXBOOT_PAYLOAD and he will clean it up in his fix.
=C2= =A0For more details, please see earlier mail thread titled [PATCH v1 4/6] U= efiPayloadPkg: Reserve Payload config in runtime services data
<= div>

> By the way, can you please add me to the = CC list since I am the maintainer of UefiPayloadPkg (newly become so you mi= ght not notice that=F0=9F=98=8A) next time you send updates?
Sure.

--
= Cheng-Chieh
On Thu, Aug 12, 2021= at 12:49 PM Ni, Ray <ray.ni@intel.com> wrote:
I will leave all comments to the patches here since I h= ave difficulty to find individual patches without these patches sending to = me directly.

In general, the commit messages are too short to explain the background/rea= son of the code change. Can you please put more explanation in the commit m= essage?

1. UefiPayloadPkg: Add LINUXBOOT payload target
=C2=A0 a. Why is IO/MEM align in AdjustRootBridgeResource() skipped for Lin= ux Payload?
=C2=A0 b. can you please run ECC to make sure the C source code complains t= o EDKII coding standard?

2. UefiPayloadPkg: Use legacy timer in Linuxboot payload
=C2=A0 a. Can you kindly explain the reason of HPET timer failure? Is this = patch a temporary workaround?

3. UefiPayloadPkg: Update maximum logic processor to 256
=C2=A0 a. It still has the limitation of 256 threads. I am ok with your cod= e change.

4. UefiPayloadPkg: Reserve Payload config in runtime services data
=C2=A0 a. Why?


By the way, can you please add me to the CC list since I am the maintainer = of UefiPayloadPkg (newly become so you might not notice that=F0=9F=98=8A) n= ext time you send updates?

Thanks,
ray

-----Original Message-----
From: devel@edk2.= groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Saturday, August 7, 2021 10:51 PM
To: devel@edk2.gr= oups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>; Schaefer, Daniel <daniel.schaefer@hpe.com>; Trammell Hudson <hudson@trmm.net>; Ma, Maurice <maurice.ma@intel.com>; Dong, Guo <<= a href=3D"mailto:guo.dong@intel.com" target=3D"_blank">guo.dong@intel.com>; You, Benjamin <benjamin.you@intel.com>
Subject: [edk2-devel] [PATCH v2 0/4] UefiPayloadPkg: LinuxBoot Support in U= efiPayload

These are necessary patches to Support LinuxBoot in UefiPayload.
With these paches, we can boot to ESXi and Windows from a linux in QEMU.
This is second parse. In addition to fixing reviwer's suggestions, I re= moved the following CLs.
* Add DISABLE_MMX_SSE to avoid generating floating points operation
-> will send a seperate patch to add these flags to BaseTools

* LinuxBoot: use a text format for the configuration block.
-> will work with Trammell Hudson to cover this patch to EDK2 style.

LinuxBoot README:
https://github.com/linuxb= oot/edk2/blob/uefipayload/UefiPayloadPkg/README.md

v2 PR to tianocore:
https://github.com/tianocore/edk2/pull/1873

Cheng-Chieh Huang (4):
=C2=A0 UefiPayloadPkg: Add LINUXBOOT payload target
=C2=A0 UefiPayloadPkg: Use legacy timer in Linuxboot payload
=C2=A0 UefiPayloadPkg: Update maximum logic processor to 256
=C2=A0 UefiPayloadPkg: Reserve Payload config in runtime services data

=C2=A0UefiPayloadPkg/UefiPayloadPkg.dsc=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2= =A0 24 ++-
=C2=A0UefiPayloadPkg/UefiPayloadPkg.fdf=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2= =A0 =C2=A05 +
=C2=A0UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|=C2=A0 39 +++++
=C2=A0UefiPayloadPkg/Library/LbParseLib/Linuxboot.h=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 47 +++++
=C2=A0UefiPayloadPkg/Library/LbParseLib/LbParseLib.c=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| 182 ++++++++++++++++++++
=C2=A0UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c |=C2= =A0 =C2=A06 +-
=C2=A0UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|=C2=A0 =C2=A04 +
=C2=A07 files changed, 299 insertions(+), 8 deletions(-)=C2=A0 create mode = 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
=C2=A0create mode 100644 UefiPayloadPkg/Library/LbParseLib/Linuxboot.h
=C2=A0create mode 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.c

Cc: Cheng-Chieh Huang <chengchieh@google.com>
Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
Cc: Trammell Hudson <hudson@trmm.net>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Guo Dong <gu= o.dong@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>


--
2.32.0.605.g8dce9f2422-goog






--0000000000003b77b105c958d93b--