Re: [PATCH RESEND v6 14/36] multi-process: setup a machine object for remote device process

[Stefan Hajnoczi]

On Wed, Apr 22, 2020 at 09:13:49PM -0700, address@hidden wrote:
> From: Jagannathan Raman <address@hidden>
> 
> remote-machine object sets up various subsystems of the remote device
> process. Instantiate PCI host bridge object and initialize RAM, IO &
> PCI memory regions.
> 
> Signed-off-by: John G Johnson <address@hidden>
> Signed-off-by: Jagannathan Raman <address@hidden>
> Signed-off-by: Elena Ufimtseva <address@hidden>
> ---
>  MAINTAINERS                   |  2 +
>  Makefile.objs                 |  1 +
>  exec.c                        |  3 +-
>  include/exec/address-spaces.h |  2 +
>  include/remote/machine.h      | 30 +++++++++++++
>  remote/Makefile.objs          |  2 +
>  remote/machine.c              | 84 +++++++++++++++++++++++++++++++++++
>  remote/remote-main.c          |  7 +++

Now that the separate remote emulation program is going away I think it
makes sense to move the PCIe host and machine type into hw/:

  hw/pci-host/remote.c <-- PCIe host
  hw/i386/remote.c     <-- machine type (could be moved again later if
                           other architectures are supported)

> diff --git a/exec.c b/exec.c
> index d0ac9545f4..5b1e414099 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -161,7 +161,6 @@ typedef struct subpage_t {
>  #define PHYS_SECTION_UNASSIGNED 0
>  
>  static void io_mem_init(void);
> -static void memory_map_init(void);

The memory_map_init() change is unnecessary once a softmmu target is
used since it will be called from cpu_exec_init_all().

> +static void remote_machine_init(Object *obj)
> +{
> +    RemMachineState *s = REMOTE_MACHINE(obj);
> +    RemPCIHost *rem_host;
> +    MemoryRegion *system_memory, *system_io, *pci_memory;
> +
> +    Error *error_abort = NULL;
> +
> +    object_property_add_child(object_get_root(), "machine", obj, 
> &error_abort);
> +    if (error_abort) {

error_abort aborts the program so handling it is not necessary.

> +        error_report_err(error_abort);
> +    }
> +
> +    memory_map_init();
> +
> +    system_memory = get_system_memory();
> +    system_io = get_system_io();
> +
> +    pci_memory = g_new(MemoryRegion, 1);
> +    memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
> +
> +    rem_host = REMOTE_HOST_DEVICE(qdev_create(NULL, 
> TYPE_REMOTE_HOST_DEVICE));
> +
> +    rem_host->mr_pci_mem = pci_memory;
> +    rem_host->mr_sys_mem = system_memory;
> +    rem_host->mr_sys_io = system_io;
> +
> +    s->host = rem_host;
> +
> +    object_property_add_child(OBJECT(s), "remote-device", OBJECT(rem_host),
> +                              &error_abort);
> +    if (error_abort) {

error_abort aborts the program so handling it is not necessary.

> +        error_report_err(error_abort);
> +        return;
> +    }
> +
> +    qemu_mutex_lock_iothread();

This will be executed with the iothread lock held. There is no need to
acquire it.

> +    memory_region_add_subregion_overlap(system_memory, 0x0, pci_memory, -1);
> +    qemu_mutex_unlock_iothread();
> +
> +    qdev_init_nofail(DEVICE(rem_host));
> +}

signature.asc
Description: PGP signature