| Message ID | 1484654591-11108-1-git-send-email-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jan 17, 2017 at 01:03:11PM +0100, Thomas Huth wrote: > Sometimes it is useful to have just a machine with CPU and RAM, without > any further hardware in it, e.g. if you just want to do some instruction > debugging for TCG with a remote GDB attached to QEMU, or run some embedded > code with the "-semihosting" QEMU parameter. qemu-system-m68k already > features a "dummy" machine, and xtensa a "sim" machine for exactly this > purpose. > All target architectures have nowadays also a "none" machine, which would > be a perfect match for this, too - but it currently does not allow to add > CPU, RAM or a kernel yet. Thus let's add these possibilities in a generic > way to the "none" machine, too, so that we hopefully do not need additional > "dummy" machines in the future anymore (and maybe can also get rid of the > already existing "dummy"/"sim" machines one day). > Note that the default behaviour of the "none" machine is not changed, i.e. > no CPU and no RAM is instantiated by default. You've explicitely got to > specify the CPU model with "-cpu" and the amount of RAM with "-m" to get > these new features. > We also introduce a wrapper called cpu_init_def() for the target-specific > macro cpu_init() in cpus.c here, so we can continue to compile the file > null-machine.c independently from the target. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > v2: > - Use the generic-loader device for providing the functionality of > the "-kernel" parameter Peter argued in v1 against providing a -kernel option that doesn't have the same capabilities as the other machines in the same architecture (I will continue the discussion there). Maybe you'll want to split this in multiple patches so we could include the RAM and CPU changes while discussing what to do about -kernel. > - Explicitely set mc->max_cpus = 1 > - Make sure that null-machine.c can be compiled independent from the > target (by introducing a wrapper function for cpu_init()) Most (or all?) architectures should work if you use cpu_generic_init(). I wonder how many architectures don't use cpu_generic_init() to implement cpu_init() yet. > > cpus.c | 5 +++++ > hw/core/null-machine.c | 40 ++++++++++++++++++++++++++++++++++++++-- > include/qom/cpu.h | 11 +++++++++++ > 3 files changed, 54 insertions(+), 2 deletions(-) > > diff --git a/cpus.c b/cpus.c > index 5213351..7c4dc38 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -80,6 +80,11 @@ static unsigned int throttle_percentage; > #define CPU_THROTTLE_PCT_MAX 99 > #define CPU_THROTTLE_TIMESLICE_NS 10000000 > > +CPUState *cpu_init_def(const char *cpu_model) > +{ > + return cpu_init(cpu_model); > +} > + So, now we have two interfaces to do exactly the same thing: cpu_init() and cpu_init_def(). But cpu_init() is a macro and cpu_init_def() is a function. cpu_init() is available only if you include cpu.h, but cpu_init_def() is available elsewhere. Ideally, code should be able to simply call a cpu_init() function, and it should work the same everywhere. In practice, cleaning this up might take a while, so cpu_init_def() might be a temporary solution. But now I am not sure if having this additional wrapper is better than simply making null-machine.o target-dependent like you did before. > bool cpu_is_stopped(CPUState *cpu) > { > return cpu->stopped || !runstate_is_running(); > diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c > index 0351ba7..eab5133 100644 > --- a/hw/core/null-machine.c > +++ b/hw/core/null-machine.c > @@ -13,18 +13,54 @@ > > #include "qemu/osdep.h" > #include "qemu-common.h" > +#include "qemu/error-report.h" > #include "hw/hw.h" > #include "hw/boards.h" > +#include "hw/core/generic-loader.h" > +#include "sysemu/sysemu.h" > +#include "exec/address-spaces.h" > +#include "qom/cpu.h" > > -static void machine_none_init(MachineState *machine) > +static void machine_none_init(MachineState *mch) > { > + CPUState *cpu = NULL; > + > + /* Initialize CPU (if a model has been specified) */ > + if (mch->cpu_model) { > + cpu = cpu_init_def(mch->cpu_model); > + if (!cpu) { > + error_report("Unable to initialize CPU"); > + exit(1); > + } > + } > + > + /* RAM at address zero */ > + if (mch->ram_size) { > + MemoryRegion *ram = g_new(MemoryRegion, 1); > + > + memory_region_allocate_system_memory(ram, NULL, "ram", mch->ram_size); > + memory_region_add_subregion(get_system_memory(), 0, ram); > + } > + > + /* Load kernel */ > + if (mch->kernel_filename) { > + DeviceState *loader; > + > + loader = qdev_create(sysbus_get_default(), TYPE_GENERIC_LOADER); > + qdev_prop_set_string(loader, "file", mch->kernel_filename); > + if (cpu) { > + qdev_prop_set_uint32(loader, "cpu-num", cpu->cpu_index); > + } > + qdev_init_nofail(loader); > + } > } > > static void machine_none_machine_init(MachineClass *mc) > { > mc->desc = "empty machine"; > mc->init = machine_none_init; > - mc->max_cpus = 0; > + mc->max_cpus = 1; > + mc->default_ram_size = 0; > } > > DEFINE_MACHINE("none", machine_none_machine_init) > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index 3f79a8e..c20da71 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -636,6 +636,17 @@ bool qemu_cpu_is_self(CPUState *cpu); > void qemu_cpu_kick(CPUState *cpu); > > /** > + * cpu_init_def: > + * @cpu_model: Specifies the CPU model which should be created. > + * > + * A wrapper around the cpu_init() macro which can be used in target > + * independent code, too. > + * > + * Returns: The CPUState of the created CPU. > + */ > +CPUState *cpu_init_def(const char *cpu_model); > + > +/** > * cpu_is_stopped: > * @cpu: The CPU to check. > * > -- > 1.8.3.1 >
On 17.01.2017 13:32, Eduardo Habkost wrote: > On Tue, Jan 17, 2017 at 01:03:11PM +0100, Thomas Huth wrote: >> Sometimes it is useful to have just a machine with CPU and RAM, without >> any further hardware in it, e.g. if you just want to do some instruction >> debugging for TCG with a remote GDB attached to QEMU, or run some embedded >> code with the "-semihosting" QEMU parameter. qemu-system-m68k already >> features a "dummy" machine, and xtensa a "sim" machine for exactly this >> purpose. >> All target architectures have nowadays also a "none" machine, which would >> be a perfect match for this, too - but it currently does not allow to add >> CPU, RAM or a kernel yet. Thus let's add these possibilities in a generic >> way to the "none" machine, too, so that we hopefully do not need additional >> "dummy" machines in the future anymore (and maybe can also get rid of the >> already existing "dummy"/"sim" machines one day). >> Note that the default behaviour of the "none" machine is not changed, i.e. >> no CPU and no RAM is instantiated by default. You've explicitely got to >> specify the CPU model with "-cpu" and the amount of RAM with "-m" to get >> these new features. >> We also introduce a wrapper called cpu_init_def() for the target-specific >> macro cpu_init() in cpus.c here, so we can continue to compile the file >> null-machine.c independently from the target. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> v2: >> - Use the generic-loader device for providing the functionality of >> the "-kernel" parameter > > Peter argued in v1 against providing a -kernel option that > doesn't have the same capabilities as the other machines in the > same architecture (I will continue the discussion there). I'd prefer to use the generic loader for -kernel, but yes, let's continue that discussion in the other thread. >> - Make sure that null-machine.c can be compiled independent from the >> target (by introducing a wrapper function for cpu_init()) > > Most (or all?) architectures should work if you use > cpu_generic_init(). I wonder how many architectures don't use > cpu_generic_init() to implement cpu_init() yet. I wanted to use cpu_generic_init() first, but that does not work for machine "none", since that function needs a "typename" parameter beside the "cpu_model", and I don't see any way to get hold of the correct string for that typename parameter in generic code like null-machine.c. Do you see any possibility to do that here? >> >> cpus.c | 5 +++++ >> hw/core/null-machine.c | 40 ++++++++++++++++++++++++++++++++++++++-- >> include/qom/cpu.h | 11 +++++++++++ >> 3 files changed, 54 insertions(+), 2 deletions(-) >> >> diff --git a/cpus.c b/cpus.c >> index 5213351..7c4dc38 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -80,6 +80,11 @@ static unsigned int throttle_percentage; >> #define CPU_THROTTLE_PCT_MAX 99 >> #define CPU_THROTTLE_TIMESLICE_NS 10000000 >> >> +CPUState *cpu_init_def(const char *cpu_model) >> +{ >> + return cpu_init(cpu_model); >> +} >> + > > So, now we have two interfaces to do exactly the same thing: > cpu_init() and cpu_init_def(). But cpu_init() is a macro and > cpu_init_def() is a function. cpu_init() is available only if you > include cpu.h, but cpu_init_def() is available elsewhere. > Ideally, code should be able to simply call a cpu_init() > function, and it should work the same everywhere. > > In practice, cleaning this up might take a while, so > cpu_init_def() might be a temporary solution. But now I am not > sure if having this additional wrapper is better than simply > making null-machine.o target-dependent like you did before. I don't mind either way ... Does anybody else got an opinion on this problem? Thomas
On Tue, Jan 17, 2017 at 02:10:35PM +0100, Thomas Huth wrote: > On 17.01.2017 13:32, Eduardo Habkost wrote: > > On Tue, Jan 17, 2017 at 01:03:11PM +0100, Thomas Huth wrote: > >> Sometimes it is useful to have just a machine with CPU and RAM, without > >> any further hardware in it, e.g. if you just want to do some instruction > >> debugging for TCG with a remote GDB attached to QEMU, or run some embedded > >> code with the "-semihosting" QEMU parameter. qemu-system-m68k already > >> features a "dummy" machine, and xtensa a "sim" machine for exactly this > >> purpose. > >> All target architectures have nowadays also a "none" machine, which would > >> be a perfect match for this, too - but it currently does not allow to add > >> CPU, RAM or a kernel yet. Thus let's add these possibilities in a generic > >> way to the "none" machine, too, so that we hopefully do not need additional > >> "dummy" machines in the future anymore (and maybe can also get rid of the > >> already existing "dummy"/"sim" machines one day). > >> Note that the default behaviour of the "none" machine is not changed, i.e. > >> no CPU and no RAM is instantiated by default. You've explicitely got to > >> specify the CPU model with "-cpu" and the amount of RAM with "-m" to get > >> these new features. > >> We also introduce a wrapper called cpu_init_def() for the target-specific > >> macro cpu_init() in cpus.c here, so we can continue to compile the file > >> null-machine.c independently from the target. > >> > >> Signed-off-by: Thomas Huth <thuth@redhat.com> > >> --- > >> v2: > >> - Use the generic-loader device for providing the functionality of > >> the "-kernel" parameter > > > > Peter argued in v1 against providing a -kernel option that > > doesn't have the same capabilities as the other machines in the > > same architecture (I will continue the discussion there). > > I'd prefer to use the generic loader for -kernel, but yes, let's > continue that discussion in the other thread. > > >> - Make sure that null-machine.c can be compiled independent from the > >> target (by introducing a wrapper function for cpu_init()) > > > > Most (or all?) architectures should work if you use > > cpu_generic_init(). I wonder how many architectures don't use > > cpu_generic_init() to implement cpu_init() yet. > > I wanted to use cpu_generic_init() first, but that does not work for > machine "none", since that function needs a "typename" parameter beside > the "cpu_model", and I don't see any way to get hold of the correct > string for that typename parameter in generic code like null-machine.c. Oops, you're right. > Do you see any possibility to do that here? This kind of information could be provided by arch_init.c, but currently that file is a bit messy. I will try to clean it up, but we will still need something that works in the meantime. > > >> > >> cpus.c | 5 +++++ > >> hw/core/null-machine.c | 40 ++++++++++++++++++++++++++++++++++++++-- > >> include/qom/cpu.h | 11 +++++++++++ > >> 3 files changed, 54 insertions(+), 2 deletions(-) > >> > >> diff --git a/cpus.c b/cpus.c > >> index 5213351..7c4dc38 100644 > >> --- a/cpus.c > >> +++ b/cpus.c > >> @@ -80,6 +80,11 @@ static unsigned int throttle_percentage; > >> #define CPU_THROTTLE_PCT_MAX 99 > >> #define CPU_THROTTLE_TIMESLICE_NS 10000000 > >> > >> +CPUState *cpu_init_def(const char *cpu_model) > >> +{ > >> + return cpu_init(cpu_model); > >> +} > >> + > > > > So, now we have two interfaces to do exactly the same thing: > > cpu_init() and cpu_init_def(). But cpu_init() is a macro and > > cpu_init_def() is a function. cpu_init() is available only if you > > include cpu.h, but cpu_init_def() is available elsewhere. > > Ideally, code should be able to simply call a cpu_init() > > function, and it should work the same everywhere. > > > > In practice, cleaning this up might take a while, so > > cpu_init_def() might be a temporary solution. But now I am not > > sure if having this additional wrapper is better than simply > > making null-machine.o target-dependent like you did before. > > I don't mind either way ... > Does anybody else got an opinion on this problem? Well, it's not the first time we have arch-dependent code in hw/core, so I think it should be OK to move it to obj-$(CONFIG_SOFTMMU) to keep things simpler.
diff --git a/cpus.c b/cpus.c index 5213351..7c4dc38 100644 --- a/cpus.c +++ b/cpus.c @@ -80,6 +80,11 @@ static unsigned int throttle_percentage; #define CPU_THROTTLE_PCT_MAX 99 #define CPU_THROTTLE_TIMESLICE_NS 10000000 +CPUState *cpu_init_def(const char *cpu_model) +{ + return cpu_init(cpu_model); +} + bool cpu_is_stopped(CPUState *cpu) { return cpu->stopped || !runstate_is_running(); diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c index 0351ba7..eab5133 100644 --- a/hw/core/null-machine.c +++ b/hw/core/null-machine.c @@ -13,18 +13,54 @@ #include "qemu/osdep.h" #include "qemu-common.h" +#include "qemu/error-report.h" #include "hw/hw.h" #include "hw/boards.h" +#include "hw/core/generic-loader.h" +#include "sysemu/sysemu.h" +#include "exec/address-spaces.h" +#include "qom/cpu.h" -static void machine_none_init(MachineState *machine) +static void machine_none_init(MachineState *mch) { + CPUState *cpu = NULL; + + /* Initialize CPU (if a model has been specified) */ + if (mch->cpu_model) { + cpu = cpu_init_def(mch->cpu_model); + if (!cpu) { + error_report("Unable to initialize CPU"); + exit(1); + } + } + + /* RAM at address zero */ + if (mch->ram_size) { + MemoryRegion *ram = g_new(MemoryRegion, 1); + + memory_region_allocate_system_memory(ram, NULL, "ram", mch->ram_size); + memory_region_add_subregion(get_system_memory(), 0, ram); + } + + /* Load kernel */ + if (mch->kernel_filename) { + DeviceState *loader; + + loader = qdev_create(sysbus_get_default(), TYPE_GENERIC_LOADER); + qdev_prop_set_string(loader, "file", mch->kernel_filename); + if (cpu) { + qdev_prop_set_uint32(loader, "cpu-num", cpu->cpu_index); + } + qdev_init_nofail(loader); + } } static void machine_none_machine_init(MachineClass *mc) { mc->desc = "empty machine"; mc->init = machine_none_init; - mc->max_cpus = 0; + mc->max_cpus = 1; + mc->default_ram_size = 0; } DEFINE_MACHINE("none", machine_none_machine_init) diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 3f79a8e..c20da71 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -636,6 +636,17 @@ bool qemu_cpu_is_self(CPUState *cpu); void qemu_cpu_kick(CPUState *cpu); /** + * cpu_init_def: + * @cpu_model: Specifies the CPU model which should be created. + * + * A wrapper around the cpu_init() macro which can be used in target + * independent code, too. + * + * Returns: The CPUState of the created CPU. + */ +CPUState *cpu_init_def(const char *cpu_model); + +/** * cpu_is_stopped: * @cpu: The CPU to check. *
Sometimes it is useful to have just a machine with CPU and RAM, without any further hardware in it, e.g. if you just want to do some instruction debugging for TCG with a remote GDB attached to QEMU, or run some embedded code with the "-semihosting" QEMU parameter. qemu-system-m68k already features a "dummy" machine, and xtensa a "sim" machine for exactly this purpose. All target architectures have nowadays also a "none" machine, which would be a perfect match for this, too - but it currently does not allow to add CPU, RAM or a kernel yet. Thus let's add these possibilities in a generic way to the "none" machine, too, so that we hopefully do not need additional "dummy" machines in the future anymore (and maybe can also get rid of the already existing "dummy"/"sim" machines one day). Note that the default behaviour of the "none" machine is not changed, i.e. no CPU and no RAM is instantiated by default. You've explicitely got to specify the CPU model with "-cpu" and the amount of RAM with "-m" to get these new features. We also introduce a wrapper called cpu_init_def() for the target-specific macro cpu_init() in cpus.c here, so we can continue to compile the file null-machine.c independently from the target. Signed-off-by: Thomas Huth <thuth@redhat.com> --- v2: - Use the generic-loader device for providing the functionality of the "-kernel" parameter - Explicitely set mc->max_cpus = 1 - Make sure that null-machine.c can be compiled independent from the target (by introducing a wrapper function for cpu_init()) cpus.c | 5 +++++ hw/core/null-machine.c | 40 ++++++++++++++++++++++++++++++++++++++-- include/qom/cpu.h | 11 +++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-)