[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH-for-5.0 12/12] hw/riscv/sifive_u: Add missing error-propagati

From: Philippe Mathieu-Daudé
Subject: Re: [PATCH-for-5.0 12/12] hw/riscv/sifive_u: Add missing error-propagation code
Date: Tue, 31 Mar 2020 19:02:35 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0

On 3/26/20 10:55 PM, Peter Maydell wrote:
On Wed, 25 Mar 2020 at 19:19, Philippe Mathieu-Daudé <address@hidden> wrote:

Running the coccinelle script produced:

   $ spatch \
     --macro-file scripts/cocci-macro-file.h --include-headers \
     --sp-file scripts/coccinelle/object_property_missing_error_propagate.cocci 
     --keep-comments --smpl-spacing --dir hw

   [[manual check required: error_propagate() might be missing in 
object_property_set_bool() hw/riscv/sifive_u.c:558:4]]
   [[manual check required: error_propagate() might be missing in 
object_property_set_bool() hw/riscv/sifive_u.c:561:4]]

Add the missing error_propagate() after manual review.

Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
  hw/riscv/sifive_u.c | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 56351c4faa..01e44018cd 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -473,113 +473,121 @@ static void riscv_sifive_u_machine_instance_init(Object 
  static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
      MachineState *ms = MACHINE(qdev_get_machine());
      SiFiveUSoCState *s = RISCV_U_SOC(dev);
      const struct MemmapEntry *memmap = sifive_u_memmap;
      MemoryRegion *system_memory = get_system_memory();
      MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
      MemoryRegion *l2lim_mem = g_new(MemoryRegion, 1);
      qemu_irq plic_gpios[SIFIVE_U_PLIC_NUM_SOURCES];
      char *plic_hart_config;
      size_t plic_hart_config_len;
      int i;
      Error *err = NULL;
      NICInfo *nd = &nd_table[0];

      object_property_set_bool(OBJECT(&s->e_cpus), true, "realized",
      object_property_set_bool(OBJECT(&s->u_cpus), true, "realized",
       * The cluster must be realized after the RISC-V hart array container,
       * as the container's CPU object is only created on realize, and the
       * CPU must exist and have been parented into the cluster before the
       * cluster is realized.
      object_property_set_bool(OBJECT(&s->e_cluster), true, "realized",
      object_property_set_bool(OBJECT(&s->u_cluster), true, "realized",

Different bug noticed in passing: these really ought not to be
using error_abort to realize things, as realize is a fairly
likely-to-fail operation on most objects (either now or in
the future if the object implementation changes).


      /* boot rom */
      memory_region_init_rom(mask_rom, OBJECT(dev), "riscv.sifive.u.mrom",
                             memmap[SIFIVE_U_MROM].size, &error_fatal);

What about this memory_region_init_rom() call (and later memory_region_init_ram) using error_fatal, same reasoning right?

      memory_region_add_subregion(system_memory, memmap[SIFIVE_U_MROM].base,

      object_property_set_bool(OBJECT(&s->prci), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
      sysbus_mmio_map(SYS_BUS_DEVICE(&s->prci), 0, memmap[SIFIVE_U_PRCI].base);

      object_property_set_bool(OBJECT(&s->otp), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }

The changes made in this patch are fine though:
Reviewed-by: Peter Maydell <address@hidden>

-- PMM

reply via email to

[Prev in Thread] Current Thread [Next in Thread]