On 24 July 2015 at 10:55, Pavel Fedin <p.fe...@samsung.com> wrote: > From: Shlomo Pongratz <shlomo.pongr...@huawei.com> > > This class is to be used by both software and KVM implementations of GICv3 >
> +++ b/include/hw/intc/arm_gicv3_common.h > @@ -0,0 +1,112 @@ > +/* > + * ARM GIC support > + * > + * Copyright (c) 2012 Linaro Limited > + * Copyright (c) 2015 Huawei. > + * Written by Peter Maydell > + * Extended to 64 cores by Shlomo Pongratz > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License along > + * with this program; if not, see <http://www.gnu.org/licenses/>. > + */ > + > +#ifndef HW_ARM_GICV3_COMMON_H > +#define HW_ARM_GICV3_COMMON_H > + > +#include "hw/sysbus.h" > +#include "hw/intc/arm_gic_common.h" > + > +/* Maximum number of possible interrupts, determined by the GIC architecture > */ > +#define GICV3_MAXIRQ 1020 So how do LPIs work? They have IDs above 1023. > +#define GICV3_NCPU 64 Where does '64' come from as a maximum limit? > + > +typedef struct gicv3_irq_state { > + /* The enable bits are only banked for per-cpu interrupts. */ > + uint64_t enabled; > + uint64_t pending; > + uint64_t active; > + uint64_t level; > + uint64_t group; Why are these uint64_t ? > + bool edge_trigger; /* true: edge-triggered, false: level-triggered */ > +} gicv3_irq_state; > + > +typedef struct gicv3_sgi_state { > + uint64_t pending[GICV3_NCPU]; > +} gicv3_sgi_state; > + > +typedef struct GICv3State { > + /*< private >*/ > + SysBusDevice parent_obj; > + /*< public >*/ > + > + qemu_irq parent_irq[GICV3_NCPU]; > + qemu_irq parent_fiq[GICV3_NCPU]; > + /* GICD_CTLR; 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 ctlr; > + /* Sim GICC_CTLR; again, the NS banked version is just aliases of bits of > + * the S banked register, so our state only needs to store the S version. > + */ "Sim" ? > + uint32_t cpu_ctlr[GICV3_NCPU]; > + bool cpu_enabled[GICV3_NCPU]; > + > + gicv3_irq_state irq_state[GICV3_MAXIRQ]; > + uint64_t irq_target[GICV3_MAXIRQ]; > + uint8_t priority1[GIC_INTERNAL][GICV3_NCPU]; > + uint8_t priority2[GICV3_MAXIRQ - GIC_INTERNAL]; > + uint16_t last_active[GICV3_MAXIRQ][GICV3_NCPU]; > + /* For each SGI on the target CPU, we store 64 bits > + * indicating which source CPUs have made this SGI > + * pending on the target CPU. These correspond to > + * the bytes in the GIC_SPENDSGIR* registers as > + * read by the target CPU. This comment doesn't make any sense. You can't fit 64 bits into a byte, and GIC_SPENDSGIR<n> only hold 8 set-pending bits per SGI. > + */ > + gicv3_sgi_state sgi_state[GIC_NR_SGIS]; > + > + uint16_t priority_mask[GICV3_NCPU]; > + uint16_t running_irq[GICV3_NCPU]; > + uint16_t running_priority[GICV3_NCPU]; > + uint16_t current_pending[GICV3_NCPU]; > + > + uint32_t cpu_mask; /* For redistributor */ > + uint32_t num_cpu; > + MemoryRegion iomem_dist; /* Distributor */ > + MemoryRegion iomem_lpi; /* Redistributor */ > + uint32_t num_irq; > + uint32_t revision; > + bool security_levels; > + int dev_fd; /* kvm device fd if backed by kvm vgic support */ > +} GICv3State; This whole struct reads like "we just took the GICv2 state and changed it as little as possible beyond bumping the NCPU define a bit". That doesn't make me very confident that it's actually correct for GICv3... thanks -- PMM
