Re: [RFC v4 PATCH 15/49] multi-process: PCI BAR read/write handling for proxy & remote endpoints

[Stefan Hajnoczi]

On Thu, Oct 24, 2019 at 05:08:56AM -0400, Jagannathan Raman wrote:
> +const MemoryRegionOps proxy_default_ops = {

Unused.  Please structure patch series so that each patch is a
self-contained logical change.  It should be possible to review the
series in order from start to finish.

If there is no user yet and this is a public API then there need to be
doc comments describing the API.

> diff --git a/include/io/mpqemu-link.h b/include/io/mpqemu-link.h
> index 7ef8207..89f04c5 100644
> --- a/include/io/mpqemu-link.h
> +++ b/include/io/mpqemu-link.h
> @@ -52,6 +52,8 @@
>   * CONF_READ        PCI config. space read
>   * CONF_WRITE       PCI config. space write
>   * SYNC_SYSMEM      Shares QEMU's RAM with remote device's RAM
> + * BAR_WRITE        Writes to PCI BAR region
> + * BAR_READ         Reads from PCI BAR region

Is it possible to generalize this to memory regions instead of PCI BARs?
That way non-PCI devices will be able to use the same protocol messages
and code.  VFIO describes BARs generically too for the same reason, see
<linux/vfio.h> struct vfio_region_info.

>   *
>   * proc_cmd_t enum type to specify the command to be executed on the remote
>   * device.
> @@ -61,6 +63,8 @@ typedef enum {
>      CONF_READ,
>      CONF_WRITE,
>      SYNC_SYSMEM,
> +    BAR_WRITE,
> +    BAR_READ,
>      MAX,
>  } mpqemu_cmd_t;
>  
> @@ -84,6 +88,13 @@ typedef struct {
>  } sync_sysmem_msg_t;
>  
>  typedef struct {
> +    hwaddr addr;
> +    uint64_t val;
> +    unsigned size;
> +    bool memory;

Why is this field necessary?  Whether this is a memory access or not
should be implicit from the address/BAR we are accessing.

signature.asc
Description: PGP signature