Re: [PATCH] x86/sgx: Declare sgx_set_attribute() for !CONFIG_X86_SGX

From: Jarkko Sakkinen
Date: Fri Sep 03 2021 - 11:58:59 EST


On Fri, 2021-09-03 at 15:29 +0000, Sean Christopherson wrote:
> On Fri, Sep 03, 2021, Jarkko Sakkinen wrote:
> > Simplify sgx_set_attribute() usage by declaring a fallback
> > implementation for it rather than requiring to have compilation
> > flag checks in the call site. The fallback unconditionally returns
> > -EINVAL.
> >
> > Refactor the call site in kvm_vm_ioctl_enable_cap() accordingly.
> > The net result is the same: KVM_CAP_SGX_ATTRIBUTE causes -EINVAL
> > when kernel is compiled without CONFIG_X86_SGX_KVM.
>
> Eh, it doesn't really simplify the usage. If anything it makes it more convoluted
> because the capability check in kvm_vm_ioctl_check_extension() still needs an
> #ifdef, e.g. readers will wonder why the check is conditional but the usage is not.

It does objectively a bit, since it's one ifdef less.

This is fairly standard practice to do in kernel APIs, used in countless
places, for instance in Tony's patch set to add MCE recovery for SGX. And
it would be nice to share common pattern here how we define API now and
futre.

I also remarked that declaration of "sgx_provisioning_allowed" is not flagged,
which is IMHO even more convolved because without SGX it is spare data.

/Jarkko