[ipxe-devel] [PATCH 2/4] [virtio] Add virtio 1.0 constants and data structures

Ladi Prosek lprosek at redhat.com
Thu Mar 10 14:35:11 UTC 2016


On Thu, Mar 10, 2016 at 2:41 PM, Michael S. Tsirkin <mst at redhat.com> wrote:
> On Wed, Mar 09, 2016 at 07:20:01PM +0100, Ladi Prosek wrote:
>> Virtio 1.0 introduces new constants and data structures, common to
>> all devices as well as specific to virtio-net. This commit adds a
>> subset of these to be able to drive the virtio-net 1.0 network
>> device.
>>
>> Signed-off-by: Ladi Prosek <lprosek at redhat.com>
>> ---
>>  src/drivers/net/virtio-net.h   | 18 ++++++++++++++
>>  src/include/ipxe/virtio-pci.h  | 55 ++++++++++++++++++++++++++++++++++++++++++
>>  src/include/ipxe/virtio-ring.h |  8 ++++++
>>  3 files changed, 81 insertions(+)
>>
>> diff --git a/src/drivers/net/virtio-net.h b/src/drivers/net/virtio-net.h
>> index 3abef28..2615d65 100644
>> --- a/src/drivers/net/virtio-net.h
>> +++ b/src/drivers/net/virtio-net.h
>> @@ -15,6 +15,14 @@
>>  #define VIRTIO_NET_F_HOST_ECN   13      /* Host can handle TSO[6] w/ ECN in. */
>>  #define VIRTIO_NET_F_HOST_UFO   14      /* Host can handle UFO in. */
>>
>> +/* Virtio 1.0 feature bits for virtio net */
>> +#define VIRTIO_NET_F_MRG_RXBUF  15      /* Driver can merge receive buffers. */
>> +#define VIRTIO_NET_F_STATUS     16      /* Configuration status field is available. */
>> +#define VIRTIO_NET_F_CTRL_VQ    17      /* Control channel is available. */
>> +#define VIRTIO_NET_F_CTRL_RX    18      /* Control channel RX mode support. */
>> +#define VIRTIO_NET_F_CTRL_VLAN  19      /* Control channel VLAN filtering. */
>> +#define VIRTIO_NET_F_GUEST_ANNOUNCE 21  /* Driver can send gratuitous packets. */
>> +
>>  struct virtio_net_config
>>  {
>>     /* The config defining mac address (if VIRTIO_NET_F_MAC) */
>> @@ -41,4 +49,14 @@ struct virtio_net_hdr
>>     uint16_t csum_start;
>>     uint16_t csum_offset;
>>  };
>> +
>> +/* Virtio 1.0 version of the first element of the scatter-gather list. */
>> +struct virtio_net_hdr_modern
>> +{
>> +   struct virtio_net_hdr legacy;
>> +
>> +   /* Used only if VIRTIO_NET_F_MRG_RXBUF: */
>> +   uint16_t num_buffers;
>> +} __attribute__((packed));
>> +
>
> virtio_net_hdr is not packed, and I don't think this one
> should be.
>
> virtio is designed intentionally to avoid need for packed
> attribute, since gcc generates a lot of unnecessary code if
> you use it.

I'm using the attribute to make it clear that these structures are
part of the ABI and therefore the exact layout matters. The layout
would be the same with or without the attribute because, as you say,
virtio structures are designed such that fields are aligned naturally,
so I don't see why gcc would generate any different code.

I'll remove the attribute if you prefer but it was a conscious decision.

> Also, I think the opening "{" should be on same line
> with struct. virtio_net_hdr is not like this but most
> other structures are.

I did my best to follow the formatting conventions in each file I
touched. All structs in this file have the opening brace on its own
new line so I did it too.

>>  #endif /* _VIRTIO_NET_H_ */
>> diff --git a/src/include/ipxe/virtio-pci.h b/src/include/ipxe/virtio-pci.h
>> index a09c463..58c9bf2 100644
>> --- a/src/include/ipxe/virtio-pci.h
>> +++ b/src/include/ipxe/virtio-pci.h
>> @@ -37,6 +37,61 @@
>>  /* Virtio ABI version, this must match exactly */
>>  #define VIRTIO_PCI_ABI_VERSION          0
>>
>> +/* PCI capability types: */
>> +#define VIRTIO_PCI_CAP_COMMON_CFG       1  /* Common configuration */
>> +#define VIRTIO_PCI_CAP_NOTIFY_CFG       2  /* Notifications */
>> +#define VIRTIO_PCI_CAP_ISR_CFG          3  /* ISR access */
>> +#define VIRTIO_PCI_CAP_DEVICE_CFG       4  /* Device specific configuration */
>> +#define VIRTIO_PCI_CAP_PCI_CFG          5  /* PCI configuration access */
>> +
>> +#define __u8       uint8_t
>> +#define __le16     uint16_t
>> +#define __le32     uint32_t
>> +#define __le64     uint64_t
>> +
>> +/* This is the PCI capability header: */
>> +struct virtio_pci_cap {
>> +    __u8 cap_vndr;    /* Generic PCI field: PCI_CAP_ID_VNDR */
>> +    __u8 cap_next;    /* Generic PCI field: next ptr. */
>> +    __u8 cap_len;     /* Generic PCI field: capability length */
>> +    __u8 cfg_type;    /* Identifies the structure. */
>> +    __u8 bar;         /* Where to find it. */
>> +    __u8 padding[3];  /* Pad to full dword. */
>> +    __le32 offset;    /* Offset within bar. */
>> +    __le32 length;    /* Length of the structure, in bytes. */
>> +} __attribute__((packed));
>> +
>> +struct virtio_pci_notify_cap {
>> +    struct virtio_pci_cap cap;
>> +    __le32 notify_off_multiplier; /* Multiplier for queue_notify_off. */
>> +} __attribute__((packed));
>> +
>> +/* Fields in VIRTIO_PCI_CAP_COMMON_CFG: */
>> +struct virtio_pci_common_cfg {
>> +    /* About the whole device. */
>> +    __le32 device_feature_select; /* read-write */
>> +    __le32 device_feature;        /* read-only */
>> +    __le32 guest_feature_select;  /* read-write */
>> +    __le32 guest_feature;         /* read-write */
>> +    __le16 msix_config;           /* read-write */
>> +    __le16 num_queues;            /* read-only */
>> +    __u8 device_status;           /* read-write */
>> +    __u8 config_generation;       /* read-only */
>> +
>> +    /* About a specific virtqueue. */
>> +    __le16 queue_select;          /* read-write */
>> +    __le16 queue_size;            /* read-write, power of 2. */
>> +    __le16 queue_msix_vector;     /* read-write */
>> +    __le16 queue_enable;          /* read-write */
>> +    __le16 queue_notify_off;      /* read-only */
>> +    __le32 queue_desc_lo;         /* read-write */
>> +    __le32 queue_desc_hi;         /* read-write */
>> +    __le32 queue_avail_lo;        /* read-write */
>> +    __le32 queue_avail_hi;        /* read-write */
>> +    __le32 queue_used_lo;         /* read-write */
>> +    __le32 queue_used_hi;         /* read-write */
>> +} __attribute__((packed));
>> +
>>  static inline u32 vp_get_features(unsigned int ioaddr)
>>  {
>>     return inl(ioaddr + VIRTIO_PCI_HOST_FEATURES);
>
> You do not really have to pack these,
> they are designed to work correctly without
> the packed attribute.
>
> But this is never used on data path so if you prefer
> it here, ok.
>
>> diff --git a/src/include/ipxe/virtio-ring.h b/src/include/ipxe/virtio-ring.h
>> index c687aca..e44d13c 100644
>> --- a/src/include/ipxe/virtio-ring.h
>> +++ b/src/include/ipxe/virtio-ring.h
>> @@ -8,9 +8,17 @@
>>  #define VIRTIO_CONFIG_S_DRIVER          2
>>  /* Driver has used its parts of the config, and is happy */
>>  #define VIRTIO_CONFIG_S_DRIVER_OK       4
>> +/* Driver has finished configuring features */
>> +#define VIRTIO_CONFIG_S_FEATURES_OK     8
>>  /* We've given up on this device. */
>>  #define VIRTIO_CONFIG_S_FAILED          0x80
>>
>> +/* Virtio feature flags used to negotiate device and driver features. */
>> +/* Can the device handle any descriptor layout? */
>> +#define VIRTIO_F_ANY_LAYOUT             27
>> +/* v1.0 compliant. */
>> +#define VIRTIO_F_VERSION_1              32
>> +
>>  #define MAX_QUEUE_NUM      (256)
>>
>>  #define VRING_DESC_F_NEXT  1
>> --
>> 2.5.0



More information about the ipxe-devel mailing list