From: Greg KH <gregkh@linuxfoundation.org>
To: Rishi Chhibber <rishi.chhibber@broadcom.com>
Cc: arnd@arndb.de, linux-kernel@vger.kernel.org,
ajay.kaher@broadcom.com, alexey.makhalov@broadcom.com,
vamsi-krishna.brahmajosyula@broadcom.com, yin.ding@broadcom.com,
tapas.kundu@broadcom.com
Subject: Re: [PATCH] misc: vmw_zerocopy: Add VMware zero-copy buffer sharing driver
Date: Thu, 18 Jun 2026 12:26:03 +0200 [thread overview]
Message-ID: <2026061818-slot-foster-bdf0@gregkh> (raw)
In-Reply-To: <20260617203125.397427-1-rishi.chhibber@broadcom.com>
On Wed, Jun 17, 2026 at 01:31:25PM -0700, Rishi Chhibber wrote:
> This driver implements a character device (/dev/vmw_zc) that allows
> guest userspace applications to share pinned memory buffers with a
> VMware hypervisor-side peer using the VMCI datagram interface.
Why is this a new char device, don't we already have virtio apis for
this type of thing?
> The driver pins user pages via get_user_pages_fast(), transmits their
> physical page frame numbers to the hypervisor peer over VMCI, and
> avoids an intermediate copy between the guest workload VM and the
> hypervisor.
>
> Signed-off-by: Rishi Chhibber <rishi.chhibber@broadcom.com>
> ---
> MAINTAINERS | 8 +
> drivers/misc/Kconfig | 1 +
> drivers/misc/Makefile | 1 +
> drivers/misc/vmw_zerocopy/Kconfig | 16 +
> drivers/misc/vmw_zerocopy/Makefile | 17 +
> .../misc/vmw_zerocopy/vmw_zerocopy_driver.c | 490 ++++++++++++++++++
> .../misc/vmw_zerocopy/vmw_zerocopy_driver.h | 51 ++
> .../uapi/linux/vmw_zerocopy_ioctl_common.h | 66 +++
> 8 files changed, 650 insertions(+)
> create mode 100644 drivers/misc/vmw_zerocopy/Kconfig
> create mode 100644 drivers/misc/vmw_zerocopy/Makefile
> create mode 100644 drivers/misc/vmw_zerocopy/vmw_zerocopy_driver.c
> create mode 100644 drivers/misc/vmw_zerocopy/vmw_zerocopy_driver.h
> create mode 100644 include/uapi/linux/vmw_zerocopy_ioctl_common.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index efd1fa7d66f0..59ee66158486 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -24790,6 +24790,14 @@ L: linux-kernel@vger.kernel.org
> S: Supported
> F: net/vmw_vsock/vmci_transport*
>
> +VMWARE ZEROCOPY DRIVER
> +M: Rishi Chhibber <rishi.chhibber@broadcom.com>
> +R: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
Please don't put closed lists in the MAINTAINERS file, that just causes
bounces.
> +static int vmw_zc_release(struct inode *inode, struct file *file)
> +{
> + pr_debug(LGPFX "release\n");
> + return 0;
> +}
If you do nothing in a function, no need to have it at all, right?
> +static int __init vmw_zc_init(void)
> +{
> + dev_t dev;
> + int ret;
> + struct device *mydev;
> +
> + pr_info(LGPFX "loading\n");
When drivers work properly, they are quiet.
> +
> + ret = alloc_chrdev_region(&dev, 0, 1, VMW_ZC_DEVICE_NAME);
As you only want one char device, why not use miscdev instead?
> + if (ret) {
> + pr_err(LGPFX "alloc_chrdev_region failed: %d\n", ret);
No need for the LGPFX stuff everywhere, you all do know about pr_fmt(),
right?
> --- /dev/null
> +++ b/drivers/misc/vmw_zerocopy/vmw_zerocopy_driver.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
I have to ask, are you sure about "or later"?
> +/*
> + * Copyright (c) 2026 Broadcom. All Rights Reserved. The term
> + * "Broadcom" refers to Broadcom Inc. and/or its subsidiaries.
> + *
> + * Wire-format messages sent to the peer over VMCI (driver private).
> + * Limits and raw size must stay in sync with include/uapi/linux/vmw_zerocopy_ioctl_common.h
How is that going to happen? Why not just keep everything in one file?
> + */
> +
> +#ifndef _VMW_ZC_HOST_H
> +#define _VMW_ZC_HOST_H
> +
> +#include <linux/mm.h>
> +#include <linux/types.h>
> +
> +#include <linux/vmw_zerocopy_ioctl_common.h>
> +
> +#define VMW_ZC_MAX_METADATA_SIZE 1024
> +#define VMW_ZC_MAX_BUFFER_SIZE (64 * 1024UL)
> +#define VMW_ZC_MAX_PAGES \
> + (((VMW_ZC_MAX_BUFFER_SIZE) + (PAGE_SIZE - 1)) / PAGE_SIZE + 1)
> +
> +struct vmw_zc_msg_unit {
> + __u32 offset;
> + __u32 length;
> + __u32 num_pages;
> + __u32 padding1;
> + __u64 page_pfns[VMW_ZC_MAX_PAGES];
Why do you have an internal structure defined with __u32 and the like?
That's only a type that crosses the user/kernel boundry, and it would be
implied this would be defined in the uapi .h file, right?
thanks,
greg k-h
next prev parent reply other threads:[~2026-06-18 10:25 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-17 20:31 [PATCH] misc: vmw_zerocopy: Add VMware zero-copy buffer sharing driver Rishi Chhibber
2026-06-18 10:26 ` Greg KH [this message]
2026-06-18 18:10 ` [PATCH v2] " Rishi Chhibber
2026-06-19 5:08 ` Greg KH
2026-06-19 5:10 ` Greg KH
2026-06-19 15:12 ` David Laight
2026-06-19 18:27 ` [PATCH] " Rishi Chhibber
2026-06-20 5:04 ` Greg KH
2026-06-20 5:06 ` Greg KH
-- strict thread matches above, loose matches on Subject: below --
2026-06-17 20:27 Rishi Chhibber
2026-06-17 20:45 ` Rishi Chhibber
2026-06-17 20:26 Rishi Chhibber
2026-06-17 20:45 ` Rishi Chhibber
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2026061818-slot-foster-bdf0@gregkh \
--to=gregkh@linuxfoundation.org \
--cc=ajay.kaher@broadcom.com \
--cc=alexey.makhalov@broadcom.com \
--cc=arnd@arndb.de \
--cc=linux-kernel@vger.kernel.org \
--cc=rishi.chhibber@broadcom.com \
--cc=tapas.kundu@broadcom.com \
--cc=vamsi-krishna.brahmajosyula@broadcom.com \
--cc=yin.ding@broadcom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.