<div dir="ltr">Hi Laszlo,<div><br></div><div>I am sorry to hear that it sounded like we are dictating a certain approach. Although I can see why it sounded that way, it certainly was not my intention.</div><div>We want to work with the EDK2 community to have a solution that is beneficial for everyone and we appreciate the inputs that we got from you and Paolo.  Code quality is always a high priority for us. Therefore, if, at some point, things get too hacky to impact the quality/maintainability of the upstream code, we will consider making adjustments on our side.<br></div><div><br></div><div>With the current discussion, I was just trying to describe our use case and the importance of having a single binary where there is no absolute need for architectural differences. As far as I know, the only problematic area is modifying the reset vector to be compatible with TDX and it seems like Intel has a solution for it without impacting the overall quality of the upstream code. I just want to reiterate that we are open for discussion and what we ask should not be considered "at all cost" and please let us know that if edk2 community/maintainers are still thinking that what Intel is proposing is not feasible. </div><div><br></div><div>>>Can Google at least propose a designated reviewer ("R") for the<br>>>"OvmfPkg: Confidential Computing" section of "Maintainers.txt", in a patch?<br></div><div>Sure I would be happy too. </div><div><br></div><div>-Erdem</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Apr 21, 2021 at 3:44 AM Laszlo Ersek <<a href="mailto:lersek@redhat.com">lersek@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 04/21/21 02:38, Yao, Jiewen wrote:<br>
> Hello <br>
> Do we have some conclusion on this topic?<br>
> <br>
> Do we agree the one-binary solution in OVMF or we need more discussion?<br>
<br>
Well it's not technically impossible to do, just very ugly and brittle.<br>
And I'm doubtful that this is a unique problem ("just fix the reset<br>
vector") the likes of which will supposedly never return during the<br>
integration of SEV and TDX. Once we make this promise ("one firmware<br>
binary at all costs"), the hacks we accept for its sake will only<br>
accumulate over time, and we'll have more and more precedent to justify<br>
the next hack. Technical debt is not exactly what we don't have enough<br>
of, in edk2.<br>
<br>
I won't make a secret out of the fact that I'm slightly annoyed that<br>
this approach is being dictated by Google (as far as I understand, at<br>
this point, anyway). I don't see or recall a lot of Google contributions<br>
in the edk2 history or the bug tracker. I'm not enthusiastic about<br>
complexity without explicit commitment / investment on the beneficiary's<br>
side.<br>
<br>
I won't nack the approach personally, but I'm quite unhappy about it.<br>
Can Google at least propose a designated reviewer ("R") for the<br>
"OvmfPkg: Confidential Computing" section of "Maintainers.txt", in a patch?<br>
<br>
Thanks<br>
Laszlo<br>
<br>
> <br>
> <br>
> Thank you<br>
> Yao Jiewen<br>
> <br>
>> -----Original Message-----<br>
>> From: Erdem Aktas <<a href="mailto:erdemaktas@google.com" target="_blank">erdemaktas@google.com</a>><br>
>> Sent: Friday, April 16, 2021 3:43 AM<br>
>> To: Paolo Bonzini <<a href="mailto:pbonzini@redhat.com" target="_blank">pbonzini@redhat.com</a>><br>
>> Cc: <a href="mailto:devel@edk2.groups.io" target="_blank">devel@edk2.groups.io</a>; <a href="mailto:jejb@linux.ibm.com" target="_blank">jejb@linux.ibm.com</a>; Yao, Jiewen<br>
>> <<a href="mailto:jiewen.yao@intel.com" target="_blank">jiewen.yao@intel.com</a>>; <a href="mailto:dgilbert@redhat.com" target="_blank">dgilbert@redhat.com</a>; Laszlo Ersek<br>
>> <<a href="mailto:lersek@redhat.com" target="_blank">lersek@redhat.com</a>>; Xu, Min M <<a href="mailto:min.m.xu@intel.com" target="_blank">min.m.xu@intel.com</a>>;<br>
>> <a href="mailto:thomas.lendacky@amd.com" target="_blank">thomas.lendacky@amd.com</a>; Brijesh Singh <<a href="mailto:brijesh.singh@amd.com" target="_blank">brijesh.singh@amd.com</a>>; Justen,<br>
>> Jordan L <<a href="mailto:jordan.l.justen@intel.com" target="_blank">jordan.l.justen@intel.com</a>>; Ard Biesheuvel<br>
>> <<a href="mailto:ardb%2Btianocore@kernel.org" target="_blank">ardb+tianocore@kernel.org</a>>; Nathaniel McCallum<br>
>> <<a href="mailto:npmccallum@redhat.com" target="_blank">npmccallum@redhat.com</a>>; Ning Yang <<a href="mailto:ningyang@google.com" target="_blank">ningyang@google.com</a>><br>
>> Subject: Re: [edk2-devel] separate OVMF binary for TDX? [was: OvmfPkg:<br>
>> Reserve the Secrets and Cpuid page for the SEV-SNP guest]<br>
>><br>
>> Thanks Paolo.<br>
>><br>
>> On Thu, Apr 15, 2021 at 12:59 AM Paolo Bonzini <<a href="mailto:pbonzini@redhat.com" target="_blank">pbonzini@redhat.com</a>><br>
>> wrote:<br>
>>><br>
>>> On 15/04/21 01:34, Erdem Aktas wrote:<br>
>>>> We do not want to generate different binaries for AMD, Intel, Intel<br>
>>>> with TDX, AMD with SEV/SNP etc<br>
>>><br>
>>> My question is why the user would want a single binary for VMs with and<br>
>>> without TDX/SNP.  I know there is attestation, but why would you even<br>
>>> want the _possibility_ that your guest starts running without TDX or SNP<br>
>>> protection, and only find out later via attestation?<br>
>><br>
>> There might be multiple reasons why customers want it but we need this<br>
>> requirement for a couple of other reasons too.<br>
>><br>
>> We do not only have hardware based confidential VMs. We might have<br>
>> some other solutions which measure the initial image before boot.<br>
>> Ultimately we might want to use a common attestation interface where<br>
>> customers might be running different kinds of VMs. Using a single<br>
>> binary will make it easier to manage/verify measurements for both of<br>
>> us and the customers. I am not a PM so I cannot give more context on<br>
>> customer use cases.<br>
>><br>
>> Another reason is how we deploy and manage guest firmware. We have a<br>
>> lot of optimization and customization to speed up firmware loading<br>
>> time and also reduce the time to deploy new builds on the whole fleet<br>
>> uniformly.  Adding a new firmware binary is a big challenge for us to<br>
>> enable these features. On the top of integration challenges, it will<br>
>> create maintainability issues in the long run for us when we provide<br>
>> tools to verify/reproduce the hashes in the attestation report.<br>
>><br>
>>> want the _possibility_ that your guest starts running without TDX or SNP<br>
>>> protection, and only find out later via attestation?<br>
>><br>
>> I am missing the point here. Customers should rely on only the<br>
>> attestation report to establish the trust.<br>
>> -If firmware does not support TDX and TDX is enabled, that firmware<br>
>> will crash at some point.<br>
>> -If firmware is generic firmware that supports TDX and SNP and others,<br>
>> and TDX is enabled or not,  still the customer needs to verify the TDX<br>
>> enablement through attestation.<br>
>> -If firmware is a customized binary compiled to support TDX,<br>
>> irrelevant of TDX being enabled or not,  still the customer needs to<br>
>> verify the TDX enablement through attestation.<br>
>><br>
>><br>
>>> For a similar reason, OVMF already supports shipping a binary that fails<br>
>>> to boot if SMM is not available to the firmware, because then secure<br>
>>> boot would be trivially circumvented.<br>
>>><br>
>>> I can understand having a single binary for both TDX or SNP.  That's not<br>
>>> a problem since you can set up the SEV startup VMSA to 32-bit protected<br>
>>> mode just like TDX wants.<br>
>><br>
>> I agree that this is doable but I am not sure if we need to also<br>
>> modify the reset vector for AMD SNP in that case. Also it will not<br>
>> solve our problem. If we start to generate a new firmware for every<br>
>> feature , it will not end well for us, I think. Both TDX and SNP are<br>
>> still new features in the same architecture, and it seems to me that<br>
>> they are sharing a lot of common/similar code. AMD has already made<br>
>> some of their patches in (SEV and SEV-ES) which works very nicely for<br>
>> our use case and integration. Looks like Intel just has an issue on<br>
>> how to fix their reset vector problem. Once they solve it and upstream<br>
>> accepts the changes, I do not see any other big blocker. OVMF was<br>
>> doing a great job on abstracting differences and providing a common<br>
>> interface without creating multiple binaries. I do not see why it<br>
>> should not do the same thing here.<br>
>><br>
>>>> therefore we were expecting the TDX<br>
>>>> changes to be part of the upstream code.<br>
>>><br>
>>> Having 1 or more binaries should be unrelated to the changes being<br>
>>> upstream (or more likely, I am misunderstanding you).<br>
>><br>
>> You are right, it is my bad for not clarifying it. What I mean is we<br>
>> want it to be part of the upstream so it can be easier for us to pull<br>
>> the changes and they are compatible with the changes that SNP is doing<br>
>> but we also do not want to use different configuration files to<br>
>> generate different binaries for each use case.<br>
>><br>
>><br>
>>> Thanks,<br>
>>><br>
>>> Paolo<br>
>>><br>
<br>
<br>
<br>
<br>
<br>
<br>
</blockquote></div>


 <div width="1" style="color:white;clear:both">_._,_._,_</div> <hr>   Groups.io Links:<p>   You receive all messages sent to this group.    <p> <a target="_blank" href="https://edk2.groups.io/g/devel/message/74336">View/Reply Online (#74336)</a> |    |  <a target="_blank" href="https://groups.io/mt/81969494/1813853">Mute This Topic</a>  | <a href="https://edk2.groups.io/g/devel/post">New Topic</a><br>    <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> |  <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a>  [edk2-devel-archive@redhat.com]<br> <div width="1" style="color:white;clear:both">_._,_._,_</div>