qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC 1/5] hw/intc/arm_gicv3_common: Add state informati


From: Peter Maydell
Subject: Re: [Qemu-devel] [RFC 1/5] hw/intc/arm_gicv3_common: Add state information
Date: Wed, 24 Feb 2016 14:46:52 +0000

On 24 February 2016 at 12:50, Shlomo Pongratz <address@hidden> wrote:
>
>
> On Monday, February 22, 2016, Peter Maydell <address@hidden>
> wrote:
>>
>> +struct GICv3CPUState {
>> +    uint64_t affinity_id;            /* Cached affinity ID of the CPU */
>> +
>> +    /* Redistributor */
>> +    uint32_t level;                  /* Current IRQ level */
>> +    /* RD_base page registers */
>> +    uint32_t gicr_ctlr;
>> +    uint32_t gicr_waker;
>> +    uint64_t gicr_propbaser;
>> +    uint64_t gicr_pendbaser;
>> +    /* SGI_base page registers */
>> +    uint32_t gicr_igroupr0;
>> +    uint32_t gicr_ienabler0;
>> +    uint32_t gicr_ipendr0;
>> +    uint32_t gicr_iactiver0;
>> +    uint32_t gicr_icfgr0;
>> +    uint32_t gicr_icfgr1;
>> +    uint8_t gicr_ipriorityr[GIC_INTERNAL];
>> +
>> +    /* CPU interface */
>> +    uint64_t icc_ctlr_el1[2];
>> +    uint64_t icc_pmr_el1;
>> +    uint64_t icc_bpr[2];
>> +    uint64_t icc_ap1r[4][2];
>> +    uint64_t icc_igrpen0_el1;
>> +    uint64_t icc_igrpen1_el1;
>> +};
>> +
>> +typedef struct GICv3CPUState GICv3CPUState;
>> +
>> +struct GICv3State {
>>      /*< private >*/
>>      SysBusDevice parent_obj;
>>      /*< public >*/
>> @@ -41,9 +83,61 @@ typedef struct GICv3State {
>>      uint32_t num_irq;
>>      uint32_t revision;
>>      bool security_extn;
>> +    bool irq_reset_nonsecure;
>>
>>      int dev_fd; /* kvm device fd if backed by kvm vgic support */
>> -} GICv3State;
>> +    Error *migration_blocker;
>> +
>> +    /* Distributor */
>> +
>> +    /* for a GIC with the security extensions the NS banked version of
>> this
>> +     * register is just an alias of bit 1 of the S banked version.
>> +     */
>> +    uint32_t gicd_ctlr;
>> +    DECLARE_BITMAP(group, GICV3_MAXSPI);        /* GICD_IGROUPR */
>> +    DECLARE_BITMAP(enabled, GICV3_MAXSPI);      /* GICD_ISENABLER */
>> +    DECLARE_BITMAP(pending, GICV3_MAXSPI);      /* GICD_ISPENDR */
>> +    DECLARE_BITMAP(active, GICV3_MAXSPI);       /* GICD_ISACTIVER */
>> +    DECLARE_BITMAP(level, GICV3_MAXSPI);        /* Current level */
>> +    DECLARE_BITMAP(edge_trigger, GICV3_MAXSPI); /* GICD_ICFGR */
>> +    uint8_t gicd_ipriority[GICV3_MAXSPI];
>> +    uint8_t gicd_itargetsr[GICV3_MAXSPI];
>> +    uint64_t gicd_irouter[GICV3_MAXSPI];
>> +
>> +    GICv3CPUState *cpu;
>> +};

> I just wonder why the internal interrupts are excluded from the new
> GICv3State and the new access API don't handle them?
> Is it because they aren't required by KVM?
> I see that they are only referenced in gicr_ipriority.
>
> What am I missing?

Which internal interrupts did you have in mind? In GICv3
the PPIs and SGIs have state which lives in the redistributor
(in this data structure layout that's in the gicr_ipendr0,
gicr_ienabler0, etc in each GICv3CPUState struct). SPIs
have state which lives in the distributor (that's the
enabled/pending/etc bitmaps in the GICv3State struct).

I removed the confusing macros which tried to make the
two things appear the same (letting you mark an interrupt
as enabled whether the irq was for an SGI/PPI or an SPI)
because we shouldn't need them. The emulation code should
clearly separate the responsibilities of the distributor
from those of the redistributor (as happens in the real
hardware), and so it should in general already know what
kind of interrupt it's dealing with, and be able to just
work directly with the right data structure. If it looks
like you need a single accessor that works on both SGI/PPI
and SPI it's probably an indication that the code should
be restructured.

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]