Looks like you forgot to CC previous revierers.
> +#define CHECK_IOCTL_IN(req) \
> +do { \
> + if ((req)->Hdr.cbIn != (sizeof((req)->Hdr) + sizeof((req)->u.In)) || \
> + (req)->Hdr.cbOut != sizeof((req)->Hdr)) \
> + return -EINVAL; \
> +} while (0)
It seems like you ignored the comments on the last version.
Get rid of the weird struct capilization.
Make these things functions instead of macros.
> + VMMDevReqHypervisorInfo *req;
> + void *guest_mappings[GUEST_MAPPINGS_TRIES];
Fix your structure names.
> + for (i = 0; i < (size >> PAGE_SHIFT); i++)
Remove pointless inner braces.
> +#ifdef CONFIG_X86_64
> + req1->guestInfo.osType = VBOXOSTYPE_Linux26_x64;
> +#else
> + req1->guestInfo.osType = VBOXOSTYPE_Linux26;
> +#endif
Hardcoding architecvtures is almost always a bug. If you think it
isn't here it needs a big fat comment explaining that rationale.
> + req->guestStatus.status = active ? VBoxGuestFacilityStatus_Active :
> + VBoxGuestFacilityStatus_Inactive;
Please use if/else for readability.
> +/** @name Memory Ballooning
> + * @{
> + */
Please get rid of this whacky comment style.
> +
> +/**
> + * Inflate the balloon by one chunk.
> + *
> + * The caller owns the balloon mutex.
> + *
> + * @returns 0 or negative errno value.
> + * @param gdev The Guest extension device.
> + * @param chunk_idx Index of the chunk.
> + */
An this one isn't proper kerneldoc either.
> + pages = kmalloc(sizeof(*pages) * VMMDEV_MEMORY_BALLOON_CHUNK_PAGES,
> +/**
> + * Callback for heartbeat timer.
> + */
/**
as the comment start is reserved for kerneldoc comments, this one
isn't a kerneldoc comment, so please remove it.
> +static void vbg_heartbeat_timer(unsigned long data)
> +{
> + struct vbg_dev *gdev = (struct vbg_dev *)data;
> +
> + vbg_req_perform(gdev, gdev->guest_heartbeat_req);
> + mod_timer(&gdev->heartbeat_timer,
> + msecs_to_jiffies(gdev->heartbeat_interval_ms));
> +}
Please use timer_setup and from_timer instead of the data argument.
(this is new in -rc3).
> + list_add(&session->list_node, &gdev->session_list);
> + spin_unlock_irqrestore(&gdev->session_spinlock, flags);
Session_list isn't used anywhere. And if it was it would probably
buggy due to the lack of reference counting.
> +
> + *session_ret = session;
> +
> + return 0;
Just return the session or an ERR_PTR.
> + guest_status = (const VMMDevReportGuestStatus *)req;
struct pointer asts are a ba idea - this probably should use
container_of magic or a more generic strucure with unioned elements.
> +int vbg_status_code_to_errno(int rc)
> +{
> + if (rc >= 0)
> + return 0;
> +
> + switch (rc) {
> + case VERR_ACCESS_DENIED: return -EPERM;
Please never put code in the same line as the switch statement.
Also in general code like this is better done by looping over a lookup
table.
Anyway, this was a super shallow review that everyone could do.
The actual ioctls and pv hardware interfaces look even worse, but
such a messy giant blob they aren't reviewable. This patch needs
a major cleanup pass first, and then a split into reviewable chunks.
