From: Max Gurtovoy <mgurtovoy@nvidia.com>
To: Linlin Zhang <linlin.zhang@oss.qualcomm.com>,
"Michael S. Tsirkin" <mst@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>,
virtio-dev@lists.linux.dev, ebiggers@kernel.org,
neeraj.soni@oss.qualcomm.com, parav@nvidia.com
Subject: Re: [PATCH v3 0/2] virtio-blk: Add inline encryption support
Date: Tue, 13 Oct 2026 21:07:33 +0300 [thread overview]
Message-ID: <4ed90e46-0dc9-40ba-b8a9-800d40b34ef9@nvidia.com> (raw)
In-Reply-To: <426abc9a-c7b4-4631-a5fe-3b2bba1d0414@oss.qualcomm.com>
On 13/10/2026 19:23, Linlin Zhang wrote:
>
> On 10/13/2026 7:12 AM, Max Gurtovoy wrote:
>> On 08/10/2026 13:15, Michael S. Tsirkin wrote:
>>> On Thu, Oct 08, 2026 at 05:17:30PM +0800, Linlin Zhang wrote:
>>>> On 9/30/2026 6:46 AM, Max Gurtovoy wrote:
>>>>> On 24/09/2026 12:35, Linlin Zhang wrote:
>>>>>> On 9/22/2026 9:14 PM, Stefan Hajnoczi wrote:
>>>>>>> On Tue, Sep 22, 2026 at 12:29:37PM +0800, Linlin Zhang wrote:
>>>>>>>> On 9/18/2026 5:08 AM, Stefan Hajnoczi wrote:
>>>>>>>>> On Sun, Sep 13, 2026 at 09:16:13AM -0700, Linlin Zhang wrote:
>>>>>>>>>> From: linlzhan <linlin.zhang@oss.qualcomm.com>
>>>>>>>>>>
>>>>>>>>>> This series adds virtio-blk inline encryption support for devices backed
>>>>>>>>>> by storage hardware with an inline crypto engine.
>>>>>>>>>>
>>>>>>>>>> The protocol exposes device capabilities such as keyslot count, maximum
>>>>>>>>>> DUN size, and supported key types. Encrypted requests identify a
>>>>>>>>>> provisioned keyslot and carry a 256-bit DUN. Key management and crypto
>>>>>>>>>> capability discovery use the block device control virtqueue.
>>>>>>>>>>
>>>>>>>>>> The control virtqueue is defined as a generic framework so that its
>>>>>>>>>> buffer layout and queue placement are independent of any particular
>>>>>>>>>> control command. Inline encryption then builds on this framework with
>>>>>>>>>> explicit crypto command formats, capability validation, and keyslot
>>>>>>>>>> state semantics.
>>>>>>>>>>
>>>>>>>>>> All key related operatios are handled in the control virtqueue, and
>>>>>>>>>> the crypto I/O request is handled in the request queue.
>>>>>>>>>>
>>>>>>>>>> For background on inline encryption in UFS and eMMC storage, see:
>>>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/block/inline-encryption.rst
>>>>>>>>>>
>>>>>>>>>> changes in v3:
>>>>>>>>>> - Add a control virtqueue
>>>>>>>>>> - Move key program/evict/derive_sw_secret/generate/prepare/import to
>>>>>>>>>> the control virtqueue
>>>>>>>>> Thank you. This was a big change, especially if you already have an
>>>>>>>>> implementation. I appreciate it!
>>>>>>>>>
>>>>>>>>> My main feedback is that the new control virtqueue commands are not yet
>>>>>>>>> documented in enough detail so that implementors could implement them.
>>>>>>>>> Once you've decided on the precise semantics, error codes, etc and added
>>>>>>>>> them to the spec, then this will round off the inline encryption
>>>>>>>>> feature. I look forward to reviewing that in the future.
>>>>>>>>>
>>>>>>>> Thanks a lot for your comment!
>>>>>>>>
>>>>>>>> Would you please help clarify what the precise semantics are about? detail
>>>>>>>> introduction of the filed in the inline encryption control command struct?
>>>>>>>> like struct virtio_blk_crypto_key_desc?
>>>>>>> By precise semantics, I mean specifying not just the constants and
>>>>>>> structs, but documenting what each command does and how it can fail.
>>>>>>> Each of the key slot programming commands needs this. There should be at
>>>>>>> least one paragraph for each of VIRTIO_BLK_T_CRYPTO_KEYSLOT_PROGRAM,
>>>>>>> VIRTIO_BLK_T_CRYPTO_KEYSLOT_EVICT, VIRTIO_BLK_T_CRYPTO_DERIVE_SW_SECRET,
>>>>>>> VIRTIO_BLK_T_CRYPTO_GENERATE_KEY, VIRTIO_BLK_T_CRYPTO_IMPORT_KEY, or
>>>>>>> VIRTIO_BLK_T_CRYPTO_PREPARE_KEY.
>>>>>>>
>>>>>>> For example:
>>>>>>>
>>>>>>> The VIRTIO_BLK_T_CRYPTO_KEYSLOT_EVICT command empties a key slot so that
>>>>>>> key information is removed and the key slot cannot be used until it is
>>>>>>> programmed again. The key slot index is specified by struct
>>>>>>> virtio_blk_crypto_key_desc \field{slot} and all other fields in the
>>>>>>> struct are ignored. The command succeeds with VIRTIO_BLK_S_OK if the key
>>>>>>> slot index is valid, including if the slot is already empty. If the key
>>>>>>> slot index is invalid, the command fails with VIRTIO_BLK_S_IOERR.
>>>>>>>
>>>>>>> Stefan
>>>>>> Thanks for the clear example.
>>>>>>
>>>>>> I'll follow the same approach and add explicit normative semantics for
>>>>>> all inline encryption control requests.
>>>>> Can you please explain why we need to introduce yet another VQ type for control?
>>>>>
>>>>> We've added the Admin VQ as a generic VQ for control operations — I'm not sure why it isn't sufficient.
>>>>>
>>>>> Adding control VQ to each device type seems strange to me after adding a generic Admin VQ.
>>>> Thanks for your comment! And I agree that the Admin VQ should be considered before
>>>> adding another device-specific control virtqueue.
>>>>
>>>> My understanding is that some of the inline encryption commands(get_crypto_modes/program key
>>>> /derive_sw_secret/evict key) are runtime operations of a specific virtio-blk deivce.
>>>> In particular, different virtio block devices may have different storage hardware sources
>>>> of crypto modes, and keyslot programming and eviction are invoked through the block device's
>>>> keyslot manager, and the programmed virtual keyslots are subsequently referenced by the
>>>> encryption requests submitted on that device's request virtqueues. These commands are
>>>> not about the management operation of device group.
>>>>
>>>> There is also a transport compatibility concern. The current virtio SPEC states that
>>>> 'Devices and drivers utilizing Virtio Over MMIO do not support VIRTIO_F_ADMIN_VQ'.
>>>> virtio MMIO is extensively used, so using the Admin VQ would make the feature unavailable
>>>> for that transport unless the proposal also extended the MMIO transport.
>>>>
>>>> For these reasons, currently I believe that a virtio-blk control virtqueue is the better
>>>> fit: it keeps the complete key lifecycle associated with the block device and remains
>>>> usable across the transports required by the feature.
>>>>
>>>> I would appreciate any feedback or insights from you and Stefan on the V5 patch series
>>>> posted for review before October ASAP.
>>>>
>>> Fundamentally, it does not make sense to force all vqs to be admin VQs.
>>>
>>> Admin vq would make sense if there is complex resource management going
>>> on - like what is happening with all the flow control things in the
>>> network device, since they have extensive functionality for managing
>>> device resources.
>>>
>>> Random device specific commands - I am not sure we want that.
>>> It remains to be proven.
>> Admin vq was added for mgmt/control/admin/non-DATA_IO operations.
>>
>> Fundamentally, it does not make sense to force random device to create new control VQ since we already defined admin VQs for this purpose.
>>
>> We could have introduced control VQ on each device type, but we didn't.
> Thanks for your comment!
>
> My concern is that these inline encryption commands are tightly coupled
> to a specific virtio-blk device rather than providing generic administrative
> functionality.
We've all the needed plumbing in place in the AQ for device specific
admin commands - see virtio-net specific admin commands.
>
> GET_CRYPTO_MODES exposes capabilities of a particular block device, while
> PROGRAM_KEY and EVICT_KEY operate on that device's keyslot manager. The
> resulting virtual keyslots are then referenced directly by encryption requests
> submitted on the same device's request VQs. The entire key lifecycle therefore
> remains local to a single virtio-blk instance and its I/O path.
>
> My understanding is that Admin VQ was introduced primarily for device agnostic
> commands. The GET_CRYPTO_MODES/PROGRAM_KEY/EVICT_KEY control commands appear to
> be inherently virtio-blk-specific rather than a cross-device interface.
>
> There is also a transport consideration: VIRTIO_F_ADMIN_VQ is currently
> unavailable for virtio-mmio, while the proposed control VQ approach remains
> transport-independent.
This limitation was introduced because we didn't want to add more noise
when AQ was added (it was already a big change).
Nothing really prevents adding AQ to mmio transport.
>
> For these reasons, I currently believe a virtio-blk control VQ is the better
> fit.
>
> I'm appreciated if you can give any feedback about it.
I think that the correct path is AQ.
Introducing another VQ type for the same purpose (mgmt cmds) seems weird
and make device implementation harder.
It is much harder to implement a new VQ type than a new opcode in the
existing AQ.
>
>>> But generally why does this go on a special VQ? Is there a reason?
>> Also in other related protocols the key mgmt goes throw a NON-IO Queue..
>>
>>
>>> You define distinct request types seemingly and we are not short on types?
>>>
next prev parent reply other threads:[~2026-10-13 18:07 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-13 16:16 [PATCH v3 0/2] virtio-blk: Add inline encryption support Linlin Zhang
2026-09-13 16:16 ` [PATCH v3 1/2] virtio-blk: Add the control virtqueue Linlin Zhang
2026-09-17 20:16 ` Stefan Hajnoczi
2026-09-22 11:10 ` Linlin Zhang
2026-09-13 16:16 ` [PATCH v3 2/2] virtio-blk: Add inline encryption support Linlin Zhang
2026-09-17 21:05 ` Stefan Hajnoczi
2026-09-24 9:32 ` Linlin Zhang
2026-09-17 21:08 ` [PATCH v3 0/2] " Stefan Hajnoczi
2026-09-22 4:29 ` Linlin Zhang
2026-09-22 13:14 ` Stefan Hajnoczi
2026-09-24 9:35 ` Linlin Zhang
2026-09-29 22:46 ` Max Gurtovoy
2026-10-08 9:17 ` Linlin Zhang
2026-10-08 10:15 ` Michael S. Tsirkin
2026-10-09 11:21 ` Linlin Zhang
2026-10-09 12:25 ` Michael S. Tsirkin
2026-10-10 10:54 ` Linlin Zhang
2026-10-12 14:51 ` Linlin Zhang
2026-10-12 22:33 ` Michael S. Tsirkin
2026-10-13 16:23 ` Linlin Zhang
2026-10-13 18:08 ` Michael S. Tsirkin
2026-10-13 19:07 ` Stefan Hajnoczi
2026-10-12 23:12 ` Max Gurtovoy
2026-10-13 16:23 ` Linlin Zhang
2026-10-13 18:07 ` Max Gurtovoy [this message]
2026-10-13 18:10 ` Michael S. Tsirkin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4ed90e46-0dc9-40ba-b8a9-800d40b34ef9@nvidia.com \
--to=mgurtovoy@nvidia.com \
--cc=ebiggers@kernel.org \
--cc=linlin.zhang@oss.qualcomm.com \
--cc=mst@redhat.com \
--cc=neeraj.soni@oss.qualcomm.com \
--cc=parav@nvidia.com \
--cc=stefanha@redhat.com \
--cc=virtio-dev@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox