Message ID | 1522160122-10744-3-git-send-email-eric.auger@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 27 March 2018 at 15:15, Eric Auger <eric.auger@redhat.com> wrote: > In the prospect to support multiple redistributor regions, > let's introduce a GICv3RDISTRegion struct datatype and a > statically sized array of those. For the time being, only > one redistributor region is used. > > Signed-off-by: Eric Auger <eric.auger@redhat.com> > --- > hw/intc/arm_gicv3_common.c | 5 +++-- > hw/intc/arm_gicv3_kvm.c | 3 ++- > include/hw/intc/arm_gicv3_common.h | 13 ++++++++++++- > 3 files changed, 17 insertions(+), 4 deletions(-) > > diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c > index 7b54d52..cb4ee0e 100644 > --- a/hw/intc/arm_gicv3_common.c > +++ b/hw/intc/arm_gicv3_common.c > @@ -199,11 +199,12 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler, > > memory_region_init_io(&s->iomem_dist, OBJECT(s), ops, s, > "gicv3_dist", 0x10000); > - memory_region_init_io(&s->iomem_redist, OBJECT(s), ops ? &ops[1] : NULL, s, > + memory_region_init_io(&s->redist_region[0].mr, OBJECT(s), > + ops ? &ops[1] : NULL, s, > "gicv3_redist", 0x20000 * s->num_cpu); > > sysbus_init_mmio(sbd, &s->iomem_dist); > - sysbus_init_mmio(sbd, &s->iomem_redist); > + sysbus_init_mmio(sbd, &s->redist_region[0].mr); > } > > static void arm_gicv3_common_realize(DeviceState *dev, Error **errp) > diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c > index ec37177..a07bc55 100644 > --- a/hw/intc/arm_gicv3_kvm.c > +++ b/hw/intc/arm_gicv3_kvm.c > @@ -755,7 +755,8 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) > > kvm_arm_register_device(&s->iomem_dist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR, > KVM_VGIC_V3_ADDR_TYPE_DIST, s->dev_fd); > - kvm_arm_register_device(&s->iomem_redist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR, > + kvm_arm_register_device(&s->redist_region[0].mr, -1, > + KVM_DEV_ARM_VGIC_GRP_ADDR, > KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd); > > if (kvm_has_gsi_routing()) { > diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h > index bccdfe1..3cf132f 100644 > --- a/include/hw/intc/arm_gicv3_common.h > +++ b/include/hw/intc/arm_gicv3_common.h > @@ -137,12 +137,20 @@ typedef struct GICv3CPUState GICv3CPUState; > #define GICV3_S 0 > #define GICV3_NS 1 > > +#define GICV3_MAX_RDIST_REGIONS 8 Where does 8 come from? Is it a limit in the kernel, or just a random number? > + > typedef struct { > int irq; > uint8_t prio; > int grp; > } PendingIrq; > > +typedef struct GICv3RDISTRegion { "GICv3RedistRegion" > + hwaddr base; > + uint32_t count; /* number or redistributors */ "of" > + MemoryRegion mr; > +} GICv3RDISTRegion ; > + > struct GICv3CPUState { > GICv3State *gic; > CPUState *cpu; > @@ -210,7 +218,8 @@ struct GICv3State { > /*< public >*/ > > MemoryRegion iomem_dist; /* Distributor */ > - MemoryRegion iomem_redist; /* Redistributors */ > + GICv3RDISTRegion redist_region[GICV3_MAX_RDIST_REGIONS]; > + uint32_t nb_redist_regions; > > uint32_t num_cpu; > uint32_t num_irq; > @@ -288,6 +297,8 @@ typedef struct ARMGICv3CommonClass { > > void (*pre_save)(GICv3State *s); > void (*post_load)(GICv3State *s); > + /* register an RDIST region at @base, containing @pfns 64kB pages */ > + int (*register_redist_region)(GICv3State *s, hwaddr base, uint32_t pfns); "pfn" is a kernelism (and I can never remember what it means) -- can you use a more comprehensible variable name, please ? > } ARMGICv3CommonClass; > > void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler, > -- thanks -- PMM
Hi Peter, On 13/04/18 15:27, Peter Maydell wrote: > On 27 March 2018 at 15:15, Eric Auger <eric.auger@redhat.com> wrote: >> In the prospect to support multiple redistributor regions, >> let's introduce a GICv3RDISTRegion struct datatype and a >> statically sized array of those. For the time being, only >> one redistributor region is used. >> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> --- >> hw/intc/arm_gicv3_common.c | 5 +++-- >> hw/intc/arm_gicv3_kvm.c | 3 ++- >> include/hw/intc/arm_gicv3_common.h | 13 ++++++++++++- >> 3 files changed, 17 insertions(+), 4 deletions(-) >> >> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c >> index 7b54d52..cb4ee0e 100644 >> --- a/hw/intc/arm_gicv3_common.c >> +++ b/hw/intc/arm_gicv3_common.c >> @@ -199,11 +199,12 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler, >> >> memory_region_init_io(&s->iomem_dist, OBJECT(s), ops, s, >> "gicv3_dist", 0x10000); >> - memory_region_init_io(&s->iomem_redist, OBJECT(s), ops ? &ops[1] : NULL, s, >> + memory_region_init_io(&s->redist_region[0].mr, OBJECT(s), >> + ops ? &ops[1] : NULL, s, >> "gicv3_redist", 0x20000 * s->num_cpu); >> >> sysbus_init_mmio(sbd, &s->iomem_dist); >> - sysbus_init_mmio(sbd, &s->iomem_redist); >> + sysbus_init_mmio(sbd, &s->redist_region[0].mr); >> } >> >> static void arm_gicv3_common_realize(DeviceState *dev, Error **errp) >> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c >> index ec37177..a07bc55 100644 >> --- a/hw/intc/arm_gicv3_kvm.c >> +++ b/hw/intc/arm_gicv3_kvm.c >> @@ -755,7 +755,8 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) >> >> kvm_arm_register_device(&s->iomem_dist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR, >> KVM_VGIC_V3_ADDR_TYPE_DIST, s->dev_fd); >> - kvm_arm_register_device(&s->iomem_redist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR, >> + kvm_arm_register_device(&s->redist_region[0].mr, -1, >> + KVM_DEV_ARM_VGIC_GRP_ADDR, >> KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd); >> >> if (kvm_has_gsi_routing()) { >> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h >> index bccdfe1..3cf132f 100644 >> --- a/include/hw/intc/arm_gicv3_common.h >> +++ b/include/hw/intc/arm_gicv3_common.h >> @@ -137,12 +137,20 @@ typedef struct GICv3CPUState GICv3CPUState; >> #define GICV3_S 0 >> #define GICV3_NS 1 >> >> +#define GICV3_MAX_RDIST_REGIONS 8 > > Where does 8 come from? Is it a limit in the kernel, or just > a random number? Kernel has no limit as the redist regions are handled through a list there. 8 is purely arbitrate. I thought it was overkill to handle a list here. > >> + >> typedef struct { >> int irq; >> uint8_t prio; >> int grp; >> } PendingIrq; >> >> +typedef struct GICv3RDISTRegion { > > "GICv3RedistRegion" > >> + hwaddr base; >> + uint32_t count; /* number or redistributors */ > > "of" > >> + MemoryRegion mr; >> +} GICv3RDISTRegion ; >> + >> struct GICv3CPUState { >> GICv3State *gic; >> CPUState *cpu; >> @@ -210,7 +218,8 @@ struct GICv3State { >> /*< public >*/ >> >> MemoryRegion iomem_dist; /* Distributor */ >> - MemoryRegion iomem_redist; /* Redistributors */ >> + GICv3RDISTRegion redist_region[GICV3_MAX_RDIST_REGIONS]; >> + uint32_t nb_redist_regions; >> >> uint32_t num_cpu; >> uint32_t num_irq; >> @@ -288,6 +297,8 @@ typedef struct ARMGICv3CommonClass { >> >> void (*pre_save)(GICv3State *s); >> void (*post_load)(GICv3State *s); >> + /* register an RDIST region at @base, containing @pfns 64kB pages */ >> + int (*register_redist_region)(GICv3State *s, hwaddr base, uint32_t pfns); > > "pfn" is a kernelism (and I can never remember what it means) -- can you > use a more comprehensible variable name, please ? In pratice it has become count (number of redistributors within the region) but I forgot to update the declaration here, sorry. Thanks Eric > >> } ARMGICv3CommonClass; >> >> void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler, >> -- > > thanks > -- PMM >
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c index 7b54d52..cb4ee0e 100644 --- a/hw/intc/arm_gicv3_common.c +++ b/hw/intc/arm_gicv3_common.c @@ -199,11 +199,12 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler, memory_region_init_io(&s->iomem_dist, OBJECT(s), ops, s, "gicv3_dist", 0x10000); - memory_region_init_io(&s->iomem_redist, OBJECT(s), ops ? &ops[1] : NULL, s, + memory_region_init_io(&s->redist_region[0].mr, OBJECT(s), + ops ? &ops[1] : NULL, s, "gicv3_redist", 0x20000 * s->num_cpu); sysbus_init_mmio(sbd, &s->iomem_dist); - sysbus_init_mmio(sbd, &s->iomem_redist); + sysbus_init_mmio(sbd, &s->redist_region[0].mr); } static void arm_gicv3_common_realize(DeviceState *dev, Error **errp) diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index ec37177..a07bc55 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -755,7 +755,8 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) kvm_arm_register_device(&s->iomem_dist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR, KVM_VGIC_V3_ADDR_TYPE_DIST, s->dev_fd); - kvm_arm_register_device(&s->iomem_redist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR, + kvm_arm_register_device(&s->redist_region[0].mr, -1, + KVM_DEV_ARM_VGIC_GRP_ADDR, KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd); if (kvm_has_gsi_routing()) { diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h index bccdfe1..3cf132f 100644 --- a/include/hw/intc/arm_gicv3_common.h +++ b/include/hw/intc/arm_gicv3_common.h @@ -137,12 +137,20 @@ typedef struct GICv3CPUState GICv3CPUState; #define GICV3_S 0 #define GICV3_NS 1 +#define GICV3_MAX_RDIST_REGIONS 8 + typedef struct { int irq; uint8_t prio; int grp; } PendingIrq; +typedef struct GICv3RDISTRegion { + hwaddr base; + uint32_t count; /* number or redistributors */ + MemoryRegion mr; +} GICv3RDISTRegion ; + struct GICv3CPUState { GICv3State *gic; CPUState *cpu; @@ -210,7 +218,8 @@ struct GICv3State { /*< public >*/ MemoryRegion iomem_dist; /* Distributor */ - MemoryRegion iomem_redist; /* Redistributors */ + GICv3RDISTRegion redist_region[GICV3_MAX_RDIST_REGIONS]; + uint32_t nb_redist_regions; uint32_t num_cpu; uint32_t num_irq; @@ -288,6 +297,8 @@ typedef struct ARMGICv3CommonClass { void (*pre_save)(GICv3State *s); void (*post_load)(GICv3State *s); + /* register an RDIST region at @base, containing @pfns 64kB pages */ + int (*register_redist_region)(GICv3State *s, hwaddr base, uint32_t pfns); } ARMGICv3CommonClass; void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler,
In the prospect to support multiple redistributor regions, let's introduce a GICv3RDISTRegion struct datatype and a statically sized array of those. For the time being, only one redistributor region is used. Signed-off-by: Eric Auger <eric.auger@redhat.com> --- hw/intc/arm_gicv3_common.c | 5 +++-- hw/intc/arm_gicv3_kvm.c | 3 ++- include/hw/intc/arm_gicv3_common.h | 13 ++++++++++++- 3 files changed, 17 insertions(+), 4 deletions(-)