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

[Shanker Donthineni <shankerd@codeaurora.org>]

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@lists.xen.org
http://lists.xen.org/xen-devel