Re: [Xen-devel] [PATCH 6/8] arm: vgic: Split vgic_domain_init() functionality into two functions

[Shanker Donthineni <shankerd@xxxxxxxxxxxxxx>]

On 06/21/2016 05:49 AM, Julien Grall wrote:
> Hello Shanker,
>
> On 19/06/16 00:45, Shanker Donthineni wrote:
> > Split code that installs mmio handlers for GICD and Re-distributor
> > regions to a new function. The intension of this separation is to defer
> > steps that registers vgic_v2/v3 mmio handlers.
> Looking at this patch and the follow-up ones, I don't think this is 
> the right way to go. You differ the registration of the IO handlers 
> just because you are unable to find the size of the handlers array.
Is there any better approach?

> I am wondering if the array for the handlers is the best solution 
> here. On another side, it would be possible to find the maximum of 
> handlers before hand.
The purpose of this change is to limit size of 'struct domain' less than 
PAGE_SIZE. I can think of second approach split vgic_init() into two 
stages, one for vgic registration and the second one for vgic_init(). 
This also requires a few lines of code changes to vgic_v2/v3_init() and 
vgic_init().
int arch_domain_create(struct domain *d, unsigned int domcr_flags,
                       struct xen_arch_domainconfig *config)
   ...
   domain_vgic_register(d));

   domain_io_init(d, mmio_count);

   domain_vgic_init(d, config->nr_spis));

> > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> > index 5df5f01..5b39e0d 100644
> > --- a/xen/arch/arm/vgic.c
> > +++ b/xen/arch/arm/vgic.c
> > @@ -151,9 +151,12 @@ int domain_vgic_init(struct domain *d, unsigned int
> nr_spis)
> >       for ( i = 0; i < NR_GIC_SGI; i++ )
> >           set_bit(i, d->arch.vgic.allocated_irqs);
> >
> > +    d->arch.vgic.handler->domain_register_mmio(d);
> > +
> >       return 0;
> >   }
> >
> > +
> Spurious change.
>
> >   void register_vgic_ops(struct domain *d, const struct vgic_ops *ops)
> >   {
> >      d->arch.vgic.handler = ops;
> > diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> > index fbb763a..8fe65b4 100644
> > --- a/xen/include/asm-arm/vgic.h
> > +++ b/xen/include/asm-arm/vgic.h
> > @@ -132,6 +132,8 @@ struct vgic_ops {
> >       void (*domain_free)(struct domain *d);
> >       /* vGIC sysreg emulation */
> >       int (*emulate_sysreg)(struct cpu_user_regs *regs, union hsr hsr);
> > +    /* Register mmio handlers */
> > +    void (*domain_register_mmio)(struct domain *d);
> >       /* Maximum number of vCPU supported */
> >       const unsigned int max_vcpus;
> >   };
> Regards,
--
Shanker Donthineni
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel