Re: [PATCH resend v2 3/3] virt: Add vboxguest driver for Virtual Box Guest integration

[Hans de Goede]

Hi,

On 27-11-17 20:46, Larry Finger wrote:
> On 11/26/2017 09:12 AM, Hans de Goede wrote:
> > This commit adds a driver for the Virtual Box Guest PCI device used in
> > Virtual Box virtual machines. Enabling this driver will add support for
> > Virtual Box Guest integration features such as copy-and-paste, seamless
> > mode and OpenGL pass-through.
> >
> > This driver also offers vboxguest IPC functionality which is needed
> > for the vboxfs driver which offers folder sharing support.
> >
> > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>
> Reviewed-by: Larry Finger <Larry.Finger.net>
>
>
> Smatch lists the following:
>
>  Â CHECKÂÂ drivers/virt/vboxguest/vboxguest_core.c
> drivers/virt/vboxguest/vboxguest_core.c:604 vbg_set_session_event_filter() error: we previously assumed 'req' could be null (see line 585)
> drivers/virt/vboxguest/vboxguest_core.c:606 vbg_set_session_event_filter() warn: variable dereferenced before check 'req' (see line 604)
> drivers/virt/vboxguest/vboxguest_core.c:698 vbg_set_session_capabilities() error: we previously assumed 'req' could be null (see line 679)
> drivers/virt/vboxguest/vboxguest_core.c:700 vbg_set_session_capabilities() warn: variable dereferenced before check 'req' (see line 698)

Good idea to run smatch, thank you for catching these.

> vbox_utils.c is clean.
>
> The reasons for the above errors, and other comments inline below.
>
> > ---
> > Changes in v2:
> > -Change all uapi headers to kernel coding style: Drop struct and enum typedefs
> > Â make type and struct-member names all lowercase, enum values all uppercase.
> > -Remove unused struct type declarations from some headers (shaving of another
> > Â 1000 lines)
> > -Remove or fixup doxygen style comments
> > -Get rid of CHECK macros, use a function taking in_ and out_size args instead
> > -Some other small codyingstyle fixes
> > -Split into multiple patches
> > ---
> > Â drivers/virt/KconfigÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂÂ 1 +
> > Â drivers/virt/MakefileÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂÂ 1 +
> > Â drivers/virt/vboxguest/KconfigÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂ 16 +
> > Â drivers/virt/vboxguest/MakefileÂÂÂÂÂÂÂÂÂÂÂ |ÂÂÂ 3 +
> > Â drivers/virt/vboxguest/vboxguest_core.cÂÂÂ | 1577 ++++++++++++++++++++++++++++
> > Â drivers/virt/vboxguest/vboxguest_core.hÂÂÂ |Â 187 ++++
> > Â drivers/virt/vboxguest/vboxguest_linux.cÂÂ |Â 469 +++++++++
> > Â drivers/virt/vboxguest/vboxguest_version.h |ÂÂ 18 +
> > Â 8 files changed, 2272 insertions(+)
> > Â create mode 100644 drivers/virt/vboxguest/Kconfig
> > Â create mode 100644 drivers/virt/vboxguest/Makefile
> > Â create mode 100644 drivers/virt/vboxguest/vboxguest_core.c
> > Â create mode 100644 drivers/virt/vboxguest/vboxguest_core.h
> > Â create mode 100644 drivers/virt/vboxguest/vboxguest_linux.c
> > Â create mode 100644 drivers/virt/vboxguest/vboxguest_version.h
> >
> > diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig
> > index 99ebdde590f8..8d9cdfbd6bcc 100644
> > --- a/drivers/virt/Kconfig
> > +++ b/drivers/virt/Kconfig
> > @@ -30,4 +30,5 @@ config FSL_HV_MANAGER
> > ÂÂÂÂÂÂÂÂÂÂÂ 4) A kernel interface for receiving callbacks when a managed
> > ÂÂÂÂÂÂÂÂÂÂ partition shuts down.
> > +source "drivers/virt/vboxguest/Kconfig"
> > Â endif
> > diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile
> > index c47f04dd343b..d3f7b2540890 100644
> > --- a/drivers/virt/Makefile
> > +++ b/drivers/virt/Makefile
> > @@ -3,3 +3,4 @@
> > Â #
> > Â obj-$(CONFIG_FSL_HV_MANAGER)ÂÂÂ += fsl_hypervisor.o
> > +obj-yÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ += vboxguest/
> > diff --git a/drivers/virt/vboxguest/Kconfig b/drivers/virt/vboxguest/Kconfig
> > new file mode 100644
> > index 000000000000..e88ee46c31d4
> > --- /dev/null
> > +++ b/drivers/virt/vboxguest/Kconfig
> > @@ -0,0 +1,16 @@
> > +config VBOXGUEST
> > +ÂÂÂ tristate "Virtual Box Guest integration support"
> > +ÂÂÂ depends on X86 && PCI && INPUT
> > +ÂÂÂ help
> > +ÂÂÂÂÂ This is a driver for the Virtual Box Guest PCI device used in
> > +ÂÂÂÂÂ Virtual Box virtual machines. Enabling this driver will add
> > +ÂÂÂÂÂ support for Virtual Box Guest integration features such as
> > +ÂÂÂÂÂ copy-and-paste, seamless mode and OpenGL pass-through.
> > +
> > +ÂÂÂÂÂ This driver also offers vboxguest IPC functionality which is needed
> > +ÂÂÂÂÂ for the vboxfs driver which offers folder sharing support.
> > +
> > +ÂÂÂÂÂ Although it is possible to build this module in, it is advised
> > +ÂÂÂÂÂ to build this driver as a module, so that it can be updated
> > +ÂÂÂÂÂ independently of the kernel. Select M to build this driver as a
> > +ÂÂÂÂÂ module.
>
> This Kconfig allows vboxguest to be built even though vboxvideo is not being built. That does not seem to be a useful combination.

As discussed in the cover-letter thread, I agree this is not useful, but
adding a "depends on" is also not really the answer, so I've added this
line to the help section:

	  If you enable this driver you should also enable the VBOXVIDEO option.


> > diff --git a/drivers/virt/vboxguest/Makefile b/drivers/virt/vboxguest/Makefile
> > new file mode 100644
> > index 000000000000..203b8f465817
> > --- /dev/null
> > +++ b/drivers/virt/vboxguest/Makefile
> > @@ -0,0 +1,3 @@
> > +vboxguest-y := vboxguest_linux.o vboxguest_core.o vboxguest_utils.o
> > +
> > +obj-$(CONFIG_VBOXGUEST) += vboxguest.o
> > diff --git a/drivers/virt/vboxguest/vboxguest_core.c b/drivers/virt/vboxguest/vboxguest_core.c
> > new file mode 100644
> > index 000000000000..4927c0d3e336
> > --- /dev/null
> > +++ b/drivers/virt/vboxguest/vboxguest_core.c
> > @@ -0,0 +1,1577 @@
> > +/*
> > + * vboxguest core guest-device handling code, VBoxGuest.cpp in upstream svn.
> > + *
> > + * Copyright (C) 2007-2016 Oracle Corporation
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * The contents of this file may alternatively be used under the terms
> > + * of the Common Development and Distribution License Version 1.0
> > + * (CDDL) only, in which case the provisions of the CDDL are applicable
> > + * instead of those of the GPL.
> > + *
> > + * You may elect to license modified versions of this file under the
> > + * terms and conditions of either the GPL or the CDDL or both.
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/mm.h>
> > +#include <linux/sched.h>
> > +#include <linux/sizes.h>
> > +#include <linux/slab.h>
> > +#include <linux/vbox_err.h>
> > +#include <linux/vbox_utils.h>
> > +#include <linux/vmalloc.h>
> > +#include "vboxguest_core.h"
> > +#include "vboxguest_version.h"
> > +
> > +/* Get the pointer to the first HGCM parameter. */
> > +#define VBG_IOCTL_HGCM_CALL_PARMS(a) \
> > +ÂÂÂ ((struct vmmdev_hgcm_function_parameter *)( \
> > +ÂÂÂÂÂÂÂ (u8 *)(a) + sizeof(struct vbg_ioctl_hgcm_call)))
> > +/* Get the pointer to the first HGCM parameter in a 32-bit request. */
> > +#define VBG_IOCTL_HGCM_CALL_PARMS32(a) \
> > +ÂÂÂ ((struct vmmdev_hgcm_function_parameter32 *)( \
> > +ÂÂÂÂÂÂÂ (u8 *)(a) + sizeof(struct vbg_ioctl_hgcm_call)))
> > +
> > +#define GUEST_MAPPINGS_TRIESÂÂÂ 5
> > +
> > +/**
> > + * Reserves memory in which the VMM can relocate any guest mappings
> > + * that are floating around.
> > + *
> > + * This operation is a little bit tricky since the VMM might not accept
> > + * just any address because of address clashes between the three contexts
> > + * it operates in, so we try several times.
> > + *
> > + * Failure to reserve the guest mappings is ignored.
> > + *
> > + * @gdev:ÂÂÂÂÂÂÂ The Guest extension device.
> > + */
> > +static void vbg_guest_mappings_init(struct vbg_dev *gdev)
> > +{
> > +ÂÂÂ struct vmmdev_hypervisorinfo *req;
> > +ÂÂÂ void *guest_mappings[GUEST_MAPPINGS_TRIES];
> > +ÂÂÂ struct page **pages = NULL;
> > +ÂÂÂ u32 size, hypervisor_size;
> > +ÂÂÂ int i, rc;
> > +
> > +ÂÂÂ /* Query the required space. */
> > +ÂÂÂ req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_GET_HYPERVISOR_INFO);
> > +ÂÂÂ if (!req)
> > +ÂÂÂÂÂÂÂ return;
> > +
> > +ÂÂÂ req->hypervisor_start = 0;
> > +ÂÂÂ req->hypervisor_size = 0;
> > +ÂÂÂ rc = vbg_req_perform(gdev, req);
> > +ÂÂÂ if (rc < 0)
> > +ÂÂÂÂÂÂÂ goto out;
> > +
> > +ÂÂÂ /*
> > +ÂÂÂÂ * The VMM will report back if there is nothing it wants to map, like
> > +ÂÂÂÂ * for instance in VT-x and AMD-V mode.
> > +ÂÂÂÂ */
> > +ÂÂÂ if (req->hypervisor_size == 0)
> > +ÂÂÂÂÂÂÂ goto out;
> > +
> > +ÂÂÂ hypervisor_size = req->hypervisor_size;
> > +ÂÂÂ /* Add 4M so that we can align the vmap to 4MiB as the host requires. */
> > +ÂÂÂ size = PAGE_ALIGN(req->hypervisor_size) + SZ_4M;
> > +
> > +ÂÂÂ pages = kmalloc(sizeof(*pages) * (size >> PAGE_SHIFT), GFP_KERNEL);
> > +ÂÂÂ if (!pages)
> > +ÂÂÂÂÂÂÂ goto out;
> > +
> > +ÂÂÂ gdev->guest_mappings_dummy_page = alloc_page(GFP_HIGHUSER);
> > +ÂÂÂ if (!gdev->guest_mappings_dummy_page)
> > +ÂÂÂÂÂÂÂ goto out;
> > +
> > +ÂÂÂ for (i = 0; i < (size >> PAGE_SHIFT); i++)
> > +ÂÂÂÂÂÂÂ pages[i] = gdev->guest_mappings_dummy_page;
> > +
> > +ÂÂÂ /* Try several times, the host can be picky about certain addresses. */
>
> This comment should repeat that address clashes could be due to the different contexts. When I read this the first time, I had missed the comment above, and I expect that other readers might make the same mistake.

Ok, fixed for v3.

> > +ÂÂÂ for (i = 0; i < GUEST_MAPPINGS_TRIES; i++) {
> > +ÂÂÂÂÂÂÂ guest_mappings[i] = vmap(pages, (size >> PAGE_SHIFT),
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ VM_MAP, PAGE_KERNEL_RO);
> > +ÂÂÂÂÂÂÂ if (!guest_mappings[i])
> > +ÂÂÂÂÂÂÂÂÂÂÂ break;
> > +
> > +ÂÂÂÂÂÂÂ req->header.request_type = VMMDEVREQ_SET_HYPERVISOR_INFO;
> > +ÂÂÂÂÂÂÂ req->header.rc = VERR_INTERNAL_ERROR;
> > +ÂÂÂÂÂÂÂ req->hypervisor_size = hypervisor_size;
> > +ÂÂÂÂÂÂÂ req->hypervisor_start =
> > +ÂÂÂÂÂÂÂÂÂÂÂ (unsigned long)PTR_ALIGN(guest_mappings[i], SZ_4M);
> > +
> > +ÂÂÂÂÂÂÂ rc = vbg_req_perform(gdev, req);
> > +ÂÂÂÂÂÂÂ if (rc >= 0) {
> > +ÂÂÂÂÂÂÂÂÂÂÂ gdev->guest_mappings = guest_mappings[i];
> > +ÂÂÂÂÂÂÂÂÂÂÂ break;
> > +ÂÂÂÂÂÂÂ }
> > +ÂÂÂ }
> > +
> > +ÂÂÂ /* Free vmap's from failed attempts. */
> > +ÂÂÂ while (--i >= 0)
> > +ÂÂÂÂÂÂÂ vunmap(guest_mappings[i]);
> > +
> > +ÂÂÂ /* On failure free the dummy-page backing the vmap */
> > +ÂÂÂ if (!gdev->guest_mappings) {
> > +ÂÂÂÂÂÂÂ __free_page(gdev->guest_mappings_dummy_page);
> > +ÂÂÂÂÂÂÂ gdev->guest_mappings_dummy_page = NULL;
> > +ÂÂÂ }
> > +
> > +out:
> > +ÂÂÂ kfree(req);
> > +ÂÂÂ kfree(pages);
> > +}
> > +
> > +/**
> > + * Undo what vbg_guest_mappings_init did.
> > + *
> > + * @gdev:ÂÂÂÂÂÂÂ The Guest extension device.
> > + */
> > +static void vbg_guest_mappings_exit(struct vbg_dev *gdev)
> > +{
> > +ÂÂÂ struct vmmdev_hypervisorinfo *req;
> > +ÂÂÂ int rc;
> > +
> > +ÂÂÂ if (!gdev->guest_mappings)
> > +ÂÂÂÂÂÂÂ return;
> > +
> > +ÂÂÂ /*
> > +ÂÂÂÂ * Tell the host that we're going to free the memory we reserved for
> > +ÂÂÂÂ * it, the free it up. (Leak the memory if anything goes wrong here.)
> > +ÂÂÂÂ */
> > +ÂÂÂ req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_SET_HYPERVISOR_INFO);
> > +ÂÂÂ if (!req)
> > +ÂÂÂÂÂÂÂ return;
> > +
> > +ÂÂÂ req->hypervisor_start = 0;
> > +ÂÂÂ req->hypervisor_size = 0;
> > +
> > +ÂÂÂ rc = vbg_req_perform(gdev, req);
> > +
> > +ÂÂÂ kfree(req);
> > +
> > +ÂÂÂ if (rc < 0) {
> > +ÂÂÂÂÂÂÂ vbg_err("%s error: %d\n", __func__, rc);
> > +ÂÂÂÂÂÂÂ return;
> > +ÂÂÂ }
> > +
> > +ÂÂÂ vunmap(gdev->guest_mappings);
> > +ÂÂÂ gdev->guest_mappings = NULL;
> > +
> > +ÂÂÂ __free_page(gdev->guest_mappings_dummy_page);
> > +ÂÂÂ gdev->guest_mappings_dummy_page = NULL;
> > +}
> > +
> > +/**
> > + * Report the guest information to the host.
> > + * Return: 0 or negative errno value.
> > + * @gdev:ÂÂÂÂÂÂÂ The Guest extension device.
> > + */
> > +static int vbg_report_guest_info(struct vbg_dev *gdev)
> > +{
> > +ÂÂÂ /*
> > +ÂÂÂÂ * Allocate and fill in the two guest info reports.
> > +ÂÂÂÂ */
> > +ÂÂÂ struct vmmdev_guest_info *req1 = NULL;
> > +ÂÂÂ struct vmmdev_guest_info2 *req2 = NULL;
> > +ÂÂÂ int rc, ret = -ENOMEM;
> > +
> > +ÂÂÂ req1 = vbg_req_alloc(sizeof(*req1), VMMDEVREQ_REPORT_GUEST_INFO);
> > +ÂÂÂ req2 = vbg_req_alloc(sizeof(*req2), VMMDEVREQ_REPORT_GUEST_INFO2);
> > +ÂÂÂ if (!req1 || !req2)
> > +ÂÂÂÂÂÂÂ goto out_free;
> > +
> > +ÂÂÂ req1->interface_version = VMMDEV_VERSION;
> > +ÂÂÂ req1->os_type = VMMDEV_OSTYPE_LINUX26;
> > +#if __BITS_PER_LONG == 64
> > +ÂÂÂ req1->os_type |= VMMDEV_OSTYPE_X64;
> > +#endif
> > +
> > +ÂÂÂ req2->additions_major = VBG_VERSION_MAJOR;
> > +ÂÂÂ req2->additions_minor = VBG_VERSION_MINOR;
> > +ÂÂÂ req2->additions_build = VBG_VERSION_BUILD;
> > +ÂÂÂ req2->additions_revision = VBG_SVN_REV;
> > +ÂÂÂ /* (no features defined yet) */
> > +ÂÂÂ req2->additions_features = 0;
> > +ÂÂÂ strlcpy(req2->name, VBG_VERSION_STRING,
> > +ÂÂÂÂÂÂÂ sizeof(req2->name));
> > +
> > +ÂÂÂ /*
> > +ÂÂÂÂ * There are two protocols here:
> > +ÂÂÂÂ *ÂÂÂÂÂ 1. INFO2 + INFO1. Supported by >=3.2.51.
> > +ÂÂÂÂ *ÂÂÂÂÂ 2. INFO1 and optionally INFO2. The old protocol.
> > +ÂÂÂÂ *
> > + * We try protocol 2 first. It will fail with VERR_NOT_SUPPORTED
> > +ÂÂÂÂ * if not supported by the VMMDev (message ordering requirement).
> > +ÂÂÂÂ */
> > +ÂÂÂ rc = vbg_req_perform(gdev, req2);
> > +ÂÂÂ if (rc >= 0) {
> > +ÂÂÂÂÂÂÂ rc = vbg_req_perform(gdev, req1);
> > +ÂÂÂ } else if (rc == VERR_NOT_SUPPORTED || rc == VERR_NOT_IMPLEMENTED) {
> > +ÂÂÂÂÂÂÂ rc = vbg_req_perform(gdev, req1);
> > +ÂÂÂÂÂÂÂ if (rc >= 0) {
> > +ÂÂÂÂÂÂÂÂÂÂÂ rc = vbg_req_perform(gdev, req2);
> > +ÂÂÂÂÂÂÂÂÂÂÂ if (rc == VERR_NOT_IMPLEMENTED)
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ rc = VINF_SUCCESS;
> > +ÂÂÂÂÂÂÂ }
> > +ÂÂÂ }
> > +ÂÂÂ ret = vbg_status_code_to_errno(rc);
> > +
> > +out_free:
> > +ÂÂÂ kfree(req2);
> > +ÂÂÂ kfree(req1);
> > +ÂÂÂ return ret;
> > +}
> > +
> > +/**
> > + * Report the guest driver status to the host.
> > + * Return: 0 or negative errno value.
> > + * @gdev:ÂÂÂÂÂÂÂ The Guest extension device.
> > + * @active:ÂÂÂÂÂÂÂ Flag whether the driver is now active or not.
> > + */
> > +static int vbg_report_driver_status(struct vbg_dev *gdev, bool active)
> > +{
> > +ÂÂÂ struct vmmdev_guest_status *req;
> > +ÂÂÂ int rc;
> > +
> > +ÂÂÂ req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_REPORT_GUEST_STATUS);
> > +ÂÂÂ if (!req)
> > +ÂÂÂÂÂÂÂ return -ENOMEM;
> > +
> > +ÂÂÂ req->facility = VBOXGUEST_FACILITY_TYPE_VBOXGUEST_DRIVER;
> > +ÂÂÂ if (active)
> > +ÂÂÂÂÂÂÂ req->status = VBOXGUEST_FACILITY_STATUS_ACTIVE;
> > +ÂÂÂ else
> > +ÂÂÂÂÂÂÂ req->status = VBOXGUEST_FACILITY_STATUS_INACTIVE;
> > +ÂÂÂ req->flags = 0;
> > +
> > +ÂÂÂ rc = vbg_req_perform(gdev, req);
> > +ÂÂÂ if (rc == VERR_NOT_IMPLEMENTED)ÂÂÂ /* Compatibility with older hosts. */
> > +ÂÂÂÂÂÂÂ rc = VINF_SUCCESS;
> > +
> > +ÂÂÂ kfree(req);
> > +
> > +ÂÂÂ return vbg_status_code_to_errno(rc);
> > +}
> > +
> > +/**
> > + * Inflate the balloon by one chunk. The caller owns the balloon mutex.
> > + * Return: 0 or negative errno value.
> > + * @gdev:ÂÂÂÂÂÂÂ The Guest extension device.
> > + * @chunk_idx:ÂÂÂÂÂÂÂ Index of the chunk.
> > + */
> > +static int vbg_balloon_inflate(struct vbg_dev *gdev, u32 chunk_idx)
> > +{
> > +ÂÂÂ struct vmmdev_memballoon_change *req = gdev->mem_balloon.change_req;
> > +ÂÂÂ struct page **pages;
> > +ÂÂÂ int i, rc, ret;
> > +
> > +ÂÂÂ pages = kmalloc(sizeof(*pages) * VMMDEV_MEMORY_BALLOON_CHUNK_PAGES,
> > +ÂÂÂÂÂÂÂÂÂÂÂ GFP_KERNEL | __GFP_NOWARN);
> > +ÂÂÂ if (!pages)
> > +ÂÂÂÂÂÂÂ return -ENOMEM;
> > +
> > +ÂÂÂ req->header.size = sizeof(*req);
> > +ÂÂÂ req->inflate = true;
> > +ÂÂÂ req->pages = VMMDEV_MEMORY_BALLOON_CHUNK_PAGES;
> > +
> > +ÂÂÂ for (i = 0; i < VMMDEV_MEMORY_BALLOON_CHUNK_PAGES; i++) {
> > +ÂÂÂÂÂÂÂ pages[i] = alloc_page(GFP_KERNEL | __GFP_NOWARN);
> > +ÂÂÂÂÂÂÂ if (!pages[i]) {
> > +ÂÂÂÂÂÂÂÂÂÂÂ ret = -ENOMEM;
> > +ÂÂÂÂÂÂÂÂÂÂÂ goto out_error;
> > +ÂÂÂÂÂÂÂ }
> > +
> > +ÂÂÂÂÂÂÂ req->phys_page[i] = page_to_phys(pages[i]);
> > +ÂÂÂ }
> > +
> > +ÂÂÂ rc = vbg_req_perform(gdev, req);
> > +ÂÂÂ if (rc < 0) {
> > +ÂÂÂÂÂÂÂ vbg_err("%s error, rc: %d\n", __func__, rc);
> > +ÂÂÂÂÂÂÂ ret = vbg_status_code_to_errno(rc);
> > +ÂÂÂÂÂÂÂ goto out_error;
> > +ÂÂÂ }
> > +
> > +ÂÂÂ gdev->mem_balloon.pages[chunk_idx] = pages;
> > +
> > +ÂÂÂ return 0;
> > +
> > +out_error:
> > +ÂÂÂ while (--i >= 0)
> > +ÂÂÂÂÂÂÂ __free_page(pages[i]);
> > +ÂÂÂ kfree(pages);
> > +
> > +ÂÂÂ return ret;
> > +}
> > +
> > +/**
> > + * Deflate the balloon by one chunk. The caller owns the balloon mutex.
> > + * Return: 0 or negative errno value.
> > + * @gdev:ÂÂÂÂÂÂÂ The Guest extension device.
> > + * @chunk_idx:ÂÂÂÂÂÂÂ Index of the chunk.
> > + */
> > +static int vbg_balloon_deflate(struct vbg_dev *gdev, u32 chunk_idx)
> > +{
> > +ÂÂÂ struct vmmdev_memballoon_change *req = gdev->mem_balloon.change_req;
> > +ÂÂÂ struct page **pages = gdev->mem_balloon.pages[chunk_idx];
> > +ÂÂÂ int i, rc;
> > +
> > +ÂÂÂ req->header.size = sizeof(*req);
> > +ÂÂÂ req->inflate = false;
> > +ÂÂÂ req->pages = VMMDEV_MEMORY_BALLOON_CHUNK_PAGES;
> > +
> > +ÂÂÂ for (i = 0; i < VMMDEV_MEMORY_BALLOON_CHUNK_PAGES; i++)
> > +ÂÂÂÂÂÂÂ req->phys_page[i] = page_to_phys(pages[i]);
> > +
> > +ÂÂÂ rc = vbg_req_perform(gdev, req);
> > +ÂÂÂ if (rc < 0) {
> > +ÂÂÂÂÂÂÂ vbg_err("%s error, rc: %d\n", __func__, rc);
> > +ÂÂÂÂÂÂÂ return vbg_status_code_to_errno(rc);
> > +ÂÂÂ }
> > +
> > +ÂÂÂ for (i = 0; i < VMMDEV_MEMORY_BALLOON_CHUNK_PAGES; i++)
> > +ÂÂÂÂÂÂÂ __free_page(pages[i]);
> > +ÂÂÂ kfree(pages);
> > +ÂÂÂ gdev->mem_balloon.pages[chunk_idx] = NULL;
> > +
> > +ÂÂÂ return 0;
> > +}
> > +
> > +/**
> > + * Respond to VMMDEV_EVENT_BALLOON_CHANGE_REQUEST events, query the size
> > + * the host wants the balloon to be and adjust accordingly.
> > + */
> > +static void vbg_balloon_work(struct work_struct *work)
> > +{
> > +ÂÂÂ struct vbg_dev *gdev =
> > +ÂÂÂÂÂÂÂ container_of(work, struct vbg_dev, mem_balloon.work);
> > +ÂÂÂ struct vmmdev_memballoon_info *req = gdev->mem_balloon.get_req;
> > +ÂÂÂ u32 i, chunks;
> > +ÂÂÂ int rc, ret;
> > +
> > +ÂÂÂ /*
> > +ÂÂÂÂ * Setting this bit means that we request the value from the host and
> > +ÂÂÂÂ * change the guest memory balloon according to the returned value.
> > +ÂÂÂÂ */
> > +ÂÂÂ req->event_ack = VMMDEV_EVENT_BALLOON_CHANGE_REQUEST;
> > +ÂÂÂ rc = vbg_req_perform(gdev, req);
> > +ÂÂÂ if (rc < 0) {
> > +ÂÂÂÂÂÂÂ vbg_err("%s error, rc: %d)\n", __func__, rc);
> > +ÂÂÂÂÂÂÂ return;
> > +ÂÂÂ }
> > +
> > +ÂÂÂ /*
> > +ÂÂÂÂ * The host always returns the same maximum amount of chunks, so
> > +ÂÂÂÂ * we do this once.
> > +ÂÂÂÂ */
> > +ÂÂÂ if (!gdev->mem_balloon.max_chunks) {
> > +ÂÂÂÂÂÂÂ gdev->mem_balloon.pages =
> > +ÂÂÂÂÂÂÂÂÂÂÂ devm_kcalloc(gdev->dev, req->phys_mem_chunks,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ sizeof(struct page **), GFP_KERNEL);
> > +ÂÂÂÂÂÂÂ if (!gdev->mem_balloon.pages)
> > +ÂÂÂÂÂÂÂÂÂÂÂ return;
> > +
> > +ÂÂÂÂÂÂÂ gdev->mem_balloon.max_chunks = req->phys_mem_chunks;
> > +ÂÂÂ }
> > +
> > +ÂÂÂ chunks = req->balloon_chunks;
> > +ÂÂÂ if (chunks > gdev->mem_balloon.max_chunks) {
> > +ÂÂÂÂÂÂÂ vbg_err("%s: illegal balloon size %u (max=%u)\n",
> > +ÂÂÂÂÂÂÂÂÂÂÂ __func__, chunks, gdev->mem_balloon.max_chunks);
> > +ÂÂÂÂÂÂÂ return;
> > +ÂÂÂ }
> > +
> > +ÂÂÂ if (chunks > gdev->mem_balloon.chunks) {
> > +ÂÂÂÂÂÂÂ /* inflate */
> > +ÂÂÂÂÂÂÂ for (i = gdev->mem_balloon.chunks; i < chunks; i++) {
> > +ÂÂÂÂÂÂÂÂÂÂÂ ret = vbg_balloon_inflate(gdev, i);
> > +ÂÂÂÂÂÂÂÂÂÂÂ if (ret < 0)
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ return;
> > +
> > +ÂÂÂÂÂÂÂÂÂÂÂ gdev->mem_balloon.chunks++;
> > +ÂÂÂÂÂÂÂ }
> > +ÂÂÂ } else {
> > +ÂÂÂÂÂÂÂ /* deflate */
> > +ÂÂÂÂÂÂÂ for (i = gdev->mem_balloon.chunks; i-- > chunks;) {
> > +ÂÂÂÂÂÂÂÂÂÂÂ ret = vbg_balloon_deflate(gdev, i);
> > +ÂÂÂÂÂÂÂÂÂÂÂ if (ret < 0)
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ return;
> > +
> > +ÂÂÂÂÂÂÂÂÂÂÂ gdev->mem_balloon.chunks--;
> > +ÂÂÂÂÂÂÂ }
> > +ÂÂÂ }
> > +}
> > +
> > +/**
> > + * Callback for heartbeat timer.
> > + */
> > +static void vbg_heartbeat_timer(struct timer_list *t)
> > +{
> > +ÂÂÂ struct vbg_dev *gdev = from_timer(gdev, t, heartbeat_timer);
> > +
> > +ÂÂÂ vbg_req_perform(gdev, gdev->guest_heartbeat_req);
> > +ÂÂÂ mod_timer(&gdev->heartbeat_timer,
> > +ÂÂÂÂÂÂÂÂÂ msecs_to_jiffies(gdev->heartbeat_interval_ms));
> > +}
> > +
> > +/**
> > + * Configure the host to check guest's heartbeat
> > + * and get heartbeat interval from the host.
> > + * Return: 0 or negative errno value.
> > + * @gdev:ÂÂÂÂÂÂÂ The Guest extension device.
> > + * @enabled:ÂÂÂÂÂÂÂ Set true to enable guest heartbeat checks on host.
> > + */
> > +static int vbg_heartbeat_host_config(struct vbg_dev *gdev, bool enabled)
> > +{
> > +ÂÂÂ struct vmmdev_heartbeat *req;
> > +ÂÂÂ int rc;
> > +
> > +ÂÂÂ req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_HEARTBEAT_CONFIGURE);
> > +ÂÂÂ if (!req)
> > +ÂÂÂÂÂÂÂ return -ENOMEM;
> > +
> > +ÂÂÂ req->enabled = enabled;
> > +ÂÂÂ req->interval_ns = 0;
> > +ÂÂÂ rc = vbg_req_perform(gdev, req);
> > +ÂÂÂ do_div(req->interval_ns, 1000000); /* ns -> ms */
> > +ÂÂÂ gdev->heartbeat_interval_ms = req->interval_ns;
> > +ÂÂÂ kfree(req);
> > +
> > +ÂÂÂ return vbg_status_code_to_errno(rc);
> > +}
> > +
> > +/**
> > + * Initializes the heartbeat timer. This feature may be disabled by the host.
> > + * Return: 0 or negative errno value.
> > + * @gdev:ÂÂÂÂÂÂÂ The Guest extension device.
> > + */
> > +static int vbg_heartbeat_init(struct vbg_dev *gdev)
> > +{
> > +ÂÂÂ int ret;
> > +
> > +ÂÂÂ /* Make sure that heartbeat checking is disabled if we fail. */
> > +ÂÂÂ ret = vbg_heartbeat_host_config(gdev, false);
> > +ÂÂÂ if (ret < 0)
> > +ÂÂÂÂÂÂÂ return ret;
> > +
> > +ÂÂÂ ret = vbg_heartbeat_host_config(gdev, true);
> > +ÂÂÂ if (ret < 0)
> > +ÂÂÂÂÂÂÂ return ret;
> > +
> > +ÂÂÂ /*
> > +ÂÂÂÂ * Preallocate the request to use it from the timer callback because:
> > +ÂÂÂÂ *ÂÂÂ 1) on Windows vbg_req_alloc must be called at IRQL <= APC_LEVEL
> > +ÂÂÂÂ *ÂÂÂÂÂÂ and the timer callback runs at DISPATCH_LEVEL;
> > +ÂÂÂÂ *ÂÂÂ 2) avoid repeated allocations.
> > +ÂÂÂÂ */
> > +ÂÂÂ gdev->guest_heartbeat_req = vbg_req_alloc(
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ sizeof(*gdev->guest_heartbeat_req),
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ VMMDEVREQ_GUEST_HEARTBEAT);
> > +ÂÂÂ if (!gdev->guest_heartbeat_req)
> > +ÂÂÂÂÂÂÂ return -ENOMEM;
> > +
> > +ÂÂÂ vbg_info("%s: Setting up heartbeat to trigger every %d milliseconds\n",
> > +ÂÂÂÂÂÂÂÂ __func__, gdev->heartbeat_interval_ms);
> > +ÂÂÂ mod_timer(&gdev->heartbeat_timer, 0);
> > +
> > +ÂÂÂ return 0;
> > +}
> > +
> > +/**
> > + * Cleanup hearbeat code, stop HB timer and disable host heartbeat checking.
> > + * @gdev:ÂÂÂÂÂÂÂ The Guest extension device.
> > + */
> > +static void vbg_heartbeat_exit(struct vbg_dev *gdev)
> > +{
> > +ÂÂÂ del_timer_sync(&gdev->heartbeat_timer);
> > +ÂÂÂ vbg_heartbeat_host_config(gdev, false);
> > +ÂÂÂ kfree(gdev->guest_heartbeat_req);
> > +
> > +}
> > +
> > +/**
> > + * Applies a change to the bit usage tracker.
> > + * Return: true if the mask changed, false if not.
> > + * @tracker:ÂÂÂÂÂÂÂ The bit usage tracker.
> > + * @changed:ÂÂÂÂÂÂÂ The bits to change.
> > + * @previous:ÂÂÂÂÂÂÂ The previous value of the bits.
> > + */
> > +static bool vbg_track_bit_usage(struct vbg_bit_usage_tracker *tracker,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ u32 changed, u32 previous)
> > +{
> > +ÂÂÂ bool global_change = false;
> > +
> > +ÂÂÂ while (changed) {
> > +ÂÂÂÂÂÂÂ u32 bit = ffs(changed) - 1;
> > +ÂÂÂÂÂÂÂ u32 bitmask = BIT(bit);
> > +
> > +ÂÂÂÂÂÂÂ if (bitmask & previous) {
> > +ÂÂÂÂÂÂÂÂÂÂÂ tracker->per_bit_usage[bit] -= 1;
> > +ÂÂÂÂÂÂÂÂÂÂÂ if (tracker->per_bit_usage[bit] == 0) {
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ global_change = true;
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ tracker->mask &= ~bitmask;
> > +ÂÂÂÂÂÂÂÂÂÂÂ }
> > +ÂÂÂÂÂÂÂ } else {
> > +ÂÂÂÂÂÂÂÂÂÂÂ tracker->per_bit_usage[bit] += 1;
> > +ÂÂÂÂÂÂÂÂÂÂÂ if (tracker->per_bit_usage[bit] == 1) {
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ global_change = true;
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ tracker->mask |= bitmask;
> > +ÂÂÂÂÂÂÂÂÂÂÂ }
> > +ÂÂÂÂÂÂÂ }
> > +
> > +ÂÂÂÂÂÂÂ changed &= ~bitmask;
> > +ÂÂÂ }
> > +
> > +ÂÂÂ return global_change;
> > +}
> > +
> > +/**
> > + * Init and termination worker for resetting the (host) event filter on the host
> > + * Return: 0 or negative errno value.
> > + * @gdev:ÂÂÂÂÂÂÂÂÂÂ The Guest extension device.
> > + * @fixed_events:ÂÂÂÂÂÂ Fixed events (init time).
> > + */
> > +static int vbg_reset_host_event_filter(struct vbg_dev *gdev,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ u32 fixed_events)
> > +{
> > +ÂÂÂ struct vmmdev_mask *req;
> > +ÂÂÂ int rc;
> > +
> > +ÂÂÂ req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_CTL_GUEST_FILTER_MASK);
> > +ÂÂÂ if (!req)
> > +ÂÂÂÂÂÂÂ return -ENOMEM;
> > +
> > +ÂÂÂ req->not_mask = U32_MAX & ~fixed_events;
> > +ÂÂÂ req->or_mask = fixed_events;
> > +ÂÂÂ rc = vbg_req_perform(gdev, req);
> > +ÂÂÂ if (rc < 0)
> > +ÂÂÂÂÂÂÂ vbg_err("%s error, rc: %d\n", __func__, rc);
> > +
> > +ÂÂÂ kfree(req);
> > +ÂÂÂ return vbg_status_code_to_errno(rc);
> > +}
> > +
> > +/**
> > + * Changes the event filter mask for the given session.
> > + *
> > + * This is called in response to VBG_IOCTL_CHANGE_FILTER_MASK as well as to
> > + * do session cleanup. Takes the session spinlock.
> > + *
> > + * Return: 0 or negative errno value.
> > + * @gdev:ÂÂÂÂÂÂÂÂÂÂÂ The Guest extension device.
> > + * @session:ÂÂÂÂÂÂÂÂÂÂÂ The session.
> > + * @or_mask:ÂÂÂÂÂÂÂÂÂÂÂ The events to add.
> > + * @not_mask:ÂÂÂÂÂÂÂÂÂÂÂ The events to remove.
> > + * @session_termination:ÂÂÂ Set if we're called by the session cleanup code.
> > + *ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ This tweaks the error handling so we perform
> > + *ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ proper session cleanup even if the host
> > + *ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ misbehaves.
> > + */
> > +static int vbg_set_session_event_filter(struct vbg_dev *gdev,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct vbg_session *session,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ u32 or_mask, u32 not_mask,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ bool session_termination)
> > +{
> > +ÂÂÂ struct vmmdev_mask *req;
> > +ÂÂÂ u32 changed, previous;
> > +ÂÂÂ int rc, ret = 0;
> > +
> > +ÂÂÂ /* Allocate a request buffer before taking the spinlock */
> > +ÂÂÂ req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_CTL_GUEST_FILTER_MASK);
> > +ÂÂÂ if (!req) {
> > +ÂÂÂÂÂÂÂ if (!session_termination)
> > +ÂÂÂÂÂÂÂÂÂÂÂ return -ENOMEM;
> > +ÂÂÂÂÂÂÂ /* Ignore failure, we must do session cleanup. */
>
> The comment should say "Ignore allocation failure ...", but that leads to problems below.

Ok, fixed for v3.

> > +ÂÂÂ }
> > +
> > +ÂÂÂ mutex_lock(&gdev->session_mutex);
> > +
> > +ÂÂÂ /* Apply the changes to the session mask. */
> > +ÂÂÂ previous = session->event_filter;
> > +ÂÂÂ session->event_filter |= or_mask;
> > +ÂÂÂ session->event_filter &= ~not_mask;
> > +
> > +ÂÂÂ /* If anything actually changed, update the global usage counters. */
> > +ÂÂÂ changed = previous ^ session->event_filter;
> > +ÂÂÂ if (!changed)
> > +ÂÂÂÂÂÂÂ goto out;
> > +
> > +ÂÂÂ vbg_track_bit_usage(&gdev->event_filter_tracker, changed, previous);
> > +ÂÂÂ req->or_mask = gdev->fixed_events | gdev->event_filter_tracker.mask;
>
> At this point, req could be NULL. I'm not sure what session cleanup is needed if req is NULL and session_termination is not, but it needs to be split out.

Right, I've solved this by using a local or_mask variable and ...

> > +
> > +ÂÂÂ if (gdev->event_filter_host == req->or_mask || !req)
> > +ÂÂÂÂÂÂÂ goto out;
> > +
> > +ÂÂÂ gdev->event_filter_host = req->or_mask;
> > +ÂÂÂ req->not_mask = ~req->or_mask;

Assigning that to req->or_mask here, after the !req check.

> > +ÂÂÂ rc = vbg_req_perform(gdev, req);
> > +ÂÂÂ if (rc < 0) {
> > +ÂÂÂÂÂÂÂ ret = vbg_status_code_to_errno(rc);
> > +
> > +ÂÂÂÂÂÂÂ /* Failed, roll back (unless it's session termination time). */
> > +ÂÂÂÂÂÂÂ gdev->event_filter_host = U32_MAX;
> > +ÂÂÂÂÂÂÂ if (session_termination)
> > +ÂÂÂÂÂÂÂÂÂÂÂ goto out;
> > +
> > +ÂÂÂÂÂÂÂ vbg_track_bit_usage(&gdev->event_filter_tracker, changed,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ session->event_filter);
> > +ÂÂÂÂÂÂÂ session->event_filter = previous;
> > +ÂÂÂ }
> > +
> > +out:
> > +ÂÂÂ mutex_unlock(&gdev->session_mutex);
> > +ÂÂÂ kfree(req);
> > +
> > +ÂÂÂ return ret;
> > +}
> > +
> > +/**
> > + * Init and termination worker for set guest capabilities to zero on the host.
> > + * Return: 0 or negative errno value.
> > + * @gdev:ÂÂÂÂÂÂÂ The Guest extension device.
> > + */
> > +static int vbg_reset_host_capabilities(struct vbg_dev *gdev)
> > +{
> > +ÂÂÂ struct vmmdev_mask *req;
> > +ÂÂÂ int rc;
> > +
> > +ÂÂÂ req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_SET_GUEST_CAPABILITIES);
> > +ÂÂÂ if (!req)
> > +ÂÂÂÂÂÂÂ return -ENOMEM;
> > +
> > +ÂÂÂ req->not_mask = U32_MAX;
> > +ÂÂÂ req->or_mask = 0;
> > +ÂÂÂ rc = vbg_req_perform(gdev, req);
> > +ÂÂÂ if (rc < 0)
> > +ÂÂÂÂÂÂÂ vbg_err("%s error, rc: %d\n", __func__, rc);
> > +
> > +ÂÂÂ kfree(req);
> > +ÂÂÂ return vbg_status_code_to_errno(rc);
> > +}
> > +
> > +/**
> > + * Sets the guest capabilities for a session. Takes the session spinlock.
> > + * Return: 0 or negative errno value.
> > + * @gdev:ÂÂÂÂÂÂÂÂÂÂÂ The Guest extension device.
> > + * @session:ÂÂÂÂÂÂÂÂÂÂÂ The session.
> > + * @or_mask:ÂÂÂÂÂÂÂÂÂÂÂ The capabilities to add.
> > + * @not_mask:ÂÂÂÂÂÂÂÂÂÂÂ The capabilities to remove.
> > + * @session_termination:ÂÂÂ Set if we're called by the session cleanup code.
> > + *ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ This tweaks the error handling so we perform
> > + *ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ proper session cleanup even if the host
> > + *ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ misbehaves.
> > + */
> > +static int vbg_set_session_capabilities(struct vbg_dev *gdev,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct vbg_session *session,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ u32 or_mask, u32 not_mask,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ bool session_termination)
> > +{
> > +ÂÂÂ struct vmmdev_mask *req;
> > +ÂÂÂ u32 changed, previous;
> > +ÂÂÂ int rc, ret = 0;
> > +
> > +ÂÂÂ /* Allocate a request buffer before taking the spinlock */
> > +ÂÂÂ req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_SET_GUEST_CAPABILITIES);
> > +ÂÂÂ if (!req) {
> > +ÂÂÂÂÂÂÂ if (!session_termination)
> > +ÂÂÂÂÂÂÂÂÂÂÂ return -ENOMEM;
> > +ÂÂÂÂÂÂÂ /* Ignore failure, we must do session cleanup. */
>
> This comment should also be changed.

Ack, also fixed.

> > +ÂÂÂ }
> > +
> > +ÂÂÂ mutex_lock(&gdev->session_mutex);
> > +
> > +ÂÂÂ /* Apply the changes to the session mask. */
> > +ÂÂÂ previous = session->guest_caps;
> > +ÂÂÂ session->guest_caps |= or_mask;
> > +ÂÂÂ session->guest_caps &= ~not_mask;
> > +
> > +ÂÂÂ /* If anything actually changed, update the global usage counters. */
> > +ÂÂÂ changed = previous ^ session->guest_caps;
> > +ÂÂÂ if (!changed)
> > +ÂÂÂÂÂÂÂ goto out;
> > +
> > +ÂÂÂ vbg_track_bit_usage(&gdev->guest_caps_tracker, changed, previous);
> > +ÂÂÂ req->or_mask = gdev->guest_caps_tracker.mask;
>
> req can be NULL here.

Ack, fixed in the same way as above.

> > +
> > +ÂÂÂ if (gdev->guest_caps_host == req->or_mask || !req)
> > +ÂÂÂÂÂÂÂ goto out;
> > +
> > +ÂÂÂ gdev->guest_caps_host = req->or_mask;
> > +ÂÂÂ req->not_mask = ~req->or_mask;
> > +ÂÂÂ rc = vbg_req_perform(gdev, req);
> > +ÂÂÂ if (rc < 0) {
> > +ÂÂÂÂÂÂÂ ret = vbg_status_code_to_errno(rc);
> > +
> > +ÂÂÂÂÂÂÂ /* Failed, roll back (unless it's session termination time). */
> > +ÂÂÂÂÂÂÂ gdev->guest_caps_host = U32_MAX;
> > +ÂÂÂÂÂÂÂ if (session_termination)
> > +ÂÂÂÂÂÂÂÂÂÂÂ goto out;
> > +
> > +ÂÂÂÂÂÂÂ vbg_track_bit_usage(&gdev->guest_caps_tracker, changed,
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ session->guest_caps);
> > +ÂÂÂÂÂÂÂ session->guest_caps = previous;
> > +ÂÂÂ }
> > +
> > +out:
> > +ÂÂÂ mutex_unlock(&gdev->session_mutex);
> > +ÂÂÂ kfree(req);
> > +
> > +ÂÂÂ return ret;
> > +}

<snip>

Regards,

Hans