[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC v4 6/8] xen/iommu: smmu-v3: Add Xen specific code to enable the ported driver



Hi,

I am slightly confused. I receive your answer on this e-mail after you resend a version. So were the comments on RFC v4 was addressed?

On 02/09/2018 05:56 PM, Sameer Goel wrote:
         /* Request interrupt lines */
       irq = smmu->evtq.q.irq;
@@ -2316,9 +2782,13 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device 
*smmu)
            * Cavium ThunderX2 implementation doesn't not support unique
            * irq lines. Use single irq line for all the SMMUv3 interrupts.
            */
-        ret = devm_request_threaded_irq(smmu->dev, irq,
+        /*
+         * Xen: Does not support threaded irqs, so serialise the setup.
+         * This is the same for pris and event interrupt lines on other
+         * systems
+         */

Above you did implemented a dummy implementation of 
devm_request_threaded_irq(...). So why did you replace the code here?
The replacement worked well for other functions, where the handler was not 
defined. So, the wrapper function calls devm_request_irq with the
argument passed in as thread. In this case really the handler hits first and it 
calls the thread in response. I can modify the code to make this fit into the 
api
but in that case I will need to swap around the functions so number of line 
changes will stay the same. Tell me your preference.

I don't understand what you mean here. Would it be possible to give a concrete example?


+        ret = devm_request_irq(smmu->dev, irq,
                       arm_smmu_combined_irq_handler,
-                    arm_smmu_combined_irq_thread,
                       IRQF_ONESHOT,
                       "arm-smmu-v3-combined-irq", smmu);
           if (ret < 0)
@@ -2542,8 +3012,14 @@ static int arm_smmu_device_hw_probe(struct 
arm_smmu_device *smmu)
           smmu->features |= ARM_SMMU_FEAT_STALLS;
       }
   +/*
+ * Xen: Block stage 1 translations. By doing this here we do not need to set 
the
+ * domain->stage explicitly.
+ */
+#if 0
       if (reg & IDR0_S1P)
           smmu->features |= ARM_SMMU_FEAT_TRANS_S1;
+#endif
         if (reg & IDR0_S2P)
           smmu->features |= ARM_SMMU_FEAT_TRANS_S2;
@@ -2616,10 +3092,12 @@ static int arm_smmu_device_hw_probe(struct 
arm_smmu_device *smmu)
       if (reg & IDR5_GRAN4K)
           smmu->pgsize_bitmap |= SZ_4K | SZ_2M | SZ_1G;
   +#if 0 /* Xen: SMMU ops do not have a pgsize_bitmap member for Xen */
       if (arm_smmu_ops.pgsize_bitmap == -1UL)
           arm_smmu_ops.pgsize_bitmap = smmu->pgsize_bitmap;
       else
           arm_smmu_ops.pgsize_bitmap |= smmu->pgsize_bitmap;
+#endif
         /* Output address size */
       switch (reg & IDR5_OAS_MASK << IDR5_OAS_SHIFT) {
@@ -2680,7 +3158,8 @@ static int arm_smmu_device_acpi_probe(struct 
platform_device *pdev,
       struct device *dev = smmu->dev;
       struct acpi_iort_node *node;
   -    node = *(struct acpi_iort_node **)dev_get_platdata(dev);
+    /* Xen: Modification to get iort_node */
+    node = (struct acpi_iort_node *)dev->acpi_node;
         /* Retrieve SMMUv3 specific data */
       iort_smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
@@ -2703,7 +3182,7 @@ static inline int arm_smmu_device_acpi_probe(struct 
platform_device *pdev,
   static int arm_smmu_device_dt_probe(struct platform_device *pdev,
                       struct arm_smmu_device *smmu)
   {
-    struct device *dev = &pdev->dev;
+    struct device *dev = pdev;
       u32 cells;
       int ret = -EINVAL;
   @@ -2716,6 +3195,7 @@ static int arm_smmu_device_dt_probe(struct 
platform_device *pdev,
         parse_driver_options(smmu);
   +    /* Xen: of_dma_is_coherent is a stub till dt support is introduced */

I think this comment matters more on top of the stub. I would also add an BUG() 
in the stub to catch it.
I have just returned a 0 here. Putting a BUG in might not have the desired 
impact, since, this will execute every time.

How about a WARN_ON_ONCE in the implementation?

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.