[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC 19/22] xen/arm: Merge gicv_setup with vgic_domain_init
On Fri, 2015-05-08 at 14:29 +0100, Julien Grall wrote: > Currently, it's hard to decide whether a part of the domain > initialization should live in gicv_setup (part of the GIC > driver) and domain_init (part of the vGIC driver). > > The code to initialize the domain for a specific vGIC version is always > the same no matter the version of the GIC. > > Move all the domain initialization code for the vGIC in the respective > domain_init callback of each vGIC drivers. Pure code motion? If yes: Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx> > > Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx> > Cc: Zoltan Kiss <zoltan.kiss@xxxxxxxxxx> > --- > xen/arch/arm/domain.c | 3 --- > xen/arch/arm/gic-hip04.c | 42 ---------------------------------- > xen/arch/arm/gic-v2.c | 42 ---------------------------------- > xen/arch/arm/gic-v3.c | 58 > ----------------------------------------------- > xen/arch/arm/gic.c | 10 ++++---- > xen/arch/arm/vgic-v2.c | 42 +++++++++++++++++++++++++++++++++- > xen/arch/arm/vgic-v3.c | 54 ++++++++++++++++++++++++++++++++++++++++++- > xen/include/asm-arm/gic.h | 4 ++-- > 8 files changed, 101 insertions(+), 154 deletions(-) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index e4d6fc8..9b113eb 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -587,9 +587,6 @@ int arch_domain_create(struct domain *d, unsigned int > domcr_flags, > } > config->gic_version = gic_version; > > - if ( (rc = gicv_setup(d)) != 0 ) > - goto fail; > - > if ( (rc = domain_vgic_init(d)) != 0 ) > goto fail; > > diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c > index 416ef83..9693666 100644 > --- a/xen/arch/arm/gic-hip04.c > +++ b/xen/arch/arm/gic-hip04.c > @@ -439,47 +439,6 @@ static void hip04gic_clear_lr(int lr) > writel_gich(0, HIP04_GICH_LR + lr * 4); > } > > -static int hip04gicv_setup(struct domain *d) > -{ > - int ret; > - > - /* > - * The hardware domain gets the hardware address. > - * Guests get the virtual platform layout. > - */ > - if ( is_hardware_domain(d) ) > - { > - d->arch.vgic.dbase = gicv2_info.dbase; > - d->arch.vgic.cbase = gicv2_info.cbase; > - } > - else > - { > - d->arch.vgic.dbase = GUEST_GICD_BASE; > - d->arch.vgic.cbase = GUEST_GICC_BASE; > - } > - > - /* > - * Map the gic virtual cpu interface in the gic cpu interface > - * region of the guest. > - * > - * The second page is always mapped at +4K irrespective of the > - * GIC_64K_STRIDE quirk. The DTB passed to the guest reflects this. > - */ > - ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase), 1, > - paddr_to_pfn(gicv2_info.vbase)); > - if ( ret ) > - return ret; > - > - if ( !platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) ) > - ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase + SZ_4K), > - 2, paddr_to_pfn(gicv2_info.vbase + SZ_4K)); > - else > - ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase + SZ_4K), > - 2, paddr_to_pfn(gicv2_info.vbase + SZ_64K)); > - > - return ret; > -} > - > static void hip04gic_read_lr(int lr, struct gic_lr *lr_reg) > { > uint32_t lrv; > @@ -771,7 +730,6 @@ const static struct gic_hw_operations hip04gic_ops = { > .save_state = hip04gic_save_state, > .restore_state = hip04gic_restore_state, > .dump_state = hip04gic_dump_state, > - .gicv_setup = hip04gicv_setup, > .gic_host_irq_type = &hip04gic_host_irq_type, > .gic_guest_irq_type = &hip04gic_guest_irq_type, > .eoi_irq = hip04gic_eoi_irq, > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index bd5603b..f53560e 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -429,47 +429,6 @@ static void gicv2_clear_lr(int lr) > writel_gich(0, GICH_LR + lr * 4); > } > > -static int gicv2v_setup(struct domain *d) > -{ > - int ret; > - > - /* > - * The hardware domain gets the hardware address. > - * Guests get the virtual platform layout. > - */ > - if ( is_hardware_domain(d) ) > - { > - d->arch.vgic.dbase = gicv2_info.dbase; > - d->arch.vgic.cbase = gicv2_info.cbase; > - } > - else > - { > - d->arch.vgic.dbase = GUEST_GICD_BASE; > - d->arch.vgic.cbase = GUEST_GICC_BASE; > - } > - > - /* > - * Map the gic virtual cpu interface in the gic cpu interface > - * region of the guest. > - * > - * The second page is always mapped at +4K irrespective of the > - * GIC_64K_STRIDE quirk. The DTB passed to the guest reflects this. > - */ > - ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase), 1, > - paddr_to_pfn(gicv2_info.vbase)); > - if ( ret ) > - return ret; > - > - if ( !platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) ) > - ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase + SZ_4K), > - 2, paddr_to_pfn(gicv2_info.vbase + SZ_4K)); > - else > - ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase + SZ_4K), > - 2, paddr_to_pfn(gicv2._info.vbase + SZ_64K)); > - > - return ret; > -} > - > static void gicv2_read_lr(int lr, struct gic_lr *lr_reg) > { > uint32_t lrv; > @@ -756,7 +715,6 @@ const static struct gic_hw_operations gicv2_ops = { > .save_state = gicv2_save_state, > .restore_state = gicv2_restore_state, > .dump_state = gicv2_dump_state, > - .gicv_setup = gicv2v_setup, > .gic_host_irq_type = &gicv2_host_irq_type, > .gic_guest_irq_type = &gicv2_guest_irq_type, > .eoi_irq = gicv2_eoi_irq, > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index 13250c5..7603a2c 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -894,63 +894,6 @@ static void gicv3_write_lr(int lr_reg, const struct > gic_lr *lr) > gicv3_ich_write_lr(lr_reg, lrv); > } > > -static int gicv_v3_init(struct domain *d) > -{ > - int i; > - > - /* > - * Domain 0 gets the hardware address. > - * Guests get the virtual platform layout. > - */ > - if ( is_hardware_domain(d) ) > - { > - unsigned int first_cpu = 0; > - > - d->arch.vgic.dbase = gicv3_info.dbase; > - > - d->arch.vgic.rdist_stride = gicv3_info.rdist_stride; > - /* > - * If the stride is not set, the default stride for GICv3 is 2 * 64K: > - * - first 64k page for Control and Physical LPIs > - * - second 64k page for Control and Generation of SGIs > - */ > - if ( !d->arch.vgic.rdist_stride ) > - d->arch.vgic.rdist_stride = 2 * SZ_64K; > - > - for ( i = 0; i < gicv3_info.nr_rdist_regions; i++ ) > - { > - paddr_t size = gicv3_info.rdist_regions[i].size; > - > - d->arch.vgic.rdist_regions[i].base = > gicv3_info.rdist_regions[i].base; > - d->arch.vgic.rdist_regions[i].size = size; > - > - /* Set the first CPU handled by this region */ > - d->arch.vgic.rdist_regions[i].first_cpu = first_cpu; > - > - first_cpu += size / d->arch.vgic.rdist_stride; > - } > - d->arch.vgic.nr_regions = gicv3_info.nr_rdist_regions; > - } > - else > - { > - d->arch.vgic.dbase = GUEST_GICV3_GICD_BASE; > - > - /* XXX: Only one Re-distributor region mapped for the guest */ > - BUILD_BUG_ON(GUEST_GICV3_RDIST_REGIONS != 1); > - > - d->arch.vgic.nr_regions = GUEST_GICV3_RDIST_REGIONS; > - d->arch.vgic.rdist_stride = GUEST_GICV3_RDIST_STRIDE; > - > - /* The first redistributor should contain enough space for all CPUs > */ > - BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) < > MAX_VIRT_CPUS); > - d->arch.vgic.rdist_regions[0].base = GUEST_GICV3_GICR0_BASE; > - d->arch.vgic.rdist_regions[0].size = GUEST_GICV3_GICR0_SIZE; > - d->arch.vgic.rdist_regions[0].first_cpu = 0; > - } > - > - return 0; > -} > - > static void gicv3_hcr_status(uint32_t flag, bool_t status) > { > uint32_t hcr; > @@ -1284,7 +1227,6 @@ static const struct gic_hw_operations gicv3_ops = { > .save_state = gicv3_save_state, > .restore_state = gicv3_restore_state, > .dump_state = gicv3_dump_state, > - .gicv_setup = gicv_v3_init, > .gic_host_irq_type = &gicv3_host_irq_type, > .gic_guest_irq_type = &gicv3_guest_irq_type, > .eoi_irq = gicv3_eoi_irq, > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 5f34997..4d8adb9 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -67,6 +67,11 @@ unsigned int gic_number_lines(void) > return gic_hw_ops->info->nr_lines; > } > > +const struct gic_info *gic_info(void) > +{ > + return gic_hw_ops->info; > +} > + > void gic_save_state(struct vcpu *v) > { > ASSERT(!local_irq_is_enabled()); > @@ -620,11 +625,6 @@ void gic_interrupt(struct cpu_user_regs *regs, int > is_fiq) > } while (1); > } > > -int gicv_setup(struct domain *d) > -{ > - return gic_hw_ops->gicv_setup(d); > -} > - > static void maintenance_interrupt(int irq, void *dev_id, struct > cpu_user_regs *regs) > { > /* > diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c > index 9eecabc..3be1a51 100644 > --- a/xen/arch/arm/vgic-v2.c > +++ b/xen/arch/arm/vgic-v2.c > @@ -24,10 +24,12 @@ > #include <xen/softirq.h> > #include <xen/irq.h> > #include <xen/sched.h> > +#include <xen/sizes.h> > > #include <asm/current.h> > > #include <asm/mmio.h> > +#include <asm/platform.h> > #include <asm/gic.h> > #include <asm/vgic.h> > > @@ -525,7 +527,45 @@ static int vgic_v2_vcpu_init(struct vcpu *v) > > static int vgic_v2_domain_init(struct domain *d) > { > - int i; > + int i, ret; > + const struct gic_info *info = gic_info(); > + > + /* > + * The hardware domain gets the hardware address. > + * Guests get the virtual platform layout. > + */ > + if ( is_hardware_domain(d) ) > + { > + d->arch.vgic.dbase = info->dbase; > + d->arch.vgic.cbase = info->cbase; > + } > + else > + { > + d->arch.vgic.dbase = GUEST_GICD_BASE; > + d->arch.vgic.cbase = GUEST_GICC_BASE; > + } > + > + /* > + * Map the gic virtual cpu interface in the gic cpu interface > + * region of the guest. > + * > + * The second page is always mapped at +4K irrespective of the > + * GIC_64K_STRIDE quirk. The DTB passed to the guest reflects this. > + */ > + ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase), 1, > + paddr_to_pfn(info->vbase)); > + if ( ret ) > + return ret; > + > + if ( !platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) ) > + ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase + SZ_4K), > + 2, paddr_to_pfn(info->vbase + SZ_64K)); > + else > + ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase + SZ_4K), > + 2, paddr_to_pfn(info->vbase + SZ_64K)); > + > + if ( ret ) > + return ret; > > /* By default deliver to CPU0 */ > for ( i = 0; i < DOMAIN_NR_RANKS(d); i++ ) > diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c > index 77ada20..45d54a2 100644 > --- a/xen/arch/arm/vgic-v3.c > +++ b/xen/arch/arm/vgic-v3.c > @@ -1146,6 +1146,57 @@ static int vgic_v3_vcpu_init(struct vcpu *v) > static int vgic_v3_domain_init(struct domain *d) > { > int i, idx; > + const struct gic_info *info = gic_info(); > + > + /* > + * Domain 0 gets the hardware address. > + * Guests get the virtual platform layout. > + */ > + if ( is_hardware_domain(d) ) > + { > + unsigned int first_cpu = 0; > + > + d->arch.vgic.dbase = info->dbase; > + > + d->arch.vgic.rdist_stride = info->rdist_stride; > + /* > + * If the stride is not set, the default stride for GICv3 is 2 * 64K: > + * - first 64k page for Control and Physical LPIs > + * - second 64k page for Control and Generation of SGIs > + */ > + if ( !d->arch.vgic.rdist_stride ) > + d->arch.vgic.rdist_stride = 2 * SZ_64K; > + > + for ( i = 0; i < info->nr_rdist_regions; i++ ) > + { > + paddr_t size = info->rdist_regions[i].size; > + > + d->arch.vgic.rdist_regions[i].base = info->rdist_regions[i].base; > + d->arch.vgic.rdist_regions[i].size = size; > + > + /* Set the first CPU handled by this region */ > + d->arch.vgic.rdist_regions[i].first_cpu = first_cpu; > + > + first_cpu += size / d->arch.vgic.rdist_stride; > + } > + d->arch.vgic.nr_regions = info->nr_rdist_regions; > + } > + else > + { > + d->arch.vgic.dbase = GUEST_GICV3_GICD_BASE; > + > + /* XXX: Only one Re-distributor region mapped for the guest */ > + BUILD_BUG_ON(GUEST_GICV3_RDIST_REGIONS != 1); > + > + d->arch.vgic.nr_regions = GUEST_GICV3_RDIST_REGIONS; > + d->arch.vgic.rdist_stride = GUEST_GICV3_RDIST_STRIDE; > + > + /* The first redistributor should contain enough space for all CPUs > */ > + BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) < > MAX_VIRT_CPUS); > + d->arch.vgic.rdist_regions[0].base = GUEST_GICV3_GICR0_BASE; > + d->arch.vgic.rdist_regions[0].size = GUEST_GICV3_GICR0_SIZE; > + d->arch.vgic.rdist_regions[0].first_cpu = 0; > + } > > /* By default deliver to CPU0 */ > for ( i = 0; i < DOMAIN_NR_RANKS(d); i++ ) > @@ -1153,7 +1204,8 @@ static int vgic_v3_domain_init(struct domain *d) > for ( idx = 0; idx < 32; idx++ ) > d->arch.vgic.shared_irqs[i].v3.irouter[idx] = 0; > } > - /* We rely on gicv init to get dbase and size */ > + > + /* Register mmio handle for the Distributor */ > register_mmio_handler(d, &vgic_distr_mmio_handler, d->arch.vgic.dbase, > SZ_64K); > > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index be0f610..4319ac4 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -307,6 +307,8 @@ struct gic_info { > #endif > }; > > +const struct gic_info *gic_info(void); > + > struct gic_hw_operations { > /* Hold GIC HW information */ > const struct gic_info *info; > @@ -318,8 +320,6 @@ struct gic_hw_operations { > void (*restore_state)(const struct vcpu *); > /* Dump GIC LR register information */ > void (*dump_state)(const struct vcpu *); > - /* Map MMIO region of GIC */ > - int (*gicv_setup)(struct domain *); > > /* hw_irq_controller to enable/disable/eoi host irq */ > hw_irq_controller *gic_host_irq_type; _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |