[PATCH v4 2/3] net: split eth_mac_addr for better error handling

2013-01-19 Thread akong
From: Stefan Hajnoczi 

When we set mac address, software mac address in system and hardware mac
address all need to be updated. Current eth_mac_addr() doesn't allow
callers to implement error handling nicely.

This patch split eth_mac_addr() to prepare part and real commit part,
then we can prepare first, and try to change hardware address, then do
the real commit if hardware address is set successfully.

Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Amos Kong 
---
 include/linux/etherdevice.h |  2 ++
 net/ethernet/eth.c  | 43 ---
 2 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 243eea1..5cc6e0e 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -40,6 +40,8 @@ extern int eth_header_cache(const struct neighbour *neigh, 
struct hh_cache *hh,
 extern void eth_header_cache_update(struct hh_cache *hh,
const struct net_device *dev,
const unsigned char *haddr);
+extern int eth_prepare_mac_addr_change(struct net_device *dev, void *p);
+extern void eth_commit_mac_addr_change(struct net_device *dev, void *p);
 extern int eth_mac_addr(struct net_device *dev, void *p);
 extern int eth_change_mtu(struct net_device *dev, int new_mtu);
 extern int eth_validate_addr(struct net_device *dev);
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 4efad53..cce73eb 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -272,16 +272,11 @@ void eth_header_cache_update(struct hh_cache *hh,
 EXPORT_SYMBOL(eth_header_cache_update);
 
 /**
- * eth_mac_addr - set new Ethernet hardware address
+ * eth_prepare_mac_addr_change - prepare for mac change
  * @dev: network device
  * @p: socket address
- *
- * Change hardware address of device.
- *
- * This doesn't change hardware matching, so needs to be overridden
- * for most real devices.
  */
-int eth_mac_addr(struct net_device *dev, void *p)
+int eth_prepare_mac_addr_change(struct net_device *dev, void *p)
 {
struct sockaddr *addr = p;
 
@@ -289,9 +284,43 @@ int eth_mac_addr(struct net_device *dev, void *p)
return -EBUSY;
if (!is_valid_ether_addr(addr->sa_data))
return -EADDRNOTAVAIL;
+   return 0;
+}
+EXPORT_SYMBOL(eth_prepare_mac_addr_change);
+
+/**
+ * eth_commit_mac_addr_change - commit mac change
+ * @dev: network device
+ * @p: socket address
+ */
+void eth_commit_mac_addr_change(struct net_device *dev, void *p)
+{
+   struct sockaddr *addr = p;
+
memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
/* if device marked as NET_ADDR_RANDOM, reset it */
dev->addr_assign_type &= ~NET_ADDR_RANDOM;
+}
+EXPORT_SYMBOL(eth_commit_mac_addr_change);
+
+/**
+ * eth_mac_addr - set new Ethernet hardware address
+ * @dev: network device
+ * @p: socket address
+ *
+ * Change hardware address of device.
+ *
+ * This doesn't change hardware matching, so needs to be overridden
+ * for most real devices.
+ */
+int eth_mac_addr(struct net_device *dev, void *p)
+{
+   int ret;
+
+   ret = eth_prepare_mac_addr_change(dev, p);
+   if (ret < 0)
+   return ret;
+   eth_commit_mac_addr_change(dev, p);
return 0;
 }
 EXPORT_SYMBOL(eth_mac_addr);
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 3/3] virtio-net: introduce a new control to set macaddr

2013-01-19 Thread akong
From: Amos Kong 

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.

Signed-off-by: Amos Kong 
---
 drivers/net/virtio_net.c| 21 ++---
 include/uapi/linux/virtio_net.h |  8 +++-
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 395ab4f..701408a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -802,14 +802,28 @@ static int virtnet_set_mac_address(struct net_device 
*dev, void *p)
struct virtnet_info *vi = netdev_priv(dev);
struct virtio_device *vdev = vi->vdev;
int ret;
+   struct sockaddr *addr = p;
+   struct scatterlist sg;
 
-   ret = eth_mac_addr(dev, p);
+   ret = eth_prepare_mac_addr_change(dev, p);
if (ret)
return ret;
 
-   if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
+   if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
+   sg_init_one(&sg, addr->sa_data, dev->addr_len);
+   if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
+ VIRTIO_NET_CTRL_MAC_ADDR_SET,
+ &sg, 1, 0)) {
+   dev_warn(&vdev->dev,
+"Failed to set mac address by vq command.\n");
+   return -EINVAL;
+   }
+   } else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
- dev->dev_addr, dev->addr_len);
+ addr->sa_data, dev->addr_len);
+   }
+
+   eth_commit_mac_addr_change(dev, p);
 
return 0;
 }
@@ -1627,6 +1641,7 @@ static unsigned int features[] = {
VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
+   VIRTIO_NET_F_CTRL_MAC_ADDR,
 };
 
 static struct virtio_driver virtio_net_driver = {
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 848e358..a5a8c88 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -53,6 +53,7 @@
 * network */
 #define VIRTIO_NET_F_MQ22  /* Device supports Receive Flow
 * Steering */
+#define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
 
 #define VIRTIO_NET_S_LINK_UP   1   /* Link is up */
 #define VIRTIO_NET_S_ANNOUNCE  2   /* Announcement is needed */
@@ -127,7 +128,7 @@ typedef __u8 virtio_net_ctrl_ack;
  #define VIRTIO_NET_CTRL_RX_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
@@ -140,6 +141,10 @@ typedef __u8 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 {
__u32 entries;
@@ -148,6 +153,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
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 1/3] move virtnet_send_command() above virtnet_set_mac_address()

2013-01-19 Thread akong
From: Amos Kong 

We want to send vq command to set mac address in
virtnet_set_mac_address(), so do this function moving.
Fixed a little issue of coding style.

Signed-off-by: Amos Kong 
---
 drivers/net/virtio_net.c | 89 
 1 file changed, 44 insertions(+), 45 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a6fcf15..395ab4f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -753,6 +753,50 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct 
net_device *dev)
return NETDEV_TX_OK;
 }
 
+/*
+ * Send command via the control virtqueue and check status.  Commands
+ * supported by the hypervisor, as indicated by feature bits, should
+ * never fail unless improperly formated.
+ */
+static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
+struct scatterlist *data, int out, int in)
+{
+   struct scatterlist *s, sg[VIRTNET_SEND_COMMAND_SG_MAX + 2];
+   struct virtio_net_ctrl_hdr ctrl;
+   virtio_net_ctrl_ack status = ~0;
+   unsigned int tmp;
+   int i;
+
+   /* Caller should know better */
+   BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ||
+   (out + in > VIRTNET_SEND_COMMAND_SG_MAX));
+
+   out++; /* Add header */
+   in++; /* Add return status */
+
+   ctrl.class = class;
+   ctrl.cmd = cmd;
+
+   sg_init_table(sg, out + in);
+
+   sg_set_buf(&sg[0], &ctrl, sizeof(ctrl));
+   for_each_sg(data, s, out + in - 2, i)
+   sg_set_buf(&sg[i + 1], sg_virt(s), s->length);
+   sg_set_buf(&sg[out + in - 1], &status, sizeof(status));
+
+   BUG_ON(virtqueue_add_buf(vi->cvq, sg, out, in, vi, GFP_ATOMIC) < 0);
+
+   virtqueue_kick(vi->cvq);
+
+   /* Spin for a response, the kick causes an ioport write, trapping
+* into the hypervisor, so the request should be handled immediately.
+*/
+   while (!virtqueue_get_buf(vi->cvq, &tmp))
+   cpu_relax();
+
+   return status == VIRTIO_NET_OK;
+}
+
 static int virtnet_set_mac_address(struct net_device *dev, void *p)
 {
struct virtnet_info *vi = netdev_priv(dev);
@@ -819,51 +863,6 @@ static void virtnet_netpoll(struct net_device *dev)
 }
 #endif
 
-/*
- * Send command via the control virtqueue and check status.  Commands
- * supported by the hypervisor, as indicated by feature bits, should
- * never fail unless improperly formated.
- */
-static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
-struct scatterlist *data, int out, int in)
-{
-   struct scatterlist *s, sg[VIRTNET_SEND_COMMAND_SG_MAX + 2];
-   struct virtio_net_ctrl_hdr ctrl;
-   virtio_net_ctrl_ack status = ~0;
-   unsigned int tmp;
-   int i;
-
-   /* Caller should know better */
-   BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ||
-   (out + in > VIRTNET_SEND_COMMAND_SG_MAX));
-
-   out++; /* Add header */
-   in++; /* Add return status */
-
-   ctrl.class = class;
-   ctrl.cmd = cmd;
-
-   sg_init_table(sg, out + in);
-
-   sg_set_buf(&sg[0], &ctrl, sizeof(ctrl));
-   for_each_sg(data, s, out + in - 2, i)
-   sg_set_buf(&sg[i + 1], sg_virt(s), s->length);
-   sg_set_buf(&sg[out + in - 1], &status, sizeof(status));
-
-   BUG_ON(virtqueue_add_buf(vi->cvq, sg, out, in, vi, GFP_ATOMIC) < 0);
-
-   virtqueue_kick(vi->cvq);
-
-   /*
-* Spin for a response, the kick causes an ioport write, trapping
-* into the hypervisor, so the request should be handled immediately.
-*/
-   while (!virtqueue_get_buf(vi->cvq, &tmp))
-   cpu_relax();
-
-   return status == VIRTIO_NET_OK;
-}
-
 static void virtnet_ack_link_announce(struct virtnet_info *vi)
 {
rtnl_lock();
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 0/3] make mac programming for virtio net more robust

2013-01-19 Thread akong
From: Amos Kong 

Currenly mac is programmed byte by byte. This means that we
have an intermediate step where mac is wrong. 

Third patch introduced a new vq control command to set mac
address, it's atomic.

V2: check return of sending command, delay eth_mac_addr()
V3: restore software address when fail to set hardware address
V4: split eth_mac_addr, fix error handle

Amos Kong (2):
  move virtnet_send_command() above virtnet_set_mac_address()
  virtio-net: introduce a new control to set macaddr

Stefan Hajnoczi (1):
  net: split eth_mac_addr for better error handling

 drivers/net/virtio_net.c| 110 ++--
 include/linux/etherdevice.h |   2 +
 include/uapi/linux/virtio_net.h |   8 ++-
 net/ethernet/eth.c  |  43 +---
 4 files changed, 107 insertions(+), 56 deletions(-)

-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-01-18 Thread akong
From: 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 cf40ff2..5700e22 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;
@@ -474,7 +474,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.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-01-18 Thread akong
From: 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 | 15 ++-
 hw/virtio-net.h | 12 ++--
 3 files changed, 28 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 113e194..cf40ff2 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,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;
+}
+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.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-01-18 Thread akong
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));
 
-if (sizeof(mac_data.entries) +
-(mac_data.entries * ETH_ALEN) > elem->out_sg[1].iov_len)
+if (s != sizeof(mac_data.entries)) {
 return VIRTIO_NET_ERR;
+}
+iov_discard_front(&iov, &iov_cnt, s);
+
+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_OK;
+}
 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));
 
-if (sizeof(mac_data.entries) +
-(mac_data.entries * ETH_ALEN) > elem->out_sg[2].iov_len)
+if (s != sizeof(mac_data.entries)) {
 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.multi_overflow = 1;
+iov_discard_front(&iov, &iov_cnt, s);
+
+if (mac_data.entries * ETH_ALEN != iov_size(iov, iov_cnt)) {
+return VIRTIO_NET_ERR;
+}
+
+if (n->mac_table.in_use + mac_data.entries <= MAC_TABLE_ENTRIES) {
+   

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

2013-01-18 Thread akong
From: 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

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 | 143 ++--
 hw/virtio-net.h |  26 +++
 3 files changed, 109 insertions(+), 64 deletions(-)

-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3] virtio-spec: set mac address by a new vq command

2013-01-18 Thread akong
From: Amos Kong 

Virtio-net driver currently programs MAC address 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
in one time, and added a new feature flag VIRTIO_NET_F_MAC_ADDR
for this feature.

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

Signed-off-by: Amos Kong 
---
v2: add more detail about new command (Stefan)
v3: update of making 'mac' field to read-only
---
 virtio-spec.lyx | 68 -
 1 file changed, 67 insertions(+), 1 deletion(-)

diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 1ba9992..ba30ae6 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -56,6 +56,7 @@
 \html_math_output 0
 \html_css_as_file 0
 \html_be_strict false
+\author -1930653948 "Amos Kong" 
 \author -608949062 "Rusty Russell,,," 
 \author -385801441 "Cornelia Huck" cornelia.h...@de.ibm.com
 \author 1112500848 "Rusty Russell" ru...@rustcorp.com.au
@@ -4391,6 +4392,14 @@ VIRTIO_NET_F_GUEST_ANNOUNCE(21) Guest can send 
gratuitous packets.
 
 \change_inserted 1986246365 1352742808
 VIRTIO_NET_F_MQ(22) Device supports multiqueue with automatic receive steering.
+\change_inserted -1930653948 1358319033
+
+\end_layout
+
+\begin_layout Description
+
+\change_inserted -1930653948 1358319080
+VIRTIO_NET_F_CTRL_MAC_ADDR(23) Set MAC address.
 \change_unchanged
 
 \end_layout
@@ -5284,7 +5293,11 @@ The class VIRTIO_NET_CTRL_RX has two commands: 
VIRTIO_NET_CTRL_RX_PROMISC
 \end_layout
 
 \begin_layout Subsubsection*
-Setting MAC Address Filtering
+Setting MAC Address
+\change_deleted -1930653948 1358318470
+ Filtering
+\change_unchanged
+
 \end_layout
 
 \begin_layout Standard
@@ -5324,6 +5337,17 @@ struct virtio_net_ctrl_mac {
 \begin_layout Plain Layout
 
  #define VIRTIO_NET_CTRL_MAC_TABLE_SET0 
+\change_inserted -1930653948 1358318313
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted -1930653948 1358318331
+
+ #define VIRTIO_NET_CTRL_MAC_ADDR_SET 1
+\change_unchanged
+
 \end_layout
 
 \end_inset
@@ -5349,6 +5373,48 @@ T_CTRL_MAC_TABLE_SET.
  The command-specific-data is two variable length tables of 6-byte MAC 
addresses.
  The first table contains unicast addresses, and the second contains multicast
  addresses.
+\change_inserted -1930653948 1358318545
+
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted -1930653948 1358506710
+The config space 
+\begin_inset Quotes eld
+\end_inset
+
+mac
+\begin_inset Quotes erd
+\end_inset
+
+ field and the VIRTIO_NET_CTRL_MAC_ADDR_SET command both set the default
+ MAC address which rx filtering accepts.
+ The VIRTIO_NET_CTRL_MAC_ADDR_SET command is atomic whereas the config space
+ 
+\begin_inset Quotes eld
+\end_inset
+
+mac
+\begin_inset Quotes erd
+\end_inset
+
+ field is not.
+ 
+\begin_inset Quotes eld
+\end_inset
+
+mac
+\begin_inset Quotes erd
+\end_inset
+
+ field will be set to read-only when the VIRTIO_NET_CTRL_MAC_ADDR_SET command
+ is supported.
+ Therefore, VIRTIO_NET_CTRL_MAC_ADDR_SET is preferred, especially while
+ the NIC is up.
+ The command-specific-data is a 6-byte MAC address.
+\change_unchanged
+
 \end_layout
 
 \begin_layout Subsection*
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 0/2] make mac programming for virtio net more robust

2013-01-17 Thread akong
From: Amos Kong 

Currenly mac is programmed byte by byte. This means that we
have an intermediate step where mac is wrong. 

Second patch introduced a new vq control command to set mac
address, it's atomic.

V2: check return of sending command, delay eth_mac_addr()
V3: restore software address when fail to set hardware address

Amos Kong (2):
  move virtnet_send_command() above virtnet_set_mac_address()
  virtio-net: introduce a new control to set macaddr

 drivers/net/virtio_net.c| 110 +++-
 include/uapi/linux/virtio_net.h |   8 ++-
 2 files changed, 71 insertions(+), 47 deletions(-)

-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 2/2] virtio-net: introduce a new control to set macaddr

2013-01-17 Thread akong
From: Amos Kong 

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.

Signed-off-by: Amos Kong 
---
 drivers/net/virtio_net.c| 21 -
 include/uapi/linux/virtio_net.h |  8 +++-
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 395ab4f..837c978 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -802,14 +802,32 @@ static int virtnet_set_mac_address(struct net_device 
*dev, void *p)
struct virtnet_info *vi = netdev_priv(dev);
struct virtio_device *vdev = vi->vdev;
int ret;
+   struct scatterlist sg;
+   char save_addr[ETH_ALEN];
+   unsigned char save_aatype;
+
+   memcpy(save_addr, dev->dev_addr, ETH_ALEN);
+   save_aatype = dev->addr_assign_type;
 
ret = eth_mac_addr(dev, p);
if (ret)
return ret;
 
-   if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
+   if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
+   sg_init_one(&sg, dev->dev_addr, dev->addr_len);
+   if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
+ VIRTIO_NET_CTRL_MAC_ADDR_SET,
+ &sg, 1, 0)) {
+   dev_warn(&vdev->dev,
+"Failed to set mac address by vq command.\n");
+   memcpy(dev->dev_addr, save_addr, ETH_ALEN);
+   dev->addr_assign_type = save_aatype;
+   return -EINVAL;
+   }
+   } else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
  dev->dev_addr, dev->addr_len);
+   }
 
return 0;
 }
@@ -1627,6 +1645,7 @@ static unsigned int features[] = {
VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
+   VIRTIO_NET_F_CTRL_MAC_ADDR,
 };
 
 static struct virtio_driver virtio_net_driver = {
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 848e358..a5a8c88 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -53,6 +53,7 @@
 * network */
 #define VIRTIO_NET_F_MQ22  /* Device supports Receive Flow
 * Steering */
+#define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
 
 #define VIRTIO_NET_S_LINK_UP   1   /* Link is up */
 #define VIRTIO_NET_S_ANNOUNCE  2   /* Announcement is needed */
@@ -127,7 +128,7 @@ typedef __u8 virtio_net_ctrl_ack;
  #define VIRTIO_NET_CTRL_RX_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
@@ -140,6 +141,10 @@ typedef __u8 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 {
__u32 entries;
@@ -148,6 +153,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
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 1/2] move virtnet_send_command() above virtnet_set_mac_address()

2013-01-17 Thread akong
From: Amos Kong 

We will send vq command to set mac address in virtnet_set_mac_address()
a little fix of coding style

Signed-off-by: Amos Kong 
---
 drivers/net/virtio_net.c | 89 
 1 file changed, 44 insertions(+), 45 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a6fcf15..395ab4f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -753,6 +753,50 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct 
net_device *dev)
return NETDEV_TX_OK;
 }
 
+/*
+ * Send command via the control virtqueue and check status.  Commands
+ * supported by the hypervisor, as indicated by feature bits, should
+ * never fail unless improperly formated.
+ */
+static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
+struct scatterlist *data, int out, int in)
+{
+   struct scatterlist *s, sg[VIRTNET_SEND_COMMAND_SG_MAX + 2];
+   struct virtio_net_ctrl_hdr ctrl;
+   virtio_net_ctrl_ack status = ~0;
+   unsigned int tmp;
+   int i;
+
+   /* Caller should know better */
+   BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ||
+   (out + in > VIRTNET_SEND_COMMAND_SG_MAX));
+
+   out++; /* Add header */
+   in++; /* Add return status */
+
+   ctrl.class = class;
+   ctrl.cmd = cmd;
+
+   sg_init_table(sg, out + in);
+
+   sg_set_buf(&sg[0], &ctrl, sizeof(ctrl));
+   for_each_sg(data, s, out + in - 2, i)
+   sg_set_buf(&sg[i + 1], sg_virt(s), s->length);
+   sg_set_buf(&sg[out + in - 1], &status, sizeof(status));
+
+   BUG_ON(virtqueue_add_buf(vi->cvq, sg, out, in, vi, GFP_ATOMIC) < 0);
+
+   virtqueue_kick(vi->cvq);
+
+   /* Spin for a response, the kick causes an ioport write, trapping
+* into the hypervisor, so the request should be handled immediately.
+*/
+   while (!virtqueue_get_buf(vi->cvq, &tmp))
+   cpu_relax();
+
+   return status == VIRTIO_NET_OK;
+}
+
 static int virtnet_set_mac_address(struct net_device *dev, void *p)
 {
struct virtnet_info *vi = netdev_priv(dev);
@@ -819,51 +863,6 @@ static void virtnet_netpoll(struct net_device *dev)
 }
 #endif
 
-/*
- * Send command via the control virtqueue and check status.  Commands
- * supported by the hypervisor, as indicated by feature bits, should
- * never fail unless improperly formated.
- */
-static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
-struct scatterlist *data, int out, int in)
-{
-   struct scatterlist *s, sg[VIRTNET_SEND_COMMAND_SG_MAX + 2];
-   struct virtio_net_ctrl_hdr ctrl;
-   virtio_net_ctrl_ack status = ~0;
-   unsigned int tmp;
-   int i;
-
-   /* Caller should know better */
-   BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ||
-   (out + in > VIRTNET_SEND_COMMAND_SG_MAX));
-
-   out++; /* Add header */
-   in++; /* Add return status */
-
-   ctrl.class = class;
-   ctrl.cmd = cmd;
-
-   sg_init_table(sg, out + in);
-
-   sg_set_buf(&sg[0], &ctrl, sizeof(ctrl));
-   for_each_sg(data, s, out + in - 2, i)
-   sg_set_buf(&sg[i + 1], sg_virt(s), s->length);
-   sg_set_buf(&sg[out + in - 1], &status, sizeof(status));
-
-   BUG_ON(virtqueue_add_buf(vi->cvq, sg, out, in, vi, GFP_ATOMIC) < 0);
-
-   virtqueue_kick(vi->cvq);
-
-   /*
-* Spin for a response, the kick causes an ioport write, trapping
-* into the hypervisor, so the request should be handled immediately.
-*/
-   while (!virtqueue_get_buf(vi->cvq, &tmp))
-   cpu_relax();
-
-   return status == VIRTIO_NET_OK;
-}
-
 static void virtnet_ack_link_announce(struct virtnet_info *vi)
 {
rtnl_lock();
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[QEMU PATCH v3] virtio-net: introduce a new macaddr control

2013-01-17 Thread akong
From: 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 
---
V2: check guest's iov_len
V3: fix of migration compatibility
make mac field in config space read-only when new feature is acked
---
 hw/pc_piix.c|  4 
 hw/virtio-net.c | 10 +-
 hw/virtio-net.h | 12 ++--
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 7268dcd..66606b9 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -295,6 +295,10 @@ static QEMUMachine pc_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 dc7c6d6..941d782 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);
 }
@@ -349,6 +350,13 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
 {
 struct virtio_net_ctrl_mac mac_data;
 
+if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET && elem->out_num == 2 &&
+elem->out_sg[1].iov_len == ETH_ALEN) {
+memcpy(n->mac, elem->out_sg[1].iov_base, elem->out_sg[1].iov_len);
+qemu_format_nic_info_str(&n->nic->nc, n->mac);
+return VIRTIO_NET_OK;
+}
+
 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))
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.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] virtio-spec: set mac address by a new vq command

2013-01-17 Thread akong
From: Amos Kong 

Virtio-net driver currently programs MAC address 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
in one time, and added a new feature flag VIRTIO_NET_F_MAC_ADDR
for this feature.

Signed-off-by: Amos Kong 
---
v2: add more detail about new command (Stefan)
---
 virtio-spec.lyx | 58 -
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 1ba9992..1ec0cd4 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -56,6 +56,7 @@
 \html_math_output 0
 \html_css_as_file 0
 \html_be_strict false
+\author -1930653948 "Amos Kong" 
 \author -608949062 "Rusty Russell,,," 
 \author -385801441 "Cornelia Huck" cornelia.h...@de.ibm.com
 \author 1112500848 "Rusty Russell" ru...@rustcorp.com.au
@@ -4391,6 +4392,14 @@ VIRTIO_NET_F_GUEST_ANNOUNCE(21) Guest can send 
gratuitous packets.
 
 \change_inserted 1986246365 1352742808
 VIRTIO_NET_F_MQ(22) Device supports multiqueue with automatic receive steering.
+\change_inserted -1930653948 1358319033
+
+\end_layout
+
+\begin_layout Description
+
+\change_inserted -1930653948 1358319080
+VIRTIO_NET_F_CTRL_MAC_ADDR(23) Set MAC address.
 \change_unchanged
 
 \end_layout
@@ -5284,7 +5293,11 @@ The class VIRTIO_NET_CTRL_RX has two commands: 
VIRTIO_NET_CTRL_RX_PROMISC
 \end_layout
 
 \begin_layout Subsubsection*
-Setting MAC Address Filtering
+Setting MAC Address
+\change_deleted -1930653948 1358318470
+ Filtering
+\change_unchanged
+
 \end_layout
 
 \begin_layout Standard
@@ -5324,6 +5337,17 @@ struct virtio_net_ctrl_mac {
 \begin_layout Plain Layout
 
  #define VIRTIO_NET_CTRL_MAC_TABLE_SET0 
+\change_inserted -1930653948 1358318313
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted -1930653948 1358318331
+
+ #define VIRTIO_NET_CTRL_MAC_ADDR_SET 1
+\change_unchanged
+
 \end_layout
 
 \end_inset
@@ -5349,6 +5373,38 @@ T_CTRL_MAC_TABLE_SET.
  The command-specific-data is two variable length tables of 6-byte MAC 
addresses.
  The first table contains unicast addresses, and the second contains multicast
  addresses.
+\change_inserted -1930653948 1358318545
+
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted -1930653948 1358418243
+The config space 
+\begin_inset Quotes eld
+\end_inset
+
+mac
+\begin_inset Quotes erd
+\end_inset
+
+ field and the command VIRTIO_NET_CTRL_MAC_ADDR_SET both set the default
+ MAC address which rx filtering accepts.
+ The command VIRTIO_NET_CTRL_MAC_ADDR_SET is atomic whereas the config space
+ 
+\begin_inset Quotes eld
+\end_inset
+
+mac
+\begin_inset Quotes erd
+\end_inset
+
+ field is not.
+ Therefore, VIRTIO_NET_CTRL_MAC_ADDR_SET is preferred, especially while
+ the NIC is up.
+ The command-specific-data is a 6-byte MAC address.
+\change_unchanged
+
 \end_layout
 
 \begin_layout Subsection*
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] virtio-spec: set mac address by a new vq command

2013-01-15 Thread akong
From: Amos Kong 

Virtio-net driver currently programs MAC address 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
in one time, and added a new feature flag VIRTIO_NET_F_MAC_ADDR
for this feature.

Signed-off-by: Amos Kong 
---
 virtio-spec.lyx | 45 -
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 1ba9992..03f5a49 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -56,6 +56,7 @@
 \html_math_output 0
 \html_css_as_file 0
 \html_be_strict false
+\author -1930653948 "Amos Kong" ak...@redhat.com
 \author -608949062 "Rusty Russell,,," 
 \author -385801441 "Cornelia Huck" cornelia.h...@de.ibm.com
 \author 1112500848 "Rusty Russell" ru...@rustcorp.com.au
@@ -4391,6 +4392,14 @@ VIRTIO_NET_F_GUEST_ANNOUNCE(21) Guest can send 
gratuitous packets.
 
 \change_inserted 1986246365 1352742808
 VIRTIO_NET_F_MQ(22) Device supports multiqueue with automatic receive steering.
+\change_inserted -1930653948 1358319033
+
+\end_layout
+
+\begin_layout Description
+
+\change_inserted -1930653948 1358319080
+VIRTIO_NET_F_CTRL_MAC_ADDR(23) Set MAC address.
 \change_unchanged
 
 \end_layout
@@ -5284,7 +5293,11 @@ The class VIRTIO_NET_CTRL_RX has two commands: 
VIRTIO_NET_CTRL_RX_PROMISC
 \end_layout
 
 \begin_layout Subsubsection*
-Setting MAC Address Filtering
+Setting MAC Address
+\change_deleted -1930653948 1358318470
+ Filtering
+\change_unchanged
+
 \end_layout
 
 \begin_layout Standard
@@ -5324,6 +5337,17 @@ struct virtio_net_ctrl_mac {
 \begin_layout Plain Layout
 
  #define VIRTIO_NET_CTRL_MAC_TABLE_SET0 
+\change_inserted -1930653948 1358318313
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted -1930653948 1358318331
+
+ #define VIRTIO_NET_CTRL_MAC_ADDR_SET 1
+\change_unchanged
+
 \end_layout
 
 \end_inset
@@ -5349,6 +5373,25 @@ T_CTRL_MAC_TABLE_SET.
  The command-specific-data is two variable length tables of 6-byte MAC 
addresses.
  The first table contains unicast addresses, and the second contains multicast
  addresses.
+\change_inserted -1930653948 1358318545
+
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted -1930653948 1358320004
+The command VIRTIO_NET_CTRL_MAC_ADDR_SET is used to set 
+\begin_inset Quotes eld
+\end_inset
+
+physical
+\begin_inset Quotes erd
+\end_inset
+
+ address of the network card.
+ The command-specific-data is a 6-byte MAC address.
+\change_unchanged
+
 \end_layout
 
 \begin_layout Subsection*
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[QEMU PATCH v2] virtio-net: introduce a new macaddr control

2013-01-15 Thread akong
From: 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 in one time.

VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility.

Signed-off-by: Amos Kong 
---
V2: check guest's iov_len before memcpy
---
 hw/virtio-net.c | 10 ++
 hw/virtio-net.h |  9 -
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index dc7c6d6..d05f98f 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -247,6 +247,7 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, 
uint32_t features)
 VirtIONet *n = to_virtio_net(vdev);
 
 features |= (1 << VIRTIO_NET_F_MAC);
+features |= (1 << VIRTIO_NET_F_CTRL_MAC_ADDR);
 
 if (!peer_has_vnet_hdr(n)) {
 features &= ~(0x1 << VIRTIO_NET_F_CSUM);
@@ -282,6 +283,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
 /* Linux kernel 2.6.25.  It understood MAC (as everyone must),
  * but also these: */
 features |= (1 << VIRTIO_NET_F_MAC);
+features |= (1 << VIRTIO_NET_F_CTRL_MAC_ADDR);
 features |= (1 << VIRTIO_NET_F_CSUM);
 features |= (1 << VIRTIO_NET_F_HOST_TSO4);
 features |= (1 << VIRTIO_NET_F_HOST_TSO6);
@@ -349,6 +351,14 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
 {
 struct virtio_net_ctrl_mac mac_data;
 
+if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET && elem->out_num == 2 &&
+elem->out_sg[1].iov_len == ETH_ALEN) {
+/* Set MAC address */
+memcpy(n->mac, elem->out_sg[1].iov_base, elem->out_sg[1].iov_len);
+qemu_format_nic_info_str(&n->nic->nc, n->mac);
+return VIRTIO_NET_OK;
+}
+
 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))
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index d46fb98..9394cc0 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
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/2] make mac programming for virtio net more robust

2013-01-15 Thread akong
From: Amos Kong 

Currenly mac is programmed byte by byte. This means that we
have an intermediate step where mac is wrong. 

Second patch introduced a new vq control command to set mac
address in one time.

V2: check return of sending command, delay eth_mac_addr()

Amos Kong (2):
  move virtnet_send_command() above virtnet_set_mac_address()
  virtio-net: introduce a new control to set macaddr

 drivers/net/virtio_net.c| 113 ++--
 include/uapi/linux/virtio_net.h |   8 ++-
 2 files changed, 68 insertions(+), 53 deletions(-)

-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/2] move virtnet_send_command() above virtnet_set_mac_address()

2013-01-15 Thread akong
From: Amos Kong 

We will send vq command to set mac address in virtnet_set_mac_address()
a little fix of coding style

Signed-off-by: Amos Kong 
---
 drivers/net/virtio_net.c | 89 
 1 file changed, 44 insertions(+), 45 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a6fcf15..395ab4f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -753,6 +753,50 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct 
net_device *dev)
return NETDEV_TX_OK;
 }
 
+/*
+ * Send command via the control virtqueue and check status.  Commands
+ * supported by the hypervisor, as indicated by feature bits, should
+ * never fail unless improperly formated.
+ */
+static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
+struct scatterlist *data, int out, int in)
+{
+   struct scatterlist *s, sg[VIRTNET_SEND_COMMAND_SG_MAX + 2];
+   struct virtio_net_ctrl_hdr ctrl;
+   virtio_net_ctrl_ack status = ~0;
+   unsigned int tmp;
+   int i;
+
+   /* Caller should know better */
+   BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ||
+   (out + in > VIRTNET_SEND_COMMAND_SG_MAX));
+
+   out++; /* Add header */
+   in++; /* Add return status */
+
+   ctrl.class = class;
+   ctrl.cmd = cmd;
+
+   sg_init_table(sg, out + in);
+
+   sg_set_buf(&sg[0], &ctrl, sizeof(ctrl));
+   for_each_sg(data, s, out + in - 2, i)
+   sg_set_buf(&sg[i + 1], sg_virt(s), s->length);
+   sg_set_buf(&sg[out + in - 1], &status, sizeof(status));
+
+   BUG_ON(virtqueue_add_buf(vi->cvq, sg, out, in, vi, GFP_ATOMIC) < 0);
+
+   virtqueue_kick(vi->cvq);
+
+   /* Spin for a response, the kick causes an ioport write, trapping
+* into the hypervisor, so the request should be handled immediately.
+*/
+   while (!virtqueue_get_buf(vi->cvq, &tmp))
+   cpu_relax();
+
+   return status == VIRTIO_NET_OK;
+}
+
 static int virtnet_set_mac_address(struct net_device *dev, void *p)
 {
struct virtnet_info *vi = netdev_priv(dev);
@@ -819,51 +863,6 @@ static void virtnet_netpoll(struct net_device *dev)
 }
 #endif
 
-/*
- * Send command via the control virtqueue and check status.  Commands
- * supported by the hypervisor, as indicated by feature bits, should
- * never fail unless improperly formated.
- */
-static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
-struct scatterlist *data, int out, int in)
-{
-   struct scatterlist *s, sg[VIRTNET_SEND_COMMAND_SG_MAX + 2];
-   struct virtio_net_ctrl_hdr ctrl;
-   virtio_net_ctrl_ack status = ~0;
-   unsigned int tmp;
-   int i;
-
-   /* Caller should know better */
-   BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ||
-   (out + in > VIRTNET_SEND_COMMAND_SG_MAX));
-
-   out++; /* Add header */
-   in++; /* Add return status */
-
-   ctrl.class = class;
-   ctrl.cmd = cmd;
-
-   sg_init_table(sg, out + in);
-
-   sg_set_buf(&sg[0], &ctrl, sizeof(ctrl));
-   for_each_sg(data, s, out + in - 2, i)
-   sg_set_buf(&sg[i + 1], sg_virt(s), s->length);
-   sg_set_buf(&sg[out + in - 1], &status, sizeof(status));
-
-   BUG_ON(virtqueue_add_buf(vi->cvq, sg, out, in, vi, GFP_ATOMIC) < 0);
-
-   virtqueue_kick(vi->cvq);
-
-   /*
-* Spin for a response, the kick causes an ioport write, trapping
-* into the hypervisor, so the request should be handled immediately.
-*/
-   while (!virtqueue_get_buf(vi->cvq, &tmp))
-   cpu_relax();
-
-   return status == VIRTIO_NET_OK;
-}
-
 static void virtnet_ack_link_announce(struct virtnet_info *vi)
 {
rtnl_lock();
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/2] virtio-net: introduce a new control to set macaddr

2013-01-15 Thread akong
From: Amos Kong 

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
in one time.

VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility.

Signed-off-by: Amos Kong 
---
 drivers/net/virtio_net.c| 24 +---
 include/uapi/linux/virtio_net.h |  8 +++-
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 395ab4f..c8901b6 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -802,16 +802,25 @@ static int virtnet_set_mac_address(struct net_device 
*dev, void *p)
struct virtnet_info *vi = netdev_priv(dev);
struct virtio_device *vdev = vi->vdev;
int ret;
+   struct sockaddr *addr = p;
+   struct scatterlist sg;
 
-   ret = eth_mac_addr(dev, p);
-   if (ret)
-   return ret;
-
-   if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
+   if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
+   sg_init_one(&sg, addr->sa_data, dev->addr_len);
+   if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
+ VIRTIO_NET_CTRL_MAC_ADDR_SET,
+ &sg, 1, 0)) {
+   dev_warn(&vdev->dev,
+"Failed to set mac address by vq command.\n");
+   return -EINVAL;
+   }
+   } else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
- dev->dev_addr, dev->addr_len);
+ addr->sa_data, dev->addr_len);
+   }
+   ret = eth_mac_addr(dev, p);
 
-   return 0;
+   return ret;
 }
 
 static struct rtnl_link_stats64 *virtnet_stats(struct net_device *dev,
@@ -1627,6 +1636,7 @@ static unsigned int features[] = {
VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
+   VIRTIO_NET_F_CTRL_MAC_ADDR,
 };
 
 static struct virtio_driver virtio_net_driver = {
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 848e358..a5a8c88 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -53,6 +53,7 @@
 * network */
 #define VIRTIO_NET_F_MQ22  /* Device supports Receive Flow
 * Steering */
+#define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
 
 #define VIRTIO_NET_S_LINK_UP   1   /* Link is up */
 #define VIRTIO_NET_S_ANNOUNCE  2   /* Announcement is needed */
@@ -127,7 +128,7 @@ typedef __u8 virtio_net_ctrl_ack;
  #define VIRTIO_NET_CTRL_RX_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
@@ -140,6 +141,10 @@ typedef __u8 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 {
__u32 entries;
@@ -148,6 +153,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
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH] virtio-net: introduce a new macaddr control

2013-01-10 Thread akong
From: 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 in one time.

VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility.

Signed-off-by: Amos Kong 
---
 hw/virtio-net.c | 9 +
 hw/virtio-net.h | 9 -
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index dc7c6d6..fc11106 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -247,6 +247,7 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, 
uint32_t features)
 VirtIONet *n = to_virtio_net(vdev);
 
 features |= (1 << VIRTIO_NET_F_MAC);
+features |= (1 << VIRTIO_NET_F_CTRL_MAC_ADDR);
 
 if (!peer_has_vnet_hdr(n)) {
 features &= ~(0x1 << VIRTIO_NET_F_CSUM);
@@ -282,6 +283,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
 /* Linux kernel 2.6.25.  It understood MAC (as everyone must),
  * but also these: */
 features |= (1 << VIRTIO_NET_F_MAC);
+features |= (1 << VIRTIO_NET_F_CTRL_MAC_ADDR);
 features |= (1 << VIRTIO_NET_F_CSUM);
 features |= (1 << VIRTIO_NET_F_HOST_TSO4);
 features |= (1 << VIRTIO_NET_F_HOST_TSO6);
@@ -349,6 +351,13 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
 {
 struct virtio_net_ctrl_mac mac_data;
 
+if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET && elem->out_num == 2) {
+/* Set MAC address */
+memcpy(n->mac, elem->out_sg[1].iov_base, elem->out_sg[1].iov_len);
+qemu_format_nic_info_str(&n->nic->nc, n->mac);
+return VIRTIO_NET_OK;
+}
+
 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))
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index d46fb98..9394cc0 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
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH 2/2] virtio-net: introduce a new control to set macaddr

2013-01-10 Thread akong
From: Amos Kong 

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
in one time.

VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility.

Signed-off-by: Amos Kong 
---
 drivers/net/virtio_net.c| 16 +++-
 include/uapi/linux/virtio_net.h |  8 +++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 395ab4f..ff22bcd 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -803,13 +803,26 @@ static int virtnet_set_mac_address(struct net_device 
*dev, void *p)
struct virtio_device *vdev = vi->vdev;
int ret;
 
+   struct scatterlist sg;
+
ret = eth_mac_addr(dev, p);
if (ret)
return ret;
 
-   if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
+   if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
+   /* Set MAC address by sending vq command */
+   sg_init_one(&sg, dev->dev_addr, dev->addr_len);
+   virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
+VIRTIO_NET_CTRL_MAC_ADDR_SET,
+&sg, 1, 0);
+   return 0;
+   }
+
+   if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
+   /* Set MAC address by writing config space */
vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
  dev->dev_addr, dev->addr_len);
+   }
 
return 0;
 }
@@ -1627,6 +1640,7 @@ static unsigned int features[] = {
VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
+   VIRTIO_NET_F_CTRL_MAC_ADDR,
 };
 
 static struct virtio_driver virtio_net_driver = {
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 848e358..a5a8c88 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -53,6 +53,7 @@
 * network */
 #define VIRTIO_NET_F_MQ22  /* Device supports Receive Flow
 * Steering */
+#define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
 
 #define VIRTIO_NET_S_LINK_UP   1   /* Link is up */
 #define VIRTIO_NET_S_ANNOUNCE  2   /* Announcement is needed */
@@ -127,7 +128,7 @@ typedef __u8 virtio_net_ctrl_ack;
  #define VIRTIO_NET_CTRL_RX_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
@@ -140,6 +141,10 @@ typedef __u8 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 {
__u32 entries;
@@ -148,6 +153,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
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH 1/2] move virtnet_send_command() above virtnet_set_mac_address()

2013-01-10 Thread akong
From: Amos Kong 

We will send vq command to set mac address in virtnet_set_mac_address()
a little fix of coding style

Signed-off-by: Amos Kong 
---
 drivers/net/virtio_net.c | 89 
 1 file changed, 44 insertions(+), 45 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a6fcf15..395ab4f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -753,6 +753,50 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct 
net_device *dev)
return NETDEV_TX_OK;
 }
 
+/*
+ * Send command via the control virtqueue and check status.  Commands
+ * supported by the hypervisor, as indicated by feature bits, should
+ * never fail unless improperly formated.
+ */
+static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
+struct scatterlist *data, int out, int in)
+{
+   struct scatterlist *s, sg[VIRTNET_SEND_COMMAND_SG_MAX + 2];
+   struct virtio_net_ctrl_hdr ctrl;
+   virtio_net_ctrl_ack status = ~0;
+   unsigned int tmp;
+   int i;
+
+   /* Caller should know better */
+   BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ||
+   (out + in > VIRTNET_SEND_COMMAND_SG_MAX));
+
+   out++; /* Add header */
+   in++; /* Add return status */
+
+   ctrl.class = class;
+   ctrl.cmd = cmd;
+
+   sg_init_table(sg, out + in);
+
+   sg_set_buf(&sg[0], &ctrl, sizeof(ctrl));
+   for_each_sg(data, s, out + in - 2, i)
+   sg_set_buf(&sg[i + 1], sg_virt(s), s->length);
+   sg_set_buf(&sg[out + in - 1], &status, sizeof(status));
+
+   BUG_ON(virtqueue_add_buf(vi->cvq, sg, out, in, vi, GFP_ATOMIC) < 0);
+
+   virtqueue_kick(vi->cvq);
+
+   /* Spin for a response, the kick causes an ioport write, trapping
+* into the hypervisor, so the request should be handled immediately.
+*/
+   while (!virtqueue_get_buf(vi->cvq, &tmp))
+   cpu_relax();
+
+   return status == VIRTIO_NET_OK;
+}
+
 static int virtnet_set_mac_address(struct net_device *dev, void *p)
 {
struct virtnet_info *vi = netdev_priv(dev);
@@ -819,51 +863,6 @@ static void virtnet_netpoll(struct net_device *dev)
 }
 #endif
 
-/*
- * Send command via the control virtqueue and check status.  Commands
- * supported by the hypervisor, as indicated by feature bits, should
- * never fail unless improperly formated.
- */
-static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
-struct scatterlist *data, int out, int in)
-{
-   struct scatterlist *s, sg[VIRTNET_SEND_COMMAND_SG_MAX + 2];
-   struct virtio_net_ctrl_hdr ctrl;
-   virtio_net_ctrl_ack status = ~0;
-   unsigned int tmp;
-   int i;
-
-   /* Caller should know better */
-   BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ||
-   (out + in > VIRTNET_SEND_COMMAND_SG_MAX));
-
-   out++; /* Add header */
-   in++; /* Add return status */
-
-   ctrl.class = class;
-   ctrl.cmd = cmd;
-
-   sg_init_table(sg, out + in);
-
-   sg_set_buf(&sg[0], &ctrl, sizeof(ctrl));
-   for_each_sg(data, s, out + in - 2, i)
-   sg_set_buf(&sg[i + 1], sg_virt(s), s->length);
-   sg_set_buf(&sg[out + in - 1], &status, sizeof(status));
-
-   BUG_ON(virtqueue_add_buf(vi->cvq, sg, out, in, vi, GFP_ATOMIC) < 0);
-
-   virtqueue_kick(vi->cvq);
-
-   /*
-* Spin for a response, the kick causes an ioport write, trapping
-* into the hypervisor, so the request should be handled immediately.
-*/
-   while (!virtqueue_get_buf(vi->cvq, &tmp))
-   cpu_relax();
-
-   return status == VIRTIO_NET_OK;
-}
-
 static void virtnet_ack_link_announce(struct virtnet_info *vi)
 {
rtnl_lock();
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH 0/2] make mac programming for virtio net more robust

2013-01-10 Thread akong
From: Amos Kong 

Currenly mac is programmed byte by byte. This means that we
have an intermediate step where mac is wrong. 

Second patch introduced a new vq control command to set mac
address in one time.

Amos Kong (2):
  move virtnet_send_command() above virtnet_set_mac_address()
  virtio-net: introduce a new control to set macaddr

 drivers/net/virtio_net.c| 105 ++--
 include/uapi/linux/virtio_net.h |   8 ++-
 2 files changed, 66 insertions(+), 47 deletions(-)

-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] virtio-spec: fix two typos

2013-01-08 Thread akong
From: Amos Kong 

VIRTIO_NET_F_VTRL_VQ -> VIRTIO_NET_F_CTRL_VQ
VIRTIO_NET_CTRL_MQ is defined to 4 in kernel code

Signed-off-by: Amos Kong 
---
 virtio-spec.lyx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 78ec5d0..1ba9992 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -5156,7 +5156,7 @@ Control Virtqueue
 \end_layout
 
 \begin_layout Standard
-The driver uses the control virtqueue (if VIRTIO_NET_F_VTRL_VQ is negotiated)
+The driver uses the control virtqueue (if VIRTIO_NET_F_CTRL_VQ is negotiated)
  to send commands to manipulate various features of the device which would
  not easily map into the configuration space.
 \end_layout
@@ -5501,7 +5501,7 @@ struct virtio_net_ctrl_mq {
 
 \change_inserted 1986246365 1353594263
 
-#define VIRTIO_NET_CTRL_MQ1
+#define VIRTIO_NET_CTRL_MQ4
 \end_layout
 
 \begin_layout Plain Layout
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] virtio-spec: Fix wrong bit number of device status

2011-06-07 Thread akong
From: Amos Kong 

qemu-kvm/hw/virtio_config.h:
 #define VIRTIO_CONFIG_S_ACKNOWLEDGE 1
 #define VIRTIO_CONFIG_S_DRIVER  2
 #define VIRTIO_CONFIG_S_DRIVER_OK   4
 #define VIRTIO_CONFIG_S_FAILED  0x80

virtio-spec:
ACKNOWLEDGE(1) :
DRIVER(2)  :
DRIVER_OK(3)   :
FAILED(128):

The spec refers to bit numbers and the headers use absolute numbers,
they are not consistent.

it shoule be 'FAILED(8)'.
2^(8-1) = 128

Signed-off-by: Amos Kong 
---
 virtio-spec.lyx |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 448af76..41b7657 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -1552,7 +1552,7 @@ FAILED
 \begin_inset space ~
 \end_inset
 
-(128) Indicates that something went wrong in the guest, and it has given
+(7) Indicates that something went wrong in the guest, and it has given
  up on the device.
  This could be an internal error, or the driver didn't like the device for
  some reason, or even a fatal error during device operation.
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] KVM: Remove redundant change of return value

2010-06-21 Thread akong
From: Amos Kong 

In the following situation, assign zero to 'r' is redundant, just remove them.

r = foo();
if (r)
goto out;
r = 0;
...

Signed-off-by: Amos Kong 
---
 arch/x86/kvm/x86.c |7 ---
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 33156a3..a23bfa0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2477,7 +2477,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
r = kvm_vcpu_ioctl_set_lapic(vcpu, u.lapic);
if (r)
goto out;
-   r = 0;
break;
}
case KVM_INTERRUPT: {
@@ -2489,14 +2488,12 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
r = kvm_vcpu_ioctl_interrupt(vcpu, &irq);
if (r)
goto out;
-   r = 0;
break;
}
case KVM_NMI: {
r = kvm_vcpu_ioctl_nmi(vcpu);
if (r)
goto out;
-   r = 0;
break;
}
case KVM_SET_CPUID: {
@@ -3227,7 +3224,6 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = kvm_vm_ioctl_set_irqchip(kvm, chip);
if (r)
goto set_irqchip_out;
-   r = 0;
set_irqchip_out:
kfree(chip);
if (r)
@@ -3260,7 +3256,6 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = kvm_vm_ioctl_set_pit(kvm, &u.ps);
if (r)
goto out;
-   r = 0;
break;
}
case KVM_GET_PIT2: {
@@ -3286,7 +3281,6 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = kvm_vm_ioctl_set_pit2(kvm, &u.ps2);
if (r)
goto out;
-   r = 0;
break;
}
case KVM_REINJECT_CONTROL: {
@@ -3297,7 +3291,6 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = kvm_vm_ioctl_reinject(kvm, &control);
if (r)
goto out;
-   r = 0;
break;
}
case KVM_XEN_HVM_CONFIG: {
-- 
1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html