| Message ID | 20190517174046.11146-5-peter.maydell@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Disable FPU/DSP for CPU0 on musca-a and mps2-an521 | expand |
Peter Maydell <peter.maydell@linaro.org> writes: > The SSE-200 hardware has configurable integration settings which > determine whether its two CPUs have the FPU and DSP: > * CPU0_FPU (default 0) > * CPU0_DSP (default 0) > * CPU1_FPU (default 1) > * CPU1_DSP (default 1) > > Similarly, the IoTKit has settings for its single CPU: > * CPU0_FPU (default 1) > * CPU0_DSP (default 1) > > Of our four boards that use either the IoTKit or the SSE-200: > * mps2-an505, mps2-an521 and musca-a use the default settings > * musca-b1 enables FPU and DSP on both CPUs > > Currently QEMU models all these boards using CPUs with > both FPU and DSP enabled. This means that we are incorrect > for mps2-an521 and musca-a, which should not have FPU or DSP > on CPU0. > > Create QOM properties on the ARMSSE devices corresponding to the > default h/w integration settings, and make the Musca-B1 board > enable FPU and DSP on both CPUs. This fixes the mps2-an521 > and musca-a behaviour, and leaves the musca-b1 and mps2-an505 > behaviour unchanged. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Given the ever growing configurable nature of the v7m platform what do we do to ensure the various combinations are tested on instantiating qemu? Or is this a case of relying on the wider community to shout if users actually find a combination that breaks? I guess fuzz testing would be a bit of a sledgehammer approach. Anyway: Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > --- > include/hw/arm/armsse.h | 7 +++++ > hw/arm/armsse.c | 58 ++++++++++++++++++++++++++++++++--------- > hw/arm/musca.c | 8 ++++++ > 3 files changed, 61 insertions(+), 12 deletions(-) > > diff --git a/include/hw/arm/armsse.h b/include/hw/arm/armsse.h > index 81e082cccf8..84080c22993 100644 > --- a/include/hw/arm/armsse.h > +++ b/include/hw/arm/armsse.h > @@ -50,6 +50,11 @@ > * address of each SRAM bank (and thus the total amount of internal SRAM) > * + QOM property "init-svtor" sets the initial value of the CPU SVTOR register > * (where it expects to load the PC and SP from the vector table on reset) > + * + QOM properties "CPU0_FPU", "CPU0_DSP", "CPU1_FPU" and "CPU1_DSP" which > + * set whether the CPUs have the FPU and DSP features present. The default > + * (matching the hardware) is that for CPU0 in an IoTKit and CPU1 in an > + * SSE-200 both are present; CPU0 in an SSE-200 has neither. > + * Since the IoTKit has only one CPU, it does not have the CPU1_* properties. > * + Named GPIO inputs "EXP_IRQ" 0..n are the expansion interrupts for CPU 0, > * which are wired to its NVIC lines 32 .. n+32 > * + Named GPIO inputs "EXP_CPU1_IRQ" 0..n are the expansion interrupts for > @@ -208,6 +213,8 @@ typedef struct ARMSSE { > uint32_t mainclk_frq; > uint32_t sram_addr_width; > uint32_t init_svtor; > + bool cpu_fpu[SSE_MAX_CPUS]; > + bool cpu_dsp[SSE_MAX_CPUS]; > } ARMSSE; > > typedef struct ARMSSEInfo ARMSSEInfo; > diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c > index 76cc6905798..e138aee673f 100644 > --- a/hw/arm/armsse.c > +++ b/hw/arm/armsse.c > @@ -37,6 +37,33 @@ struct ARMSSEInfo { > bool has_cachectrl; > bool has_cpusecctrl; > bool has_cpuid; > + Property *props; > +}; > + > +static Property iotkit_properties[] = { > + DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION, > + MemoryRegion *), > + DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64), > + DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0), > + DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15), > + DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000), > + DEFINE_PROP_BOOL("CPU0_FPU", ARMSSE, cpu_fpu[0], true), > + DEFINE_PROP_BOOL("CPU0_DSP", ARMSSE, cpu_dsp[0], true), > + DEFINE_PROP_END_OF_LIST() > +}; > + > +static Property armsse_properties[] = { > + DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION, > + MemoryRegion *), > + DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64), > + DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0), > + DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15), > + DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000), > + DEFINE_PROP_BOOL("CPU0_FPU", ARMSSE, cpu_fpu[0], false), > + DEFINE_PROP_BOOL("CPU0_DSP", ARMSSE, cpu_dsp[0], false), > + DEFINE_PROP_BOOL("CPU1_FPU", ARMSSE, cpu_fpu[1], true), > + DEFINE_PROP_BOOL("CPU1_DSP", ARMSSE, cpu_dsp[1], true), > + DEFINE_PROP_END_OF_LIST() > }; > > static const ARMSSEInfo armsse_variants[] = { > @@ -52,6 +79,7 @@ static const ARMSSEInfo armsse_variants[] = { > .has_cachectrl = false, > .has_cpusecctrl = false, > .has_cpuid = false, > + .props = iotkit_properties, > }, > { > .name = TYPE_SSE200, > @@ -65,6 +93,7 @@ static const ARMSSEInfo armsse_variants[] = { > .has_cachectrl = true, > .has_cpusecctrl = true, > .has_cpuid = true, > + .props = armsse_properties, > }, > }; > > @@ -532,6 +561,20 @@ static void armsse_realize(DeviceState *dev, Error **errp) > return; > } > } > + if (!s->cpu_fpu[i]) { > + object_property_set_bool(cpuobj, false, "vfp", &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > + } > + if (!s->cpu_dsp[i]) { > + object_property_set_bool(cpuobj, false, "dsp", &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > + } > > if (i > 0) { > memory_region_add_subregion_overlap(&s->cpu_container[i], 0, > @@ -1221,16 +1264,6 @@ static const VMStateDescription armsse_vmstate = { > } > }; > > -static Property armsse_properties[] = { > - DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION, > - MemoryRegion *), > - DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64), > - DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0), > - DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15), > - DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000), > - DEFINE_PROP_END_OF_LIST() > -}; > - > static void armsse_reset(DeviceState *dev) > { > ARMSSE *s = ARMSSE(dev); > @@ -1243,13 +1276,14 @@ static void armsse_class_init(ObjectClass *klass, void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > IDAUInterfaceClass *iic = IDAU_INTERFACE_CLASS(klass); > ARMSSEClass *asc = ARMSSE_CLASS(klass); > + const ARMSSEInfo *info = data; > > dc->realize = armsse_realize; > dc->vmsd = &armsse_vmstate; > - dc->props = armsse_properties; > + dc->props = info->props; > dc->reset = armsse_reset; > iic->check = armsse_idau_check; > - asc->info = data; > + asc->info = info; > } > > static const TypeInfo armsse_info = { > diff --git a/hw/arm/musca.c b/hw/arm/musca.c > index 23aff43f4bc..736f37b774c 100644 > --- a/hw/arm/musca.c > +++ b/hw/arm/musca.c > @@ -385,6 +385,14 @@ static void musca_init(MachineState *machine) > qdev_prop_set_uint32(ssedev, "init-svtor", mmc->init_svtor); > qdev_prop_set_uint32(ssedev, "SRAM_ADDR_WIDTH", mmc->sram_addr_width); > qdev_prop_set_uint32(ssedev, "MAINCLK", SYSCLK_FRQ); > + /* > + * Musca-A takes the default SSE-200 FPU/DSP settings (ie no for > + * CPU0 and yes for CPU1); Musca-B1 explicitly enables them for CPU0. > + */ > + if (mmc->type == MUSCA_B1) { > + qdev_prop_set_bit(ssedev, "CPU0_FPU", true); > + qdev_prop_set_bit(ssedev, "CPU0_DSP", true); > + } > object_property_set_bool(OBJECT(&mms->sse), true, "realized", > &error_fatal); -- Alex Bennée
On Thu, 13 Jun 2019 at 16:13, Alex Bennée <alex.bennee@linaro.org> wrote: > Given the ever growing configurable nature of the v7m platform what do we do > to ensure the various combinations are tested on instantiating qemu? Or is > this a case of relying on the wider community to shout if users actually > find a combination that breaks? I guess fuzz testing would be a bit of a > sledgehammer approach. It's not really all that much more configurable than the A-profile cores; I've just been a bit more careful to actually make our emulation reflect what the hardware actually does... a really careful testing of the A profile cores would want to test the various different cortex-* CPUs individually to make sure they UNDEFfed on the features they didn't have, and so on. thanks -- PMM
diff --git a/include/hw/arm/armsse.h b/include/hw/arm/armsse.h index 81e082cccf8..84080c22993 100644 --- a/include/hw/arm/armsse.h +++ b/include/hw/arm/armsse.h @@ -50,6 +50,11 @@ * address of each SRAM bank (and thus the total amount of internal SRAM) * + QOM property "init-svtor" sets the initial value of the CPU SVTOR register * (where it expects to load the PC and SP from the vector table on reset) + * + QOM properties "CPU0_FPU", "CPU0_DSP", "CPU1_FPU" and "CPU1_DSP" which + * set whether the CPUs have the FPU and DSP features present. The default + * (matching the hardware) is that for CPU0 in an IoTKit and CPU1 in an + * SSE-200 both are present; CPU0 in an SSE-200 has neither. + * Since the IoTKit has only one CPU, it does not have the CPU1_* properties. * + Named GPIO inputs "EXP_IRQ" 0..n are the expansion interrupts for CPU 0, * which are wired to its NVIC lines 32 .. n+32 * + Named GPIO inputs "EXP_CPU1_IRQ" 0..n are the expansion interrupts for @@ -208,6 +213,8 @@ typedef struct ARMSSE { uint32_t mainclk_frq; uint32_t sram_addr_width; uint32_t init_svtor; + bool cpu_fpu[SSE_MAX_CPUS]; + bool cpu_dsp[SSE_MAX_CPUS]; } ARMSSE; typedef struct ARMSSEInfo ARMSSEInfo; diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c index 76cc6905798..e138aee673f 100644 --- a/hw/arm/armsse.c +++ b/hw/arm/armsse.c @@ -37,6 +37,33 @@ struct ARMSSEInfo { bool has_cachectrl; bool has_cpusecctrl; bool has_cpuid; + Property *props; +}; + +static Property iotkit_properties[] = { + DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION, + MemoryRegion *), + DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64), + DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0), + DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15), + DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000), + DEFINE_PROP_BOOL("CPU0_FPU", ARMSSE, cpu_fpu[0], true), + DEFINE_PROP_BOOL("CPU0_DSP", ARMSSE, cpu_dsp[0], true), + DEFINE_PROP_END_OF_LIST() +}; + +static Property armsse_properties[] = { + DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION, + MemoryRegion *), + DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64), + DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0), + DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15), + DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000), + DEFINE_PROP_BOOL("CPU0_FPU", ARMSSE, cpu_fpu[0], false), + DEFINE_PROP_BOOL("CPU0_DSP", ARMSSE, cpu_dsp[0], false), + DEFINE_PROP_BOOL("CPU1_FPU", ARMSSE, cpu_fpu[1], true), + DEFINE_PROP_BOOL("CPU1_DSP", ARMSSE, cpu_dsp[1], true), + DEFINE_PROP_END_OF_LIST() }; static const ARMSSEInfo armsse_variants[] = { @@ -52,6 +79,7 @@ static const ARMSSEInfo armsse_variants[] = { .has_cachectrl = false, .has_cpusecctrl = false, .has_cpuid = false, + .props = iotkit_properties, }, { .name = TYPE_SSE200, @@ -65,6 +93,7 @@ static const ARMSSEInfo armsse_variants[] = { .has_cachectrl = true, .has_cpusecctrl = true, .has_cpuid = true, + .props = armsse_properties, }, }; @@ -532,6 +561,20 @@ static void armsse_realize(DeviceState *dev, Error **errp) return; } } + if (!s->cpu_fpu[i]) { + object_property_set_bool(cpuobj, false, "vfp", &err); + if (err) { + error_propagate(errp, err); + return; + } + } + if (!s->cpu_dsp[i]) { + object_property_set_bool(cpuobj, false, "dsp", &err); + if (err) { + error_propagate(errp, err); + return; + } + } if (i > 0) { memory_region_add_subregion_overlap(&s->cpu_container[i], 0, @@ -1221,16 +1264,6 @@ static const VMStateDescription armsse_vmstate = { } }; -static Property armsse_properties[] = { - DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION, - MemoryRegion *), - DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64), - DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0), - DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15), - DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000), - DEFINE_PROP_END_OF_LIST() -}; - static void armsse_reset(DeviceState *dev) { ARMSSE *s = ARMSSE(dev); @@ -1243,13 +1276,14 @@ static void armsse_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); IDAUInterfaceClass *iic = IDAU_INTERFACE_CLASS(klass); ARMSSEClass *asc = ARMSSE_CLASS(klass); + const ARMSSEInfo *info = data; dc->realize = armsse_realize; dc->vmsd = &armsse_vmstate; - dc->props = armsse_properties; + dc->props = info->props; dc->reset = armsse_reset; iic->check = armsse_idau_check; - asc->info = data; + asc->info = info; } static const TypeInfo armsse_info = { diff --git a/hw/arm/musca.c b/hw/arm/musca.c index 23aff43f4bc..736f37b774c 100644 --- a/hw/arm/musca.c +++ b/hw/arm/musca.c @@ -385,6 +385,14 @@ static void musca_init(MachineState *machine) qdev_prop_set_uint32(ssedev, "init-svtor", mmc->init_svtor); qdev_prop_set_uint32(ssedev, "SRAM_ADDR_WIDTH", mmc->sram_addr_width); qdev_prop_set_uint32(ssedev, "MAINCLK", SYSCLK_FRQ); + /* + * Musca-A takes the default SSE-200 FPU/DSP settings (ie no for + * CPU0 and yes for CPU1); Musca-B1 explicitly enables them for CPU0. + */ + if (mmc->type == MUSCA_B1) { + qdev_prop_set_bit(ssedev, "CPU0_FPU", true); + qdev_prop_set_bit(ssedev, "CPU0_DSP", true); + } object_property_set_bool(OBJECT(&mms->sse), true, "realized", &error_fatal);
The SSE-200 hardware has configurable integration settings which determine whether its two CPUs have the FPU and DSP: * CPU0_FPU (default 0) * CPU0_DSP (default 0) * CPU1_FPU (default 1) * CPU1_DSP (default 1) Similarly, the IoTKit has settings for its single CPU: * CPU0_FPU (default 1) * CPU0_DSP (default 1) Of our four boards that use either the IoTKit or the SSE-200: * mps2-an505, mps2-an521 and musca-a use the default settings * musca-b1 enables FPU and DSP on both CPUs Currently QEMU models all these boards using CPUs with both FPU and DSP enabled. This means that we are incorrect for mps2-an521 and musca-a, which should not have FPU or DSP on CPU0. Create QOM properties on the ARMSSE devices corresponding to the default h/w integration settings, and make the Musca-B1 board enable FPU and DSP on both CPUs. This fixes the mps2-an521 and musca-a behaviour, and leaves the musca-b1 and mps2-an505 behaviour unchanged. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/armsse.h | 7 +++++ hw/arm/armsse.c | 58 ++++++++++++++++++++++++++++++++--------- hw/arm/musca.c | 8 ++++++ 3 files changed, 61 insertions(+), 12 deletions(-)