Patchwork [RFC,2/2] arm_gic: Use new __private macro to mark private fields

login
register
mail settings
Submitter Peter Maydell
Date July 25, 2013, 4:42 p.m.
Message ID <1374770523-6570-3-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/261797/
State New
Headers show

Comments

Peter Maydell - July 25, 2013, 4:42 p.m.
Use the new __private macro infrastructure to mark private fields for
the arm_gic classes.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/arm_gic.c         |    3 +++
 hw/intc/arm_gic_common.c  |    2 ++
 hw/intc/arm_gic_kvm.c     |    2 ++
 hw/intc/armv7m_nvic.c     |    2 ++
 include/hw/intc/arm_gic.h |   64 ++++++++++++++++++++++++++++-----------------
 5 files changed, 49 insertions(+), 24 deletions(-)
Alexander Graf - July 25, 2013, 7:04 p.m.
On 25.07.2013, at 18:42, Peter Maydell wrote:

> Use the new __private macro infrastructure to mark private fields for
> the arm_gic classes.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/intc/arm_gic.c         |    3 +++
> hw/intc/arm_gic_common.c  |    2 ++
> hw/intc/arm_gic_kvm.c     |    2 ++
> hw/intc/armv7m_nvic.c     |    2 ++
> include/hw/intc/arm_gic.h |   64 ++++++++++++++++++++++++++++-----------------
> 5 files changed, 49 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 8e34004..cbe7d27 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -18,6 +18,9 @@
>  *  armv7m_nvic device.
>  */
> 
> +#define IMPLEMENTING_ARM_GIC_COMMON
> +#define IMPLEMENTING_ARM_GIC
> +
> #include "hw/sysbus.h"
> #include "gic_internal.h"
> #include "qom/cpu.h"
> diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
> index a89c786..c27c5c1 100644
> --- a/hw/intc/arm_gic_common.c
> +++ b/hw/intc/arm_gic_common.c
> @@ -18,6 +18,8 @@
>  * with this program; if not, see <http://www.gnu.org/licenses/>.
>  */
> 
> +#define IMPLEMENTING_ARM_GIC_COMMON
> +
> #include "gic_internal.h"
> 
> static void gic_pre_save(void *opaque)
> diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
> index f713975..0acd676 100644
> --- a/hw/intc/arm_gic_kvm.c
> +++ b/hw/intc/arm_gic_kvm.c
> @@ -18,6 +18,8 @@
>  * with this program; if not, see <http://www.gnu.org/licenses/>.
>  */
> 
> +#define IMPLEMENTING_ARM_GIC_COMMON
> +
> #include "hw/sysbus.h"
> #include "sysemu/kvm.h"
> #include "kvm_arm.h"
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index 178344b..54d62fe 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -10,6 +10,8 @@
>  * NVIC.  Much of that is also implemented here.
>  */
> 
> +#define IMPLEMENTING_ARM_GIC_COMMON
> +
> #include "hw/sysbus.h"
> #include "qemu/timer.h"
> #include "hw/arm/arm.h"
> diff --git a/include/hw/intc/arm_gic.h b/include/hw/intc/arm_gic.h
> index be945ec..d978d72 100644
> --- a/include/hw/intc/arm_gic.h
> +++ b/include/hw/intc/arm_gic.h
> @@ -40,36 +40,42 @@ typedef struct gic_irq_state {
>     bool trigger; /* nonzero = edge triggered.  */
> } gic_irq_state;
> 
> +#ifdef IMPLEMENTING_ARM_GIC_COMMON
> +#define __private
> +#else
> +#define __private QEMU_PRIVATE_ATTR
> +#endif
> +
> typedef struct GICState {
>     /*< private >*/
> -    SysBusDevice busdev;
> +    __private SysBusDevice busdev;
>     /*< public >*/

I've been wondering about this for a while, but why are private fields in the public section?


Alex

> 
> -    qemu_irq parent_irq[GIC_NCPU];
> -    bool enabled;
> -    bool cpu_enabled[GIC_NCPU];
> +    __private qemu_irq parent_irq[GIC_NCPU];
> +    __private bool enabled;
> +    __private bool cpu_enabled[GIC_NCPU];
> 
> -    gic_irq_state irq_state[GIC_MAXIRQ];
> -    uint8_t irq_target[GIC_MAXIRQ];
> -    uint8_t priority1[GIC_INTERNAL][GIC_NCPU];
> -    uint8_t priority2[GIC_MAXIRQ - GIC_INTERNAL];
> -    uint16_t last_active[GIC_MAXIRQ][GIC_NCPU];
> +    __private gic_irq_state irq_state[GIC_MAXIRQ];
> +    __private uint8_t irq_target[GIC_MAXIRQ];
> +    __private uint8_t priority1[GIC_INTERNAL][GIC_NCPU];
> +    __private uint8_t priority2[GIC_MAXIRQ - GIC_INTERNAL];
> +    __private uint16_t last_active[GIC_MAXIRQ][GIC_NCPU];
> 
> -    uint16_t priority_mask[GIC_NCPU];
> -    uint16_t running_irq[GIC_NCPU];
> -    uint16_t running_priority[GIC_NCPU];
> -    uint16_t current_pending[GIC_NCPU];
> +    __private uint16_t priority_mask[GIC_NCPU];
> +    __private uint16_t running_irq[GIC_NCPU];
> +    __private uint16_t running_priority[GIC_NCPU];
> +    __private uint16_t current_pending[GIC_NCPU];
> 
> -    uint32_t num_cpu;
> +    __private uint32_t num_cpu;
> 
> -    MemoryRegion iomem; /* Distributor */
> +    __private MemoryRegion iomem; /* Distributor */
>     /* This is just so we can have an opaque pointer which identifies
>      * both this GIC and which CPU interface we should be accessing.
>      */
> -    struct GICState *backref[GIC_NCPU];
> -    MemoryRegion cpuiomem[GIC_NCPU + 1]; /* CPU interfaces */
> -    uint32_t num_irq;
> -    uint32_t revision;
> +    __private struct GICState *backref[GIC_NCPU];
> +    __private MemoryRegion cpuiomem[GIC_NCPU + 1]; /* CPU interfaces */
> +    __private uint32_t num_irq;
> +    __private uint32_t revision;
> } GICState;
> 
> #define TYPE_ARM_GIC_COMMON "arm_gic_common"
> @@ -82,13 +88,21 @@ typedef struct GICState {
> 
> typedef struct ARMGICCommonClass {
>     /*< private >*/
> -    SysBusDeviceClass parent_class;
> +    __private SysBusDeviceClass parent_class;
>     /*< public >*/
> 
> -    void (*pre_save)(GICState *s);
> -    void (*post_load)(GICState *s);
> +    __private void (*pre_save)(GICState *s);
> +    __private void (*post_load)(GICState *s);
> } ARMGICCommonClass;
> 
> +#undef __private
> +
> +#ifdef IMPLEMENTING_ARM_GIC
> +#define __private
> +#else
> +#define __private QEMU_PRIVATE_ATTR
> +#endif
> +
> #define TYPE_ARM_GIC "arm_gic"
> #define ARM_GIC(obj) \
>      OBJECT_CHECK(GICState, (obj), TYPE_ARM_GIC)
> @@ -99,10 +113,12 @@ typedef struct ARMGICCommonClass {
> 
> typedef struct ARMGICClass {
>     /*< private >*/
> -    ARMGICCommonClass parent_class;
> +    __private ARMGICCommonClass parent_class;
>     /*< public >*/
> 
> -    DeviceRealize parent_realize;
> +    __private DeviceRealize parent_realize;
> } ARMGICClass;
> 
> +#undef __private
> +
> #endif
> -- 
> 1.7.9.5
>

Patch

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 8e34004..cbe7d27 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -18,6 +18,9 @@ 
  *  armv7m_nvic device.
  */
 
+#define IMPLEMENTING_ARM_GIC_COMMON
+#define IMPLEMENTING_ARM_GIC
+
 #include "hw/sysbus.h"
 #include "gic_internal.h"
 #include "qom/cpu.h"
diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index a89c786..c27c5c1 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -18,6 +18,8 @@ 
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#define IMPLEMENTING_ARM_GIC_COMMON
+
 #include "gic_internal.h"
 
 static void gic_pre_save(void *opaque)
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index f713975..0acd676 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -18,6 +18,8 @@ 
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#define IMPLEMENTING_ARM_GIC_COMMON
+
 #include "hw/sysbus.h"
 #include "sysemu/kvm.h"
 #include "kvm_arm.h"
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 178344b..54d62fe 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -10,6 +10,8 @@ 
  * NVIC.  Much of that is also implemented here.
  */
 
+#define IMPLEMENTING_ARM_GIC_COMMON
+
 #include "hw/sysbus.h"
 #include "qemu/timer.h"
 #include "hw/arm/arm.h"
diff --git a/include/hw/intc/arm_gic.h b/include/hw/intc/arm_gic.h
index be945ec..d978d72 100644
--- a/include/hw/intc/arm_gic.h
+++ b/include/hw/intc/arm_gic.h
@@ -40,36 +40,42 @@  typedef struct gic_irq_state {
     bool trigger; /* nonzero = edge triggered.  */
 } gic_irq_state;
 
+#ifdef IMPLEMENTING_ARM_GIC_COMMON
+#define __private
+#else
+#define __private QEMU_PRIVATE_ATTR
+#endif
+
 typedef struct GICState {
     /*< private >*/
-    SysBusDevice busdev;
+    __private SysBusDevice busdev;
     /*< public >*/
 
-    qemu_irq parent_irq[GIC_NCPU];
-    bool enabled;
-    bool cpu_enabled[GIC_NCPU];
+    __private qemu_irq parent_irq[GIC_NCPU];
+    __private bool enabled;
+    __private bool cpu_enabled[GIC_NCPU];
 
-    gic_irq_state irq_state[GIC_MAXIRQ];
-    uint8_t irq_target[GIC_MAXIRQ];
-    uint8_t priority1[GIC_INTERNAL][GIC_NCPU];
-    uint8_t priority2[GIC_MAXIRQ - GIC_INTERNAL];
-    uint16_t last_active[GIC_MAXIRQ][GIC_NCPU];
+    __private gic_irq_state irq_state[GIC_MAXIRQ];
+    __private uint8_t irq_target[GIC_MAXIRQ];
+    __private uint8_t priority1[GIC_INTERNAL][GIC_NCPU];
+    __private uint8_t priority2[GIC_MAXIRQ - GIC_INTERNAL];
+    __private uint16_t last_active[GIC_MAXIRQ][GIC_NCPU];
 
-    uint16_t priority_mask[GIC_NCPU];
-    uint16_t running_irq[GIC_NCPU];
-    uint16_t running_priority[GIC_NCPU];
-    uint16_t current_pending[GIC_NCPU];
+    __private uint16_t priority_mask[GIC_NCPU];
+    __private uint16_t running_irq[GIC_NCPU];
+    __private uint16_t running_priority[GIC_NCPU];
+    __private uint16_t current_pending[GIC_NCPU];
 
-    uint32_t num_cpu;
+    __private uint32_t num_cpu;
 
-    MemoryRegion iomem; /* Distributor */
+    __private MemoryRegion iomem; /* Distributor */
     /* This is just so we can have an opaque pointer which identifies
      * both this GIC and which CPU interface we should be accessing.
      */
-    struct GICState *backref[GIC_NCPU];
-    MemoryRegion cpuiomem[GIC_NCPU + 1]; /* CPU interfaces */
-    uint32_t num_irq;
-    uint32_t revision;
+    __private struct GICState *backref[GIC_NCPU];
+    __private MemoryRegion cpuiomem[GIC_NCPU + 1]; /* CPU interfaces */
+    __private uint32_t num_irq;
+    __private uint32_t revision;
 } GICState;
 
 #define TYPE_ARM_GIC_COMMON "arm_gic_common"
@@ -82,13 +88,21 @@  typedef struct GICState {
 
 typedef struct ARMGICCommonClass {
     /*< private >*/
-    SysBusDeviceClass parent_class;
+    __private SysBusDeviceClass parent_class;
     /*< public >*/
 
-    void (*pre_save)(GICState *s);
-    void (*post_load)(GICState *s);
+    __private void (*pre_save)(GICState *s);
+    __private void (*post_load)(GICState *s);
 } ARMGICCommonClass;
 
+#undef __private
+
+#ifdef IMPLEMENTING_ARM_GIC
+#define __private
+#else
+#define __private QEMU_PRIVATE_ATTR
+#endif
+
 #define TYPE_ARM_GIC "arm_gic"
 #define ARM_GIC(obj) \
      OBJECT_CHECK(GICState, (obj), TYPE_ARM_GIC)
@@ -99,10 +113,12 @@  typedef struct ARMGICCommonClass {
 
 typedef struct ARMGICClass {
     /*< private >*/
-    ARMGICCommonClass parent_class;
+    __private ARMGICCommonClass parent_class;
     /*< public >*/
 
-    DeviceRealize parent_realize;
+    __private DeviceRealize parent_realize;
 } ARMGICClass;
 
+#undef __private
+
 #endif