Re: [PATCH RESEND v6 17/36] multi-process: introduce proxy object

[Stefan Hajnoczi]

On Wed, Apr 22, 2020 at 09:13:52PM -0700, address@hidden wrote:
> From: Elena Ufimtseva <address@hidden>
> 
> Defines a PCI Device proxy object as a parent of TYPE_PCI_DEVICE.

s/parent/child/

> 
> PCI Proxy Object registers as a PCI device with QEMU and forwards all
> PCI accesses to the remote process using the communication channel.

Please include that functionality in this patch. The code below just
sets up a skeleton PCI device. There is no code that forwards accesses
to the remote process.

> Signed-off-by: Elena Ufimtseva <address@hidden>
> Signed-off-by: Jagannathan Raman <address@hidden>
> Signed-off-by: John G Johnson <address@hidden>
> ---
>  MAINTAINERS                   |  3 ++
>  hw/Makefile.objs              |  2 ++
>  hw/proxy/Makefile.objs        |  1 +
>  hw/proxy/qemu-proxy.c         | 56 +++++++++++++++++++++++++++++++++++
>  include/hw/proxy/qemu-proxy.h | 46 ++++++++++++++++++++++++++++
>  include/io/mpqemu-link.h      |  1 +
>  6 files changed, 109 insertions(+)
>  create mode 100644 hw/proxy/Makefile.objs
>  create mode 100644 hw/proxy/qemu-proxy.c
>  create mode 100644 include/hw/proxy/qemu-proxy.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 96f8d7ff19..3da3dcd311 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2866,6 +2866,9 @@ F: include/remote/machine.h
>  F: remote/machine.c
>  F: include/remote/memory.h
>  F: remote/memory.c
> +F: hw/proxy/Makefile.objs
> +F: hw/proxy/qemu-proxy.c
> +F: include/hw/proxy/qemu-proxy.h

It's a generic PCI device. hw/pci/proxy.c would be a good location for
it.

By the way an alternative to the "proxy"/"remote" terminology is
RemotePCIClient/RemotePCIServer. That makes it more obvious that "proxy"
is related the "remote" feature. Feel free to keep the existing
terminology, I just wanted to suggest another possibility.

>  
>  Build and test automation
>  -------------------------
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index af9235b6f2..7b489b12a5 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -45,6 +45,8 @@ endif
>  common-obj-y += $(devices-dirs-y)
>  obj-y += $(devices-dirs-y)
>  
> +common-obj-$(CONFIG_MPQEMU) += proxy/
> +
>  remote-pci-obj-$(CONFIG_MPQEMU) += core/
>  remote-pci-obj-$(CONFIG_MPQEMU) += block/
>  remote-pci-obj-$(CONFIG_MPQEMU) += pci/
> diff --git a/hw/proxy/Makefile.objs b/hw/proxy/Makefile.objs
> new file mode 100644
> index 0000000000..eb81624cf8
> --- /dev/null
> +++ b/hw/proxy/Makefile.objs
> @@ -0,0 +1 @@
> +common-obj-$(CONFIG_MPQEMU) += qemu-proxy.o
> diff --git a/hw/proxy/qemu-proxy.c b/hw/proxy/qemu-proxy.c
> new file mode 100644
> index 0000000000..bf6c4117ef
> --- /dev/null
> +++ b/hw/proxy/qemu-proxy.c
> @@ -0,0 +1,56 @@
> +/*
> + * Copyright © 2018, 2020 Oracle and/or its affiliates.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +
> +#include "qapi/error.h"
> +#include "io/mpqemu-link.h"
> +#include "hw/proxy/qemu-proxy.h"
> +#include "hw/pci/pci.h"
> +
> +static void pci_proxy_dev_realize(PCIDevice *device, Error **errp)
> +{
> +    PCIProxyDev *dev = PCI_PROXY_DEV(device);
> +    PCIProxyDevClass *k = PCI_PROXY_DEV_GET_CLASS(dev);
> +    Error *local_err = NULL;
> +
> +    if (k->realize) {

Will anything inherit from this class? I thought this is the remote PCI
client that can acts as a stand-in for all remote PCI devices, so it's
not clear why it's acting as a base class here.

> diff --git a/include/io/mpqemu-link.h b/include/io/mpqemu-link.h
> index d46cb81058..73cc59b874 100644
> --- a/include/io/mpqemu-link.h
> +++ b/include/io/mpqemu-link.h
> @@ -14,6 +14,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu-common.h"
>  
> +#include "sys/eventfd.h"

Why? Nothing in this patch uses this header.

signature.asc
Description: PGP signature