From: Jarkko Sakkinen <jarkko@kernel.org>
To: Sean Christopherson <seanjc@google.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
Wanpeng Li <wanpengli@tencent.com>,
Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
Tony Luck <tony.luck@intel.com>,
linux-sgx@vger.kernel.org, linux-kernel@vger.kernel.org,
kvm@vger.kernel.org
Subject: Re: [PATCH] x86/sgx: Declare sgx_set_attribute() for !CONFIG_X86_SGX
Date: Fri, 03 Sep 2021 18:58:55 +0300 [thread overview]
Message-ID: <f7e6b2f444f34064e34d7bd680d2c863b9ce6a41.camel@kernel.org> (raw)
In-Reply-To: <YTI/dTORBZEmGgux@google.com>
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
next prev parent reply other threads:[~2021-09-03 15:58 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-03 6:41 [PATCH] x86/sgx: Declare sgx_set_attribute() for !CONFIG_X86_SGX Jarkko Sakkinen
2021-09-03 15:29 ` Sean Christopherson
2021-09-03 15:58 ` Jarkko Sakkinen [this message]
2021-09-03 16:33 ` Jarkko Sakkinen
2021-09-06 8:35 ` Paolo Bonzini
2021-09-07 13:37 ` Jarkko Sakkinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f7e6b2f444f34064e34d7bd680d2c863b9ce6a41.camel@kernel.org \
--to=jarkko@kernel.org \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sgx@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=tglx@linutronix.de \
--cc=tony.luck@intel.com \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox