Re: [PATCH 1/6] virtio_host: host-side implementation of virtio rings.

2013-01-22 Thread Rusty Russell
Asias He  writes:

> On 01/17/2013 06:29 PM, Rusty Russell wrote:
>> Getting use of virtio rings correct is tricky, and a recent patch saw
>> an implementation of in-kernel rings (as separate from userspace).
>> 
>> This patch attempts to abstract the business of dealing with the
>> virtio ring layout from the access (userspace or direct); to do this,
>> we use function pointers, which gcc inlines correctly.
>> 
>> Signed-off-by: Rusty Russell 
>> ---
>>  drivers/Makefile |2 +-
>>  drivers/vhost/Kconfig|8 +
>>  drivers/vhost/Kconfig.tcm|1 +
>>  drivers/vhost/Makefile   |2 +
>>  drivers/vhost/vringh.c   |  818 
>> ++
>>  drivers/virtio/virtio_ring.c |   33 +-
>>  include/linux/virtio_ring.h  |   57 +++
>
>
> Why vringh_notify_enable_user() and vringh_notify_disable_user() are not
> declared in include/linux/virtio_ring.h? Missed that?

Yes, I did... I've fixed it in my vringh branch, where I'm doing
development from now on.

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


Re: [PATCH 4/6] tools/virtio: add vring_test.

2013-01-22 Thread Asias He
On 01/23/2013 07:03 AM, Rusty Russell wrote:
> Asias He  writes:
> 
>> On 01/17/2013 06:29 PM, Rusty Russell wrote:
>>> This is mainly to test the drivers/vhost/vringh.c code, but it also
>>> uses the drivers/virtio/virtio_ring.c code for the guest side.
>>
>> vringh_test.c does not compile here:
>> (This series on top of 9a9284153d965a57edc7162a8e57c14c97f3a935)
>>
>> $ cd tools/virtio
>> $ make
>> cc -g -O2 -Wall -I. -I ../../usr/include/ -Wno-pointer-sign
>> -fno-strict-overflow  -MMDvringh_test.c   -o vringh_test
>> In file included from ./linux/vringh.h:1:0,
>>  from ./../../drivers/vhost/vringh.c:6,
>>  from vringh_test.c:7:
>> ./linux/../../../include/linux/vringh.h:27:28: fatal error:
>> uapi/linux/uio.h: No such file or directory
> 
> Oops, I forgot to add the file... it's a one-liner:
> 
> tools/virtio/uapi/linux/uio.h:
> #include 
> 
> I'll make a new branch for this, called vringh.  It'll probably rebase
> as I neaten things up, but I'll try not to go crazy...

It compiles now.

FYI, I got some warnings:

../../drivers/virtio/virtio_ring.c: In function ‘virtqueue_kick_prepare’:
../../drivers/virtio/virtio_ring.c:309:3: warning: dereferencing
type-punned pointer will break strict-aliasing rules [-Wstrict-aliasin
g]
cc   virtio_test.o virtio_ring.o   -o virtio_test
cc -g -O2 -Wall -I. -I ../../usr/include/ -Wno-pointer-sign
-fno-strict-overflow  -MMDvringh_test.c   -o vringh_test
In file included from vringh_test.c:7:0:
./../../drivers/vhost/vringh.c: In function ‘check_range’:
./../../drivers/vhost/vringh.c:119:2: warning: format ‘%llx’ expects
argument of type ‘long long unsigned int’, but argument 3 has type
 ‘u64’ [-Wformat]
./../../drivers/vhost/vringh.c: In function ‘__vringh_notify_enable’:
./../../drivers/vhost/vringh.c:402:3: warning: dereferencing type-punned
pointer will break strict-aliasing rules [-Wstrict-aliasing]
./../../drivers/vhost/vringh.c:405:8: warning: dereferencing type-punned
pointer will break strict-aliasing rules [-Wstrict-aliasing]
./../../drivers/vhost/vringh.c: In function ‘vringh_init_user’:
./../../drivers/vhost/vringh.c:499:3: warning: format ‘%zu’ expects
argument of type ‘size_t’, but argument 2 has type ‘unsigned int’ [
-Wformat]
./../../drivers/vhost/vringh.c: In function ‘vringh_init_kern’:
./../../drivers/vhost/vringh.c:707:3: warning: format ‘%zu’ expects
argument of type ‘size_t’, but argument 2 has type ‘unsigned int’ [
-Wformat]
In file included from vringh_test.c:8:0:
./../../drivers/virtio/virtio_ring.c: In function ‘virtqueue_kick_prepare’:
./../../drivers/virtio/virtio_ring.c:309:3: warning: dereferencing
type-punned pointer will break strict-aliasing rules [-Wstrict-alias
ing]

...


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

Re: [PATCH 4/6] tools/virtio: add vring_test.

2013-01-22 Thread Rusty Russell
Asias He  writes:

> On 01/17/2013 06:29 PM, Rusty Russell wrote:
>> This is mainly to test the drivers/vhost/vringh.c code, but it also
>> uses the drivers/virtio/virtio_ring.c code for the guest side.
>
> vringh_test.c does not compile here:
> (This series on top of 9a9284153d965a57edc7162a8e57c14c97f3a935)
>
> $ cd tools/virtio
> $ make
> cc -g -O2 -Wall -I. -I ../../usr/include/ -Wno-pointer-sign
> -fno-strict-overflow  -MMDvringh_test.c   -o vringh_test
> In file included from ./linux/vringh.h:1:0,
>  from ./../../drivers/vhost/vringh.c:6,
>  from vringh_test.c:7:
> ./linux/../../../include/linux/vringh.h:27:28: fatal error:
> uapi/linux/uio.h: No such file or directory

Oops, I forgot to add the file... it's a one-liner:

tools/virtio/uapi/linux/uio.h:
#include 

I'll make a new branch for this, called vringh.  It'll probably rebase
as I neaten things up, but I'll try not to go crazy...

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


Re: [PATCH] x86, Allow x2apic without IR on VMware platform.

2013-01-22 Thread Alok Kataria
Hi Peter, Ingo,

Can you please consider this patch, it allows linux guests to use x2apic
when running on VMware platform. 

Thanks,
Alok

On Thu, 2013-01-17 at 15:44 -0800, Alok Kataria wrote:
> Please consider this patch to allow x2apic without IR support when
> running on VMware platform. Tested on top of 3.8-rc3.
> 
> Thanks,
> Alok
> 
> --
> Allow x2apic without IR on VMware platform.
> 
> From: Alok N Kataria 
> 
> This patch updates x2apic initializaition code to allow x2apic on VMware
> platform even without interrupt remapping support.
> The hypervisor_x2apic_available hook was added in x2apic initialization code
> and used by KVM and XEN, before this.
> I have also cleaned up that code to export this hook through the
> hypervisor_x86 structure.
> 
> Compile tested for KVM and XEN configs, this patch doesn't have any functional
> effect on those two platforms.
> 
> On VMware platform, verified that x2apic is used in physical mode on products 
> that
> support this.
> 
> Signed-off-by: Alok N Kataria 
> Reviewed-by: Doug Covelli 
> Reviewed-by: Dan Hecht 
> Cc: "H. Peter Anvin" 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Jeremy Fitzhardinge 
> Cc: Avi Kivity 
> 
> ---
> 
>  arch/x86/include/asm/hypervisor.h |   13 -
>  arch/x86/kernel/cpu/hypervisor.c  |7 +++
>  arch/x86/kernel/cpu/vmware.c  |   13 +
>  arch/x86/kernel/kvm.c |1 +
>  arch/x86/xen/enlighten.c  |1 +
>  5 files changed, 26 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/arch/x86/include/asm/hypervisor.h 
> b/arch/x86/include/asm/hypervisor.h
> index b518c75..86095ed 100644
> --- a/arch/x86/include/asm/hypervisor.h
> +++ b/arch/x86/include/asm/hypervisor.h
> @@ -25,6 +25,7 @@
>  
>  extern void init_hypervisor(struct cpuinfo_x86 *c);
>  extern void init_hypervisor_platform(void);
> +extern bool hypervisor_x2apic_available(void);
>  
>  /*
>   * x86 hypervisor information
> @@ -41,6 +42,9 @@ struct hypervisor_x86 {
>  
>   /* Platform setup (run once per boot) */
>   void(*init_platform)(void);
> +
> + /* X2APIC detection (run once per boot) */
> + bool(*x2apic_available)(void);
>  };
>  
>  extern const struct hypervisor_x86 *x86_hyper;
> @@ -51,13 +55,4 @@ extern const struct hypervisor_x86 x86_hyper_ms_hyperv;
>  extern const struct hypervisor_x86 x86_hyper_xen_hvm;
>  extern const struct hypervisor_x86 x86_hyper_kvm;
>  
> -static inline bool hypervisor_x2apic_available(void)
> -{
> - if (kvm_para_available())
> - return true;
> - if (xen_x2apic_para_available())
> - return true;
> - return false;
> -}
> -
>  #endif
> diff --git a/arch/x86/kernel/cpu/hypervisor.c 
> b/arch/x86/kernel/cpu/hypervisor.c
> index a8f8fa9..1e7e84a 100644
> --- a/arch/x86/kernel/cpu/hypervisor.c
> +++ b/arch/x86/kernel/cpu/hypervisor.c
> @@ -79,3 +79,10 @@ void __init init_hypervisor_platform(void)
>   if (x86_hyper->init_platform)
>   x86_hyper->init_platform();
>  }
> +
> +bool __init hypervisor_x2apic_available(void)
> +{
> + return x86_hyper   &&
> +x86_hyper->x2apic_available &&
> +x86_hyper->x2apic_available();
> +}
> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> index d22d0c4..03a3632 100644
> --- a/arch/x86/kernel/cpu/vmware.c
> +++ b/arch/x86/kernel/cpu/vmware.c
> @@ -33,6 +33,9 @@
>  
>  #define VMWARE_PORT_CMD_GETVERSION   10
>  #define VMWARE_PORT_CMD_GETHZ45
> +#define VMWARE_PORT_CMD_GETVCPU_INFO 68
> +#define VMWARE_PORT_CMD_LEGACY_X2APIC3
> +#define VMWARE_PORT_CMD_VCPU_RESERVED31
>  
>  #define VMWARE_PORT(cmd, eax, ebx, ecx, edx) \
>   __asm__("inl (%%dx)" :  \
> @@ -125,10 +128,20 @@ static void __cpuinit vmware_set_cpu_features(struct 
> cpuinfo_x86 *c)
>   set_cpu_cap(c, X86_FEATURE_TSC_RELIABLE);
>  }
>  
> +/* Checks if hypervisor supports x2apic without VT-D interrupt remapping. */
> +static bool __init vmware_legacy_x2apic_available(void)
> +{
> + uint32_t eax, ebx, ecx, edx;
> + VMWARE_PORT(GETVCPU_INFO, eax, ebx, ecx, edx);
> + return (eax & (1 << VMWARE_PORT_CMD_VCPU_RESERVED)) == 0 &&
> +(eax & (1 << VMWARE_PORT_CMD_LEGACY_X2APIC)) != 0;
> +}
> +
>  const __refconst struct hypervisor_x86 x86_hyper_vmware = {
>   .name   = "VMware",
>   .detect = vmware_platform,
>   .set_cpu_features   = vmware_set_cpu_features,
>   .init_platform  = vmware_platform_setup,
> + .x2apic_available   = vmware_legacy_x2apic_available,
>  };
>  EXPORT_SYMBOL(x86_hyper_vmware);
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 9c2bd8b..2b44ea5 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -505,6 +505,7 @@ static bool __init kvm_detect(void)
>  const struct hypervisor_x86 x86_hyper_kvm __refconst 

Workshops in the CISTI'2013 - 8th Iberian Conference on IST

2013-01-22 Thread Maria Lemos
***
Workshop in the CISTI'2013
8th Iberian Conference on Information Systems and Technologies
Lisbon, Portugal, June 19 - 23, 2013
http://www.aisti.eu/cisti2013/index.php?option=com_content&view=article&id=82&Itemid=67&lang=en
***

Complete list of workshops accepted in the CISTI'2013 - 8th Iberian Conference 
on Information Systems and Technologies:

> IAwDQ 2013 - Fourth Ibero-American Workshop on Data Quality

> SGaMePlay 2013 - Third Iberian Workshop on Serious Games and Meaningful Play 

> TICAMES 2013 - First Workshop on Information and Communication Technology in 
> Higher Education: Learning Mathematics

> WIA 2013 - Primero Workshop en Innovación Abierta

> WISA 2013 - Fifth Workshop on Intelligent Systems and Applications

> WISIS 2013 - Third Workshop on Information Systems for Interactive Spaces

> WSEQP 2013 - First Workshop in Software Engineering and Quality Process 

Best regards,

Maria Lemos
AISTI / CISTI'2013
http://www.aisti.eu/cisti2013

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

Re: [RFC] virtio_console: Add DRIVER and INTERFACE to uevent.

2013-01-22 Thread Greg KH
On Tue, Jan 22, 2013 at 09:55:20AM +1030, Rusty Russell wrote:
> sjur.brandel...@stericsson.com writes:
> 
> > From: Sjur Brændeland 
> >
> > Add information so rproc-serial can be easily recogniced
> > from user space. Add the following information to uevent:
> > DRIVER=virtio_console|virtio_rproc_serial
> > INTERFACE=grand-parent/parent/name
> >
> > Signed-off-by: Sjur Brændeland 
> > ---
> > Hi,
> >
> > I need some way to identify the major/minor number for
> > the rproc-serial device, given the udev event. 
> > Review comments are welcomed.
> >
> > Thanks,
> > Sjur
> 
> Hmm, I send all uevent questions to Greg KH (CC'd).

The "INTERFACE" uevent variable is only for USB devices, so for anything
else to be sending userspace that variable, would confuse things
greatly.

Sjur, what exactly are you trying to do here?  What do you want
userspace to know, and what should it do with that information?

thanks,

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


[QEMU PATCH v5 3/3] virtio-net: rename ctrl rx commands

2013-01-22 Thread Amos Kong
This patch makes rx commands consistent with specification.

Signed-off-by: Amos Kong 
---
 hw/virtio-net.c |   14 +++---
 hw/virtio-net.h |   14 +++---
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index acef5a5..ac4434e 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -326,17 +326,17 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, 
uint8_t cmd,
 return VIRTIO_NET_ERR;
 }
 
-if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC) {
+if (cmd == VIRTIO_NET_CTRL_RX_PROMISC) {
 n->promisc = on;
-} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI) {
+} else if (cmd == VIRTIO_NET_CTRL_RX_ALLMULTI) {
 n->allmulti = on;
-} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI) {
+} else if (cmd == VIRTIO_NET_CTRL_RX_ALLUNI) {
 n->alluni = on;
-} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI) {
+} else if (cmd == VIRTIO_NET_CTRL_RX_NOMULTI) {
 n->nomulti = on;
-} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI) {
+} else if (cmd == VIRTIO_NET_CTRL_RX_NOUNI) {
 n->nouni = on;
-} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST) {
+} else if (cmd == VIRTIO_NET_CTRL_RX_NOBCAST) {
 n->nobcast = on;
 } else {
 return VIRTIO_NET_ERR;
@@ -473,7 +473,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, 
VirtQueue *vq)
 iov_discard_front(&iov, &iov_cnt, sizeof(ctrl));
 if (s != sizeof(ctrl)) {
 status = VIRTIO_NET_ERR;
-} else if (ctrl.class == VIRTIO_NET_CTRL_RX_MODE) {
+} else if (ctrl.class == VIRTIO_NET_CTRL_RX) {
 status = virtio_net_handle_rx_mode(n, ctrl.cmd, iov, iov_cnt);
 } else if (ctrl.class == VIRTIO_NET_CTRL_MAC) {
 status = virtio_net_handle_mac(n, ctrl.cmd, iov, iov_cnt);
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 1ec632f..c0bb284 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -99,13 +99,13 @@ typedef uint8_t virtio_net_ctrl_ack;
  * 0 and 1 are supported with the VIRTIO_NET_F_CTRL_RX feature.
  * Commands 2-5 are added with VIRTIO_NET_F_CTRL_RX_EXTRA.
  */
-#define VIRTIO_NET_CTRL_RX_MODE0
- #define VIRTIO_NET_CTRL_RX_MODE_PROMISC  0
- #define VIRTIO_NET_CTRL_RX_MODE_ALLMULTI 1
- #define VIRTIO_NET_CTRL_RX_MODE_ALLUNI   2
- #define VIRTIO_NET_CTRL_RX_MODE_NOMULTI  3
- #define VIRTIO_NET_CTRL_RX_MODE_NOUNI4
- #define VIRTIO_NET_CTRL_RX_MODE_NOBCAST  5
+#define VIRTIO_NET_CTRL_RX0
+ #define VIRTIO_NET_CTRL_RX_PROMISC  0
+ #define VIRTIO_NET_CTRL_RX_ALLMULTI 1
+ #define VIRTIO_NET_CTRL_RX_ALLUNI   2
+ #define VIRTIO_NET_CTRL_RX_NOMULTI  3
+ #define VIRTIO_NET_CTRL_RX_NOUNI4
+ #define VIRTIO_NET_CTRL_RX_NOBCAST  5
 
 /*
  * Control the MAC
-- 
1.7.1

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


[QEMU PATCH v5 2/3] virtio-net: introduce a new macaddr control

2013-01-22 Thread Amos Kong
In virtio-net guest driver, currently we write MAC address to
pci config space byte by byte, this means that we have an
intermediate step where mac is wrong. This patch introduced
a new control command to set MAC address, it's atomic.

VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility.

"mac" field will be set to read-only when VIRTIO_NET_F_CTRL_MAC_ADDR
is acked.

Signed-off-by: Amos Kong 
---
 hw/pc_piix.c|4 
 hw/virtio-net.c |   13 -
 hw/virtio-net.h |   12 ++--
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 0a6923d..6218350 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -297,6 +297,10 @@ static QEMUMachine pc_i440fx_machine_v1_4 = {
 .driver   = "usb-tablet",\
 .property = "usb_version",\
 .value= stringify(1),\
+},{\
+.driver   = "virtio-net-pci",\
+.property = "ctrl_mac_addr",\
+.value= "off",  \
 }
 
 static QEMUMachine pc_machine_v1_3 = {
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index af1f3a1..acef5a5 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -93,7 +93,8 @@ static void virtio_net_set_config(VirtIODevice *vdev, const 
uint8_t *config)
 
 memcpy(&netcfg, config, sizeof(netcfg));
 
-if (memcmp(netcfg.mac, n->mac, ETH_ALEN)) {
+if (!(n->vdev.guest_features >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) &&
+memcmp(netcfg.mac, n->mac, ETH_ALEN)) {
 memcpy(n->mac, netcfg.mac, ETH_ALEN);
 qemu_format_nic_info_str(&n->nic->nc, n->mac);
 }
@@ -350,6 +351,16 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
 struct virtio_net_ctrl_mac mac_data;
 size_t s;
 
+if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET) {
+if (iov_size(iov, iov_cnt) != sizeof(n->mac)) {
+return VIRTIO_NET_ERR;
+}
+s = iov_to_buf(iov, iov_cnt, 0, &n->mac, sizeof(n->mac));
+assert(s == sizeof(n->mac));
+qemu_format_nic_info_str(&n->nic->nc, n->mac);
+return VIRTIO_NET_OK;
+}
+
 if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET) {
 return VIRTIO_NET_ERR;
 }
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index d46fb98..1ec632f 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -44,6 +44,8 @@
 #define VIRTIO_NET_F_CTRL_VLAN  19  /* Control channel VLAN filtering */
 #define VIRTIO_NET_F_CTRL_RX_EXTRA 20   /* Extra RX mode control support */
 
+#define VIRTIO_NET_F_CTRL_MAC_ADDR   23 /* Set MAC address */
+
 #define VIRTIO_NET_S_LINK_UP1   /* Link is up */
 
 #define TX_TIMER_INTERVAL 15 /* 150 us */
@@ -106,7 +108,7 @@ typedef uint8_t virtio_net_ctrl_ack;
  #define VIRTIO_NET_CTRL_RX_MODE_NOBCAST  5
 
 /*
- * Control the MAC filter table.
+ * Control the MAC
  *
  * The MAC filter table is managed by the hypervisor, the guest should
  * assume the size is infinite.  Filtering should be considered
@@ -119,6 +121,10 @@ typedef uint8_t virtio_net_ctrl_ack;
  * first sg list contains unicast addresses, the second is for multicast.
  * This functionality is present if the VIRTIO_NET_F_CTRL_RX feature
  * is available.
+ *
+ * The ADDR_SET command requests one out scatterlist, it contains a
+ * 6 bytes MAC address. This functionality is present if the
+ * VIRTIO_NET_F_CTRL_MAC_ADDR feature is available.
  */
 struct virtio_net_ctrl_mac {
 uint32_t entries;
@@ -126,6 +132,7 @@ struct virtio_net_ctrl_mac {
 };
 #define VIRTIO_NET_CTRL_MAC1
  #define VIRTIO_NET_CTRL_MAC_TABLE_SET0
+ #define VIRTIO_NET_CTRL_MAC_ADDR_SET 1
 
 /*
  * Control VLAN filtering
@@ -158,5 +165,6 @@ struct virtio_net_ctrl_mac {
 DEFINE_PROP_BIT("ctrl_vq", _state, _field, VIRTIO_NET_F_CTRL_VQ, 
true), \
 DEFINE_PROP_BIT("ctrl_rx", _state, _field, VIRTIO_NET_F_CTRL_RX, 
true), \
 DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, 
true), \
-DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, 
VIRTIO_NET_F_CTRL_RX_EXTRA, true)
+DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, 
VIRTIO_NET_F_CTRL_RX_EXTRA, true), \
+DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, 
VIRTIO_NET_F_CTRL_MAC_ADDR, true)
 #endif
-- 
1.7.1

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


[QEMU PATCH v5 1/3] virtio-net: remove layout assumptions for ctrl vq

2013-01-22 Thread Amos Kong
From: Michael S. Tsirkin 

Virtio-net code makes assumption about virtqueue descriptor layout
(e.g. sg[0] is the header, sg[1] is the data buffer).

This patch makes code not rely on the layout of descriptors.

Signed-off-by: Michael S. Tsirkin 
Signed-off-by: Amos Kong 
---
 hw/virtio-net.c |  129 ---
 1 files changed, 75 insertions(+), 54 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 3bb01b1..af1f3a1 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -315,44 +315,44 @@ static void virtio_net_set_features(VirtIODevice *vdev, 
uint32_t features)
 }
 
 static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
- VirtQueueElement *elem)
+ struct iovec *iov, unsigned int iov_cnt)
 {
 uint8_t on;
+size_t s;
 
-if (elem->out_num != 2 || elem->out_sg[1].iov_len != sizeof(on)) {
-error_report("virtio-net ctrl invalid rx mode command");
-exit(1);
+s = iov_to_buf(iov, iov_cnt, 0, &on, sizeof(on));
+if (s != sizeof(on)) {
+return VIRTIO_NET_ERR;
 }
 
-on = ldub_p(elem->out_sg[1].iov_base);
-
-if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC)
+if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC) {
 n->promisc = on;
-else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI)
+} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI) {
 n->allmulti = on;
-else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI)
+} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI) {
 n->alluni = on;
-else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI)
+} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI) {
 n->nomulti = on;
-else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI)
+} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI) {
 n->nouni = on;
-else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST)
+} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST) {
 n->nobcast = on;
-else
+} else {
 return VIRTIO_NET_ERR;
+}
 
 return VIRTIO_NET_OK;
 }
 
 static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
- VirtQueueElement *elem)
+ struct iovec *iov, unsigned int iov_cnt)
 {
 struct virtio_net_ctrl_mac mac_data;
+size_t s;
 
-if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET || elem->out_num != 3 ||
-elem->out_sg[1].iov_len < sizeof(mac_data) ||
-elem->out_sg[2].iov_len < sizeof(mac_data))
+if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET) {
 return VIRTIO_NET_ERR;
+}
 
 n->mac_table.in_use = 0;
 n->mac_table.first_multi = 0;
@@ -360,54 +360,72 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t 
cmd,
 n->mac_table.multi_overflow = 0;
 memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
 
-mac_data.entries = ldl_p(elem->out_sg[1].iov_base);
+s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
+   sizeof(mac_data.entries));
+mac_data.entries = ldl_p(&mac_data.entries);
+if (s != sizeof(mac_data.entries)) {
+return VIRTIO_NET_ERR;
+}
+iov_discard_front(&iov, &iov_cnt, s);
 
-if (sizeof(mac_data.entries) +
-(mac_data.entries * ETH_ALEN) > elem->out_sg[1].iov_len)
+if (mac_data.entries * ETH_ALEN > iov_size(iov, iov_cnt)) {
 return VIRTIO_NET_ERR;
+}
 
 if (mac_data.entries <= MAC_TABLE_ENTRIES) {
-memcpy(n->mac_table.macs, elem->out_sg[1].iov_base + sizeof(mac_data),
-   mac_data.entries * ETH_ALEN);
+s = iov_to_buf(iov, iov_cnt, 0, n->mac_table.macs,
+   mac_data.entries * ETH_ALEN);
+if (s != mac_data.entries * ETH_ALEN) {
+return VIRTIO_NET_ERR;
+}
 n->mac_table.in_use += mac_data.entries;
 } else {
 n->mac_table.uni_overflow = 1;
 }
 
+iov_discard_front(&iov, &iov_cnt, mac_data.entries * ETH_ALEN);
+
 n->mac_table.first_multi = n->mac_table.in_use;
 
-mac_data.entries = ldl_p(elem->out_sg[2].iov_base);
+s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
+   sizeof(mac_data.entries));
+mac_data.entries = ldl_p(&mac_data.entries);
+if (s != sizeof(mac_data.entries)) {
+return VIRTIO_NET_ERR;
+}
+
+iov_discard_front(&iov, &iov_cnt, s);
 
-if (sizeof(mac_data.entries) +
-(mac_data.entries * ETH_ALEN) > elem->out_sg[2].iov_len)
+if (mac_data.entries * ETH_ALEN != iov_size(iov, iov_cnt)) {
 return VIRTIO_NET_ERR;
+}
 
-if (mac_data.entries) {
-if (n->mac_table.in_use + mac_data.entries <= MAC_TABLE_ENTRIES) {
-memcpy(n->mac_table.macs + (n->mac_table.in_use * ETH_ALEN),
-   elem->out_sg[2].iov_base + sizeof(mac_data),
-   mac_data.entries * ETH_ALEN);
-n->mac_table.in_use += mac_data.entries;
-} else {
-n->mac_table.mult

[QEMU PATCH v5 0/3] virtio-net: fix of ctrl commands

2013-01-22 Thread Amos Kong
Currently virtio-net code relys on the layout of descriptor,
this patchset removed the assumptions and introduced a control
command to set mac address. Last patch is a trivial renaming.

V2: check guest's iov_len
V3: fix of migration compatibility
make mac field in config space read-only when new feature is acked
V4: add fix of descriptor layout assumptions, trivial rename
V5: fix endianness after iov_to_buf copy

Amos Kong (2):
  virtio-net: introduce a new macaddr control
  virtio-net: rename ctrl rx commands

Michael S. Tsirkin (1):
  virtio-net: remove layout assumptions for ctrl vq

 hw/pc_piix.c|4 ++
 hw/virtio-net.c |  142 +-
 hw/virtio-net.h |   26 +++
 3 files changed, 108 insertions(+), 64 deletions(-)

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


Re: [PATCH 1/6] virtio_host: host-side implementation of virtio rings.

2013-01-22 Thread Sjur Brændeland
On Mon, Jan 21, 2013 at 3:36 AM, Rusty Russell  wrote:
> Sjur Brændeland  writes:
>> On Thu, Jan 17, 2013 at 12:23 PM, Michael S. Tsirkin  wrote:
>>> in otgher words, we might need to split a single desc to multiple
>>> iov entries.
>>
>> Splitting descriptors doesn't work for me. As described earlier, I
>> receive one IP-frame for each descriptor. So I need to keep
>> the original boundaries.
>
> Yes, but the _kern helpers don't do range checking or offsetting at the
> moment, so they'll be preserved.
>
> If you want to do range checking, I can add that...

I don't think I need that. Initially I can just check if the received address
is readable.

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

Re: [QEMU PATCH v4 1/3] virtio-net: remove layout assumptions for ctrl vq

2013-01-22 Thread Stefan Hajnoczi
On Tue, Jan 22, 2013 at 10:38:14PM +0800, Amos Kong wrote:
> On Mon, Jan 21, 2013 at 05:03:30PM +0100, Stefan Hajnoczi wrote:
> > On Sat, Jan 19, 2013 at 09:54:26AM +0800, ak...@redhat.com wrote:
> > > From: "Michael S. Tsirkin" 
> > > 
> > > Virtio-net code makes assumption about virtqueue descriptor layout
> > > (e.g. sg[0] is the header, sg[1] is the data buffer).
> > > 
> > > This patch makes code not rely on the layout of descriptors.
> > > 
> > > Signed-off-by: Michael S. Tsirkin 
> > > Signed-off-by: Amos Kong 
> > > ---
> > >  hw/virtio-net.c | 128 
> > > 
> > >  1 file changed, 74 insertions(+), 54 deletions(-)
> > > 
> > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> > > index 3bb01b1..113e194 100644
> > > --- a/hw/virtio-net.c
> > > +++ b/hw/virtio-net.c
> > > @@ -315,44 +315,44 @@ static void virtio_net_set_features(VirtIODevice 
> > > *vdev, uint32_t features)
> > >  }
> > >  
> > >  static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
> > > - VirtQueueElement *elem)
> > > + struct iovec *iov, unsigned int 
> > > iov_cnt)
> > >  {
> > >  uint8_t on;
> > > +size_t s;
> > >  
> > > -if (elem->out_num != 2 || elem->out_sg[1].iov_len != sizeof(on)) {
> > > -error_report("virtio-net ctrl invalid rx mode command");
> > > -exit(1);
> > > +s = iov_to_buf(iov, iov_cnt, 0, &on, sizeof(on));
> > > +if (s != sizeof(on)) {
> > > +return VIRTIO_NET_ERR;
> > >  }
> > >  
> > > -on = ldub_p(elem->out_sg[1].iov_base);
> > > -
> > > -if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC)
> > > +if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC) {
> > >  n->promisc = on;
> > > -else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI)
> > > +} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI) {
> > >  n->allmulti = on;
> > > -else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI)
> > > +} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI) {
> > >  n->alluni = on;
> > > -else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI)
> > > +} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI) {
> > >  n->nomulti = on;
> > > -else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI)
> > > +} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI) {
> > >  n->nouni = on;
> > > -else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST)
> > > +} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST) {
> > >  n->nobcast = on;
> > > -else
> > > +} else {
> > >  return VIRTIO_NET_ERR;
> > > +}
> > >  
> > >  return VIRTIO_NET_OK;
> > >  }
> > >  
> > >  static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
> > > - VirtQueueElement *elem)
> > > + struct iovec *iov, unsigned int iov_cnt)
> > >  {
> > >  struct virtio_net_ctrl_mac mac_data;
> > > +size_t s;
> > >  
> > > -if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET || elem->out_num != 3 ||
> > > -elem->out_sg[1].iov_len < sizeof(mac_data) ||
> > > -elem->out_sg[2].iov_len < sizeof(mac_data))
> > > +if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET) {
> > >  return VIRTIO_NET_ERR;
> > > +}
> > >  
> > >  n->mac_table.in_use = 0;
> > >  n->mac_table.first_multi = 0;
> > > @@ -360,54 +360,71 @@ static int virtio_net_handle_mac(VirtIONet *n, 
> > > uint8_t cmd,
> > >  n->mac_table.multi_overflow = 0;
> > >  memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
> > >  
> > > -mac_data.entries = ldl_p(elem->out_sg[1].iov_base);
> > > +s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
> > > +   sizeof(mac_data.entries));
> 
> Hi Stefan, can we adjust the endianness after each iov_to_buf() copy?

Yes.

It's only necessary for uint16_t and larger types since a single byte
cannot be swapped (so ldub_p() is not needed).

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


Re: [QEMU PATCH v4 1/3] virtio-net: remove layout assumptions for ctrl vq

2013-01-22 Thread Amos Kong
On Mon, Jan 21, 2013 at 05:03:30PM +0100, Stefan Hajnoczi wrote:
> On Sat, Jan 19, 2013 at 09:54:26AM +0800, ak...@redhat.com wrote:
> > From: "Michael S. Tsirkin" 
> > 
> > Virtio-net code makes assumption about virtqueue descriptor layout
> > (e.g. sg[0] is the header, sg[1] is the data buffer).
> > 
> > This patch makes code not rely on the layout of descriptors.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > Signed-off-by: Amos Kong 
> > ---
> >  hw/virtio-net.c | 128 
> > 
> >  1 file changed, 74 insertions(+), 54 deletions(-)
> > 
> > diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> > index 3bb01b1..113e194 100644
> > --- a/hw/virtio-net.c
> > +++ b/hw/virtio-net.c
> > @@ -315,44 +315,44 @@ static void virtio_net_set_features(VirtIODevice 
> > *vdev, uint32_t features)
> >  }
> >  
> >  static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
> > - VirtQueueElement *elem)
> > + struct iovec *iov, unsigned int 
> > iov_cnt)
> >  {
> >  uint8_t on;
> > +size_t s;
> >  
> > -if (elem->out_num != 2 || elem->out_sg[1].iov_len != sizeof(on)) {
> > -error_report("virtio-net ctrl invalid rx mode command");
> > -exit(1);
> > +s = iov_to_buf(iov, iov_cnt, 0, &on, sizeof(on));
> > +if (s != sizeof(on)) {
> > +return VIRTIO_NET_ERR;
> >  }
> >  
> > -on = ldub_p(elem->out_sg[1].iov_base);
> > -
> > -if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC)
> > +if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC) {
> >  n->promisc = on;
> > -else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI)
> > +} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI) {
> >  n->allmulti = on;
> > -else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI)
> > +} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI) {
> >  n->alluni = on;
> > -else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI)
> > +} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI) {
> >  n->nomulti = on;
> > -else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI)
> > +} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI) {
> >  n->nouni = on;
> > -else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST)
> > +} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST) {
> >  n->nobcast = on;
> > -else
> > +} else {
> >  return VIRTIO_NET_ERR;
> > +}
> >  
> >  return VIRTIO_NET_OK;
> >  }
> >  
> >  static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
> > - VirtQueueElement *elem)
> > + struct iovec *iov, unsigned int iov_cnt)
> >  {
> >  struct virtio_net_ctrl_mac mac_data;
> > +size_t s;
> >  
> > -if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET || elem->out_num != 3 ||
> > -elem->out_sg[1].iov_len < sizeof(mac_data) ||
> > -elem->out_sg[2].iov_len < sizeof(mac_data))
> > +if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET) {
> >  return VIRTIO_NET_ERR;
> > +}
> >  
> >  n->mac_table.in_use = 0;
> >  n->mac_table.first_multi = 0;
> > @@ -360,54 +360,71 @@ static int virtio_net_handle_mac(VirtIONet *n, 
> > uint8_t cmd,
> >  n->mac_table.multi_overflow = 0;
> >  memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
> >  
> > -mac_data.entries = ldl_p(elem->out_sg[1].iov_base);
> > +s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
> > +   sizeof(mac_data.entries));

Hi Stefan, can we adjust the endianness after each iov_to_buf() copy?


diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 72d7857..0088d6c 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -321,6 +321,7 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t 
cmd,
 size_t s;
 
 s = iov_to_buf(iov, iov_cnt, 0, &on, sizeof(on));
+on = ldub_p(&on);
 if (s != sizeof(on)) {
 return VIRTIO_NET_ERR;
 }
@@ -362,7 +363,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
 
 s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
sizeof(mac_data.entries));
-
+mac_data.entries = ldl_p(&mac_data.entries);
 if (s != sizeof(mac_data.entries)) {
 return VIRTIO_NET_ERR;
 }
@@ -389,7 +390,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
 
 s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
sizeof(mac_data.entries));
-
+mac_data.entries = ldl_p(&mac_data.entries);
 if (s != sizeof(mac_data.entries)) {
 return VIRTIO_NET_ERR;
 }
@@ -421,6 +422,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, 
uint8_t cmd,
 size_t s;
 
 s = iov_to_buf(iov, iov_cnt, 0, &vid, sizeof(vid));
+vid = lduw_p(&vid);
 if (s != sizeof(vid)) {
 return VIRTIO_NET_ERR;
 }
@@ -458,6 +460,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, 
VirtQueue *vq)
 iov = elem.out_sg;
 

Re: [QEMU PATCH v4 2/3] virtio-net: introduce a new macaddr control

2013-01-22 Thread Amos Kong
On Mon, Jan 21, 2013 at 05:08:26PM +0100, Stefan Hajnoczi wrote:
> On Sat, Jan 19, 2013 at 09:54:27AM +0800, ak...@redhat.com wrote:
> > @@ -350,6 +351,18 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t 
> > cmd,
> >  struct virtio_net_ctrl_mac mac_data;
> >  size_t s;
> >  
> > +if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET) {
> > +if (iov_size(iov, iov_cnt) != ETH_ALEN) {
> > +return VIRTIO_NET_ERR;
> > +}
> > +s = iov_to_buf(iov, iov_cnt, 0, &n->mac, sizeof(n->mac));
> > +if (s != sizeof(n->mac)) {
> > +return VIRTIO_NET_ERR;
> > +}

 
> Since iov_size() was checked before iov_to_buf(), we never hit this
> error.  And if we did n->mac would be trashed (i.e. error handling is
> not complete).

You are right.
iov_size() computes the size by accounting iov[].iov_lens, the first
check is enough.
 
> I think assert(s == sizeof(n->mac)) is more appropriate appropriate.
> Also, please change ETH_ALEN to sizeof(n->mac) to make the relationship
> between the check and the copy clear.
> 

Will update this patch.

 if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET) {
 if (iov_size(iov, iov_cnt) != sizeof(n->mac)) {
 return VIRTIO_NET_ERR;
 }
 s = iov_to_buf(iov, iov_cnt, 0, &n->mac, sizeof(n->mac));
 assert(s == sizeof(n->mac));
 qemu_format_nic_info_str(&n->nic->nc, n->mac);
 return VIRTIO_NET_OK;
 }

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


Re: [PATCH 4/6] tools/virtio: add vring_test.

2013-01-22 Thread Asias He
On 01/17/2013 06:29 PM, Rusty Russell wrote:
> This is mainly to test the drivers/vhost/vringh.c code, but it also
> uses the drivers/virtio/virtio_ring.c code for the guest side.

vringh_test.c does not compile here:
(This series on top of 9a9284153d965a57edc7162a8e57c14c97f3a935)

$ cd tools/virtio
$ make
cc -g -O2 -Wall -I. -I ../../usr/include/ -Wno-pointer-sign
-fno-strict-overflow  -MMDvringh_test.c   -o vringh_test
In file included from ./linux/vringh.h:1:0,
 from ./../../drivers/vhost/vringh.c:6,
 from vringh_test.c:7:
./linux/../../../include/linux/vringh.h:27:28: fatal error:
uapi/linux/uio.h: No such file or directory
compilation terminated.
make: *** [vringh_test] Error 1


> Usage for testing the basic implementation:
> 
>   ./vringh_test
>   # Test with indirect descriptors
>   ./vringh_test --indirect
>   # Test with indirect descriptors and event indexex
>   ./vringh_test --indirect --eventidx
> 
> You can run a parallel stress test by adding --parallel to any of the
> above options.
> 
> eg ./vringh_test --parallel:
>   Using CPUS 0 and 3
>   Guest: notified 10107974, pinged 107970
>   Host: notified 108158, pinged 3172148
>   Time: R=17.659 U=6.640 S=6.640
> 
> ./vringh_test --eventidx --parallel:
>   Using CPUS 0 and 3
>   Guest: notified 156357, pinged 156251
>   Host: notified 156251, pinged 78179
>   Time: R=4.518 U=3.536 S=3.536
> 
> Signed-off-by: Rusty Russell 
> ---
>  tools/virtio/Makefile  |4 +-
>  tools/virtio/vringh_test.c |  591 
> 
>  2 files changed, 593 insertions(+), 2 deletions(-)
>  create mode 100644 tools/virtio/vringh_test.c
> 
> diff --git a/tools/virtio/Makefile b/tools/virtio/Makefile
> index d1d442e..b928c3e 100644
> --- a/tools/virtio/Makefile
> +++ b/tools/virtio/Makefile
> @@ -1,5 +1,5 @@
>  all: test mod
> -test: virtio_test
> +test: virtio_test vringh_test
>  virtio_test: virtio_ring.o virtio_test.o
>  CFLAGS += -g -O2 -Wall -I. -I ../../usr/include/ -Wno-pointer-sign 
> -fno-strict-overflow  -MMD
>  vpath %.c ../../drivers/virtio
> @@ -7,6 +7,6 @@ mod:
>   ${MAKE} -C `pwd`/../.. M=`pwd`/vhost_test
>  .PHONY: all test mod clean
>  clean:
> - ${RM} *.o vhost_test/*.o vhost_test/.*.cmd \
> + ${RM} *.o vringh_test virtio_test vhost_test/*.o vhost_test/.*.cmd \
>vhost_test/Module.symvers vhost_test/modules.order *.d
>  -include *.d
> diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c
> new file mode 100644
> index 000..f3868f4
> --- /dev/null
> +++ b/tools/virtio/vringh_test.c
> @@ -0,0 +1,591 @@
> +/* Simple test of virtio code, entirely in userpsace. */
> +#define _GNU_SOURCE
> +#include 
> +#include 
> +#include 
> +#include 
> +#include <../../drivers/vhost/vringh.c>
> +#include <../../drivers/virtio/virtio_ring.c>
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define USER_MEM (1024*1024)
> +void *__user_addr_min, *__user_addr_max;
> +void *__kmalloc_fake, *__kfree_ignore_start, *__kfree_ignore_end;
> +static u64 user_addr_offset;
> +
> +#define RINGSIZE 256
> +#define ALIGN 4096
> +
> +static void never_notify_host(struct virtqueue *vq)
> +{
> + abort();
> +}
> +
> +static void never_callback_guest(struct virtqueue *vq)
> +{
> + abort();
> +}
> +
> +static inline bool getrange_iov(u64 addr, struct vringh_range *r)
> +{
> + r->start = (u64)(unsigned long)__user_addr_min - user_addr_offset;
> + r->end_incl = (u64)(unsigned long)__user_addr_max - 1 - 
> user_addr_offset;
> + r->offset = user_addr_offset;
> + return true;
> +}
> +
> +struct guest_virtio_device {
> + struct virtio_device vdev;
> + int to_host_fd;
> + unsigned long notifies;
> +};
> +
> +static void parallel_notify_host(struct virtqueue *vq)
> +{
> + struct guest_virtio_device *gvdev;
> +
> + gvdev = container_of(vq->vdev, struct guest_virtio_device, vdev);
> + write(gvdev->to_host_fd, "", 1);
> + gvdev->notifies++;
> +}
> +
> +#define NUM_XFERS (1000)
> +
> +/* We aim for two "distant" cpus. */
> +static void find_cpus(unsigned int *first, unsigned int *last)
> +{
> + unsigned int i;
> +
> + *first = -1U;
> + *last = 0;
> + for (i = 0; i < 4096; i++) {
> + cpu_set_t set;
> + CPU_ZERO(&set);
> + CPU_SET(i, &set);
> + if (sched_setaffinity(getpid(), sizeof(set), &set) == 0) {
> + if (i < *first)
> + *first = i;
> + if (i > *last)
> + *last = i;
> + }
> + }
> +}
> +
> +static int parallel_test(unsigned long features)
> +{
> + void *host_map, *guest_map;
> + int fd, mapsize, to_guest[2], to_host[2];
> + unsigned long xfers = 0, notifies = 0, receives = 0;
> + unsigned int first_cpu, last_cpu;
> + cpu_set_t cpu_set;
> +
> + /* Create rea

Re: [PATCH 1/6] virtio_host: host-side implementation of virtio rings.

2013-01-22 Thread Asias He
On 01/17/2013 06:29 PM, Rusty Russell wrote:
> Getting use of virtio rings correct is tricky, and a recent patch saw
> an implementation of in-kernel rings (as separate from userspace).
> 
> This patch attempts to abstract the business of dealing with the
> virtio ring layout from the access (userspace or direct); to do this,
> we use function pointers, which gcc inlines correctly.
> 
> Signed-off-by: Rusty Russell 
> ---
>  drivers/Makefile |2 +-
>  drivers/vhost/Kconfig|8 +
>  drivers/vhost/Kconfig.tcm|1 +
>  drivers/vhost/Makefile   |2 +
>  drivers/vhost/vringh.c   |  818 
> ++
>  drivers/virtio/virtio_ring.c |   33 +-
>  include/linux/virtio_ring.h  |   57 +++


Why vringh_notify_enable_user() and vringh_notify_disable_user() are not
declared in include/linux/virtio_ring.h? Missed that?


>  include/linux/vringh.h   |  115 ++
>  8 files changed, 1008 insertions(+), 28 deletions(-)
>  create mode 100644 drivers/vhost/vringh.c
>  create mode 100644 include/linux/vringh.h
> 
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 7863b9f..351a34f 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -123,7 +123,7 @@ obj-$(CONFIG_PPC_PS3) += ps3/
>  obj-$(CONFIG_OF) += of/
>  obj-$(CONFIG_SSB)+= ssb/
>  obj-$(CONFIG_BCMA)   += bcma/
> -obj-$(CONFIG_VHOST_NET)  += vhost/
> +obj-$(CONFIG_VHOST_RING) += vhost/
>  obj-$(CONFIG_VLYNQ)  += vlynq/
>  obj-$(CONFIG_STAGING)+= staging/
>  obj-y+= platform/
> diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> index 202bba6..613b074 100644
> --- a/drivers/vhost/Kconfig
> +++ b/drivers/vhost/Kconfig
> @@ -1,6 +1,7 @@
>  config VHOST_NET
>   tristate "Host kernel accelerator for virtio net (EXPERIMENTAL)"
>   depends on NET && EVENTFD && (TUN || !TUN) && (MACVTAP || !MACVTAP) && 
> EXPERIMENTAL
> + select VHOST_RING
>   ---help---
> This kernel module can be loaded in host kernel to accelerate
> guest networking with virtio_net. Not to be confused with virtio_net
> @@ -12,3 +13,10 @@ config VHOST_NET
>  if STAGING
>  source "drivers/vhost/Kconfig.tcm"
>  endif
> +
> +config VHOST_RING
> + tristate
> + ---help---
> +   This option is selected by any driver which needs to access
> +   the host side of a virtio ring.
> +
> diff --git a/drivers/vhost/Kconfig.tcm b/drivers/vhost/Kconfig.tcm
> index a9c6f76..0218f77 100644
> --- a/drivers/vhost/Kconfig.tcm
> +++ b/drivers/vhost/Kconfig.tcm
> @@ -1,6 +1,7 @@
>  config TCM_VHOST
>   tristate "TCM_VHOST fabric module (EXPERIMENTAL)"
>   depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && m
> + select VHOST_RING
>   default n
>   ---help---
>   Say M here to enable the TCM_VHOST fabric module for use with 
> virtio-scsi guests
> diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile
> index a27b053..1d37f5e 100644
> --- a/drivers/vhost/Makefile
> +++ b/drivers/vhost/Makefile
> @@ -2,3 +2,5 @@ obj-$(CONFIG_VHOST_NET) += vhost_net.o
>  vhost_net-y := vhost.o net.o
>  
>  obj-$(CONFIG_TCM_VHOST) += tcm_vhost.o
> +
> +obj-$(CONFIG_VHOST_RING) += vringh.o
> diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> new file mode 100644
> index 000..b28670f
> --- /dev/null
> +++ b/drivers/vhost/vringh.c
> @@ -0,0 +1,818 @@
> +/*
> + * Helpers for the host side of a virtio ring.
> + *
> + * Since these may be in userspace, we use (inline) accessors.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static __printf(1,2) __cold void vringh_bad(const char *fmt, ...)
> +{
> + static DEFINE_RATELIMIT_STATE(vringh_rs,
> +   DEFAULT_RATELIMIT_INTERVAL,
> +   DEFAULT_RATELIMIT_BURST);
> + if (__ratelimit(&vringh_rs)) {
> + va_list ap;
> + va_start(ap, fmt);
> + printk(KERN_NOTICE "vringh:");
> + vprintk(fmt, ap);
> + va_end(ap);
> + }
> +}
> +
> +/* Returns vring->num if empty, -ve on error. */
> +static inline int __vringh_get_head(const struct vringh *vrh,
> + int (*getu16)(u16 *val, const u16 *p),
> + u16 *last_avail_idx)
> +{
> + u16 avail_idx, i, head;
> + int err;
> +
> + err = getu16(&avail_idx, &vrh->vring.avail->idx);
> + if (err) {
> + vringh_bad("Failed to access avail idx at %p",
> +&vrh->vring.avail->idx);
> + return err;
> + }
> +
> + if (*last_avail_idx == avail_idx)
> + return vrh->vring.num;
> +
> + /* Only get avail ring entries after they have been exposed by guest. */
> + virtio_rmb(vrh->weak_barriers);
> +
> + i = *last_avail_idx & (vrh->vring.num - 1);
> +
> + err = getu16(