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

[Julien Grall <julien@xxxxxxx>]

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.
> > > -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.

Cheers,

--
Julien Grall