Re: [Xen-devel] [PATCH v4 06/28] vtd: clean-up and preparation for vvtd

2018-02-09 Thread Chao Gao 
On Fri, Feb 09, 2018 at 03:17:59PM +, Roger Pau Monné wrote:
>On Fri, Nov 17, 2017 at 02:22:13PM +0800, Chao Gao wrote:
>> This patch contains following changes:
>> - align register definitions
>> - use MASK_EXTR to define some macros about extended capabilies
>> rather than open-coding the masks
>> - define fields of FECTL and FESTS as uint32_t rather than u64 since
>> FECTL and FESTS are 32 bit registers.
>> 
>> No functional changes.
>> 
>> Signed-off-by: Chao Gao 
>> Signed-off-by: Lan Tianyu 
>
>Reviewed-by: Roger Pau Monné 

Thanks. It is the first Reviewed-by we get. :).
>
>Just one nit...
>
>> 
>> ---
>> v4:
>>  - Only fix the alignment and defer introducing new definition to when
>>  they are needed
>>  (Suggested-by Roger Pau Monné)
>>  - remove parts of open-coded masks
>> v3:
>>  - new
>> ---
>>  xen/drivers/passthrough/vtd/iommu.h | 86 
>> +
>>  1 file changed, 48 insertions(+), 38 deletions(-)
>> 
>> diff --git a/xen/drivers/passthrough/vtd/iommu.h 
>> b/xen/drivers/passthrough/vtd/iommu.h
>> index 72c1a2e..db80b31 100644
>> --- a/xen/drivers/passthrough/vtd/iommu.h
>> +++ b/xen/drivers/passthrough/vtd/iommu.h
>> +#define DMA_ECAP_SNP_CTL((uint64_t)1 << 7)
>> +#define DMA_ECAP_PASS_THRU  ((uint64_t)1 << 6)
>> +#define DMA_ECAP_CACHE_HINTS((uint64_t)1 << 5)
>> +#define DMA_ECAP_EIM((uint64_t)1 << 4)
>> +#define DMA_ECAP_INTR_REMAP ((uint64_t)1 << 3)
>> +#define DMA_ECAP_DEV_IOTLB  ((uint64_t)1 << 2)
>> +#define DMA_ECAP_QUEUED_INVAL   ((uint64_t)1 << 1)
>> +#define DMA_ECAP_COHERENT   ((uint64_t)1 << 0)
>
>I think the general practice is to use 1UL (because it's shorter), or
>1U for 32bits.

Will do.

Thanks
chao

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

Re: [Xen-devel] [PATCH v4 06/28] vtd: clean-up and preparation for vvtd

2018-02-09 Thread Roger Pau Monné 
On Fri, Nov 17, 2017 at 02:22:13PM +0800, Chao Gao wrote:
> This patch contains following changes:
> - align register definitions
> - use MASK_EXTR to define some macros about extended capabilies
> rather than open-coding the masks
> - define fields of FECTL and FESTS as uint32_t rather than u64 since
> FECTL and FESTS are 32 bit registers.
> 
> No functional changes.
> 
> Signed-off-by: Chao Gao 
> Signed-off-by: Lan Tianyu 

Reviewed-by: Roger Pau Monné 

Just one nit...

> 
> ---
> v4:
>  - Only fix the alignment and defer introducing new definition to when
>  they are needed
>  (Suggested-by Roger Pau Monné)
>  - remove parts of open-coded masks
> v3:
>  - new
> ---
>  xen/drivers/passthrough/vtd/iommu.h | 86 
> +
>  1 file changed, 48 insertions(+), 38 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/vtd/iommu.h 
> b/xen/drivers/passthrough/vtd/iommu.h
> index 72c1a2e..db80b31 100644
> --- a/xen/drivers/passthrough/vtd/iommu.h
> +++ b/xen/drivers/passthrough/vtd/iommu.h
> +#define DMA_ECAP_SNP_CTL((uint64_t)1 << 7)
> +#define DMA_ECAP_PASS_THRU  ((uint64_t)1 << 6)
> +#define DMA_ECAP_CACHE_HINTS((uint64_t)1 << 5)
> +#define DMA_ECAP_EIM((uint64_t)1 << 4)
> +#define DMA_ECAP_INTR_REMAP ((uint64_t)1 << 3)
> +#define DMA_ECAP_DEV_IOTLB  ((uint64_t)1 << 2)
> +#define DMA_ECAP_QUEUED_INVAL   ((uint64_t)1 << 1)
> +#define DMA_ECAP_COHERENT   ((uint64_t)1 << 0)

I think the general practice is to use 1UL (because it's shorter), or
1U for 32bits.

Thanks, Roger.

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