Re: [RFC PATCH 00/23] KVM SGX virtualization support

Jarkko Sakkinen
2021-01-12T02:03:49Z

On Tue, Jan 12, 2021 at 02:14:28PM +1300, Kai Huang wrote:
> On Mon, 11 Jan 2021 19:20:48 +0200 Jarkko Sakkinen wrote:
> > On Wed, 2021-01-06 at 14:55 +1300, Kai Huang wrote:
> > > --- Disclaimer ---
> > >
> > > These patches were originally written by Sean Christopherson while at Intel.
> > > Now that Sean has left Intel, I (Kai) have taken over getting them upstream.
> > > This series needs more review before it can be merged. It is being posted
> > > publicly and under RFC so Sean and others can review it. Maintainers are safe
> > > ignoring it for now.
> > >
> > > ------------------
> > >
> > > Hi all,
> > >
> > > This series adds KVM SGX virtualization support. The first 12 patches starting
> > > with x86/sgx or x86/cpu.. are necessary changes to x86 and SGX core/driver to
> > > support KVM SGX virtualization, while the rest are patches to KVM subsystem.
> > >
> > > Please help to review this series. Also I'd like to hear what is the proper
> > > way to merge this series, since it contains change to both x86/SGX and KVM
> > > subsystem. Any feedback is highly appreciated. And please let me know if I
> > > forgot to CC anyone, or anyone wants to be removed from CC. Thanks in advance!
> > >
> > > This series is based against latest tip tree's x86/sgx branch. You can also get
> > > the code from tip branch of kvm-sgx repo on github:
> > >
> > > https://github.com/intel/kvm-sgx.git tip
> > >
> > > It also requires Qemu changes to create VM with SGX support. You can find Qemu
> > > repo here:
> > >
> > > https://github.com/intel/qemu-sgx.git next
> > >
> > > Please refer to README.md of above qemu-sgx repo for detail on how to create
> > > guest with SGX support. At meantime, for your quick reference you can use below
> > > command to create SGX guest:
> > >
> > > #qemu-system-x86_64 -smp 4 -m 2G -drive file=<your_vm_image>,if=virtio \
> > > -cpu host,+sgx_provisionkey \
> > > -sgx-epc id=epc1,memdev=mem1 \
> > > -object memory-backend-epc,id=mem1,size=64M,prealloc
> > >
> > > Please note that the SGX relevant part is:
> > >
> > > -cpu host,+sgx_provisionkey \
> > > -sgx-epc id=epc1,memdev=mem1 \
> > > -object memory-backend-epc,id=mem1,size=64M,prealloc
> > >
> > > And you can change other parameters of your qemu command based on your needs.
> >
> > Thanks a lot documenting these snippets to the cover letter. I dig these
> > up from lore once my environment is working.
> >
> > I'm setting up Arch based test environment with the eye on this patch set
> > and generic Linux keyring patches:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/arch.git/
> >
> > Still have some minor bits to adjust before I can start deploying it for SGX
> > testing. For this patch set I'll use two instances of it.
>
> Thanks. Please let me know if you need anything more.
>
> >
> > > =========
> > > KVM SGX virtualization Overview
> > >
> > > - Virtual EPC
> > >
> > > "Virtual EPC" is the EPC section exposed by KVM to guest so SGX software in
> > > guest can discover it and use it to create SGX enclaves. KVM exposes SGX to
> >
> > Virtual EPC is a representation of an EPC section. And there is no "the".
> >
> > > guest via CPUID, and exposes one or more "virtual EPC" sections for guest.
> > > The size of "virtual EPC" is passed as Qemu parameter when creating the
> > > guest, and the base address is calcualted internally according to guest's
> > > configuration.
> > >
> > > To support virtual EPC, add a new misc device /dev/sgx_virt_epc to SGX
> > > core/driver to allow userspace (Qemu) to allocate "raw" EPC, and use it as
> > > "virtual EPC" for guest. Obviously, unlike EPC allocated for host SGX driver,
> > > virtual EPC allocated via /dev/sgx_virt_epc doesn't have enclave associated,
> > > and how virtual EPC is used by guest is compeletely controlled by guest's SGX
> > > software.
> >
> > I think that /dev/sgx_vepc would be a clear enough name for the device. This
> > text has now a bit confusing "terminology" related to this.
>
> /dev/sgx_virt_epc may be clearer from userspace's perspective, for instance,
> if people see /dev/sgx_vepc, they may have to think about what it is,
> while /dev/sgx_virt_epc they may not.
>
> But I don't have strong objection here. Does anyone has anything to say here?

It's already an abberevation to start with, why leave it halfways?

Especially when three remaining words have been shrunk to single
characters ('E', 'P' and 'C').

/Jarkko