From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f171.google.com (mail-pl1-f171.google.com [209.85.214.171]) by mx.groups.io with SMTP id smtpd.web12.15781.1619024845057684124 for ; Wed, 21 Apr 2021 10:07:25 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@google.com header.s=20161025 header.b=O0JKR0Ad; spf=pass (domain: google.com, ip: 209.85.214.171, mailfrom: erdemaktas@google.com) Received: by mail-pl1-f171.google.com with SMTP id u15so13459808plf.10 for ; Wed, 21 Apr 2021 10:07:25 -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=IxVPEf7SAbP3d6mSknyjjrl6Z5kcZB3xs82GutMNoos=; b=O0JKR0AdLKPIq3ovdFGD1AFdJAa5bhINgXQq0YRT7ydvry7wmFRV8rJ5+XsmfH0+S8 Ajwmju7FEQTMuZhslGbL8aA8Tg8GQokKVDYds4+EbV6QD3ZnyR+4DRte6aUuhuORLyPY lqfsMGbE/YBdeVh2MTQLVU5kCmBl0+tsKubET/NDzpNHXmVBTE1ET1aZ6ZyqizvL3Y7o VEgAyg8YFWZNVk1kZiGeyrA7v+1B7b+YLhhjtQpJDewA38gvPUxzIGRj6vNqW1EtoXMM JMYOdZOgKNlqzih9qOnIxo9wUi2knmknV0QycEkkyfLCGNJkLC+jOOqFE3WG4y9gzKGq 29/w== 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=IxVPEf7SAbP3d6mSknyjjrl6Z5kcZB3xs82GutMNoos=; b=QHG9ubHdfoiAatHVba1KLaM2mlqGOUtcqdAYtFVHJr1XVjWPAKln9dZvoTOW0WW//u b6BBvs3wbj/Cq5L0RmDgBpN8/CQUkyykQDGEpeGDgicYFpN7l8bgsoGobWD1RNpHAdF3 hA5fYGv84H2m7NDfgTnbTomST0UGCd6mW1xkGw30w0uZ62m/iFUP9KEG6ybvAoIq6LhJ NEqc5/zVua1Bj85XrADbpUD+Z4ji/Uyw/diavN8ck1VDGcaVJ+FNZjVR+XwAsDanDKsI 9fa2LgBFK7luxWxgTZpKe16vBHTBDZCY/P1VHV3uyB5RKUqBKkdzZyHn8W7jMlpSlbyh VNMg== X-Gm-Message-State: AOAM530863S1udKK6lHkaEwXIq6v7yy308WqZaaRP5nOwXgit5Tt3msL ujoo1dorcyeCbbCR9nVxC5TIGgiL5jqPgiDh+SUO439klewG8Z4t X-Google-Smtp-Source: ABdhPJyHazY8RR0gqJTQwlR8yBhZmryXPAh0BgXdJpXG94oWemxxrXJk+CfPjMpOyM4IdCfwvXNRUKQLxbU8xESoxG8= X-Received: by 2002:a17:902:e993:b029:ec:7cc0:9390 with SMTP id f19-20020a170902e993b02900ec7cc09390mr27944160plb.27.1619024844024; Wed, 21 Apr 2021 10:07:24 -0700 (PDT) MIME-Version: 1.0 References: <719a63e555376ca65a7bbe0c7e23c20b6b631cd3.camel@linux.ibm.com> <9aa00ba0-def0-9a4e-1578-0b55b8047ebd@redhat.com> <2ff2c569-1032-3e5f-132a-159c47c9f067@amd.com> <18180548-016d-4e37-68fd-050dfc3b4e77@redhat.com> <5183d5fd-9bba-6f0a-52e0-a3e27a6784de@redhat.com> In-Reply-To: From: "Erdem Aktas" Date: Wed, 21 Apr 2021 10:07:12 -0700 Message-ID: Subject: Re: [edk2-devel] separate OVMF binary for TDX? [was: OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest] To: devel@edk2.groups.io, Laszlo Ersek Cc: "Yao, Jiewen" , Paolo Bonzini , "jejb@linux.ibm.com" , "dgilbert@redhat.com" , "Xu, Min M" , "thomas.lendacky@amd.com" , Brijesh Singh , "Justen, Jordan L" , Ard Biesheuvel , Nathaniel McCallum , Ning Yang Content-Type: multipart/alternative; boundary="000000000000b66f5805c07e9844" --000000000000b66f5805c07e9844 Content-Type: text/plain; charset="UTF-8" Hi Laszlo, 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. 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. 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. >>Can Google at least propose a designated reviewer ("R") for the >>"OvmfPkg: Confidential Computing" section of "Maintainers.txt", in a patch? Sure I would be happy too. -Erdem On Wed, Apr 21, 2021 at 3:44 AM Laszlo Ersek wrote: > On 04/21/21 02:38, Yao, Jiewen wrote: > > Hello > > Do we have some conclusion on this topic? > > > > Do we agree the one-binary solution in OVMF or we need more discussion? > > Well it's not technically impossible to do, just very ugly and brittle. > And I'm doubtful that this is a unique problem ("just fix the reset > vector") the likes of which will supposedly never return during the > integration of SEV and TDX. Once we make this promise ("one firmware > binary at all costs"), the hacks we accept for its sake will only > accumulate over time, and we'll have more and more precedent to justify > the next hack. Technical debt is not exactly what we don't have enough > of, in edk2. > > I won't make a secret out of the fact that I'm slightly annoyed that > this approach is being dictated by Google (as far as I understand, at > this point, anyway). I don't see or recall a lot of Google contributions > in the edk2 history or the bug tracker. I'm not enthusiastic about > complexity without explicit commitment / investment on the beneficiary's > side. > > I won't nack the approach personally, but I'm quite unhappy about it. > Can Google at least propose a designated reviewer ("R") for the > "OvmfPkg: Confidential Computing" section of "Maintainers.txt", in a patch? > > Thanks > Laszlo > > > > > > > Thank you > > Yao Jiewen > > > >> -----Original Message----- > >> From: Erdem Aktas > >> Sent: Friday, April 16, 2021 3:43 AM > >> To: Paolo Bonzini > >> Cc: devel@edk2.groups.io; jejb@linux.ibm.com; Yao, Jiewen > >> ; dgilbert@redhat.com; Laszlo Ersek > >> ; Xu, Min M ; > >> thomas.lendacky@amd.com; Brijesh Singh ; Justen, > >> Jordan L ; Ard Biesheuvel > >> ; Nathaniel McCallum > >> ; Ning Yang > >> Subject: Re: [edk2-devel] separate OVMF binary for TDX? [was: OvmfPkg: > >> Reserve the Secrets and Cpuid page for the SEV-SNP guest] > >> > >> Thanks Paolo. > >> > >> On Thu, Apr 15, 2021 at 12:59 AM Paolo Bonzini > >> wrote: > >>> > >>> On 15/04/21 01:34, Erdem Aktas wrote: > >>>> We do not want to generate different binaries for AMD, Intel, Intel > >>>> with TDX, AMD with SEV/SNP etc > >>> > >>> My question is why the user would want a single binary for VMs with and > >>> without TDX/SNP. I know there is attestation, but why would you even > >>> want the _possibility_ that your guest starts running without TDX or > SNP > >>> protection, and only find out later via attestation? > >> > >> There might be multiple reasons why customers want it but we need this > >> requirement for a couple of other reasons too. > >> > >> We do not only have hardware based confidential VMs. We might have > >> some other solutions which measure the initial image before boot. > >> Ultimately we might want to use a common attestation interface where > >> customers might be running different kinds of VMs. Using a single > >> binary will make it easier to manage/verify measurements for both of > >> us and the customers. I am not a PM so I cannot give more context on > >> customer use cases. > >> > >> Another reason is how we deploy and manage guest firmware. We have a > >> lot of optimization and customization to speed up firmware loading > >> time and also reduce the time to deploy new builds on the whole fleet > >> uniformly. Adding a new firmware binary is a big challenge for us to > >> enable these features. On the top of integration challenges, it will > >> create maintainability issues in the long run for us when we provide > >> tools to verify/reproduce the hashes in the attestation report. > >> > >>> want the _possibility_ that your guest starts running without TDX or > SNP > >>> protection, and only find out later via attestation? > >> > >> I am missing the point here. Customers should rely on only the > >> attestation report to establish the trust. > >> -If firmware does not support TDX and TDX is enabled, that firmware > >> will crash at some point. > >> -If firmware is generic firmware that supports TDX and SNP and others, > >> and TDX is enabled or not, still the customer needs to verify the TDX > >> enablement through attestation. > >> -If firmware is a customized binary compiled to support TDX, > >> irrelevant of TDX being enabled or not, still the customer needs to > >> verify the TDX enablement through attestation. > >> > >> > >>> For a similar reason, OVMF already supports shipping a binary that > fails > >>> to boot if SMM is not available to the firmware, because then secure > >>> boot would be trivially circumvented. > >>> > >>> I can understand having a single binary for both TDX or SNP. That's > not > >>> a problem since you can set up the SEV startup VMSA to 32-bit protected > >>> mode just like TDX wants. > >> > >> I agree that this is doable but I am not sure if we need to also > >> modify the reset vector for AMD SNP in that case. Also it will not > >> solve our problem. If we start to generate a new firmware for every > >> feature , it will not end well for us, I think. Both TDX and SNP are > >> still new features in the same architecture, and it seems to me that > >> they are sharing a lot of common/similar code. AMD has already made > >> some of their patches in (SEV and SEV-ES) which works very nicely for > >> our use case and integration. Looks like Intel just has an issue on > >> how to fix their reset vector problem. Once they solve it and upstream > >> accepts the changes, I do not see any other big blocker. OVMF was > >> doing a great job on abstracting differences and providing a common > >> interface without creating multiple binaries. I do not see why it > >> should not do the same thing here. > >> > >>>> therefore we were expecting the TDX > >>>> changes to be part of the upstream code. > >>> > >>> Having 1 or more binaries should be unrelated to the changes being > >>> upstream (or more likely, I am misunderstanding you). > >> > >> You are right, it is my bad for not clarifying it. What I mean is we > >> want it to be part of the upstream so it can be easier for us to pull > >> the changes and they are compatible with the changes that SNP is doing > >> but we also do not want to use different configuration files to > >> generate different binaries for each use case. > >> > >> > >>> Thanks, > >>> > >>> Paolo > >>> > > > > > > > --000000000000b66f5805c07e9844 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Laszlo,

I am sorry to hear that it s= ounded like we are dictating a certain approach. Although I can see why it = sounded that way, it certainly was not my intention.
We want to w= ork with the EDK2 community to have a solution that is beneficial=C2=A0for = everyone and we appreciate the inputs that we got from you and=C2=A0Paolo.= =C2=A0 Code quality is always a high priority for us. Therefore, if, at so= me point, things get too hacky to impact the quality/maintainability=C2=A0o= f the upstream code, we will consider making adjustments on our side.

With the current discussion, I was just trying to d= escribe our use case and the importance of having a single binary where the= re is no absolute need for architectural differences. As far as I know, the= only problematic=C2=A0area is modifying the reset vector to be compatible = with TDX and it seems like Intel has a solution for it without impacting th= e overall quality of the upstream code. I just want to reiterate that we ar= e 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 s= till thinking that what Intel is proposing is not feasible.=C2=A0

>>Can Google at least propose a designated reviewer (= "R") for the
>>"OvmfPkg: Confidential Computing&quo= t; section of "Maintainers.txt", in a patch?
Sure I= would be happy too.=C2=A0

-Erdem

<= div class=3D"gmail_quote">
On Wed, Apr= 21, 2021 at 3:44 AM Laszlo Ersek <= lersek@redhat.com> wrote:
On 04/21/21 02:38, Yao, Jiewen wrote:
> Hello
> Do we have some conclusion on this topic?
>
> Do we agree the one-binary solution in OVMF or we need more discussio= n?

Well it's not technically impossible to do, just very ugly and brittle= .
And I'm doubtful that this is a unique problem ("just fix the res= et
vector") the likes of which will supposedly never return during the 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
accumulate over time, and we'll have more and more precedent to justif= y
the next hack. Technical debt is not exactly what we don't have enough=
of, in edk2.

I won't make a secret out of the fact that I'm slightly annoyed th= at
this approach is being dictated by Google (as far as I understand, at
this point, anyway). I don't see or recall a lot of Google contributio= ns
in the edk2 history or the bug tracker. I'm not enthusiastic about
complexity without explicit commitment / investment on the beneficiary'= ;s
side.

I won't nack the approach personally, but I'm quite unhappy about = it.
Can Google at least propose a designated reviewer ("R") for the<= br> "OvmfPkg: Confidential Computing" section of "Maintainers.t= xt", in a patch?

Thanks
Laszlo

>
>
> Thank you
> Yao Jiewen
>
>> -----Original Message-----
>> From: Erdem Aktas <erdemaktas@google.com>
>> Sent: Friday, April 16, 2021 3:43 AM
>> To: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: dev= el@edk2.groups.io; jejb@linux.ibm.com; Yao, Jiewen
>> <jie= wen.yao@intel.com>; dgilbert@redhat.com; Laszlo Ersek
>> <lersek= @redhat.com>; Xu, Min M <min.m.xu@intel.com>;
>> thom= as.lendacky@amd.com; Brijesh Singh <brijesh.singh@amd.com>; Justen,
>> Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel
>> <ardb+tianocore@kernel.org>; Nathaniel McCallum
>> <np= mccallum@redhat.com>; Ning Yang <ningyang@google.com>
>> Subject: Re: [edk2-devel] separate OVMF binary for TDX? [was: Ovm= fPkg:
>> Reserve the Secrets and Cpuid page for the SEV-SNP guest]
>>
>> Thanks Paolo.
>>
>> On Thu, Apr 15, 2021 at 12:59 AM Paolo Bonzini <pbonzini@redhat.com>
>> wrote:
>>>
>>> On 15/04/21 01:34, Erdem Aktas wrote:
>>>> We do not want to generate different binaries for AMD, In= tel, Intel
>>>> with TDX, AMD with SEV/SNP etc
>>>
>>> My question is why the user would want a single binary for VM= s with and
>>> without TDX/SNP.=C2=A0 I know there is attestation, but why w= ould you even
>>> want the _possibility_ that your guest starts running without= TDX or SNP
>>> protection, and only find out later via attestation?
>>
>> There might be multiple reasons why customers want it but we need= this
>> requirement for a couple of other reasons too.
>>
>> We do not only have hardware based confidential VMs. We might hav= e
>> some other solutions which measure the initial image before boot.=
>> Ultimately we might want to use a common attestation interface wh= ere
>> customers might be running different kinds of VMs. Using a single=
>> binary will make it easier to manage/verify measurements for both= of
>> us and the customers. I am not a PM so I cannot give more context= on
>> customer use cases.
>>
>> Another reason is how we deploy and manage guest firmware. We hav= e a
>> lot of optimization and customization to speed up firmware loadin= g
>> time and also reduce the time to deploy new builds on the whole f= leet
>> uniformly.=C2=A0 Adding a new firmware binary is a big challenge = for us to
>> enable these features. On the top of integration challenges, it w= ill
>> create maintainability issues in the long run for us when we prov= ide
>> tools to verify/reproduce the hashes in the attestation report. >>
>>> want the _possibility_ that your guest starts running without= TDX or SNP
>>> protection, and only find out later via attestation?
>>
>> I am missing the point here. Customers should rely on only the >> attestation report to establish the trust.
>> -If firmware does not support TDX and TDX is enabled, that firmwa= re
>> will crash at some point.
>> -If firmware is generic firmware that supports TDX and SNP and ot= hers,
>> and TDX is enabled or not,=C2=A0 still the customer needs to veri= fy the TDX
>> enablement through attestation.
>> -If firmware is a customized binary compiled to support TDX,
>> irrelevant of TDX being enabled or not,=C2=A0 still the customer = needs to
>> verify the TDX enablement through attestation.
>>
>>
>>> For a similar reason, OVMF already supports shipping a binary= that fails
>>> to boot if SMM is not available to the firmware, because then= secure
>>> boot would be trivially circumvented.
>>>
>>> I can understand having a single binary for both TDX or SNP.= =C2=A0 That's not
>>> a problem since you can set up the SEV startup VMSA to 32-bit= protected
>>> mode just like TDX wants.
>>
>> 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 no= t
>> solve our problem. If we start to generate a new firmware for eve= ry
>> feature , it will not end well for us, I think. Both TDX and SNP = are
>> still new features in the same architecture, and it seems to me t= hat
>> they are sharing a lot of common/similar code. AMD has already ma= de
>> some of their patches in (SEV and SEV-ES) which works very nicely= for
>> our use case and integration. Looks like Intel just has an issue = on
>> how to fix their reset vector problem. Once they solve it and ups= tream
>> accepts the changes, I do not see any other big blocker. OVMF was=
>> doing a great job on abstracting differences and providing a comm= on
>> interface without creating multiple binaries. I do not see why it=
>> should not do the same thing here.
>>
>>>> therefore we were expecting the TDX
>>>> changes to be part of the upstream code.
>>>
>>> Having 1 or more binaries should be unrelated to the changes = being
>>> upstream (or more likely, I am misunderstanding you).
>>
>> You are right, it is my bad for not clarifying it. What I mean is= we
>> want it to be part of the upstream so it can be easier for us to = pull
>> the changes and they are compatible with the changes that SNP is = doing
>> but we also do not want to use different configuration files to >> generate different binaries for each use case.
>>
>>
>>> Thanks,
>>>
>>> Paolo
>>>






--000000000000b66f5805c07e9844--