Re: [PATCH 2/2] arm: xlnx-versal-virt: Add machine property ospi-flash
On Tue, 5 Dec 2023 at 09:52, Sai Pavan Boddu wrote:
>
> This property allows users to change flash model on command line as
> below.
>
>ex: "-M xlnx-versal-virt,ospi-flash=mt35xu02gbba"
>
> Signed-off-by: Sai Pavan Boddu
> ---
> hw/arm/xlnx-versal-virt.c | 31 ++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
> index 537118224f..c57cff74d8 100644
> --- a/hw/arm/xlnx-versal-virt.c
> +++ b/hw/arm/xlnx-versal-virt.c
> @@ -49,6 +49,7 @@ struct VersalVirt {
> struct {
> bool secure;
> } cfg;
> +char *ospi_model;
> };
>
> static void fdt_create(VersalVirt *s)
> @@ -637,6 +638,22 @@ static void sd_plugin_card(SDHCIState *sd, DriveInfo *di)
> _fatal);
> }
>
> +static char *versal_get_ospi_model(Object *obj, Error **errp)
> +{
> +VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(obj);
> +
> +return g_strdup(s->ospi_model);
> +}
> +
> +static void versal_set_ospi_model(Object *obj, const char *value, Error
> **errp)
> +{
> +VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(obj);
> +
> +g_free(s->ospi_model);
> +s->ospi_model = g_strdup(value);
> +}
> +
> +
> static void versal_virt_init(MachineState *machine)
> {
> VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(machine);
> @@ -736,7 +753,7 @@ static void versal_virt_init(MachineState *machine)
>
> spi_bus = qdev_get_child_bus(DEVICE(>soc.pmc.iou.ospi), "spi0");
>
> -flash_dev = qdev_new("mt35xu01g");
> +flash_dev = qdev_new(s->ospi_model ? s->ospi_model : "mt35xu01g");
> if (dinfo) {
> qdev_prop_set_drive_err(flash_dev, "drive",
> blk_by_legacy_dinfo(dinfo),
> _fatal);
This doesn't do any checking of the string the user passes,
which means the user can make us hit an abort() with a not
terribly helpful error message:
$ ./build/arm-clang/qemu-system-aarch64 -M xlnx-versal-virt,ospi-flash=bang
qemu-system-aarch64: unknown type 'bang'
Aborted (core dumped)
or complain about trying to create an abstract type:
$ ./build/arm-clang/qemu-system-aarch64 -M
xlnx-versal-virt,ospi-flash=m25p80-generic
**
ERROR:../../qom/object.c:525:object_initialize_with_type: assertion
failed: (type->abstract == false)
Bail out! ERROR:../../qom/object.c:525:object_initialize_with_type:
assertion failed: (type->abstract == false)
Aborted (core dumped)
or do some weird stuff if you pass it something that isn't a
flash device type name:
$ ./build/arm-clang/qemu-system-aarch64 -M xlnx-versal-virt,ospi-flash=e1000
Unexpected error in object_property_find_err() at ../../qom/object.c:1330:
qemu-system-aarch64: Property 'e1000.cs' not found
Aborted (core dumped)
I think you need to check that the string corresponds
to a type that actually exists and is a subtype of TYPE_M25P80
and isn't an abstract type.
thanks
-- PMM
Re: [PATCH 2/2] arm: xlnx-versal-virt: Add machine property ospi-flash
On [2023 Dec 05] Tue 15:22:26, Sai Pavan Boddu wrote:
> This property allows users to change flash model on command line as
> below.
>
>ex: "-M xlnx-versal-virt,ospi-flash=mt35xu02gbba"
>
> Signed-off-by: Sai Pavan Boddu
Reviewed-by: Francisco Iglesias
> ---
> hw/arm/xlnx-versal-virt.c | 31 ++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
> index 537118224f..c57cff74d8 100644
> --- a/hw/arm/xlnx-versal-virt.c
> +++ b/hw/arm/xlnx-versal-virt.c
> @@ -49,6 +49,7 @@ struct VersalVirt {
> struct {
> bool secure;
> } cfg;
> +char *ospi_model;
> };
>
> static void fdt_create(VersalVirt *s)
> @@ -637,6 +638,22 @@ static void sd_plugin_card(SDHCIState *sd, DriveInfo *di)
> _fatal);
> }
>
> +static char *versal_get_ospi_model(Object *obj, Error **errp)
> +{
> +VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(obj);
> +
> +return g_strdup(s->ospi_model);
> +}
> +
> +static void versal_set_ospi_model(Object *obj, const char *value, Error
> **errp)
> +{
> +VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(obj);
> +
> +g_free(s->ospi_model);
> +s->ospi_model = g_strdup(value);
> +}
> +
> +
> static void versal_virt_init(MachineState *machine)
> {
> VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(machine);
> @@ -736,7 +753,7 @@ static void versal_virt_init(MachineState *machine)
>
> spi_bus = qdev_get_child_bus(DEVICE(>soc.pmc.iou.ospi), "spi0");
>
> -flash_dev = qdev_new("mt35xu01g");
> +flash_dev = qdev_new(s->ospi_model ? s->ospi_model : "mt35xu01g");
> if (dinfo) {
> qdev_prop_set_drive_err(flash_dev, "drive",
> blk_by_legacy_dinfo(dinfo),
> _fatal);
> @@ -769,6 +786,13 @@ static void versal_virt_machine_instance_init(Object
> *obj)
> 0);
> }
>
> +static void versal_virt_machine_finalize(Object *obj)
> +{
> +VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(obj);
> +
> +g_free(s->ospi_model);
> +}
> +
> static void versal_virt_machine_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> @@ -780,6 +804,10 @@ static void versal_virt_machine_class_init(ObjectClass
> *oc, void *data)
> mc->default_cpus = XLNX_VERSAL_NR_ACPUS + XLNX_VERSAL_NR_RCPUS;
> mc->no_cdrom = true;
> mc->default_ram_id = "ddr";
> +object_class_property_add_str(oc, "ospi-flash", versal_get_ospi_model,
> + versal_set_ospi_model);
> +object_class_property_set_description(oc, "ospi-flash",
> + "Change the OSPI Flash model");
> }
>
> static const TypeInfo versal_virt_machine_init_typeinfo = {
> @@ -788,6 +816,7 @@ static const TypeInfo versal_virt_machine_init_typeinfo =
> {
> .class_init = versal_virt_machine_class_init,
> .instance_init = versal_virt_machine_instance_init,
> .instance_size = sizeof(VersalVirt),
> +.instance_finalize = versal_virt_machine_finalize,
> };
>
> static void versal_virt_machine_init_register_types(void)
> --
> 2.25.1
>
[PATCH 2/2] arm: xlnx-versal-virt: Add machine property ospi-flash
This property allows users to change flash model on command line as
below.
ex: "-M xlnx-versal-virt,ospi-flash=mt35xu02gbba"
Signed-off-by: Sai Pavan Boddu
---
hw/arm/xlnx-versal-virt.c | 31 ++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 537118224f..c57cff74d8 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -49,6 +49,7 @@ struct VersalVirt {
struct {
bool secure;
} cfg;
+char *ospi_model;
};
static void fdt_create(VersalVirt *s)
@@ -637,6 +638,22 @@ static void sd_plugin_card(SDHCIState *sd, DriveInfo *di)
_fatal);
}
+static char *versal_get_ospi_model(Object *obj, Error **errp)
+{
+VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(obj);
+
+return g_strdup(s->ospi_model);
+}
+
+static void versal_set_ospi_model(Object *obj, const char *value, Error **errp)
+{
+VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(obj);
+
+g_free(s->ospi_model);
+s->ospi_model = g_strdup(value);
+}
+
+
static void versal_virt_init(MachineState *machine)
{
VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(machine);
@@ -736,7 +753,7 @@ static void versal_virt_init(MachineState *machine)
spi_bus = qdev_get_child_bus(DEVICE(>soc.pmc.iou.ospi), "spi0");
-flash_dev = qdev_new("mt35xu01g");
+flash_dev = qdev_new(s->ospi_model ? s->ospi_model : "mt35xu01g");
if (dinfo) {
qdev_prop_set_drive_err(flash_dev, "drive",
blk_by_legacy_dinfo(dinfo), _fatal);
@@ -769,6 +786,13 @@ static void versal_virt_machine_instance_init(Object *obj)
0);
}
+static void versal_virt_machine_finalize(Object *obj)
+{
+VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(obj);
+
+g_free(s->ospi_model);
+}
+
static void versal_virt_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -780,6 +804,10 @@ static void versal_virt_machine_class_init(ObjectClass
*oc, void *data)
mc->default_cpus = XLNX_VERSAL_NR_ACPUS + XLNX_VERSAL_NR_RCPUS;
mc->no_cdrom = true;
mc->default_ram_id = "ddr";
+object_class_property_add_str(oc, "ospi-flash", versal_get_ospi_model,
+ versal_set_ospi_model);
+object_class_property_set_description(oc, "ospi-flash",
+ "Change the OSPI Flash model");
}
static const TypeInfo versal_virt_machine_init_typeinfo = {
@@ -788,6 +816,7 @@ static const TypeInfo versal_virt_machine_init_typeinfo = {
.class_init = versal_virt_machine_class_init,
.instance_init = versal_virt_machine_instance_init,
.instance_size = sizeof(VersalVirt),
+.instance_finalize = versal_virt_machine_finalize,
};
static void versal_virt_machine_init_register_types(void)
--
2.25.1
