hw/arm/smmu-common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Two small changes to support TTB1. Note that I had to modify the Linux
driver in order to test this (see below), but other OSes might use TTB1.
Jean-Philippe Brucker (2):
hw/arm/smmu-common: Support 64-bit addresses
hw/arm/smmu-common: Fix TTB1 handling
hw/arm/smmu-common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- 8< --- Linux hacks:
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 8d772ea8a583..bf0ff699b64b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -276,6 +276,11 @@
#define CTXDESC_CD_0_TCR_IRGN0 GENMASK_ULL(9, 8)
#define CTXDESC_CD_0_TCR_ORGN0 GENMASK_ULL(11, 10)
#define CTXDESC_CD_0_TCR_SH0 GENMASK_ULL(13, 12)
+#define CTXDESC_CD_0_TCR_T1SZ GENMASK_ULL(21, 16)
+#define CTXDESC_CD_0_TCR_TG1 GENMASK_ULL(23, 22)
+#define CTXDESC_CD_0_TCR_IRGN1 GENMASK_ULL(25, 24)
+#define CTXDESC_CD_0_TCR_ORGN1 GENMASK_ULL(27, 26)
+#define CTXDESC_CD_0_TCR_SH1 GENMASK_ULL(29, 28)
#define CTXDESC_CD_0_TCR_EPD0 (1ULL << 14)
#define CTXDESC_CD_0_TCR_EPD1 (1ULL << 30)
@@ -293,6 +298,7 @@
#define CTXDESC_CD_0_ASID GENMASK_ULL(63, 48)
#define CTXDESC_CD_1_TTB0_MASK GENMASK_ULL(51, 4)
+#define CTXDESC_CD_2_TTB1_MASK GENMASK_ULL(51, 4)
/*
* When the SMMU only supports linear context descriptor tables, pick a
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index f2425b0f0cd6..3a4343e60a54 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1075,8 +1075,8 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
* this substream's traffic
*/
} else { /* (1) and (2) */
- cdptr[1] = cpu_to_le64(cd->ttbr & CTXDESC_CD_1_TTB0_MASK);
- cdptr[2] = 0;
+ cdptr[1] = 0;
+ cdptr[2] = cpu_to_le64(cd->ttbr & CTXDESC_CD_2_TTB1_MASK);
cdptr[3] = cpu_to_le64(cd->mair);
/*
@@ -2108,13 +2108,13 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
cfg->cd.asid = (u16)asid;
cfg->cd.ttbr = pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
- cfg->cd.tcr = FIELD_PREP(CTXDESC_CD_0_TCR_T0SZ, tcr->tsz) |
- FIELD_PREP(CTXDESC_CD_0_TCR_TG0, tcr->tg) |
- FIELD_PREP(CTXDESC_CD_0_TCR_IRGN0, tcr->irgn) |
- FIELD_PREP(CTXDESC_CD_0_TCR_ORGN0, tcr->orgn) |
- FIELD_PREP(CTXDESC_CD_0_TCR_SH0, tcr->sh) |
+ cfg->cd.tcr = FIELD_PREP(CTXDESC_CD_0_TCR_T1SZ, tcr->tsz) |
+ FIELD_PREP(CTXDESC_CD_0_TCR_TG1, tcr->tg) |
+ FIELD_PREP(CTXDESC_CD_0_TCR_IRGN1, tcr->irgn) |
+ FIELD_PREP(CTXDESC_CD_0_TCR_ORGN1, tcr->orgn) |
+ FIELD_PREP(CTXDESC_CD_0_TCR_SH1, tcr->sh) |
FIELD_PREP(CTXDESC_CD_0_TCR_IPS, tcr->ips) |
- CTXDESC_CD_0_TCR_EPD1 | CTXDESC_CD_0_AA64;
+ CTXDESC_CD_0_TCR_EPD0 | CTXDESC_CD_0_AA64;
cfg->cd.mair = pgtbl_cfg->arm_lpae_s1_cfg.mair;
/*
@@ -2212,6 +2212,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
.pgsize_bitmap = smmu->pgsize_bitmap,
.ias = ias,
.oas = oas,
+ .quirks = IO_PGTABLE_QUIRK_ARM_TTBR1,
.coherent_walk = smmu->features & ARM_SMMU_FEAT_COHERENCY,
.tlb = &arm_smmu_flush_ops,
.iommu_dev = smmu->dev,
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 38434203bf04..3fe154c9782d 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -677,6 +677,10 @@ static int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
}
}
+/* HACK */
+#define VA_SIZE 39
+#define VA_MASK (~0ULL << VA_SIZE)
+
static dma_addr_t iommu_dma_alloc_iova(struct iommu_domain *domain,
size_t size, u64 dma_limit, struct device *dev)
{
@@ -706,7 +710,7 @@ static dma_addr_t iommu_dma_alloc_iova(struct iommu_domain *domain,
iova = alloc_iova_fast(iovad, iova_len, dma_limit >> shift,
true);
- return (dma_addr_t)iova << shift;
+ return (dma_addr_t)iova << shift | VA_MASK;
}
static void iommu_dma_free_iova(struct iommu_dma_cookie *cookie,
@@ -714,6 +718,7 @@ static void iommu_dma_free_iova(struct iommu_dma_cookie *cookie,
{
struct iova_domain *iovad = &cookie->iovad;
+ iova &= ~VA_MASK;
/* The MSI case is only ever cleaning up its most recent allocation */
if (cookie->type == IOMMU_DMA_MSI_COOKIE)
cookie->msi_iova -= size;Addresses targeting the second translation table (TTB1) in the SMMU have
all upper bits set. Ensure the IOMMU region covers all 64 bits.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
hw/arm/smmu-common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 733c964778..2b8c67b9a1 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -439,7 +439,7 @@ static AddressSpace *smmu_find_add_as(PCIBus *bus, void *opaque, int devfn)
memory_region_init_iommu(&sdev->iommu, sizeof(sdev->iommu),
s->mrtypename,
- OBJECT(s), name, 1ULL << SMMU_MAX_VA_BITS);
+ OBJECT(s), name, UINT64_MAX);
address_space_init(&sdev->as,
MEMORY_REGION(&sdev->iommu), name);
trace_smmu_add_mr(name);
--
2.39.0Hi Jean, On 2/10/23 17:37, Jean-Philippe Brucker wrote: > Addresses targeting the second translation table (TTB1) in the SMMU have > all upper bits set. Ensure the IOMMU region covers all 64 bits. > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > --- > hw/arm/smmu-common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c > index 733c964778..2b8c67b9a1 100644 > --- a/hw/arm/smmu-common.c > +++ b/hw/arm/smmu-common.c > @@ -439,7 +439,7 @@ static AddressSpace *smmu_find_add_as(PCIBus *bus, void *opaque, int devfn) > > memory_region_init_iommu(&sdev->iommu, sizeof(sdev->iommu), > s->mrtypename, > - OBJECT(s), name, 1ULL << SMMU_MAX_VA_BITS); > + OBJECT(s), name, UINT64_MAX); as SMMU_MAX_VA_BITS is not used anymore, please remove it. > address_space_init(&sdev->as, > MEMORY_REGION(&sdev->iommu), name); > trace_smmu_add_mr(name); Thanks Eric
On 2/10/23 06:37, Jean-Philippe Brucker wrote: > Addresses targeting the second translation table (TTB1) in the SMMU have > all upper bits set. Ensure the IOMMU region covers all 64 bits. > > Signed-off-by: Jean-Philippe Brucker<jean-philippe@linaro.org> > --- > hw/arm/smmu-common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
Addresses targeting the second translation table (TTB1) in the SMMU have
all upper bits set (except for the top byte when TBI is enabled). Fix
the TTB1 check.
Reported-by: Ola Hugosson <ola.hugosson@arm.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
hw/arm/smmu-common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 2b8c67b9a1..0a5a60ca1e 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -249,7 +249,7 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova)
/* there is a ttbr0 region and we are in it (high bits all zero) */
return &cfg->tt[0];
} else if (cfg->tt[1].tsz &&
- !extract64(iova, 64 - cfg->tt[1].tsz, cfg->tt[1].tsz - tbi_byte)) {
+ sextract64(iova, 64 - cfg->tt[1].tsz, cfg->tt[1].tsz - tbi_byte) == -1) {
/* there is a ttbr1 region and we are in it (high bits all one) */
return &cfg->tt[1];
} else if (!cfg->tt[0].tsz) {
--
2.39.0Hi Jean,
On 2/10/23 17:37, Jean-Philippe Brucker wrote:
> Addresses targeting the second translation table (TTB1) in the SMMU have
> all upper bits set (except for the top byte when TBI is enabled). Fix
> the TTB1 check.
>
> Reported-by: Ola Hugosson <ola.hugosson@arm.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> hw/arm/smmu-common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 2b8c67b9a1..0a5a60ca1e 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -249,7 +249,7 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova)
> /* there is a ttbr0 region and we are in it (high bits all zero) */
> return &cfg->tt[0];
> } else if (cfg->tt[1].tsz &&
> - !extract64(iova, 64 - cfg->tt[1].tsz, cfg->tt[1].tsz - tbi_byte)) {
> + sextract64(iova, 64 - cfg->tt[1].tsz, cfg->tt[1].tsz - tbi_byte) == -1) {
> /* there is a ttbr1 region and we are in it (high bits all one) */
> return &cfg->tt[1];
> } else if (!cfg->tt[0].tsz) {
Reviewed-by: Eric Auger <eric.auger@redhat.com>
While reading the spec again, I noticed we do not support VAX. Is it
something that we would need to support?
Thanks!
Eric
On Mon, Feb 13, 2023 at 05:30:03PM +0100, Eric Auger wrote:
> Hi Jean,
>
> On 2/10/23 17:37, Jean-Philippe Brucker wrote:
> > Addresses targeting the second translation table (TTB1) in the SMMU have
> > all upper bits set (except for the top byte when TBI is enabled). Fix
> > the TTB1 check.
> >
> > Reported-by: Ola Hugosson <ola.hugosson@arm.com>
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > ---
> > hw/arm/smmu-common.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> > index 2b8c67b9a1..0a5a60ca1e 100644
> > --- a/hw/arm/smmu-common.c
> > +++ b/hw/arm/smmu-common.c
> > @@ -249,7 +249,7 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova)
> > /* there is a ttbr0 region and we are in it (high bits all zero) */
> > return &cfg->tt[0];
> > } else if (cfg->tt[1].tsz &&
> > - !extract64(iova, 64 - cfg->tt[1].tsz, cfg->tt[1].tsz - tbi_byte)) {
> > + sextract64(iova, 64 - cfg->tt[1].tsz, cfg->tt[1].tsz - tbi_byte) == -1) {
> > /* there is a ttbr1 region and we are in it (high bits all one) */
> > return &cfg->tt[1];
> > } else if (!cfg->tt[0].tsz) {
>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>
> While reading the spec again, I noticed we do not support VAX. Is it
> something that we would need to support?
I guess it would be needed to support sharing page tables with the CPU, if
the CPU supports and the OS uses FEAT_LVA. But in order to share the
stage-1, Linux would need more complex features as well (ATS+PRI/Stall,
PASID).
For a private DMA address space, I think 48 bits of VA is already plenty.
Thanks,
Jean
On 2/14/23 17:46, Jean-Philippe Brucker wrote:
> On Mon, Feb 13, 2023 at 05:30:03PM +0100, Eric Auger wrote:
>> Hi Jean,
>>
>> On 2/10/23 17:37, Jean-Philippe Brucker wrote:
>>> Addresses targeting the second translation table (TTB1) in the SMMU have
>>> all upper bits set (except for the top byte when TBI is enabled). Fix
>>> the TTB1 check.
>>>
>>> Reported-by: Ola Hugosson <ola.hugosson@arm.com>
>>> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
>>> ---
>>> hw/arm/smmu-common.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
>>> index 2b8c67b9a1..0a5a60ca1e 100644
>>> --- a/hw/arm/smmu-common.c
>>> +++ b/hw/arm/smmu-common.c
>>> @@ -249,7 +249,7 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova)
>>> /* there is a ttbr0 region and we are in it (high bits all zero) */
>>> return &cfg->tt[0];
>>> } else if (cfg->tt[1].tsz &&
>>> - !extract64(iova, 64 - cfg->tt[1].tsz, cfg->tt[1].tsz - tbi_byte)) {
>>> + sextract64(iova, 64 - cfg->tt[1].tsz, cfg->tt[1].tsz - tbi_byte) == -1) {
>>> /* there is a ttbr1 region and we are in it (high bits all one) */
>>> return &cfg->tt[1];
>>> } else if (!cfg->tt[0].tsz) {
>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>>
>> While reading the spec again, I noticed we do not support VAX. Is it
>> something that we would need to support?
> I guess it would be needed to support sharing page tables with the CPU, if
> the CPU supports and the OS uses FEAT_LVA. But in order to share the
> stage-1, Linux would need more complex features as well (ATS+PRI/Stall,
> PASID).
>
> For a private DMA address space, I think 48 bits of VA is already plenty.
OK thanks!
Eric
>
> Thanks,
> Jean
>
On 2/10/23 06:37, Jean-Philippe Brucker wrote: > Addresses targeting the second translation table (TTB1) in the SMMU have > all upper bits set (except for the top byte when TBI is enabled). Fix > the TTB1 check. > > Reported-by: Ola Hugosson<ola.hugosson@arm.com> > Signed-off-by: Jean-Philippe Brucker<jean-philippe@linaro.org> > --- > hw/arm/smmu-common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
© 2016 - 2024 Red Hat, Inc.