Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <cover.1684048511.git.sathyanarayanan.kuppuswamy@linux.intel.com>
 <972e1d5c5ec53e2757fb17a586558c5385e987dd.1684048511.git.sathyanarayanan.kuppuswamy@linux.intel.com>
 <64876bf6c30e2_1433ac29415@dwillia2-xfh.jf.intel.com.notmuch>
In-Reply-To: <64876bf6c30e2_1433ac29415@dwillia2-xfh.jf.intel.com.notmuch>
From:   Erdem Aktas <erdemaktas@google.com>
Date:   Thu, 22 Jun 2023 16:31:43 -0700
Message-ID: <CAAYXXYzOUQC0G7yoh1i2+SW-FodCABwhD6TgArLf28iG=i65zw@mail.gmail.com>
Subject: Re: [PATCH v3 3/3] selftests/tdx: Test GetQuote TDX attestation feature
To:     Dan Williams <dan.j.williams@intel.com>
Cc:     Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        Shuah Khan <shuah@kernel.org>,
        Jonathan Corbet <corbet@lwn.net>,
        "H . Peter Anvin" <hpa@zytor.com>,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
        Tony Luck <tony.luck@intel.com>,
        Wander Lairson Costa <wander@redhat.com>,
        Dionna Amalie Glaze <dionnaglaze@google.com>,
        Chong Cai <chongc@google.com>, Qinkun Bao <qinkun@apache.org>,
        Guorui Yu <GuoRui.Yu@linux.alibaba.com>,
        Du Fan <fan.du@intel.com>, linux-kernel@vger.kernel.org,
        linux-kselftest@vger.kernel.org, linux-doc@vger.kernel.org,
        dhowells@redhat.com, brijesh.singh@amd.com, atishp@rivosinc.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Jun 12, 2023 at 12:03=E2=80=AFPM Dan Williams <dan.j.williams@intel=
.com> wrote:
>
> [ add David, Brijesh, and Atish]
>
> Kuppuswamy Sathyanarayanan wrote:
> > In TDX guest, the second stage of the attestation process is Quote
> > generation. This process is required to convert the locally generated
> > TDREPORT into a remotely verifiable Quote. It involves sending the
> > TDREPORT data to a Quoting Enclave (QE) which will verify the
> > integrity of the TDREPORT and sign it with an attestation key.
> >
> > Intel's TDX attestation driver exposes TDX_CMD_GET_QUOTE IOCTL to
> > allow the user agent to get the TD Quote.
> >
> > Add a kernel selftest module to verify the Quote generation feature.
> >
> > TD Quote generation involves following steps:
> >
> > * Get the TDREPORT data using TDX_CMD_GET_REPORT IOCTL.
> > * Embed the TDREPORT data in quote buffer and request for quote
> >   generation via TDX_CMD_GET_QUOTE IOCTL request.
> > * Upon completion of the GetQuote request, check for non zero value
> >   in the status field of Quote header to make sure the generated
> >   quote is valid.
>
> What this cover letter does not say is that this is adding another
> instance of the similar pattern as SNP_GET_REPORT.
>
> Linux is best served when multiple vendors trying to do similar
> operations are brought together behind a common ABI. We see this in the
> history of wrangling SCSI vendors behind common interfaces. Now multiple

Compared to the number of SCSI vendors, I think the number of CPU
vendors for confidential computing seems manageable to me. Is this
really a good comparison?

> confidential computing vendors trying to develop similar flows with
> differentiated formats where that differentiation need not leak over the
> ABI boundary.

<Just my personal opinion below>
I agree with this statement in the high level but it is also somehow
surprising for me after all the discussion happened around this topic.
Honestly, I feel like there are multiple versions of "Intel"  working
in different directions.

If we want multiple vendors trying to do the similar things behind a
common ABI, it should start with the spec. Since this comment is
coming from Intel, I wonder if there is any plan to combine the GHCB
and GHCI interfaces under common ABI in the future or why it did not
even happen in the first place.

What I see is that Intel has GETQUOTE TDVMCALL interface in its spec
and again Intel does not really want to provide support for it in
linux. It feels really frustrating.

>
> My observation of SNP_GET_REPORT and TDX_CMD_GET_REPORT is that they are
> both passing blobs across the user/kernel and platform/kernel boundary
> for the purposes of unlocking other resources. To me that is a flow that
> the Keys subsystem has infrastructure to handle. It has the concept of
> upcalls and asynchronous population of blobs by handles and mechanisms
> to protect and cache those communications. Linux / the Keys subsystem
> could benefit from the enhancements it would need to cover these 2
> cases. Specifically, the benefit that when ARM and RISC-V arrive with
> similar communications with platform TSMs (Trusted Security Module) they
> can build upon the same infrastructure.
>
> David, am I reaching with that association? My strawman mapping of
> TDX_CMD_GET_QUOTE to request_key() is something like:
>
> request_key(coco_quote, "description", "<uuencoded tdreport>")
>
> Where this is a common key_type for all vendors, but the description and
> arguments have room for vendor differentiation when doing the upcall to
> the platform TSM, but userspace never needs to contend with the
> different vendor formats, that is all handled internally to the kernel.

I think the problem definition here is not accurate. With AMD SNP,
guests need to do a hypercall to KVM and KVM needs to issue a
SNP_GUEST_REQUEST(MSG_REPORT_REQ) to the SP firmware. In TDX, guests
need to do a TDCALL to TDXMODULE to get the TDREPORT and then it needs
to get that report delivered to the host userspace to get the TDQUOTE
generated by the SGX quoting enclave. Also TDQUOTE is designed to work
async while the SNP_GUEST_REQUESTS are blocking vmcalls.

Those are completely different flows. Are you suggesting that intel
should also come down to a single call to get the TDQUOTE like AMD
SNP?

The TDCALL interface asking for the TDREPORT is already there. AMD
does not need to ask the report and the quote separately.

Here, the problem was that Intel (or "upstream community") did not
want to implement/accept hypercall for TDQUOTE which would be handled
by the user space VMM. The alternative implementation (using vsock)
does not work for many use cases including ours. I do not see how your
suggestion addresses the problem that this patch was trying to solve.

So while I like the suggested direction, I am not sure how much it is
possible to come up with a common ABI even with just only for 2
vendors (AMD and Intel) without doing spec changes which is a multi
year effort imho.

>
> At this point I am just looking for confirmation that the "every vendor
> invent a new character device + ioctl" does not scale and a deeper
> conversation is needed. Keys is a plausible solution to that ABI
> proliferation problem.