[edk2-devel] [PATCH 0/2] Update SevSecret API to work for TDX

James Bottomley posted 2 patches 3 years, 4 months ago
Failed in applying to current master (apply log)
OvmfPkg/OvmfPkg.dec                                |  2 +-
OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf             |  2 +-
...aunchSecret.h => ConfidentialComputingSecret.h} | 14 +++++++-------
OvmfPkg/AmdSev/SecretDxe/SecretDxe.c               |  6 +++---
4 files changed, 12 insertions(+), 12 deletions(-)
rename OvmfPkg/Include/Guid/{SevLaunchSecret.h => ConfidentialComputingSecret.h} (65%)
[edk2-devel] [PATCH 0/2] Update SevSecret API to work for TDX
Posted by James Bottomley 3 years, 4 months ago
This patch series changes the EFI configuration table information
which is queried by the bootloader to make it more compatible with
Intel TDX.  The first patch changes the ABI to make the table contain
two 64 bit integers instead of two 32 bit ones.  The second patch is a
cosmetic one to change the names of the GUIDs and tables to have a
confidential computing prefix instead of a SEV Launch one.

The first patch *must* be applied before the next stable tag to avoid
ABI breakage.  The second is purely cosmetic and doesn't change the
code output.

Ultimately there will still need to be a TDX collector for the secret,
which would feed the value into the SecretDxe, but these changes
should ensure that no further changes would be required by the secret
consumers.

James

---

James Bottomley (2):
  OvmfPkg: Change SEV Launch Secret API to be UINT64 for base and size
  OvmfPkg/AmdSev/SecretDxe: make secret location naming generic

 OvmfPkg/OvmfPkg.dec                                |  2 +-
 OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf             |  2 +-
 ...aunchSecret.h => ConfidentialComputingSecret.h} | 14 +++++++-------
 OvmfPkg/AmdSev/SecretDxe/SecretDxe.c               |  6 +++---
 4 files changed, 12 insertions(+), 12 deletions(-)
 rename OvmfPkg/Include/Guid/{SevLaunchSecret.h => ConfidentialComputingSecret.h} (65%)

-- 
2.26.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68919): https://edk2.groups.io/g/devel/message/68919
Mute This Topic: https://groups.io/mt/78991600/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/2] Update SevSecret API to work for TDX
Posted by Yao, Jiewen 3 years, 4 months ago
Series: Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: James Bottomley <jejb@linux.ibm.com>
> Sent: Wednesday, December 16, 2020 9:42 AM
> To: devel@edk2.groups.io
> Cc: dovmurik@linux.vnet.ibm.com; Dov.Murik1@il.ibm.com;
> ashish.kalra@amd.com; brijesh.singh@amd.com; tobin@ibm.com;
> david.kaplan@amd.com; jon.grimm@amd.com; thomas.lendacky@amd.com;
> jejb@linux.ibm.com; frankeh@us.ibm.com; Dr . David Alan Gilbert
> <dgilbert@redhat.com>; Laszlo Ersek <lersek@redhat.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Ard Biesheuvel <ard.biesheuvel@arm.com>;
> Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH 0/2] Update SevSecret API to work for TDX
> 
> This patch series changes the EFI configuration table information
> which is queried by the bootloader to make it more compatible with
> Intel TDX.  The first patch changes the ABI to make the table contain
> two 64 bit integers instead of two 32 bit ones.  The second patch is a
> cosmetic one to change the names of the GUIDs and tables to have a
> confidential computing prefix instead of a SEV Launch one.
> 
> The first patch *must* be applied before the next stable tag to avoid
> ABI breakage.  The second is purely cosmetic and doesn't change the
> code output.
> 
> Ultimately there will still need to be a TDX collector for the secret,
> which would feed the value into the SecretDxe, but these changes
> should ensure that no further changes would be required by the secret
> consumers.
> 
> James
> 
> ---
> 
> James Bottomley (2):
>   OvmfPkg: Change SEV Launch Secret API to be UINT64 for base and size
>   OvmfPkg/AmdSev/SecretDxe: make secret location naming generic
> 
>  OvmfPkg/OvmfPkg.dec                                |  2 +-
>  OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf             |  2 +-
>  ...aunchSecret.h => ConfidentialComputingSecret.h} | 14 +++++++-------
>  OvmfPkg/AmdSev/SecretDxe/SecretDxe.c               |  6 +++---
>  4 files changed, 12 insertions(+), 12 deletions(-)
>  rename OvmfPkg/Include/Guid/{SevLaunchSecret.h =>
> ConfidentialComputingSecret.h} (65%)
> 
> --
> 2.26.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68924): https://edk2.groups.io/g/devel/message/68924
Mute This Topic: https://groups.io/mt/78991600/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/2] Update SevSecret API to work for TDX
Posted by Laszlo Ersek 3 years, 4 months ago
+Mike

On 12/16/20 02:41, James Bottomley wrote:
> This patch series changes the EFI configuration table information
> which is queried by the bootloader to make it more compatible with
> Intel TDX.  The first patch changes the ABI to make the table contain
> two 64 bit integers instead of two 32 bit ones.  The second patch is a
> cosmetic one to change the names of the GUIDs and tables to have a
> confidential computing prefix instead of a SEV Launch one.
> 
> The first patch *must* be applied before the next stable tag to avoid
> ABI breakage.  The second is purely cosmetic and doesn't change the
> code output.
> 
> Ultimately there will still need to be a TDX collector for the secret,
> which would feed the value into the SecretDxe, but these changes
> should ensure that no further changes would be required by the secret
> consumers.
> 
> James
> 
> ---
> 
> James Bottomley (2):
>   OvmfPkg: Change SEV Launch Secret API to be UINT64 for base and size
>   OvmfPkg/AmdSev/SecretDxe: make secret location naming generic
> 
>  OvmfPkg/OvmfPkg.dec                                |  2 +-
>  OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf             |  2 +-
>  ...aunchSecret.h => ConfidentialComputingSecret.h} | 14 +++++++-------
>  OvmfPkg/AmdSev/SecretDxe/SecretDxe.c               |  6 +++---
>  4 files changed, 12 insertions(+), 12 deletions(-)
>  rename OvmfPkg/Include/Guid/{SevLaunchSecret.h => ConfidentialComputingSecret.h} (65%)
> 

series
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

I tried merging this:

https://github.com/tianocore/edk2/pull/1235

but the Ubuntu builds all failed. I've checked two logs:

https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=16967&view=logs&j=cf2d8b26-a21c-5c68-abf4-b944c123e462&t=5ffbbe5c-1d3a-55f5-5ef3-8a0ef80d76a1&l=184
https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=16968&view=logs&j=47cf355a-6eb4-51a8-46a8-ff4028bfcac0&t=beedef5d-00d0-5a8c-fa35-57d7319988c2&l=182

They say,

INFO - /bin/sh: 1: qemu-system-aarch64: not found
INFO - /bin/sh: 1: qemu-system-x86_64: not found

I guess I won't be merging the three patch sets that I had planned for
this evening...

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69148): https://edk2.groups.io/g/devel/message/69148
Mute This Topic: https://groups.io/mt/78991600/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/2] Update SevSecret API to work for TDX
Posted by Laszlo Ersek 3 years, 4 months ago
On 12/17/20 19:43, Laszlo Ersek wrote:

> I tried merging this:
> 
> https://github.com/tianocore/edk2/pull/1235
> 
> but the Ubuntu builds all failed. I've checked two logs:
> 
> https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=16967&view=logs&j=cf2d8b26-a21c-5c68-abf4-b944c123e462&t=5ffbbe5c-1d3a-55f5-5ef3-8a0ef80d76a1&l=184
> https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=16968&view=logs&j=47cf355a-6eb4-51a8-46a8-ff4028bfcac0&t=beedef5d-00d0-5a8c-fa35-57d7319988c2&l=182
> 
> They say,
> 
> INFO - /bin/sh: 1: qemu-system-aarch64: not found
> INFO - /bin/sh: 1: qemu-system-x86_64: not found

The "Install qemu" tasks earlier seem to complete:

https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=16967&view=logs&j=cf2d8b26-a21c-5c68-abf4-b944c123e462&t=a5c654c1-e049-5a30-61a9-da81b8ec031f
https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=16968&view=logs&j=47cf355a-6eb4-51a8-46a8-ff4028bfcac0&t=9a629c6e-a36d-5733-3aff-19ed2a42cf75

However, the qemu "4.2-3ubuntu6.10" package is a dummy package:

  https://packages.ubuntu.com/focal/qemu

and as shown under the link, it has no dependency on the packages with the actual qemu executables. So the latter do not get pulled in.

(Even the logs make that clear: "Need to get 14.3 kB of archives" -- obviously, a real QEMU won't fit in that, and no other packages get pulled in).

The meta-package that pulls in all system emulators is called "qemu-system":

  https://packages.ubuntu.com/focal/qemu-system

What I don't understand at this point is how the CI scripts could work previously.

... Aha! I do understand it now. Look at one of the last successful PRs:

  https://github.com/tianocore/edk2/pull/1232

The CI logs contain this message:

  "##[warning]Ubuntu-latest pipelines will use Ubuntu-20.04 soon. For more details, see https://github.com/actions/virtual-environments/issues/1816"

So let's check out:

  https://github.com/actions/virtual-environments/issues/1816

Okay... so it looks like I'm the victim of "Ubuntu-latest" switching to 20.04 ("focal") from 18.04 ("bionic"). Compare the "qemu" package in both:

  https://packages.ubuntu.com/bionic/qemu
  https://packages.ubuntu.com/focal/qemu

In the former, qemu depends on qemu-system (which depends further on the actual emulator subpackages), in the latter, qemu doesn't depend on anything.

According to <https://github.com/actions/virtual-environments/issues/1816>, we could change:

.azurepipelines/Ubuntu-GCC5.yml:    vm_image: 'ubuntu-latest'
.azurepipelines/Ubuntu-PatchCheck.yml:  vmImage: 'ubuntu-latest'
ArmVirtPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml:      vm_image: 'ubuntu-latest'
EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml:      vm_image: 'ubuntu-latest'
OvmfPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml:      vm_image: 'ubuntu-latest'

to "ubuntu-18.04". But perhaps we should change:

ArmVirtPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml:        - bash: sudo apt-get install qemu
OvmfPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml:        - bash: sudo apt-get install qemu

to "qemu-system", instead.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69150): https://edk2.groups.io/g/devel/message/69150
Mute This Topic: https://groups.io/mt/78991600/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/2] Update SevSecret API to work for TDX
Posted by Laszlo Ersek 3 years, 4 months ago
On 12/17/20 20:23, Laszlo Ersek wrote:
> On 12/17/20 19:43, Laszlo Ersek wrote:
> 
>> I tried merging this:
>>
>> https://github.com/tianocore/edk2/pull/1235
>>
>> but the Ubuntu builds all failed. I've checked two logs:
>>
>> https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=16967&view=logs&j=cf2d8b26-a21c-5c68-abf4-b944c123e462&t=5ffbbe5c-1d3a-55f5-5ef3-8a0ef80d76a1&l=184
>> https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=16968&view=logs&j=47cf355a-6eb4-51a8-46a8-ff4028bfcac0&t=beedef5d-00d0-5a8c-fa35-57d7319988c2&l=182
>>
>> They say,
>>
>> INFO - /bin/sh: 1: qemu-system-aarch64: not found
>> INFO - /bin/sh: 1: qemu-system-x86_64: not found
> 
> The "Install qemu" tasks earlier seem to complete:
> 
> https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=16967&view=logs&j=cf2d8b26-a21c-5c68-abf4-b944c123e462&t=a5c654c1-e049-5a30-61a9-da81b8ec031f
> https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=16968&view=logs&j=47cf355a-6eb4-51a8-46a8-ff4028bfcac0&t=9a629c6e-a36d-5733-3aff-19ed2a42cf75
> 
> However, the qemu "4.2-3ubuntu6.10" package is a dummy package:
> 
>   https://packages.ubuntu.com/focal/qemu
> 
> and as shown under the link, it has no dependency on the packages with the actual qemu executables. So the latter do not get pulled in.
> 
> (Even the logs make that clear: "Need to get 14.3 kB of archives" -- obviously, a real QEMU won't fit in that, and no other packages get pulled in).
> 
> The meta-package that pulls in all system emulators is called "qemu-system":
> 
>   https://packages.ubuntu.com/focal/qemu-system
> 
> What I don't understand at this point is how the CI scripts could work previously.
> 
> ... Aha! I do understand it now. Look at one of the last successful PRs:
> 
>   https://github.com/tianocore/edk2/pull/1232
> 
> The CI logs contain this message:
> 
>   "##[warning]Ubuntu-latest pipelines will use Ubuntu-20.04 soon. For more details, see https://github.com/actions/virtual-environments/issues/1816"
> 
> So let's check out:
> 
>   https://github.com/actions/virtual-environments/issues/1816
> 
> Okay... so it looks like I'm the victim of "Ubuntu-latest" switching to 20.04 ("focal") from 18.04 ("bionic"). Compare the "qemu" package in both:
> 
>   https://packages.ubuntu.com/bionic/qemu
>   https://packages.ubuntu.com/focal/qemu
> 
> In the former, qemu depends on qemu-system (which depends further on the actual emulator subpackages), in the latter, qemu doesn't depend on anything.
> 
> According to <https://github.com/actions/virtual-environments/issues/1816>, we could change:
> 
> .azurepipelines/Ubuntu-GCC5.yml:    vm_image: 'ubuntu-latest'
> .azurepipelines/Ubuntu-PatchCheck.yml:  vmImage: 'ubuntu-latest'
> ArmVirtPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml:      vm_image: 'ubuntu-latest'
> EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml:      vm_image: 'ubuntu-latest'
> OvmfPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml:      vm_image: 'ubuntu-latest'
> 
> to "ubuntu-18.04". But perhaps we should change:
> 
> ArmVirtPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml:        - bash: sudo apt-get install qemu
> OvmfPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml:        - bash: sudo apt-get install qemu
> 
> to "qemu-system", instead.

Well, no. I've just tried that:

  https://github.com/tianocore/edk2/pull/1236

and we have two problems with it:

(1) the emulator is now available, but it crashes:

https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=16975&view=logs&j=cf2d8b26-a21c-5c68-abf4-b944c123e462&t=5ffbbe5c-1d3a-55f5-5ef3-8a0ef80d76a1

"INFO - Segmentation fault (core dumped)"

(2) in spite of this failure, the mergify bot says, "All checks passed.
Auto close personal build." :/

So at the moment we can only go back to the 18.04LTS image. I'll try
that next.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69151): https://edk2.groups.io/g/devel/message/69151
Mute This Topic: https://groups.io/mt/78991600/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/2] Update SevSecret API to work for TDX
Posted by Laszlo Ersek 3 years, 4 months ago
On 12/16/20 02:41, James Bottomley wrote:
> This patch series changes the EFI configuration table information
> which is queried by the bootloader to make it more compatible with
> Intel TDX.  The first patch changes the ABI to make the table contain
> two 64 bit integers instead of two 32 bit ones.  The second patch is a
> cosmetic one to change the names of the GUIDs and tables to have a
> confidential computing prefix instead of a SEV Launch one.
> 
> The first patch *must* be applied before the next stable tag to avoid
> ABI breakage.  The second is purely cosmetic and doesn't change the
> code output.
> 
> Ultimately there will still need to be a TDX collector for the secret,
> which would feed the value into the SecretDxe, but these changes
> should ensure that no further changes would be required by the secret
> consumers.
> 
> James
> 
> ---
> 
> James Bottomley (2):
>   OvmfPkg: Change SEV Launch Secret API to be UINT64 for base and size
>   OvmfPkg/AmdSev/SecretDxe: make secret location naming generic
> 
>  OvmfPkg/OvmfPkg.dec                                |  2 +-
>  OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf             |  2 +-
>  ...aunchSecret.h => ConfidentialComputingSecret.h} | 14 +++++++-------
>  OvmfPkg/AmdSev/SecretDxe/SecretDxe.c               |  6 +++---
>  4 files changed, 12 insertions(+), 12 deletions(-)
>  rename OvmfPkg/Include/Guid/{SevLaunchSecret.h => ConfidentialComputingSecret.h} (65%)
> 

Merged as commit range c487970ac89d..96201ae7bf97, via
<https://github.com/tianocore/edk2/pull/1235>.

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69180): https://edk2.groups.io/g/devel/message/69180
Mute This Topic: https://groups.io/mt/78991600/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-