Re: [RFC v2 12/20] dma-iommu: Implement NESTED_MSI cookie
On 2018-10-24 7:44 pm, Auger Eric wrote: Hi Robin, On 10/24/18 8:02 PM, Robin Murphy wrote: Hi Eric, On 2018-09-18 3:24 pm, Eric Auger wrote: Up to now, when the type was UNMANAGED, we used to allocate IOVA pages within a range provided by the user. This does not work in nested mode. If both the host and the guest are exposed with SMMUs, each would allocate an IOVA. The guest allocates an IOVA (gIOVA) to map onto the guest MSI doorbell (gDB). The Host allocates another IOVA (hIOVA) to map onto the physical doorbell (hDB). So we end up with 2 unrelated mappings, at S1 and S2: S1 S2 gIOVA -> gDB hIOVA -> hDB The PCI device would be programmed with hIOVA. iommu_dma_bind_doorbell allows to pass gIOVA/gDB to the host so that gIOVA can be used by the host instead of re-allocating a new IOVA. That way the host can create the following nested mapping: S1 S2 gIOVA -> gDB -> hDB this time, the PCI device will be programmed with the gIOVA MSI doorbell which is correctly map through the 2 stages. If I'm understanding things correctly, this plus a couple of the preceding patches all add up to a rather involved way of coercing an automatic allocator to only "allocate" predetermined addresses in an entirely known-ahead-of-time manner. agreed Given that the guy calling iommu_dma_bind_doorbell() could seemingly just as easily call iommu_map() at that point and not bother with an allocator cookie and all this machinery at all, what am I missing? Well iommu_dma_map_msi_msg() gets called and is part of this existing MSI mapping machinery. If we do not do anything this function allocates an hIOVA that is not involved in any nested setup. So either we coerce the allocator in place (which is what this series does) or we unplug the allocator to replace this latter with a simple S2 mapping, as you suggest, ie. iommu_map(gDB, hDB). Assuming we unplug the allocator, the guy who actually calls iommu_dma_bind_doorbell() knows gDB but does not know hDB. So I don't really get how we can simplify things. OK, there's what I was missing :D But that then seems to reveal a somewhat bigger problem - if the callers are simply registering IPAs, and relying on the ITS driver to grab an entry and fill in a PA later, then how does either one know *which* PA is supposed to belong to a given IPA in the case where you have multiple devices with different ITS targets assigned to the same guest? (and if it's possible to assume a guest will use per-device stage 1 mappings and present it with a single vITS backed by multiple pITSes, I think things start breaking even harder.) Other than allowing arbitrary disjoint IOVA pages, I'm not sure this really works any differently from the existing MSI cookie now that I look more closely :/ Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v2 12/20] dma-iommu: Implement NESTED_MSI cookie
Hi Robin,
On 10/24/18 8:02 PM, Robin Murphy wrote:
> Hi Eric,
>
> On 2018-09-18 3:24 pm, Eric Auger wrote:
>> Up to now, when the type was UNMANAGED, we used to
>> allocate IOVA pages within a range provided by the user.
>> This does not work in nested mode.
>>
>> If both the host and the guest are exposed with SMMUs, each
>> would allocate an IOVA. The guest allocates an IOVA (gIOVA)
>> to map onto the guest MSI doorbell (gDB). The Host allocates
>> another IOVA (hIOVA) to map onto the physical doorbell (hDB).
>>
>> So we end up with 2 unrelated mappings, at S1 and S2:
>> S1 S2
>> gIOVA -> gDB
>> hIOVA -> hDB
>>
>> The PCI device would be programmed with hIOVA.
>>
>> iommu_dma_bind_doorbell allows to pass gIOVA/gDB to the host
>> so that gIOVA can be used by the host instead of re-allocating
>> a new IOVA. That way the host can create the following nested
>> mapping:
>>
>> S1 S2
>> gIOVA -> gDB -> hDB
>>
>> this time, the PCI device will be programmed with the gIOVA MSI
>> doorbell which is correctly map through the 2 stages.
>
> If I'm understanding things correctly, this plus a couple of the
> preceding patches all add up to a rather involved way of coercing an
> automatic allocator to only "allocate" predetermined addresses in an
> entirely known-ahead-of-time manner.
agreed
Given that the guy calling
> iommu_dma_bind_doorbell() could seemingly just as easily call
> iommu_map() at that point and not bother with an allocator cookie and
> all this machinery at all, what am I missing?
Well iommu_dma_map_msi_msg() gets called and is part of this existing
MSI mapping machinery. If we do not do anything this function allocates
an hIOVA that is not involved in any nested setup. So either we coerce
the allocator in place (which is what this series does) or we unplug the
allocator to replace this latter with a simple S2 mapping, as you
suggest, ie. iommu_map(gDB, hDB). Assuming we unplug the allocator, the
guy who actually calls iommu_dma_bind_doorbell() knows gDB but does not
know hDB. So I don't really get how we can simplify things.
Thanks
Eric
>
> Robin.
>
>>
>> Signed-off-by: Eric Auger
>>
>> ---
>>
>> v1 -> v2:
>> - unmap stage2 on put()
>> ---
>> drivers/iommu/dma-iommu.c | 97 +--
>> include/linux/dma-iommu.h | 11 +
>> 2 files changed, 105 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index 511ff9a1d6d9..53444c3e8f2f 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -37,12 +37,14 @@
>> struct iommu_dma_msi_page {
>> struct list_head list;
>> dma_addr_t iova;
>> + dma_addr_t ipa;
>> phys_addr_t phys;
>> };
>> enum iommu_dma_cookie_type {
>> IOMMU_DMA_IOVA_COOKIE,
>> IOMMU_DMA_MSI_COOKIE,
>> + IOMMU_DMA_NESTED_MSI_COOKIE,
>> };
>> struct iommu_dma_cookie {
>> @@ -109,14 +111,17 @@ EXPORT_SYMBOL(iommu_get_dma_cookie);
>> *
>> * Users who manage their own IOVA allocation and do not want DMA
>> API support,
>> * but would still like to take advantage of automatic MSI
>> remapping, can use
>> - * this to initialise their own domain appropriately. Users should
>> reserve a
>> + * this to initialise their own domain appropriately. Users may
>> reserve a
>> * contiguous IOVA region, starting at @base, large enough to
>> accommodate the
>> * number of PAGE_SIZE mappings necessary to cover every MSI
>> doorbell address
>> - * used by the devices attached to @domain.
>> + * used by the devices attached to @domain. The other way round is to
>> provide
>> + * usable iova pages through the iommu_dma_bind_doorbell API (nested
>> stages
>> + * use case)
>> */
>> int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
>> {
>> struct iommu_dma_cookie *cookie;
>> + int nesting, ret;
>> if (domain->type != IOMMU_DOMAIN_UNMANAGED)
>> return -EINVAL;
>> @@ -124,7 +129,12 @@ int iommu_get_msi_cookie(struct iommu_domain
>> *domain, dma_addr_t base)
>> if (domain->iova_cookie)
>> return -EEXIST;
>> - cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
>> + ret = iommu_domain_get_attr(domain, DOMAIN_ATTR_NESTING, &nesting);
>> + if (!ret && nesting)
>> + cookie = cookie_alloc(IOMMU_DMA_NESTED_MSI_COOKIE);
>> + else
>> + cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
>> +
>> if (!cookie)
>> return -ENOMEM;
>> @@ -145,6 +155,7 @@ void iommu_put_dma_cookie(struct iommu_domain
>> *domain)
>> {
>> struct iommu_dma_cookie *cookie = domain->iova_cookie;
>> struct iommu_dma_msi_page *msi, *tmp;
>> + bool s2_unmap = false;
>> if (!cookie)
>> return;
>> @@ -152,7 +163,15 @@ void iommu_put_dma_cookie(struct iommu_domain
>> *domain)
>> if (cookie->type == IOMMU_DMA_IOVA_COOKIE && cooki
Re: [RFC v2 12/20] dma-iommu: Implement NESTED_MSI cookie
Hi Eric,
On 2018-09-18 3:24 pm, Eric Auger wrote:
Up to now, when the type was UNMANAGED, we used to
allocate IOVA pages within a range provided by the user.
This does not work in nested mode.
If both the host and the guest are exposed with SMMUs, each
would allocate an IOVA. The guest allocates an IOVA (gIOVA)
to map onto the guest MSI doorbell (gDB). The Host allocates
another IOVA (hIOVA) to map onto the physical doorbell (hDB).
So we end up with 2 unrelated mappings, at S1 and S2:
S1 S2
gIOVA-> gDB
hIOVA->hDB
The PCI device would be programmed with hIOVA.
iommu_dma_bind_doorbell allows to pass gIOVA/gDB to the host
so that gIOVA can be used by the host instead of re-allocating
a new IOVA. That way the host can create the following nested
mapping:
S1 S2
gIOVA->gDB->hDB
this time, the PCI device will be programmed with the gIOVA MSI
doorbell which is correctly map through the 2 stages.
If I'm understanding things correctly, this plus a couple of the
preceding patches all add up to a rather involved way of coercing an
automatic allocator to only "allocate" predetermined addresses in an
entirely known-ahead-of-time manner. Given that the guy calling
iommu_dma_bind_doorbell() could seemingly just as easily call
iommu_map() at that point and not bother with an allocator cookie and
all this machinery at all, what am I missing?
Robin.
Signed-off-by: Eric Auger
---
v1 -> v2:
- unmap stage2 on put()
---
drivers/iommu/dma-iommu.c | 97 +--
include/linux/dma-iommu.h | 11 +
2 files changed, 105 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 511ff9a1d6d9..53444c3e8f2f 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -37,12 +37,14 @@
struct iommu_dma_msi_page {
struct list_headlist;
dma_addr_t iova;
+ dma_addr_t ipa;
phys_addr_t phys;
};
enum iommu_dma_cookie_type {
IOMMU_DMA_IOVA_COOKIE,
IOMMU_DMA_MSI_COOKIE,
+ IOMMU_DMA_NESTED_MSI_COOKIE,
};
struct iommu_dma_cookie {
@@ -109,14 +111,17 @@ EXPORT_SYMBOL(iommu_get_dma_cookie);
*
* Users who manage their own IOVA allocation and do not want DMA API support,
* but would still like to take advantage of automatic MSI remapping, can use
- * this to initialise their own domain appropriately. Users should reserve a
+ * this to initialise their own domain appropriately. Users may reserve a
* contiguous IOVA region, starting at @base, large enough to accommodate the
* number of PAGE_SIZE mappings necessary to cover every MSI doorbell address
- * used by the devices attached to @domain.
+ * used by the devices attached to @domain. The other way round is to provide
+ * usable iova pages through the iommu_dma_bind_doorbell API (nested stages
+ * use case)
*/
int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
{
struct iommu_dma_cookie *cookie;
+ int nesting, ret;
if (domain->type != IOMMU_DOMAIN_UNMANAGED)
return -EINVAL;
@@ -124,7 +129,12 @@ int iommu_get_msi_cookie(struct iommu_domain *domain,
dma_addr_t base)
if (domain->iova_cookie)
return -EEXIST;
- cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
+ ret = iommu_domain_get_attr(domain, DOMAIN_ATTR_NESTING, &nesting);
+ if (!ret && nesting)
+ cookie = cookie_alloc(IOMMU_DMA_NESTED_MSI_COOKIE);
+ else
+ cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
+
if (!cookie)
return -ENOMEM;
@@ -145,6 +155,7 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
{
struct iommu_dma_cookie *cookie = domain->iova_cookie;
struct iommu_dma_msi_page *msi, *tmp;
+ bool s2_unmap = false;
if (!cookie)
return;
@@ -152,7 +163,15 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
if (cookie->type == IOMMU_DMA_IOVA_COOKIE && cookie->iovad.granule)
put_iova_domain(&cookie->iovad);
+ if (cookie->type == IOMMU_DMA_NESTED_MSI_COOKIE)
+ s2_unmap = true;
+
list_for_each_entry_safe(msi, tmp, &cookie->msi_page_list, list) {
+ if (s2_unmap && msi->phys) {
+ size_t size = cookie_msi_granule(cookie);
+
+ WARN_ON(iommu_unmap(domain, msi->ipa, size) != size);
+ }
list_del(&msi->list);
kfree(msi);
}
@@ -161,6 +180,50 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
}
EXPORT_SYMBOL(iommu_put_dma_cookie);
+/**
+ * iommu_dma_bind_doorbell - Allows to provide a usable IOVA page
+ * @domain: domain handle
+ * @binding: IOVA/IPA binding
+ *
+ * In nested stage use case, the user can provide IOVA/IPA bindings
+ * corr
Re: [PATCH 1/1] iommu/arm-smmu: Add support to use Last level cache
Hi Tomasz,
On Tue, Oct 23, 2018 at 9:45 AM Tomasz Figa wrote:
>
> Hi Vivek,
>
> On Fri, Jun 15, 2018 at 7:53 PM Vivek Gautam
> wrote:
> >
> > Qualcomm SoCs have an additional level of cache called as
> > System cache or Last level cache[1]. This cache sits right
> > before the DDR, and is tightly coupled with the memory
> > controller.
> > The cache is available to all the clients present in the
> > SoC system. The clients request their slices from this system
> > cache, make it active, and can then start using it. For these
> > clients with smmu, to start using the system cache for
> > dma buffers and related page tables [2], few of the memory
> > attributes need to be set accordingly.
> > This change makes the related memory Outer-Shareable, and
> > updates the MAIR with necessary protection.
> >
> > The MAIR attribute requirements are:
> > Inner Cacheablity = 0
> > Outer Cacheablity = 1, Write-Back Write Allocate
> > Outer Shareablity = 1
> >
> > This change is a realisation of following changes
> > from downstream msm-4.9:
> > iommu: io-pgtable-arm: Support DOMAIN_ATTRIBUTE_USE_UPSTREAM_HINT
> > iommu: io-pgtable-arm: Implement IOMMU_USE_UPSTREAM_HINT
>
> Would you be able to provide links to those 2 downstream changes?
Thanks for the review.
Here are the links for the changes:
[1] -- iommu: io-pgtable-arm: Support DOMAIN_ATTRIBUTE_USE_UPSTREAM_HINT
[2] -- iommu: io-pgtable-arm: Implement IOMMU_USE_UPSTREAM_HINT
[1]
https://source.codeaurora.org/quic/la/kernel/msm-4.9/commit/?h=msm-4.9&id=bf762276796e79ca90014992f4d9da5593fa7d51
[2]
https://source.codeaurora.org/quic/la/kernel/msm-4.9/commit/?h=msm-4.9&id=d4c72c413ea27c43f60825193d4de9cb8ffd9602
>
> >
> > [1] https://patchwork.kernel.org/patch/10422531/
> > [2] https://patchwork.kernel.org/patch/10302791/
> >
> > Signed-off-by: Vivek Gautam
> > ---
> > drivers/iommu/arm-smmu.c | 14 ++
> > drivers/iommu/io-pgtable-arm.c | 24 +++-
> > drivers/iommu/io-pgtable.h | 4
> > include/linux/iommu.h | 4
> > 4 files changed, 41 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> > index f7a96bcf94a6..8058e7205034 100644
> > --- a/drivers/iommu/arm-smmu.c
> > +++ b/drivers/iommu/arm-smmu.c
> > @@ -249,6 +249,7 @@ struct arm_smmu_domain {
> > struct mutexinit_mutex; /* Protects smmu
> > pointer */
> > spinlock_t cb_lock; /* Serialises ATS1* ops
> > and TLB syncs */
> > struct iommu_domain domain;
> > + boolhas_sys_cache;
> > };
> >
> > struct arm_smmu_option_prop {
> > @@ -862,6 +863,8 @@ static int arm_smmu_init_domain_context(struct
> > iommu_domain *domain,
> >
> > if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
> > pgtbl_cfg.quirks = IO_PGTABLE_QUIRK_NO_DMA;
> > + if (smmu_domain->has_sys_cache)
> > + pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_SYS_CACHE;
> >
> > smmu_domain->smmu = smmu;
> > pgtbl_ops = alloc_io_pgtable_ops(fmt, &pgtbl_cfg, smmu_domain);
> > @@ -1477,6 +1480,9 @@ static int arm_smmu_domain_get_attr(struct
> > iommu_domain *domain,
> > case DOMAIN_ATTR_NESTING:
> > *(int *)data = (smmu_domain->stage ==
> > ARM_SMMU_DOMAIN_NESTED);
> > return 0;
> > + case DOMAIN_ATTR_USE_SYS_CACHE:
> > + *((int *)data) = smmu_domain->has_sys_cache;
> > + return 0;
> > default:
> > return -ENODEV;
> > }
> > @@ -1506,6 +1512,14 @@ static int arm_smmu_domain_set_attr(struct
> > iommu_domain *domain,
> > smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
> >
> > break;
> > + case DOMAIN_ATTR_USE_SYS_CACHE:
> > + if (smmu_domain->smmu) {
> > + ret = -EPERM;
> > + goto out_unlock;
> > + }
> > + if (*((int *)data))
> > + smmu_domain->has_sys_cache = true;
> > + break;
> > default:
> > ret = -ENODEV;
> > }
> > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> > index 010a254305dd..b2aee1828524 100644
> > --- a/drivers/iommu/io-pgtable-arm.c
> > +++ b/drivers/iommu/io-pgtable-arm.c
> > @@ -169,9 +169,11 @@
> > #define ARM_LPAE_MAIR_ATTR_DEVICE 0x04
> > #define ARM_LPAE_MAIR_ATTR_NC 0x44
> > #define ARM_LPAE_MAIR_ATTR_WBRWA 0xff
> > +#define ARM_LPAE_MAIR_ATTR_SYS_CACHE 0xf4
> > #define ARM_LPAE_MAIR_ATTR_IDX_NC 0
> > #define ARM_LPAE_MAIR_ATTR_IDX_CACHE 1
> > #define ARM_LPAE_MAIR_ATTR_IDX_DEV 2
> > +#define ARM_LPAE_MAIR_ATTR_IDX_SYS_CACHE 3
> >
> > /* IOPTE accessors */
> > #define iopte_deref(pte,d) __va(iopte_to_paddr(pte, d))
> > @@ -442,6 +444,10 @@ static arm_lpae_iopte arm_lpae_prot_to_pt
Re: [PATCH 3/3] iommu/tegra194_smmu: Add Tegra194 SMMU driver
Hi Krishna, Thank you for the patch! Yet something to improve: [auto build test ERROR on iommu/next] [also build test ERROR on next-20181019] [cannot apply to v4.19] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Krishna-Reddy/Add-Tegra194-Dual-ARM-SMMU-driver/20181024-123501 base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next config: arm-multi_v7_defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=arm All errors (new ones prefixed by >>): drivers/iommu/tegra194-smmu.o: In function `arm_smmu_attach_dev': >> tegra194-smmu.c:(.text+0x1800): undefined reference to `alloc_io_pgtable_ops' drivers/iommu/tegra194-smmu.o: In function `arm_smmu_domain_free': >> tegra194-smmu.c:(.text+0x1af8): undefined reference to `free_io_pgtable_ops' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
