Re: [RFC v3 3/5] KVM: implement wire protocol

Stefan Hajnoczi
2021-02-24T11:06:50Z

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

On Sun, Feb 21, 2021 at 03:04:39PM +0300, Elena Afanasova wrote:
> +static bool
> +pack_cmd(struct ioregionfd_cmd *cmd, u64 offset, u64 len, u8 opt, u8 resp,
> + u64 user_data, const void *val)
> +{
> + switch (len) {
> + case 0:
> + break;

The 0 case might be non-obvious. A comment would be nice:

/* FAST_MMIO does not specify a length */

> + case 1:
> + cmd->size_exponent = IOREGIONFD_SIZE_8BIT;
> + break;
> + case 2:
> + cmd->size_exponent = IOREGIONFD_SIZE_16BIT;
> + break;
> + case 4:
> + cmd->size_exponent = IOREGIONFD_SIZE_32BIT;
> + break;
> + case 8:
> + cmd->size_exponent = IOREGIONFD_SIZE_64BIT;
> + break;
> + default:
> + return false;
> + }
> +
> + if (val)
> + memcpy(&cmd->data, val, len);
> + cmd->user_data = user_data;
> + cmd->offset = offset;
> + cmd->cmd = opt;
> + cmd->resp = resp;
> +
> + return true;
> +}
> +
> +enum {

A comment would help explain why the enum is needed:

/* A way to remember our state when interrupted by a signal */

> + SEND_CMD,
> + GET_REPLY,
> + COMPLETE
> +};

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]