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

From: Jarkko Sakkinen <jarkko@kernel.org>
To: Kai Huang <kai.huang@intel.com>
Cc: linux-sgx@vger.kernel.org, kvm@vger.kernel.org, x86@kernel.org,
	seanjc@google.com, luto@kernel.org, dave.hansen@intel.com,
	haitao.huang@intel.com, pbonzini@redhat.com, bp@alien8.de,
	tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com,
	jethro@fortanix.com, b.thiel@posteo.de, jmattson@google.com,
	joro@8bytes.org, vkuznets@redhat.com, wanpengli@tencent.com,
	corbet@lwn.net
Subject: Re: [RFC PATCH 00/23] KVM SGX virtualization support
Date: Sat, 16 Jan 2021 11:50:49 +0200	[thread overview]
Message-ID: <YAK2+d0OFFbo9uJ0@kernel.org> (raw)
In-Reply-To: <YAKyfrrl6AlWIQhw@kernel.org>

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