Re: [virtio-dev] packed ring layout proposal

2016-09-19 Thread Paolo Bonzini 


On 19/09/2016 21:14, Michael S. Tsirkin wrote:
> > But I prefer all these tricks to be hidden within the driver.  It seems
> > a good idea in the beginning to rig the device to second-guess what a
> > driver could do, but then it makes things awkward.  (This is also why
> > I'd rather get rid of VRING_DESC_F_NEXT).
> 
> Right, I'll CC dpdk list on this proposal. As dpdk uses _NEXT almost
> exclusively I'd like to make sure we are not messing things up.
> 
> Maybe the right thing to do is to disallow _NEXT if _INDIRECT is
> negotiated. Each device can then decide whether it wants to
> use _INDIRECT or _NEXT (or both). How's that?

Still a bit of feature creep if we can avoid it, but at least it lets
you write two fast loops to parse the descriptors.  So that's already a
huge improvement.

Negotiating _INDIRECT would still allow a single direct buffer.

Just one thing: in the  _NEXT case, does the driver write only one
available descriptor for the head (effectively ignoring desc.index on
all descriptors but the first)?  Or does it have to write all the
descriptors?  If the latter, _INDIRECT would almost surely end up faster.

Paolo

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] packed ring layout proposal

2016-09-19 Thread Maxime Coquelin 

Hi Michael,

On 09/19/2016 07:24 PM, Michael S. Tsirkin wrote:

On Mon, Sep 19, 2016 at 10:03:36AM -0700, Stephen Hemminger wrote:

On Sun, 18 Sep 2016 08:36:05 -0400 (EDT)
Paolo Bonzini  wrote:


Without indirect the usable ring size is cut in half on older kernels that
don't support any layout. That limit on qemu ends up being 128 packets


Hi Stephen,

note that here we were talking about limiting direct descriptors to
requests with a single buffer.

Paolo


The test I was looking at was small packet transmit (like pktgen or RFC2544).
The idea was to make the DPDK driver use all the same algorithms as the Linux
kernel which is the baseline for virtio. Also handling jumbo MTU packets
efficiently with scatter/gather.

With DPDK the issue was that the typical packet required several transmit
slot entries. Particularly bad for jumbo packets which usually are made up
of several mbuf's. A 9K packet typically has 5 mbuf's plus one more for the
virtio header.

Ideally, the DPDK driver should use the best algorithm it can negotiate.
Be sure and test with old RHEL6 type systems, modern enterprise (RHEL/SLES)
and also Google Compute which has there own virtio.  Not sure what virtual
box is doing??  Why can't code be written like ixgbe driver which has a
fast path if config allows it but makes decision at run time.


Interesting.  Maxime here posted integrating the indirect buffer
support to address this but this didn't get accepted
since he could not prove the performance gain.

Maxime, could you share the link to your patch so we
can discuss this on the dpdk mailing list?

Here it is:
http://dpdk.org/dev/patchwork/patch/14797/

Thanks,
Maxime



Thanks!



-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] packed ring layout proposal

2016-09-19 Thread Michael S. Tsirkin 
On Mon, Sep 19, 2016 at 07:00:10PM +0200, Paolo Bonzini wrote:
> 
> 
> On 19/09/2016 18:48, Michael S. Tsirkin wrote:
> > > > 
> > > > #define BATCH_NOT_FIRST 0x0010
> > > > #define BATCH_NOT_LAST  0x0020
> > > > 
> > > > In other words only descriptor in batch is .
> > > > 
> > > > Batch does not have to be processed together, so
> > > > !first/!last flags can be changed when descriptor is used.
> > > > 
> > > 
> > > I don't exactly understand why this is necessary.
> > > Is there any added value by knowing that a bunch of messages have been 
> > > written at 
> > > the same time.
> > 
> > virtio net mergeable buffers pretty much rely on this.
> 
> How so?  num_buffers is specified in the virtio-net header, which is
> going to be in the first received descriptor.

BTW this access causes an extra cache miss for some short packets when
said packets don't need to be examined immediately.  Andrew's testing
shows an immediate performance gain just by removing this lookup.  I
also feel this is a kind of transport thing that really belongs in the
descriptor, e.g. if you know there are more descriptors coming
prefetch might be more justified. I didn't test this idea yet.

>  You don't need explicit
> marking of which descriptor is the first or the last.

No but you do need to make sure that when get_buf returns
the first buffer, all buffers are available.


> If you want to do parallel polling of the ring, perhaps we can define
> extra device-specific flags, and define one (e.g.,
> VIRTIO_NET_F_RXBUF_HAS_VIRTIO_NET_HDR) or two (adding also
> VIRTIO_NET_F_RXBUF_LAST) for MRG_RXBUF.

>From performance POV, there's a bunch of unused bits in
the descriptor that can be put to this use, this is better
than a buffer access which might not need to be in cache
for a while.
And from the achitecture POV, I feel this is a transport
thing, not a device specific thing.

My kvm forum talk also mentions possible optimization opportunities
if the transport can pass a batch of packets around,
such as implementing xmit_more.



> > Which is optimal depends on the workload:
> > - index requires and update each time we enable interrupts
> > - flag requires an ipdate each time we switch to enabled/disabled
> >   interrupts
> > 
> > Accordingly, for workloads that enable interrupts often,
> > flags are better. For workloads that keep interrupts
> > disabled most of the time, index is better.
> 
> Yeah, this makes sense.  So I guess both indexes and flags should stay.
> 
> Paolo

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] packed ring layout proposal

2016-09-19 Thread Michael S. Tsirkin 
On Mon, Sep 19, 2016 at 02:01:57PM +0200, Paolo Bonzini wrote:
> 
> 
> On 19/09/2016 09:37, Pierre Pfister (ppfister) wrote:
> > Paolo proposed in a later mail to drop VRING_DESC_F_NEXT altogether,
> > and I think he is right at least in the sense that having two solutions
> > for doing almost the same thing is not necessary (IMHO, even harmful).
> > 
> > But, VRING_DESC_F_INDIRECT adds a layer of indirection and makes
> > prefetching a bit more complicated. The reason why VRING_DESC_F_INDIRECT
> > was useful in the first place is that virtio 1.0 didn't have enough
> > descriptors in the main queue (it is equal to the queue size).
> > 
> > Therefore, ideally, I'd rather have a longer queue and use
> > VRING_DESC_F_NEXT only and drop VRING_DESC_F_INDIRECT altogether.
> 
> VRING_DESC_F_INDIRECT is necessary for other devices.  For storage it is
> not rare to have s/g lists composed of 10-15 descriptors.
> 
> The main issue with VRING_DESC_F_INDIRECT, at least as it is implemented
> in Linux, is the cost of allocating and freeing the indirect descriptor.
>  But if you know that the number of descriptor is fixed (e.g. 2 for
> request header + data) you can pre-allocate the descriptors with a 1:1
> relationship with the entries of the ring buffer.  This might be useful
> for the virtio-net tx ring.

Yes, I've been thinking the same thing. Is there any advantage
to have the indirect addresses pre-programmed as
opposed to stored in the descriptors?

In theory
if (desc.flags & indirect)
use(*address[i])

might be speculated as compared to
if (desc.flags & indirect)
use(*desc.addr)
which can't.

But OTOH this might add to cache pressure as desc is already in cache
anyway.

If all descriptors are known ahead of the time to be indirect we could
pack more of them per cache line by dropping addr (and maybe len?).
This might work well for virtio blk.

Another consideration is numa locality though. It might be benefitial
to move the indirect buffers to the correct numa node,
if these are always preallocated you can't.


> > With this flag, every-time you look at a new descriptor, before doing
> > anything you will have to look at the flags. Meaning that you will
> > have to wait for the result of a test on a very newly fetched piece
> > of memory.
> 
> Yeah, it trades less cache traffic with higher latency on the remaining
> traffic.  But I like the idea.  For L2 the processor's speculative
> execution should still hide most of the latency (the branch should be
> well predicted in the direction of "another descriptor"---and if there's
> no other descriptor you don't care too much about the speculation
> penalty because you have no work to do).

Yes, it seems to work well.

> You could also try a blind prefetch of desc.addr before looking at the
> flags, in order to help with indirect descriptors.

Tried and this does not seem to help at least in the micro-benchmark.

> >> struct event {
> >>__le16 idx;
> >>__le16 flags;
> >> }
> >>
> >> Flags can be used like in virtio 1.0, by storing a special
> >> value there:
> >>
> >> #define VRING_F_EVENT_ENABLE  0x0
> >>
> >> #define VRING_F_EVENT_DISABLE 0x1
> >>
> >> or it can be used to switch to event idx:
> >>
> >> #define VRING_F_EVENT_IDX 0x2
> >>
> >> in that case, interrupt triggers when descriptor with
> >> a given index value is used.
> > 
> > Is there a reason why we would keep both ways and not just use the IDX one ?
> 
> Probably a good idea, too, to keep it simple.  Pre-1.0 can go away.
> 
> Paolo

It's not there for compatibility.  I did put some thought into this, and
I feel different workloads need different mechanisms.  I'll add some
motivation in the next revision I post.

-- 
MST

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] packed ring layout proposal

2016-09-19 Thread Paolo Bonzini 


On 19/09/2016 18:48, Michael S. Tsirkin wrote:
> > > 
> > > #define BATCH_NOT_FIRST 0x0010
> > > #define BATCH_NOT_LAST  0x0020
> > > 
> > > In other words only descriptor in batch is .
> > > 
> > > Batch does not have to be processed together, so
> > > !first/!last flags can be changed when descriptor is used.
> > > 
> > 
> > I don't exactly understand why this is necessary.
> > Is there any added value by knowing that a bunch of messages have been 
> > written at 
> > the same time.
> 
> virtio net mergeable buffers pretty much rely on this.

How so?  num_buffers is specified in the virtio-net header, which is
going to be in the first received descriptor.  You don't need explicit
marking of which descriptor is the first or the last.

If you want to do parallel polling of the ring, perhaps we can define
extra device-specific flags, and define one (e.g.,
VIRTIO_NET_F_RXBUF_HAS_VIRTIO_NET_HDR) or two (adding also
VIRTIO_NET_F_RXBUF_LAST) for MRG_RXBUF.

> Which is optimal depends on the workload:
> - index requires and update each time we enable interrupts
> - flag requires an ipdate each time we switch to enabled/disabled
>   interrupts
> 
> Accordingly, for workloads that enable interrupts often,
> flags are better. For workloads that keep interrupts
> disabled most of the time, index is better.

Yeah, this makes sense.  So I guess both indexes and flags should stay.

Paolo

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] packed ring layout proposal

2016-09-19 Thread Michael S. Tsirkin 
On Mon, Sep 19, 2016 at 07:37:53AM +, Pierre Pfister (ppfister) wrote:
> Hello Michael, 
> 
> Thanks for doing this. ;-)
> 
> 
> > Le 16 sept. 2016 à 00:39, Michael S. Tsirkin  a écrit :
> > 
> > 
> > Let's start a discussion around an alternative ring layout.
> > This has been in my kvm forum 2016 presentation.
> > The idea is to have a r/w descriptor in a ring structure,
> > replacing the used and available ring, index and descriptor
> > buffer.
> > 
> > * Descriptor ring:
> > 
> > Guest adds descriptors with unique index values and DESC_HW set in flags.
> > Host overwrites used descriptors with correct len, index, and DESC_HW
> > clear.  Flags are always set/cleared last.
> > 
> > #define DESC_HW 0x0080
> > 
> > struct desc {
> >__le64 addr;
> >__le32 len;
> >__le16 index;
> >__le16 flags;
> > };
> > 
> > When DESC_HW is set, descriptor belongs to device. When it is clear,
> > it belongs to the driver.
> 
> I think the simplified ring layout is a good idea. 
> It reduces the number of indirections, and will probably help pre-fetching 
> data in order to avoid cache misses.
> 
> I am not completely convinced by the flag-based approach instead of the 
> index-based approach though.
> With the index-based approach, you have only one-read to do in order
> to know up to what point in the queue you can go.
> With this flag, every-time you look at a new descriptor, before doing 
> anything you will have to look at the flags.
> Meaning that you will have to wait for the result of a test on a very newly 
> fetched piece of memory.
> 
> So maybe it would be interesting to still have a used_index and 
> available_index (put in different cache lines).

Problem is, index access is guaranteed to be a cache miss.
You need to stall waiting for descriptor length anyway -
go ahead and implement an alternative, and prove me wrong,
but my testing shows removing the index speeds things up.

Find the prototype here:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/tools/virtio/ringtest/ring.c



> 
> > 
> > 
> > * Scatter/gather support
> > 
> > We can use 3 bits to set direction
> > and to chain s/g entries in a request, same as virtio 1.0:
> > 
> > /* This marks a buffer as continuing via the next field. */
> > #define VRING_DESC_F_NEXT   1
> > /* This marks a buffer as write-only (otherwise read-only). */
> > #define VRING_DESC_F_WRITE  2
> > 
> > Unlike virtio 1.0, all descriptors must have distinct ID
> > values.
> > 
> > * Indirect buffers
> > 
> > Can be done like in virtio 1.0:
> > 
> > /* This means the buffer contains a list of buffer descriptors. */
> > #define VRING_DESC_F_INDIRECT   4
> > 
> > In the descriptors in the indirect buffers, I think we should drop index
> > field altogether, just put s/g entries one after the other.
> > 
> > Also, length in the indirect descriptor should mark
> > the list of the chain.
> > 
> > virtio 1.0 seems to allow a s/g entry followed by
> > an indirect descriptor. This does not seem useful,
> > so let's not allow that anymore.
> 
> First of all, if I understand correctly, VRING_DESC_F_NEXT doesn't use a 
> 'next' field, and rather just
> say that the next descriptor in the queue is part of the same message. I 
> think this is really, really, good.
> 
> Paolo proposed in a later mail to drop VRING_DESC_F_NEXT altogether, and I 
> think
> he is right at least in the sense that having two solutions for doing almost 
> the same thing 
> is not necessary (IMHO, even harmful).

I'd like to see some code and numbers to back this up.  In particular,
testing dpdk performance with/without indirect descriptors would be
interesting.
If the optimal things depends on workload or the type of device in use,
then it might be necessary.

> But, VRING_DESC_F_INDIRECT adds a layer of indirection and makes prefetching 
> a bit
> more complicated.
> The reason why VRING_DESC_F_INDIRECT was useful in the first place is that 
> virtio 1.0 
> didn't have enough descriptors in the main queue (it is equal to the queue 
> size).

rings are physically contigious, and allocating large physically
contigious buffers is problematic for many guests.



> Therefore, ideally, I'd rather have a longer queue and use VRING_DESC_F_NEXT 
> only and drop
> VRING_DESC_F_INDIRECT altogether.

One can always just avoid implementing VRING_DESC_F_INDIRECT if desired.

> And if delay spent in the queue is an issue. I think it's driver's job to 
> avoid bufferbloat if necessary 
> by adapting the number of descriptors is allows the device to use.
>
> > 
> > 
> > * Batching descriptors:
> > 
> > virtio 1.0 allows passing a batch of descriptors in both directions, by
> > incrementing the used/avail index by values > 1.  We can support this by
> > tagging first/middle/last descriptors in the flags field. For
> > comptibility, polarity is reversed so bit is set for non-first and
> > non-last descriptors in a batch:
> > 
> > #define BATCH_NOT_FIRST

[virtio-dev] [PATCH v11] virtio-net: add Max MTU configuration field

2016-09-19 Thread Aaron Conole 
It is helpful for a host to indicate it's MTU to be set on guest NICs
other than the assumed 1500 byte value.  This helps in situations where
the host network is using Jumbo Frames, or aiding in PMTU discovery by
configuring a homogenous network.  It is also helpful for sizing receive
buffers correctly.

The change adds a new field to configuration area of network
devices.  It will be used to pass a maximum MTU from the device to
the driver.  This will be used by the driver as a maximum value for
packet sizes during transmission, without segmentation offloading.

In addition, in order to support backward and forward compatibility,
we introduce a new feature bit called VIRTIO_NET_F_MTU.

VIRTIO-152
Signed-off-by: Aaron Conole 
Cc: Victor Kaplansky 
Reviewed-by: Hannes Reiencke 
Acked-by: Michael S. Tsirkin 
Acked-by: Maxime Coquelin 
---
   Note: should this proposal be accepted and approved, one or more
   claims disclosed to the TC admin and listed on the Virtio TC
   IPR page https://www.oasis-open.org/committees/virtio/ipr.php
   might become Essential Claims.

v1:
This is an attempt at continuing the work done by Victor Kaplansky on
mtu negiotiation for virtio-net devices. It attempts to pick up from
https://lists.oasis-open.org/archives/virtio-dev/201508/msg7.html
and is just a minor blurb from the first patch along with the 2nd patch
from the series, and some of the feedback integrated.

v2:
Rephrase and provide a mechanism for guest->host and host->guest
communication through a driver read-only and driver write-only field.

v3:
Converted to just support initial MTU. Guest->host and Host->guest MTU
changes are outside the scope of this change.

v4:
Removed references to 'initial', since that condition cannot be tested.
Simply state that if the driver will use the mtu field, it must
negotiate the feature bit, and if not, it must not.

v5:
After feedback from Michael S. Tsirkin

v6:
Bit has to change to bit 25 due to an undocumented bit 24 being taken.

v7:
Partial rewrite with feedback from MST.  Additional conformance statements
added.

v8:
Clarified the new conformance statements.

v9:
Updated the commit log for correctness.  Added ACKs from
Michael S. Tsirkin, and Maxime Coquelin.  Included the virtio
issue id.

v10:
Update the conformance statement wordings from previous 'offered' form
to 'succesfully negotiated' form.

v11:
Clarified size w.r.t. MTU - buffer sizes must also add extra space for ethernet
headers.  Also updated the feature bit description.

 content.tex | 37 +
 1 file changed, 37 insertions(+)

diff --git a/content.tex b/content.tex
index 4b45678..45b94c6 100644
--- a/content.tex
+++ b/content.tex
@@ -3049,6 +3049,11 @@ features.
 \item[VIRTIO_NET_F_CTRL_GUEST_OFFLOADS (2)] Control channel offloads
 reconfiguration support.
 
+\item[VIRTIO_NET_F_MTU(3)] Device maximum MTU reporting is supported. If
+offered by the device, device advises driver about the value of
+its maximum MTU. If negotiated, the driver uses \field{mtu} as
+the maximum MTU value.
+
 \item[VIRTIO_NET_F_MAC (5)] Device has given MAC address.
 
 \item[VIRTIO_NET_F_GUEST_TSO4 (7)] Driver can receive TSOv4.
@@ -3140,11 +3145,16 @@ of each of transmit and receive virtqueues 
(receiveq1\ldots receiveqN
 and transmitq1\ldots transmitqN respectively) that can be configured once 
VIRTIO_NET_F_MQ
 is negotiated.
 
+The following driver-read-only field, \field{mtu} only exists if
+VIRTIO_NET_F_MTU is set. This field specifies the maximum MTU for the driver to
+use.
+
 \begin{lstlisting}
 struct virtio_net_config {
 u8 mac[6];
 le16 status;
 le16 max_virtqueue_pairs;
+le16 mtu;
 };
 \end{lstlisting}
 
@@ -3153,6 +3163,23 @@ struct virtio_net_config {
 The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000 
inclusive,
 if it offers VIRTIO_NET_F_MQ.
 
+The device MUST set \field{mtu} to between 68 and 65535 inclusive,
+if it offers VIRTIO_NET_F_MTU.
+
+The device SHOULD set \field{mtu} to at least 1280, if it offers
+VIRTIO_NET_F_MTU.
+
+The device MUST NOT modify \field{mtu} once it has been set.
+
+The device MUST NOT pass received packets that exceed \field{mtu} (plus low
+level ethernet header length) size with \field{gso_type} NONE or ECN
+after VIRTIO_NET_F_MTU has been successfully negotiated.
+
+The device MUST forward transmitted packets of up to \field{mtu} (plus low
+level ethernet header length) size with \field{gso_type} NONE or ECN, and do
+so without fragmentation, after VIRTIO_NET_F_MTU has been successfully
+negotiated.
+
 \drivernormative{\subsubsection}{Device configuration layout}{Device Types / 
Network Device / Device configuration layout}
 
 A driver SHOULD negotiate VIRTIO_NET_F_MAC if the device offers it.
@@ -3165,6 +3192,16 @@ If the driver does not negotiate the VIRTIO_NET_F_STATUS 
feature, it

[virtio-dev] [PATCH v3 00/10] virtio-crypto: introduce framework and device emulation

2016-09-19 Thread Gonglei 
Changes since v2:
 According to Daniel's comments:
 - drop cryptodev kernel module as a cryptodev backend
 - rename crypto stuff to cryptodev stuff
 - change some files' license to GPLv2+
 - remove cryptodev command line instead of QOM to define the cryptodev backend
 - rename all functions and structures in crypto sub-directory.
 - add full inline documentation for cryptodev.h
 And:
 - drop crypto-queue.c [Paolo]
 - merge some patches

Great thanks to Daniel and Paolo. Please review again, thanks!

Changes since v1:
 - rmmove mixed endian-ness handler for virtio-crypto device, just
   use little-endian. [mst]
 - add sg list support according virtio-crypto spec v10 (will be posted soon).
 - fix a memory leak in session handler.
 - add a feature page link in qemu.org 
(http://qemu-project.org/Features/VirtioCrypto)
 - fix some trivial problems, sush as 's/Since 2.7/Since 2.8/g' in 
qapi-schema.json
 - rebase the latest qemu master tree.


This patch series realize the framework and emulation of a new
virtio crypto device, which is similar with virtio net device.
 
 - I introduce the cryptodev backend as the client of virtio crypto device
   which can be realized by different methods, such as cryptodev-backend-gcrypt 
in my series,
   vhost-crypto kernel module, vhost-user etc.
 - The patch set abides by the virtio crypto speccification.
 - The virtio crypto support symmetric algorithms (including CIPHER and 
algorithm chainning)
   at present, except HASH, MAC and AEAD services.
 - unsupport hot plug/unplug cryptodev backend at this moment.

Firstly build QEMU with libgcrypt cryptography support. 

QEMU can then be started using the following parameters:

qemu-system-x86_64 \
[...] \
-object cryptodev-backend-gcrypt,id=cryptodev0 \
-device virtio-crypto-pci,id=crypto0,cryptodev=cryptodev0 \
[...]

The front-end linux kernel driver (Experimental at present) is publicly 
accessible from:
 
   https://github.com/gongleiarei/virtio-crypto-linux-driver.git

After insmod virtio-crypto.ko, you can use cryptodev-linux test the crypto 
function
in the guest. For example:

linux-guest:/home/gonglei/cryptodev-linux/tests # ./cipher -
requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver virtio_crypto_aes_cbc
AES Test passed
requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver virtio_crypto_aes_cbc
requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver virtio_crypto_aes_cbc
Test passed

QEMU code also can be accessible from:

 https://github.com/gongleiarei/qemu.git 

 branch virtio-crypto

For more information, please see:
 http://qemu-project.org/Features/VirtioCrypto


Gonglei (10):
  cryptodev: introduce cryptodev backend interface
  cryptodev: add symmetric algorithm operation stuff
  virtio-crypto: introduce virtio_crypto.h
  cryptodev: introduce gcrypt lib as a new cryptodev backend
  virtio-crypto: add virtio crypto device emulation
  virtio-crypto-pci: add virtio crypto pci support
  virtio-crypto: set capacity of algorithms supported
  virtio-crypto: add control queue handler
  virtio-crypto: add data queue processing handler
  cryptodev: introduce an unified wrapper for crypto operation

 crypto/Makefile.objs   |   2 +
 crypto/cryptodev-gcrypt.c  | 329 +
 crypto/cryptodev.c | 243 +++
 hw/virtio/Makefile.objs|   2 +
 hw/virtio/virtio-crypto-pci.c  |  76 +++
 hw/virtio/virtio-crypto.c  | 904 +
 hw/virtio/virtio-pci.h |  15 +
 include/crypto/cryptodev.h | 276 
 include/hw/virtio/virtio-crypto.h  | 100 +++
 include/standard-headers/linux/virtio_crypto.h | 466 +
 qemu-options.hx|  18 +
 11 files changed, 2431 insertions(+)
 create mode 100644 crypto/cryptodev-gcrypt.c
 create mode 100644 crypto/cryptodev.c
 create mode 100644 hw/virtio/virtio-crypto-pci.c
 create mode 100644 hw/virtio/virtio-crypto.c
 create mode 100644 include/crypto/cryptodev.h
 create mode 100644 include/hw/virtio/virtio-crypto.h
 create mode 100644 include/standard-headers/linux/virtio_crypto.h

-- 
1.7.12.4



-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] [PATCH v3 07/10] virtio-crypto: set capacity of algorithms supported

2016-09-19 Thread Gonglei 
Expose the capacity of algorithms supported by
virtio crypto device to the frontend driver using
pci configuration space.

Signed-off-by: Gonglei 
---
 hw/virtio/virtio-crypto.c | 39 ++-
 include/hw/virtio/virtio-crypto.h | 14 ++
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index b1c10b2..e78656c 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -87,6 +87,26 @@ static void virtio_crypto_reset(VirtIODevice *vdev)
 }
 }
 
+static void virtio_crypto_init_config(VirtIODevice *vdev)
+{
+VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev);
+
+vcrypto->conf.crypto_services =
+ vcrypto->conf.cryptodev->conf.crypto_services;
+vcrypto->conf.cipher_algo_l =
+ vcrypto->conf.cryptodev->conf.cipher_algo_l;
+vcrypto->conf.cipher_algo_h =
+ vcrypto->conf.cryptodev->conf.cipher_algo_h;
+vcrypto->conf.hash_algo = vcrypto->conf.cryptodev->conf.hash_algo;
+vcrypto->conf.mac_algo_l = vcrypto->conf.cryptodev->conf.mac_algo_l;
+vcrypto->conf.mac_algo_h = vcrypto->conf.cryptodev->conf.mac_algo_h;
+vcrypto->conf.asym_algo = vcrypto->conf.cryptodev->conf.asym_algo;
+vcrypto->conf.kdf_algo = vcrypto->conf.cryptodev->conf.kdf_algo;
+vcrypto->conf.aead_algo = vcrypto->conf.cryptodev->conf.aead_algo;
+vcrypto->conf.primitive_algo =
+ vcrypto->conf.cryptodev->conf.primitive_algo;
+}
+
 static void virtio_crypto_device_realize(DeviceState *dev, Error **errp)
 {
 VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -120,6 +140,7 @@ static void virtio_crypto_device_realize(DeviceState *dev, 
Error **errp)
 } else {
 vcrypto->status |= VIRTIO_CRYPTO_S_HW_READY;
 }
+virtio_crypto_init_config(vdev);
 register_savevm(dev, "virtio-crypto", -1, 1, virtio_crypto_save,
 virtio_crypto_load, vcrypto);
 }
@@ -140,7 +161,23 @@ static Property virtio_crypto_properties[] = {
 
 static void virtio_crypto_get_config(VirtIODevice *vdev, uint8_t *config)
 {
-
+VirtIOCrypto *c = VIRTIO_CRYPTO(vdev);
+struct virtio_crypto_config crypto_cfg;
+
+crypto_cfg.status = c->status;
+crypto_cfg.max_dataqueues = c->max_queues;
+crypto_cfg.crypto_services = c->conf.crypto_services;
+crypto_cfg.cipher_algo_l = c->conf.cipher_algo_l;
+crypto_cfg.cipher_algo_h = c->conf.cipher_algo_h;
+crypto_cfg.hash_algo = c->conf.hash_algo;
+crypto_cfg.mac_algo_l = c->conf.mac_algo_l;
+crypto_cfg.mac_algo_h = c->conf.mac_algo_h;
+crypto_cfg.asym_algo = c->conf.asym_algo;
+crypto_cfg.kdf_algo = c->conf.kdf_algo;
+crypto_cfg.aead_algo = c->conf.aead_algo;
+crypto_cfg.primitive_algo = c->conf.primitive_algo;
+
+memcpy(config, _cfg, c->config_size);
 }
 
 static void virtio_crypto_set_config(VirtIODevice *vdev, const uint8_t *config)
diff --git a/include/hw/virtio/virtio-crypto.h 
b/include/hw/virtio/virtio-crypto.h
index 2fe2499..f6fe2d8 100644
--- a/include/hw/virtio/virtio-crypto.h
+++ b/include/hw/virtio/virtio-crypto.h
@@ -39,6 +39,20 @@ do { printf("virtio_crypto: " fmt , ## __VA_ARGS__); } while 
(0)
 
 typedef struct VirtIOCryptoConf {
 QCryptoCryptoDevBackend *cryptodev;
+
+/* Supported service mask */
+uint32_t crypto_services;
+
+/* Detailed algorithms mask */
+uint32_t cipher_algo_l;
+uint32_t cipher_algo_h;
+uint32_t hash_algo;
+uint32_t mac_algo_l;
+uint32_t mac_algo_h;
+uint32_t asym_algo;
+uint32_t kdf_algo;
+uint32_t aead_algo;
+uint32_t primitive_algo;
 } VirtIOCryptoConf;
 
 struct VirtIOCrypto;
-- 
1.7.12.4



-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] [PATCH v3 05/10] virtio-crypto: add virtio crypto device emulation

2016-09-19 Thread Gonglei 
Introduce the virtio crypto realization, I'll
finish the core code in the following patches. The
thoughts came from virtio net realization.

For more information see:
http://qemu-project.org/Features/VirtioCrypto

Signed-off-by: Gonglei 
---
 hw/virtio/Makefile.objs   |   1 +
 hw/virtio/virtio-crypto.c | 197 ++
 include/hw/virtio/virtio-crypto.h |  75 +++
 3 files changed, 273 insertions(+)
 create mode 100644 hw/virtio/virtio-crypto.c
 create mode 100644 include/hw/virtio/virtio-crypto.h

diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index e716308..968f392 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -7,3 +7,4 @@ obj-y += virtio.o virtio-balloon.o
 obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
 
 obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
+obj-y += virtio-crypto.o
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
new file mode 100644
index 000..b1c10b2
--- /dev/null
+++ b/hw/virtio/virtio-crypto.c
@@ -0,0 +1,197 @@
+/*
+ * Virtio crypto Support
+ *
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *Gonglei 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "qemu/iov.h"
+#include "hw/qdev.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-crypto.h"
+#include "hw/virtio/virtio-access.h"
+
+static void virtio_crypto_process(VirtIOCrypto *vcrypto)
+{
+}
+
+static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+{
+
+}
+
+static void virtio_crypto_handle_dataq(VirtIODevice *vdev, VirtQueue *vq)
+{
+
+}
+
+static uint64_t virtio_crypto_get_features(VirtIODevice *vdev,
+   uint64_t features,
+   Error **errp)
+{
+return features;
+}
+
+static void virtio_crypto_set_features(VirtIODevice *vdev, uint64_t features)
+{
+
+}
+
+static void virtio_crypto_save(QEMUFile *f, void *opaque)
+{
+VirtIODevice *vdev = opaque;
+
+virtio_save(vdev, f);
+}
+
+static int virtio_crypto_load(QEMUFile *f, void *opaque, int version_id)
+{
+VirtIOCrypto *vcrypto = opaque;
+int ret;
+
+if (version_id != 1) {
+return -EINVAL;
+}
+ret = virtio_load(VIRTIO_DEVICE(vcrypto), f, version_id);
+if (ret != 0) {
+return ret;
+}
+
+/* We may have an element ready but couldn't process it due to a quota
+ * limit.  Make sure to try again after live migration when the quota may
+ * have been reset.
+ */
+virtio_crypto_process(vcrypto);
+
+return 0;
+}
+
+static void virtio_crypto_reset(VirtIODevice *vdev)
+{
+VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev);
+/* multiqueue is disabled by default */
+vcrypto->curr_queues = 1;
+if (!vcrypto->cryptodev->ready) {
+vcrypto->status &= ~VIRTIO_CRYPTO_S_HW_READY;
+} else {
+vcrypto->status |= VIRTIO_CRYPTO_S_HW_READY;
+}
+}
+
+static void virtio_crypto_device_realize(DeviceState *dev, Error **errp)
+{
+VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(dev);
+int i;
+
+vcrypto->cryptodev = vcrypto->conf.cryptodev;
+if (vcrypto->cryptodev == NULL) {
+error_setg(errp, "'cryptodev' parameter expects a valid object");
+return;
+}
+
+vcrypto->max_queues = MAX(vcrypto->cryptodev->conf.peers.queues, 1);
+if (vcrypto->max_queues + 1 > VIRTIO_QUEUE_MAX) {
+error_setg(errp, "Invalid number of queues (= %" PRIu16 "), "
+   "must be a postive integer less than %d.",
+   vcrypto->max_queues, VIRTIO_QUEUE_MAX - 1);
+return;
+}
+
+virtio_init(vdev, "virtio-crypto", VIRTIO_ID_CRYPTO, vcrypto->config_size);
+vcrypto->curr_queues = 1;
+
+for (i = 0; i < vcrypto->max_queues; i++) {
+virtio_add_queue(vdev, 1024, virtio_crypto_handle_dataq);
+}
+
+vcrypto->ctrl_vq = virtio_add_queue(vdev, 64, virtio_crypto_handle_ctrl);
+if (!vcrypto->cryptodev->ready) {
+vcrypto->status &= ~VIRTIO_CRYPTO_S_HW_READY;
+} else {
+vcrypto->status |= VIRTIO_CRYPTO_S_HW_READY;
+}
+register_savevm(dev, "virtio-crypto", -1, 1, virtio_crypto_save,
+virtio_crypto_load, vcrypto);
+}
+
+static void virtio_crypto_device_unrealize(DeviceState *dev, Error **errp)
+{
+VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(dev);
+
+unregister_savevm(dev, "virtio-crypto", vcrypto);
+
+virtio_cleanup(vdev);
+}
+
+static Property virtio_crypto_properties[] = {
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_crypto_get_config(VirtIODevice *vdev, uint8_t *config)