On Fri, 2021-09-03 at 18:58 +0300, Jarkko Sakkinen wrote: > 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. This should have had RFC tho (my bad forgot --subject-prefix="PATCH RFC"), given that this makes less sense alone than within context of patch set. I get that like this it's not worth of applying even if it makes sense as a change. I prefer sending patches, rather than attaching patches to responses, because: 1. They get a lore.kernel.org link. 2. Can be fluently applied to other patch sets with b4: https://people.kernel.org/monsieuricon/introducing-4-and-patch-attestation 3. They get a patchwork link. Attachments are not as nice objects to manage as distinct emails. /Jarkko
