Re: [RFC PATCH] virt: tdx-guest: Remove quote generation via ioctl

[Nikolay Borisov <nik.borisov@suse.com>]

On 23.01.24 г. 19:51 ч., Kuppuswamy Sathyanarayanan wrote:
> 
> On 1/23/24 8:07 AM, Nikolay Borisov wrote:
>> When this driver got merged initially there was no widely agreed upon
>> interface how the quote generation interface will work so having an
>> ioctl made sense. However, there's now a vendor-neutral interface via
>> configfs. Just remove the old ioctl interface and leave only the the
>> configfs one.
>>
>> Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
>> ---
> 
> This ABI allows the user to get the raw report which is further used
> for Quote generation via vsock. AFAIK, some vendors (TDX users) and
> DCAP user libraries are still using this ABI to support attestation over
> vsock model.
> 
> Don't you think we should wait till there are no users before considering
> removing it?

Given that hw with TDX was just released I'd be surprised if there are 
any users? But then again, this is an RFC so let's get opinions :)


> 
>>
>> I think it's better to remove the legacy interface now, before there's been a
>> wide adoption of it, cementing it as an ABI of sorts.
>>
>>   drivers/virt/coco/tdx-guest/tdx-guest.c | 75 +------------------------
>>   1 file changed, 2 insertions(+), 73 deletions(-)
>>
>> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
>> index 1253bf76b570..1642a0f70333 100644
>> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
>> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
>> @@ -66,41 +66,6 @@ static DEFINE_MUTEX(quote_lock);
>>    */
>>   static u32 getquote_timeout = 30;
>>
>> -static long tdx_get_report0(struct tdx_report_req __user *req)
>> -{
>> -	u8 *reportdata, *tdreport;
>> -	long ret;
>> -
>> -	reportdata = kmalloc(TDX_REPORTDATA_LEN, GFP_KERNEL);
>> -	if (!reportdata)
>> -		return -ENOMEM;
>> -
>> -	tdreport = kzalloc(TDX_REPORT_LEN, GFP_KERNEL);
>> -	if (!tdreport) {
>> -		ret = -ENOMEM;
>> -		goto out;
>> -	}
>> -
>> -	if (copy_from_user(reportdata, req->reportdata, TDX_REPORTDATA_LEN)) {
>> -		ret = -EFAULT;
>> -		goto out;
>> -	}
>> -
>> -	/* Generate TDREPORT0 using "TDG.MR.REPORT" TDCALL */
>> -	ret = tdx_mcall_get_report0(reportdata, tdreport);
>> -	if (ret)
>> -		goto out;
>> -
>> -	if (copy_to_user(req->tdreport, tdreport, TDX_REPORT_LEN))
>> -		ret = -EFAULT;
>> -
>> -out:
>> -	kfree(reportdata);
>> -	kfree(tdreport);
>> -
>> -	return ret;
>> -}
>> -
>>   static void free_quote_buf(void *buf)
>>   {
>>   	size_t len = PAGE_ALIGN(GET_QUOTE_BUF_SIZE);
>> @@ -249,29 +214,6 @@ static int tdx_report_new(struct tsm_report *report, void *data)
>>   	return ret;
>>   }
>>
>> -static long tdx_guest_ioctl(struct file *file, unsigned int cmd,
>> -			    unsigned long arg)
>> -{
>> -	switch (cmd) {
>> -	case TDX_CMD_GET_REPORT0:
>> -		return tdx_get_report0((struct tdx_report_req __user *)arg);
>> -	default:
>> -		return -ENOTTY;
>> -	}
>> -}
>> -
>> -static const struct file_operations tdx_guest_fops = {
>> -	.owner = THIS_MODULE,
>> -	.unlocked_ioctl = tdx_guest_ioctl,
>> -	.llseek = no_llseek,
>> -};
>> -
>> -static struct miscdevice tdx_misc_dev = {
>> -	.name = KBUILD_MODNAME,
>> -	.minor = MISC_DYNAMIC_MINOR,
>> -	.fops = &tdx_guest_fops,
>> -};
>> -
>>   static const struct x86_cpu_id tdx_guest_ids[] = {
>>   	X86_MATCH_FEATURE(X86_FEATURE_TDX_GUEST, NULL),
>>   	{}
>> @@ -290,27 +232,15 @@ static int __init tdx_guest_init(void)
>>   	if (!x86_match_cpu(tdx_guest_ids))
>>   		return -ENODEV;
>>
>> -	ret = misc_register(&tdx_misc_dev);
>> -	if (ret)
>> -		return ret;
>> -
>>   	quote_data = alloc_quote_buf();
>>   	if (!quote_data) {
>>   		pr_err("Failed to allocate Quote buffer\n");
>> -		ret = -ENOMEM;
>> -		goto free_misc;
>> +		return -ENOMEM;
>>   	}
>>
>>   	ret = tsm_register(&tdx_tsm_ops, NULL, NULL);
>>   	if (ret)
>> -		goto free_quote;
>> -
>> -	return 0;
>> -
>> -free_quote:
>> -	free_quote_buf(quote_data);
>> -free_misc:
>> -	misc_deregister(&tdx_misc_dev);
>> +		free_quote_buf(quote_data);
>>
>>   	return ret;
>>   }
>> @@ -320,7 +250,6 @@ static void __exit tdx_guest_exit(void)
>>   {
>>   	tsm_unregister(&tdx_tsm_ops);
>>   	free_quote_buf(quote_data);
>> -	misc_deregister(&tdx_misc_dev);
>>   }
>>   module_exit(tdx_guest_exit);
>>
>> --
>> 2.34.1
>>
>>