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

Philippe Mathieu-Daudé posted 5 patches 10 months, 4 weeks ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
[RFC PATCH 4/5] hw/arm/armv7m: Error if trying to set unavailable ARMCPU::vfp property
Posted by Philippe Mathieu-Daudé 10 months, 4 weeks ago
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;
     }
     if (object_property_find(OBJECT(s->cpu), "dsp")) {
         if (!object_property_set_bool(OBJECT(s->cpu), "dsp", s->dsp, errp)) {
-- 
2.41.0


Re: [RFC PATCH 4/5] hw/arm/armv7m: Error if trying to set unavailable ARMCPU::vfp property
Posted by Peter Maydell 10 months, 2 weeks ago
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