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

[Manish Jaggi]

On 06/02/2018 12:28 AM, Sameer Goel wrote:


On 5/31/2018 11:16 PM, Manish Jaggi wrote:


On 05/31/2018 09:27 PM, Sameer Goel wrote:

On 5/30/2018 10:13 PM, Manish Jaggi wrote:

On 05/31/2018 04:31 AM, Sameer Goel wrote:

+
+static int arm_smmu_iommu_domain_init(struct domain *d)

Where is iommu_domain initialized?
The function does not use a iommu_domain * variable

Please check iommu.c 2 levels up.

In this function do you see iommu_domain getting allocated or initialized?
As per the name of function arm_smmu iommu_domain_init.
Where is init of iommu_domain in this function?

Well without the xen_domain the iommu_domain is not initialized. It is just the 
default value. This generic iommu code makes an .init call to our code for 
whatever initialization is needed. So the name here seemed absolutely fine to 
me.

Initialization does not always refer to allocation. In this case this is driver 
specific initialization. Since, the iommu code is making an init call to the 
smmu code hence the name arm_smmu_iommu_domain_init. So, again I agree with 
your comments on the domain variable names and I'm making these changes as they 
would make the code cleaner. This function name change probably will not do 
much but the move along the discussion, let me know what you were thinking.

Sameer, few points
a. all the functions are prefixed with arm_smmu_ , so what the function is 
doing can be understood by the rest part of the name
In this case it is iommu_domain_init.

b. By the name it seems to suggest that you are  doing some kind of init for 
iommu_domain

c. But in this complete function, iommu_domain pointer is never used.

If I take your point, the appropriate name of the function should be 
arm_smmu_xen_domain_init().

Its not the iommu domain defined within the current driver.  I'll change the 
name to arm_smmu_xen_iommu_init().

arm_smmu for just keeping the prefixes as needed. Sounds good?

yes. thanks.


Thanks,
Sameer

-Manish

+static int arm_smmu_iommu_domain_init(struct domain *d)
+{
+    struct arm_smmu_xen_domain *xen_domain;
+
+    xen_domain = xzalloc(struct arm_smmu_xen_domain);
+    if (!xen_domain)
+    return -ENOMEM;
+
+    spin_lock_init(_domain->lock);
+    INIT_LIST_HEAD(_domain->contexts);
+
+    dom_iommu(d)->arch.priv = xen_domain;
+
+    return 0;
+}
+




Thanks,
Sameer

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

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


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


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



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

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

[Sameer Goel]

On 5/31/2018 11:16 PM, Manish Jaggi wrote:
>
>
> On 05/31/2018 09:27 PM, Sameer Goel wrote:
>>
>> On 5/30/2018 10:13 PM, Manish Jaggi wrote:
>>>
>>> On 05/31/2018 04:31 AM, Sameer Goel wrote:
 +
 +static int arm_smmu_iommu_domain_init(struct domain *d)
>>> Where is iommu_domain initialized?
>>> The function does not use a iommu_domain * variable
 Please check iommu.c 2 levels up.
>>> In this function do you see iommu_domain getting allocated or initialized?
>>> As per the name of function arm_smmu iommu_domain_init.
>>> Where is init of iommu_domain in this function?
>> Well without the xen_domain the iommu_domain is not initialized. It is just 
>> the default value. This generic iommu code makes an .init call to our code 
>> for whatever initialization is needed. So the name here seemed absolutely 
>> fine to me.
>>
>> Initialization does not always refer to allocation. In this case this is 
>> driver specific initialization. Since, the iommu code is making an init call 
>> to the smmu code hence the name arm_smmu_iommu_domain_init. So, again I 
>> agree with your comments on the domain variable names and I'm making these 
>> changes as they would make the code cleaner. This function name change 
>> probably will not do much but the move along the discussion, let me know 
>> what you were thinking.
> Sameer, few points
> a. all the functions are prefixed with arm_smmu_ , so what the function is 
> doing can be understood by the rest part of the name
> In this case it is iommu_domain_init.
>
> b. By the name it seems to suggest that you are  doing some kind of init for 
> iommu_domain
>
> c. But in this complete function, iommu_domain pointer is never used.
>
> If I take your point, the appropriate name of the function should be 
> arm_smmu_xen_domain_init().
Its not the iommu domain defined within the current driver.  I'll change the 
name to arm_smmu_xen_iommu_init().

arm_smmu for just keeping the prefixes as needed. Sounds good?

Thanks,
Sameer
>
> -Manish
>>> +static int arm_smmu_iommu_domain_init(struct domain *d)
>>> +{
>>> +    struct arm_smmu_xen_domain *xen_domain;
>>> +
>>> +    xen_domain = xzalloc(struct arm_smmu_xen_domain);
>>> +    if (!xen_domain)
>>> +    return -ENOMEM;
>>> +
>>> +    spin_lock_init(_domain->lock);
>>> +    INIT_LIST_HEAD(_domain->contexts);
>>> +
>>> +    dom_iommu(d)->arch.priv = xen_domain;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>
>>>
>>>
 Thanks,
 Sameer
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
>>>
>>> ___
>>> Xen-devel mailing list
>>> Xen-devel@lists.xenproject.org
>>> https://lists.xenproject.org/mailman/listinfo/xen-devel
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel


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

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

[Manish Jaggi]

On 05/31/2018 09:27 PM, Sameer Goel wrote:


On 5/30/2018 10:13 PM, Manish Jaggi wrote:


On 05/31/2018 04:31 AM, Sameer Goel wrote:

+
+static int arm_smmu_iommu_domain_init(struct domain *d)

Where is iommu_domain initialized?
The function does not use a iommu_domain * variable

Please check iommu.c 2 levels up.

In this function do you see iommu_domain getting allocated or initialized?
As per the name of function arm_smmu iommu_domain_init.
Where is init of iommu_domain in this function?

Well without the xen_domain the iommu_domain is not initialized. It is just the 
default value. This generic iommu code makes an .init call to our code for 
whatever initialization is needed. So the name here seemed absolutely fine to 
me.

Initialization does not always refer to allocation. In this case this is driver 
specific initialization. Since, the iommu code is making an init call to the 
smmu code hence the name arm_smmu_iommu_domain_init. So, again I agree with 
your comments on the domain variable names and I'm making these changes as they 
would make the code cleaner. This function name change probably will not do 
much but the move along the discussion, let me know what you were thinking.

Sameer, few points
a. all the functions are prefixed with arm_smmu_ , so what the function 
is doing can be understood by the rest part of the name

In this case it is iommu_domain_init.

b. By the name it seems to suggest that you are  doing some kind of init 
for iommu_domain


c. But in this complete function, iommu_domain pointer is never used.

If I take your point, the appropriate name of the function should be 
arm_smmu_xen_domain_init().


-Manish

+static int arm_smmu_iommu_domain_init(struct domain *d)
+{
+    struct arm_smmu_xen_domain *xen_domain;
+
+    xen_domain = xzalloc(struct arm_smmu_xen_domain);
+    if (!xen_domain)
+    return -ENOMEM;
+
+    spin_lock_init(_domain->lock);
+    INIT_LIST_HEAD(_domain->contexts);
+
+    dom_iommu(d)->arch.priv = xen_domain;
+
+    return 0;
+}
+




Thanks,
Sameer

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


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



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

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

[Goel, Sameer]

On 5/30/2018 10:10 PM, Manish Jaggi wrote:
> 
> 
> On 05/31/2018 01:16 AM, Sameer Goel wrote:
>> Manish, I'll take another look at the variable names. I might not have 
>> enough time :).

 On 05/23/2018 10:48 PM, Manish Jaggi wrote:
> Hi Sameer,
>
> General Comment, please use appropriate variable names for XXX_domain 
> structures in code which is xen specific.
 I thought that we had discussed this before on one of the RFCs.
>>> Yes and no one replied to my last comment that the var names have to be non 
>>> confusing in Xen specific code.
>> I'm sorry about that. I just assumed that since the policy on ported 
>> variable names was consistent we so not nee to change the names.
>>
 At this point we are just using the format used for smmu-v2.
>>> smmu-v2 has a lot of confusing variable names with _domain.
>>> I believe that file needs to be fixed as well.
 I don't think that the variable names are inappropriate. Unless there is a 
 very specific issue with the variable names,
>>> The issue is in code readability and understanding the flow.
>>> It is confusing so many _domain variable names are used which are not 
>>> verbose.
>> These names are coming from the original Linux code. So, we do not change 
>> them as per the policy. That being said I can add a comment section to 
>> explain the confusing variables in the Xen specific code? Please let me know 
>> if this will help us resolve this issue?
>>   
> If it is in Xen specific functions, then it can be changed, this is what 
> julien also confirmed.

Agreed. Changing the var names.
> 
>>> Two functions different and confusing variable names
>> +    struct iommu_domain *domain;
>> +};
>> +
> As this is a xen specific code, can the variable names be used 
> appropriately.
> Repeating my comment  from earlier version.
> a domain is  usually a VM in Xen. So it is a bit confusing to use domain 
> for iommu_domain.
>> The struct is fine but you have an issue with the var name right?
> Yes.
>>
>> +/*
>> + * Xen: io_pgtable compatibility defines.
>> + * Most of these are to port in the S1 translation code as is.
>> + */
>> +struct io_pgtable_ops {
>> +};
>> +
>> +struct iommu_gather_ops {
>> +    void (*tlb_flush_all)(void *cookie);
>> +    void (*tlb_add_flush)(unsigned long iova, size_t size, size_t 
>> granule,
>> +  bool leaf, void *cookie);
>> +    void (*tlb_sync)(void *cookie);
>> +};
>> +
>> +struct io_pgtable_cfg {
>> +    /*
>> + * IO_PGTABLE_QUIRK_ARM_NS: (ARM formats) Set NS and NSTABLE bits in
>> + *    stage 1 PTEs, for hardware which insists on validating them
>> + *    even in    non-secure state where they should normally be 
>> ignored.
>> + *
>> + * IO_PGTABLE_QUIRK_NO_PERMS: Ignore the IOMMU_READ, IOMMU_WRITE and
>> + *    IOMMU_NOEXEC flags and map everything with full access, for
>> + *    hardware which does not implement the permissions of a given
>> + *    format, and/or requires some format-specific default value.
>> + *
>> + * IO_PGTABLE_QUIRK_TLBI_ON_MAP: If the format forbids caching 
>> invalid
>> + *    (unmapped) entries but the hardware might do so anyway, 
>> perform
>> + *    TLB maintenance when mapping as well as when unmapping.
>> + *
>> + * IO_PGTABLE_QUIRK_ARM_MTK_4GB: (ARM v7s format) Set bit 9 in all
>> + *    PTEs, for Mediatek IOMMUs which treat it as a 33rd address bit
>> + *    when the SoC is in "4GB mode" and they can only access the 
>> high
>> + *    remap of DRAM (0x1_ to 0x1_).
>> + *
>> + * IO_PGTABLE_QUIRK_NO_DMA: Guarantees that the tables will only 
>> ever
>> + *    be accessed by a fully cache-coherent IOMMU or CPU (e.g. for a
>> + *    software-emulated IOMMU), such that pagetable updates need not
>> + *    be treated as explicit DMA data.
>> + */
>> +    #define IO_PGTABLE_QUIRK_ARM_NS    BIT(0)
>> +    #define IO_PGTABLE_QUIRK_NO_PERMS    BIT(1)
>> +    #define IO_PGTABLE_QUIRK_TLBI_ON_MAP    BIT(2)
>> +    #define IO_PGTABLE_QUIRK_ARM_MTK_4GB    BIT(3)
>> +    #define IO_PGTABLE_QUIRK_NO_DMA    BIT(4)
>> +    unsigned long    quirks;
>> +    unsigned long    pgsize_bitmap;
>> +    unsigned int    ias;
>> +    unsigned int    oas;
>> +    const struct iommu_gather_ops    *tlb;
>> +    struct device    *iommu_dev;
>> +
>> +    /* Low-level data specific to the table format */
>> +    union {
>> +    struct {
>> +    u64    ttbr[2];
>> +    u64    tcr;
>> +    u64    mair[2];
>> +    } arm_lpae_s1_cfg;
>> +
>> +    struct {
>> +    u64    vttbr;
>> 

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

[Sameer Goel]

On 5/30/2018 10:13 PM, Manish Jaggi wrote:
>
>
> On 05/31/2018 04:31 AM, Sameer Goel wrote:
>> +
>> +static int arm_smmu_iommu_domain_init(struct domain *d)
> Where is iommu_domain initialized?
> The function does not use a iommu_domain * variable
>> Please check iommu.c 2 levels up.
> In this function do you see iommu_domain getting allocated or initialized?
> As per the name of function arm_smmu iommu_domain_init.
> Where is init of iommu_domain in this function?
Well without the xen_domain the iommu_domain is not initialized. It is just the 
default value. This generic iommu code makes an .init call to our code for 
whatever initialization is needed. So the name here seemed absolutely fine to 
me.

Initialization does not always refer to allocation. In this case this is driver 
specific initialization. Since, the iommu code is making an init call to the 
smmu code hence the name arm_smmu_iommu_domain_init. So, again I agree with 
your comments on the domain variable names and I'm making these changes as they 
would make the code cleaner. This function name change probably will not do 
much but the move along the discussion, let me know what you were thinking.
>
> +static int arm_smmu_iommu_domain_init(struct domain *d)
> +{
> +    struct arm_smmu_xen_domain *xen_domain;
> +
> +    xen_domain = xzalloc(struct arm_smmu_xen_domain);
> +    if (!xen_domain)
> +    return -ENOMEM;
> +
> +    spin_lock_init(_domain->lock);
> +    INIT_LIST_HEAD(_domain->contexts);
> +
> +    dom_iommu(d)->arch.priv = xen_domain;
> +
> +    return 0;
> +}
> +
>
>
>
>> Thanks,
>> Sameer
>>>
>>> ___
>>> Xen-devel mailing list
>>> Xen-devel@lists.xenproject.org
>>> https://lists.xenproject.org/mailman/listinfo/xen-devel
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel


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

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

[Manish Jaggi]

On 05/31/2018 04:31 AM, Sameer Goel wrote:

+
+static int arm_smmu_iommu_domain_init(struct domain *d)

Where is iommu_domain initialized?
The function does not use a iommu_domain * variable

Please check iommu.c 2 levels up.

In this function do you see iommu_domain getting allocated or initialized?
As per the name of function arm_smmu iommu_domain_init.
Where is init of iommu_domain in this function?

+static int arm_smmu_iommu_domain_init(struct domain *d)
+{
+   struct arm_smmu_xen_domain *xen_domain;
+
+   xen_domain = xzalloc(struct arm_smmu_xen_domain);
+   if (!xen_domain)
+   return -ENOMEM;
+
+   spin_lock_init(_domain->lock);
+   INIT_LIST_HEAD(_domain->contexts);
+
+   dom_iommu(d)->arch.priv = xen_domain;
+
+   return 0;
+}
+




Thanks,
Sameer


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



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

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

[Manish Jaggi]

On 05/31/2018 01:16 AM, Sameer Goel wrote:

Manish, I'll take another look at the variable names. I might not have enough 
time :).


On 05/23/2018 10:48 PM, Manish Jaggi wrote:

Hi Sameer,

General Comment, please use appropriate variable names for XXX_domain 
structures in code which is xen specific.

I thought that we had discussed this before on one of the RFCs.

Yes and no one replied to my last comment that the var names have to be non 
confusing in Xen specific code.

I'm sorry about that. I just assumed that since the policy on ported variable 
names was consistent we so not nee to change the names.


At this point we are just using the format used for smmu-v2.

smmu-v2 has a lot of confusing variable names with _domain.
I believe that file needs to be fixed as well.

I don't think that the variable names are inappropriate. Unless there is a very 
specific issue with the variable names,

The issue is in code readability and understanding the flow.
It is confusing so many _domain variable names are used which are not verbose.

These names are coming from the original Linux code. So, we do not change them 
as per the policy. That being said I can add a comment section to explain the 
confusing variables in the Xen specific code? Please let me know if this will 
help us resolve this issue?
  
If it is in Xen specific functions, then it can be changed, this is what 
julien also confirmed.



Two functions different and confusing variable names

+static int __must_check arm_smmu_iotlb_flush_all(struct domain *d)
+{
+    struct arm_smmu_xen_domain *smmu_domain;

+static struct iommu_domain *arm_smmu_get_domain(struct domain *d,
+    struct device *dev)
+{
+    struct iommu_domain *domain;
+    struct arm_smmu_xen_domain *xen_domain;

Why use smmu_domain at one place and xen_domain at another ?
Intuitively xen_domain should mean a VM.

Doesn't is look confusing?

Please take a look, I have pointed out the other specific ones below.

I think we should stick with the current version.



On 05/24/2018 06:16 AM, Sameer Goel wrote:

This driver follows an approach similar to smmu driver. The intent here
is to reuse as much Linux code as possible.
- Glue code has been introduced to bridge the API calls.
- Called Linux functions from the Xen IOMMU function calls.
- Xen modifications are preceded by /*Xen: comment */
- xen/linux_compat: Add a Linux compat header
    For porting files directly from Linux it is useful to have a function 
mapping
    definitions from Linux to Xen. This file adds common API functions and
    other defines that are needed for porting arm SMMU drivers.

Signed-off-by: Sameer Goel 
---
   xen/arch/arm/p2m.c    |   1 +
   xen/drivers/Kconfig   |   2 +
   xen/drivers/passthrough/arm/Kconfig   |   8 +
   xen/drivers/passthrough/arm/Makefile  |   1 +
   xen/drivers/passthrough/arm/smmu-v3.c | 934 +-
   xen/include/xen/linux_compat.h    |  84 +++
   6 files changed, 1001 insertions(+), 29 deletions(-)
   create mode 100644 xen/drivers/passthrough/arm/Kconfig
   create mode 100644 xen/include/xen/linux_compat.h

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d43c3aa896..38aa9f00c1 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1454,6 +1454,7 @@ err:
   static void __init setup_virt_paging_one(void *data)
   {
   unsigned long val = (unsigned long)data;
+    /* SMMUv3 S2 cfg vtcr reuses the following value */
   WRITE_SYSREG32(val, VTCR_EL2);
   isb();
   }
diff --git a/xen/drivers/Kconfig b/xen/drivers/Kconfig
index db94393f47..59ca00f850 100644
--- a/xen/drivers/Kconfig
+++ b/xen/drivers/Kconfig
@@ -15,4 +15,6 @@ source "drivers/video/Kconfig"
   config HAS_VPCI
   bool
   +source "drivers/passthrough/arm/Kconfig"
+
   endmenu
diff --git a/xen/drivers/passthrough/arm/Kconfig 
b/xen/drivers/passthrough/arm/Kconfig
new file mode 100644
index 00..cda899f608
--- /dev/null
+++ b/xen/drivers/passthrough/arm/Kconfig
@@ -0,0 +1,8 @@
+
+config ARM_SMMU_v3
+    bool "ARM SMMUv3 Support"
+    depends on ARM_64
+    help
+ Support for implementations of the ARM System MMU architecture
+ version 3.
+
diff --git a/xen/drivers/passthrough/arm/Makefile 
b/xen/drivers/passthrough/arm/Makefile
index f4cd26e15d..e14732b55c 100644
--- a/xen/drivers/passthrough/arm/Makefile
+++ b/xen/drivers/passthrough/arm/Makefile
@@ -1,2 +1,3 @@
   obj-y += iommu.o
   obj-y += smmu.o
+obj-$(CONFIG_ARM_SMMU_v3) += smmu-v3.o
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
b/xen/drivers/passthrough/arm/smmu-v3.c
index e67ba6c40f..df81626785 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -18,28 +18,414 @@
    * Author: Will Deacon 
    *
    * This driver is powered by bad coffee and bombay mix.
+ *
+ *
+ * Based on Linux drivers/iommu/arm-smmu-v3.c
+ * => commit 7aa8619a66aea52b145e04cbab4f8d6a4e5f3f3b
+ *
+ * Xen modifications:
+ * 

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

[Sameer Goel]

 +
 +static int arm_smmu_iommu_domain_init(struct domain *d)
>>> Where is iommu_domain initialized?
>>> The function does not use a iommu_domain * variable
Please check iommu.c 2 levels up.
Thanks,
Sameer

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


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

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

[Sameer Goel]

Manish, I'll take another look at the variable names. I might not have enough 
time :).
>>
>>
>> On 05/23/2018 10:48 PM, Manish Jaggi wrote:
>>> Hi Sameer,
>>>
>>> General Comment, please use appropriate variable names for XXX_domain 
>>> structures in code which is xen specific.
>> I thought that we had discussed this before on one of the RFCs.
> Yes and no one replied to my last comment that the var names have to be non 
> confusing in Xen specific code.
I'm sorry about that. I just assumed that since the policy on ported variable 
names was consistent we so not nee to change the names.

>> At this point we are just using the format used for smmu-v2.
> smmu-v2 has a lot of confusing variable names with _domain.
> I believe that file needs to be fixed as well.
>> I don't think that the variable names are inappropriate. Unless there is a 
>> very specific issue with the variable names,
> The issue is in code readability and understanding the flow.
> It is confusing so many _domain variable names are used which are not verbose.
These names are coming from the original Linux code. So, we do not change them 
as per the policy. That being said I can add a comment section to explain the 
confusing variables in the Xen specific code? Please let me know if this will 
help us resolve this issue?
 
>
> Two functions different and confusing variable names
>
> +static int __must_check arm_smmu_iotlb_flush_all(struct domain *d)
> +{
> +    struct arm_smmu_xen_domain *smmu_domain;
>
> +static struct iommu_domain *arm_smmu_get_domain(struct domain *d,
> +    struct device *dev)
> +{
> +    struct iommu_domain *domain;
> +    struct arm_smmu_xen_domain *xen_domain;
>
> Why use smmu_domain at one place and xen_domain at another ?
> Intuitively xen_domain should mean a VM.
>
> Doesn't is look confusing?
>
> Please take a look, I have pointed out the other specific ones below.
>> I think we should stick with the current version.
>>
>>>
>>>
>>> On 05/24/2018 06:16 AM, Sameer Goel wrote:
 This driver follows an approach similar to smmu driver. The intent here
 is to reuse as much Linux code as possible.
 - Glue code has been introduced to bridge the API calls.
 - Called Linux functions from the Xen IOMMU function calls.
 - Xen modifications are preceded by /*Xen: comment */
 - xen/linux_compat: Add a Linux compat header
    For porting files directly from Linux it is useful to have a function 
 mapping
    definitions from Linux to Xen. This file adds common API functions and
    other defines that are needed for porting arm SMMU drivers.

 Signed-off-by: Sameer Goel 
 ---
   xen/arch/arm/p2m.c    |   1 +
   xen/drivers/Kconfig   |   2 +
   xen/drivers/passthrough/arm/Kconfig   |   8 +
   xen/drivers/passthrough/arm/Makefile  |   1 +
   xen/drivers/passthrough/arm/smmu-v3.c | 934 +-
   xen/include/xen/linux_compat.h    |  84 +++
   6 files changed, 1001 insertions(+), 29 deletions(-)
   create mode 100644 xen/drivers/passthrough/arm/Kconfig
   create mode 100644 xen/include/xen/linux_compat.h

 diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
 index d43c3aa896..38aa9f00c1 100644
 --- a/xen/arch/arm/p2m.c
 +++ b/xen/arch/arm/p2m.c
 @@ -1454,6 +1454,7 @@ err:
   static void __init setup_virt_paging_one(void *data)
   {
   unsigned long val = (unsigned long)data;
 +    /* SMMUv3 S2 cfg vtcr reuses the following value */
   WRITE_SYSREG32(val, VTCR_EL2);
   isb();
   }
 diff --git a/xen/drivers/Kconfig b/xen/drivers/Kconfig
 index db94393f47..59ca00f850 100644
 --- a/xen/drivers/Kconfig
 +++ b/xen/drivers/Kconfig
 @@ -15,4 +15,6 @@ source "drivers/video/Kconfig"
   config HAS_VPCI
   bool
   +source "drivers/passthrough/arm/Kconfig"
 +
   endmenu
 diff --git a/xen/drivers/passthrough/arm/Kconfig 
 b/xen/drivers/passthrough/arm/Kconfig
 new file mode 100644
 index 00..cda899f608
 --- /dev/null
 +++ b/xen/drivers/passthrough/arm/Kconfig
 @@ -0,0 +1,8 @@
 +
 +config ARM_SMMU_v3
 +    bool "ARM SMMUv3 Support"
 +    depends on ARM_64
 +    help
 + Support for implementations of the ARM System MMU architecture
 + version 3.
 +
 diff --git a/xen/drivers/passthrough/arm/Makefile 
 b/xen/drivers/passthrough/arm/Makefile
 index f4cd26e15d..e14732b55c 100644
 --- a/xen/drivers/passthrough/arm/Makefile
 +++ b/xen/drivers/passthrough/arm/Makefile
 @@ -1,2 +1,3 @@
   obj-y += iommu.o
   obj-y += smmu.o
 +obj-$(CONFIG_ARM_SMMU_v3) += smmu-v3.o
 diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
 b/xen/drivers/passthrough/arm/smmu-v3.c
 index e67ba6c40f..df81626785 100644
 --- a/xen/drivers/passthrough/arm/smmu-v3.c
 +++ 

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

[Sameer Goel]

On 5/25/2018 1:10 AM, Jan Beulich wrote:
 On 24.05.18 at 22:26,  wrote:
>> On 05/24/2018 01:57 AM, Jan Beulich wrote:
>> On 24.05.18 at 02:46,  wrote:
 --- /dev/null
 +++ b/xen/include/xen/linux_compat.h
>>> I continue to dislike the idea of having a header with these contents in 
>> this location.
>> As explained previously this header can be used for the any driver that 
>> we want to port from Linux. This is not arm specific. This seemed like 
>> the best location for the file.
This code was added to every source file that was being ported. I found this 
explicitly in smmu-v2.c. Eventually this might be needed for the iort code that 
Manish might pull in.

The function signatures in the header file are consistent and should not change.

So, if we plan on porting a lot of Linux code, I would argue that this file is 
a necessity so that we do not define these functions in every ported source 
file.  (If we want to port Linux code with minimal changes.) The only example I 
found in the code base is arm smmu driver and the iort code that is being 
pulled in. So, we can move it to an arm specific location if needed, but this 
header is needed.

Thanks,
Sameer
> Please take into consideration that with lots of code originally having come
> from Linux, in all the years we've never had a need to have such a header.
> With that history, it needs good reasoning to introduce one now in such a
> global fashion.
>
>> Which other common location should I pick?
> None - that's the whole point of my comment: I don't see this as something
> that should go in a common location.
>
> Jan
>
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel


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

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

[Jan Beulich]

>>> On 24.05.18 at 22:26,  wrote:
> On 05/24/2018 01:57 AM, Jan Beulich wrote:
> On 24.05.18 at 02:46,  wrote:
>>> --- /dev/null
>>> +++ b/xen/include/xen/linux_compat.h
>> I continue to dislike the idea of having a header with these contents in 
> this location.
> As explained previously this header can be used for the any driver that 
> we want to port from Linux. This is not arm specific. This seemed like 
> the best location for the file.

Please take into consideration that with lots of code originally having come
from Linux, in all the years we've never had a need to have such a header.
With that history, it needs good reasoning to introduce one now in such a
global fashion.

> Which other common location should I pick?

None - that's the whole point of my comment: I don't see this as something
that should go in a common location.

Jan



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

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

[Sameer Goel]

On 05/23/2018 10:48 PM, Manish Jaggi wrote:

Hi Sameer,

General Comment, please use appropriate variable names for XXX_domain 
structures in code which is xen specific.
I thought that we had discussed this before on one of the RFCs. At this 
point we are just using the format used for smmu-v2. I don't think that 
the variable names are inappropriate. Unless there is a very specific 
issue with the variable names, I think we should stick with the current 
version.





On 05/24/2018 06:16 AM, Sameer Goel wrote:

This driver follows an approach similar to smmu driver. The intent here
is to reuse as much Linux code as possible.
- Glue code has been introduced to bridge the API calls.
- Called Linux functions from the Xen IOMMU function calls.
- Xen modifications are preceded by /*Xen: comment */
- xen/linux_compat: Add a Linux compat header
   For porting files directly from Linux it is useful to have a 
function mapping
   definitions from Linux to Xen. This file adds common API functions 
and

   other defines that are needed for porting arm SMMU drivers.

Signed-off-by: Sameer Goel 
---
  xen/arch/arm/p2m.c    |   1 +
  xen/drivers/Kconfig   |   2 +
  xen/drivers/passthrough/arm/Kconfig   |   8 +
  xen/drivers/passthrough/arm/Makefile  |   1 +
  xen/drivers/passthrough/arm/smmu-v3.c | 934 +-
  xen/include/xen/linux_compat.h    |  84 +++
  6 files changed, 1001 insertions(+), 29 deletions(-)
  create mode 100644 xen/drivers/passthrough/arm/Kconfig
  create mode 100644 xen/include/xen/linux_compat.h

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d43c3aa896..38aa9f00c1 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1454,6 +1454,7 @@ err:
  static void __init setup_virt_paging_one(void *data)
  {
  unsigned long val = (unsigned long)data;
+    /* SMMUv3 S2 cfg vtcr reuses the following value */
  WRITE_SYSREG32(val, VTCR_EL2);
  isb();
  }
diff --git a/xen/drivers/Kconfig b/xen/drivers/Kconfig
index db94393f47..59ca00f850 100644
--- a/xen/drivers/Kconfig
+++ b/xen/drivers/Kconfig
@@ -15,4 +15,6 @@ source "drivers/video/Kconfig"
  config HAS_VPCI
  bool
  +source "drivers/passthrough/arm/Kconfig"
+
  endmenu
diff --git a/xen/drivers/passthrough/arm/Kconfig 
b/xen/drivers/passthrough/arm/Kconfig

new file mode 100644
index 00..cda899f608
--- /dev/null
+++ b/xen/drivers/passthrough/arm/Kconfig
@@ -0,0 +1,8 @@
+
+config ARM_SMMU_v3
+    bool "ARM SMMUv3 Support"
+    depends on ARM_64
+    help
+ Support for implementations of the ARM System MMU architecture
+ version 3.
+
diff --git a/xen/drivers/passthrough/arm/Makefile 
b/xen/drivers/passthrough/arm/Makefile

index f4cd26e15d..e14732b55c 100644
--- a/xen/drivers/passthrough/arm/Makefile
+++ b/xen/drivers/passthrough/arm/Makefile
@@ -1,2 +1,3 @@
  obj-y += iommu.o
  obj-y += smmu.o
+obj-$(CONFIG_ARM_SMMU_v3) += smmu-v3.o
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
b/xen/drivers/passthrough/arm/smmu-v3.c

index e67ba6c40f..df81626785 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -18,28 +18,414 @@
   * Author: Will Deacon 
   *
   * This driver is powered by bad coffee and bombay mix.
+ *
+ *
+ * Based on Linux drivers/iommu/arm-smmu-v3.c
+ * => commit 7aa8619a66aea52b145e04cbab4f8d6a4e5f3f3b
+ *
+ * Xen modifications:
+ * Sameer Goel 
+ * Copyright (C) 2017, The Linux Foundation, All rights reserved.
+ *
   */
  -#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include 
-
-#include "io-pgtable.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Alias to Xen device tree helpers */
+#define device_node dt_device_node
+#define of_phandle_args dt_phandle_args
+#define of_device_id dt_device_match
+#define of_match_node dt_match_node
+#define of_property_read_u32(np, pname, out) 
(!dt_property_read_u32(np, pname, out))

+#define of_property_read_bool dt_property_read_bool
+#define of_parse_phandle_with_args dt_parse_phandle_with_args
+
+/* Xen: Helpers to get device MMIO and IRQs */
+struct resource {
+    u64 addr;
+    u64 size;
+    unsigned int type;
+};
+
+#define resource_size(res) ((res)->size)
+
+#define platform_device device
+
+#define IORESOURCE_MEM 0
+#define IORESOURCE_IRQ 1
+
+static struct resource *platform_get_resource(struct platform_device 
*pdev,

+  unsigned int type,
+  unsigned int num)
+{
+    /*
+ * The resource is only used between 2 calls of 
platform_get_resource.

+ * It's quite ugly but it's avoid to add too much code in the part
+ * 

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

[Sameer Goel]

Can you please point out the specific instances? This patch is no 
different than the last v1 patch. I have just added tasklets to it.



On 05/23/2018 10:48 PM, Manish Jaggi wrote:

Hi Sameer,

General Comment, please use appropriate variable names for XXX_domain 
structures in code which is xen specific.



On 05/24/2018 06:16 AM, Sameer Goel wrote:

This driver follows an approach similar to smmu driver. The intent here
is to reuse as much Linux code as possible.
- Glue code has been introduced to bridge the API calls.
- Called Linux functions from the Xen IOMMU function calls.
- Xen modifications are preceded by /*Xen: comment */
- xen/linux_compat: Add a Linux compat header
   For porting files directly from Linux it is useful to have a 
function mapping
   definitions from Linux to Xen. This file adds common API functions 
and

   other defines that are needed for porting arm SMMU drivers.

Signed-off-by: Sameer Goel 
---
  xen/arch/arm/p2m.c    |   1 +
  xen/drivers/Kconfig   |   2 +
  xen/drivers/passthrough/arm/Kconfig   |   8 +
  xen/drivers/passthrough/arm/Makefile  |   1 +
  xen/drivers/passthrough/arm/smmu-v3.c | 934 +-
  xen/include/xen/linux_compat.h    |  84 +++
  6 files changed, 1001 insertions(+), 29 deletions(-)
  create mode 100644 xen/drivers/passthrough/arm/Kconfig
  create mode 100644 xen/include/xen/linux_compat.h

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d43c3aa896..38aa9f00c1 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1454,6 +1454,7 @@ err:
  static void __init setup_virt_paging_one(void *data)
  {
  unsigned long val = (unsigned long)data;
+    /* SMMUv3 S2 cfg vtcr reuses the following value */
  WRITE_SYSREG32(val, VTCR_EL2);
  isb();
  }
diff --git a/xen/drivers/Kconfig b/xen/drivers/Kconfig
index db94393f47..59ca00f850 100644
--- a/xen/drivers/Kconfig
+++ b/xen/drivers/Kconfig
@@ -15,4 +15,6 @@ source "drivers/video/Kconfig"
  config HAS_VPCI
  bool
  +source "drivers/passthrough/arm/Kconfig"
+
  endmenu
diff --git a/xen/drivers/passthrough/arm/Kconfig 
b/xen/drivers/passthrough/arm/Kconfig

new file mode 100644
index 00..cda899f608
--- /dev/null
+++ b/xen/drivers/passthrough/arm/Kconfig
@@ -0,0 +1,8 @@
+
+config ARM_SMMU_v3
+    bool "ARM SMMUv3 Support"
+    depends on ARM_64
+    help
+ Support for implementations of the ARM System MMU architecture
+ version 3.
+
diff --git a/xen/drivers/passthrough/arm/Makefile 
b/xen/drivers/passthrough/arm/Makefile

index f4cd26e15d..e14732b55c 100644
--- a/xen/drivers/passthrough/arm/Makefile
+++ b/xen/drivers/passthrough/arm/Makefile
@@ -1,2 +1,3 @@
  obj-y += iommu.o
  obj-y += smmu.o
+obj-$(CONFIG_ARM_SMMU_v3) += smmu-v3.o
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
b/xen/drivers/passthrough/arm/smmu-v3.c

index e67ba6c40f..df81626785 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -18,28 +18,414 @@
   * Author: Will Deacon 
   *
   * This driver is powered by bad coffee and bombay mix.
+ *
+ *
+ * Based on Linux drivers/iommu/arm-smmu-v3.c
+ * => commit 7aa8619a66aea52b145e04cbab4f8d6a4e5f3f3b
+ *
+ * Xen modifications:
+ * Sameer Goel 
+ * Copyright (C) 2017, The Linux Foundation, All rights reserved.
+ *
   */
  -#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include 
-
-#include "io-pgtable.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Alias to Xen device tree helpers */
+#define device_node dt_device_node
+#define of_phandle_args dt_phandle_args
+#define of_device_id dt_device_match
+#define of_match_node dt_match_node
+#define of_property_read_u32(np, pname, out) 
(!dt_property_read_u32(np, pname, out))

+#define of_property_read_bool dt_property_read_bool
+#define of_parse_phandle_with_args dt_parse_phandle_with_args
+
+/* Xen: Helpers to get device MMIO and IRQs */
+struct resource {
+    u64 addr;
+    u64 size;
+    unsigned int type;
+};
+
+#define resource_size(res) ((res)->size)
+
+#define platform_device device
+
+#define IORESOURCE_MEM 0
+#define IORESOURCE_IRQ 1
+
+static struct resource *platform_get_resource(struct platform_device 
*pdev,

+  unsigned int type,
+  unsigned int num)
+{
+    /*
+ * The resource is only used between 2 calls of 
platform_get_resource.

+ * It's quite ugly but it's avoid to add too much code in the part
+ * imported from Linux
+ */
+    static struct resource res;
+    struct acpi_iort_node *iort_node;
+    struct acpi_iort_smmu_v3 *node_smmu_data;
+    int ret = 0;
+
+    

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

[Manish Jaggi]

Hi Sameer,

General Comment, please use appropriate variable names for XXX_domain 
structures in code which is xen specific.



On 05/24/2018 06:16 AM, Sameer Goel wrote:

This driver follows an approach similar to smmu driver. The intent here
is to reuse as much Linux code as possible.
- Glue code has been introduced to bridge the API calls.
- Called Linux functions from the Xen IOMMU function calls.
- Xen modifications are preceded by /*Xen: comment */
- xen/linux_compat: Add a Linux compat header
   For porting files directly from Linux it is useful to have a function mapping
   definitions from Linux to Xen. This file adds common API functions and
   other defines that are needed for porting arm SMMU drivers.

Signed-off-by: Sameer Goel 
---
  xen/arch/arm/p2m.c|   1 +
  xen/drivers/Kconfig   |   2 +
  xen/drivers/passthrough/arm/Kconfig   |   8 +
  xen/drivers/passthrough/arm/Makefile  |   1 +
  xen/drivers/passthrough/arm/smmu-v3.c | 934 +-
  xen/include/xen/linux_compat.h|  84 +++
  6 files changed, 1001 insertions(+), 29 deletions(-)
  create mode 100644 xen/drivers/passthrough/arm/Kconfig
  create mode 100644 xen/include/xen/linux_compat.h

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d43c3aa896..38aa9f00c1 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1454,6 +1454,7 @@ err:
  static void __init setup_virt_paging_one(void *data)
  {
  unsigned long val = (unsigned long)data;
+/* SMMUv3 S2 cfg vtcr reuses the following value */
  WRITE_SYSREG32(val, VTCR_EL2);
  isb();
  }
diff --git a/xen/drivers/Kconfig b/xen/drivers/Kconfig
index db94393f47..59ca00f850 100644
--- a/xen/drivers/Kconfig
+++ b/xen/drivers/Kconfig
@@ -15,4 +15,6 @@ source "drivers/video/Kconfig"
  config HAS_VPCI
bool
  
+source "drivers/passthrough/arm/Kconfig"

+
  endmenu
diff --git a/xen/drivers/passthrough/arm/Kconfig 
b/xen/drivers/passthrough/arm/Kconfig
new file mode 100644
index 00..cda899f608
--- /dev/null
+++ b/xen/drivers/passthrough/arm/Kconfig
@@ -0,0 +1,8 @@
+
+config ARM_SMMU_v3
+   bool "ARM SMMUv3 Support"
+   depends on ARM_64
+   help
+Support for implementations of the ARM System MMU architecture
+version 3.
+
diff --git a/xen/drivers/passthrough/arm/Makefile 
b/xen/drivers/passthrough/arm/Makefile
index f4cd26e15d..e14732b55c 100644
--- a/xen/drivers/passthrough/arm/Makefile
+++ b/xen/drivers/passthrough/arm/Makefile
@@ -1,2 +1,3 @@
  obj-y += iommu.o
  obj-y += smmu.o
+obj-$(CONFIG_ARM_SMMU_v3) += smmu-v3.o
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
b/xen/drivers/passthrough/arm/smmu-v3.c
index e67ba6c40f..df81626785 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -18,28 +18,414 @@
   * Author: Will Deacon 
   *
   * This driver is powered by bad coffee and bombay mix.
+ *
+ *
+ * Based on Linux drivers/iommu/arm-smmu-v3.c
+ * => commit 7aa8619a66aea52b145e04cbab4f8d6a4e5f3f3b
+ *
+ * Xen modifications:
+ * Sameer Goel 
+ * Copyright (C) 2017, The Linux Foundation, All rights reserved.
+ *
   */
  
-#include 

-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include 
-
-#include "io-pgtable.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Alias to Xen device tree helpers */
+#define device_node dt_device_node
+#define of_phandle_args dt_phandle_args
+#define of_device_id dt_device_match
+#define of_match_node dt_match_node
+#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, 
out))
+#define of_property_read_bool dt_property_read_bool
+#define of_parse_phandle_with_args dt_parse_phandle_with_args
+
+/* Xen: Helpers to get device MMIO and IRQs */
+struct resource {
+   u64 addr;
+   u64 size;
+   unsigned int type;
+};
+
+#define resource_size(res) ((res)->size)
+
+#define platform_device device
+
+#define IORESOURCE_MEM 0
+#define IORESOURCE_IRQ 1
+
+static struct resource *platform_get_resource(struct platform_device *pdev,
+ unsigned int type,
+ unsigned int num)
+{
+   /*
+* The resource is only used between 2 calls of platform_get_resource.
+* It's quite ugly but it's avoid to add too much code in the part
+* imported from Linux
+*/
+   static struct resource res;
+   struct acpi_iort_node *iort_node;
+   struct acpi_iort_smmu_v3 *node_smmu_data;
+   int ret = 0;
+
+   res.type = type;
+
+   switch (type) {
+   case IORESOURCE_MEM:
+   if 

[Xen-devel] [v2 4/6] xen/iommu: smmu-v3: Add Xen specific code to enable the ported driver

[Sameer Goel]

This driver follows an approach similar to smmu driver. The intent here
is to reuse as much Linux code as possible.
- Glue code has been introduced to bridge the API calls.
- Called Linux functions from the Xen IOMMU function calls.
- Xen modifications are preceded by /*Xen: comment */
- xen/linux_compat: Add a Linux compat header
  For porting files directly from Linux it is useful to have a function mapping
  definitions from Linux to Xen. This file adds common API functions and
  other defines that are needed for porting arm SMMU drivers.

Signed-off-by: Sameer Goel 
---
 xen/arch/arm/p2m.c|   1 +
 xen/drivers/Kconfig   |   2 +
 xen/drivers/passthrough/arm/Kconfig   |   8 +
 xen/drivers/passthrough/arm/Makefile  |   1 +
 xen/drivers/passthrough/arm/smmu-v3.c | 934 +-
 xen/include/xen/linux_compat.h|  84 +++
 6 files changed, 1001 insertions(+), 29 deletions(-)
 create mode 100644 xen/drivers/passthrough/arm/Kconfig
 create mode 100644 xen/include/xen/linux_compat.h

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d43c3aa896..38aa9f00c1 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1454,6 +1454,7 @@ err:
 static void __init setup_virt_paging_one(void *data)
 {
 unsigned long val = (unsigned long)data;
+/* SMMUv3 S2 cfg vtcr reuses the following value */
 WRITE_SYSREG32(val, VTCR_EL2);
 isb();
 }
diff --git a/xen/drivers/Kconfig b/xen/drivers/Kconfig
index db94393f47..59ca00f850 100644
--- a/xen/drivers/Kconfig
+++ b/xen/drivers/Kconfig
@@ -15,4 +15,6 @@ source "drivers/video/Kconfig"
 config HAS_VPCI
bool
 
+source "drivers/passthrough/arm/Kconfig"
+
 endmenu
diff --git a/xen/drivers/passthrough/arm/Kconfig 
b/xen/drivers/passthrough/arm/Kconfig
new file mode 100644
index 00..cda899f608
--- /dev/null
+++ b/xen/drivers/passthrough/arm/Kconfig
@@ -0,0 +1,8 @@
+
+config ARM_SMMU_v3
+   bool "ARM SMMUv3 Support"
+   depends on ARM_64
+   help
+Support for implementations of the ARM System MMU architecture
+version 3.
+
diff --git a/xen/drivers/passthrough/arm/Makefile 
b/xen/drivers/passthrough/arm/Makefile
index f4cd26e15d..e14732b55c 100644
--- a/xen/drivers/passthrough/arm/Makefile
+++ b/xen/drivers/passthrough/arm/Makefile
@@ -1,2 +1,3 @@
 obj-y += iommu.o
 obj-y += smmu.o
+obj-$(CONFIG_ARM_SMMU_v3) += smmu-v3.o
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
b/xen/drivers/passthrough/arm/smmu-v3.c
index e67ba6c40f..df81626785 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -18,28 +18,414 @@
  * Author: Will Deacon 
  *
  * This driver is powered by bad coffee and bombay mix.
+ *
+ *
+ * Based on Linux drivers/iommu/arm-smmu-v3.c
+ * => commit 7aa8619a66aea52b145e04cbab4f8d6a4e5f3f3b
+ *
+ * Xen modifications:
+ * Sameer Goel 
+ * Copyright (C) 2017, The Linux Foundation, All rights reserved.
+ *
  */
 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include 
-
-#include "io-pgtable.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Alias to Xen device tree helpers */
+#define device_node dt_device_node
+#define of_phandle_args dt_phandle_args
+#define of_device_id dt_device_match
+#define of_match_node dt_match_node
+#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, 
out))
+#define of_property_read_bool dt_property_read_bool
+#define of_parse_phandle_with_args dt_parse_phandle_with_args
+
+/* Xen: Helpers to get device MMIO and IRQs */
+struct resource {
+   u64 addr;
+   u64 size;
+   unsigned int type;
+};
+
+#define resource_size(res) ((res)->size)
+
+#define platform_device device
+
+#define IORESOURCE_MEM 0
+#define IORESOURCE_IRQ 1
+
+static struct resource *platform_get_resource(struct platform_device *pdev,
+ unsigned int type,
+ unsigned int num)
+{
+   /*
+* The resource is only used between 2 calls of platform_get_resource.
+* It's quite ugly but it's avoid to add too much code in the part
+* imported from Linux
+*/
+   static struct resource res;
+   struct acpi_iort_node *iort_node;
+   struct acpi_iort_smmu_v3 *node_smmu_data;
+   int ret = 0;
+
+   res.type = type;
+
+   switch (type) {
+   case IORESOURCE_MEM:
+   if (pdev->type == DEV_ACPI) {
+   ret = 1;
+   iort_node = pdev->acpi_node;
+   node_smmu_data =
+   (struct