Re: [edk2-devel] [PATCH v7 01/43] MdeModulePkg: Create PCDs to be used in support of SEV-ES

["Dong, Eric" <eric.dong@intel.com>]

Hi Tom,

> -----Original Message-----
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Sent: Thursday, May 7, 2020 2:33 AM
> To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io;
> lersek@redhat.com
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> Subject: Re: [edk2-devel] [PATCH v7 01/43] MdeModulePkg: Create PCDs to
> be used in support of SEV-ES
> 
> Hi Eric,
> 
> On 5/6/20 10:06 AM, Dong, Eric wrote:
> > Hi Tom,
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> >> Lendacky, Thomas
> >> Sent: Wednesday, May 6, 2020 9:20 PM
> >> To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io;
> >> lersek@redhat.com
> >> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel
> >> <ard.biesheuvel@linaro.org>; Kinney, Michael D
> >> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Ni,
> >> Ray <ray.ni@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; Wang,
> >> Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> >> Subject: Re: [edk2-devel] [PATCH v7 01/43] MdeModulePkg: Create PCDs
> >> to be used in support of SEV-ES
> >>
> >> On 5/5/20 8:53 PM, Dong, Eric wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf
> >>>> Of Laszlo Ersek
> >>>> Sent: Tuesday, May 5, 2020 11:30 PM
> >>>> To: Tom Lendacky <thomas.lendacky@amd.com>; Dong, Eric
> >>>> <eric.dong@intel.com>; devel@edk2.groups.io
> >>>> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel
> >>>> <ard.biesheuvel@linaro.org>; Kinney, Michael D
> >>>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
> >>>> Ni, Ray <ray.ni@intel.com>; Brijesh Singh <brijesh.singh@amd.com>;
> >>>> Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> >>>> <hao.a.wu@intel.com>
> >>>> Subject: Re: [edk2-devel] [PATCH v7 01/43] MdeModulePkg: Create
> >>>> PCDs to be used in support of SEV-ES
> >>>>
> >>>> On 05/04/20 18:41, Tom Lendacky wrote:
> >>>>
> >>>>> Is there an easy way to run everything that this link points, too?
> >>>>> Is it just creating a pull request that does this? I don't want to
> >>>>> take up a lot of your time, so if there's some documentation on
> >>>>> how to run an integration test to find and fix issues like this, just point
> me to it.
> >>>>
> >>>> Just create a pull request; it will set off CI, and you can review
> >>>> VS build errors there (if any).
> >>>>
> >>>> Your PR will automatically be closed (rejected) regardless of
> >>>> whether CI succeeds or not. PRs are merged -- in fact,
> >>>> *auto*-merged, by the "mergify bot" -- if and only if (a) the CI
> >>>> run succeeds, and (b) the PR has
> >> the "push"
> >>>> label set.
> >>>>
> >>>> And only edk2 maintainers have permission to set the "push" label.
> >>>> Any PR without the "push" label qualifies as a "personal test build".
> >>>> So you can freely experiment with PRs, because you can't (even
> >>>> unwittingly) satisfy condition (b).
> >>>>
> >>>>
> >>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
> >>>> hub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FEDK-II-
> >> Development
> >>>> -
> >>
> &amp;data=02%7C01%7Cthomas.lendacky%40amd.com%7C9cff3475aff84a9
> >> 57285
> >>>>
> >>
> 08d7f1604c99%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63724
> >> 326821
> >>>>
> >>
> 7382019&amp;sdata=3%2FIKB174QaVLaqO0u1gdrL0izXmhEZ%2Byvj3iC13UYB
> >> c%3D&
> >>>> amp;reserved=0
> >>>> Process
> >>>>
> >>>
> >>> Thanks Laszlo for your explanation.
> >>>
> >>> I found this patch serial is incompatible for the existed platforms.
> >>> Can you help to fix the build failure for these platforms in
> >>>
> >>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
> >> h
> >>> ub.com%2Ftianocore%2Fedk2-
> >> platforms&amp;data=02%7C01%7Cthomas.lendacky
> >>> %40amd.com%7C9cff3475aff84a95728508d7f1604c99%7C3dd8961fe4884
> e6
> >> 08e11a8
> >>>
> >>
> 2d994e183d%7C0%7C0%7C637243268217382019&amp;sdata=jU0qrB%2BV6Z
> >> vFmPzjcx
> >>> Go9o2Pu1%2FrhRW0gUZTMv%2BiXDQ%3D&amp;reserved=0
> >>>
> >>
> >> I have fixed all of the build issues associated with the VS compiler
> >> using the pull request method that Laszlo mentioned. I then
> >> successfully built the RPi4 platform under GCC (build -n 32 -a
> >> AARCH64 -t GCC5 -p
> >> Platform/RaspberryPi/RPi4/RPi4.dsc) using the AARCH64 cross compiler.
> >>
> >> Is there a particular platform that experiences an issue or are the
> >> failures related to the VS compiler errors that my next series will have
> fixed?
> >
> > I used the KabylakeRvp3 platform with your changes in Edk2 and met
> failures.
> > KabylakeRvp3 code at
> > Edk2-platforms\Platform\Intel\KabylakeOpenBoardPkg\
> >
> >   I used below command to build the code.
> > C:\Code\OpenSource\edk2-platforms\Platform\Intel>build_bios.py
> > --platform KabylakeRvp3
> >
> > You need clone below repositories to build the code.
> > Edk2: git@github.com:tianocore/edk2.git Edk2-Platform
> > git@github.com:tianocore/edk2-platforms.git
> > edk2-non-osi :  git@github.com:tianocore/edk2-non-osi.git
> > FSP: git@github.com:IntelFsp/FSP.git
> 
> On my Linux system, I cloned all the libraries and set the WORKSPACE and
> PACKAGES_PATH env variables, sourced edksetup.sh and issued:
> 
> python3 edk2-platforms/Platform/Intel/build_bios.py --platform
> KabylakeRvp3
> 
> and got the following errors:
> 
> Traceback (most recent call last):
>   File "edk2-platforms/Platform/Intel/build_bios.py", line 1097, in <module>
>     main()
>   File "edk2-platforms/Platform/Intel/build_bios.py", line 1067, in main
>     platform_config = get_platform_config(arguments.platform, build_config)
>   File "edk2-platforms/Platform/Intel/build_bios.py", line 904, in
> get_platform_config
>     path = platform_data.get(platform_name)
> AttributeError: 'NoneType' object has no attribute 'get'

I build it in windows environment and I not met this error.

> 
> I don't know what I'm missing as to why this error pops up. How is this done
> on a Linux system?
> 
> Did your build fail because of the VmgExitLib library not being specified?
> If the platform includes the CpuExceptionHandlerLib or MpInitLib libraries
> then it will now also need to include the VmgExitLib library.

With your V7 patches, I also met other code errors. Just like I replied to you in other mails.

> 
> I wish the build system could figure out that because the VmgExitLib library is
> specified in the various CpuExceptionHandlerLib INF files and the various
> MpInitLib INF files, it would automatically know to include it in whatever uses
> those libraries. It doesn't seem right that you would have to know and
> specify the library requirements of a library you are including when the
> system could figure it out.

Current build system not support this yet. So, this is the impact for the exited platforms. We need
to prepare the patches for the existed platforms we cared before push all your patches.

Thanks,
Eric
> 
> Thanks,
> Tom
> 
> >
> >>
> >>> I think you also needs to add an wiki page to explain what need to
> >>> do if an
> >> platform needs to integrate your changes, also it's better to explain
> >> this feature in the page.
> >>>
> >>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
> >> h
> >>>
> >>
> ub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki&amp;data=02%7C01%7
> >> Ctho
> >>>
> >>
> mas.lendacky%40amd.com%7C9cff3475aff84a95728508d7f1604c99%7C3dd89
> >> 61fe4
> >>>
> >>
> 884e608e11a82d994e183d%7C0%7C0%7C637243268217382019&amp;sdata=x
> >> LkoV4zW
> >>> hxtsbqszqPc0lEAl%2BYLL%2B2wg1nIXql8a64E%3D&amp;reserved=0
> >>
> >> I don't see any platform other than OVMF using this feature as it is
> >> a virtualization feature. Having said that I can add an explanation
> >> of what is needed should another virtualization platform be created
> >> under EDK2 that wants to support SEV-ES. And, as you said, I can also
> >> explain the feature overall on the page.
> >>
> >
> > I think your page includes two parts, one is how to change the
> > platform code to make the platform pass build, the other is if the platform
> needs to enable the virtualization feature, how to enable it.
> >
> >>>
> >>>
> >>> If you want to include this change in the next edk2 release, you
> >>> need to add one item for it in the release plan page, sample can be
> >>> found in below pages:
> >>>
> >>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
> >> h
> >>> ub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FEDK-II-Release-
> >> Plann
> >>>
> >>
> ing&amp;data=02%7C01%7Cthomas.lendacky%40amd.com%7C9cff3475aff84
> >> a95728
> >>>
> >>
> 508d7f1604c99%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6372
> >> 4326821
> >>>
> >>
> 7382019&amp;sdata=kcDVjYHMS9bRRZOlKEk5ynFNT39AnxchJAMak%2Bn870
> >> I%3D&amp
> >>> ;reserved=0
> >>
> >> Thanks. Is there anyone in particular that I need to request this
> >> feature be added?
> >
> > You can syn with Liming, he is the edk2 release manager. He owns edk2
> stable tag release task.
> >
> > Thanks,
> > Eric
> >>
> >> Thanks,
> >> Tom
> >>
> >>>
> >>>
> >>> Thanks,
> >>> Eric
> >>>
> >>>> Thanks,
> >>>> Laszlo
> >>>>
> >>>>
> >>>>
> >>
> >> 
> >