Re: [Qemu-devel] [PATCH v8 1/2] virtio-crypto: Add virtio crypto device specification

[Alexander Graf <agraf@suse.de>]

On 08/30/2016 02:12 PM, Gonglei wrote:
> The virtio crypto device is a virtual crypto device (ie. hardware
> crypto accelerator card). The virtio crypto device can provide
> five crypto services: CIPHER, MAC, HASH, AEAD, KDF, ASYM, PRIMITIVE.
>
> In this patch, CIPHER, MAC, HASH, AEAD services are introduced.

I have mostly a few high level comments.

For starters, a lot of the structs rely on the compiler to pad them to 
natural alignment. That may get us into trouble when trying to emulate a 
virtio device on a host with different guest architecture (like arm on 
x86). It'd be a good idea to manually pad everything to be 64bit aligned 
- then all fields are always at the same spot.

I also have a hard time getting my head around the final flow of 
everything. Do I always have to create a session to be able to emit a 
command? In that case, doesn't that slow down everything, since a 
request would then need to wait for the host reply to receive its 
session id? There should be some way to fire off a simple non-iv 
operation without any session set up imho.

Also, I don't fully understand the split between control and data 
queues. As far as I read things, the control queue is used to create 
session ids and the data queues can then be used to push data. Is there 
any particular reason for the split? One thing that seems natural to me 
would be to have sessions be per-queue, so you would create a session on 
a particular queue and only have it ever be available there. That way 
you get rid of any locking for sessions.


Alex