diff mbox series

[V2,4/6] iommu/arm: Add lightweight iommu_fwspec support

Message ID 1564763985-20312-5-git-send-email-olekstysh@gmail.com (mailing list archive)
State Superseded
Headers show
Series iommu/arm: Add Renesas IPMMU-VMSA support + Linux's iommu_fwspec | expand

Commit Message

Oleksandr Tyshchenko Aug. 2, 2019, 4:39 p.m. UTC
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

We need to have some abstract way to add new device to the IOMMU
based on the generic IOMMU DT binding [1] which can be used for
both DT (right now) and ACPI (in future).

For that reason we can borrow the idea used in Linux these days
called "iommu_fwspec". Having this in, it will be possible
to configure IOMMU master interfaces of the device (device IDs)
from a single common place and avoid keeping almost identifical look-up
implementations in each IOMMU driver.

There is no need to port the whole implementation of "iommu_fwspec"
to Xen, we could, probably, end up with a much simpler solution,
some "stripped down" version which fits our requirments.

So, this patch adds the following:
1. A common structure "iommu_fwspec" to hold the the per-device
   firmware data
2. New member "iommu_fwspec" of struct device
3. Functions/helpers to deal with "dev->iommu_fwspec"

It should be noted that in comparing with original "iommu_fwspec"
Xen's variant doesn't contain some fields, which are not really
needed at the moment (ops, flag) and "iommu_fwnode" field was replaced
by "iommu_dev" to avoid porting a lot of code (to support "fwnode_handle")
with little benefit.

Next patch in this series will make use of that support.

[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/iommu/iommu.txt

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
 xen/drivers/passthrough/arm/Makefile       |  2 +-
 xen/drivers/passthrough/arm/iommu_fwspec.c | 91 ++++++++++++++++++++++++++++++
 xen/include/asm-arm/device.h               |  1 +
 xen/include/asm-arm/iommu.h                |  2 +
 xen/include/asm-arm/iommu_fwspec.h         | 65 +++++++++++++++++++++
 5 files changed, 160 insertions(+), 1 deletion(-)
 create mode 100644 xen/drivers/passthrough/arm/iommu_fwspec.c
 create mode 100644 xen/include/asm-arm/iommu_fwspec.h

Comments

Julien Grall Aug. 13, 2019, 12:39 p.m. UTC | #1
Hi Oleksandr,

On 8/2/19 5:39 PM, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> We need to have some abstract way to add new device to the IOMMU
> based on the generic IOMMU DT binding [1] which can be used for
> both DT (right now) and ACPI (in future).
> 
> For that reason we can borrow the idea used in Linux these days
> called "iommu_fwspec". Having this in, it will be possible
> to configure IOMMU master interfaces of the device (device IDs)
> from a single common place and avoid keeping almost identifical look-up

s/identifical/identical/

> implementations in each IOMMU driver.
> 
> There is no need to port the whole implementation of "iommu_fwspec"
> to Xen, we could, probably, end up with a much simpler solution,
> some "stripped down" version which fits our requirments.

s/requirments/requirements/

> 
> So, this patch adds the following:
> 1. A common structure "iommu_fwspec" to hold the the per-device
>     firmware data
> 2. New member "iommu_fwspec" of struct device
> 3. Functions/helpers to deal with "dev->iommu_fwspec"
> 
> It should be noted that in comparing with original "iommu_fwspec"
> Xen's variant doesn't contain some fields, which are not really
> needed at the moment (ops, flag) and "iommu_fwnode" field was replaced
> by "iommu_dev" to avoid porting a lot of code (to support "fwnode_handle")
> with little benefit.
> 
> Next patch in this series will make use of that support.
> 
> [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/iommu/iommu.txt
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> ---
>   xen/drivers/passthrough/arm/Makefile       |  2 +-
>   xen/drivers/passthrough/arm/iommu_fwspec.c | 91 ++++++++++++++++++++++++++++++
>   xen/include/asm-arm/device.h               |  1 +
>   xen/include/asm-arm/iommu.h                |  2 +
>   xen/include/asm-arm/iommu_fwspec.h         | 65 +++++++++++++++++++++
>   5 files changed, 160 insertions(+), 1 deletion(-)
>   create mode 100644 xen/drivers/passthrough/arm/iommu_fwspec.c
>   create mode 100644 xen/include/asm-arm/iommu_fwspec.h
> 
> diff --git a/xen/drivers/passthrough/arm/Makefile b/xen/drivers/passthrough/arm/Makefile
> index 4abb87a..5fbad45 100644
> --- a/xen/drivers/passthrough/arm/Makefile
> +++ b/xen/drivers/passthrough/arm/Makefile
> @@ -1,2 +1,2 @@
> -obj-y += iommu.o iommu_helpers.o
> +obj-y += iommu.o iommu_helpers.o iommu_fwspec.o
>   obj-$(CONFIG_ARM_SMMU) += smmu.o
> diff --git a/xen/drivers/passthrough/arm/iommu_fwspec.c b/xen/drivers/passthrough/arm/iommu_fwspec.c
> new file mode 100644
> index 0000000..3474192
> --- /dev/null
> +++ b/xen/drivers/passthrough/arm/iommu_fwspec.c
> @@ -0,0 +1,91 @@
> +/*
> + * xen/drivers/passthrough/arm/iommu_fwspec.c
> + *
> + * Contains functions to maintain per-device firmware data
> + *
> + * Based on Linux's iommu_fwspec support you can find at:
> + *    drivers/iommu/iommu.c
> + *
> + * Copyright (C) 2019 EPAM Systems Inc.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms and conditions of the GNU General Public
> + * License, version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/lib.h>
> +#include <xen/iommu.h>

Please order the headers alphabetically.

NIT: Can you a newline between xen and asm headers?

> +#include <asm/device.h>
> +#include <asm/iommu_fwspec.h>

> +
> +int iommu_fwspec_init(struct device *dev, struct device *iommu_dev)
> +{
> +    struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +
> +    if ( fwspec )
> +        return 0;
> +
> +    fwspec = xzalloc(struct iommu_fwspec);
> +    if ( !fwspec )
> +        return -ENOMEM;
> +
> +    fwspec->iommu_dev = iommu_dev;
> +    dev_iommu_fwspec_set(dev, fwspec);
> +
> +    return 0;
> +}
> +
> +void iommu_fwspec_free(struct device *dev)
> +{
> +    struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +
> +    if ( fwspec )

xfree is able to deal with NULL pointer, so the check is not necessary.

> +    {
> +        xfree(fwspec);
> +        dev_iommu_fwspec_set(dev, NULL);
> +    }
> +}
> +
> +int iommu_fwspec_add_ids(struct device *dev, uint32_t *ids, int num_ids)

While I realize the prototype is coming from Linux, num_ids cannot be 
negative (the code below would not work properly). So the parameter 
should be unsigned.

> +{
> +    struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +    size_t size;
> +    int i;

Any variable that can't be negative should be unsigned.

> +
> +    if ( !fwspec )
> +        return -EINVAL;
> +
> +    size = offsetof(struct iommu_fwspec, ids[fwspec->num_ids + num_ids]);
> +    if ( size > sizeof(*fwspec) )
> +    {
> +        fwspec = _xrealloc(fwspec, size, sizeof(void *));
> +        if ( !fwspec )
> +            return -ENOMEM;
> +
> +        dev_iommu_fwspec_set(dev, fwspec);
> +    }
> +
> +    for ( i = 0; i < num_ids; i++ )
> +        fwspec->ids[fwspec->num_ids + i] = ids[i];
> +
> +    fwspec->num_ids += num_ids;
> +
> +    return 0;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
> index ee1c3bc..ee7cff2 100644
> --- a/xen/include/asm-arm/device.h
> +++ b/xen/include/asm-arm/device.h
> @@ -18,6 +18,7 @@ struct device
>       struct dt_device_node *of_node; /* Used by drivers imported from Linux */
>   #endif
>       struct dev_archdata archdata;
> +    struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU instance data */
>   };
>   
>   typedef struct device device_t;
> diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
> index 20d865e..1853bd9 100644
> --- a/xen/include/asm-arm/iommu.h
> +++ b/xen/include/asm-arm/iommu.h
> @@ -14,6 +14,8 @@
>   #ifndef __ARCH_ARM_IOMMU_H__
>   #define __ARCH_ARM_IOMMU_H__
>   
> +#include <asm/iommu_fwspec.h>

iommu.h does not seem to use anything defined in iommu_fwspec.h. So why 
do you include it here?

> +
>   struct arch_iommu
>   {
>       /* Private information for the IOMMU drivers */
> diff --git a/xen/include/asm-arm/iommu_fwspec.h b/xen/include/asm-arm/iommu_fwspec.h
> new file mode 100644
> index 0000000..0676285
> --- /dev/null
> +++ b/xen/include/asm-arm/iommu_fwspec.h
> @@ -0,0 +1,65 @@
> +/*
> + * xen/include/asm-arm/iommu_fwspec.h
> + *
> + * Contains a common structure to hold the per-device firmware data and
> + * declaration of functions used to maintain that data
> + *
> + * Based on Linux's iommu_fwspec support you can find at:
> + *    include/linux/iommu.h
> + *
> + * Copyright (C) 2019 EPAM Systems Inc.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms and conditions of the GNU General Public
> + * License, version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __ARCH_ARM_IOMMU_FWSPEC_H__
> +#define __ARCH_ARM_IOMMU_FWSPEC_H__
> +
> +/* per-device IOMMU instance data */
> +struct iommu_fwspec {
> +    /* device which represents this IOMMU H/W */

Did you intend to say "this device's IOMMU"?

> +    struct device *iommu_dev;
> +    /* IOMMU driver private data for this device */
> +    void *iommu_priv;
> +    /* number of associated device IDs */
> +    unsigned int num_ids;
> +    /* IDs which this device may present to the IOMMU */
> +    uint32_t ids[1];
> +};
> +
> +int iommu_fwspec_init(struct device *dev, struct device *iommu_dev);
> +void iommu_fwspec_free(struct device *dev);
> +int iommu_fwspec_add_ids(struct device *dev, uint32_t *ids, int num_ids);
> +
> +static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
> +{
> +    return dev->iommu_fwspec;
> +}
> +
> +static inline void dev_iommu_fwspec_set(struct device *dev,
> +                                        struct iommu_fwspec *fwspec)
> +{
> +    dev->iommu_fwspec = fwspec;
> +}
> +
> +#endif /* __ARCH_ARM_IOMMU_FWSPEC_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> 

Cheers,
Julien Grall Aug. 13, 2019, 1:40 p.m. UTC | #2
Hi Oleksandr,

One more comment :).

On 8/2/19 5:39 PM, Oleksandr Tyshchenko wrote:
> +int iommu_fwspec_init(struct device *dev, struct device *iommu_dev)
> +{
> +    struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +
> +    if ( fwspec )

I would actually check the iommu_dev passed in parameter is the same as 
stored. We expect all device to be protected by only one IOMMU. So 
better to be safe than allow overriding ;).

Cheers,
Oleksandr Tyshchenko Aug. 13, 2019, 3:17 p.m. UTC | #3
On 13.08.19 15:39, Julien Grall wrote:
> Hi Oleksandr,

Hi Julien.


>
> On 8/2/19 5:39 PM, Oleksandr Tyshchenko wrote:
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>
>> We need to have some abstract way to add new device to the IOMMU
>> based on the generic IOMMU DT binding [1] which can be used for
>> both DT (right now) and ACPI (in future).
>>
>> For that reason we can borrow the idea used in Linux these days
>> called "iommu_fwspec". Having this in, it will be possible
>> to configure IOMMU master interfaces of the device (device IDs)
>> from a single common place and avoid keeping almost identifical look-up
>
> s/identifical/identical/

ok


>
>> implementations in each IOMMU driver.
>>
>> There is no need to port the whole implementation of "iommu_fwspec"
>> to Xen, we could, probably, end up with a much simpler solution,
>> some "stripped down" version which fits our requirments.
>
> s/requirments/requirements/

ok


>
>> + */
>> +
>> +#include <xen/lib.h>
>> +#include <xen/iommu.h>
>
> Please order the headers alphabetically.
>
> NIT: Can you a newline between xen and asm headers?

Will do


>
>> +#include <asm/device.h>
>> +#include <asm/iommu_fwspec.h>
>
>> +
>> +int iommu_fwspec_init(struct device *dev, struct device *iommu_dev)
>> +{
>> +    struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>> +
>> +    if ( fwspec )
>> +        return 0;
>> +
>> +    fwspec = xzalloc(struct iommu_fwspec);
>> +    if ( !fwspec )
>> +        return -ENOMEM;
>> +
>> +    fwspec->iommu_dev = iommu_dev;
>> +    dev_iommu_fwspec_set(dev, fwspec);
>> +
>> +    return 0;
>> +}
>> +
>> +void iommu_fwspec_free(struct device *dev)
>> +{
>> +    struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>> +
>> +    if ( fwspec )
>
> xfree is able to deal with NULL pointer, so the check is not necessary.

Yes, the reason I left this check is to not perform an extra operation 
(dev_iommu_fwspec_set). Shall I drop this check anyway?

>
>> +    {
>> +        xfree(fwspec);
>> +        dev_iommu_fwspec_set(dev, NULL);
>> +    }
>> +}
>> +
>> +int iommu_fwspec_add_ids(struct device *dev, uint32_t *ids, int 
>> num_ids)
>
> While I realize the prototype is coming from Linux, num_ids cannot be 
> negative (the code below would not work properly). So the parameter 
> should be unsigned.

Agree, will use unsigned.


>
>> +{
>> +    struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>> +    size_t size;
>> +    int i;
>
> Any variable that can't be negative should be unsigned.

Yes, will follow.


>> diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
>> index 20d865e..1853bd9 100644
>> --- a/xen/include/asm-arm/iommu.h
>> +++ b/xen/include/asm-arm/iommu.h
>> @@ -14,6 +14,8 @@
>>   #ifndef __ARCH_ARM_IOMMU_H__
>>   #define __ARCH_ARM_IOMMU_H__
>>   +#include <asm/iommu_fwspec.h>
>
> iommu.h does not seem to use anything defined in iommu_fwspec.h. So 
> why do you include it here?

I was thinking that every source which includes iommu.h will get 
iommu_fwspec.h included indirectly. No need to include iommu_fwspec.h in 
many sources.

This was a reason. Shall I included it directly where needed?


>
>> +
>>   struct arch_iommu
>>   {
>>       /* Private information for the IOMMU drivers */
>> diff --git a/xen/include/asm-arm/iommu_fwspec.h 
>> b/xen/include/asm-arm/iommu_fwspec.h
>> new file mode 100644
>> index 0000000..0676285
>> --- /dev/null
>> +++ b/xen/include/asm-arm/iommu_fwspec.h
>> @@ -0,0 +1,65 @@
>> +/*
>> + * xen/include/asm-arm/iommu_fwspec.h
>> + *
>> + * Contains a common structure to hold the per-device firmware data and
>> + * declaration of functions used to maintain that data
>> + *
>> + * Based on Linux's iommu_fwspec support you can find at:
>> + *    include/linux/iommu.h
>> + *
>> + * Copyright (C) 2019 EPAM Systems Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms and conditions of the GNU General Public
>> + * License, version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public
>> + * License along with this program; If not, see 
>> <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef __ARCH_ARM_IOMMU_FWSPEC_H__
>> +#define __ARCH_ARM_IOMMU_FWSPEC_H__
>> +
>> +/* per-device IOMMU instance data */
>> +struct iommu_fwspec {
>> +    /* device which represents this IOMMU H/W */
>
> Did you intend to say "this device's IOMMU"?

Exactly)
Julien Grall Aug. 13, 2019, 3:28 p.m. UTC | #4
Hi,

On 8/13/19 4:17 PM, Oleksandr wrote:
> 
> On 13.08.19 15:39, Julien Grall wrote:
>>
>> xfree is able to deal with NULL pointer, so the check is not necessary.
> 
> Yes, the reason I left this check is to not perform an extra operation 
> (dev_iommu_fwspec_set). Shall I drop this check anyway?

I can't see any issue to do the extra operation. This is not hotpath and 
it is harmless.


>>> diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
>>> index 20d865e..1853bd9 100644
>>> --- a/xen/include/asm-arm/iommu.h
>>> +++ b/xen/include/asm-arm/iommu.h
>>> @@ -14,6 +14,8 @@
>>>   #ifndef __ARCH_ARM_IOMMU_H__
>>>   #define __ARCH_ARM_IOMMU_H__
>>>   +#include <asm/iommu_fwspec.h>
>>
>> iommu.h does not seem to use anything defined in iommu_fwspec.h. So 
>> why do you include it here?
> 
> I was thinking that every source which includes iommu.h will get 
> iommu_fwspec.h included indirectly. No need to include iommu_fwspec.h in 
> many sources.
> 
> This was a reason. Shall I included it directly where needed?

There are a few cases where iommu.h is required but not iommu_fwspec.h. 
In general, I would prefer if headers are only included when strictly 
necessary.

Cheers,
Oleksandr Tyshchenko Aug. 13, 2019, 4:18 p.m. UTC | #5
On 13.08.19 18:28, Julien Grall wrote:
> Hi,

Hi Julien


>
> On 8/13/19 4:17 PM, Oleksandr wrote:
>>
>> On 13.08.19 15:39, Julien Grall wrote:
>>>
>>> xfree is able to deal with NULL pointer, so the check is not necessary.
>>
>> Yes, the reason I left this check is to not perform an extra 
>> operation (dev_iommu_fwspec_set). Shall I drop this check anyway?
>
> I can't see any issue to do the extra operation. This is not hotpath 
> and it is harmless.

ok, will drop.


>
>
>>>> diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
>>>> index 20d865e..1853bd9 100644
>>>> --- a/xen/include/asm-arm/iommu.h
>>>> +++ b/xen/include/asm-arm/iommu.h
>>>> @@ -14,6 +14,8 @@
>>>>   #ifndef __ARCH_ARM_IOMMU_H__
>>>>   #define __ARCH_ARM_IOMMU_H__
>>>>   +#include <asm/iommu_fwspec.h>
>>>
>>> iommu.h does not seem to use anything defined in iommu_fwspec.h. So 
>>> why do you include it here?
>>
>> I was thinking that every source which includes iommu.h will get 
>> iommu_fwspec.h included indirectly. No need to include iommu_fwspec.h 
>> in many sources.
>>
>> This was a reason. Shall I included it directly where needed?
>
> There are a few cases where iommu.h is required but not 
> iommu_fwspec.h. In general, I would prefer if headers are only 
> included when strictly necessary.

got it, will drop from here and include where necessary.
Oleksandr Tyshchenko Aug. 13, 2019, 4:28 p.m. UTC | #6
On 13.08.19 16:40, Julien Grall wrote:
> Hi Oleksandr,

Hi Julien.


>
> One more comment :).
>
> On 8/2/19 5:39 PM, Oleksandr Tyshchenko wrote:
>> +int iommu_fwspec_init(struct device *dev, struct device *iommu_dev)
>> +{
>> +    struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>> +
>> +    if ( fwspec )
>
> I would actually check the iommu_dev passed in parameter is the same 
> as stored. We expect all device to be protected by only one IOMMU. So 
> better to be safe than allow overriding ;).

Actually, it makes sense, will add appropriate check.
diff mbox series

Patch

diff --git a/xen/drivers/passthrough/arm/Makefile b/xen/drivers/passthrough/arm/Makefile
index 4abb87a..5fbad45 100644
--- a/xen/drivers/passthrough/arm/Makefile
+++ b/xen/drivers/passthrough/arm/Makefile
@@ -1,2 +1,2 @@ 
-obj-y += iommu.o iommu_helpers.o
+obj-y += iommu.o iommu_helpers.o iommu_fwspec.o
 obj-$(CONFIG_ARM_SMMU) += smmu.o
diff --git a/xen/drivers/passthrough/arm/iommu_fwspec.c b/xen/drivers/passthrough/arm/iommu_fwspec.c
new file mode 100644
index 0000000..3474192
--- /dev/null
+++ b/xen/drivers/passthrough/arm/iommu_fwspec.c
@@ -0,0 +1,91 @@ 
+/*
+ * xen/drivers/passthrough/arm/iommu_fwspec.c
+ *
+ * Contains functions to maintain per-device firmware data
+ *
+ * Based on Linux's iommu_fwspec support you can find at:
+ *    drivers/iommu/iommu.c
+ *
+ * Copyright (C) 2019 EPAM Systems Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms and conditions of the GNU General Public
+ * License, version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/lib.h>
+#include <xen/iommu.h>
+#include <asm/device.h>
+#include <asm/iommu_fwspec.h>
+
+int iommu_fwspec_init(struct device *dev, struct device *iommu_dev)
+{
+    struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+
+    if ( fwspec )
+        return 0;
+
+    fwspec = xzalloc(struct iommu_fwspec);
+    if ( !fwspec )
+        return -ENOMEM;
+
+    fwspec->iommu_dev = iommu_dev;
+    dev_iommu_fwspec_set(dev, fwspec);
+
+    return 0;
+}
+
+void iommu_fwspec_free(struct device *dev)
+{
+    struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+
+    if ( fwspec )
+    {
+        xfree(fwspec);
+        dev_iommu_fwspec_set(dev, NULL);
+    }
+}
+
+int iommu_fwspec_add_ids(struct device *dev, uint32_t *ids, int num_ids)
+{
+    struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+    size_t size;
+    int i;
+
+    if ( !fwspec )
+        return -EINVAL;
+
+    size = offsetof(struct iommu_fwspec, ids[fwspec->num_ids + num_ids]);
+    if ( size > sizeof(*fwspec) )
+    {
+        fwspec = _xrealloc(fwspec, size, sizeof(void *));
+        if ( !fwspec )
+            return -ENOMEM;
+
+        dev_iommu_fwspec_set(dev, fwspec);
+    }
+
+    for ( i = 0; i < num_ids; i++ )
+        fwspec->ids[fwspec->num_ids + i] = ids[i];
+
+    fwspec->num_ids += num_ids;
+
+    return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
index ee1c3bc..ee7cff2 100644
--- a/xen/include/asm-arm/device.h
+++ b/xen/include/asm-arm/device.h
@@ -18,6 +18,7 @@  struct device
     struct dt_device_node *of_node; /* Used by drivers imported from Linux */
 #endif
     struct dev_archdata archdata;
+    struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU instance data */
 };
 
 typedef struct device device_t;
diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
index 20d865e..1853bd9 100644
--- a/xen/include/asm-arm/iommu.h
+++ b/xen/include/asm-arm/iommu.h
@@ -14,6 +14,8 @@ 
 #ifndef __ARCH_ARM_IOMMU_H__
 #define __ARCH_ARM_IOMMU_H__
 
+#include <asm/iommu_fwspec.h>
+
 struct arch_iommu
 {
     /* Private information for the IOMMU drivers */
diff --git a/xen/include/asm-arm/iommu_fwspec.h b/xen/include/asm-arm/iommu_fwspec.h
new file mode 100644
index 0000000..0676285
--- /dev/null
+++ b/xen/include/asm-arm/iommu_fwspec.h
@@ -0,0 +1,65 @@ 
+/*
+ * xen/include/asm-arm/iommu_fwspec.h
+ *
+ * Contains a common structure to hold the per-device firmware data and
+ * declaration of functions used to maintain that data
+ *
+ * Based on Linux's iommu_fwspec support you can find at:
+ *    include/linux/iommu.h
+ *
+ * Copyright (C) 2019 EPAM Systems Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms and conditions of the GNU General Public
+ * License, version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __ARCH_ARM_IOMMU_FWSPEC_H__
+#define __ARCH_ARM_IOMMU_FWSPEC_H__
+
+/* per-device IOMMU instance data */
+struct iommu_fwspec {
+    /* device which represents this IOMMU H/W */
+    struct device *iommu_dev;
+    /* IOMMU driver private data for this device */
+    void *iommu_priv;
+    /* number of associated device IDs */
+    unsigned int num_ids;
+    /* IDs which this device may present to the IOMMU */
+    uint32_t ids[1];
+};
+
+int iommu_fwspec_init(struct device *dev, struct device *iommu_dev);
+void iommu_fwspec_free(struct device *dev);
+int iommu_fwspec_add_ids(struct device *dev, uint32_t *ids, int num_ids);
+
+static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
+{
+    return dev->iommu_fwspec;
+}
+
+static inline void dev_iommu_fwspec_set(struct device *dev,
+                                        struct iommu_fwspec *fwspec)
+{
+    dev->iommu_fwspec = fwspec;
+}
+
+#endif /* __ARCH_ARM_IOMMU_FWSPEC_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */