2015-06-05 14:18:04

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH 10/22] iommu: Introduce direct mapped region handling

Hi Joerg,

On Thu, May 28, 2015 at 05:41:33PM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> Add two new functions to the IOMMU-API to allow the IOMMU
> drivers to export the requirements for direct mapped regions
> per device.
> This is useful for exporting the information in Intel VT-d's
> RMRR entries or AMD-Vi's unity mappings.
>
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> drivers/iommu/iommu.c | 16 ++++++++++++++++
> include/linux/iommu.h | 31 +++++++++++++++++++++++++++++++
> 2 files changed, 47 insertions(+)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index a0a38bd..6b8d6e7 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1469,3 +1469,19 @@ int iommu_domain_set_attr(struct iommu_domain *domain,
> return ret;
> }
> EXPORT_SYMBOL_GPL(iommu_domain_set_attr);
> +
> +void iommu_get_dm_regions(struct device *dev, struct list_head *list)
> +{
> + const struct iommu_ops *ops = dev->bus->iommu_ops;
> +
> + if (ops && ops->get_dm_regions)
> + ops->get_dm_regions(dev, list);
> +}
> +
> +void iommu_put_dm_regions(struct device *dev, struct list_head *list)
> +{
> + const struct iommu_ops *ops = dev->bus->iommu_ops;
> +
> + if (ops && ops->put_dm_regions)
> + ops->put_dm_regions(dev, list);
> +}
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 683a1c4..6894999 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -114,6 +114,20 @@ enum iommu_attr {
> DOMAIN_ATTR_MAX,
> };
>
> +/**
> + * struct iommu_dm_region - descriptor for a direct mapped memory region
> + * @list: Linked list pointers
> + * @start: System physical start address of the region
> + * @length: Length of the region in bytes
> + * @prot: IOMMU Protection flags (READ/WRITE/...)
> + */
> +struct iommu_dm_region {
> + struct list_head list;
> + phys_addr_t start;
> + size_t length;
> + int prot;
> +};

I'm slightly puzzled about this. It looks to me like we're asking the
IOMMU driver to construct a description of the system's physical address
space, but this information tends to be known elsewhere for things like
initialising lowmem on the CPU using memblock.

Also, it looks like we just use these regions to create the default
domain using iommu_map calls -- why don't we just have an IOMMU callback
to initialise the default domain instead? That would allow IOMMUs with a
per-master bypass mode to avoid allocating page tables altogether.

Will


2015-06-05 14:32:11

by Jörg Rödel

[permalink] [raw]
Subject: Re: [PATCH 10/22] iommu: Introduce direct mapped region handling

Hi Will,

On Fri, Jun 05, 2015 at 03:17:50PM +0100, Will Deacon wrote:
> On Thu, May 28, 2015 at 05:41:33PM +0100, Joerg Roedel wrote:
> > +/**
> > + * struct iommu_dm_region - descriptor for a direct mapped memory region
> > + * @list: Linked list pointers
> > + * @start: System physical start address of the region
> > + * @length: Length of the region in bytes
> > + * @prot: IOMMU Protection flags (READ/WRITE/...)
> > + */
> > +struct iommu_dm_region {
> > + struct list_head list;
> > + phys_addr_t start;
> > + size_t length;
> > + int prot;
> > +};
>
> I'm slightly puzzled about this. It looks to me like we're asking the
> IOMMU driver to construct a description of the system's physical address
> space, but this information tends to be known elsewhere for things like
> initialising lowmem on the CPU using memblock.

Well, this is not about the general memory layout of the machine, it is
more about the requirements of the firmware. The firmware might have
their own mapping requirements, for example a USB controler that is
handled by the BIOS. Other devices (be2net adapters with special
firmware) might have such requirements too. On x86 these requirements
are described in the IOMMU ACPI tables (RMRR entries on Intel, Unity
mappings on AMD).

> Also, it looks like we just use these regions to create the default
> domain using iommu_map calls -- why don't we just have an IOMMU callback
> to initialise the default domain instead? That would allow IOMMUs with a
> per-master bypass mode to avoid allocating page tables altogether.

In theory yes, but this information is not only needed for the creation
of default domains, but also for a generic DMA-API implementation for
IOMMU drivers. A DMA-API implementation has to mark these address ranges
as reserved in its address allocator, so it is better to export this
information than doing the handling in the iommu drivers.


Joerg