Re: [Qemu-devel] [PATCH 4/5] hw: arm_gic: Support setting/getting binary point reg

"arm_gic:" is sufficient, "hw/arm_gic:" if you see the need.

Am 23.08.2013 22:10, schrieb Christoffer Dall:

Add a binary_point field to the gic emulation structure and support setting/getting this register now when we have it. We don't actually support interrupt grouping yet, oh well.

Signed-off-by: Christoffer Dall christoffer.dall@linaro.org

hw/intc/arm_gic.c | 5 ++--- hw/intc/arm_gic_common.c | 1 + hw/intc/gic_internal.h | 3 +++ 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 4da534f..cb2004d 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -568,8 +568,7 @@ static uint32_t gic_cpu_read(GICState *s, int cpu, int offset) case 0x04: /* Priority mask */ return s->priority_mask[cpu]; case 0x08: /* Binary Point */

•    /* ??? Not implemented.  */
  

•    return 0;
  

•    return s->binary_point[0][cpu];
  

  case 0x0c: /* Acknowledge */ value = gic_acknowledge_irq(s, cpu); value |= (GIC_SGI_SRC(value, cpu) & 0x7) << 10;

@@ -596,7 +595,7 @@ static void gic_cpu_write(GICState *s, int cpu, int offset, uint32_t value) s->priority_mask[cpu] = (value & 0xff); break; case 0x08: /* Binary Point */

•    /* ??? Not implemented.  */
  

•    s->binary_point[0][cpu] = (value & 0x7);
     break;
  

  case 0x10: /* End Of Interrupt */ return gic_complete_irq(s, cpu, value & 0x3ff);

diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c index 7a1c9e5..a50ded2 100644 --- a/hw/intc/arm_gic_common.c +++ b/hw/intc/arm_gic_common.c @@ -76,6 +76,7 @@ static const VMStateDescription vmstate_gic = { VMSTATE_UINT16_ARRAY(running_irq, GICState, NCPU), VMSTATE_UINT16_ARRAY(running_priority, GICState, NCPU), VMSTATE_UINT16_ARRAY(current_pending, GICState, NCPU),

•    VMSTATE_UINT8_2DARRAY(binary_point, GICState, 2, NCPU),
     VMSTATE_END_OF_LIST()
  

  }

};

You can't just add VMState fields without bumping the version_id and marking the field as starting in that version. That breaks loadvm/migration format compatibility.

diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h index 6f04885..424a41f 100644 --- a/hw/intc/gic_internal.h +++ b/hw/intc/gic_internal.h @@ -98,6 +98,9 @@ typedef struct GICState { uint16_t running_priority[NCPU]; uint16_t current_pending[NCPU];

• /* these registers are mainly used for save/restore of KVM state */
• uint8_t binary_point[2][NCPU]; /* [0]: group 0, [1]: group 1 */

binary_point[1] is never used, but changing the VMState field from length 1 to length 2 is probably more difficult than having a few extra bytes here.

Regards, Andreas

• uint32_t num_cpu;

MemoryRegion iomem; /* Distributor */