[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RESEND v6 14/36] multi-process: setup a machine object for re
From: |
Jag Raman |
Subject: |
Re: [PATCH RESEND v6 14/36] multi-process: setup a machine object for remote device process |
Date: |
Tue, 12 May 2020 08:12:48 -0400 |
> On May 12, 2020, at 6:43 AM, Stefan Hajnoczi <address@hidden> wrote:
>
> 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)
OK, got it.
>
>> 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().
OK.
>
>> +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.
OK, thanks!
>
>> + 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.
Yes, this wouldn’t be necessary from QEMU’s main loop.
Thanks!
--
Jag
>
>> + memory_region_add_subregion_overlap(system_memory, 0x0, pci_memory, -1);
>> + qemu_mutex_unlock_iothread();
>> +
>> + qdev_init_nofail(DEVICE(rem_host));
>> +}