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

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hello Julien,   

> On 4 Dec 2020, at 9:05 am, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi Rahul,
> 
> On 03/12/2020 14:33, Rahul Singh wrote:
>>> On 2 Dec 2020, at 2:45 pm, Julien Grall <julien@xxxxxxx> 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