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

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

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?

-- 
MST