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

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Oct 14, 2026 at 12:23:16AM +0800, Linlin Zhang wrote:
> 
> 
> On 10/13/2026 6:33 AM, Michael S. Tsirkin wrote:
> > On Sat, Oct 10, 2026 at 06:54:06PM +0800, Linlin Zhang wrote:
> >>
> >>
> >> 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.
> > 
> > 
> > Actually even simpler. Option C: spec allows any mix. Linux practically
> > can choose a VQ and dedicates it to the key slot management.
> > It can then submit requests there in any way it wants.
> > No?
> 
> Thanks, this seems a simpler implementation. If I understand correctly, the driver could
> reserve one of the request VQs exclusively for keyslot-management commands and raw-submit
> the commands to that VQ.
> 
> But I see a few issues that may need to be defined:
>   1. This approach depends on VIRTIO_BLK_F_MQ to be negotiated and requires the device to expose
>      at least two request virtqueues. It would therefore provide no usable fallback for a
>      single-queue device.

A control vq is also a vq so ...

>   2. All negotiated queues are exposed through the existing requestq interface, and the
>     spec provides no standardized way to identify one of them as a dedicate queue. Reserving
>     one request VQ for key management would therefore be a driver/backend convention rather
>     than an explicitly negotiated device interface. This also decreasing the code portability.


I don't understand what is code portability.

>   3. The dedicated queue would still use the normal virtio-blk request framing, including
>     struct virtio_blk_outhdr, even though fields such as ioprio and sector have no meaningful
>     semantics for key-management commands.


Not "such as" there are just these two. You can overlay them with different fields, if you prefer.


> This is workable, but it makes the control protocol
>     inherit data-request framing that it does not naturally need.
> 
> If these are acceptable, Option C could avoid adding a new control VQ. But I think the
> queue-selection and feature-dependency rules would still need to be specified explicitly.

FEature dependency, sure. It's a pretty standard thing though.
For queue selection I'd just leave all this to the driver by preference. 


> Could you please help comments about these above concerns? Taking this concern into account,
> I feel a dedicate control queue seems more proper.
> 
> > 
> > 
> >> 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?
> >