[PATCH-for-9.0 05/11] target/arm: Move GTIMER definitions to 'cpu-defs.h'

Philippe Mathieu-Daudé posted 11 patches 1 year ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>
[PATCH-for-9.0 05/11] target/arm: Move GTIMER definitions to 'cpu-defs.h'
Posted by Philippe Mathieu-Daudé 1 year ago
To allow GTIMER_* definitions to be used by non-ARM specific
hardware models, move them to a new target agnostic "cpu-defs.h"
header.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/cpu-defs.h | 19 +++++++++++++++++++
 target/arm/cpu.h      |  8 +-------
 hw/arm/bcm2836.c      |  1 +
 3 files changed, 21 insertions(+), 7 deletions(-)
 create mode 100644 target/arm/cpu-defs.h

diff --git a/target/arm/cpu-defs.h b/target/arm/cpu-defs.h
new file mode 100644
index 0000000000..1ad76aff14
--- /dev/null
+++ b/target/arm/cpu-defs.h
@@ -0,0 +1,19 @@
+/*
+ * ARM "target agnostic" CPU definitions
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef ARM_CPU_DEFS_H
+#define ARM_CPU_DEFS_H
+
+#define GTIMER_PHYS     0
+#define GTIMER_VIRT     1
+#define GTIMER_HYP      2
+#define GTIMER_SEC      3
+#define GTIMER_HYPVIRT  4
+#define NUM_GTIMERS     5
+
+#endif
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 124d829742..8107e4d446 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -24,6 +24,7 @@
 #include "qemu/cpu-float.h"
 #include "hw/registerfields.h"
 #include "cpu-qom.h"
+#include "target/arm/cpu-defs.h"
 #include "exec/cpu-defs.h"
 #include "qapi/qapi-types-common.h"
 
@@ -154,13 +155,6 @@ typedef struct ARMGenericTimer {
     uint64_t ctl; /* Timer Control register */
 } ARMGenericTimer;
 
-#define GTIMER_PHYS     0
-#define GTIMER_VIRT     1
-#define GTIMER_HYP      2
-#define GTIMER_SEC      3
-#define GTIMER_HYPVIRT  4
-#define NUM_GTIMERS     5
-
 #define VTCR_NSW (1u << 29)
 #define VTCR_NSA (1u << 30)
 #define VSTCR_SW VTCR_NSW
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 166dc896c0..6986b71cb4 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -15,6 +15,7 @@
 #include "hw/arm/bcm2836.h"
 #include "hw/arm/raspi_platform.h"
 #include "hw/sysbus.h"
+#include "target/arm/cpu-defs.h"
 
 struct BCM283XClass {
     /*< private >*/
-- 
2.41.0


Re: [PATCH-for-9.0 05/11] target/arm: Move GTIMER definitions to 'cpu-defs.h'
Posted by Richard Henderson 12 months ago
On 11/22/23 12:30, Philippe Mathieu-Daudé wrote:
> To allow GTIMER_* definitions to be used by non-ARM specific
> hardware models, move them to a new target agnostic "cpu-defs.h"
> header.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/arm/cpu-defs.h | 19 +++++++++++++++++++
>   target/arm/cpu.h      |  8 +-------
>   hw/arm/bcm2836.c      |  1 +
>   3 files changed, 21 insertions(+), 7 deletions(-)
>   create mode 100644 target/arm/cpu-defs.h
> 
> diff --git a/target/arm/cpu-defs.h b/target/arm/cpu-defs.h
> new file mode 100644
> index 0000000000..1ad76aff14
> --- /dev/null
> +++ b/target/arm/cpu-defs.h
> @@ -0,0 +1,19 @@
> +/*
> + * ARM "target agnostic" CPU definitions
> + *
> + *  Copyright (c) 2003 Fabrice Bellard
> + *
> + * SPDX-License-Identifier: LGPL-2.1-or-later
> + */
> +
> +#ifndef ARM_CPU_DEFS_H
> +#define ARM_CPU_DEFS_H
> +
> +#define GTIMER_PHYS     0
> +#define GTIMER_VIRT     1
> +#define GTIMER_HYP      2
> +#define GTIMER_SEC      3
> +#define GTIMER_HYPVIRT  4
> +#define NUM_GTIMERS     5
> +
> +#endif

Hmm.  cpu-defs.h is pretty generic.
Without looking forward in the patch series, perhaps better as gtimer.h?

Is hw/arm/bcm2836.c really "non-arm-specific"?  Or did you mean "non-ARMCPU-specific"?


r~

Re: [PATCH-for-9.0 05/11] target/arm: Move GTIMER definitions to 'cpu-defs.h'
Posted by Philippe Mathieu-Daudé 12 months ago
On 28/11/23 15:02, Richard Henderson wrote:
> On 11/22/23 12:30, Philippe Mathieu-Daudé wrote:
>> To allow GTIMER_* definitions to be used by non-ARM specific
>> hardware models, move them to a new target agnostic "cpu-defs.h"
>> header.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/arm/cpu-defs.h | 19 +++++++++++++++++++
>>   target/arm/cpu.h      |  8 +-------
>>   hw/arm/bcm2836.c      |  1 +
>>   3 files changed, 21 insertions(+), 7 deletions(-)
>>   create mode 100644 target/arm/cpu-defs.h
>>
>> diff --git a/target/arm/cpu-defs.h b/target/arm/cpu-defs.h
>> new file mode 100644
>> index 0000000000..1ad76aff14
>> --- /dev/null
>> +++ b/target/arm/cpu-defs.h
>> @@ -0,0 +1,19 @@
>> +/*
>> + * ARM "target agnostic" CPU definitions
>> + *
>> + *  Copyright (c) 2003 Fabrice Bellard
>> + *
>> + * SPDX-License-Identifier: LGPL-2.1-or-later
>> + */
>> +
>> +#ifndef ARM_CPU_DEFS_H
>> +#define ARM_CPU_DEFS_H
>> +
>> +#define GTIMER_PHYS     0
>> +#define GTIMER_VIRT     1
>> +#define GTIMER_HYP      2
>> +#define GTIMER_SEC      3
>> +#define GTIMER_HYPVIRT  4
>> +#define NUM_GTIMERS     5
>> +
>> +#endif
> 
> Hmm.  cpu-defs.h is pretty generic.
> Without looking forward in the patch series, perhaps better as gtimer.h?

- target specific parameters used by accel/ (stay) in "cpu-param.h"
   (Ideally the single header included by accel/)

- target accelerator implementation details (stay) in "cpu.h"
   Shouldn't be used outside of target/

- architecture definitions in "arch-defs.h"
   (used by target/ and hw/)

- QEMU specific implementation details in "cpu-qom.h"
   (mostly used by hw/)

> 
> Is hw/arm/bcm2836.c really "non-arm-specific"?  Or did you mean 
> "non-ARMCPU-specific"?

I'd like to eventually have it instantiate a CPU which is not ARM based.