diff mbox series

[RFC,4/5] hw/arm/armv7m: Error if trying to set unavailable ARMCPU::vfp property

Message ID 20240102160455.68612-5-philmd@linaro.org
State New
Headers show
Series qdev-properties: Try to improve use of dynamic property introspection | expand

Commit Message

Philippe Mathieu-Daudé Jan. 2, 2024, 4:04 p.m. UTC
Do not ignore impossible configuration requested by the user.
For example, when trying to enable VFP on a Cortex-M33 we now get:

  qemu-system-arm: 'cortex-m33-arm-cpu' does not support VFP

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/armv7m.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Peter Maydell Jan. 12, 2024, 4:41 p.m. UTC | #1
On Tue, 2 Jan 2024 at 16:05, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Do not ignore impossible configuration requested by the user.
> For example, when trying to enable VFP on a Cortex-M33 we now get:
>
>   qemu-system-arm: 'cortex-m33-arm-cpu' does not support VFP
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/arm/armv7m.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 3610f6f4a1..12cdad09f9 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -328,6 +328,9 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
>          if (!object_property_set_bool(OBJECT(s->cpu), "vfp", s->vfp, errp)) {
>              return;
>          }
> +    } else if (s->vfp == OPTIONAL_BOOL_TRUE) {
> +        error_setg(errp, "'%s' does not support VFP", s->cpu_type);
> +        return;
>      }

I'm not sure exactly what this series is trying to do, but
this isn't the right error message, at least at the moment.
Our Cortex-M33 model does support VFP -- in fact, there's
currently no way to turn it off, since we only expose the vfp
property for AArch64 CPUs.

I think we broke this in commit 4315f7c61474 last year,
accidentally restricting the definition of the "vfp"
property to ARM_FEATURE_AARCH64 CPUs only.
(filed https://gitlab.com/qemu-project/qemu/-/issues/2098
to track that)

I suppose if we fixed that regression then the error message
would become correct again, since we'd be back to exposing
the 'vfp' property only if the CPU did support VFP.

-- PMM
diff mbox series

Patch

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 3610f6f4a1..12cdad09f9 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -328,6 +328,9 @@  static void armv7m_realize(DeviceState *dev, Error **errp)
         if (!object_property_set_bool(OBJECT(s->cpu), "vfp", s->vfp, errp)) {
             return;
         }
+    } else if (s->vfp == OPTIONAL_BOOL_TRUE) {
+        error_setg(errp, "'%s' does not support VFP", s->cpu_type);
+        return;
     }
     if (object_property_find(OBJECT(s->cpu), "dsp")) {
         if (!object_property_set_bool(OBJECT(s->cpu), "dsp", s->dsp, errp)) {