| Message ID | 20211208064252.375360-2-alistair.francis@opensource.wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | A collection of RISC-V cleanups and improvements | expand |
Hi Alistair, On 12/8/21 07:42, Alistair Francis wrote: > From: Alistair Francis <alistair.francis@wdc.com> > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > hw/intc/sifive_plic.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c > index 877e76877c..35f097799a 100644 > --- a/hw/intc/sifive_plic.c > +++ b/hw/intc/sifive_plic.c > @@ -355,6 +355,17 @@ static const MemoryRegionOps sifive_plic_ops = { > } > }; > > +static void sifive_plic_reset(DeviceState *dev) > +{ > + SiFivePLICState *s = SIFIVE_PLIC(dev); > + > + memset(s->source_priority, 0, sizeof(uint32_t) * s->num_sources); > + memset(s->target_priority, 0, sizeof(uint32_t) * s->num_addrs); > + memset(s->pending, 0, sizeof(uint32_t) * s->bitfield_words); > + memset(s->claimed, 0, sizeof(uint32_t) * s->bitfield_words); > + memset(s->enable, 0, sizeof(uint32_t) * s->num_enables); Looking at sifive_plic_realize(): - Should we reset the external IRQs in a default state? - Shouldn't riscv_cpu_claim_interrupts() be called at reset? Note: parse_hart_config() name is slightly confusing since beside parsing, it also allocates addr_config. Maybe consider renaming?
On Wed, Dec 8, 2021 at 10:00 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > Hi Alistair, > > On 12/8/21 07:42, Alistair Francis wrote: > > From: Alistair Francis <alistair.francis@wdc.com> > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > hw/intc/sifive_plic.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c > > index 877e76877c..35f097799a 100644 > > --- a/hw/intc/sifive_plic.c > > +++ b/hw/intc/sifive_plic.c > > @@ -355,6 +355,17 @@ static const MemoryRegionOps sifive_plic_ops = { > > } > > }; > > > > +static void sifive_plic_reset(DeviceState *dev) > > +{ > > + SiFivePLICState *s = SIFIVE_PLIC(dev); > > + > > + memset(s->source_priority, 0, sizeof(uint32_t) * s->num_sources); > > + memset(s->target_priority, 0, sizeof(uint32_t) * s->num_addrs); > > + memset(s->pending, 0, sizeof(uint32_t) * s->bitfield_words); > > + memset(s->claimed, 0, sizeof(uint32_t) * s->bitfield_words); > > + memset(s->enable, 0, sizeof(uint32_t) * s->num_enables); > > Looking at sifive_plic_realize(): > > - Should we reset the external IRQs in a default state? Good point, I'll add that. > - Shouldn't riscv_cpu_claim_interrupts() be called at reset? I don't think so. riscv_cpu_claim_interrupts is a once and done call. Alistair > > Note: parse_hart_config() name is slightly confusing since > beside parsing, it also allocates addr_config. Maybe consider > renaming?
diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c index 877e76877c..35f097799a 100644 --- a/hw/intc/sifive_plic.c +++ b/hw/intc/sifive_plic.c @@ -355,6 +355,17 @@ static const MemoryRegionOps sifive_plic_ops = { } }; +static void sifive_plic_reset(DeviceState *dev) +{ + SiFivePLICState *s = SIFIVE_PLIC(dev); + + memset(s->source_priority, 0, sizeof(uint32_t) * s->num_sources); + memset(s->target_priority, 0, sizeof(uint32_t) * s->num_addrs); + memset(s->pending, 0, sizeof(uint32_t) * s->bitfield_words); + memset(s->claimed, 0, sizeof(uint32_t) * s->bitfield_words); + memset(s->enable, 0, sizeof(uint32_t) * s->num_enables); +} + /* * parse PLIC hart/mode address offset config * @@ -501,6 +512,7 @@ static void sifive_plic_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); + dc->reset = sifive_plic_reset; device_class_set_props(dc, sifive_plic_properties); dc->realize = sifive_plic_realize; dc->vmsd = &vmstate_sifive_plic;