Re: [PATCH v3 0/2] virtio-blk: Add inline encryption support

[Max Gurtovoy <mgurtovoy@nvidia.com>]

[-- Attachment #1: Type: text/plain, Size: 11233 bytes --]


On 15/10/2026 14:19, Linlin Zhang wrote:
>
> On 10/14/2026 2:07 AM, Max Gurtovoy wrote:
>> 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.
>>
> Thanks for your comments!
>
> I agree that reusing the existing AQ avoids introducing another VQ type in virtio-blk.

I'm glad that we're aligned on this one.

>
> My concern is that AQ is currently not supported with virtio-mmio, and adding such
> support appears to be significantly more involved than introducing a
> virtio-blk-specific control VQ.

I don't think that this is a good enough reason for introducing a new VQ 
type in Vblk.

We need the best spec definition, one that will serve us for years to 
come, rather than worrying about 1-2 weeks of additional effort (to a 
task which is completely orthogonal for adding enc to vblk, IMO).

The high level spec enhancements I envision are:

  * Allow ADMIN_VQ on MMIO - Remove VIRTIO_F_ADMIN_VQ from the
    “reserved” MMIO list. Update the feature-bit text to align.
  * Add two read-only registers to the MMIO register table, mirroring
    PCI’s admin_queue_index and admin_queue_num. They are valid only
    when ADMIN_VQ is negotiated. My suggested offsets are 0x0c4 and
    0x0c8, which appear unassigned in my version.
  * Reuse the existing queue flow. PCI’s rule refers to num_queues,
    which MMIO lacks. So either reword it as “admin index is above every
    regular queue index and index plus count stays within 0x10000”, or
    add a NumQueues register.
  * State the Self-group scope - An MMIO owner device supports only
    group type 0x0. Other group types fail with INVALID_GROUP, following
    the existing validation rule.


>
> Given this constraint, Admin VQ support in virtio MMIO will impact the inline
> encryption support on virtio block, so, I prefer to the virtio-blk-specific
> control VQ as this requirement is a more widely and general request, especially
> in the virtualization platform with a Android guest virtual machine.

This is not a constraint worth considering a non-optimal architecture.

Creation of VQs takes device resources. We prefer not introducing new 
VQs if not needed. Adding a new opcode is more reasonable for your 
requirement.

Imagine a block device with N resources for VQs: N-1 for request VQ and 
1 for Admin VQ. If we need to introduce a new CVQ then it will be 
instead of one request VQ resource.

I prefer having a correct architecture.


>>>>> 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?
>>>>>

[-- Attachment #2: Type: text/html, Size: 14417 bytes --]