Re: [Xen-devel] [PATCH V3 3/29] DOMCTL: Introduce new DOMCTL commands for vIOMMU support

[Lan Tianyu <tianyu.lan@xxxxxxxxx>]

On 2017年10月18日 22:18, Roger Pau Monné wrote:
> On Thu, Sep 21, 2017 at 11:01:44PM -0400, Lan Tianyu wrote:
>> This patch is to introduce create, destroy and query capabilities
>> command for vIOMMU. vIOMMU layer will deal with requests and call
>> arch vIOMMU ops.
>>
>> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
>> ---
>>  xen/common/domctl.c         |  6 ++++++
>>  xen/common/viommu.c         | 30 ++++++++++++++++++++++++++++++
>>  xen/include/public/domctl.h | 42 ++++++++++++++++++++++++++++++++++++++++++
>>  xen/include/xen/viommu.h    |  2 ++
>>  4 files changed, 80 insertions(+)
>>
>> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
>> index 42658e5..7e28237 100644
>> --- a/xen/common/domctl.c
>> +++ b/xen/common/domctl.c
>> @@ -1149,6 +1149,12 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
>> u_domctl)
>>              copyback = 1;
>>          break;
>>  
>> +#ifdef CONFIG_VIOMMU
>> +    case XEN_DOMCTL_viommu_op:
>> +        ret = viommu_domctl(d, &op->u.viommu_op, &copyback);
> 
> IMHO, I'm not really sure if it's worth to pass the copyback parameter
> around. Can you just do the copy if !ret?

Yes, will update.

> 
>> +        break;
>> +#endif
> 
> Instead of guarding every call to a viommu related function with
> CONFIG_VIOMMU I would rather add dummy replacements for them in the
> !CONFIG_VIOMMU case in the viommu.h header.


OK.

> 
>> +
>>      default:
>>          ret = arch_do_domctl(op, d, u_domctl);
>>          break;
>>  /*
>>   * Local variables:
>>   * mode: C
>> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
>> index 50ff58f..68854b6 100644
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -1163,6 +1163,46 @@ struct xen_domctl_psr_cat_op {
>>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>>  
>> +/*  vIOMMU helper
>> + *
>> + *  vIOMMU interface can be used to create/destroy vIOMMU and
>> + *  query vIOMMU capabilities.
>> + */
>> +
>> +/* vIOMMU type - specify vendor vIOMMU device model */
>> +#define VIOMMU_TYPE_INTEL_VTD           0
>> +
>> +/* vIOMMU capabilities */
>> +#define VIOMMU_CAP_IRQ_REMAPPING  (1u << 0)
> 
> Please put those two defines next to the fields they belong to.

OK.




-- 
Best regards
Tianyu Lan

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