Re: [PATCH v2 7/8] xen/arm: Remove Linux specific code that is not usable in XEN

[Rahul Singh <Rahul.Singh@arm.com>]

Hello Julien,	

> On 4 Dec 2020, at 9:05 am, Julien Grall <julien@xen.org> wrote:
> 
> Hi Rahul,
> 
> On 03/12/2020 14:33, Rahul Singh wrote:
>>> On 2 Dec 2020, at 2:45 pm, Julien Grall <julien@xen.org> wrote:
>>>> -
>>>> -static struct iommu_device *arm_smmu_probe_device(struct device *dev)
>>>> -{
>>> 
>>> Most of the code here looks useful to Xen. I think you want to keep the code and re-use it afterwards.
>> Ok. I removed the code here and added the XEN compatible code to add devices in next patch.
>> I will keep it in this patch and will modifying the code to make XEN compatible.
> 
> In general, it is prefer if the code the code rather than dropping in patch A and then add it again differently patch B. This makes easier to check that the code outcome of the function is mostly the same.
> 

Ok.

>>>> -static struct iommu_ops arm_smmu_ops = {
>>>> -	.capable		= arm_smmu_capable,
>>>> -	.domain_alloc		= arm_smmu_domain_alloc,
>>>> -	.domain_free		= arm_smmu_domain_free,
>>>> -	.attach_dev		= arm_smmu_attach_dev,
>>>> -	.map			= arm_smmu_map,
>>>> -	.unmap			= arm_smmu_unmap,
>>>> -	.flush_iotlb_all	= arm_smmu_flush_iotlb_all,
>>>> -	.iotlb_sync		= arm_smmu_iotlb_sync,
>>>> -	.iova_to_phys		= arm_smmu_iova_to_phys,
>>>> -	.probe_device		= arm_smmu_probe_device,
>>>> -	.release_device		= arm_smmu_release_device,
>>>> -	.device_group		= arm_smmu_device_group,
>>>> -	.domain_get_attr	= arm_smmu_domain_get_attr,
>>>> -	.domain_set_attr	= arm_smmu_domain_set_attr,
>>>> -	.of_xlate		= arm_smmu_of_xlate,
>>>> -	.get_resv_regions	= arm_smmu_get_resv_regions,
>>>> -	.put_resv_regions	= generic_iommu_put_resv_regions,
>>>> -	.pgsize_bitmap		= -1UL, /* Restricted during device attach */
>>>> -};
>>>> -
>>>>  /* Probing and initialisation functions */
>>>>  static int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
>>>>  				   struct arm_smmu_queue *q,
>>>> @@ -2406,7 +2032,6 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
>>>>  	switch (FIELD_GET(IDR0_STALL_MODEL, reg)) {
>>>>  	case IDR0_STALL_MODEL_FORCE:
>>>>  		smmu->features |= ARM_SMMU_FEAT_STALL_FORCE;
>>>> -		fallthrough;
>>> 
>>> We should keep all the fallthrough documented. So I think we want to introduce the fallthrough in Xen as well.
>> Ok I will keep fallthrough documented in this patch.
>> fallthrough implementation in XEN should be another patch. I am not sure when we can implement but we will try to implement.
> 
> Yes, I didn't ask to implement "fallthrough" in this patch, but instead as a pre-requirement patch.
> 
> I would implement it in include/xen/compiler.h.

Ok. I will implement and will share the patch for “__attribute__((__fallthrough__)) ” but for SMMUv3 is that ok if we can proceed with “ /* fallthrough */  ". 

As “fallthorugh” implementation is common for all architecture it required review for all stakeholder as per my understanding. I don’t want to block SMMUv3 because of this.

Regards,
Rahul
 
> 
> Cheers,
> 
> -- 
> Julien Grall