Re: [Qemu-devel] [RFC v1 8/9] virtio-crypto: add host feature bits support

[Cornelia Huck]

On Fri, 12 May 2017 00:55:23 +0000
"Gonglei (Arei)" <address@hidden> wrote:

> >
> > From: Cornelia Huck [mailto:address@hidden
> > Sent: Thursday, May 11, 2017 11:05 PM
> > Subject: Re: [RFC v1 8/9] virtio-crypto: add host feature bits support
> > 
> > On Mon, 8 May 2017 19:38:23 +0800
> > Gonglei <address@hidden> wrote:
> > 
> > > We enable all feature bits acquiescently.
> > >
> > > Signed-off-by: Gonglei <address@hidden>
> > > ---
> > >  hw/virtio/virtio-crypto.c         | 15 +++++++++++++++
> > >  include/hw/virtio/virtio-crypto.h |  1 +
> > >  2 files changed, 16 insertions(+)
> > >
> > > diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
> > > index 5422f25..3dc0ff2 100644
> > > --- a/hw/virtio/virtio-crypto.c
> > > +++ b/hw/virtio/virtio-crypto.c
> > > @@ -1034,6 +1034,11 @@ static uint64_t
> > virtio_crypto_get_features(VirtIODevice *vdev,
> > >                                             uint64_t features,
> > >                                             Error **errp)
> > >  {
> > > +    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev);
> > > +
> > > +    /* Firstly sync all virtio-crypto possible supported features */
> > > +    features |= vcrypto->host_features;
> > > +
> > >      return features;
> > >  }
> > >
> > > @@ -1144,6 +1149,16 @@ static const VMStateDescription
> > vmstate_virtio_crypto = {
> > >  };
> > >
> > >  static Property virtio_crypto_properties[] = {
> > > +    DEFINE_PROP_BIT("mux_mode", VirtIOCrypto, host_features,
> > > +                    VIRTIO_CRYPTO_F_MUX_MODE, true),
> > > +    DEFINE_PROP_BIT("cipher_stateless_mode", VirtIOCrypto,
> > host_features,
> > > +                    VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE,
> > true),
> > > +    DEFINE_PROP_BIT("hash_stateless_mode", VirtIOCrypto,
> > host_features,
> > > +                    VIRTIO_CRYPTO_F_HASH_STATELESS_MODE,
> > true),
> > > +    DEFINE_PROP_BIT("mac_stateless_mode", VirtIOCrypto,
> > host_features,
> > > +                    VIRTIO_CRYPTO_F_MAC_STATELESS_MODE, true),
> > > +    DEFINE_PROP_BIT("aead_stateless_mode", VirtIOCrypto,
> > host_features,
> > > +                    VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE,
> > true),
> > >      DEFINE_PROP_END_OF_LIST(),
> > >  };
> > >
> > > diff --git a/include/hw/virtio/virtio-crypto.h 
> > > b/include/hw/virtio/virtio-crypto.h
> > > index 465ad20..30ea51d 100644
> > > --- a/include/hw/virtio/virtio-crypto.h
> > > +++ b/include/hw/virtio/virtio-crypto.h
> > > @@ -97,6 +97,7 @@ typedef struct VirtIOCrypto {
> > >      int multiqueue;
> > >      uint32_t curr_queues;
> > >      size_t config_size;
> > > +    uint32_t host_features;
> > 
> > I'd just make that 64 bits from the start.
> > 
> Yes, that's better.
> 
> > >  } VirtIOCrypto;
> > >
> > >  #endif /* _QEMU_VIRTIO_CRYPTO_H */
> > 
> > Don't you need some kind of compat handling?
> 
> I did that in patch 6 according to the results of those feature bits 
> negotiated.
> Patch 9 tests both session mode and stateless mode, they are work. :)

Ah, I meant machine compat handling. You probably don't want to offer
these feature bits in older compat machines.