Re: [Xen-devel] [PATCH v4 04/28] VIOMMU: Add get irq info callback to convert irq remapping request

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Nov 17, 2017 at 02:22:11PM +0800, Chao Gao wrote:
> From: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> 
> This patch is to add get_irq_info callback for platform implementation
> to convert irq remapping request to irq info (E,G vector, dest, dest_mode
> and so on).
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> ---
>  xen/common/viommu.c          | 16 ++++++++++++++++
>  xen/include/asm-x86/viommu.h |  8 ++++++++
>  xen/include/xen/viommu.h     |  6 ++++++
>  3 files changed, 30 insertions(+)
> 
> diff --git a/xen/common/viommu.c b/xen/common/viommu.c
> index 53d4b70..9eafdef 100644
> --- a/xen/common/viommu.c
> +++ b/xen/common/viommu.c
> @@ -129,6 +129,22 @@ int viommu_handle_irq_request(const struct domain *d,
>      return viommu->ops->handle_irq_request(d, request);
>  }
>  
> +int viommu_get_irq_info(const struct domain *d,
> +                        const struct arch_irq_remapping_request *request,
> +                        struct arch_irq_remapping_info *irq_info)
> +{
> +    const struct viommu *viommu = d->arch.hvm_domain.viommu;
> +
> +    if ( !viommu )
> +        return -EINVAL;
> +
> +    ASSERT(viommu->ops);
> +    if ( !viommu->ops->get_irq_info )
> +        return -EINVAL;

EOPNOTSUPP.

> +
> +    return viommu->ops->get_irq_info(d, request, irq_info);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/asm-x86/viommu.h b/xen/include/asm-x86/viommu.h
> index 01ec80e..3d995ba 100644
> --- a/xen/include/asm-x86/viommu.h
> +++ b/xen/include/asm-x86/viommu.h
> @@ -26,6 +26,14 @@ enum viommu_irq_request_type {
>      VIOMMU_REQUEST_IRQ_APIC = 1
>  };
>  
> +struct arch_irq_remapping_info
> +{
> +    uint8_t dest_mode:1;
> +    uint8_t delivery_mode:3;
> +    uint8_t  vector;
              ^ double space.

> +    uint32_t dest;
> +};

The same issue again, introducing this structure without the code in
get_irq_info makes it impossible to review IMHO.

Also this should be introduced below the arch_irq_remapping_request
struct.

> +
>  struct arch_irq_remapping_request
>  {
>      union {
> diff --git a/xen/include/xen/viommu.h b/xen/include/xen/viommu.h
> index 67e25d5..73b853f 100644
> --- a/xen/include/xen/viommu.h
> +++ b/xen/include/xen/viommu.h
> @@ -32,6 +32,9 @@ struct viommu_ops {
>      int (*destroy)(struct viommu *viommu);
>      int (*handle_irq_request)(const struct domain *d,
>                                const struct arch_irq_remapping_request 
> *request);
> +    int (*get_irq_info)(const struct domain *d,
> +                        const struct arch_irq_remapping_request *request,
> +                        struct arch_irq_remapping_info *info);
>  };
>  
>  struct viommu {
> @@ -50,6 +53,9 @@ int viommu_destroy_domain(struct domain *d);
>  int viommu_domctl(struct domain *d, struct xen_domctl_viommu_op *op);
>  int viommu_handle_irq_request(const struct domain *d,
>                                const struct arch_irq_remapping_request 
> *request);
> +int viommu_get_irq_info(const struct domain *d,
> +                        const struct arch_irq_remapping_request *request,

Why do you need 'request' here?

> +                        struct arch_irq_remapping_info *irq_info);
>  #else
>  static inline int viommu_destroy_domain(struct domain *d)
>  {
> -- 
> 1.8.3.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel