Re: [PATCH v1 03/11] soc: qcom: crypto_virt: add support for create, prepare and import keys

[Krzysztof Kozlowski <krzk@kernel.org>]

On 01/09/2026 12:31, Linlin Zhang wrote:
> 
> 
> On 8/31/2026 2:58 PM, Krzysztof Kozlowski wrote:
>> On 27/08/2026 18:07, Linlin Zhang wrote:
>>> +static void crypto_virt_remove(struct platform_device *pdev)
>>> +{
>>> +	virtblk_set_crypto_ops(NULL);
>>> +}
>>> +
>>> +static const struct of_device_id crypto_virt_of_match[] = {
>>> +	{ .compatible = "qcom,crypto-virt" },
>>
>> Undocumented ABI, also not really allowed. We don't take bindings for
>> drivers.
>>
> 
> The dt-binding document is included in another patch in this series.

And what does the submitting patches document ask you?


> The concern about introducing a DT binding for a software-only driver.
> The underlying problem I am trying to solve is how to communicate the
> wrapped-key size to the guest.

I am not asking about your problem. Can you please stop answering to
actual problems with irrelevant other aspects?

You cannot have DT for a driver.

That's it, please read DTS101 slides.


> 
> Because the guest has no access to the ICE hardware, it cannot get
> the key size by reading the ICE hardware directly. In the other hand,
> the wrapped key size may vary across ICE versions, it's infeasible
> to have a fixed wrapped key size in the code. Additionally, wrapped
> key size is vendor-specific, it's nor proper to pass it via virtio.
> 
> As an experiment, I used a DT property to set this wrapped key and
> wanted to get feedback on whether this is a reasonable approach.
> 
> Is there a preferred mechanism for handling this kind of vendor-specific
> configuration, or are there similar examples elsewhere in the kernel
> that I should follow?

Not DT. Use whatever other mechanism kernel provides.


> 
>>> +	{ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, crypto_virt_of_match);
>>> +
>>> +static struct platform_driver crypto_virt_driver = {
>>> +	.probe = crypto_virt_probe,
>>> +	.remove = crypto_virt_remove,
>>> +	.driver = {
>>> +		.name = "crypto_virt",
>>> +		.of_match_table = crypto_virt_of_match,
>>> +	},
>>> +};
>>> +
>>> +static int __init crypto_virt_init(void)
>>> +{
>>> +	return platform_driver_register(&crypto_virt_driver);
>>> +}
>>>  module_init(crypto_virt_init);
>>>  
>>>  #if IS_MODULE(CONFIG_QCOM_CRYPTO_VIRT)
>>>  static void __exit crypto_virt_exit(void)
>>>  {
>>> -	virtblk_set_crypto_ops(NULL);
>>> +	platform_driver_unregister(&crypto_virt_driver);
>>
>> You just added this line, so previous code was simply wrong?
> No. In previous patch, virtblk_set_crypto_ops is called in
> crypto_virt_init and crypto_virt_exit. This patch moves the caller
> to the driver probe and remove APIs.

Heh?

Let's look at your previous patch:

+#if IS_MODULE(CONFIG_QCOM_CRYPTO_VIRT)
+static void __exit crypto_virt_exit(void)
+{
+	virtblk_set_crypto_ops(NULL);
+}
+module_exit(crypto_virt_exit);
+#endif

And you claim that you did not add this line?

Do you understand how diff format works?

Best regards,
Krzysztof