lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 26 Mar 2023 12:06:26 -0700
From:   Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        Shuah Khan <shuah@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        "H . Peter Anvin" <hpa@...or.com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Tony Luck <tony.luck@...el.com>,
        Wander Lairson Costa <wander@...hat.com>,
        Erdem Aktas <erdemaktas@...gle.com>,
        Guorui Yu <GuoRui.Yu@...ux.alibaba.com>,
        Du Fan <fan.du@...el.com>, linux-kernel@...r.kernel.org,
        linux-kselftest@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH v1 2/3] virt: tdx-guest: Add Quote generation support

Hi Greg,

Thanks for the review comments.

On 3/25/23 11:34 PM, Greg KH wrote:
> On Sat, Mar 25, 2023 at 11:20:38PM -0700, Kuppuswamy Sathyanarayanan wrote:
>> Since GetQuote support requires usage of DMA APIs, convert TDX guest
>> driver to a platform driver.
> 
> Sorry, but that's not a valid reason to use a platform device for fake
> things like this:
> 
>> +static struct platform_device *tdx_dev;
> 
> Especially a single static one.
> 
>> +static int tdx_guest_probe(struct platform_device *pdev)
>> +{
>> +	if (tdx_register_event_irq_cb(attestation_callback_handler, pdev))
>> +		return -EIO;
>> +
>> +	return misc_register(&tdx_misc_dev);
>> +}
>> +
>> +static int tdx_guest_remove(struct platform_device *pdev)
>> +{
>> +	tdx_unregister_event_irq_cb(attestation_callback_handler, pdev);
>> +	misc_deregister(&tdx_misc_dev);
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver tdx_guest_driver = {
>> +	.probe = tdx_guest_probe,
>> +	.remove = tdx_guest_remove,
>> +	.driver.name = KBUILD_MODNAME,
>> +};
>> +
>>  static const struct x86_cpu_id tdx_guest_ids[] = {
>>  	X86_MATCH_FEATURE(X86_FEATURE_TDX_GUEST, NULL),
>>  	{}
>> @@ -84,16 +310,35 @@ MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids);
>>  
>>  static int __init tdx_guest_init(void)
>>  {
>> +	int ret;
>> +
>>  	if (!x86_match_cpu(tdx_guest_ids))
>>  		return -ENODEV;
>>  
>> -	return misc_register(&tdx_misc_dev);
>> +	ret = platform_driver_register(&tdx_guest_driver);
>> +	if (ret) {
>> +		pr_err("failed to register driver, err=%d\n", ret);
>> +		return ret;
>> +	}
> 
> No, please do not create a fake platform driver.
> 
>> +	tdx_dev = platform_device_register_simple(KBUILD_MODNAME,
>> +						  PLATFORM_DEVID_NONE,
>> +						  NULL, 0);
> 
> And please do not create a fake platform device.
> 
> As always, do not create fake platform devices for things that are NOT
> platform devices.
> 
> If this device needs DMA (but why?) then make it a real device and tie

Since the Quote generation process requires the guest and VMM to share data,
we need to use shared buffers. But in TDX guest, VMM cannot directly access
the guest private memory. Any memory that is required for communication with
the VMM must be shared explicitly. One way is to allocate the memory on
demand and share it explicitly using set_memory_decrypted() call. Although
this approach is clean, since it breaks the direct mapping, it is not
efficient. An alternative way is to reserve a pool of shared buffers during
boot time and re-use them for guest/VMM communication. Since in TDX, DMA API
is configured to tap into the SWIOTLB memory framework (which is shared by
default), we try to use dma_alloc_* APIs to allocate the shared buffers from
the shared pool without breaking the direct map.

If usage of DMA APIs / platform device is not acceptable for this use case,
an alternative approach is to allocate a fixed number of shared buffers during
the TDX guest driver probe and use it for GetQuote requests. Although it would
limit the amount of memory we can use for GetQuote requests at a time and also
reserve a chunk of memory during the init() time, I think it is an acceptable
tradeoff when compared to alternative choices. The AMD SEV guest driver also
adopts the similar approach. Please let me know if this approach is acceptable.

> it to the bus it belongs to (that it is obviously doing DMA on.)


> 
> thanks,
> 
> greg k-h

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ