2019-08-23 22:53:33

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH] powerpc/8xx: Fix permanently mapped IMMR region.

When not using large TLBs, the IMMR region is still
mapped as a whole block in the FIXMAP area.

Do not remove pages mapped in the FIXMAP region when
initialising paging.

Properly report that the IMMR region is block-mapped even
when not using large TLBs.

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/mm/mem.c | 8 --------
arch/powerpc/mm/nohash/8xx.c | 13 +++++++------
2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 69f99128a8d6..8e221d8744ba 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -216,14 +216,6 @@ void __init paging_init(void)
unsigned long long total_ram = memblock_phys_mem_size();
phys_addr_t top_of_ram = memblock_end_of_DRAM();

-#ifdef CONFIG_PPC32
- unsigned long v = __fix_to_virt(__end_of_fixed_addresses - 1);
- unsigned long end = __fix_to_virt(FIX_HOLE);
-
- for (; v < end; v += PAGE_SIZE)
- map_kernel_page(v, 0, __pgprot(0)); /* XXX gross */
-#endif
-
#ifdef CONFIG_HIGHMEM
map_kernel_page(PKMAP_BASE, 0, __pgprot(0)); /* XXX gross */
pkmap_page_table = virt_to_kpte(PKMAP_BASE);
diff --git a/arch/powerpc/mm/nohash/8xx.c b/arch/powerpc/mm/nohash/8xx.c
index 4a06cb342da2..e6918235fe04 100644
--- a/arch/powerpc/mm/nohash/8xx.c
+++ b/arch/powerpc/mm/nohash/8xx.c
@@ -21,33 +21,34 @@ extern int __map_without_ltlbs;
static unsigned long block_mapped_ram;

/*
- * Return PA for this VA if it is in an area mapped with LTLBs.
+ * Return PA for this VA if it is in an area mapped with LTLBs or fixmap.
* Otherwise, returns 0
*/
phys_addr_t v_block_mapped(unsigned long va)
{
unsigned long p = PHYS_IMMR_BASE;

- if (__map_without_ltlbs)
- return 0;
if (va >= VIRT_IMMR_BASE && va < VIRT_IMMR_BASE + IMMR_SIZE)
return p + va - VIRT_IMMR_BASE;
+ if (__map_without_ltlbs)
+ return 0;
if (va >= PAGE_OFFSET && va < PAGE_OFFSET + block_mapped_ram)
return __pa(va);
return 0;
}

/*
- * Return VA for a given PA mapped with LTLBs or 0 if not mapped
+ * Return VA for a given PA mapped with LTLBs or fixmap
+ * Return 0 if not mapped
*/
unsigned long p_block_mapped(phys_addr_t pa)
{
unsigned long p = PHYS_IMMR_BASE;

- if (__map_without_ltlbs)
- return 0;
if (pa >= p && pa < p + IMMR_SIZE)
return VIRT_IMMR_BASE + pa - p;
+ if (__map_without_ltlbs)
+ return 0;
if (pa < block_mapped_ram)
return (unsigned long)__va(pa);
return 0;
--
2.13.3


2019-11-18 11:19:16

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH] powerpc/8xx: Fix permanently mapped IMMR region.

Christophe Leroy <[email protected]> writes:
> When not using large TLBs, the IMMR region is still
> mapped as a whole block in the FIXMAP area.
>
> Do not remove pages mapped in the FIXMAP region when
> initialising paging.
>
> Properly report that the IMMR region is block-mapped even
> when not using large TLBs.
>
> Signed-off-by: Christophe Leroy <[email protected]>
> ---
> arch/powerpc/mm/mem.c | 8 --------
> arch/powerpc/mm/nohash/8xx.c | 13 +++++++------
> 2 files changed, 7 insertions(+), 14 deletions(-)

This blows up pmac32_defconfig + qemu mac99 for me with:

NET: Registered protocol family 1
RPC: Registered named UNIX socket transport module.
RPC: Registered udp transport module.
RPC: Registered tcp transport module.
RPC: Registered tcp NFSv4.1 backchannel transport module.
PCI: CLS 0 bytes, default 32
Trying to unpack rootfs image as initramfs...
BUG: Unable to handle kernel data access on write at 0xfffdf000
Faulting instruction address: 0xc001eb4c
Oops: Kernel access of bad area, sig: 11 [#1]
BE PAGE_SIZE=4K MMU=Hash PowerMac
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 5.4.0-rc2-gcc49+ #50
NIP: c001eb4c LR: c0394214 CTR: 00000080
REGS: ef0b5a70 TRAP: 0300 Not tainted (5.4.0-rc2-gcc49+)
MSR: 00009032 <EE,ME,IR,DR,RI> CR: 44088444 XER: 20000000
DAR: fffdf000 DSISR: 42000000
GPR00: 00000080 ef0b5b28 ef0b0000 fffdf000 ef2001e4 00000000 fffdeffc fffe0000
GPR08: ef2011e8 00000008 00000009 00000004 28088442 00000000 c0005734 00000000
GPR16: 00000000 00000000 00000000 00000000 c07f0000 c0735600 00000000 ef09f500
GPR24: fffdf000 00000000 00000000 00000000 fffdf000 ef0b5c38 00001000 00001000
NIP [c001eb4c] memcpy+0x88/0x10c
LR [c0394214] iov_iter_copy_from_user_atomic+0x1fc/0x468
Call Trace:
[ef0b5b28] [c0394060] iov_iter_copy_from_user_atomic+0x48/0x468 (unreliable)
[ef0b5b68] [c0131438] generic_perform_write+0xfc/0x218
[ef0b5bb8] [c01360bc] __generic_file_write_iter+0x134/0x26c
[ef0b5bf8] [c0136324] generic_file_write_iter+0x130/0x1d8
[ef0b5c28] [c01a1754] __vfs_write+0x170/0x220
[ef0b5ca8] [c01a34f0] vfs_write+0xcc/0x1e0
[ef0b5cd8] [c01a386c] ksys_write+0xd0/0x120
[ef0b5d08] [c08a03a4] xwrite+0x44/0xa4
[ef0b5d28] [c08a0b64] do_copy+0x94/0xdc
[ef0b5d48] [c08a0044] write_buffer+0x40/0x68
[ef0b5d68] [c08a00cc] flush_buffer+0x60/0xe4
[ef0b5d98] [c08e87bc] __gunzip+0x2f0/0x3d0
[ef0b5dd8] [c08a0974] unpack_to_rootfs+0x1d8/0x334
[ef0b5e38] [c08a1238] populate_rootfs+0x80/0x168
[ef0b5e68] [c0005494] do_one_initcall+0x58/0x1f4
[ef0b5ed8] [c089e918] kernel_init_freeable+0x198/0x29c
[ef0b5f18] [c000574c] kernel_init+0x18/0x110
[ef0b5f38] [c0016274] ret_from_kernel_thread+0x14/0x1c
Instruction dump:
4200fff0 5400f0bf 7c0903a6 41820010 85240004 95260004 4200fff8 54a0d97f
54a506fe 39600004 7c0903a6 4182004c <7c0b37ec> 80e40004 81040008 8124000c
---[ end trace 52276ec2410ac084 ]---


cheers

2019-11-19 17:51:20

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH] powerpc/8xx: Fix permanently mapped IMMR region.

Michael Ellerman <[email protected]> a écrit :

> Christophe Leroy <[email protected]> writes:
>> When not using large TLBs, the IMMR region is still
>> mapped as a whole block in the FIXMAP area.
>>
>> Do not remove pages mapped in the FIXMAP region when
>> initialising paging.
>>
>> Properly report that the IMMR region is block-mapped even
>> when not using large TLBs.
>>
>> Signed-off-by: Christophe Leroy <[email protected]>
>> ---
>> arch/powerpc/mm/mem.c | 8 --------
>> arch/powerpc/mm/nohash/8xx.c | 13 +++++++------
>> 2 files changed, 7 insertions(+), 14 deletions(-)
>
> This blows up pmac32_defconfig + qemu mac99 for me with:

Ok, then there is still something I have not understood about fixmap.
I'll look at it next week

Christophe


2019-11-26 08:06:26

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH] powerpc/8xx: Fix permanently mapped IMMR region.



On 11/18/2019 11:17 AM, Michael Ellerman wrote:
> Christophe Leroy <[email protected]> writes:
>> When not using large TLBs, the IMMR region is still
>> mapped as a whole block in the FIXMAP area.
>>
>> Do not remove pages mapped in the FIXMAP region when
>> initialising paging.
>>
>> Properly report that the IMMR region is block-mapped even
>> when not using large TLBs.
>>
>> Signed-off-by: Christophe Leroy <[email protected]>
>> ---
>> arch/powerpc/mm/mem.c | 8 --------
>> arch/powerpc/mm/nohash/8xx.c | 13 +++++++------
>> 2 files changed, 7 insertions(+), 14 deletions(-)
>
> This blows up pmac32_defconfig + qemu mac99 for me with:
>
> NET: Registered protocol family 1
> RPC: Registered named UNIX socket transport module.
> RPC: Registered udp transport module.
> RPC: Registered tcp transport module.
> RPC: Registered tcp NFSv4.1 backchannel transport module.
> PCI: CLS 0 bytes, default 32
> Trying to unpack rootfs image as initramfs...
> BUG: Unable to handle kernel data access on write at 0xfffdf000

I tested it with pmac32_defconfig and qemu mac99 and don't get the problem:

NET: Registered protocol family 1
RPC: Registered named UNIX socket transport module.
RPC: Registered udp transport module.
RPC: Registered tcp transport module.
RPC: Registered tcp NFSv4.1 backchannel transport module.
PCI: CLS 0 bytes, default 32
Initialise system trusted keyrings
workingset: timestamp_bits=30 max_order=15 bucket_order=0
NFS: Registering the id_resolver key type
Key type id_resolver registered
...

Looks like I don't get that 'Trying to unpack rootfs image as
initramfs...', do you change anything to pmac32_defconfig ?

Anyway, when rebasing this patch on next branch, only the
arch/powerpc/mm/nohash/8xx.c change remains. The other part is already
applied through another patch.

So I believe the remaining part is safe to apply

Christophe