| Message ID | 20220223090706.4888-9-damien.hedde@greensocs.com |
|---|---|
| State | New |
| Headers | show |
| Series | Initial support for machine creation via QMP | expand |
On 23/2/22 10:07, Damien Hedde wrote: > Add the property to configure a the base address of the ram. > The default value remains zero. > > This commit is needed to use the 'none' machine as a base, and > subsequently to dynamically populate it using qapi commands. Having > a non null 'ram' is really hard to workaround because of the actual > constraints on the generic loader: it prevents loading binaries > bigger than ram_size (with a null ram, we cannot load anything). > For now we need to be able to use the existing ram creation > feature of the none machine with a configurable base address. > > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > --- > hw/core/null-machine.c | 34 ++++++++++++++++++++++++++++++++-- > 1 file changed, 32 insertions(+), 2 deletions(-) > > diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c > index 7eb258af07..5fd1cc0218 100644 > --- a/hw/core/null-machine.c > +++ b/hw/core/null-machine.c > @@ -16,9 +16,11 @@ > #include "hw/boards.h" > #include "exec/address-spaces.h" > #include "hw/core/cpu.h" > +#include "qapi/visitor.h" > > struct NoneMachineState { > MachineState parent; > + uint64_t ram_addr; > }; > > #define TYPE_NONE_MACHINE MACHINE_TYPE_NAME("none") > @@ -26,6 +28,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(NoneMachineState, NONE_MACHINE) > > static void machine_none_init(MachineState *mch) > { > + NoneMachineState *nms = NONE_MACHINE(mch); > CPUState *cpu = NULL; > > /* Initialize CPU (if user asked for it) */ > @@ -37,9 +40,13 @@ static void machine_none_init(MachineState *mch) > } > } > > - /* RAM at address zero */ > + /* RAM at configured address (default: 0) */ > if (mch->ram) { > - memory_region_add_subregion(get_system_memory(), 0, mch->ram); > + memory_region_add_subregion(get_system_memory(), nms->ram_addr, > + mch->ram); > + } else if (nms->ram_addr) { > + error_report("'ram-addr' has been specified but the size is zero"); I'm not sure about this error message, IIUC we can get here if no ram backend is provided, not if we have one zero-sized. Otherwise LGTM. > + exit(1); > }
On 3/3/22 15:41, Philippe Mathieu-Daudé wrote: > On 23/2/22 10:07, Damien Hedde wrote: >> Add the property to configure a the base address of the ram. >> The default value remains zero. >> >> This commit is needed to use the 'none' machine as a base, and >> subsequently to dynamically populate it using qapi commands. Having >> a non null 'ram' is really hard to workaround because of the actual >> constraints on the generic loader: it prevents loading binaries >> bigger than ram_size (with a null ram, we cannot load anything). >> For now we need to be able to use the existing ram creation >> feature of the none machine with a configurable base address. >> >> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> >> --- >> hw/core/null-machine.c | 34 ++++++++++++++++++++++++++++++++-- >> 1 file changed, 32 insertions(+), 2 deletions(-) >> >> diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c >> index 7eb258af07..5fd1cc0218 100644 >> --- a/hw/core/null-machine.c >> +++ b/hw/core/null-machine.c >> @@ -16,9 +16,11 @@ >> #include "hw/boards.h" >> #include "exec/address-spaces.h" >> #include "hw/core/cpu.h" >> +#include "qapi/visitor.h" >> struct NoneMachineState { >> MachineState parent; >> + uint64_t ram_addr; >> }; >> #define TYPE_NONE_MACHINE MACHINE_TYPE_NAME("none") >> @@ -26,6 +28,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(NoneMachineState, >> NONE_MACHINE) >> static void machine_none_init(MachineState *mch) >> { >> + NoneMachineState *nms = NONE_MACHINE(mch); >> CPUState *cpu = NULL; >> /* Initialize CPU (if user asked for it) */ >> @@ -37,9 +40,13 @@ static void machine_none_init(MachineState *mch) >> } >> } >> - /* RAM at address zero */ >> + /* RAM at configured address (default: 0) */ >> if (mch->ram) { >> - memory_region_add_subregion(get_system_memory(), 0, mch->ram); >> + memory_region_add_subregion(get_system_memory(), nms->ram_addr, >> + mch->ram); >> + } else if (nms->ram_addr) { >> + error_report("'ram-addr' has been specified but the size is >> zero"); > > I'm not sure about this error message, IIUC we can get here if no ram > backend is provided, not if we have one zero-sized. Otherwise LGTM. You're most probably right. Keeping the ram_size to 0 is just one way of getting here. I can replace the message by a more generic formulation "'ram-addr' has been specified but the machine has no ram"
Tested-by: Jim Shu <jim.shu@sifive.com> On Fri, Mar 4, 2022 at 12:36 AM Damien Hedde <damien.hedde@greensocs.com> wrote: > > > On 3/3/22 15:41, Philippe Mathieu-Daudé wrote: > > On 23/2/22 10:07, Damien Hedde wrote: > >> Add the property to configure a the base address of the ram. > >> The default value remains zero. > >> > >> This commit is needed to use the 'none' machine as a base, and > >> subsequently to dynamically populate it using qapi commands. Having > >> a non null 'ram' is really hard to workaround because of the actual > >> constraints on the generic loader: it prevents loading binaries > >> bigger than ram_size (with a null ram, we cannot load anything). > >> For now we need to be able to use the existing ram creation > >> feature of the none machine with a configurable base address. > >> > >> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > >> --- > >> hw/core/null-machine.c | 34 ++++++++++++++++++++++++++++++++-- > >> 1 file changed, 32 insertions(+), 2 deletions(-) > >> > >> diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c > >> index 7eb258af07..5fd1cc0218 100644 > >> --- a/hw/core/null-machine.c > >> +++ b/hw/core/null-machine.c > >> @@ -16,9 +16,11 @@ > >> #include "hw/boards.h" > >> #include "exec/address-spaces.h" > >> #include "hw/core/cpu.h" > >> +#include "qapi/visitor.h" > >> struct NoneMachineState { > >> MachineState parent; > >> + uint64_t ram_addr; > >> }; > >> #define TYPE_NONE_MACHINE MACHINE_TYPE_NAME("none") > >> @@ -26,6 +28,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(NoneMachineState, > >> NONE_MACHINE) > >> static void machine_none_init(MachineState *mch) > >> { > >> + NoneMachineState *nms = NONE_MACHINE(mch); > >> CPUState *cpu = NULL; > >> /* Initialize CPU (if user asked for it) */ > >> @@ -37,9 +40,13 @@ static void machine_none_init(MachineState *mch) > >> } > >> } > >> - /* RAM at address zero */ > >> + /* RAM at configured address (default: 0) */ > >> if (mch->ram) { > >> - memory_region_add_subregion(get_system_memory(), 0, mch->ram); > >> + memory_region_add_subregion(get_system_memory(), nms->ram_addr, > >> + mch->ram); > >> + } else if (nms->ram_addr) { > >> + error_report("'ram-addr' has been specified but the size is > >> zero"); > > > > I'm not sure about this error message, IIUC we can get here if no ram > > backend is provided, not if we have one zero-sized. Otherwise LGTM. > > You're most probably right. Keeping the ram_size to 0 is just one way of > getting here. I can replace the message by a more generic formulation > "'ram-addr' has been specified but the machine has no ram" > > >
diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c index 7eb258af07..5fd1cc0218 100644 --- a/hw/core/null-machine.c +++ b/hw/core/null-machine.c @@ -16,9 +16,11 @@ #include "hw/boards.h" #include "exec/address-spaces.h" #include "hw/core/cpu.h" +#include "qapi/visitor.h" struct NoneMachineState { MachineState parent; + uint64_t ram_addr; }; #define TYPE_NONE_MACHINE MACHINE_TYPE_NAME("none") @@ -26,6 +28,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(NoneMachineState, NONE_MACHINE) static void machine_none_init(MachineState *mch) { + NoneMachineState *nms = NONE_MACHINE(mch); CPUState *cpu = NULL; /* Initialize CPU (if user asked for it) */ @@ -37,9 +40,13 @@ static void machine_none_init(MachineState *mch) } } - /* RAM at address zero */ + /* RAM at configured address (default: 0) */ if (mch->ram) { - memory_region_add_subregion(get_system_memory(), 0, mch->ram); + memory_region_add_subregion(get_system_memory(), nms->ram_addr, + mch->ram); + } else if (nms->ram_addr) { + error_report("'ram-addr' has been specified but the size is zero"); + exit(1); } if (mch->kernel_filename) { @@ -49,6 +56,22 @@ static void machine_none_init(MachineState *mch) } } +static void machine_none_get_ram_addr(Object *obj, Visitor *v, const char *name, + void *opaque, Error **errp) +{ + NoneMachineState *nms = NONE_MACHINE(obj); + + visit_type_uint64(v, name, &nms->ram_addr, errp); +} + +static void machine_none_set_ram_addr(Object *obj, Visitor *v, const char *name, + void *opaque, Error **errp) +{ + NoneMachineState *nms = NONE_MACHINE(obj); + + visit_type_uint64(v, name, &nms->ram_addr, errp); +} + static void machine_none_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); @@ -63,6 +86,13 @@ static void machine_none_class_init(ObjectClass *oc, void *data) mc->no_floppy = 1; mc->no_cdrom = 1; mc->no_sdcard = 1; + + object_class_property_add(oc, "ram-addr", "int", + machine_none_get_ram_addr, + machine_none_set_ram_addr, + NULL, NULL); + object_class_property_set_description(oc, "ram-addr", + "Base address of the RAM (default is 0)"); } static const TypeInfo none_machine_info = {
Add the property to configure a the base address of the ram. The default value remains zero. This commit is needed to use the 'none' machine as a base, and subsequently to dynamically populate it using qapi commands. Having a non null 'ram' is really hard to workaround because of the actual constraints on the generic loader: it prevents loading binaries bigger than ram_size (with a null ram, we cannot load anything). For now we need to be able to use the existing ram creation feature of the none machine with a configurable base address. Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> --- hw/core/null-machine.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-)