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

[Gonglei (Arei)]

Hi Alex,


> -----Original Message-----
> From: Alexander Graf [mailto:address@hidden
> Sent: Thursday, September 01, 2016 9:37 PM
> Subject: Re: [PATCH v8 1/2] virtio-crypto: Add virtio crypto device 
> specification
> 
> 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.
> 
Good point! I'll do this in the next version. Thanks!

> 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.
> 
For symmetric algorithms, we'd better create a session before executing 
encryption
Or decryption, because the session usually be kept for a specific
algorithm with specific key in the production environment. And if we only 
change the iv, 
we don't need to re-create the session. 

For the asymmetric algorithms, we don't need create a session IIRC.

So, I don't think this is a performance degradation, but a performance 
enhancement.

> 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.
> 
We want to keep a unify request type (structure) for data queue, so we can
keep the session operation in the control queue. Of course the control queue
only used to create sessions currently, but we can extend its functions if 
needed
in the future.

> 
> Alex

Regards,
-Gonglei