Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Message-ID: <920a671c-1927-4086-950f-db8e3293a989@linaro.org>
Date:   Tue, 10 Oct 2023 09:22:21 -0400
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [Stratos-dev] [PATCH V3 2/2] xen: privcmd: Add support for
 ioeventfd
Content-Language: en-US
To:     Viresh Kumar <viresh.kumar@linaro.org>,
        Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Cc:     stratos-dev@op-lists.linaro.org,
        Erik Schilling <erik.schilling@linaro.org>,
        Manos Pitsidianakis <manos.pitsidianakis@linaro.org>,
        linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org
References: <cover.1696850733.git.viresh.kumar@linaro.org>
 <957de2a53dde99a653b47af9c95bc17692870294.1696850733.git.viresh.kumar@linaro.org>
From:   Bill Mills <bill.mills@linaro.org>
In-Reply-To: <957de2a53dde99a653b47af9c95bc17692870294.1696850733.git.viresh.kumar@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Viresh,

On 10/9/23 7:29 AM, Viresh Kumar via Stratos-dev wrote:
> Virtio guests send VIRTIO_MMIO_QUEUE_NOTIFY notification when they need
> to notify the backend of an update to the status of the virtqueue. The
> backend or another entity, polls the MMIO address for updates to know
> when the notification is sent.
> 
> It works well if the backend does this polling by itself. But as we move
> towards generic backend implementations, we end up implementing this in
> a separate user-space program.
> 
> Generally, the Virtio backends are implemented to work with the Eventfd
> based mechanism. In order to make such backends work with Xen, another
> software layer needs to do the polling and send an event via eventfd to
> the backend once the notification from guest is received. This results
> in an extra context switch.
> 
> This is not a new problem in Linux though. It is present with other
> hypervisors like KVM, etc. as well. The generic solution implemented in
> the kernel for them is to provide an IOCTL call to pass the address to
> poll and eventfd, which lets the kernel take care of polling and raise
> an event on the eventfd, instead of handling this in user space (which
> involves an extra context switch).
> 
> This patch adds similar support for xen.
> 
> Inspired by existing implementations for KVM, etc..
> 
> This also copies ioreq.h header file (only struct ioreq and related
> macros) from Xen's source tree (Top commit 5d84f07fe6bf ("xen/pci: drop
> remaining uses of bool_t")).
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>   drivers/xen/Kconfig               |   8 +-
>   drivers/xen/privcmd.c             | 405 +++++++++++++++++++++++++++++-
>   include/uapi/xen/privcmd.h        |  18 ++
>   include/xen/interface/hvm/ioreq.h |  51 ++++
>   4 files changed, 476 insertions(+), 6 deletions(-)
>   create mode 100644 include/xen/interface/hvm/ioreq.h
> 
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index d43153fec18e..d5989871dd5d 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -269,12 +269,12 @@ config XEN_PRIVCMD
>   	  disaggregated Xen setups this driver might be needed for other
>   	  domains, too.
>   
> -config XEN_PRIVCMD_IRQFD
> -	bool "Xen irqfd support"
> +config XEN_PRIVCMD_EVENTFD
> +	bool "Xen Ioeventfd and irqfd support"

You certainly know about kernel policies better than I but why the 
CONGIG_ name change?

With this change all downstream distros need to find this and re-enable 
it if they want to keep a working Xen system.

I presume the kernel can still work in the old way and not use your new 
mode correct?

I would certainly change the description but I wonder about keeping the 
CONFIG_ name the same.

No issue if I am obviously wrong and this is done all the time.

Thanks,
Bill