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

[Chao Gao <chao.gao@intel.com>]

On Fri, Feb 09, 2018 at 03:06:07PM +0000, Roger Pau Monné wrote:
>On Fri, Nov 17, 2017 at 02:22:11PM +0800, Chao Gao wrote:
>> From: Lan Tianyu <tianyu.lan@intel.com>
>> 
>> 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@intel.com>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> ---
>>  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.

Will pay attention to this kind of issue.
>
>> +
>>  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?
>

A request is a abstract of legacy interrupt or remappable interrupt.
vIOMMU can deliver a request (or an interrupt) to vlapic. It also
can translate the request to interrupt attributes (vector,
destination...) with the help of an interrupt remapping table.
This function gets the corresponding interrupt attributes of the request.

Thanks
Chao

>> +                        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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel