| Message ID | 20230615201042.2291867-4-jonas@kwiboo.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iommu: rockchip: Fix discovery table address encoding | expand |
On 2023-06-15 21:10, Jonas Karlman wrote: > Newer Rockchip SoCs, RK356x and RK3588, support more then 4GB of memory. Older ones supposedly did too, e.g. commit 79db45be2b8b ("ARM: dts: rockchip: convert rk3288 device tree files to 64 bits"). Are we certain that nobody actually has a system with IOMMUv1 and more than 4GB of RAM? > However, the RK IOMMU driver is using the GFP_DMA32 flag to limit > allocation of the discovery and page tables into memory below 4GB. > > Let's remove this limitation now that the discovery table address is Nit: s/discovery/directory/g again Thanks, Robin. > correctly configured for addresses above 4GB. > > Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > --- > v2: > - no change > > drivers/iommu/rockchip-iommu.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c > index 62be9bf42390..46498fc382ee 100644 > --- a/drivers/iommu/rockchip-iommu.c > +++ b/drivers/iommu/rockchip-iommu.c > @@ -727,7 +727,7 @@ static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain, > if (rk_dte_is_pt_valid(dte)) > goto done; > > - page_table = (u32 *)get_zeroed_page(GFP_ATOMIC | GFP_DMA32); > + page_table = (u32 *)get_zeroed_page(GFP_ATOMIC); > if (!page_table) > return ERR_PTR(-ENOMEM); > > @@ -1076,7 +1076,7 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type) > * Each level1 (dt) and level2 (pt) table has 1024 4-byte entries. > * Allocate one 4 KiB page for each table. > */ > - rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL | GFP_DMA32); > + rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL); > if (!rk_domain->dt) > goto err_free_domain; >
On 2023-06-15 23:25, Robin Murphy wrote: > On 2023-06-15 21:10, Jonas Karlman wrote: >> Newer Rockchip SoCs, RK356x and RK3588, support more then 4GB of memory. > > Older ones supposedly did too, e.g. commit 79db45be2b8b ("ARM: dts: > rockchip: convert rk3288 device tree files to 64 bits"). Are we certain > that nobody actually has a system with IOMMUv1 and more than 4GB of RAM? In IOMMU v1 bit 11:0 read back as 0 from MMU_DTE_ADDR reg, so I expect that the old limit for v1 is 4GB. I will reword this to focus on IOMMU v1 vs v2 instead of SoCs in v3. > >> However, the RK IOMMU driver is using the GFP_DMA32 flag to limit >> allocation of the discovery and page tables into memory below 4GB. >> >> Let's remove this limitation now that the discovery table address is > > Nit: s/discovery/directory/g again Will fix in v3 :-) Regards, Jonas > > Thanks, > Robin. > >> correctly configured for addresses above 4GB. >> >> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> >> --- >> v2: >> - no change >> >> drivers/iommu/rockchip-iommu.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c >> index 62be9bf42390..46498fc382ee 100644 >> --- a/drivers/iommu/rockchip-iommu.c >> +++ b/drivers/iommu/rockchip-iommu.c >> @@ -727,7 +727,7 @@ static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain, >> if (rk_dte_is_pt_valid(dte)) >> goto done; >> >> - page_table = (u32 *)get_zeroed_page(GFP_ATOMIC | GFP_DMA32); >> + page_table = (u32 *)get_zeroed_page(GFP_ATOMIC); >> if (!page_table) >> return ERR_PTR(-ENOMEM); >> >> @@ -1076,7 +1076,7 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type) >> * Each level1 (dt) and level2 (pt) table has 1024 4-byte entries. >> * Allocate one 4 KiB page for each table. >> */ >> - rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL | GFP_DMA32); >> + rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL); >> if (!rk_domain->dt) >> goto err_free_domain; >>
On 2023-06-15 23:26, Jonas Karlman wrote: > On 2023-06-15 23:25, Robin Murphy wrote: >> On 2023-06-15 21:10, Jonas Karlman wrote: >>> Newer Rockchip SoCs, RK356x and RK3588, support more then 4GB of memory. >> >> Older ones supposedly did too, e.g. commit 79db45be2b8b ("ARM: dts: >> rockchip: convert rk3288 device tree files to 64 bits"). Are we certain >> that nobody actually has a system with IOMMUv1 and more than 4GB of RAM? > > In IOMMU v1 bit 11:0 read back as 0 from MMU_DTE_ADDR reg, so I expect > that the old limit for v1 is 4GB. Right, that was my point, sorry if it was unclear - on at least RK3288, the *SoC* apparently supports RAM above 4GB, even if it's only the CPUs that can access it. GFP_DMA32 technically *is* correct and appropriate for IOMMUv1, so it's only safe to unconditionally remove it if we're sure that, on all relevant IOMMUv1 SoCs in practice, no RAM above 4GB exists such that ZONE_NORMAL is empty and all allocations come from ZONE_DMA32 by default anyway. (in truth this only matters for 64-bit SoCs, since on 32-bit there is no ZONE_DMA32, but any >4GB RAM would already have to be in ZONE_HIGHMEM, so safely out of scope) Thanks, Robin. > I will reword this to focus on IOMMU > v1 vs v2 instead of SoCs in v3. > >> >>> However, the RK IOMMU driver is using the GFP_DMA32 flag to limit >>> allocation of the discovery and page tables into memory below 4GB. >>> >>> Let's remove this limitation now that the discovery table address is >> >> Nit: s/discovery/directory/g again > > Will fix in v3 :-) > > Regards, > Jonas > >> >> Thanks, >> Robin. >> >>> correctly configured for addresses above 4GB. >>> >>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> >>> --- >>> v2: >>> - no change >>> >>> drivers/iommu/rockchip-iommu.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c >>> index 62be9bf42390..46498fc382ee 100644 >>> --- a/drivers/iommu/rockchip-iommu.c >>> +++ b/drivers/iommu/rockchip-iommu.c >>> @@ -727,7 +727,7 @@ static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain, >>> if (rk_dte_is_pt_valid(dte)) >>> goto done; >>> >>> - page_table = (u32 *)get_zeroed_page(GFP_ATOMIC | GFP_DMA32); >>> + page_table = (u32 *)get_zeroed_page(GFP_ATOMIC); >>> if (!page_table) >>> return ERR_PTR(-ENOMEM); >>> >>> @@ -1076,7 +1076,7 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type) >>> * Each level1 (dt) and level2 (pt) table has 1024 4-byte entries. >>> * Allocate one 4 KiB page for each table. >>> */ >>> - rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL | GFP_DMA32); >>> + rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL); >>> if (!rk_domain->dt) >>> goto err_free_domain; >>> >
On 2023-06-16 12:58, Robin Murphy wrote: > On 2023-06-15 23:26, Jonas Karlman wrote: >> On 2023-06-15 23:25, Robin Murphy wrote: >>> On 2023-06-15 21:10, Jonas Karlman wrote: >>>> Newer Rockchip SoCs, RK356x and RK3588, support more then 4GB of memory. >>> >>> Older ones supposedly did too, e.g. commit 79db45be2b8b ("ARM: dts: >>> rockchip: convert rk3288 device tree files to 64 bits"). Are we certain >>> that nobody actually has a system with IOMMUv1 and more than 4GB of RAM? >> >> In IOMMU v1 bit 11:0 read back as 0 from MMU_DTE_ADDR reg, so I expect >> that the old limit for v1 is 4GB. > > Right, that was my point, sorry if it was unclear - on at least RK3288, > the *SoC* apparently supports RAM above 4GB, even if it's only the CPUs > that can access it. GFP_DMA32 technically *is* correct and appropriate > for IOMMUv1, so it's only safe to unconditionally remove it if we're > sure that, on all relevant IOMMUv1 SoCs in practice, no RAM above 4GB > exists such that ZONE_NORMAL is empty and all allocations come from > ZONE_DMA32 by default anyway. > > (in truth this only matters for 64-bit SoCs, since on 32-bit there is no > ZONE_DMA32, but any >4GB RAM would already have to be in ZONE_HIGHMEM, > so safely out of scope) Thanks for the insights! I will keep the use of GFP_DMA32 flag on IOMMUv1 to be safe and only remove it for IOMMUv2 in a v3 series. Regards, Jonas > > Thanks, > Robin. > >> I will reword this to focus on IOMMU >> v1 vs v2 instead of SoCs in v3. >> >>> >>>> However, the RK IOMMU driver is using the GFP_DMA32 flag to limit >>>> allocation of the discovery and page tables into memory below 4GB. >>>> >>>> Let's remove this limitation now that the discovery table address is >>> >>> Nit: s/discovery/directory/g again >> >> Will fix in v3 :-) >> >> Regards, >> Jonas >> >>> >>> Thanks, >>> Robin. >>> >>>> correctly configured for addresses above 4GB. >>>> >>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> >>>> --- >>>> v2: >>>> - no change >>>> >>>> drivers/iommu/rockchip-iommu.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c >>>> index 62be9bf42390..46498fc382ee 100644 >>>> --- a/drivers/iommu/rockchip-iommu.c >>>> +++ b/drivers/iommu/rockchip-iommu.c >>>> @@ -727,7 +727,7 @@ static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain, >>>> if (rk_dte_is_pt_valid(dte)) >>>> goto done; >>>> >>>> - page_table = (u32 *)get_zeroed_page(GFP_ATOMIC | GFP_DMA32); >>>> + page_table = (u32 *)get_zeroed_page(GFP_ATOMIC); >>>> if (!page_table) >>>> return ERR_PTR(-ENOMEM); >>>> >>>> @@ -1076,7 +1076,7 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type) >>>> * Each level1 (dt) and level2 (pt) table has 1024 4-byte entries. >>>> * Allocate one 4 KiB page for each table. >>>> */ >>>> - rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL | GFP_DMA32); >>>> + rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL); >>>> if (!rk_domain->dt) >>>> goto err_free_domain; >>>> >>
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 62be9bf42390..46498fc382ee 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -727,7 +727,7 @@ static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain, if (rk_dte_is_pt_valid(dte)) goto done; - page_table = (u32 *)get_zeroed_page(GFP_ATOMIC | GFP_DMA32); + page_table = (u32 *)get_zeroed_page(GFP_ATOMIC); if (!page_table) return ERR_PTR(-ENOMEM); @@ -1076,7 +1076,7 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type) * Each level1 (dt) and level2 (pt) table has 1024 4-byte entries. * Allocate one 4 KiB page for each table. */ - rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL | GFP_DMA32); + rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL); if (!rk_domain->dt) goto err_free_domain;
Newer Rockchip SoCs, RK356x and RK3588, support more then 4GB of memory. However, the RK IOMMU driver is using the GFP_DMA32 flag to limit allocation of the discovery and page tables into memory below 4GB. Let's remove this limitation now that the discovery table address is correctly configured for addresses above 4GB. Signed-off-by: Jonas Karlman <jonas@kwiboo.se> --- v2: - no change drivers/iommu/rockchip-iommu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)