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

Jarkko Sakkinen
2021-01-16T09:51:54Z

On Sat, Jan 16, 2021 at 11:31:49AM +0200, Jarkko Sakkinen wrote:
> On Sat, Jan 16, 2021 at 03:43:18AM +1300, Kai Huang wrote:
> > On Tue, 2021-01-12 at 15:07 +1300, Kai Huang wrote:
> > > > > > >
> > > > > > > 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').
> > > >
> > >
> > > I have expressed my opinion above. And as I said I don't have strong objection
> > > here. I'll change to /dev/sgx_vepc if no one opposes.
> >
> > Hi Jarkko,
> >
> > I am reluctant to change to /dev/sgx_vepc now, because there are lots of
> > 'sgx_virt_epc' in the code. For instance, 'struct sgx_virt_epc', and function names
> > in sgx/virt.c are all sgx_virt_epc_xxx(), which has 'sgx_virt_epc' as prefix. I feel
> > changing to /dev/sgx_vepc only is kinda incomplete, but I really don't want to change
> > so many 'sgx_virt_epc' to 'sgx_vepc'.
> >
> > (Plus I still think 'virt_epc' is more obvious than 'vepc' from userspace's
> > perspective.)
> >
> > Does it make sense?
>
> We can reconsider naming later on for sure, and maybe it's better to do
> so. It's probably too early to define the final name.
>
> As far as naming goes, I'm actually wondering is this usable outside of
> KVM by any means? If not, then probably the best name for this device
> would be sgx_kvm_epc. Better to be always as explicit as possible.

You can easily do such renames with git filter-branch over a patch set:

https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History

Having to rename something in too many places is not an argument.
Considering it too early is.

/Jarkko