Discussion of the implementations of VIRTIO specification
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Linlin Zhang <linlin.zhang@oss.qualcomm.com>
Cc: Max Gurtovoy <mgurtovoy@nvidia.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	virtio-dev@lists.linux.dev, ebiggers@kernel.org,
	neeraj.soni@oss.qualcomm.com
Subject: Re: [PATCH v3 0/2] virtio-blk: Add inline encryption support
Date: Fri, 9 Oct 2026 08:25:31 -0400	[thread overview]
Message-ID: <20261009081523-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <bfbb51cb-8a90-4b23-8ed1-26ea41102dd6@oss.qualcomm.com>

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?

-- 
MST


      reply	other threads:[~2026-10-09 12:25 UTC|newest]

Thread overview: 16+ 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 [this message]

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=20261009081523-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=ebiggers@kernel.org \
    --cc=linlin.zhang@oss.qualcomm.com \
    --cc=mgurtovoy@nvidia.com \
    --cc=neeraj.soni@oss.qualcomm.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