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

Re: [PATCH v5 10/10] xen/arm: smmuv3: Add support for SMMUv3 driver


  • To: Oleksandr <olekstysh@xxxxxxxxx>
  • From: Rahul Singh <Rahul.Singh@xxxxxxx>
  • Date: Thu, 21 Jan 2021 17:18:30 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=HizH60qX4bvEdG5ahpydQe3GuW0tpIwgjFD5ZfVyXz4=; b=gl65K9KafpAJAE7TOAZyi123leTEUYJCv4GbOv8b3Df+iRdYlR5CMOOp+tBARG1WFemKt511hQ1xPwrpoRNU87+BKmQRlvaoBvELv40sTowu4ckHeumjxpJCMn+Nu40Q7gQZ6RHWaCZG20IWpHWGP+N0XV2fERSK3K0VYNoNCdSgzhqwP+bGzC6FgqeBTVMduSFDcq2fGMRapUmlZbFwjCG8wOGkwp+CFs490EoPTIkHfqXbgJl4k+72VC5zLg3YuzoQCJVrSKRqBLwpNy/3TpIeksMelgnZ9ZfLvlFw0TZ0C1j5dYBhBhMIOByVkVlarCUErkPK11BDdwqiQBf6yw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ek2L+u5WJWLAZyc9WFx7LBenn0+n+DanpkrS33FGSPbISW+5Kw8XW3uKkqEiJ/d9VNhRKmNIwGN99I26fB0iPLS8325LX4CxiaH2a3D13QwDomoAPAk4Oj2kM//drwjAAUM+RI6Rg08a5X5LuBQN6ltridj1UPaaNDyj90PJw0PuTq9CN94jV4fkf7QLFqm7juwYWHhg7/yZ1UlfdjdGJ+aKM4zSAg3n8q7n5f8bvUNt0SCm4/ZJZ3kPRoYDyCwxc5dD3uWLsrnzQus4ZyW0Nb+91diU5uEgKyfwYIiXRRKGIJCZQmH0Z1kJO7Tc6QFtYt1QftZR66flXovec8S6fw==
  • Authentication-results-original: gmail.com; dkim=none (message not signed) header.d=none;gmail.com; dmarc=none action=none header.from=arm.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Paul Durrant <paul@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Thu, 21 Jan 2021 17:18:51 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: gmail.com; dkim=none (message not signed) header.d=none;gmail.com; dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHW7zzXDLVfiB5hzUKpOAlBcRNn7KoxCUaAgAFLH4A=
  • Thread-topic: [PATCH v5 10/10] xen/arm: smmuv3: Add support for SMMUv3 driver

Hello Oleksandr,

> On 20 Jan 2021, at 9:33 pm, Oleksandr <olekstysh@xxxxxxxxx> wrote:
> 
> 
> On 20.01.21 16:52, Rahul Singh wrote:
> 
> Hi Rahul
> 
>> Add support for ARM architected SMMUv3 implementation. It is based on
>> the Linux SMMUv3 driver.
>> 
>> Driver is currently supported as Tech Preview.
>> 
>> Major differences with regard to Linux driver are as follows:
>> 2. Only Stage-2 translation is supported as compared to the Linux driver
>>    that supports both Stage-1 and Stage-2 translations.
>> 3. Use P2M  page table instead of creating one as SMMUv3 has the
>>    capability to share the page tables with the CPU.
>> 4. Tasklets are used in place of threaded IRQ's in Linux for event queue
>>    and priority queue IRQ handling.
>> 5. Latest version of the Linux SMMUv3 code implements the commands queue
>>    access functions based on atomic operations implemented in Linux.
>>    Atomic functions used by the commands queue access functions are not
>>    implemented in XEN therefore we decided to port the earlier version
>>    of the code. Atomic operations are introduced to fix the bottleneck
>>    of the SMMU command queue insertion operation. A new algorithm for
>>    inserting commands into the queue is introduced, which is lock-free
>>    on the fast-path.
>>    Consequence of reverting the patch is that the command queue
>>    insertion will be slow for large systems as spinlock will be used to
>>    serializes accesses from all CPUs to the single queue supported by
>>    the hardware. Once the proper atomic operations will be available in
>>    XEN the driver can be updated.
>> 6. Spin lock is used in place of mutex when attaching a device to the
>>    SMMU, as there is no blocking locks implementation available in XEN.
>>    This might introduce latency in XEN. Need to investigate before
>>    driver is out for tech preview.
>> 7. PCI ATS functionality is not supported, as there is no support
>>    available in XEN to test the functionality. Code is not tested and
>>    compiled. Code is guarded by the flag CONFIG_PCI_ATS.
>> 8. MSI interrupts are not supported as there is no support available in
>>    XEN to request MSI interrupts. Code is not tested and compiled. Code
>>    is guarded by the flag CONFIG_MSI.
>> 
>> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
>> ---
>> Changes since v2:
>> - added return statement for readx_poll_timeout function.
>> - remove iommu_get_dma_cookie and iommu_put_dma_cookie.
>> - remove struct arm_smmu_xen_device as not required.
>> - move dt_property_match_string to device_tree.c file.
>> - replace arm_smmu_*_thread to arm_smmu_*_tasklet to avoid confusion.
>> - use ARM_SMMU_REG_SZ as size when map memory to XEN.
>> - remove bypass keyword to make sure when device-tree probe is failed we
>>   are reporting error and not continuing to configure SMMU in bypass
>>   mode.
>> - fixed minor comments.
>> Changes since v3:
>> - Fixed typo for CONFIG_MSI
>> - Added back the mutex code
>> - Rebase the patch on top of newly added WARN_ON().
>> - Remove the direct read of register VTCR_EL2.
>> - Fixed minor comments.
>> Changes since v4:
>> - Replace the ffsll() with ffs64() function.
>> - Add code to free resources when probe failed.
> 
> Thank you for addressing, patch looks ok to me, just one small remark below:
> 
> 
>> +
>> +static void __hwdom_init arm_smmu_iommu_hwdom_init(struct domain *d)
>> +{
>> +}
> 
> We discussed in V4 about adding some code here which all IOMMUs on Arm 
> already have, copy it below for the convenience:
> 
> 
>      /* Set to false options not supported on ARM. */
>      if ( iommu_hwdom_inclusive )
>          printk(XENLOG_WARNING
>          "map-inclusive dom0-iommu option is not supported on ARM\n");
>      iommu_hwdom_inclusive = false;
>      if ( iommu_hwdom_reserved == 1 )
>          printk(XENLOG_WARNING
>          "map-reserved dom0-iommu option is not supported on ARM\n");
>      iommu_hwdom_reserved = 0;
> 
>      arch_iommu_hwdom_init(d);
> 
> 
> Also we discussed about possibility to fold the part of code (which disables 
> unsupported options) in arch_iommu_hwdom_init() to avoid duplication as a 
> follow-up.
> At least, I expected to see arch_iommu_hwdom_init() to be called by 
> arm_smmu_iommu_hwdom_init() it current patch... Please note, this is *not* a 
> request for change immediately,
> I think, driver is functional even without this code (hopefully 
> arch_iommu_hwdom_init is empty now, etc).  But, if you happen to do V6 or 
> probably it could be done on commit ...
> 

Yes I will send the patch to move the code to arch_iommu_hwdom_init() to avoid 
duplication once SMMUv3 driver will be merged.
I thought anyway I have to remove the code from SMMUv1 and IPMMU I will take 
care of all the IOMMU driver at the same time because of that I didn’t modify 
the SMMUv3 driver. Yes if there is a need for v6 I will add the 
arch_iommu_hwdom_init(d) in arm_smmu_iommu_hwdom_init().

Regards,
Rahul

> 
>> +
>> +static void arm_smmu_iommu_xen_domain_teardown(struct domain *d)
>> +{
>> +    struct arm_smmu_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
>> +
>> +    ASSERT(list_empty(&xen_domain->contexts));
>> +    xfree(xen_domain);
>> +}
>> +
>> +static const struct iommu_ops arm_smmu_iommu_ops = {
>> +    .init           = arm_smmu_iommu_xen_domain_init,
>> +    .hwdom_init             = arm_smmu_iommu_hwdom_init,
>> +    .teardown               = arm_smmu_iommu_xen_domain_teardown,
>> +    .iotlb_flush            = arm_smmu_iotlb_flush,
>> +    .iotlb_flush_all        = arm_smmu_iotlb_flush_all,
>> +    .assign_device          = arm_smmu_assign_dev,
>> +    .reassign_device        = arm_smmu_reassign_dev,
>> +    .map_page               = arm_iommu_map_page,
>> +    .unmap_page             = arm_iommu_unmap_page,
>> +    .dt_xlate               = arm_smmu_dt_xlate,
>> +    .add_device             = arm_smmu_add_device,
>> +};
>> +
>> +static __init int arm_smmu_dt_init(struct dt_device_node *dev,
>> +                            const void *data)
>> +{
>> +    int rc;
>> +
>> +    /*
>> +     * Even if the device can't be initialized, we don't want to
>> +     * give the SMMU device to dom0.
>> +     */
>> +    dt_device_set_used_by(dev, DOMID_XEN);
>> +
>> +    rc = arm_smmu_device_probe(dt_to_dev(dev));
>> +    if (rc)
>> +            return rc;
>> +
>> +    iommu_set_ops(&arm_smmu_iommu_ops);
>> +
>> +    return 0;
>> +}
>> +
>> +DT_DEVICE_START(smmuv3, "ARM SMMU V3", DEVICE_IOMMU)
>> +.dt_match = arm_smmu_of_match,
>> +.init = arm_smmu_dt_init,
>> +DT_DEVICE_END
> 
> -- 
> Regards,
> 
> Oleksandr Tyshchenko


 


Rackspace

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