Message ID | 20230210163731.970130-3-jean-philippe@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | [1/2] hw/arm/smmu-common: Support 64-bit addresses | expand |
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~
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? 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 >
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) {
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(-)