Re: [PATCH RFC] virtio 1.0 vring endian-ness

[Cornelia Huck]

On Wed, 22 Oct 2014 01:09:40 +0300
Michael S. Tsirkin m...@redhat.com wrote:

 This adds wrappers to switch between native endian-ness
 (virtio 0.9) and virtio endian-ness (virtio 1.0).
 Add new typedefs as well, so that we can check
 statically that we didn't miss any accesses.
 All callers simply pass in false (0.9) so no
 functional change for now.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 
 ---
 
 Sending this early so I can get feedback on this style.

Hm...

http://marc.info/?l=linux-virtualizationm=141270444612625w=2

(and other in that series. Forgot to cc: you on those patches...)

 Rusty, what's your opinion? Reasonable?
 
 diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
 index 67e06fe..32211aa 100644
 --- a/include/linux/virtio_ring.h
 +++ b/include/linux/virtio_ring.h
 @@ -62,6 +62,26 @@ static inline void virtio_wmb(bool weak_barriers)
  }
  #endif
 
 +#define DEFINE_VIRTIO_XX_TO_CPU(bits) \
 +static inline u##bits virtio##bits##_to_cpu(bool little_endian, 
 __virtio##bits val) \
 +{ \
 + if (little_endian) \
 + return le##bits##_to_cpu((__force __le##bits)val); \
 + else \
 + return (__force u##bits)val; \
 +} \
 +static inline __virtio##bits cpu_to_virtio##bits(bool little_endian, u##bits 
 val) \
 +{ \
 + if (little_endian) \
 + return (__force __virtio##bits)cpu_to_le##bits(val); \
 + else \
 + return val; \
 +}
 +
 +DEFINE_VIRTIO_XX_TO_CPU(16)
 +DEFINE_VIRTIO_XX_TO_CPU(32)
 +DEFINE_VIRTIO_XX_TO_CPU(64)
 +

I'm usually not very fond of creating functions via macros like that as
it makes it hard to grep for them.

  struct virtio_device;
  struct virtqueue;
 
 diff --git a/include/uapi/linux/virtio_ring.h 
 b/include/uapi/linux/virtio_ring.h
 index a99f9b7..744cee1 100644
 --- a/include/uapi/linux/virtio_ring.h
 +++ b/include/uapi/linux/virtio_ring.h
 @@ -33,6 +33,10 @@
   * Copyright Rusty Russell IBM Corporation 2007. */
  #include linux/types.h
 
 +typedef __u16 __bitwise __virtio16;
 +typedef __u32 __bitwise __virtio32;
 +typedef __u64 __bitwise __virtio64;
 +
  /* This marks a buffer as continuing via the next field. */
  #define VRING_DESC_F_NEXT1
  /* This marks a buffer as write-only (otherwise read-only). */
 @@ -61,32 +65,32 @@
  /* Virtio ring descriptors: 16 bytes.  These can chain together via next. 
 */
  struct vring_desc {
   /* Address (guest-physical). */
 - __u64 addr;
 + __virtio64 addr;
   /* Length. */
 - __u32 len;
 + __virtio32 len;
   /* The flags as indicated above. */
 - __u16 flags;
 + __virtio16 flags;
   /* We chain unused descriptors via this, too */
 - __u16 next;
 + __virtio16 next;
  };
 
  struct vring_avail {
 - __u16 flags;
 - __u16 idx;
 - __u16 ring[];
 + __virtio16 flags;
 + __virtio16 idx;
 + __virtio16 ring[];
  };

This looks weird. At least to me, that would scream virtio endian,
which is only true for legacy devices.

I understand that you want it to be statically checkable, but I think
it decreases readability.

 
  /* u32 is used here for ids for padding reasons. */
  struct vring_used_elem {
   /* Index of start of used descriptor chain. */
 - __u32 id;
 + __virtio32 id;
   /* Total length of the descriptor chain which was used (written to) */
 - __u32 len;
 + __virtio32 len;
  };
 
  struct vring_used {
 - __u16 flags;
 - __u16 idx;
 + __virtio16 flags;
 + __virtio16 idx;
   struct vring_used_elem ring[];
  };
 

 diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
 index 61a1fe1..a2f2f22 100644
 --- a/drivers/virtio/virtio_ring.c
 +++ b/drivers/virtio/virtio_ring.c
 @@ -98,6 +98,8 @@ struct vring_virtqueue
  };
 
  #define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
 +/* Will become vq-little_endian once we support virtio 1.0 */
 +#define vq_le(vq) (false)

All virtqueues inherit this property from their device, right? Do you
want to propagate this to the virtqueues if the guest negotiated
virtio-1 for the device?

 
  static struct vring_desc *alloc_indirect(unsigned int total_sg, gfp_t gfp)
  {

 @@ -235,13 +237,13 @@ static inline int virtqueue_add(struct virtqueue *_vq,
 
   /* Put entry in available array (but don't update avail-idx until they
* do sync). */
 - avail = (vq-vring.avail-idx  (vq-vring.num-1));
 - vq-vring.avail-ring[avail] = head;
 + avail = virtio16_to_cpu(vq_le(_vq), vq-vring.avail-idx)  
 (vq-vring.num - 1);
 + vq-vring.avail-ring[avail] = cpu_to_virtio16(vq_le(_vq), head);
 
   /* Descriptors and available array need to be set before we expose the
* new available array entries. */
   virtio_wmb(vq-weak_barriers);
 - vq-vring.avail-idx++;
 + vq-vring.avail-idx = cpu_to_virtio16(vq_le(_vq), 
 virtio16_to_cpu(vq_le(_vq), vq-vring.avail-idx) + 1);

I think this line looks awful :(

I also notice that you need to call the 

Re: [PATCH RFC] virtio 1.0 vring endian-ness

[Michael S. Tsirkin]

On Wed, Oct 22, 2014 at 10:24:17AM +0200, Cornelia Huck wrote:
 On Wed, 22 Oct 2014 01:09:40 +0300
 Michael S. Tsirkin m...@redhat.com wrote:
 
  This adds wrappers to switch between native endian-ness
  (virtio 0.9) and virtio endian-ness (virtio 1.0).
  Add new typedefs as well, so that we can check
  statically that we didn't miss any accesses.
  All callers simply pass in false (0.9) so no
  functional change for now.
  
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
  
  ---
  
  Sending this early so I can get feedback on this style.
 
 Hm...
 
 http://marc.info/?l=linux-virtualizationm=141270444612625w=2
 
 (and other in that series. Forgot to cc: you on those patches...)

Thanks, will review.


  Rusty, what's your opinion? Reasonable?
  
  diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
  index 67e06fe..32211aa 100644
  --- a/include/linux/virtio_ring.h
  +++ b/include/linux/virtio_ring.h
  @@ -62,6 +62,26 @@ static inline void virtio_wmb(bool weak_barriers)
   }
   #endif
  
  +#define DEFINE_VIRTIO_XX_TO_CPU(bits) \
  +static inline u##bits virtio##bits##_to_cpu(bool little_endian, 
  __virtio##bits val) \
  +{ \
  +   if (little_endian) \
  +   return le##bits##_to_cpu((__force __le##bits)val); \
  +   else \
  +   return (__force u##bits)val; \
  +} \
  +static inline __virtio##bits cpu_to_virtio##bits(bool little_endian, 
  u##bits val) \
  +{ \
  +   if (little_endian) \
  +   return (__force __virtio##bits)cpu_to_le##bits(val); \
  +   else \
  +   return val; \
  +}
  +
  +DEFINE_VIRTIO_XX_TO_CPU(16)
  +DEFINE_VIRTIO_XX_TO_CPU(32)
  +DEFINE_VIRTIO_XX_TO_CPU(64)
  +
 
 I'm usually not very fond of creating functions via macros like that as
 it makes it hard to grep for them.
 
   struct virtio_device;
   struct virtqueue;
  
  diff --git a/include/uapi/linux/virtio_ring.h 
  b/include/uapi/linux/virtio_ring.h
  index a99f9b7..744cee1 100644
  --- a/include/uapi/linux/virtio_ring.h
  +++ b/include/uapi/linux/virtio_ring.h
  @@ -33,6 +33,10 @@
* Copyright Rusty Russell IBM Corporation 2007. */
   #include linux/types.h
  
  +typedef __u16 __bitwise __virtio16;
  +typedef __u32 __bitwise __virtio32;
  +typedef __u64 __bitwise __virtio64;
  +
   /* 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). */
  @@ -61,32 +65,32 @@
   /* Virtio ring descriptors: 16 bytes.  These can chain together via 
  next. */
   struct vring_desc {
  /* Address (guest-physical). */
  -   __u64 addr;
  +   __virtio64 addr;
  /* Length. */
  -   __u32 len;
  +   __virtio32 len;
  /* The flags as indicated above. */
  -   __u16 flags;
  +   __virtio16 flags;
  /* We chain unused descriptors via this, too */
  -   __u16 next;
  +   __virtio16 next;
   };
  
   struct vring_avail {
  -   __u16 flags;
  -   __u16 idx;
  -   __u16 ring[];
  +   __virtio16 flags;
  +   __virtio16 idx;
  +   __virtio16 ring[];
   };
 
 This looks weird. At least to me, that would scream virtio endian,
 which is only true for legacy devices.
 
 I understand that you want it to be statically checkable, but I think
 it decreases readability.
 
  
   /* u32 is used here for ids for padding reasons. */
   struct vring_used_elem {
  /* Index of start of used descriptor chain. */
  -   __u32 id;
  +   __virtio32 id;
  /* Total length of the descriptor chain which was used (written to) */
  -   __u32 len;
  +   __virtio32 len;
   };
  
   struct vring_used {
  -   __u16 flags;
  -   __u16 idx;
  +   __virtio16 flags;
  +   __virtio16 idx;
  struct vring_used_elem ring[];
   };
  
 
  diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
  index 61a1fe1..a2f2f22 100644
  --- a/drivers/virtio/virtio_ring.c
  +++ b/drivers/virtio/virtio_ring.c
  @@ -98,6 +98,8 @@ struct vring_virtqueue
   };
  
   #define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
  +/* Will become vq-little_endian once we support virtio 1.0 */
  +#define vq_le(vq) (false)
 
 All virtqueues inherit this property from their device, right? Do you
 want to propagate this to the virtqueues if the guest negotiated
 virtio-1 for the device?
 
  
   static struct vring_desc *alloc_indirect(unsigned int total_sg, gfp_t gfp)
   {
 
  @@ -235,13 +237,13 @@ static inline int virtqueue_add(struct virtqueue *_vq,
  
  /* Put entry in available array (but don't update avail-idx until they
   * do sync). */
  -   avail = (vq-vring.avail-idx  (vq-vring.num-1));
  -   vq-vring.avail-ring[avail] = head;
  +   avail = virtio16_to_cpu(vq_le(_vq), vq-vring.avail-idx)  
  (vq-vring.num - 1);
  +   vq-vring.avail-ring[avail] = cpu_to_virtio16(vq_le(_vq), head);
  
  /* Descriptors and available array need to be set before we expose the
   * new available array entries. */
  virtio_wmb(vq-weak_barriers);
  -   vq-vring.avail-idx++;
  +   vq-vring.avail-idx = 

Re: [PATCH RFC 00/11] qemu: towards virtio-1 host support

[Michael S. Tsirkin]

On Wed, Oct 08, 2014 at 11:04:28AM +0200, Cornelia Huck wrote:
 On Tue, 07 Oct 2014 18:24:22 -0700
 Andy Lutomirski l...@amacapital.net wrote:
 
  On 10/07/2014 07:39 AM, Cornelia Huck wrote:
   This patchset aims to get us some way to implement virtio-1 compliant
   and transitional devices in qemu. Branch available at
   
   git://github.com/cohuck/qemu virtio-1
   
   I've mainly focused on:
   - endianness handling
   - extended feature bits
   - virtio-ccw new/changed commands
  
  At the risk of some distraction, would it be worth thinking about a
  solution to the IOMMU bypassing mess as part of this?
 
 I think that is a whole different issue. virtio-1 is basically done - we
 just need to implement it - while the IOMMU/DMA stuff certainly needs
 more discussion. Therefore, I'd like to defer to the other discussion
 thread here.

I agree, let's do a separate thread for this.
I also think it's up to the hypervisors at this point.
People talked about using ACPI to report IOMMU bypass
to guest.
If that happens, we don't need a feature bit.


 ___
 Virtualization mailing list
 Virtualization@lists.linux-foundation.org
 https://lists.linuxfoundation.org/mailman/listinfo/virtualization
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] [PATCH RFC 03/11] virtio: endianess conversion helpers

[Michael S. Tsirkin]

On Tue, Oct 07, 2014 at 04:39:44PM +0200, Cornelia Huck wrote:
 Provide helper functions that convert from/to LE for virtio devices that
 are not operating in legacy mode. We check for the VERSION_1 feature bit
 to determine that.
 
 Based on original patches by Rusty Russell and Thomas Huth.
 
 Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
 Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com


I'm worried that this might miss some conversion.
Let's add new typedefs __virtio16/__virtio32/__virtio64
instead.

This way we can use static checkers to catch bugs.

This is what my patch does, let me try to split
it up so parts are reusable for you.


Also if we do this, then virtio32_to_cpu is a better API
since it's closer to the type name.

 ---
  drivers/virtio/virtio.c|4 
  include/linux/virtio.h |   40 
 
  include/uapi/linux/virtio_config.h |3 +++
  3 files changed, 47 insertions(+)
 
 diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
 index cfd5d00..8f74cd6 100644
 --- a/drivers/virtio/virtio.c
 +++ b/drivers/virtio/virtio.c
 @@ -144,6 +144,10 @@ static int virtio_dev_probe(struct device *_d)
   if (device_features  (1ULL  i))
   dev-features |= (1ULL  i);
  
 + /* Version 1.0 compliant devices set the VIRTIO_F_VERSION_1 bit */
 + if (device_features  (1ULL  VIRTIO_F_VERSION_1))
 + dev-features |= (1ULL  VIRTIO_F_VERSION_1);
 +
   dev-config-finalize_features(dev);
  
   err = drv-probe(dev);
 diff --git a/include/linux/virtio.h b/include/linux/virtio.h
 index a24b41f..68cadd4 100644
 --- a/include/linux/virtio.h
 +++ b/include/linux/virtio.h
 @@ -9,6 +9,7 @@
  #include linux/mod_devicetable.h
  #include linux/gfp.h
  #include linux/vringh.h
 +#include uapi/linux/virtio_config.h
  
  /**
   * virtqueue - a queue to register buffers for sending or receiving.
 @@ -102,6 +103,11 @@ static inline struct virtio_device *dev_to_virtio(struct 
 device *_dev)
   return container_of(_dev, struct virtio_device, dev);
  }
  
 +static inline bool virtio_device_legacy(const struct virtio_device *dev)
 +{
 + return !(dev-features  (1ULL  VIRTIO_F_VERSION_1));
 +}
 +
  int register_virtio_device(struct virtio_device *dev);
  void unregister_virtio_device(struct virtio_device *dev);
  
 @@ -149,4 +155,38 @@ void unregister_virtio_driver(struct virtio_driver *drv);
  #define module_virtio_driver(__virtio_driver) \
   module_driver(__virtio_driver, register_virtio_driver, \
   unregister_virtio_driver)
 +
 +/*
 + * v1.0 specifies LE headers, legacy was native endian. Therefore, we must
 + * convert from/to LE if and only if vdev is not legacy.
 + */
 +static inline u16 virtio_to_cpu_u16(const struct virtio_device *vdev, u16 v)
 +{
 + return virtio_device_legacy(vdev) ? v : le16_to_cpu(v);
 +}
 +
 +static inline u32 virtio_to_cpu_u32(const struct virtio_device *vdev, u32 v)
 +{
 + return virtio_device_legacy(vdev) ? v : le32_to_cpu(v);
 +}
 +
 +static inline u64 virtio_to_cpu_u64(const struct virtio_device *vdev, u64 v)
 +{
 + return virtio_device_legacy(vdev) ? v : le64_to_cpu(v);
 +}
 +
 +static inline u16 cpu_to_virtio_u16(const struct virtio_device *vdev, u16 v)
 +{
 + return virtio_device_legacy(vdev) ? v : cpu_to_le16(v);
 +}
 +
 +static inline u32 cpu_to_virtio_u32(const struct virtio_device *vdev, u32 v)
 +{
 + return virtio_device_legacy(vdev) ? v : cpu_to_le32(v);
 +}
 +
 +static inline u64 cpu_to_virtio_u64(const struct virtio_device *vdev, u64 v)
 +{
 + return virtio_device_legacy(vdev) ? v : cpu_to_le64(v);
 +}
  #endif /* _LINUX_VIRTIO_H */

Would be nicer to allow callers to pass in the legacy flag I think?
This way they can keep it on stack to avoid re-reading features
all the time ...


 diff --git a/include/uapi/linux/virtio_config.h 
 b/include/uapi/linux/virtio_config.h
 index 3ce768c..80e7381 100644
 --- a/include/uapi/linux/virtio_config.h
 +++ b/include/uapi/linux/virtio_config.h
 @@ -54,4 +54,7 @@
  /* Can the device handle any descriptor layout? */
  #define VIRTIO_F_ANY_LAYOUT  27
  
 +/* v1.0 compliant. */
 +#define VIRTIO_F_VERSION_1   32
 +
  #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
 -- 
 1.7.9.5
 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC 04/11] virtio_ring: implement endian reversal based on VERSION_1 feature.

[Michael S. Tsirkin]

On Tue, Oct 07, 2014 at 04:39:45PM +0200, Cornelia Huck wrote:
 From: Rusty Russell ru...@rustcorp.com.au
 
 [Cornelia Huck: we don't need the vq-vring.num - vq-ring_mask change]
 Signed-off-by: Rusty Russell ru...@rustcorp.com.au
 Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
 ---
  drivers/virtio/virtio_ring.c |  195 
 ++
  1 file changed, 138 insertions(+), 57 deletions(-)
 
 diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
 index 1cfb5ba..350c39b 100644
 --- a/drivers/virtio/virtio_ring.c
 +++ b/drivers/virtio/virtio_ring.c
 @@ -145,42 +145,54 @@ static inline int vring_add_indirect(struct 
 vring_virtqueue *vq,
   i = 0;
   for (n = 0; n  out_sgs; n++) {
   for (sg = sgs[n]; sg; sg = next(sg, total_out)) {
 - desc[i].flags = VRING_DESC_F_NEXT;
 - desc[i].addr = sg_phys(sg);
 - desc[i].len = sg-length;
 - desc[i].next = i+1;
 + desc[i].flags = cpu_to_virtio16(vq-vq.vdev,
 +   VRING_DESC_F_NEXT);
 + desc[i].addr = cpu_to_virtio64(vq-vq.vdev,
 +  sg_phys(sg));
 + desc[i].len = cpu_to_virtio32(vq-vq.vdev,
 + sg-length);
 + desc[i].next = cpu_to_virtio16(vq-vq.vdev,
 +  i+1);
   i++;
   }
   }
   for (; n  (out_sgs + in_sgs); n++) {
   for (sg = sgs[n]; sg; sg = next(sg, total_in)) {
 - desc[i].flags = VRING_DESC_F_NEXT|VRING_DESC_F_WRITE;
 - desc[i].addr = sg_phys(sg);
 - desc[i].len = sg-length;
 - desc[i].next = i+1;
 + desc[i].flags = cpu_to_virtio16(vq-vq.vdev,
 +   VRING_DESC_F_NEXT|
 +   VRING_DESC_F_WRITE);
 + desc[i].addr = cpu_to_virtio64(vq-vq.vdev,
 +  sg_phys(sg));
 + desc[i].len = cpu_to_virtio32(vq-vq.vdev,
 + sg-length);
 + desc[i].next = cpu_to_virtio16(vq-vq.vdev, i+1);
   i++;
   }
   }
 - BUG_ON(i != total_sg);
  
   /* Last one doesn't continue. */
 - desc[i-1].flags = ~VRING_DESC_F_NEXT;
 + desc[i-1].flags = ~cpu_to_virtio16(vq-vq.vdev, VRING_DESC_F_NEXT);
   desc[i-1].next = 0;
  
 - /* We're about to use a buffer */
 - vq-vq.num_free--;
 -
   /* Use a single buffer which doesn't continue */
   head = vq-free_head;
 - vq-vring.desc[head].flags = VRING_DESC_F_INDIRECT;
 - vq-vring.desc[head].addr = virt_to_phys(desc);
 + vq-vring.desc[head].flags =
 + cpu_to_virtio16(vq-vq.vdev, VRING_DESC_F_INDIRECT);
 + vq-vring.desc[head].addr =
 + cpu_to_virtio64(vq-vq.vdev, virt_to_phys(desc));
   /* kmemleak gives a false positive, as it's hidden by virt_to_phys */
   kmemleak_ignore(desc);
 - vq-vring.desc[head].len = i * sizeof(struct vring_desc);
 + vq-vring.desc[head].len =
 + cpu_to_virtio32(vq-vq.vdev, i * sizeof(struct vring_desc));
  
 - /* Update free pointer */
 + BUG_ON(i != total_sg);
 +

Why move the BUG_ON here?
I think I'll move it back ...

 + /* Update free pointer (we store this in native endian) */
   vq-free_head = vq-vring.desc[head].next;
  
 + /* We've just used a buffer */
 + vq-vq.num_free--;
 +
   return head;
  }
  
 @@ -199,6 +211,7 @@ static inline int virtqueue_add(struct virtqueue *_vq,
   struct scatterlist *sg;
   unsigned int i, n, avail, uninitialized_var(prev), total_sg;
   int head;
 + u16 nexti;
  
   START_USE(vq);
  
 @@ -253,26 +266,46 @@ static inline int virtqueue_add(struct virtqueue *_vq,
   vq-vq.num_free -= total_sg;
  
   head = i = vq-free_head;
 +
   for (n = 0; n  out_sgs; n++) {
   for (sg = sgs[n]; sg; sg = next(sg, total_out)) {
 - vq-vring.desc[i].flags = VRING_DESC_F_NEXT;
 - vq-vring.desc[i].addr = sg_phys(sg);
 - vq-vring.desc[i].len = sg-length;
 + vq-vring.desc[i].flags =
 + cpu_to_virtio16(vq-vq.vdev,
 +   VRING_DESC_F_NEXT);
 + vq-vring.desc[i].addr =
 + cpu_to_virtio64(vq-vq.vdev, sg_phys(sg));
 + vq-vring.desc[i].len =
 + cpu_to_virtio32(vq-vq.vdev, sg-length);
 +
 + /* We chained .next in native: 

Re: [PATCH RFC 00/11] qemu: towards virtio-1 host support

[Jan Kiszka]

On 2014-10-22 10:44, Michael S. Tsirkin wrote:
 On Wed, Oct 08, 2014 at 11:04:28AM +0200, Cornelia Huck wrote:
 On Tue, 07 Oct 2014 18:24:22 -0700
 Andy Lutomirski l...@amacapital.net wrote:

 On 10/07/2014 07:39 AM, Cornelia Huck wrote:
 This patchset aims to get us some way to implement virtio-1 compliant
 and transitional devices in qemu. Branch available at

 git://github.com/cohuck/qemu virtio-1

 I've mainly focused on:
 - endianness handling
 - extended feature bits
 - virtio-ccw new/changed commands

 At the risk of some distraction, would it be worth thinking about a
 solution to the IOMMU bypassing mess as part of this?

 I think that is a whole different issue. virtio-1 is basically done - we
 just need to implement it - while the IOMMU/DMA stuff certainly needs
 more discussion. Therefore, I'd like to defer to the other discussion
 thread here.
 
 I agree, let's do a separate thread for this.
 I also think it's up to the hypervisors at this point.
 People talked about using ACPI to report IOMMU bypass
 to guest.
 If that happens, we don't need a feature bit.

I thought about this again, and I'm not sure anymore if we can use ACPI
to black-list the incompatible virtio devices. Reason: hotplug. To my
understanding, the ACPI DRHD tables won't change during runtime when a
device shows up or disappears. We would have to isolate virtio devices
from the rest of the system by using separate buses for it (and avoid
listing those in any DRHD table) and enforce that they only get plugged
into those buses. I suppose that is not desirable.

Maybe it's better to fix virtio /wrt IOMMUs.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC 04/11] virtio_ring: implement endian reversal based on VERSION_1 feature.

[Cornelia Huck]

On Wed, 22 Oct 2014 17:02:26 +0300
Michael S. Tsirkin m...@redhat.com wrote:

 On Tue, Oct 07, 2014 at 04:39:45PM +0200, Cornelia Huck wrote:
  From: Rusty Russell ru...@rustcorp.com.au
  
  [Cornelia Huck: we don't need the vq-vring.num - vq-ring_mask change]
  Signed-off-by: Rusty Russell ru...@rustcorp.com.au
  Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
  ---
   drivers/virtio/virtio_ring.c |  195 
  ++
   1 file changed, 138 insertions(+), 57 deletions(-)
  
  diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
  index 1cfb5ba..350c39b 100644
  --- a/drivers/virtio/virtio_ring.c
  +++ b/drivers/virtio/virtio_ring.c
  @@ -145,42 +145,54 @@ static inline int vring_add_indirect(struct 
  vring_virtqueue *vq,
  i = 0;
  for (n = 0; n  out_sgs; n++) {
  for (sg = sgs[n]; sg; sg = next(sg, total_out)) {
  -   desc[i].flags = VRING_DESC_F_NEXT;
  -   desc[i].addr = sg_phys(sg);
  -   desc[i].len = sg-length;
  -   desc[i].next = i+1;
  +   desc[i].flags = cpu_to_virtio16(vq-vq.vdev,
  + VRING_DESC_F_NEXT);
  +   desc[i].addr = cpu_to_virtio64(vq-vq.vdev,
  +sg_phys(sg));
  +   desc[i].len = cpu_to_virtio32(vq-vq.vdev,
  +   sg-length);
  +   desc[i].next = cpu_to_virtio16(vq-vq.vdev,
  +i+1);
  i++;
  }
  }
  for (; n  (out_sgs + in_sgs); n++) {
  for (sg = sgs[n]; sg; sg = next(sg, total_in)) {
  -   desc[i].flags = VRING_DESC_F_NEXT|VRING_DESC_F_WRITE;
  -   desc[i].addr = sg_phys(sg);
  -   desc[i].len = sg-length;
  -   desc[i].next = i+1;
  +   desc[i].flags = cpu_to_virtio16(vq-vq.vdev,
  + VRING_DESC_F_NEXT|
  + VRING_DESC_F_WRITE);
  +   desc[i].addr = cpu_to_virtio64(vq-vq.vdev,
  +sg_phys(sg));
  +   desc[i].len = cpu_to_virtio32(vq-vq.vdev,
  +   sg-length);
  +   desc[i].next = cpu_to_virtio16(vq-vq.vdev, i+1);
  i++;
  }
  }
  -   BUG_ON(i != total_sg);
   
  /* Last one doesn't continue. */
  -   desc[i-1].flags = ~VRING_DESC_F_NEXT;
  +   desc[i-1].flags = ~cpu_to_virtio16(vq-vq.vdev, VRING_DESC_F_NEXT);
  desc[i-1].next = 0;
   
  -   /* We're about to use a buffer */
  -   vq-vq.num_free--;
  -
  /* Use a single buffer which doesn't continue */
  head = vq-free_head;
  -   vq-vring.desc[head].flags = VRING_DESC_F_INDIRECT;
  -   vq-vring.desc[head].addr = virt_to_phys(desc);
  +   vq-vring.desc[head].flags =
  +   cpu_to_virtio16(vq-vq.vdev, VRING_DESC_F_INDIRECT);
  +   vq-vring.desc[head].addr =
  +   cpu_to_virtio64(vq-vq.vdev, virt_to_phys(desc));
  /* kmemleak gives a false positive, as it's hidden by virt_to_phys */
  kmemleak_ignore(desc);
  -   vq-vring.desc[head].len = i * sizeof(struct vring_desc);
  +   vq-vring.desc[head].len =
  +   cpu_to_virtio32(vq-vq.vdev, i * sizeof(struct vring_desc));
   
  -   /* Update free pointer */
  +   BUG_ON(i != total_sg);
  +
 
 Why move the BUG_ON here?
 I think I'll move it back ...

IIRC this was in the original patch I applied - but you're right, the
statement can stay in the old place.

 
  +   /* Update free pointer (we store this in native endian) */
  vq-free_head = vq-vring.desc[head].next;
   
  +   /* We've just used a buffer */
  +   vq-vq.num_free--;
  +
  return head;
   }
   

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC 00/11] qemu: towards virtio-1 host support

[Michael S. Tsirkin]

On Wed, Oct 22, 2014 at 04:17:40PM +0200, Jan Kiszka wrote:
 On 2014-10-22 10:44, Michael S. Tsirkin wrote:
  On Wed, Oct 08, 2014 at 11:04:28AM +0200, Cornelia Huck wrote:
  On Tue, 07 Oct 2014 18:24:22 -0700
  Andy Lutomirski l...@amacapital.net wrote:
 
  On 10/07/2014 07:39 AM, Cornelia Huck wrote:
  This patchset aims to get us some way to implement virtio-1 compliant
  and transitional devices in qemu. Branch available at
 
  git://github.com/cohuck/qemu virtio-1
 
  I've mainly focused on:
  - endianness handling
  - extended feature bits
  - virtio-ccw new/changed commands
 
  At the risk of some distraction, would it be worth thinking about a
  solution to the IOMMU bypassing mess as part of this?
 
  I think that is a whole different issue. virtio-1 is basically done - we
  just need to implement it - while the IOMMU/DMA stuff certainly needs
  more discussion. Therefore, I'd like to defer to the other discussion
  thread here.
  
  I agree, let's do a separate thread for this.
  I also think it's up to the hypervisors at this point.
  People talked about using ACPI to report IOMMU bypass
  to guest.
  If that happens, we don't need a feature bit.
 
 I thought about this again, and I'm not sure anymore if we can use ACPI
 to black-list the incompatible virtio devices. Reason: hotplug. To my
 understanding, the ACPI DRHD tables won't change during runtime when a
 device shows up or disappears. We would have to isolate virtio devices
 from the rest of the system by using separate buses for it (and avoid
 listing those in any DRHD table) and enforce that they only get plugged
 into those buses. I suppose that is not desirable.

That's reasonable I think.

 Maybe it's better to fix virtio /wrt IOMMUs.
 
 Jan

Yes but this seems unlikely for 2.2:

The wish to run old guests with iommu remains.
So we'll need to support iommu bypass on the host, and so as
a minimum new guest needs to detect such bypass host and fail.

Unrelated: we also need to teach vhost and dataplane virtio
to get mappings from the iommu.

 -- 
 Siemens AG, Corporate Technology, CT RTC ITP SES-DE
 Corporate Competence Center Embedded Linux
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC 04/11] virtio_ring: implement endian reversal based on VERSION_1 feature.

[Michael S. Tsirkin]

On Wed, Oct 22, 2014 at 04:28:34PM +0200, Cornelia Huck wrote:
 On Wed, 22 Oct 2014 17:02:26 +0300
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Tue, Oct 07, 2014 at 04:39:45PM +0200, Cornelia Huck wrote:
   From: Rusty Russell ru...@rustcorp.com.au
   
   [Cornelia Huck: we don't need the vq-vring.num - vq-ring_mask change]
   Signed-off-by: Rusty Russell ru...@rustcorp.com.au
   Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
   ---
drivers/virtio/virtio_ring.c |  195 
   ++
1 file changed, 138 insertions(+), 57 deletions(-)
   
   diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
   index 1cfb5ba..350c39b 100644
   --- a/drivers/virtio/virtio_ring.c
   +++ b/drivers/virtio/virtio_ring.c
   @@ -145,42 +145,54 @@ static inline int vring_add_indirect(struct 
   vring_virtqueue *vq,
 i = 0;
 for (n = 0; n  out_sgs; n++) {
 for (sg = sgs[n]; sg; sg = next(sg, total_out)) {
   - desc[i].flags = VRING_DESC_F_NEXT;
   - desc[i].addr = sg_phys(sg);
   - desc[i].len = sg-length;
   - desc[i].next = i+1;
   + desc[i].flags = cpu_to_virtio16(vq-vq.vdev,
   +   VRING_DESC_F_NEXT);
   + desc[i].addr = cpu_to_virtio64(vq-vq.vdev,
   +  sg_phys(sg));
   + desc[i].len = cpu_to_virtio32(vq-vq.vdev,
   + sg-length);
   + desc[i].next = cpu_to_virtio16(vq-vq.vdev,
   +  i+1);
 i++;
 }
 }
 for (; n  (out_sgs + in_sgs); n++) {
 for (sg = sgs[n]; sg; sg = next(sg, total_in)) {
   - desc[i].flags = VRING_DESC_F_NEXT|VRING_DESC_F_WRITE;
   - desc[i].addr = sg_phys(sg);
   - desc[i].len = sg-length;
   - desc[i].next = i+1;
   + desc[i].flags = cpu_to_virtio16(vq-vq.vdev,
   +   VRING_DESC_F_NEXT|
   +   VRING_DESC_F_WRITE);
   + desc[i].addr = cpu_to_virtio64(vq-vq.vdev,
   +  sg_phys(sg));
   + desc[i].len = cpu_to_virtio32(vq-vq.vdev,
   + sg-length);
   + desc[i].next = cpu_to_virtio16(vq-vq.vdev, i+1);
 i++;
 }
 }
   - BUG_ON(i != total_sg);

 /* Last one doesn't continue. */
   - desc[i-1].flags = ~VRING_DESC_F_NEXT;
   + desc[i-1].flags = ~cpu_to_virtio16(vq-vq.vdev, VRING_DESC_F_NEXT);
 desc[i-1].next = 0;

   - /* We're about to use a buffer */
   - vq-vq.num_free--;
   -
 /* Use a single buffer which doesn't continue */
 head = vq-free_head;
   - vq-vring.desc[head].flags = VRING_DESC_F_INDIRECT;
   - vq-vring.desc[head].addr = virt_to_phys(desc);
   + vq-vring.desc[head].flags =
   + cpu_to_virtio16(vq-vq.vdev, VRING_DESC_F_INDIRECT);
   + vq-vring.desc[head].addr =
   + cpu_to_virtio64(vq-vq.vdev, virt_to_phys(desc));
 /* kmemleak gives a false positive, as it's hidden by virt_to_phys */
 kmemleak_ignore(desc);
   - vq-vring.desc[head].len = i * sizeof(struct vring_desc);
   + vq-vring.desc[head].len =
   + cpu_to_virtio32(vq-vq.vdev, i * sizeof(struct vring_desc));

   - /* Update free pointer */
   + BUG_ON(i != total_sg);
   +
  
  Why move the BUG_ON here?
  I think I'll move it back ...
 
 IIRC this was in the original patch I applied - but you're right, the
 statement can stay in the old place.

I think I'll do it more gradually: mechanically add accessors
everywhere as a 1st step. Split long lines and other cleanup
as a second step.

  
   + /* Update free pointer (we store this in native endian) */
 vq-free_head = vq-vring.desc[head].next;

   + /* We've just used a buffer */
   + vq-vq.num_free--;
   +
 return head;
}

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: IPv6 UFO for VMs

[Hannes Frederic Sowa]

On Mi, 2014-10-22 at 00:44 +0100, Ben Hutchings wrote:
 There are several ways that VMs can take advantage of UFO and get the
 host to do fragmentation for them:
 
 drivers/net/macvtap.c:  gso_type = SKB_GSO_UDP;
 drivers/net/tun.c:  skb_shinfo(skb)-gso_type = 
 SKB_GSO_UDP;
 drivers/net/virtio_net.c:   skb_shinfo(skb)-gso_type = 
 SKB_GSO_UDP;
 
 Our implementation of UFO for IPv6 does:
 
   fptr = (struct frag_hdr *)(skb_network_header(skb) + 
 unfrag_ip6hlen);
   fptr-nexthdr = nexthdr;
   fptr-reserved = 0;
   fptr-identification = skb_shinfo(skb)-ip6_frag_id;
 
 which assumes ip6_frag_id has been set.  That's only true if the local
 stack constructed the skb; otherwise it appears we get zero.
 
 This seems to be a regression as a result of:
 
 commit 916e4cf46d0204806c062c8c6c4d1f633852c5b6
 Author: Hannes Frederic Sowa han...@stressinduktion.org
 Date:   Fri Feb 21 02:55:35 2014 +0100
 
 ipv6: reuse ip6_frag_id from ip6_ufo_append_data
 
 However, that change seems reasonable - we *shouldn't* be choosing IDs
 for any other stack.  Any paravirt net driver that can use IPv6 UFO
 needs to have some way of passing a fragmentation ID to put in
 skb_shared_info::ip6_frag_id.

Do we really gain a lot of performance by enabling UFO on those devices
or would it make sense to just drop support? It only helps fragmenting
large UDP packets, so I don't think it is worth it.

Otherwise I agree with Ben, we need to pass a fragmentation id from the
host over to the system segmenting the gso frame. Fragmentation ids must
be generated by the end system.

Hmm...

Bye,
Hannes


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v2 02/16] virtio_ring: switch to new memory access APIs

[Michael S. Tsirkin]

Use virtioXX_to_cpu and friends for access to
all multibyte structures in memory.

Note: this is intentionally mechanical.
A follow-up patch will split long lines etc.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/virtio/virtio_ring.c | 89 ++--
 1 file changed, 45 insertions(+), 44 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 3b1f89b..048e1bc 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -99,7 +99,8 @@ struct vring_virtqueue
 
 #define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
 
-static struct vring_desc *alloc_indirect(unsigned int total_sg, gfp_t gfp)
+static struct vring_desc *alloc_indirect(struct virtqueue *_vq,
+unsigned int total_sg, gfp_t gfp)
 {
struct vring_desc *desc;
unsigned int i;
@@ -116,7 +117,7 @@ static struct vring_desc *alloc_indirect(unsigned int 
total_sg, gfp_t gfp)
return NULL;
 
for (i = 0; i  total_sg; i++)
-   desc[i].next = i+1;
+   desc[i].next = cpu_to_virtio16(_vq-vdev, i + 1);
return desc;
 }
 
@@ -165,17 +166,17 @@ static inline int virtqueue_add(struct virtqueue *_vq,
/* If the host supports indirect descriptor tables, and we have multiple
 * buffers, then go indirect. FIXME: tune this threshold */
if (vq-indirect  total_sg  1  vq-vq.num_free)
-   desc = alloc_indirect(total_sg, gfp);
+   desc = alloc_indirect(_vq, total_sg, gfp);
else
desc = NULL;
 
if (desc) {
/* Use a single buffer which doesn't continue */
-   vq-vring.desc[head].flags = VRING_DESC_F_INDIRECT;
-   vq-vring.desc[head].addr = virt_to_phys(desc);
+   vq-vring.desc[head].flags = cpu_to_virtio16(_vq-vdev, 
VRING_DESC_F_INDIRECT);
+   vq-vring.desc[head].addr = cpu_to_virtio64(_vq-vdev, 
virt_to_phys(desc));
/* avoid kmemleak false positive (hidden by virt_to_phys) */
kmemleak_ignore(desc);
-   vq-vring.desc[head].len = total_sg * sizeof(struct vring_desc);
+   vq-vring.desc[head].len = cpu_to_virtio32(_vq-vdev, total_sg 
* sizeof(struct vring_desc));
 
/* Set up rest to use this indirect table. */
i = 0;
@@ -205,28 +206,28 @@ static inline int virtqueue_add(struct virtqueue *_vq,
 
for (n = 0; n  out_sgs; n++) {
for (sg = sgs[n]; sg; sg = sg_next(sg)) {
-   desc[i].flags = VRING_DESC_F_NEXT;
-   desc[i].addr = sg_phys(sg);
-   desc[i].len = sg-length;
+   desc[i].flags = cpu_to_virtio16(_vq-vdev, 
VRING_DESC_F_NEXT);
+   desc[i].addr = cpu_to_virtio64(_vq-vdev, sg_phys(sg));
+   desc[i].len = cpu_to_virtio32(_vq-vdev, sg-length);
prev = i;
-   i = desc[i].next;
+   i = virtio16_to_cpu(_vq-vdev, desc[i].next);
}
}
for (; n  (out_sgs + in_sgs); n++) {
for (sg = sgs[n]; sg; sg = sg_next(sg)) {
-   desc[i].flags = VRING_DESC_F_NEXT|VRING_DESC_F_WRITE;
-   desc[i].addr = sg_phys(sg);
-   desc[i].len = sg-length;
+   desc[i].flags = cpu_to_virtio16(_vq-vdev, 
VRING_DESC_F_NEXT | VRING_DESC_F_WRITE);
+   desc[i].addr = cpu_to_virtio64(_vq-vdev, sg_phys(sg));
+   desc[i].len = cpu_to_virtio32(_vq-vdev, sg-length);
prev = i;
-   i = desc[i].next;
+   i = virtio16_to_cpu(_vq-vdev, desc[i].next);
}
}
/* Last one doesn't continue. */
-   desc[prev].flags = ~VRING_DESC_F_NEXT;
+   desc[prev].flags = cpu_to_virtio16(_vq-vdev, ~VRING_DESC_F_NEXT);
 
/* Update free pointer */
if (indirect)
-   vq-free_head = vq-vring.desc[head].next;
+   vq-free_head = virtio16_to_cpu(_vq-vdev, 
vq-vring.desc[head].next);
else
vq-free_head = i;
 
@@ -235,13 +236,13 @@ static inline int virtqueue_add(struct virtqueue *_vq,
 
/* Put entry in available array (but don't update avail-idx until they
 * do sync). */
-   avail = (vq-vring.avail-idx  (vq-vring.num-1));
-   vq-vring.avail-ring[avail] = head;
+   avail = virtio16_to_cpu(_vq-vdev, vq-vring.avail-idx)  
(vq-vring.num - 1);
+   vq-vring.avail-ring[avail] = cpu_to_virtio16(_vq-vdev, head);
 
/* Descriptors and available array need to be set before we expose the
 * new available array entries. */
virtio_wmb(vq-weak_barriers);
-   vq-vring.avail-idx++;
+   vq-vring.avail-idx = cpu_to_virtio16(_vq-vdev, 

[PATCH RFC v2 07/16] virtio_config: endian conversion for v1.0

[Michael S. Tsirkin]

We (ab)use virtio conversion functions for device-specific
config space accesses.

Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/linux/virtio_config.h | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index be0f6dd..09cd89c 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -255,12 +255,13 @@ static inline u16 virtio_cread16(struct virtio_device 
*vdev,
 {
u16 ret;
vdev-config-get(vdev, offset, ret, sizeof(ret));
-   return ret;
+   return virtio16_to_cpu(vdev, (__force __virtio16)ret);
 }
 
 static inline void virtio_cwrite16(struct virtio_device *vdev,
   unsigned int offset, u16 val)
 {
+   val = (__force u16)cpu_to_virtio16(vdev, val);
vdev-config-set(vdev, offset, val, sizeof(val));
 }
 
@@ -269,12 +270,13 @@ static inline u32 virtio_cread32(struct virtio_device 
*vdev,
 {
u32 ret;
vdev-config-get(vdev, offset, ret, sizeof(ret));
-   return ret;
+   return virtio32_to_cpu(vdev, (__force __virtio32)ret);
 }
 
 static inline void virtio_cwrite32(struct virtio_device *vdev,
   unsigned int offset, u32 val)
 {
+   val = (__force u32)cpu_to_virtio32(vdev, val);
vdev-config-set(vdev, offset, val, sizeof(val));
 }
 
@@ -283,12 +285,13 @@ static inline u64 virtio_cread64(struct virtio_device 
*vdev,
 {
u64 ret;
vdev-config-get(vdev, offset, ret, sizeof(ret));
-   return ret;
+   return virtio64_to_cpu(vdev, (__force __virtio64)ret);
 }
 
 static inline void virtio_cwrite64(struct virtio_device *vdev,
   unsigned int offset, u64 val)
 {
+   val = (__force u64)cpu_to_virtio64(vdev, val);
vdev-config-set(vdev, offset, val, sizeof(val));
 }
 
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v2 06/16] virtio: make endian-ness depend on virtio 1.0

[Michael S. Tsirkin]

virtio 1.0 is LE, virtio without 1.0 is native endian.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/linux/virtio_config.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 7d46280..be0f6dd 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -157,11 +157,11 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
 #define DEFINE_VIRTIO_XX_TO_CPU(bits) \
 static inline u##bits virtio##bits##_to_cpu(struct virtio_device *vdev, 
__virtio##bits val) \
 { \
-   return __virtio##bits##_to_cpu(false, val); \
+   return __virtio##bits##_to_cpu(virtio_has_feature(vdev, 
VIRTIO_F_VERSION_1), val); \
 } \
 static inline __virtio##bits cpu_to_virtio##bits(struct virtio_device *vdev, 
u##bits val) \
 { \
-   return __cpu_to_virtio##bits(false, val); \
+   return __cpu_to_virtio##bits(virtio_has_feature(vdev, 
VIRTIO_F_VERSION_1), val); \
 }
 
 DEFINE_VIRTIO_XX_TO_CPU(16)
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v2 05/16] virtio: add virtio 1.0 feature bit

[Michael S. Tsirkin]

Based on original patches by Rusty Russell, Thomas Huth
and Cornelia Huck.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/uapi/linux/virtio_config.h | 7 +--
 drivers/virtio/virtio_ring.c   | 2 ++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/virtio_config.h 
b/include/uapi/linux/virtio_config.h
index 3ce768c..f3fe33a 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -41,11 +41,11 @@
 /* We've given up on this device. */
 #define VIRTIO_CONFIG_S_FAILED 0x80
 
-/* Some virtio feature bits (currently bits 28 through 31) are reserved for the
+/* Some virtio feature bits (currently bits 28 through 32) are reserved for the
  * transport being used (eg. virtio_ring), the rest are per-device feature
  * bits. */
 #define VIRTIO_TRANSPORT_F_START   28
-#define VIRTIO_TRANSPORT_F_END 32
+#define VIRTIO_TRANSPORT_F_END 33
 
 /* Do we get callbacks when the ring is completely used, even if we've
  * suppressed them? */
@@ -54,4 +54,7 @@
 /* Can the device handle any descriptor layout? */
 #define VIRTIO_F_ANY_LAYOUT27
 
+/* v1.0 compliant. */
+#define VIRTIO_F_VERSION_1 32
+
 #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index b311fa7..9f5dfe3 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -780,6 +780,8 @@ void vring_transport_features(struct virtio_device *vdev)
break;
case VIRTIO_RING_F_EVENT_IDX:
break;
+   case VIRTIO_F_VERSION_1:
+   break;
default:
/* We don't understand this bit. */
vdev-features = ~(1ULL  i);
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v2 10/16] virtio_net: use v1.0 endian.

[Michael S. Tsirkin]

From: Rusty Russell ru...@rustcorp.com.au

[Cornelia Huck: converted some missed fields]
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/net/virtio_net.c | 31 +++
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index d75256bd..2e6561e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -347,13 +347,14 @@ err:
 }
 
 static struct sk_buff *receive_mergeable(struct net_device *dev,
+struct virtnet_info *vi,
 struct receive_queue *rq,
 unsigned long ctx,
 unsigned int len)
 {
void *buf = mergeable_ctx_to_buf_address(ctx);
struct skb_vnet_hdr *hdr = buf;
-   int num_buf = hdr-mhdr.num_buffers;
+   u16 num_buf = virtio16_to_cpu(rq-vq-vdev, hdr-mhdr.num_buffers);
struct page *page = virt_to_head_page(buf);
int offset = buf - page_address(page);
unsigned int truesize = max(len, mergeable_ctx_to_buf_truesize(ctx));
@@ -369,7 +370,9 @@ static struct sk_buff *receive_mergeable(struct net_device 
*dev,
ctx = (unsigned long)virtqueue_get_buf(rq-vq, len);
if (unlikely(!ctx)) {
pr_debug(%s: rx error: %d buffers out of %d missing\n,
-dev-name, num_buf, hdr-mhdr.num_buffers);
+dev-name, num_buf,
+virtio16_to_cpu(rq-vq-vdev,
+hdr-mhdr.num_buffers));
dev-stats.rx_length_errors++;
goto err_buf;
}
@@ -454,7 +457,7 @@ static void receive_buf(struct receive_queue *rq, void 
*buf, unsigned int len)
}
 
if (vi-mergeable_rx_bufs)
-   skb = receive_mergeable(dev, rq, (unsigned long)buf, len);
+   skb = receive_mergeable(dev, vi, rq, (unsigned long)buf, len);
else if (vi-big_packets)
skb = receive_big(dev, rq, buf, len);
else
@@ -473,8 +476,8 @@ static void receive_buf(struct receive_queue *rq, void 
*buf, unsigned int len)
if (hdr-hdr.flags  VIRTIO_NET_HDR_F_NEEDS_CSUM) {
pr_debug(Needs csum!\n);
if (!skb_partial_csum_set(skb,
- hdr-hdr.csum_start,
- hdr-hdr.csum_offset))
+ virtio16_to_cpu(vi-vdev, hdr-hdr.csum_start),
+ virtio16_to_cpu(vi-vdev, hdr-hdr.csum_offset)))
goto frame_err;
} else if (hdr-hdr.flags  VIRTIO_NET_HDR_F_DATA_VALID) {
skb-ip_summed = CHECKSUM_UNNECESSARY;
@@ -505,7 +508,8 @@ static void receive_buf(struct receive_queue *rq, void 
*buf, unsigned int len)
if (hdr-hdr.gso_type  VIRTIO_NET_HDR_GSO_ECN)
skb_shinfo(skb)-gso_type |= SKB_GSO_TCP_ECN;
 
-   skb_shinfo(skb)-gso_size = hdr-hdr.gso_size;
+   skb_shinfo(skb)-gso_size = virtio16_to_cpu(vi-vdev,
+   hdr-hdr.gso_size);
if (skb_shinfo(skb)-gso_size == 0) {
net_warn_ratelimited(%s: zero gso size.\n, dev-name);
goto frame_err;
@@ -867,16 +871,19 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff 
*skb)
 
if (skb-ip_summed == CHECKSUM_PARTIAL) {
hdr-hdr.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
-   hdr-hdr.csum_start = skb_checksum_start_offset(skb);
-   hdr-hdr.csum_offset = skb-csum_offset;
+   hdr-hdr.csum_start = cpu_to_virtio16(vi-vdev,
+   skb_checksum_start_offset(skb));
+   hdr-hdr.csum_offset = cpu_to_virtio16(vi-vdev,
+skb-csum_offset);
} else {
hdr-hdr.flags = 0;
hdr-hdr.csum_offset = hdr-hdr.csum_start = 0;
}
 
if (skb_is_gso(skb)) {
-   hdr-hdr.hdr_len = skb_headlen(skb);
-   hdr-hdr.gso_size = skb_shinfo(skb)-gso_size;
+   hdr-hdr.hdr_len = cpu_to_virtio16(vi-vdev, skb_headlen(skb));
+   hdr-hdr.gso_size = cpu_to_virtio16(vi-vdev,
+   skb_shinfo(skb)-gso_size);
if (skb_shinfo(skb)-gso_type  SKB_GSO_TCPV4)
hdr-hdr.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (skb_shinfo(skb)-gso_type  SKB_GSO_TCPV6)
@@ -1182,7 +1189,7 @@ static void virtnet_set_rx_mode(struct net_device *dev)
sg_init_table(sg, 2);
 
   

[PATCH RFC v2 09/16] virtio: set FEATURES_OK

[Michael S. Tsirkin]

set FEATURES_OK as per virtio 1.0 spec

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/uapi/linux/virtio_config.h |  2 ++
 drivers/virtio/virtio.c| 29 ++---
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/virtio_config.h 
b/include/uapi/linux/virtio_config.h
index f3fe33a..a6d0cde 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -38,6 +38,8 @@
 #define VIRTIO_CONFIG_S_DRIVER 2
 /* Driver has used its parts of the config, and is happy */
 #define VIRTIO_CONFIG_S_DRIVER_OK  4
+/* Driver has finished configuring features */
+#define VIRTIO_CONFIG_S_FEATURES_OK8
 /* We've given up on this device. */
 #define VIRTIO_CONFIG_S_FAILED 0x80
 
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index d213567..a3df817 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -160,6 +160,7 @@ static int virtio_dev_probe(struct device *_d)
struct virtio_device *dev = dev_to_virtio(_d);
struct virtio_driver *drv = drv_to_virtio(dev-dev.driver);
u64 device_features;
+   unsigned status;
 
/* We have a driver! */
add_status(dev, VIRTIO_CONFIG_S_DRIVER);
@@ -183,18 +184,32 @@ static int virtio_dev_probe(struct device *_d)
 
dev-config-finalize_features(dev);
 
+   if (virtio_has_feature(dev, VIRTIO_F_VERSION_1)) {
+   add_status(dev, VIRTIO_CONFIG_S_FEATURES_OK);
+   status = dev-config-get_status(dev);
+   if (!(status  VIRTIO_CONFIG_S_FEATURES_OK)) {
+   printk(KERN_ERR virtio: device refuses features: %x\n,
+  status);
+   err = -ENODEV;
+   goto err;
+   }
+   }
+
err = drv-probe(dev);
if (err)
-   add_status(dev, VIRTIO_CONFIG_S_FAILED);
-   else {
-   add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
-   if (drv-scan)
-   drv-scan(dev);
+   goto err;
 
-   virtio_config_enable(dev);
-   }
+   add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
+   if (drv-scan)
+   drv-scan(dev);
+
+   virtio_config_enable(dev);
 
+   return 0;
+err:
+   add_status(dev, VIRTIO_CONFIG_S_FAILED);
return err;
+
 }
 
 static int virtio_dev_remove(struct device *_d)
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v2 01/16] virtio: memory access APIs

[Michael S. Tsirkin]

virtio 1.0 makes all memory structures LE, so
we need APIs to conditionally do a byteswap on BE
architectures.

To make it easier to check code statically,
add virtio specific types for multi-byte integers
in memory.

Add low level wrappers that do a byteswap conditionally, these will be
useful e.g. for vhost.  Add high level wrappers that will (in the
future) query device endian-ness and act accordingly.

At the moment, stub them out and assume native endian-ness everywhere.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/linux/virtio_config.h| 16 +
 include/uapi/linux/virtio_ring.h | 49 
 include/uapi/linux/Kbuild|  1 +
 3 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 7f4ef66..d38d3c2 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -4,6 +4,7 @@
 #include linux/err.h
 #include linux/bug.h
 #include linux/virtio.h
+#include linux/virtio_byteorder.h
 #include uapi/linux/virtio_config.h
 
 /**
@@ -152,6 +153,21 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
return 0;
 }
 
+/* Memory accessors */
+#define DEFINE_VIRTIO_XX_TO_CPU(bits) \
+static inline u##bits virtio##bits##_to_cpu(struct virtio_device *vdev, 
__virtio##bits val) \
+{ \
+   return __virtio##bits##_to_cpu(false, val); \
+} \
+static inline __virtio##bits cpu_to_virtio##bits(struct virtio_device *vdev, 
u##bits val) \
+{ \
+   return __cpu_to_virtio##bits(false, val); \
+}
+
+DEFINE_VIRTIO_XX_TO_CPU(16)
+DEFINE_VIRTIO_XX_TO_CPU(32)
+DEFINE_VIRTIO_XX_TO_CPU(64)
+
 /* Config space accessors. */
 #define virtio_cread(vdev, structname, member, ptr)\
do {\
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index a99f9b7..6c00632 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -32,6 +32,7 @@
  *
  * Copyright Rusty Russell IBM Corporation 2007. */
 #include linux/types.h
+#include linux/virtio_types.h
 
 /* This marks a buffer as continuing via the next field. */
 #define VRING_DESC_F_NEXT  1
@@ -61,32 +62,32 @@
 /* Virtio ring descriptors: 16 bytes.  These can chain together via next. */
 struct vring_desc {
/* Address (guest-physical). */
-   __u64 addr;
+   __virtio64 addr;
/* Length. */
-   __u32 len;
+   __virtio32 len;
/* The flags as indicated above. */
-   __u16 flags;
+   __virtio16 flags;
/* We chain unused descriptors via this, too */
-   __u16 next;
+   __virtio16 next;
 };
 
 struct vring_avail {
-   __u16 flags;
-   __u16 idx;
-   __u16 ring[];
+   __virtio16 flags;
+   __virtio16 idx;
+   __virtio16 ring[];
 };
 
 /* u32 is used here for ids for padding reasons. */
 struct vring_used_elem {
/* Index of start of used descriptor chain. */
-   __u32 id;
+   __virtio32 id;
/* Total length of the descriptor chain which was used (written to) */
-   __u32 len;
+   __virtio32 len;
 };
 
 struct vring_used {
-   __u16 flags;
-   __u16 idx;
+   __virtio16 flags;
+   __virtio16 idx;
struct vring_used_elem ring[];
 };
 
@@ -109,25 +110,25 @@ struct vring {
  * struct vring_desc desc[num];
  *
  * // A ring of available descriptor heads with free-running index.
- * __u16 avail_flags;
- * __u16 avail_idx;
- * __u16 available[num];
- * __u16 used_event_idx;
+ * __virtio16 avail_flags;
+ * __virtio16 avail_idx;
+ * __virtio16 available[num];
+ * __virtio16 used_event_idx;
  *
  * // Padding to the next align boundary.
  * char pad[];
  *
  * // A ring of used descriptor heads with free-running index.
- * __u16 used_flags;
- * __u16 used_idx;
+ * __virtio16 used_flags;
+ * __virtio16 used_idx;
  * struct vring_used_elem used[num];
- * __u16 avail_event_idx;
+ * __virtio16 avail_event_idx;
  * };
  */
 /* We publish the used event index at the end of the available ring, and vice
  * versa. They are at the end for backwards compatibility. */
 #define vring_used_event(vr) ((vr)-avail-ring[(vr)-num])
-#define vring_avail_event(vr) (*(__u16 *)(vr)-used-ring[(vr)-num])
+#define vring_avail_event(vr) (*(__virtio16 *)(vr)-used-ring[(vr)-num])
 
 static inline void vring_init(struct vring *vr, unsigned int num, void *p,
  unsigned long align)
@@ -135,29 +136,29 @@ static inline void vring_init(struct vring *vr, unsigned 
int num, void *p,
vr-num = num;
vr-desc = p;
vr-avail = p + num*sizeof(struct vring_desc);
-   vr-used = (void *)(((unsigned long)vr-avail-ring[num] + 
sizeof(__u16)
+   vr-used = (void *)(((unsigned long)vr-avail-ring[num] + 
sizeof(__virtio16)
+ align-1)  ~(align - 

[PATCH RFC v2 08/16] virtio: allow transports to get avail/used addresses

[Michael S. Tsirkin]

From: Cornelia Huck cornelia.h...@de.ibm.com

For virtio-1, we can theoretically have a more complex virtqueue
layout with avail and used buffers not on a contiguous memory area
with the descriptor table. For now, it's fine for a transport driver
to stay with the old layout: It needs, however, a way to access
the locations of the avail/used rings so it can register them with
the host.

Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/linux/virtio.h   |  3 +++
 drivers/virtio/virtio_ring.c | 16 
 2 files changed, 19 insertions(+)

diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 149284e..d6359a5 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -75,6 +75,9 @@ unsigned int virtqueue_get_vring_size(struct virtqueue *vq);
 
 bool virtqueue_is_broken(struct virtqueue *vq);
 
+void *virtqueue_get_avail(struct virtqueue *vq);
+void *virtqueue_get_used(struct virtqueue *vq);
+
 /**
  * virtio_device - representation of a device using virtio
  * @index: unique position on the virtio bus
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 9f5dfe3..1db44ba 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -829,4 +829,20 @@ void virtio_break_device(struct virtio_device *dev)
 }
 EXPORT_SYMBOL_GPL(virtio_break_device);
 
+void *virtqueue_get_avail(struct virtqueue *_vq)
+{
+   struct vring_virtqueue *vq = to_vvq(_vq);
+
+   return vq-vring.avail;
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_avail);
+
+void *virtqueue_get_used(struct virtqueue *_vq)
+{
+   struct vring_virtqueue *vq = to_vvq(_vq);
+
+   return vq-vring.used;
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_used);
+
 MODULE_LICENSE(GPL);
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v2 04/16] virtio: add support for 64 bit features.

[Michael S. Tsirkin]

From: Rusty Russell ru...@rustcorp.com.au

Change the u32 to a u64, and make sure to use 1ULL everywhere!

Cc: Brian Swetland swetl...@google.com
Cc: Christian Borntraeger borntrae...@de.ibm.com
[Thomas Huth: fix up virtio-ccw get_features]
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Acked-by: Pawel Moll pawel.m...@arm.com
Acked-by: Ohad Ben-Cohen o...@wizery.com

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/linux/virtio.h |  2 +-
 include/linux/virtio_config.h  |  8 
 tools/virtio/linux/virtio.h|  2 +-
 tools/virtio/linux/virtio_config.h |  2 +-
 drivers/char/virtio_console.c  |  2 +-
 drivers/lguest/lguest_device.c | 10 +-
 drivers/remoteproc/remoteproc_virtio.c |  5 -
 drivers/s390/kvm/kvm_virtio.c  | 10 +-
 drivers/s390/kvm/virtio_ccw.c  | 29 -
 drivers/virtio/virtio.c| 12 ++--
 drivers/virtio/virtio_mmio.c   | 14 +-
 drivers/virtio/virtio_pci.c|  5 ++---
 drivers/virtio/virtio_ring.c   |  2 +-
 13 files changed, 64 insertions(+), 39 deletions(-)

diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 7828a7f..149284e 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -101,7 +101,7 @@ struct virtio_device {
const struct virtio_config_ops *config;
const struct vringh_config_ops *vringh_config;
struct list_head vqs;
-   u32 features;
+   u64 features;
void *priv;
 };
 
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 946bed7..7d46280 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -67,7 +67,7 @@ struct virtio_config_ops {
vq_callback_t *callbacks[],
const char *names[]);
void (*del_vqs)(struct virtio_device *);
-   u32 (*get_features)(struct virtio_device *vdev);
+   u64 (*get_features)(struct virtio_device *vdev);
void (*finalize_features)(struct virtio_device *vdev);
const char *(*bus_name)(struct virtio_device *vdev);
int (*set_vq_affinity)(struct virtqueue *vq, int cpu);
@@ -87,14 +87,14 @@ static inline bool virtio_has_feature(const struct 
virtio_device *vdev,
 {
/* Did you forget to fix assumptions on max features? */
if (__builtin_constant_p(fbit))
-   BUILD_BUG_ON(fbit = 32);
+   BUILD_BUG_ON(fbit = 64);
else
-   BUG_ON(fbit = 32);
+   BUG_ON(fbit = 64);
 
if (fbit  VIRTIO_TRANSPORT_F_START)
virtio_check_driver_offered_feature(vdev, fbit);
 
-   return vdev-features  (1  fbit);
+   return vdev-features  (1ULL  fbit);
 }
 
 static inline
diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h
index 72bff70..8eb6421 100644
--- a/tools/virtio/linux/virtio.h
+++ b/tools/virtio/linux/virtio.h
@@ -10,7 +10,7 @@
 
 struct virtio_device {
void *dev;
-   u32 features;
+   u64 features;
 };
 
 struct virtqueue {
diff --git a/tools/virtio/linux/virtio_config.h 
b/tools/virtio/linux/virtio_config.h
index 1f1636b..a254c2b 100644
--- a/tools/virtio/linux/virtio_config.h
+++ b/tools/virtio/linux/virtio_config.h
@@ -2,5 +2,5 @@
 #define VIRTIO_TRANSPORT_F_END 32
 
 #define virtio_has_feature(dev, feature) \
-   ((dev)-features  (1  feature))
+   ((dev)-features  (1ULL  feature))
 
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index d79baf0..1690b2a 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -355,7 +355,7 @@ static inline bool use_multiport(struct ports_device 
*portdev)
 */
if (!portdev-vdev)
return 0;
-   return portdev-vdev-features  (1  VIRTIO_CONSOLE_F_MULTIPORT);
+   return portdev-vdev-features  (1ULL  VIRTIO_CONSOLE_F_MULTIPORT);
 }
 
 static DEFINE_SPINLOCK(dma_bufs_lock);
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index c831c47..4d29bcd 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -94,17 +94,17 @@ static unsigned desc_size(const struct lguest_device_desc 
*desc)
 }
 
 /* This gets the device's feature bits. */
-static u32 lg_get_features(struct virtio_device *vdev)
+static u64 lg_get_features(struct virtio_device *vdev)
 {
unsigned int i;
-   u32 features = 0;
+   u64 features = 0;
struct lguest_device_desc *desc = to_lgdev(vdev)-desc;
u8 *in_features = lg_features(desc);
 
/* We do this the slow but generic way. */
-   for (i = 0; i  min(desc-feature_len * 8, 32); i++)
+   for (i = 0; i  min(desc-feature_len * 8, 64); i++)
if (in_features[i / 8]  (1  (i % 8)))
-   features |= (1  i);
+   

[PATCH RFC v2 03/16] virtio: use u32, not bitmap for struct virtio_device's features

[Michael S. Tsirkin]

From: Rusty Russell ru...@rustcorp.com.au

It seemed like a good idea, but it's actually a pain when we get more
than 32 feature bits.  Just change it to a u32 for now.

Cc: Brian Swetland swetl...@google.com
Cc: Christian Borntraeger borntrae...@de.ibm.com
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Acked-by: Pawel Moll pawel.m...@arm.com
Acked-by: Ohad Ben-Cohen o...@wizery.com

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/linux/virtio.h |  3 +--
 include/linux/virtio_config.h  |  2 +-
 tools/virtio/linux/virtio.h| 22 +-
 tools/virtio/linux/virtio_config.h |  2 +-
 drivers/char/virtio_console.c  |  2 +-
 drivers/lguest/lguest_device.c |  8 
 drivers/remoteproc/remoteproc_virtio.c |  2 +-
 drivers/s390/kvm/kvm_virtio.c  |  2 +-
 drivers/s390/kvm/virtio_ccw.c  | 23 +--
 drivers/virtio/virtio.c| 10 +-
 drivers/virtio/virtio_mmio.c   |  8 ++--
 drivers/virtio/virtio_pci.c|  3 +--
 drivers/virtio/virtio_ring.c   |  2 +-
 tools/virtio/virtio_test.c |  5 ++---
 tools/virtio/vringh_test.c | 16 
 15 files changed, 39 insertions(+), 71 deletions(-)

diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 65261a7..7828a7f 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -101,8 +101,7 @@ struct virtio_device {
const struct virtio_config_ops *config;
const struct vringh_config_ops *vringh_config;
struct list_head vqs;
-   /* Note that this is a Linux set_bit-style bitmap. */
-   unsigned long features[1];
+   u32 features;
void *priv;
 };
 
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index d38d3c2..946bed7 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -94,7 +94,7 @@ static inline bool virtio_has_feature(const struct 
virtio_device *vdev,
if (fbit  VIRTIO_TRANSPORT_F_START)
virtio_check_driver_offered_feature(vdev, fbit);
 
-   return test_bit(fbit, vdev-features);
+   return vdev-features  (1  fbit);
 }
 
 static inline
diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h
index 5a2d1f0..72bff70 100644
--- a/tools/virtio/linux/virtio.h
+++ b/tools/virtio/linux/virtio.h
@@ -6,31 +6,11 @@
 /* TODO: empty stubs for now. Broken but enough for virtio_ring.c */
 #define list_add_tail(a, b) do {} while (0)
 #define list_del(a) do {} while (0)
-
-#define BIT_WORD(nr)   ((nr) / BITS_PER_LONG)
-#define BITS_PER_BYTE  8
-#define BITS_PER_LONG (sizeof(long) * BITS_PER_BYTE)
-#define BIT_MASK(nr)   (1UL  ((nr) % BITS_PER_LONG))
-
-/* TODO: Not atomic as it should be:
- * we don't use this for anything important. */
-static inline void clear_bit(int nr, volatile unsigned long *addr)
-{
-   unsigned long mask = BIT_MASK(nr);
-   unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
-
-   *p = ~mask;
-}
-
-static inline int test_bit(int nr, const volatile unsigned long *addr)
-{
-return 1UL  (addr[BIT_WORD(nr)]  (nr  (BITS_PER_LONG-1)));
-}
 /* end of stubs */
 
 struct virtio_device {
void *dev;
-   unsigned long features[1];
+   u32 features;
 };
 
 struct virtqueue {
diff --git a/tools/virtio/linux/virtio_config.h 
b/tools/virtio/linux/virtio_config.h
index 5049967..1f1636b 100644
--- a/tools/virtio/linux/virtio_config.h
+++ b/tools/virtio/linux/virtio_config.h
@@ -2,5 +2,5 @@
 #define VIRTIO_TRANSPORT_F_END 32
 
 #define virtio_has_feature(dev, feature) \
-   test_bit((feature), (dev)-features)
+   ((dev)-features  (1  feature))
 
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index bfa6400..d79baf0 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -355,7 +355,7 @@ static inline bool use_multiport(struct ports_device 
*portdev)
 */
if (!portdev-vdev)
return 0;
-   return portdev-vdev-features[0]  (1  VIRTIO_CONSOLE_F_MULTIPORT);
+   return portdev-vdev-features  (1  VIRTIO_CONSOLE_F_MULTIPORT);
 }
 
 static DEFINE_SPINLOCK(dma_bufs_lock);
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index d0a1d8a..c831c47 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -137,14 +137,14 @@ static void lg_finalize_features(struct virtio_device 
*vdev)
vring_transport_features(vdev);
 
/*
-* The vdev-feature array is a Linux bitmask: this isn't the same as a
-* the simple array of bits used by lguest devices for features.  So we
-* do this slow, manual conversion which is completely general.
+* Since lguest is currently x86-only, we're little-endian.  That
+* means we could 

[PATCH RFC v2 11/16] virtio_blk: use virtio v1.0 endian

[Michael S. Tsirkin]

From: Cornelia Huck cornelia.h...@de.ibm.com

Note that we care only about the fields still in use for virtio v1.0.

Reviewed-by: Thomas Huth th...@linux.vnet.ibm.com
Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/block/virtio_blk.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index c6a27d5..327e601 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -113,6 +113,10 @@ static int __virtblk_add_req(struct virtqueue *vq,
sg_init_one(status, vbr-status, sizeof(vbr-status));
sgs[num_out + num_in++] = status;
 
+   /* we only care about fields valid for virtio-1 */
+   vbr-out_hdr.type = cpu_to_virtio32(vq-vdev, vbr-out_hdr.type);
+   vbr-out_hdr.sector = cpu_to_virtio64(vq-vdev, vbr-out_hdr.sector);
+
return virtqueue_add_sgs(vq, sgs, num_out, num_in, vbr, GFP_ATOMIC);
 }
 
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v2 15/16] virtio_net: fix types for in memory structures

[Michael S. Tsirkin]

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/uapi/linux/virtio_net.h | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 172a7f0..b5f1677 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -28,6 +28,7 @@
 #include linux/types.h
 #include linux/virtio_ids.h
 #include linux/virtio_config.h
+#include linux/virtio_types.h
 #include linux/if_ether.h
 
 /* The feature bitmap for virtio net */
@@ -84,17 +85,17 @@ struct virtio_net_hdr {
 #define VIRTIO_NET_HDR_GSO_TCPV6   4   // GSO frame, IPv6 TCP
 #define VIRTIO_NET_HDR_GSO_ECN 0x80// TCP has ECN set
__u8 gso_type;
-   __u16 hdr_len;  /* Ethernet + IP + tcp/udp hdrs */
-   __u16 gso_size; /* Bytes to append to hdr_len per frame */
-   __u16 csum_start;   /* Position to start checksumming from */
-   __u16 csum_offset;  /* Offset after that to place checksum */
+   __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
+   __virtio16 gso_size;/* Bytes to append to hdr_len per frame 
*/
+   __virtio16 csum_start;  /* Position to start checksumming from */
+   __virtio16 csum_offset; /* Offset after that to place checksum */
 };
 
 /* This is the version of the header to use when the MRG_RXBUF
  * feature has been negotiated. */
 struct virtio_net_hdr_mrg_rxbuf {
struct virtio_net_hdr hdr;
-   __u16 num_buffers;  /* Number of merged rx buffers */
+   __virtio16 num_buffers; /* Number of merged rx buffers */
 };
 
 /*
@@ -149,7 +150,7 @@ typedef __u8 virtio_net_ctrl_ack;
  * VIRTIO_NET_F_CTRL_MAC_ADDR feature is available.
  */
 struct virtio_net_ctrl_mac {
-   __u32 entries;
+   __virtio32 entries;
__u8 macs[][ETH_ALEN];
 } __attribute__((packed));
 
@@ -193,7 +194,7 @@ struct virtio_net_ctrl_mac {
  * specified.
  */
 struct virtio_net_ctrl_mq {
-   __u16 virtqueue_pairs;
+   __virtio16 virtqueue_pairs;
 };
 
 #define VIRTIO_NET_CTRL_MQ   4
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v2 16/16] virtio_blk: fix types for in memory structures

[Michael S. Tsirkin]

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/uapi/linux/virtio_blk.h | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
index 9ad67b2..247c8ba 100644
--- a/include/uapi/linux/virtio_blk.h
+++ b/include/uapi/linux/virtio_blk.h
@@ -28,6 +28,7 @@
 #include linux/types.h
 #include linux/virtio_ids.h
 #include linux/virtio_config.h
+#include linux/virtio_types.h
 
 /* Feature bits */
 #define VIRTIO_BLK_F_BARRIER   0   /* Does host support barriers? */
@@ -114,18 +115,18 @@ struct virtio_blk_config {
 /* This is the first element of the read scatter-gather list. */
 struct virtio_blk_outhdr {
/* VIRTIO_BLK_T* */
-   __u32 type;
+   __virtio32 type;
/* io priority. */
-   __u32 ioprio;
+   __virtio32 ioprio;
/* Sector (ie. 512 byte offset) */
-   __u64 sector;
+   __virtio64 sector;
 };
 
 struct virtio_scsi_inhdr {
-   __u32 errors;
-   __u32 data_len;
-   __u32 sense_len;
-   __u32 residual;
+   __virtio32 errors;
+   __virtio32 data_len;
+   __virtio32 sense_len;
+   __virtio32 residual;
 };
 
 /* And this is the final byte of the write scatter-gather list. */
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC v2 01/16] virtio: memory access APIs

[Christopher Covington]

Hi Michael,

On 10/22/2014 11:50 AM, Michael S. Tsirkin wrote:
 virtio 1.0 makes all memory structures LE, so
 we need APIs to conditionally do a byteswap on BE
 architectures.
 
 To make it easier to check code statically,
 add virtio specific types for multi-byte integers
 in memory.
 
 Add low level wrappers that do a byteswap conditionally, these will be
 useful e.g. for vhost.  Add high level wrappers that will (in the
 future) query device endian-ness and act accordingly.
 
 At the moment, stub them out and assume native endian-ness everywhere.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  include/linux/virtio_config.h| 16 +
  include/uapi/linux/virtio_ring.h | 49 
 
  include/uapi/linux/Kbuild|  1 +
  3 files changed, 42 insertions(+), 24 deletions(-)
 
 diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
 index 7f4ef66..d38d3c2 100644
 --- a/include/linux/virtio_config.h
 +++ b/include/linux/virtio_config.h
 @@ -4,6 +4,7 @@
  #include linux/err.h
  #include linux/bug.h
  #include linux/virtio.h
 +#include linux/virtio_byteorder.h

What patch creates this file?

  #include uapi/linux/virtio_config.h
  
  /**
 @@ -152,6 +153,21 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
   return 0;
  }
  
 +/* Memory accessors */
 +#define DEFINE_VIRTIO_XX_TO_CPU(bits) \
 +static inline u##bits virtio##bits##_to_cpu(struct virtio_device *vdev, 
 __virtio##bits val) \
 +{ \
 + return __virtio##bits##_to_cpu(false, val); \
 +} \
 +static inline __virtio##bits cpu_to_virtio##bits(struct virtio_device *vdev, 
 u##bits val) \
 +{ \
 + return __cpu_to_virtio##bits(false, val); \
 +}
 +
 +DEFINE_VIRTIO_XX_TO_CPU(16)
 +DEFINE_VIRTIO_XX_TO_CPU(32)
 +DEFINE_VIRTIO_XX_TO_CPU(64)
 +
  /* Config space accessors. */
  #define virtio_cread(vdev, structname, member, ptr)  \
   do {\
 diff --git a/include/uapi/linux/virtio_ring.h 
 b/include/uapi/linux/virtio_ring.h
 index a99f9b7..6c00632 100644
 --- a/include/uapi/linux/virtio_ring.h
 +++ b/include/uapi/linux/virtio_ring.h
 @@ -32,6 +32,7 @@
   *
   * Copyright Rusty Russell IBM Corporation 2007. */
  #include linux/types.h
 +#include linux/virtio_types.h
  
  /* This marks a buffer as continuing via the next field. */
  #define VRING_DESC_F_NEXT1
 @@ -61,32 +62,32 @@
  /* Virtio ring descriptors: 16 bytes.  These can chain together via next. 
 */
  struct vring_desc {
   /* Address (guest-physical). */
 - __u64 addr;
 + __virtio64 addr;
   /* Length. */
 - __u32 len;
 + __virtio32 len;
   /* The flags as indicated above. */
 - __u16 flags;
 + __virtio16 flags;
   /* We chain unused descriptors via this, too */
 - __u16 next;
 + __virtio16 next;
  };

How does __virtio64 differ from __le64?

Thanks,
Chris

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC v2 01/16] virtio: memory access APIs

[Michael S. Tsirkin]

On Wed, Oct 22, 2014 at 01:45:55PM -0400, Christopher Covington wrote:
 Hi Michael,
 
 On 10/22/2014 11:50 AM, Michael S. Tsirkin wrote:
  virtio 1.0 makes all memory structures LE, so
  we need APIs to conditionally do a byteswap on BE
  architectures.
  
  To make it easier to check code statically,
  add virtio specific types for multi-byte integers
  in memory.
  
  Add low level wrappers that do a byteswap conditionally, these will be
  useful e.g. for vhost.  Add high level wrappers that will (in the
  future) query device endian-ness and act accordingly.
  
  At the moment, stub them out and assume native endian-ness everywhere.
  
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
  ---
   include/linux/virtio_config.h| 16 +
   include/uapi/linux/virtio_ring.h | 49 
  
   include/uapi/linux/Kbuild|  1 +
   3 files changed, 42 insertions(+), 24 deletions(-)
  
  diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
  index 7f4ef66..d38d3c2 100644
  --- a/include/linux/virtio_config.h
  +++ b/include/linux/virtio_config.h
  @@ -4,6 +4,7 @@
   #include linux/err.h
   #include linux/bug.h
   #include linux/virtio.h
  +#include linux/virtio_byteorder.h
 
 What patch creates this file?

Oops I forgot to git add it.
Will resend.

   #include uapi/linux/virtio_config.h
   
   /**
  @@ -152,6 +153,21 @@ int virtqueue_set_affinity(struct virtqueue *vq, int 
  cpu)
  return 0;
   }
   
  +/* Memory accessors */
  +#define DEFINE_VIRTIO_XX_TO_CPU(bits) \
  +static inline u##bits virtio##bits##_to_cpu(struct virtio_device *vdev, 
  __virtio##bits val) \
  +{ \
  +   return __virtio##bits##_to_cpu(false, val); \
  +} \
  +static inline __virtio##bits cpu_to_virtio##bits(struct virtio_device 
  *vdev, u##bits val) \
  +{ \
  +   return __cpu_to_virtio##bits(false, val); \
  +}
  +
  +DEFINE_VIRTIO_XX_TO_CPU(16)
  +DEFINE_VIRTIO_XX_TO_CPU(32)
  +DEFINE_VIRTIO_XX_TO_CPU(64)
  +
   /* Config space accessors. */
   #define virtio_cread(vdev, structname, member, ptr)
  \
  do {\
  diff --git a/include/uapi/linux/virtio_ring.h 
  b/include/uapi/linux/virtio_ring.h
  index a99f9b7..6c00632 100644
  --- a/include/uapi/linux/virtio_ring.h
  +++ b/include/uapi/linux/virtio_ring.h
  @@ -32,6 +32,7 @@
*
* Copyright Rusty Russell IBM Corporation 2007. */
   #include linux/types.h
  +#include linux/virtio_types.h
   
   /* This marks a buffer as continuing via the next field. */
   #define VRING_DESC_F_NEXT  1
  @@ -61,32 +62,32 @@
   /* Virtio ring descriptors: 16 bytes.  These can chain together via 
  next. */
   struct vring_desc {
  /* Address (guest-physical). */
  -   __u64 addr;
  +   __virtio64 addr;
  /* Length. */
  -   __u32 len;
  +   __virtio32 len;
  /* The flags as indicated above. */
  -   __u16 flags;
  +   __virtio16 flags;
  /* We chain unused descriptors via this, too */
  -   __u16 next;
  +   __virtio16 next;
   };
 
 How does __virtio64 differ from __le64?
 
 Thanks,
 Chris

__le64 would require cpu_to_le, I wanted to force the use
of our byte swapping macros.

 -- 
 Qualcomm Innovation Center, Inc.
 The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
 a Linux Foundation Collaborative Project
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v3 02/16] virtio_ring: switch to new memory access APIs

[Michael S. Tsirkin]

Use virtioXX_to_cpu and friends for access to
all multibyte structures in memory.

Note: this is intentionally mechanical.
A follow-up patch will split long lines etc.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/virtio/virtio_ring.c | 89 ++--
 1 file changed, 45 insertions(+), 44 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 3b1f89b..048e1bc 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -99,7 +99,8 @@ struct vring_virtqueue
 
 #define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
 
-static struct vring_desc *alloc_indirect(unsigned int total_sg, gfp_t gfp)
+static struct vring_desc *alloc_indirect(struct virtqueue *_vq,
+unsigned int total_sg, gfp_t gfp)
 {
struct vring_desc *desc;
unsigned int i;
@@ -116,7 +117,7 @@ static struct vring_desc *alloc_indirect(unsigned int 
total_sg, gfp_t gfp)
return NULL;
 
for (i = 0; i  total_sg; i++)
-   desc[i].next = i+1;
+   desc[i].next = cpu_to_virtio16(_vq-vdev, i + 1);
return desc;
 }
 
@@ -165,17 +166,17 @@ static inline int virtqueue_add(struct virtqueue *_vq,
/* If the host supports indirect descriptor tables, and we have multiple
 * buffers, then go indirect. FIXME: tune this threshold */
if (vq-indirect  total_sg  1  vq-vq.num_free)
-   desc = alloc_indirect(total_sg, gfp);
+   desc = alloc_indirect(_vq, total_sg, gfp);
else
desc = NULL;
 
if (desc) {
/* Use a single buffer which doesn't continue */
-   vq-vring.desc[head].flags = VRING_DESC_F_INDIRECT;
-   vq-vring.desc[head].addr = virt_to_phys(desc);
+   vq-vring.desc[head].flags = cpu_to_virtio16(_vq-vdev, 
VRING_DESC_F_INDIRECT);
+   vq-vring.desc[head].addr = cpu_to_virtio64(_vq-vdev, 
virt_to_phys(desc));
/* avoid kmemleak false positive (hidden by virt_to_phys) */
kmemleak_ignore(desc);
-   vq-vring.desc[head].len = total_sg * sizeof(struct vring_desc);
+   vq-vring.desc[head].len = cpu_to_virtio32(_vq-vdev, total_sg 
* sizeof(struct vring_desc));
 
/* Set up rest to use this indirect table. */
i = 0;
@@ -205,28 +206,28 @@ static inline int virtqueue_add(struct virtqueue *_vq,
 
for (n = 0; n  out_sgs; n++) {
for (sg = sgs[n]; sg; sg = sg_next(sg)) {
-   desc[i].flags = VRING_DESC_F_NEXT;
-   desc[i].addr = sg_phys(sg);
-   desc[i].len = sg-length;
+   desc[i].flags = cpu_to_virtio16(_vq-vdev, 
VRING_DESC_F_NEXT);
+   desc[i].addr = cpu_to_virtio64(_vq-vdev, sg_phys(sg));
+   desc[i].len = cpu_to_virtio32(_vq-vdev, sg-length);
prev = i;
-   i = desc[i].next;
+   i = virtio16_to_cpu(_vq-vdev, desc[i].next);
}
}
for (; n  (out_sgs + in_sgs); n++) {
for (sg = sgs[n]; sg; sg = sg_next(sg)) {
-   desc[i].flags = VRING_DESC_F_NEXT|VRING_DESC_F_WRITE;
-   desc[i].addr = sg_phys(sg);
-   desc[i].len = sg-length;
+   desc[i].flags = cpu_to_virtio16(_vq-vdev, 
VRING_DESC_F_NEXT | VRING_DESC_F_WRITE);
+   desc[i].addr = cpu_to_virtio64(_vq-vdev, sg_phys(sg));
+   desc[i].len = cpu_to_virtio32(_vq-vdev, sg-length);
prev = i;
-   i = desc[i].next;
+   i = virtio16_to_cpu(_vq-vdev, desc[i].next);
}
}
/* Last one doesn't continue. */
-   desc[prev].flags = ~VRING_DESC_F_NEXT;
+   desc[prev].flags = cpu_to_virtio16(_vq-vdev, ~VRING_DESC_F_NEXT);
 
/* Update free pointer */
if (indirect)
-   vq-free_head = vq-vring.desc[head].next;
+   vq-free_head = virtio16_to_cpu(_vq-vdev, 
vq-vring.desc[head].next);
else
vq-free_head = i;
 
@@ -235,13 +236,13 @@ static inline int virtqueue_add(struct virtqueue *_vq,
 
/* Put entry in available array (but don't update avail-idx until they
 * do sync). */
-   avail = (vq-vring.avail-idx  (vq-vring.num-1));
-   vq-vring.avail-ring[avail] = head;
+   avail = virtio16_to_cpu(_vq-vdev, vq-vring.avail-idx)  
(vq-vring.num - 1);
+   vq-vring.avail-ring[avail] = cpu_to_virtio16(_vq-vdev, head);
 
/* Descriptors and available array need to be set before we expose the
 * new available array entries. */
virtio_wmb(vq-weak_barriers);
-   vq-vring.avail-idx++;
+   vq-vring.avail-idx = cpu_to_virtio16(_vq-vdev, 

[PATCH RFC v3 05/16] virtio: add virtio 1.0 feature bit

[Michael S. Tsirkin]

Based on original patches by Rusty Russell, Thomas Huth
and Cornelia Huck.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/uapi/linux/virtio_config.h | 7 +--
 drivers/virtio/virtio_ring.c   | 2 ++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/virtio_config.h 
b/include/uapi/linux/virtio_config.h
index 3ce768c..f3fe33a 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -41,11 +41,11 @@
 /* We've given up on this device. */
 #define VIRTIO_CONFIG_S_FAILED 0x80
 
-/* Some virtio feature bits (currently bits 28 through 31) are reserved for the
+/* Some virtio feature bits (currently bits 28 through 32) are reserved for the
  * transport being used (eg. virtio_ring), the rest are per-device feature
  * bits. */
 #define VIRTIO_TRANSPORT_F_START   28
-#define VIRTIO_TRANSPORT_F_END 32
+#define VIRTIO_TRANSPORT_F_END 33
 
 /* Do we get callbacks when the ring is completely used, even if we've
  * suppressed them? */
@@ -54,4 +54,7 @@
 /* Can the device handle any descriptor layout? */
 #define VIRTIO_F_ANY_LAYOUT27
 
+/* v1.0 compliant. */
+#define VIRTIO_F_VERSION_1 32
+
 #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index b311fa7..9f5dfe3 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -780,6 +780,8 @@ void vring_transport_features(struct virtio_device *vdev)
break;
case VIRTIO_RING_F_EVENT_IDX:
break;
+   case VIRTIO_F_VERSION_1:
+   break;
default:
/* We don't understand this bit. */
vdev-features = ~(1ULL  i);
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v3 06/16] virtio: make endian-ness depend on virtio 1.0

[Michael S. Tsirkin]

virtio 1.0 is LE, virtio without 1.0 is native endian.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/linux/virtio_config.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 7d46280..be0f6dd 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -157,11 +157,11 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
 #define DEFINE_VIRTIO_XX_TO_CPU(bits) \
 static inline u##bits virtio##bits##_to_cpu(struct virtio_device *vdev, 
__virtio##bits val) \
 { \
-   return __virtio##bits##_to_cpu(false, val); \
+   return __virtio##bits##_to_cpu(virtio_has_feature(vdev, 
VIRTIO_F_VERSION_1), val); \
 } \
 static inline __virtio##bits cpu_to_virtio##bits(struct virtio_device *vdev, 
u##bits val) \
 { \
-   return __cpu_to_virtio##bits(false, val); \
+   return __cpu_to_virtio##bits(virtio_has_feature(vdev, 
VIRTIO_F_VERSION_1), val); \
 }
 
 DEFINE_VIRTIO_XX_TO_CPU(16)
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v3 08/16] virtio: allow transports to get avail/used addresses

[Michael S. Tsirkin]

From: Cornelia Huck cornelia.h...@de.ibm.com

For virtio-1, we can theoretically have a more complex virtqueue
layout with avail and used buffers not on a contiguous memory area
with the descriptor table. For now, it's fine for a transport driver
to stay with the old layout: It needs, however, a way to access
the locations of the avail/used rings so it can register them with
the host.

Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/linux/virtio.h   |  3 +++
 drivers/virtio/virtio_ring.c | 16 
 2 files changed, 19 insertions(+)

diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 149284e..d6359a5 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -75,6 +75,9 @@ unsigned int virtqueue_get_vring_size(struct virtqueue *vq);
 
 bool virtqueue_is_broken(struct virtqueue *vq);
 
+void *virtqueue_get_avail(struct virtqueue *vq);
+void *virtqueue_get_used(struct virtqueue *vq);
+
 /**
  * virtio_device - representation of a device using virtio
  * @index: unique position on the virtio bus
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 9f5dfe3..1db44ba 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -829,4 +829,20 @@ void virtio_break_device(struct virtio_device *dev)
 }
 EXPORT_SYMBOL_GPL(virtio_break_device);
 
+void *virtqueue_get_avail(struct virtqueue *_vq)
+{
+   struct vring_virtqueue *vq = to_vvq(_vq);
+
+   return vq-vring.avail;
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_avail);
+
+void *virtqueue_get_used(struct virtqueue *_vq)
+{
+   struct vring_virtqueue *vq = to_vvq(_vq);
+
+   return vq-vring.used;
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_used);
+
 MODULE_LICENSE(GPL);
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v3 09/16] virtio: set FEATURES_OK

[Michael S. Tsirkin]

set FEATURES_OK as per virtio 1.0 spec

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/uapi/linux/virtio_config.h |  2 ++
 drivers/virtio/virtio.c| 29 ++---
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/virtio_config.h 
b/include/uapi/linux/virtio_config.h
index f3fe33a..a6d0cde 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -38,6 +38,8 @@
 #define VIRTIO_CONFIG_S_DRIVER 2
 /* Driver has used its parts of the config, and is happy */
 #define VIRTIO_CONFIG_S_DRIVER_OK  4
+/* Driver has finished configuring features */
+#define VIRTIO_CONFIG_S_FEATURES_OK8
 /* We've given up on this device. */
 #define VIRTIO_CONFIG_S_FAILED 0x80
 
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index d213567..a3df817 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -160,6 +160,7 @@ static int virtio_dev_probe(struct device *_d)
struct virtio_device *dev = dev_to_virtio(_d);
struct virtio_driver *drv = drv_to_virtio(dev-dev.driver);
u64 device_features;
+   unsigned status;
 
/* We have a driver! */
add_status(dev, VIRTIO_CONFIG_S_DRIVER);
@@ -183,18 +184,32 @@ static int virtio_dev_probe(struct device *_d)
 
dev-config-finalize_features(dev);
 
+   if (virtio_has_feature(dev, VIRTIO_F_VERSION_1)) {
+   add_status(dev, VIRTIO_CONFIG_S_FEATURES_OK);
+   status = dev-config-get_status(dev);
+   if (!(status  VIRTIO_CONFIG_S_FEATURES_OK)) {
+   printk(KERN_ERR virtio: device refuses features: %x\n,
+  status);
+   err = -ENODEV;
+   goto err;
+   }
+   }
+
err = drv-probe(dev);
if (err)
-   add_status(dev, VIRTIO_CONFIG_S_FAILED);
-   else {
-   add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
-   if (drv-scan)
-   drv-scan(dev);
+   goto err;
 
-   virtio_config_enable(dev);
-   }
+   add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
+   if (drv-scan)
+   drv-scan(dev);
+
+   virtio_config_enable(dev);
 
+   return 0;
+err:
+   add_status(dev, VIRTIO_CONFIG_S_FAILED);
return err;
+
 }
 
 static int virtio_dev_remove(struct device *_d)
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v3 01/16] virtio: memory access APIs

[Michael S. Tsirkin]

virtio 1.0 makes all memory structures LE, so
we need APIs to conditionally do a byteswap on BE
architectures.

To make it easier to check code statically,
add virtio specific types for multi-byte integers
in memory.

Add low level wrappers that do a byteswap conditionally, these will be
useful e.g. for vhost.  Add high level wrappers that will (in the
future) query device endian-ness and act accordingly.

At the moment, stub them out and assume native endian-ness everywhere.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/linux/virtio_byteorder.h | 29 
 include/linux/virtio_config.h| 16 +
 include/uapi/linux/virtio_ring.h | 49 
 include/uapi/linux/Kbuild|  1 +
 4 files changed, 71 insertions(+), 24 deletions(-)
 create mode 100644 include/linux/virtio_byteorder.h

diff --git a/include/linux/virtio_byteorder.h b/include/linux/virtio_byteorder.h
new file mode 100644
index 000..7afdd8a
--- /dev/null
+++ b/include/linux/virtio_byteorder.h
@@ -0,0 +1,29 @@
+#ifndef _LINUX_VIRTIO_BYTEORDER_H
+#define _LINUX_VIRTIO_BYTEORDER_H
+#include linux/types.h
+#include uapi/linux/virtio_types.h
+
+/* Memory accessors for handling virtio in modern little endian and in
+ * compatibility big endian format. */
+
+#define __DEFINE_VIRTIO_XX_TO_CPU(bits) \
+static inline u##bits __virtio##bits##_to_cpu(bool little_endian, 
__virtio##bits val) \
+{ \
+   if (little_endian) \
+   return le##bits##_to_cpu((__force __le##bits)val); \
+   else \
+   return (__force u##bits)val; \
+} \
+static inline __virtio##bits __cpu_to_virtio##bits(bool little_endian, u##bits 
val) \
+{ \
+   if (little_endian) \
+   return (__force __virtio##bits)cpu_to_le##bits(val); \
+   else \
+   return val; \
+}
+
+__DEFINE_VIRTIO_XX_TO_CPU(16)
+__DEFINE_VIRTIO_XX_TO_CPU(32)
+__DEFINE_VIRTIO_XX_TO_CPU(64)
+
+#endif /* _LINUX_VIRTIO_BYTEORDER */
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 7f4ef66..d38d3c2 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -4,6 +4,7 @@
 #include linux/err.h
 #include linux/bug.h
 #include linux/virtio.h
+#include linux/virtio_byteorder.h
 #include uapi/linux/virtio_config.h
 
 /**
@@ -152,6 +153,21 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
return 0;
 }
 
+/* Memory accessors */
+#define DEFINE_VIRTIO_XX_TO_CPU(bits) \
+static inline u##bits virtio##bits##_to_cpu(struct virtio_device *vdev, 
__virtio##bits val) \
+{ \
+   return __virtio##bits##_to_cpu(false, val); \
+} \
+static inline __virtio##bits cpu_to_virtio##bits(struct virtio_device *vdev, 
u##bits val) \
+{ \
+   return __cpu_to_virtio##bits(false, val); \
+}
+
+DEFINE_VIRTIO_XX_TO_CPU(16)
+DEFINE_VIRTIO_XX_TO_CPU(32)
+DEFINE_VIRTIO_XX_TO_CPU(64)
+
 /* Config space accessors. */
 #define virtio_cread(vdev, structname, member, ptr)\
do {\
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index a99f9b7..6c00632 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -32,6 +32,7 @@
  *
  * Copyright Rusty Russell IBM Corporation 2007. */
 #include linux/types.h
+#include linux/virtio_types.h
 
 /* This marks a buffer as continuing via the next field. */
 #define VRING_DESC_F_NEXT  1
@@ -61,32 +62,32 @@
 /* Virtio ring descriptors: 16 bytes.  These can chain together via next. */
 struct vring_desc {
/* Address (guest-physical). */
-   __u64 addr;
+   __virtio64 addr;
/* Length. */
-   __u32 len;
+   __virtio32 len;
/* The flags as indicated above. */
-   __u16 flags;
+   __virtio16 flags;
/* We chain unused descriptors via this, too */
-   __u16 next;
+   __virtio16 next;
 };
 
 struct vring_avail {
-   __u16 flags;
-   __u16 idx;
-   __u16 ring[];
+   __virtio16 flags;
+   __virtio16 idx;
+   __virtio16 ring[];
 };
 
 /* u32 is used here for ids for padding reasons. */
 struct vring_used_elem {
/* Index of start of used descriptor chain. */
-   __u32 id;
+   __virtio32 id;
/* Total length of the descriptor chain which was used (written to) */
-   __u32 len;
+   __virtio32 len;
 };
 
 struct vring_used {
-   __u16 flags;
-   __u16 idx;
+   __virtio16 flags;
+   __virtio16 idx;
struct vring_used_elem ring[];
 };
 
@@ -109,25 +110,25 @@ struct vring {
  * struct vring_desc desc[num];
  *
  * // A ring of available descriptor heads with free-running index.
- * __u16 avail_flags;
- * __u16 avail_idx;
- * __u16 available[num];
- * __u16 used_event_idx;
+ * __virtio16 avail_flags;
+ * __virtio16 avail_idx;
+ * __virtio16 available[num];
+ * __virtio16 used_event_idx;
  

[PATCH RFC v3 07/16] virtio_config: endian conversion for v1.0

[Michael S. Tsirkin]

We (ab)use virtio conversion functions for device-specific
config space accesses.

Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/linux/virtio_config.h | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index be0f6dd..09cd89c 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -255,12 +255,13 @@ static inline u16 virtio_cread16(struct virtio_device 
*vdev,
 {
u16 ret;
vdev-config-get(vdev, offset, ret, sizeof(ret));
-   return ret;
+   return virtio16_to_cpu(vdev, (__force __virtio16)ret);
 }
 
 static inline void virtio_cwrite16(struct virtio_device *vdev,
   unsigned int offset, u16 val)
 {
+   val = (__force u16)cpu_to_virtio16(vdev, val);
vdev-config-set(vdev, offset, val, sizeof(val));
 }
 
@@ -269,12 +270,13 @@ static inline u32 virtio_cread32(struct virtio_device 
*vdev,
 {
u32 ret;
vdev-config-get(vdev, offset, ret, sizeof(ret));
-   return ret;
+   return virtio32_to_cpu(vdev, (__force __virtio32)ret);
 }
 
 static inline void virtio_cwrite32(struct virtio_device *vdev,
   unsigned int offset, u32 val)
 {
+   val = (__force u32)cpu_to_virtio32(vdev, val);
vdev-config-set(vdev, offset, val, sizeof(val));
 }
 
@@ -283,12 +285,13 @@ static inline u64 virtio_cread64(struct virtio_device 
*vdev,
 {
u64 ret;
vdev-config-get(vdev, offset, ret, sizeof(ret));
-   return ret;
+   return virtio64_to_cpu(vdev, (__force __virtio64)ret);
 }
 
 static inline void virtio_cwrite64(struct virtio_device *vdev,
   unsigned int offset, u64 val)
 {
+   val = (__force u64)cpu_to_virtio64(vdev, val);
vdev-config-set(vdev, offset, val, sizeof(val));
 }
 
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v3 10/16] virtio_net: use v1.0 endian.

[Michael S. Tsirkin]

From: Rusty Russell ru...@rustcorp.com.au

[Cornelia Huck: converted some missed fields]
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/net/virtio_net.c | 31 +++
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index d75256bd..2e6561e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -347,13 +347,14 @@ err:
 }
 
 static struct sk_buff *receive_mergeable(struct net_device *dev,
+struct virtnet_info *vi,
 struct receive_queue *rq,
 unsigned long ctx,
 unsigned int len)
 {
void *buf = mergeable_ctx_to_buf_address(ctx);
struct skb_vnet_hdr *hdr = buf;
-   int num_buf = hdr-mhdr.num_buffers;
+   u16 num_buf = virtio16_to_cpu(rq-vq-vdev, hdr-mhdr.num_buffers);
struct page *page = virt_to_head_page(buf);
int offset = buf - page_address(page);
unsigned int truesize = max(len, mergeable_ctx_to_buf_truesize(ctx));
@@ -369,7 +370,9 @@ static struct sk_buff *receive_mergeable(struct net_device 
*dev,
ctx = (unsigned long)virtqueue_get_buf(rq-vq, len);
if (unlikely(!ctx)) {
pr_debug(%s: rx error: %d buffers out of %d missing\n,
-dev-name, num_buf, hdr-mhdr.num_buffers);
+dev-name, num_buf,
+virtio16_to_cpu(rq-vq-vdev,
+hdr-mhdr.num_buffers));
dev-stats.rx_length_errors++;
goto err_buf;
}
@@ -454,7 +457,7 @@ static void receive_buf(struct receive_queue *rq, void 
*buf, unsigned int len)
}
 
if (vi-mergeable_rx_bufs)
-   skb = receive_mergeable(dev, rq, (unsigned long)buf, len);
+   skb = receive_mergeable(dev, vi, rq, (unsigned long)buf, len);
else if (vi-big_packets)
skb = receive_big(dev, rq, buf, len);
else
@@ -473,8 +476,8 @@ static void receive_buf(struct receive_queue *rq, void 
*buf, unsigned int len)
if (hdr-hdr.flags  VIRTIO_NET_HDR_F_NEEDS_CSUM) {
pr_debug(Needs csum!\n);
if (!skb_partial_csum_set(skb,
- hdr-hdr.csum_start,
- hdr-hdr.csum_offset))
+ virtio16_to_cpu(vi-vdev, hdr-hdr.csum_start),
+ virtio16_to_cpu(vi-vdev, hdr-hdr.csum_offset)))
goto frame_err;
} else if (hdr-hdr.flags  VIRTIO_NET_HDR_F_DATA_VALID) {
skb-ip_summed = CHECKSUM_UNNECESSARY;
@@ -505,7 +508,8 @@ static void receive_buf(struct receive_queue *rq, void 
*buf, unsigned int len)
if (hdr-hdr.gso_type  VIRTIO_NET_HDR_GSO_ECN)
skb_shinfo(skb)-gso_type |= SKB_GSO_TCP_ECN;
 
-   skb_shinfo(skb)-gso_size = hdr-hdr.gso_size;
+   skb_shinfo(skb)-gso_size = virtio16_to_cpu(vi-vdev,
+   hdr-hdr.gso_size);
if (skb_shinfo(skb)-gso_size == 0) {
net_warn_ratelimited(%s: zero gso size.\n, dev-name);
goto frame_err;
@@ -867,16 +871,19 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff 
*skb)
 
if (skb-ip_summed == CHECKSUM_PARTIAL) {
hdr-hdr.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
-   hdr-hdr.csum_start = skb_checksum_start_offset(skb);
-   hdr-hdr.csum_offset = skb-csum_offset;
+   hdr-hdr.csum_start = cpu_to_virtio16(vi-vdev,
+   skb_checksum_start_offset(skb));
+   hdr-hdr.csum_offset = cpu_to_virtio16(vi-vdev,
+skb-csum_offset);
} else {
hdr-hdr.flags = 0;
hdr-hdr.csum_offset = hdr-hdr.csum_start = 0;
}
 
if (skb_is_gso(skb)) {
-   hdr-hdr.hdr_len = skb_headlen(skb);
-   hdr-hdr.gso_size = skb_shinfo(skb)-gso_size;
+   hdr-hdr.hdr_len = cpu_to_virtio16(vi-vdev, skb_headlen(skb));
+   hdr-hdr.gso_size = cpu_to_virtio16(vi-vdev,
+   skb_shinfo(skb)-gso_size);
if (skb_shinfo(skb)-gso_type  SKB_GSO_TCPV4)
hdr-hdr.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (skb_shinfo(skb)-gso_type  SKB_GSO_TCPV6)
@@ -1182,7 +1189,7 @@ static void virtnet_set_rx_mode(struct net_device *dev)
sg_init_table(sg, 2);
 
   

[PATCH RFC v3 15/16] virtio_net: fix types for in memory structures

[Michael S. Tsirkin]

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/uapi/linux/virtio_net.h | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 172a7f0..b5f1677 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -28,6 +28,7 @@
 #include linux/types.h
 #include linux/virtio_ids.h
 #include linux/virtio_config.h
+#include linux/virtio_types.h
 #include linux/if_ether.h
 
 /* The feature bitmap for virtio net */
@@ -84,17 +85,17 @@ struct virtio_net_hdr {
 #define VIRTIO_NET_HDR_GSO_TCPV6   4   // GSO frame, IPv6 TCP
 #define VIRTIO_NET_HDR_GSO_ECN 0x80// TCP has ECN set
__u8 gso_type;
-   __u16 hdr_len;  /* Ethernet + IP + tcp/udp hdrs */
-   __u16 gso_size; /* Bytes to append to hdr_len per frame */
-   __u16 csum_start;   /* Position to start checksumming from */
-   __u16 csum_offset;  /* Offset after that to place checksum */
+   __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
+   __virtio16 gso_size;/* Bytes to append to hdr_len per frame 
*/
+   __virtio16 csum_start;  /* Position to start checksumming from */
+   __virtio16 csum_offset; /* Offset after that to place checksum */
 };
 
 /* This is the version of the header to use when the MRG_RXBUF
  * feature has been negotiated. */
 struct virtio_net_hdr_mrg_rxbuf {
struct virtio_net_hdr hdr;
-   __u16 num_buffers;  /* Number of merged rx buffers */
+   __virtio16 num_buffers; /* Number of merged rx buffers */
 };
 
 /*
@@ -149,7 +150,7 @@ typedef __u8 virtio_net_ctrl_ack;
  * VIRTIO_NET_F_CTRL_MAC_ADDR feature is available.
  */
 struct virtio_net_ctrl_mac {
-   __u32 entries;
+   __virtio32 entries;
__u8 macs[][ETH_ALEN];
 } __attribute__((packed));
 
@@ -193,7 +194,7 @@ struct virtio_net_ctrl_mac {
  * specified.
  */
 struct virtio_net_ctrl_mq {
-   __u16 virtqueue_pairs;
+   __virtio16 virtqueue_pairs;
 };
 
 #define VIRTIO_NET_CTRL_MQ   4
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v3 11/16] virtio_blk: use virtio v1.0 endian

[Michael S. Tsirkin]

From: Cornelia Huck cornelia.h...@de.ibm.com

Note that we care only about the fields still in use for virtio v1.0.

Reviewed-by: Thomas Huth th...@linux.vnet.ibm.com
Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/block/virtio_blk.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index c6a27d5..327e601 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -113,6 +113,10 @@ static int __virtblk_add_req(struct virtqueue *vq,
sg_init_one(status, vbr-status, sizeof(vbr-status));
sgs[num_out + num_in++] = status;
 
+   /* we only care about fields valid for virtio-1 */
+   vbr-out_hdr.type = cpu_to_virtio32(vq-vdev, vbr-out_hdr.type);
+   vbr-out_hdr.sector = cpu_to_virtio64(vq-vdev, vbr-out_hdr.sector);
+
return virtqueue_add_sgs(vq, sgs, num_out, num_in, vbr, GFP_ATOMIC);
 }
 
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v3 16/16] virtio_blk: fix types for in memory structures

[Michael S. Tsirkin]

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/uapi/linux/virtio_blk.h | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
index 9ad67b2..247c8ba 100644
--- a/include/uapi/linux/virtio_blk.h
+++ b/include/uapi/linux/virtio_blk.h
@@ -28,6 +28,7 @@
 #include linux/types.h
 #include linux/virtio_ids.h
 #include linux/virtio_config.h
+#include linux/virtio_types.h
 
 /* Feature bits */
 #define VIRTIO_BLK_F_BARRIER   0   /* Does host support barriers? */
@@ -114,18 +115,18 @@ struct virtio_blk_config {
 /* This is the first element of the read scatter-gather list. */
 struct virtio_blk_outhdr {
/* VIRTIO_BLK_T* */
-   __u32 type;
+   __virtio32 type;
/* io priority. */
-   __u32 ioprio;
+   __virtio32 ioprio;
/* Sector (ie. 512 byte offset) */
-   __u64 sector;
+   __virtio64 sector;
 };
 
 struct virtio_scsi_inhdr {
-   __u32 errors;
-   __u32 data_len;
-   __u32 sense_len;
-   __u32 residual;
+   __virtio32 errors;
+   __virtio32 data_len;
+   __virtio32 sense_len;
+   __virtio32 residual;
 };
 
 /* And this is the final byte of the write scatter-gather list. */
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH RFC v3 04/16] virtio: add support for 64 bit features.

[Michael S. Tsirkin]

From: Rusty Russell ru...@rustcorp.com.au

Change the u32 to a u64, and make sure to use 1ULL everywhere!

Cc: Brian Swetland swetl...@google.com
Cc: Christian Borntraeger borntrae...@de.ibm.com
[Thomas Huth: fix up virtio-ccw get_features]
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Acked-by: Pawel Moll pawel.m...@arm.com
Acked-by: Ohad Ben-Cohen o...@wizery.com

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/linux/virtio.h |  2 +-
 include/linux/virtio_config.h  |  8 
 tools/virtio/linux/virtio.h|  2 +-
 tools/virtio/linux/virtio_config.h |  2 +-
 drivers/char/virtio_console.c  |  2 +-
 drivers/lguest/lguest_device.c | 10 +-
 drivers/remoteproc/remoteproc_virtio.c |  5 -
 drivers/s390/kvm/kvm_virtio.c  | 10 +-
 drivers/s390/kvm/virtio_ccw.c  | 29 -
 drivers/virtio/virtio.c| 12 ++--
 drivers/virtio/virtio_mmio.c   | 14 +-
 drivers/virtio/virtio_pci.c|  5 ++---
 drivers/virtio/virtio_ring.c   |  2 +-
 13 files changed, 64 insertions(+), 39 deletions(-)

diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 7828a7f..149284e 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -101,7 +101,7 @@ struct virtio_device {
const struct virtio_config_ops *config;
const struct vringh_config_ops *vringh_config;
struct list_head vqs;
-   u32 features;
+   u64 features;
void *priv;
 };
 
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 946bed7..7d46280 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -67,7 +67,7 @@ struct virtio_config_ops {
vq_callback_t *callbacks[],
const char *names[]);
void (*del_vqs)(struct virtio_device *);
-   u32 (*get_features)(struct virtio_device *vdev);
+   u64 (*get_features)(struct virtio_device *vdev);
void (*finalize_features)(struct virtio_device *vdev);
const char *(*bus_name)(struct virtio_device *vdev);
int (*set_vq_affinity)(struct virtqueue *vq, int cpu);
@@ -87,14 +87,14 @@ static inline bool virtio_has_feature(const struct 
virtio_device *vdev,
 {
/* Did you forget to fix assumptions on max features? */
if (__builtin_constant_p(fbit))
-   BUILD_BUG_ON(fbit = 32);
+   BUILD_BUG_ON(fbit = 64);
else
-   BUG_ON(fbit = 32);
+   BUG_ON(fbit = 64);
 
if (fbit  VIRTIO_TRANSPORT_F_START)
virtio_check_driver_offered_feature(vdev, fbit);
 
-   return vdev-features  (1  fbit);
+   return vdev-features  (1ULL  fbit);
 }
 
 static inline
diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h
index 72bff70..8eb6421 100644
--- a/tools/virtio/linux/virtio.h
+++ b/tools/virtio/linux/virtio.h
@@ -10,7 +10,7 @@
 
 struct virtio_device {
void *dev;
-   u32 features;
+   u64 features;
 };
 
 struct virtqueue {
diff --git a/tools/virtio/linux/virtio_config.h 
b/tools/virtio/linux/virtio_config.h
index 1f1636b..a254c2b 100644
--- a/tools/virtio/linux/virtio_config.h
+++ b/tools/virtio/linux/virtio_config.h
@@ -2,5 +2,5 @@
 #define VIRTIO_TRANSPORT_F_END 32
 
 #define virtio_has_feature(dev, feature) \
-   ((dev)-features  (1  feature))
+   ((dev)-features  (1ULL  feature))
 
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index d79baf0..1690b2a 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -355,7 +355,7 @@ static inline bool use_multiport(struct ports_device 
*portdev)
 */
if (!portdev-vdev)
return 0;
-   return portdev-vdev-features  (1  VIRTIO_CONSOLE_F_MULTIPORT);
+   return portdev-vdev-features  (1ULL  VIRTIO_CONSOLE_F_MULTIPORT);
 }
 
 static DEFINE_SPINLOCK(dma_bufs_lock);
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index c831c47..4d29bcd 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -94,17 +94,17 @@ static unsigned desc_size(const struct lguest_device_desc 
*desc)
 }
 
 /* This gets the device's feature bits. */
-static u32 lg_get_features(struct virtio_device *vdev)
+static u64 lg_get_features(struct virtio_device *vdev)
 {
unsigned int i;
-   u32 features = 0;
+   u64 features = 0;
struct lguest_device_desc *desc = to_lgdev(vdev)-desc;
u8 *in_features = lg_features(desc);
 
/* We do this the slow but generic way. */
-   for (i = 0; i  min(desc-feature_len * 8, 32); i++)
+   for (i = 0; i  min(desc-feature_len * 8, 64); i++)
if (in_features[i / 8]  (1  (i % 8)))
-   features |= (1  i);
+   

[PATCH RFC v3 03/16] virtio: use u32, not bitmap for struct virtio_device's features

[Michael S. Tsirkin]

From: Rusty Russell ru...@rustcorp.com.au

It seemed like a good idea, but it's actually a pain when we get more
than 32 feature bits.  Just change it to a u32 for now.

Cc: Brian Swetland swetl...@google.com
Cc: Christian Borntraeger borntrae...@de.ibm.com
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Acked-by: Pawel Moll pawel.m...@arm.com
Acked-by: Ohad Ben-Cohen o...@wizery.com

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 include/linux/virtio.h |  3 +--
 include/linux/virtio_config.h  |  2 +-
 tools/virtio/linux/virtio.h| 22 +-
 tools/virtio/linux/virtio_config.h |  2 +-
 drivers/char/virtio_console.c  |  2 +-
 drivers/lguest/lguest_device.c |  8 
 drivers/remoteproc/remoteproc_virtio.c |  2 +-
 drivers/s390/kvm/kvm_virtio.c  |  2 +-
 drivers/s390/kvm/virtio_ccw.c  | 23 +--
 drivers/virtio/virtio.c| 10 +-
 drivers/virtio/virtio_mmio.c   |  8 ++--
 drivers/virtio/virtio_pci.c|  3 +--
 drivers/virtio/virtio_ring.c   |  2 +-
 tools/virtio/virtio_test.c |  5 ++---
 tools/virtio/vringh_test.c | 16 
 15 files changed, 39 insertions(+), 71 deletions(-)

diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 65261a7..7828a7f 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -101,8 +101,7 @@ struct virtio_device {
const struct virtio_config_ops *config;
const struct vringh_config_ops *vringh_config;
struct list_head vqs;
-   /* Note that this is a Linux set_bit-style bitmap. */
-   unsigned long features[1];
+   u32 features;
void *priv;
 };
 
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index d38d3c2..946bed7 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -94,7 +94,7 @@ static inline bool virtio_has_feature(const struct 
virtio_device *vdev,
if (fbit  VIRTIO_TRANSPORT_F_START)
virtio_check_driver_offered_feature(vdev, fbit);
 
-   return test_bit(fbit, vdev-features);
+   return vdev-features  (1  fbit);
 }
 
 static inline
diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h
index 5a2d1f0..72bff70 100644
--- a/tools/virtio/linux/virtio.h
+++ b/tools/virtio/linux/virtio.h
@@ -6,31 +6,11 @@
 /* TODO: empty stubs for now. Broken but enough for virtio_ring.c */
 #define list_add_tail(a, b) do {} while (0)
 #define list_del(a) do {} while (0)
-
-#define BIT_WORD(nr)   ((nr) / BITS_PER_LONG)
-#define BITS_PER_BYTE  8
-#define BITS_PER_LONG (sizeof(long) * BITS_PER_BYTE)
-#define BIT_MASK(nr)   (1UL  ((nr) % BITS_PER_LONG))
-
-/* TODO: Not atomic as it should be:
- * we don't use this for anything important. */
-static inline void clear_bit(int nr, volatile unsigned long *addr)
-{
-   unsigned long mask = BIT_MASK(nr);
-   unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
-
-   *p = ~mask;
-}
-
-static inline int test_bit(int nr, const volatile unsigned long *addr)
-{
-return 1UL  (addr[BIT_WORD(nr)]  (nr  (BITS_PER_LONG-1)));
-}
 /* end of stubs */
 
 struct virtio_device {
void *dev;
-   unsigned long features[1];
+   u32 features;
 };
 
 struct virtqueue {
diff --git a/tools/virtio/linux/virtio_config.h 
b/tools/virtio/linux/virtio_config.h
index 5049967..1f1636b 100644
--- a/tools/virtio/linux/virtio_config.h
+++ b/tools/virtio/linux/virtio_config.h
@@ -2,5 +2,5 @@
 #define VIRTIO_TRANSPORT_F_END 32
 
 #define virtio_has_feature(dev, feature) \
-   test_bit((feature), (dev)-features)
+   ((dev)-features  (1  feature))
 
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index bfa6400..d79baf0 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -355,7 +355,7 @@ static inline bool use_multiport(struct ports_device 
*portdev)
 */
if (!portdev-vdev)
return 0;
-   return portdev-vdev-features[0]  (1  VIRTIO_CONSOLE_F_MULTIPORT);
+   return portdev-vdev-features  (1  VIRTIO_CONSOLE_F_MULTIPORT);
 }
 
 static DEFINE_SPINLOCK(dma_bufs_lock);
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index d0a1d8a..c831c47 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -137,14 +137,14 @@ static void lg_finalize_features(struct virtio_device 
*vdev)
vring_transport_features(vdev);
 
/*
-* The vdev-feature array is a Linux bitmask: this isn't the same as a
-* the simple array of bits used by lguest devices for features.  So we
-* do this slow, manual conversion which is completely general.
+* Since lguest is currently x86-only, we're little-endian.  That
+* means we could 

Re: [PATCH RFC 00/11] qemu: towards virtio-1 host support

[Benjamin Herrenschmidt]

On Wed, 2014-10-22 at 16:17 +0200, Jan Kiszka wrote:
 I thought about this again, and I'm not sure anymore if we can use
 ACPI
 to black-list the incompatible virtio devices. Reason: hotplug. To
 my
 understanding, the ACPI DRHD tables won't change during runtime when a
 device shows up or disappears. We would have to isolate virtio devices
 from the rest of the system by using separate buses for it (and avoid
 listing those in any DRHD table) and enforce that they only get
 plugged
 into those buses. I suppose that is not desirable.
 
 Maybe it's better to fix virtio /wrt IOMMUs.

I always go back to my initial proposal which is to define that current
virtio always bypass any iommu (which is what it does really) and have
it expose via a new capability if that isn't the case. That means fixing
that Xen thingy to allow qemu to know what to expose I assume but that
seems to be the less bad approach.

Cheers,
Ben.


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization