Re: [PATCH] blkif: add indirect descriptors interface to public headers

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

> > I must be really thick here but I am not seeing it. Could you explain to me
> > exactly why we would not get the same size?
> > 
> 
> Maybe I'm misunderstanding this but by my reading this section:
> 
> uint8_t        indirect_op;
> uint16_t       nr_segments;
> #ifdef CONFIG_X86_64
> uint32_t       _pad1;        /* offsetof(blkif_...,u.indirect.id) == 8 */
> #endif
> uint64_t       id;
> 
> would be 11 bytes long in a 32-bit build and 15 bytes long in a 64-bit build (as it's part of a packed struct).

Damm! I somehow thought we had it done _Right_ so it would be the same
exact and offset on both 64-bit and 32-bit. ARGH!

konrad@phenom:/tmp$ ./blk-interface-64
RING TOTAL SIZE         2368
Each entry              64
Each request            60
64-bit
op:0, indirect_op=1, nr_segment=2,id=8 sector_number=16
gref[0] == (24)
gref[1] == (28)
gref[2] == (32)
gref[3] == (36)
gref[4] == (40)
gref[5] == (44)
gref[6] == (48)
gref[7] == (52)
konrad@phenom:/tmp$ ./blk-interface-32
konrad@phenom:/tmp$ ./blk-interface
RING TOTAL SIZE         1792
Each entry              48
Each request            48
32-bit
op:0, indirect_op=1, nr_segment=2,id=4 sector_number=8
gref[0] == (12)
gref[1] == (16)
gref[2] == (20)
gref[3] == (24)
gref[4] == (28)
gref[5] == (32)
gref[6] == (36)
gref[7] == (40)

Attached is the simple test program.

#include <stdio.h>
#include <sys/types.h>

#include <stddef.h>

/* These are only 64-bit defined */
typedef __signed__ char __s8;
typedef unsigned char __u8;

typedef __signed__ short __s16;
typedef unsigned short __u16;

typedef __signed__ int __s32;
typedef unsigned int __u32;

typedef __signed__ long __s64;
typedef unsigned long __u64;

typedef		__u8		uint8_t;
typedef		__u16		uint16_t;
typedef		__u32		uint32_t;

#if defined(__GNUC__)
typedef		__u64		uint64_t;
//typedef		__u64		u_int64_t;
//typedef		__s64		int64_t;
#endif

typedef uint16_t blkif_vdev_t;
typedef uint64_t blkif_sector_t;
typedef uint32_t grant_ref_t;

/*
 * Maximum scatter/gather segments per request.
 * This is carefully chosen so that sizeof(struct blkif_ring) <= PAGE_SIZE.
 * NB. This could be 12 if the ring indexes weren't stored in the same page.
 */
#define BLKIF_MAX_SEGMENTS_PER_REQUEST 11


//#define CONFIG_X86_64 1

struct blkif_request_rw {
	uint8_t        nr_segments;  /* number of segments                   */
	blkif_vdev_t   handle;       /* only for read/write requests         */
#ifdef CONFIG_X86_64
	uint32_t       _pad1;	     /* offsetof(blkif_request,u.rw.id) == 8 */
#endif
	uint64_t       id;           /* private guest value, echoed in resp  */
	blkif_sector_t sector_number;/* start sector idx on disk (r/w only)  */
	struct blkif_request_segment {
		grant_ref_t gref;        /* reference to I/O buffer frame        */
		/* @first_sect: first sector in frame to transfer (inclusive).   */
		/* @last_sect: last sector in frame to transfer (inclusive).     */
		uint8_t     first_sect, last_sect;
	} seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
} __attribute__((__packed__));

#define BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST 8


/* so 64 bytes under 64-bit  */
struct blkif_request_indirect {
	uint8_t        indirect_op;  /* BLKIF_OP_* (usually READ or WRITE    */	// 1
	uint16_t       nr_segments;
#ifdef CONFIG_X86_64
	uint32_t       _pad1;	     /* offsetof(blkif_request,u.rw.id) == 8 */ // 2
#endif
	uint64_t       id;           /* private guest value, echoed in resp  */
	blkif_sector_t sector_number;
	grant_ref_t    indirect_grefs[BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST];
#ifdef CONFIG_X86_64
        uint32_t      _pad3;         /* make it 64 byte aligned */
#else
        uint64_t      _pad3;         /* make it 64 byte aligned */
#endif

} __attribute__((__packed__));


struct blkif_request {
	uint8_t        operation;    /* BLKIF_OP_???                         */
	union {
		/* struct blkif_request_rw rw; */
		struct blkif_request_indirect indirect;
	} u;
} __attribute__((__packed__));

struct blkif_response {
	uint64_t        id;              /* copied from request */
	uint8_t         operation;       /* copied from request */
	int16_t         status;          /* BLKIF_RSP_???       */
};
typedef unsigned int RING_IDX; /* 4 bytes */

union blkif_sring_entry {
	struct blkif_request req;
	struct blkif_response rsp;
};

struct blkif_ring {
	/* 8 */
    RING_IDX req_prod, req_event; /* #1 front writest req_prod,
				 blkback updates req_event by interal reqs_cons consumed + 1 */
	/* 8 */
    RING_IDX rsp_prod, rsp_event; /* #2 blkback writes rsp_prod, and #4 blkfront updates rsp_event
			by its internal rsp_cons + 1 */
    uint8_t  pad[48];
	/* 48 + 16 = 64 */
    union blkif_sring_entry ring[36]; /* variable-length */
};

void main(void)
{
	int i;
	printf("RING TOTAL SIZE\t\t%d\n", sizeof(struct blkif_ring));
	printf("Each entry\t\t%d\n", sizeof(union blkif_sring_entry));
	printf("Each request\t\t%d\n", sizeof(struct blkif_request));
	printf("%s\n", 
#ifdef CONFIG_X86_64
		"64-bit"
#else
		"32-bit"
#endif
	);

	printf("op:%d, indirect_op=%d, nr_segment=%d,id=%d sector_number=%d\n",
		offsetof(struct blkif_request, operation),
		offsetof(struct blkif_request, u.indirect.indirect_op),
		offsetof(struct blkif_request, u.indirect.nr_segments),
		offsetof(struct blkif_request, u.indirect.id),
		offsetof(struct blkif_request, u.indirect.sector_number));

	for (i = 0; i < BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST; i++) {
		printf("gref[%d] == (%d)\n", i, offsetof(struct blkif_request, u.indirect.indirect_grefs[i]));
	}
	return;
}