|
[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 |