On Tue, 2022-04-19 at 07:00 -0700, Sathyanarayanan Kuppuswamy wrote: > > On 4/19/22 1:16 AM, Kai Huang wrote: > > In fact after slightly thinking more, I think you can split TDREPORT TDCALL > > support with GetQuote/SetupEventNotifyInterrupt support. The reason is as I > > said, GetQuote isn't mandatory to support attestation. TD attestation agent can > > use i.e. vsock, tcp/ip, to communicate to QE directly. Whether kernel needs to > > support GetQuote is actually arguable. > > IMO, we should not use a usage model to categorize "GetQuote" support > as a mandatory or non-mandatory requirement. > > For customers who use VSOCK, they can get away without GetQuote > TDVMCALL support. But for customers who do not want to use > VSOCK model, this is a required support. AFAIK, our current customer > requirement is to use TDVMCALL approach for attestation support. > > If your suggestion is to split GetQuote support as separate > patch to make it easier for review, I am fine with such > suggestion. > I am not saying we should get rid of GetQuote support. If there's customer wants this with a good reason, we can certainly support it. I understand that some customer wants to deploy QE in host and don't want additional communication channel (i.e. vsock) between guest and host, which may add additional attack window and/or customer's validation resource. My point is regardless whether we need to support GetQuote, logically this driver can be split to two parts as I said: 1) basic TDREPORT support to userspace; 2) additional GetQuote support. And I think there are many benefits if you do in this way as I commented below. > > > > So IMHO you can split this attestation driver into two parts: > > > > 1) A "basic" driver which supports reporting TDREPORT to userspace > > 2) Additional support of GetQuote/SetupEventNotifyInterrupt. > > > > The 1) can even be in a single patch (I guess it won't be complicated). It is > > easy to review (and i.e. can be merged separately), and with it, you will > > immediately have one way to support attestation. > > > > 2) can be reviewed separately, perhaps with one additional Kconfig option (i.e. > > CONFIG_INTEL_TDX_ATTESTATION_GET_QUOTE). I think this part has most of the > > > GetQuote IOCTL support is a very simple feature support, so, IMO, we > don't need to complicate it with additional config. > > > Additional Kconfig can reduce attack window by turning it off for people don't need it. Anyway no strong opinion here. -- Thanks, -Kai