qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <aliguori@us.ibm.com>
To: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
Cc: Alex Fishman <alex.fishman@ravellosystems.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Izik Eidus <izik.eidus@ravellosystems.com>,
	qemu-devel@nongnu.org, Yan Vugenfirer <yan@ravellosystems.com>,
	Gerhard Wiesinger <lists@wiesinger.com>
Subject: Re: [Qemu-devel] [PATCH v3] VMXNET3 paravirtual NIC device implementation
Date: Fri, 09 Mar 2012 13:50:59 -0600	[thread overview]
Message-ID: <4F5A5F23.6030108@us.ibm.com> (raw)
In-Reply-To: <1331134726-6634-2-git-send-email-dmitry.fleytman@ravellosystems.com>

On 03/07/2012 09:38 AM, Dmitry Fleytman wrote:
>      Implementation of VMWare VMXNET3 paravirtual NIC device.
>      Supports of all the device features including offload capabilties,
>      VLANs and etc.
>      The device is tested on different OSes:
>      Fedora 15
>      Ubuntu 10.4
>      Centos 6.2
>      Windows 2008R2
>      Windows 2008 64bit
>      Windows 2008 32bit
>      Windows 2003 64bit
>      Windows 2003 32bit
>      Currently live migration is not supported.
>
>      Signed-off-by: Dmitry Fleytman<dmitry@daynix.com>
>      Signed-off-by: Yan Vugenfirer<yan@daynix.com>
> ---
>   Makefile.objs           |    1 +
>   default-configs/pci.mak |    1 +
>   hw/pci.c                |    2 +
>   hw/pci.h                |    1 +
>   hw/virtio-net.h         |   13 +-

Why is this patch touching virtio-net?

>   hw/vmware_utils.h       |  131 +++
>   hw/vmxnet3.c            | 2744 +++++++++++++++++++++++++++++++++++++++++++++++
>   hw/vmxnet3.h            |  727 +++++++++++++
>   hw/vmxnet3_debug.h      |  104 ++
>   hw/vmxnet_utils.c       |  202 ++++
>   hw/vmxnet_utils.h       |  263 +++++
>   iov.c                   |   27 +
>   iov.h                   |    3 +
>   net.c                   |    2 +-
>   net/checksum.h          |    7 +

You should split out the common code changes into independent patches.

>   15 files changed, 4221 insertions(+), 7 deletions(-)
>   create mode 100644 hw/vmware_utils.h
>   create mode 100644 hw/vmxnet3.c
>   create mode 100644 hw/vmxnet3.h
>   create mode 100644 hw/vmxnet3_debug.h
>   create mode 100644 hw/vmxnet_utils.c
>   create mode 100644 hw/vmxnet_utils.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 808de6a..3f846a6 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -264,6 +264,7 @@ hw-obj-$(CONFIG_PCNET_PCI) += pcnet-pci.o
>   hw-obj-$(CONFIG_PCNET_COMMON) += pcnet.o
>   hw-obj-$(CONFIG_E1000_PCI) += e1000.o
>   hw-obj-$(CONFIG_RTL8139_PCI) += rtl8139.o
> +hw-obj-$(CONFIG_VMXNET3_PCI) += vmxnet3.o vmxnet_utils.o
>
>   hw-obj-$(CONFIG_SMC91C111) += smc91c111.o
>   hw-obj-$(CONFIG_LAN9118) += lan9118.o
> diff --git a/default-configs/pci.mak b/default-configs/pci.mak
> index 21e4ccf..f8e6ee1 100644
> --- a/default-configs/pci.mak
> +++ b/default-configs/pci.mak
> @@ -13,6 +13,7 @@ CONFIG_PCNET_COMMON=y
>   CONFIG_LSI_SCSI_PCI=y
>   CONFIG_RTL8139_PCI=y
>   CONFIG_E1000_PCI=y
> +CONFIG_VMXNET3_PCI=y
>   CONFIG_IDE_CORE=y
>   CONFIG_IDE_QDEV=y
>   CONFIG_IDE_PCI=y
> diff --git a/hw/pci.c b/hw/pci.c
> index bf046bf..f0fb1ee 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1350,6 +1350,7 @@ static const char * const pci_nic_models[] = {
>       "e1000",
>       "pcnet",
>       "virtio",
> +    "vmxnet3",
>       NULL
>   };
>
> @@ -1362,6 +1363,7 @@ static const char * const pci_nic_names[] = {
>       "e1000",
>       "pcnet",
>       "virtio-net-pci",
> +    "vmxnet3",
>       NULL
>   };
>
> diff --git a/hw/pci.h b/hw/pci.h
> index 4f19fdb..fee8250 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -60,6 +60,7 @@
>   #define PCI_DEVICE_ID_VMWARE_NET         0x0720
>   #define PCI_DEVICE_ID_VMWARE_SCSI        0x0730
>   #define PCI_DEVICE_ID_VMWARE_IDE         0x1729
> +#define PCI_DEVICE_ID_VMWARE_VMXNET3     0x07B0
>
>   /* Intel (0x8086) */
>   #define PCI_DEVICE_ID_INTEL_82551IT      0x1209
> diff --git a/hw/virtio-net.h b/hw/virtio-net.h
> index 4468741..fa3c17b 100644
> --- a/hw/virtio-net.h
> +++ b/hw/virtio-net.h
> @@ -78,13 +78,14 @@ struct virtio_net_config
>    * specify GSO or CSUM features, you can simply ignore the header. */
>   struct virtio_net_hdr
>   {
> -#define VIRTIO_NET_HDR_F_NEEDS_CSUM     1       // Use csum_start, csum_offset
> +#define VIRTIO_NET_HDR_F_NEEDS_CSUM     1     /* Use csum_start, csum_offset */
> +#define VIRTIO_NET_HDR_F_DATA_VALID     2     /* Csum is valid               */
>       uint8_t flags;
> -#define VIRTIO_NET_HDR_GSO_NONE         0       // Not a GSO frame
> -#define VIRTIO_NET_HDR_GSO_TCPV4        1       // GSO frame, IPv4 TCP (TSO)
> -#define VIRTIO_NET_HDR_GSO_UDP          3       // GSO frame, IPv4 UDP (UFO)
> -#define VIRTIO_NET_HDR_GSO_TCPV6        4       // GSO frame, IPv6 TCP
> -#define VIRTIO_NET_HDR_GSO_ECN          0x80    // TCP has ECN set
> +#define VIRTIO_NET_HDR_GSO_NONE         0     /* Not a GSO frame             */
> +#define VIRTIO_NET_HDR_GSO_TCPV4        1     /* GSO frame, IPv4 TCP (TSO)   */
> +#define VIRTIO_NET_HDR_GSO_UDP          3     /* GSO frame, IPv4 UDP (UFO)   */
> +#define VIRTIO_NET_HDR_GSO_TCPV6        4     /* GSO frame, IPv6 TCP         */
> +#define VIRTIO_NET_HDR_GSO_ECN          0x80  /* TCP has ECN set             */
>       uint8_t gso_type;
>       uint16_t hdr_len;
>       uint16_t gso_size;
> diff --git a/hw/vmware_utils.h b/hw/vmware_utils.h
> new file mode 100644
> index 0000000..304bb48
> --- /dev/null
> +++ b/hw/vmware_utils.h
> @@ -0,0 +1,131 @@
> +/*
> + * QEMU VMWARE paravirtual devices - auxiliary code
> + *
> + * Copyright (c) 2012 Ravello Systems LTD (http://ravellosystems.com)
> + *
> + * Developed by Daynix Computing LTD (http://www.daynix.com)
> + *
> + * Authors:
> + * Dmitry Fleytman<dmitry@daynix.com>
> + * Yan Vugenfirer<yan@daynix.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +/* Shared memory access functions with byte swap support */
> +static inline void
> +vmw_shmem_read(target_phys_addr_t addr, void *buf, int len)
> +{
> +    DSHPRINTF("SHMEM r: %" PRIx64 ", len: %d to %p",
> +              (uint64_t) addr, len, buf);
> +    cpu_physical_memory_read(addr, buf, len);
> +}
> +
> +static inline void
> +vmw_shmem_write(target_phys_addr_t addr, void *buf, int len)
> +{
> +    DSHPRINTF("SHMEM w: %" PRIx64 ", len: %d to %p",
> +              (uint64_t) addr, len, buf);
> +    cpu_physical_memory_write(addr, buf, len);
> +}
> +
> +static inline void
> +vmw_shmem_rw(target_phys_addr_t addr, void *buf, int len, int is_write)
> +{
> +    DSHPRINTF("SHMEM r/w: %" PRIx64 ", len: %d (to %p), is write: %d",
> +              (uint64_t) addr, len, buf, is_write);
> +
> +    cpu_physical_memory_rw(addr, buf, len, is_write);
> +}
> +
> +static inline void
> +vmw_shmem_set(target_phys_addr_t addr, uint8 val, int len)
> +{
> +    int i;
> +    DSHPRINTF("SHMEM set: %" PRIx64 ", len: %d (value 0x%X)",
> +              (uint64_t) addr, len, val);
> +
> +    for (i = 0; i<  len; i++) {
> +        cpu_physical_memory_write(addr + i,&val, 1);
> +    }
> +}
> +
> +static inline uint32_t
> +vmw_shmem_ld8(target_phys_addr_t addr)
> +{
> +    uint8_t res = ldub_phys(addr);
> +    DSHPRINTF("SHMEM load8: %" PRIx64 " (value 0x%X)",
> +              (uint64_t) addr, res);
> +    return res;
> +}
> +
> +static inline void
> +vmw_shmem_st8(target_phys_addr_t addr, uint8_t value)
> +{
> +    DSHPRINTF("SHMEM store8: %" PRIx64 " (value 0x%X)",
> +              (uint64_t) addr, value);
> +    stb_phys(addr, value);
> +}
> +
> +static inline uint32_t
> +vmw_shmem_ld16(target_phys_addr_t addr)
> +{
> +    uint16_t res = lduw_le_phys(addr);
> +    DSHPRINTF("SHMEM load16: %" PRIx64 " (value 0x%X)",
> +              (uint64_t) addr, res);
> +    return res;
> +}
> +
> +static inline void
> +vmw_shmem_st16(target_phys_addr_t addr, uint16_t value)
> +{
> +    DSHPRINTF("SHMEM store16: %" PRIx64 " (value 0x%X)",
> +              (uint64_t) addr, value);
> +    stw_le_phys(addr, value);
> +}
> +
> +static inline uint32_t
> +vmw_shmem_ld32(target_phys_addr_t addr)
> +{
> +    uint32_t res = ldl_le_phys(addr);
> +    DSHPRINTF("SHMEM load32: %" PRIx64 " (value 0x%X)",
> +              (uint64_t) addr, res);
> +    return res;
> +}
> +
> +static inline void
> +vmw_shmem_st32(target_phys_addr_t addr, uint32_t value)
> +{
> +    DSHPRINTF("SHMEM store32: %" PRIx64 " (value 0x%X)",
> +              (uint64_t) addr, value);
> +    stl_le_phys(addr, value);
> +}
> +
> +static inline uint64_t
> +vmw_shmem_ld64(target_phys_addr_t addr)
> +{
> +    uint64_t res = ldq_le_phys(addr);
> +    DSHPRINTF("SHMEM load64: %" PRIx64 " (value %" PRIx64 ")",
> +              (uint64_t) addr, res);
> +    return res;
> +}
> +
> +static inline void
> +vmw_shmem_st64(target_phys_addr_t addr, uint64_t value)
> +{
> +    DSHPRINTF("SHMEM store64: %" PRIx64 " (value %" PRIx64 ")",
> +              (uint64_t) addr, value);
> +    stq_le_phys(addr, value);
> +}
> +
> +/* MACROS for simplification of operations on array-style registers */
> +#define IS_MULTIREG_ADDR(addr, base, cnt, regsize)                 \
> +    (((addr)>= (base))&&  ((addr)<  (base) + (cnt) * (regsize)))
> +
> +#define MULTIREG_IDX_BY_ADDR(addr, base, regsize)                  \
> +    (((addr) - (base)) / (regsize))
> +
> +/* Bitfields */
> +#define FLAG_IS_SET(field, flag) (((field)&  (flag)) == (flag))
> diff --git a/hw/vmxnet3.c b/hw/vmxnet3.c
> new file mode 100644
> index 0000000..05daeed
> --- /dev/null
> +++ b/hw/vmxnet3.c
> @@ -0,0 +1,2744 @@
> +/*
> + * QEMU VMWARE VMXNET3 paravirtual NIC
> + *
> + * Copyright (c) 2012 Ravello Systems LTD (http://ravellosystems.com)
> + *
> + * Developed by Daynix Computing LTD (http://www.daynix.com)
> + *
> + * Authors:
> + * Dmitry Fleytman<dmitry@daynix.com>
> + * Yan Vugenfirer<yan@daynix.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#define VMXNET3_ENABLE_MSIX
> +#define VMXNET3_ENABLE_MSI
> +
> +/* Define this constant to non-zero to enable IP4       */
> +/* fragmentation feature                                */
> +
> +/* #define VMXNET_MAX_IP_PLOAD_LEN ETH_MAX_IP_PLOAD_LEN */
> +#define VMXNET3_MAX_IP_PLOAD_LEN 0
> +
> +#include "hw.h"
> +#include "pci.h"
> +#include "net.h"
> +#include "virtio-net.h"
> +#include "net/tap.h"
> +#include "net/checksum.h"
> +#include "sysemu.h"
> +#include "iov.h"
> +#include "bswap.h"
> +#ifdef VMXNET3_ENABLE_MSIX
> +#include "msix.h"
> +#endif
> +#ifdef VMXNET3_ENABLE_MSI
> +#include "msi.h"
> +#endif
> +
> +#include "vmxnet3_debug.h"
> +#include "vmxnet3.h"
> +#include "vmware_utils.h"
> +#include "vmxnet_utils.h"
> +
> +#define PCI_DEVICE_ID_VMWARE_VMXNET3_REVISION 0x1
> +#define VMXNET3_MSIX_BAR_SIZE 0x2000
> +
> +#define VMXNET3_BAR0_IDX      (0)
> +#define VMXNET3_BAR1_IDX      (1)
> +#define VMXNET3_MSIX_BAR_IDX  (2)
> +
> +/* Link speed in Mbps should be shifted by 16 */
> +#define VMXNET3_LINK_SPEED      (1000<<  16)
> +
> +/* Link status: 1 - up, 0 - down. */
> +#define VMXNET3_LINK_STATUS_UP  0x1
> +
> +/* Least significant bit should be set for revision and version */
> +#define VMXNET3_DEVICE_VERSION    0x1
> +#define VMXNET3_DEVICE_REVISION   0x1
> +
> +/* Macros for rings descriptors access */
> +#define VMXNET3_READ_TX_QUEUE_DESCR8(dpa, field) \
> +    (vmw_shmem_ld8(dpa + offsetof(struct Vmxnet3_TxQueueDesc, field)))
> +
> +#define VMXNET3_WRITE_TX_QUEUE_DESCR8(dpa, field, value) \
> +    (vmw_shmem_st8(dpa + offsetof(struct Vmxnet3_TxQueueDesc, field, value)))
> +
> +#define VMXNET3_READ_TX_QUEUE_DESCR32(dpa, field) \
> +    (vmw_shmem_ld32(dpa + offsetof(struct Vmxnet3_TxQueueDesc, field)))
> +
> +#define VMXNET3_WRITE_TX_QUEUE_DESCR32(dpa, field, value) \
> +    (vmw_shmem_st32(dpa + offsetof(struct Vmxnet3_TxQueueDesc, field), value))
> +
> +#define VMXNET3_READ_TX_QUEUE_DESCR64(dpa, field) \
> +    (vmw_shmem_ld64(dpa + offsetof(struct Vmxnet3_TxQueueDesc, field)))
> +
> +#define VMXNET3_WRITE_TX_QUEUE_DESCR64(dpa, field, value) \
> +    (vmw_shmem_st64(dpa + offsetof(struct Vmxnet3_TxQueueDesc, field), value))
> +
> +#define VMXNET3_READ_RX_QUEUE_DESCR64(dpa, field) \
> +    (vmw_shmem_ld64(dpa + offsetof(struct Vmxnet3_RxQueueDesc, field)))
> +
> +#define VMXNET3_READ_RX_QUEUE_DESCR32(dpa, field) \
> +    (vmw_shmem_ld32(dpa + offsetof(struct Vmxnet3_RxQueueDesc, field)))
> +
> +#define VMXNET3_WRITE_RX_QUEUE_DESCR64(dpa, field, value) \
> +    (vmw_shmem_st64(dpa + offsetof(struct Vmxnet3_RxQueueDesc, field), value))
> +
> +#define VMXNET3_WRITE_RX_QUEUE_DESCR8(dpa, field, value) \
> +    (vmw_shmem_st8(dpa + offsetof(struct Vmxnet3_RxQueueDesc, field), value))
> +
> +/* Macros for guest driver shared area access */
> +#define VMXNET3_READ_DRV_SHARED64(shpa, field) \
> +    (vmw_shmem_ld64(shpa + offsetof(struct Vmxnet3_DriverShared, field)))
> +
> +#define VMXNET3_READ_DRV_SHARED32(shpa, field) \
> +    (vmw_shmem_ld32(shpa + offsetof(struct Vmxnet3_DriverShared, field)))
> +
> +#define VMXNET3_WRITE_DRV_SHARED32(shpa, field, val) \
> +    (vmw_shmem_st32(shpa + offsetof(struct Vmxnet3_DriverShared, field), val))
> +
> +#define VMXNET3_READ_DRV_SHARED16(shpa, field) \
> +    (vmw_shmem_ld16(shpa + offsetof(struct Vmxnet3_DriverShared, field)))
> +
> +#define VMXNET3_READ_DRV_SHARED8(shpa, field) \
> +    (vmw_shmem_ld8(shpa + offsetof(struct Vmxnet3_DriverShared, field)))
> +
> +#define VMXNET3_READ_DRV_SHARED(shpa, field, b, l) \
> +    (vmw_shmem_read(shpa + offsetof(struct Vmxnet3_DriverShared, field), b, l))
> +
> +/* TX/RX packets abstractions */
> +typedef struct Vmxnet3_TxPktMdata {
> +    uint32_t offload_mode;
> +    uint32_t cso_or_gso_size;
> +    uint32_t hdr_length;
> +    eth_pkt_types_e packet_type;
> +} Vmxnet3_TxPktMdata;
> +
> +typedef struct _Vmxnet3_RxPktMdata {
> +    uint32_t tot_len;
> +    uint16_t vlan_tag;
> +    bool vlan_stripped;
> +    bool vhdr_valid;
> +    eth_pkt_types_e packet_type;
> +} Vmxnet3_RxPktMdata;
> +
> +#define VMXNET3_TXPKT_REBUILT_HDR_LEN (1024)
> +
> +typedef struct _Vmxnet3_TxPkt {
> +    Vmxnet3_TxPktMdata mdata;
> +    struct virtio_net_hdr virt_hdr;
> +    bool has_virt_hdr;
> +
> +    struct iovec *vec;
> +
> +    uint8_t __l2_hdr[ETH_MAX_L2_HDR_LEN];
> +    uint8_t __l3_hdr[ETH_MAX_L3_HDR_LEN];

Please avoid using underscores as prefixes.

> +
> +    uint32_t payload_len;
> +    uint32_t max_payload_len;
> +
> +    uint32_t payload_frags;
> +    uint32_t max_payload_frags;
> +
> +    struct {
> +        uint32_t offset;
> +        bool more_frags;
> +        bool orig_more_frags;
> +    } fragmentation;
> +} Vmxnet3_TxPkt;
> +
> +#define VMXNET3_TXPKT_VHDR_FRAG        (0)
> +#define VMXNET3_TXPKT_L2HDR_FRAG       (1)
> +#define VMXNET3_TXPKT_L3HDR_FRAG       (2)
> +#define VMXNET3_TXPKT_NUM_NETHDR_FRAGS (2)
> +#define VMXNET3_TXPKT_PL_START_FRAG    (3)
> +
> +#define vmxnet3_txpkt_get_mdata(p) (&((p)->mdata))
> +#define vmxnet3_txpkt_get_vhdr(p) (&((p)->virt_hdr))
> +
> +#define vmxnet3_txpkt_get_l2hdr(p)         \
> +    ((p)->vec[VMXNET3_TXPKT_L2HDR_FRAG].iov_base)
> +#define vmxnet3_txpkt_get_l2hdr_len(p)     \
> +    ((p)->vec[VMXNET3_TXPKT_L2HDR_FRAG].iov_len)
> +#define vmxnet3_txpkt_set_l2hdr_len(p, l)  \
> +    ((p)->vec[VMXNET3_TXPKT_L2HDR_FRAG].iov_len = l)
> +#define vmxnet3_txpkt_get_l3hdr(p)         \
> +    ((p)->vec[VMXNET3_TXPKT_L3HDR_FRAG].iov_base)
> +#define vmxnet3_txpkt_get_l3hdr_len(p)     \
> +    ((p)->vec[VMXNET3_TXPKT_L3HDR_FRAG].iov_len)
> +#define vmxnet3_txpkt_set_l3hdr_len(p, l)  \
> +    ((p)->vec[VMXNET3_TXPKT_L3HDR_FRAG].iov_len = l)
> +#define vmxnet3_txpkt_get_payload_len(p)   \
> +    ((p)->payload_len)
> +
> +#define vmxnet3_txpkt_set_more_frags(p, mf)    \
> +    ((p)->fragmentation.more_frags = mf)
> +#define vmxnet3_txpkt_get_more_frags(p)        \
> +    ((p)->fragmentation.more_frags | \
> +    (p)->fragmentation.orig_more_frags)
> +#define vmxnet3_txpkt_set_frag_off(p, off)     \
> +    ((p)->fragmentation.offset = off)
> +#define vmxnet3_txpkt_get_frag_off(p)          \
> +    ((p)->fragmentation.offset)
> +#define vmxnet3_txpkt_advance_frag_off(p, off) \
> +    ((p)->fragmentation.offset += off)

I'd prefer most of these macros to be static functions.  Static functions are 
much easier to debug and the compiler is very good at inlining as appropriate. 
I don't see any benefit to using macros here.

> +static inline size_t
> +vmxnet3_txpkt_get_total_len(const Vmxnet3_TxPkt *p)
> +{
> +    return vmxnet3_txpkt_get_l2hdr_len(p) +
> +           vmxnet3_txpkt_get_l3hdr_len(p) +
> +           vmxnet3_txpkt_get_payload_len(p);
> +}
> +
> +static inline struct iovec*
> +vmxnet3_txpkt_get_payload_frag(Vmxnet3_TxPkt *p, uint32_t num)
> +{
> +    assert(num<  p->max_payload_frags);
> +    return&p->vec[num + VMXNET3_TXPKT_PL_START_FRAG];
> +}
> +
> +static inline void
> +vmxnet3_txpkt_set_num_pl_frags(Vmxnet3_TxPkt *p, uint32_t num)
> +{
> +     assert(num<= p->max_payload_frags);
> +     p->payload_frags = num;
> +}
> +
> +static inline uint32_t
> +vmxnet3_txpkt_get_num_pl_frags(Vmxnet3_TxPkt *p)
> +{
> +    return p->payload_frags;
> +}
> +
> +static inline void
> +vmxnet3_txpkt_reset_payload(Vmxnet3_TxPkt *p)
> +{
> +    p->payload_len = 0;
> +}
> +
> +static void vmxnet3_txpkt_reset(Vmxnet3_TxPkt *p)
> +{
> +    memset(&p->mdata, 0, sizeof(p->mdata));
> +    vmxnet3_txpkt_set_num_pl_frags(p, 0);
> +    vmxnet3_txpkt_reset_payload(p);
> +    vmxnet3_txpkt_set_more_frags(p, 0);
> +    vmxnet3_txpkt_set_frag_off(p, 0);
> +    p->max_payload_len = 0;
> +
> +    if (NULL != p->vec) {
> +        p->vec[VMXNET3_TXPKT_L2HDR_FRAG].iov_len = 0;
> +        p->vec[VMXNET3_TXPKT_L3HDR_FRAG].iov_len = 0;
> +    }
> +}
> +
> +static bool
> +vmxnet3_txpkt_prealloc(Vmxnet3_TxPkt *p, uint32_t max_frags, bool has_virt_hdr)
> +{
> +    if (NULL != p->vec) {
> +        g_free(p->vec);
> +    }
> +
> +    p->vec =
> +        g_malloc(sizeof(*p->vec) * (max_frags + VMXNET3_TXPKT_PL_START_FRAG));
> +    if (NULL == p->vec) {
> +        return false;
> +    }
> +
> +    p->max_payload_frags = max_frags;
> +    p->has_virt_hdr = has_virt_hdr;
> +    p->vec[VMXNET3_TXPKT_VHDR_FRAG].iov_base =&p->virt_hdr;
> +    p->vec[VMXNET3_TXPKT_VHDR_FRAG].iov_len =
> +        p->has_virt_hdr ? sizeof(p->virt_hdr) : 0;
> +    p->vec[VMXNET3_TXPKT_L2HDR_FRAG].iov_base =&p->__l2_hdr;
> +    p->vec[VMXNET3_TXPKT_L3HDR_FRAG].iov_base =&p->__l3_hdr;
> +    vmxnet3_txpkt_reset(p);
> +    return true;
> +}
> +
> +static void vmxnet3_txpkt_init(Vmxnet3_TxPkt *p)
> +{
> +    p->vec = NULL;
> +}
> +
> +static void vmxnet3_txpkt_cleanup(Vmxnet3_TxPkt *p)
> +{
> +    g_free(p->vec);
> +}
> +
> +static void vmxnet3_txpkt_unmap(Vmxnet3_TxPkt *p, bool is_write)
> +{
> +    int i;
> +
> +    for (i = VMXNET3_TXPKT_PL_START_FRAG;
> +        i<  p->payload_frags + VMXNET3_TXPKT_PL_START_FRAG; i++) {
> +        cpu_physical_memory_unmap(p->vec[i].iov_base, p->vec[i].iov_len,
> +                                  is_write, p->vec[i].iov_len);
> +    }
> +}
> +
> +static void*
> +vmxnet3_txpkt_map(Vmxnet3_TxPkt *p, uint32_t *mapped_fragments, bool is_write)
> +{
> +    int i;
> +
> +    for (i = VMXNET3_TXPKT_PL_START_FRAG;
> +        i<  p->payload_frags + VMXNET3_TXPKT_PL_START_FRAG; i++) {
> +        target_phys_addr_t mapped_len = p->vec[i].iov_len;
> +        size_t orig_len = p->vec[i].iov_len;
> +        p->vec[i].iov_base =
> +            cpu_physical_memory_map((uint64_t) p->vec[i].iov_base,
> +&mapped_len, is_write);
> +        p->vec[i].iov_len = mapped_len;
> +
> +        if ((NULL == p->vec[i].iov_base) || (orig_len != mapped_len)) {
> +            p->payload_frags = i + !!p->vec[i].iov_base;
> +            vmxnet3_txpkt_unmap(p, is_write);
> +            return NULL;
> +        }
> +    }
> +
> +    *mapped_fragments = VMXNET3_TXPKT_PL_START_FRAG + p->payload_frags;
> +    return p->vec;
> +}
> +
> +static inline void
> +vmxnet3_txpkt_dump(Vmxnet3_TxPkt *p)
> +{
> +#ifdef DEBUG_VMXNET3_PACKETS
> +    Vmxnet3_TxPktMdata *m = vmxnet3_txpkt_get_mdata(p);
> +#endif
> +
> +    DPKPRINTF("TXPKT MDATA: om: %d, cso/gso_size: %d, hdr_len: %d, "
> +              "pkt_type: 0x%X, l2hdr_len: %lu l3hdr_len: %lu, payload_len: %u",
> +              m->offload_mode, m->cso_or_gso_size,
> +              m->hdr_length, m->packet_type,
> +              vmxnet3_txpkt_get_l2hdr_len(p),
> +              vmxnet3_txpkt_get_l3hdr_len(p),
> +              vmxnet3_txpkt_get_payload_len(p));
> +};
> +
> +/* RX packet may contain up to 2 fragments - rebuilt eth header */
> +/* in case of VLAN tag stripping                                */
> +/* and payload received from QEMU - in any case                 */
> +#define VMXNET3_MAX_RX_PACKET_FRAGMENTS (2)
> +
> +typedef struct _Vmxnet3_RxPkt {
> +    Vmxnet3_RxPktMdata mdata;
> +    struct virtio_net_hdr virt_hdr;
> +    struct eth_header eth_hdr;
> +    struct iovec vec[VMXNET3_MAX_RX_PACKET_FRAGMENTS];
> +    uint16 vec_len;
> +} Vmxnet3_RxPkt;
> +
> +#define vmxnet3_rxpkt_get_mdata(p)          (&((p)->mdata))
> +#define vmxnet3_rxpkt_get_ehdr(p)           (&((p)->eth_hdr))
> +#define vmxnet3_rxpkt_get_vhdr(p)           (&((p)->virt_hdr))
> +#define vmxnet3_rxpkt_get_frag(p, n)        (&((p)->vec[(n)]))
> +#define vmxnet3_rxpkt_set_num_frags(p, n)   ((p)->vec_len = (n))
> +#define vmxnet3_rxpkt_get_num_frags(p)      ((p)->vec_len)
> +
> +static inline void vmxnet3_rxpkt_attach_ehdr(Vmxnet3_RxPkt *p)
> +{
> +    vmxnet3_rxpkt_get_frag(p, 0)->iov_base =&p->eth_hdr;
> +    vmxnet3_rxpkt_get_frag(p, 0)->iov_len = sizeof(p->eth_hdr);
> +}
> +
> +static inline void vmxnet3_rxpkt_reset(Vmxnet3_RxPkt *p)
> +{
> +    memset(&p->mdata, 0, sizeof(p->mdata));
> +    memset(&p->virt_hdr, 0, sizeof(p->virt_hdr));
> +    vmxnet3_rxpkt_set_num_frags(p, 0);
> +}
> +
> +static void vmxnet3_rxpkt_init(Vmxnet3_RxPkt *p)
> +{
> +    vmxnet3_rxpkt_reset(p);
> +}
> +
> +static inline void
> +vmxnet3_rxpkt_dump(Vmxnet3_RxPkt *p)
> +{
> +#ifdef DEBUG_VMXNET3_PACKETS
> +    Vmxnet3_RxPktMdata *m = vmxnet3_rxpkt_get_mdata(p);
> +#endif
> +
> +    DPKPRINTF("RXPKT MDATA: tot_len: %d, pkt_type: 0x%X, "
> +              "vlan_stripped: %d, vlan_tag: %d, vhdr_valid: %d",
> +              m->tot_len, m->packet_type,
> +              m->vlan_stripped, m->vlan_tag, m->vhdr_valid);
> +};
> +
> +/* Cyclic ring abstraction */
> +typedef struct _Vmxnet3_Ring {
> +    target_phys_addr_t pa;
> +    size_t size;
> +    size_t cell_size;
> +    size_t next;
> +    uint8_t gen;
> +} Vmxnet3_Ring;

Watch the underscores and move the types all to the top of the file please.

<snip>

Please split this up to make it more reviewable.

> +
> +static void vmxnet3_class_init(ObjectClass *class, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(class);
> +    PCIDeviceClass *c = PCI_DEVICE_CLASS(class);
> +
> +    c->init = vmxnet3_pci_init;
> +    c->exit = vmxnet3_pci_uninit;
> +    c->romfile = "pxe-e1000.rom";

I assume this is a mistake.

> +    c->vendor_id = PCI_VENDOR_ID_VMWARE;
> +    c->device_id = PCI_DEVICE_ID_VMWARE_VMXNET3;
> +    c->revision = PCI_DEVICE_ID_VMWARE_VMXNET3_REVISION;
> +    c->class_id = PCI_CLASS_NETWORK_ETHERNET;
> +    c->subsystem_vendor_id = PCI_VENDOR_ID_VMWARE;
> +    c->subsystem_id = PCI_DEVICE_ID_VMWARE_VMXNET3;
> +#if defined(VMXNET3_ENABLE_MSI) || defined(VMXNET3_ENABLE_MSIX)
> +    c->config_write = vmxnet3_write_config,
> +#endif
> +    dc->desc = "VMWare Paravirtualized Ethernet v3";
> +    dc->reset = vmxnet3_qdev_reset;
> +    dc->vmsd =&vmstate_vmxnet3;
> +    dc->props = vmxnet3_properties;
> +}
> +
> +static TypeInfo vmxnet3_info = {
> +    .name          = "vmxnet3",
> +    .parent        = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(VMXNET3_State),
> +    .class_init    = vmxnet3_class_init,
> +};
> +
> +static void vmxnet3_register_types(void)
> +{
> +    DCBPRINTF("vmxnet3_register_types called...");
> +    type_register_static(&vmxnet3_info);
> +}
> +
> +type_init(vmxnet3_register_types)
> diff --git a/hw/vmxnet3.h b/hw/vmxnet3.h
> new file mode 100644
> index 0000000..6ec3fd5
> --- /dev/null
> +++ b/hw/vmxnet3.h
> @@ -0,0 +1,727 @@
> +/*
> + * QEMU VMWARE VMXNET3 paravirtual NIC
> + *
> + * Copyright (c) 2012 Ravello Systems LTD (http://ravellosystems.com)
> + *
> + * Developed by Daynix Computing LTD (http://www.daynix.com)
> + *
> + * Authors:
> + * Dmitry Fleytman<dmitry@daynix.com>
> + * Yan Vugenfirer<yan@daynix.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef _QEMU_VMXNET3_H
> +#define _QEMU_VMXNET3_H
> +
> +#define VMXNET3_DEVICE_MAX_TX_QUEUES 8
> +#define VMXNET3_DEVICE_MAX_RX_QUEUES 8   /* Keep this value as a power of 2 */
> +
> +/* Defines needed to integrate VMWARE headers */
> +#define u64     uint64_t
> +#define u32     uint32_t
> +#define u16     uint16_t
> +#define u8      uint8_t
> +#define __le16  uint16_t
> +#define __le32  uint32_t
> +#define __le64  uint64_t
> +#define __packed QEMU_PACKED
> +
> +#if defined(HOST_WORDS_BIGENDIAN)
> +#define const_cpu_to_le64(x) bswap_64(x)
> +#define __BIG_ENDIAN_BITFIELD
> +#else
> +#define const_cpu_to_le64(x) (x)
> +#endif
> +
> +/* Following is an interface definition for */
> +/* VMXNET3 device as provided by VMWARE     */
> +/* Original file and copyright is available */
> +/* in Linux kernel v3.2.8 at                */
> +/* drivers/net/vmxnet3/vmxnet3_defs.h       */

You need to include the original VMware copyrights in this file.  Ideally, we'd 
just import the original header directly.

Regards,

Anthony Liguori

  reply	other threads:[~2012-03-09 19:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-07 15:38 [Qemu-devel] [PATCH v3] VMXNET3 paravirtual NIC device implementation Dmitry Fleytman
2012-03-07 15:38 ` Dmitry Fleytman
2012-03-09 19:50   ` Anthony Liguori [this message]
2012-03-09  6:41 ` Gerhard Wiesinger
2012-03-09 18:04   ` Dmitry Fleytman
  -- strict thread matches above, loose matches on Subject: below --
2012-03-11 15:15 Dmitry Fleytman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F5A5F23.6030108@us.ibm.com \
    --to=aliguori@us.ibm.com \
    --cc=alex.fishman@ravellosystems.com \
    --cc=dmitry.fleytman@ravellosystems.com \
    --cc=izik.eidus@ravellosystems.com \
    --cc=lists@wiesinger.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=yan@ravellosystems.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).