From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web12.4106.1588782294243713284 for ; Wed, 06 May 2020 09:24:54 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=LkDOAC9S; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1588782293; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ylpQNhoL06QYAAb30L3+h9FxqVcfVIL0clh6lduE66w=; b=LkDOAC9Scp0Qyuxh6yCBf4ViPdgEwzgVTEgWkjPkmR4VpSOdGGbAGk0Nj+KJcdg1oGYT0s D4iC8cqwPZitzb6JLD9lDQN7B9B/igJotQ9xIUsS7GvXNg7T6f6dXeq3TvBwqyXw7KhkDH SaLJsfdmKsvm71DDEQHNK6eyahdUhWk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-197-nGud8RIKODizIHOwsM99Vg-1; Wed, 06 May 2020 12:24:44 -0400 X-MC-Unique: nGud8RIKODizIHOwsM99Vg-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 976501005510; Wed, 6 May 2020 16:24:42 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-227.ams2.redhat.com [10.36.112.227]) by smtp.corp.redhat.com (Postfix) with ESMTP id CDA6E60619; Wed, 6 May 2020 16:24:39 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v7 01/43] MdeModulePkg: Create PCDs to be used in support of SEV-ES To: Tom Lendacky , "Dong, Eric" , "devel@edk2.groups.io" Cc: "Justen, Jordan L" , Ard Biesheuvel , "Kinney, Michael D" , "Gao, Liming" , "Ni, Ray" , Brijesh Singh , "Wang, Jian J" , "Wu, Hao A" References: <3af2e4a8-fa4d-bb29-a282-c406ada7cf06@amd.com> <66c3ab32-0b1c-676b-2924-f14214be5de6@redhat.com> <226c8566-6550-613c-e07c-636686a851f9@amd.com> From: "Laszlo Ersek" Message-ID: Date: Wed, 6 May 2020 18:24:35 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <226c8566-6550-613c-e07c-636686a851f9@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 05/06/20 15:19, Tom Lendacky wrote: > 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 ; Dong, Eric >>> ; devel@edk2.groups.io >>> Cc: Justen, Jordan L ; Ard Biesheuvel >>> ; Kinney, Michael D >>> ; Gao, Liming ; Ni, >>> Ray >>> ; Brijesh Singh ; Wang, Jian J >>> ; Wu, Hao A >>> 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%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FEDK-II-Development-&data=02%7C01%7Cthomas.lendacky%40amd.com%7C9cff3475aff84a95728508d7f1604c99%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243268217382019&sdata=3%2FIKB174QaVLaqO0u1gdrL0izXmhEZ%2Byvj3iC13UYBc%3D&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%2Fgithub.com%2Ftianocore%2Fedk2-platforms&data=02%7C01%7Cthomas.lendacky%40amd.com%7C9cff3475aff84a95728508d7f1604c99%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243268217382019&sdata=jU0qrB%2BV6ZvFmPzjcxGo9o2Pu1%2FrhRW0gUZTMv%2BiXDQ%3D&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 think that Eric refers to various (as of yet, unspecified) platform DSCs in the edk2-platforms tree, which consume edk2. https://github.com/tianocore/edk2-platforms/ I don't fully agree with Eric on this observation. I agree somewhat. I have always made the argument that *all* platform code should reside in edk2; in other words, we should not have a separate edk2-platforms repository. Then it is feasible for a contributor to adapt dependent platform code in the same patch series that modifies core code. But I have not been successful in arguing against the edk2-platforms tree's existence, and so we have two projects now, with separate git histories. Therefore, the pattern sometiems followed is to split the edk2 (core) patch series into multiple sub-series, and to post an edk2-platforms series in the middle, to help edk2-platforms cope with the edk2 changes. The question is of course how complicated this is. If the edk2-platforms patches are not difficult, then I think the edk2-platforms maintainers may ask the contributor to submit patches for edk2-platforms too, as a *courtesy*. But if the edk2-platforms patches are many or complicated, then I think it's *unfair* to block edk2 changes *only* due to edk2-platforms dependencies. The edk2 contributor may not be familiar with the edk2-platforms stuff at all; worse, firmware platforms in the latter tree have often very quirky build instructions. Inside a single git tree, a contributor is justifiedly expected to track down dependencies and to update them (see for example Tom's patches for UefiPayloadPkg!), but in other trees, that's not necessarily the case. If we'd like Tom to adapt edk2-platforms to the edk2 changes, and maybe even restructure the edk2 series in order to accommodate this, then the *bare minimum* is to provide specific error messages and build instructions. > >> 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%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki&data=02%7C01%7Cthomas.lendacky%40amd.com%7C9cff3475aff84a95728508d7f1604c99%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243268217382019&sdata=xLkoV4zWhxtsbqszqPc0lEAl%2BYLL%2B2wg1nIXql8a64E%3D&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. This should be approached from the "out of tree platform build errors" angle described above. For example, if there's a pre-existent / widely used library instance in UefiCpuPkg, and you add a new library class dependency to it, such that even the library class is entirely new in edk2, that will require existent platforms to resolve the new lib class to a proper lib instence in their DSC files -- otherwise they will get a build error. The wiki page should basically explain such DSC updates in natural language. If there are new PCDs in UefiCpuPkg.dec that a platform might want to customize in its DSC file, that could be useful to mention in the Wiki page too. Because SEV-ES is only really relevant for OVMF at this point, I think the wiki page should only help out of tree platform owners prevent build errors, at this point. But for that, Eric should please provide build instructions and error messages for such out of tree platforms that actually break, with this edk2 series applied. > >> >> >> 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%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FEDK-II-Release-Planning&data=02%7C01%7Cthomas.lendacky%40amd.com%7C9cff3475aff84a95728508d7f1604c99%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243268217382019&sdata=kcDVjYHMS9bRRZOlKEk5ynFNT39AnxchJAMak%2Bn870I%3D&reserved=0 >> > > Thanks. Is there anyone in particular that I need to request this > feature be added? Here's a general approach: (1) Clone the git repository that is the edk2 wiki, locally: git://github.com/tianocore/tianocore.github.io.wiki or https://github.com/tianocore/tianocore.github.io.wiki.git (2) On a topic branch, implement your wiki changes. In this case, the "EDK-II-Release-Planning.md" file should be updated. Pick one of the upcoming stable releases, and list under it. Use existent items as example. (3) In order to check a "rendered" view of your edk2 wiki changes, I recommend having a personal fork of the edk2 project on github. Then, you can use the wiki associated with *that* personal project as a personal review space for your edk2 wiki changes. For example, the personal wiki that I use for this purpose has the following URL in my clone: git@github.com:lersek/edk2.wiki.git ^^^^^^ ^^^^ and I use the following URL in my browser: https://github.com/lersek/edk2/wiki ^^^^^^ ^^^^ (4) Push your local changes (on the local topic branch) to the remote *master* branch (github.com ignores branches different from "master" in wiki repositories). Verify the "rendered" view of your wiki changes. (5) Post the patch to edk2-devel, using the subject prefix "edk2-wiki PATCH" and also include a URL to the "rendered view" from (4). I don't know the exact set of people having push access to the wiki; CC me on your wiki patch, and I'll push it. Thanks Laszlo