Re: [Xen-devel] [PATCH 4/4] xen: Add V4V implementation

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.09.12 at 20:09, Jean Guyader <jean.guyader@xxxxxxxxxx> wrote:
>+typedef struct v4v_iov
>+{
>+    uint64_t iov_base;
>+    uint32_t iov_len;
>+} v4v_iov_t;

Lost padding here?

>+struct v4v_ring
>+{
>+    uint64_t magic;
>+    v4v_ring_id_t id;
>+    uint32_t len;
>+    uint32_t rx_ptr;
>+    uint32_t tx_ptr;
>+    uint8_t reserved[32];
>+    uint8_t ring[0];
>+};

Zero-sized arrays are not standard C (not even C11 iirc), this is
purely a GNU extension. This appears again elsewhere in this file.

>+typedef struct v4v_ring_data_ent
>+{
>+    v4v_addr_t ring;
>+    uint16_t flags;
>+    uint16_t pad;
>+    uint32_t space_required;
>+    uint32_t max_message_size;
>+} v4v_ring_data_ent_t;

For me, this sums up to 20 bytes.

>+typedef struct v4v_ring_data
>+{
>+    uint64_t magic;
>+    uint32_t nent;
>+    uint32_t pad;
>+    uint64_t reserved[4];
>+    v4v_ring_data_ent_t data[0];
>+} v4v_ring_data_t;

Consequently, sizeof(v4v_ring_data_t) will differ for 32- and
64-bit x86 guests. I can't tell whether that would matter
though (due to the intended, but again wrongly done,
trailing variable size array).

>+ * V4VOP_register_ring

This conflicts with ...

>+ *
>+ * Registers a ring with Xen, if a ring with the same v4v_ring_id exists,
>+ * this ring takes its place, registration will not change tx_ptr
>+ * unless it is invalid
>+ *
>+ * do_v4v_op(V4VOP_unregister_ring,

... this.

>+ *           v4v_ring, XEN_GUEST_HANDLE(xen_pfn_t),

Missing indirection here? I doubt you want a structure passed
by value... (Again reoccurs further down.)

>+ * V4VOP_unregister_ring

Again conflicting with ...

>+ *
>+ * Unregister a ring.
>+ *
>+ * do_v4v_op(V4VOP_send, v4v_ring, NULL, 0, 0)

... this.

>+ * do_v4v_op(V4VOP_send,
>+ *           v4v_send_addr_t addr,
>+ *           void* buf,

XEN_GUEST_HANDLE(void) or, given this is a send, probably rather
XEN_GUEST_HANDLE(const_void). But then again this would
conflict with the real do_v4v_op() declaration.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel