Re: [PATCH v4 10/16] xen/arm: move vgic defines to vgic header file

[Julien Grall <julien.grall@linaro.org>]

Hi Vijay,

On 05/26/2014 11:26 AM, vijay.kilari@gmail.com wrote:
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index f4c467a..2aadcb6 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -185,13 +185,6 @@ struct gic_lr {
>  
>  extern int gic_hw_version(void);
>  
> -extern int domain_vgic_init(struct domain *d);
> -extern void domain_vgic_free(struct domain *d);
> -
> -extern int vcpu_vgic_init(struct vcpu *v);
> -
> -extern void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq);
> -extern void vgic_clear_pending_irqs(struct vcpu *v);
>  extern struct pending_irq *irq_to_pending(struct vcpu *v, unsigned int irq);

irq_to_pending is also a VGIC function. Can you move it to vgic.h?

[..]

> +static inline uint32_t byte_read(uint32_t val, int sign, int offset)

[..]

> +static inline void byte_write(uint32_t *reg, uint32_t var, int offset)

On V3, I told you that these 2 names are too generic too be exported
here. Please add vgic_* before, so developer won't use them any where...

> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 44851ae..03416b5 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -826,6 +826,22 @@ void watchdog_domain_destroy(struct domain *d);
>  #define need_iommu(d)    (0)
>  #endif
>  
> +static inline int is_vcpu_running(struct domain *d, int vcpuid)
> +{

Please add the relevant maintainers when you add code in other part than
ARM. This time, I've cced them.

I think this code movement should be in another patch and justify why
this function should go in common code.

> +    struct vcpu *v;
> +
> +    if ( vcpuid >= d->max_vcpus )
> +        return 0;
> +
> +    v = d->vcpu[vcpuid];
> +    if ( v == NULL )
> +        return 0;
> +    if (test_bit(_VPF_down, &v->pause_flags) )
> +        return 0;
> +
> +    return 1;
> +}
> +
>  void set_vcpu_migration_delay(unsigned int delay);
>  unsigned int get_vcpu_migration_delay(void);
>  
> 

Regards,

-- 
Julien Grall