Re: [PATCH v4 08/14] none-machine: add 'ram-addr' property
Tested-by: Jim Shu
On Fri, Mar 4, 2022 at 12:36 AM Damien Hedde
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
> >> ---
> >> 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"
>
>
>
Re: [PATCH v4 08/14] none-machine: add 'ram-addr' property
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
---
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"
Re: [PATCH v4 08/14] none-machine: add 'ram-addr' property
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
---
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);
}
[PATCH v4 08/14] none-machine: add 'ram-addr' property
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
---
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");
+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, >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, >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 = {
--
2.35.1
