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

[Linlin Zhang <linlin.zhang@oss.qualcomm.com>]

On 10/9/2026 8:25 PM, Michael S. Tsirkin wrote:
> On Fri, Oct 09, 2026 at 07:21:13PM +0800, Linlin Zhang wrote:
>>
>>
>> On 10/8/2026 6:15 PM, 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.
>>>
>>> But generally why does this go on a special VQ? Is there a reason?
>>>
>>> You define distinct request types seemingly and we are not short on types?
>>>
>>
>> Thanks for your comments!
>>
>> The key programming operation is initiated while preparing a block I/O
>> request and sent to the VQ after the block I/O request has entered the
>> block request queue, before the request is submitted to the device's VQ.
>> see blk_mq_submit_bio(). Using the same virtqueue for both key-program
>> command and normal I/O could lead to bio_queue_enter() called 2 times
>> for the same block queue, which is possible to have deadlock if
>> blk_mq_freeze_queue() is called between above 2 bio_queue_enter() caller.
>>
>> A separate control virtqueue provides an independent execution path for
>> key programming, key eviction, and related operations. It allows these
>> operations to be issued even when the normal request virtqueues are full
>> or unable to make progress. Once the operation completes, the corresponding
>> request can safely reference the programmed virtual keyslot.
>>
>> For such reasons, we believe that a virtio-blk-specific control VQ is a
>> more appropriate mechanism than the standard VQ.
> 
> I don't understand. Do these commands go through bio queue?
> Don't they go to VQ directly, just like you would with the control VQ?
> 

Thanks for your comment! my earlier reply conflated two separate concerns.
Let me address them precisely based on Linux kernel code.

I agree that block-layer queue entry and virtqueue selection are independent
issues.

Whether key program commands going through bio queue or not depends on the
implementation.

  - Option A: Reuse the standard block-request helpers (e.g.,
    blk_mq_alloc_request() + blk_execute_rq(), the same pattern virtblk_get_id()
    uses):
    
      - blk_mq_submit_bio()
         -> blk_crypto_submit_bio()   # outer bio_queue_enter() already held
           -> keyslot manager
             -> virtblk_crypto_keyslot_program()
               -> blk_mq_alloc_request()
                 -> blk_queue_enter()    # deadlock: freeze in progress

    This is not safe at the keyslot-manager call site. Key programming is invoked
    from inside blk_mq_submit_bio(), at a point where the submitting thread already
    holds an unreleased reference from an earlier bio_queue_enter() call on the same
    queue. Calling blk_queue_enter() a second time on the same thread and queue —
    while a concurrent blk_mq_freeze_queue() is in flight — produces a circular wait:
    the freezer blocks waiting for all references (including the outer one) to drop to
    zero, while the submitting thread blocks waiting for the freeze depth to clear.
    Neither can make progress. The problem is not allocation starvation but queue-entry
    reentrancy under freeze. 

    This is the deadlock concern I mentioned last time.

  - Option B: Bypass the block layer entirely and raw-submit to a data VQ (i.e.,
    virtqueue_add_sgs() / virtqueue_kick() directly, just targeting a request VQ instead
    of the control VQ):

    This would indeed sidestep the blk_queue_enter() reentrancy problem. However, it is
    not equivalent to simply adding a new request type — it would require changing the
    data VQ token and completion contract:

    virtblk_done() — the completion callback registered for every data VQ — unconditionally
    calls blk_mq_rq_from_pdu() and then blk_mq_complete_request() for every buffer it pulls
    off the ring. It assumes every returned token is a struct virtblk_req embedded in a
    blk-mq–allocated struct request. A new specific virtblk_crypto_request injected into a
    data VQ ring is not such an object. The completion path has no way to distinguish it,
    and treating it as one computes a bogus request pointer and corrupts blk-mq tag state.

    Making this safe would require adding out-of-band type tagging and dispatch logic into
    the hot I/O completion path, making the data-VQ callback polymorphic in a way that
    currently has no precedent.

    Furthermore, there is a scope mismatch: data VQs are scoped to individual blk-mq hardware
    queues (hctx->queue_num), whereas keyslot management is a per-device operation. On a
    multi-queue device, there is no single canonical data VQ to target. Choosing queue[0]
    arbitrarily concentrates control traffic, causes contention with that hctx's in-flight
    I/O and submission lock (vblk->vqs[i].lock), and can be blocked entirely if that ring
    is saturated.

    In summary, using a data VQ in this way is implementable in principle, but it would require
    changing the established data-VQ token/completion contract and coupling a device-scoped
    control operation to an arbitrarily chosen hctx-scoped queue.

While the separate control virtqueue provides a direct submission path without entering the
block layer, while also providing a distinct request format, completion callback, lock, and
descriptor pool. This keeps key-management operations separate from blk-mq request/tag
and from the per-hardware-queue I/O completion path.

Given the above, I prefer to having a new control VQ. I'm appreciated if you have any
insights about it. 

In addition, the control VQ is currently used for inline encryption. From the virtio
specification perspective, should we state that it is currently used only for inline
encryption, or leave room for future extensions?