| Message ID | 20220511112413.559734-1-xianting.tian@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RISC-V: Remove IORESOURCE_BUSY flag for no-map reserved memory | expand |
On 11/05/2022 12:24, Xianting Tian wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Commit 00ab027a3b82 ("RISC-V: Add kernel image sections to the resource tree") > added IORESOURCE_BUSY flag for no-map reserved memory, this casued > devm_ioremap_resource() failed for the no-map reserved memory in subsequent > operations of related driver, so remove the IORESOURCE_BUSY flag. > > The code to reproduce the issue, > dts: > mem0: memory@a0000000 { > reg = <0x0 0xa0000000 0 0x1000000>; > no-map; > }; > > &test { > status = "okay"; > memory-region = <&mem0>; > }; > > code: > np = of_parse_phandle(pdev->dev.of_node, "memory-region", 0); > ret = of_address_to_resource(np, 0, &r); > base = devm_ioremap_resource(&pdev->dev, &r); > // base = -EBUSY > > Fixes: 00ab027a3b82 ("RISC-V: Add kernel image sections to the resource tree") > Reported-by: Huaming Jiang <jianghuaming.jhm@alibaba-inc.com> > Reviewed-by: Guo Ren <guoren@kernel.org> > CC: Nick Kossifidis <mick@ics.forth.gr> > Signed-off-by: Xianting Tian <xianting.tian@linux.alibaba.com> Hey Xianting, This resend is no different to the patch you sent 5 mins ago, right? Thanks, Conor.
On 11.05.22 13:24, Xianting Tian wrote: > Commit 00ab027a3b82 ("RISC-V: Add kernel image sections to the resource tree") > added IORESOURCE_BUSY flag for no-map reserved memory, this casued > devm_ioremap_resource() failed for the no-map reserved memory in subsequent > operations of related driver, so remove the IORESOURCE_BUSY flag. > > The code to reproduce the issue, > dts: > mem0: memory@a0000000 { > reg = <0x0 0xa0000000 0 0x1000000>; > no-map; > }; > > &test { > status = "okay"; > memory-region = <&mem0>; > }; > > code: > np = of_parse_phandle(pdev->dev.of_node, "memory-region", 0); > ret = of_address_to_resource(np, 0, &r); > base = devm_ioremap_resource(&pdev->dev, &r); > // base = -EBUSY > > Fixes: 00ab027a3b82 ("RISC-V: Add kernel image sections to the resource tree") > Reported-by: Huaming Jiang <jianghuaming.jhm@alibaba-inc.com> > Reviewed-by: Guo Ren <guoren@kernel.org> > CC: Nick Kossifidis <mick@ics.forth.gr> > Signed-off-by: Xianting Tian <xianting.tian@linux.alibaba.com> > --- > arch/riscv/kernel/setup.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > index 834eb652a7b9..71f2966b1474 100644 > --- a/arch/riscv/kernel/setup.c > +++ b/arch/riscv/kernel/setup.c > @@ -214,7 +214,7 @@ static void __init init_resources(void) > > if (unlikely(memblock_is_nomap(region))) { > res->name = "Reserved"; > - res->flags = IORESOURCE_MEM | IORESOURCE_BUSY; > + res->flags = IORESOURCE_MEM; > } else { > res->name = "System RAM"; > res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; I assume the "Reserved" part is essentially unused by the kernel correct?
在 2022/5/11 下午8:27, David Hildenbrand 写道: > On 11.05.22 13:24, Xianting Tian wrote: >> Commit 00ab027a3b82 ("RISC-V: Add kernel image sections to the resource tree") >> added IORESOURCE_BUSY flag for no-map reserved memory, this casued >> devm_ioremap_resource() failed for the no-map reserved memory in subsequent >> operations of related driver, so remove the IORESOURCE_BUSY flag. >> >> The code to reproduce the issue, >> dts: >> mem0: memory@a0000000 { >> reg = <0x0 0xa0000000 0 0x1000000>; >> no-map; >> }; >> >> &test { >> status = "okay"; >> memory-region = <&mem0>; >> }; >> >> code: >> np = of_parse_phandle(pdev->dev.of_node, "memory-region", 0); >> ret = of_address_to_resource(np, 0, &r); >> base = devm_ioremap_resource(&pdev->dev, &r); >> // base = -EBUSY >> >> Fixes: 00ab027a3b82 ("RISC-V: Add kernel image sections to the resource tree") >> Reported-by: Huaming Jiang <jianghuaming.jhm@alibaba-inc.com> >> Reviewed-by: Guo Ren <guoren@kernel.org> >> CC: Nick Kossifidis <mick@ics.forth.gr> >> Signed-off-by: Xianting Tian <xianting.tian@linux.alibaba.com> >> --- >> arch/riscv/kernel/setup.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c >> index 834eb652a7b9..71f2966b1474 100644 >> --- a/arch/riscv/kernel/setup.c >> +++ b/arch/riscv/kernel/setup.c >> @@ -214,7 +214,7 @@ static void __init init_resources(void) >> >> if (unlikely(memblock_is_nomap(region))) { >> res->name = "Reserved"; >> - res->flags = IORESOURCE_MEM | IORESOURCE_BUSY; >> + res->flags = IORESOURCE_MEM; >> } else { >> res->name = "System RAM"; >> res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > I assume the "Reserved" part is essentially unused by the kernel correct? I think we may use it, actually we found the issue in our product after merged kdump functionality. Actually, the code didn't add IORESOURCE_BUSY for no-map reserved memory before 00ab027a3b82 merged, so it is a typo for commit 00ab027a3b82 to add IORESOURCE_BUSY? >
在 2022/5/11 下午8:37, Xianting Tian 写道: > > 在 2022/5/11 下午8:27, David Hildenbrand 写道: >> On 11.05.22 13:24, Xianting Tian wrote: >>> Commit 00ab027a3b82 ("RISC-V: Add kernel image sections to the >>> resource tree") >>> added IORESOURCE_BUSY flag for no-map reserved memory, this casued >>> devm_ioremap_resource() failed for the no-map reserved memory in >>> subsequent >>> operations of related driver, so remove the IORESOURCE_BUSY flag. >>> >>> The code to reproduce the issue, >>> dts: >>> mem0: memory@a0000000 { >>> reg = <0x0 0xa0000000 0 0x1000000>; >>> no-map; >>> }; >>> >>> &test { >>> status = "okay"; >>> memory-region = <&mem0>; >>> }; >>> >>> code: >>> np = of_parse_phandle(pdev->dev.of_node, "memory-region", 0); >>> ret = of_address_to_resource(np, 0, &r); >>> base = devm_ioremap_resource(&pdev->dev, &r); >>> // base = -EBUSY >>> >>> Fixes: 00ab027a3b82 ("RISC-V: Add kernel image sections to the >>> resource tree") >>> Reported-by: Huaming Jiang <jianghuaming.jhm@alibaba-inc.com> >>> Reviewed-by: Guo Ren <guoren@kernel.org> >>> CC: Nick Kossifidis <mick@ics.forth.gr> >>> Signed-off-by: Xianting Tian <xianting.tian@linux.alibaba.com> >>> --- >>> arch/riscv/kernel/setup.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c >>> index 834eb652a7b9..71f2966b1474 100644 >>> --- a/arch/riscv/kernel/setup.c >>> +++ b/arch/riscv/kernel/setup.c >>> @@ -214,7 +214,7 @@ static void __init init_resources(void) >>> if (unlikely(memblock_is_nomap(region))) { >>> res->name = "Reserved"; >>> - res->flags = IORESOURCE_MEM | IORESOURCE_BUSY; >>> + res->flags = IORESOURCE_MEM; >>> } else { >>> res->name = "System RAM"; >>> res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; >> I assume the "Reserved" part is essentially unused by the kernel >> correct? > > I think we may use it, actually we found the issue in our product > after merged kdump functionality. > > Actually, the code didn't add IORESOURCE_BUSY for no-map reserved > memory before 00ab027a3b82 merged, so it is a typo for commit > 00ab027a3b82 to add IORESOURCE_BUSY? This is arm64 code, which doesn't add IORESOURCE_BUSY for no-map reserved memory, arch/arm64/kernel/setup.c for_each_mem_region(region) { res = &standard_resources[i++]; if (memblock_is_nomap(region)) { res->name = "reserved"; res->flags = IORESOURCE_MEM; res->start = __pfn_to_phys(memblock_region_reserved_base_pfn(region)); res->end = __pfn_to_phys(memblock_region_reserved_end_pfn(region)) - 1; > >>
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index 834eb652a7b9..71f2966b1474 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -214,7 +214,7 @@ static void __init init_resources(void) if (unlikely(memblock_is_nomap(region))) { res->name = "Reserved"; - res->flags = IORESOURCE_MEM | IORESOURCE_BUSY; + res->flags = IORESOURCE_MEM; } else { res->name = "System RAM"; res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
Commit 00ab027a3b82 ("RISC-V: Add kernel image sections to the resource tree") added IORESOURCE_BUSY flag for no-map reserved memory, this casued devm_ioremap_resource() failed for the no-map reserved memory in subsequent operations of related driver, so remove the IORESOURCE_BUSY flag. The code to reproduce the issue, dts: mem0: memory@a0000000 { reg = <0x0 0xa0000000 0 0x1000000>; no-map; }; &test { status = "okay"; memory-region = <&mem0>; }; code: np = of_parse_phandle(pdev->dev.of_node, "memory-region", 0); ret = of_address_to_resource(np, 0, &r); base = devm_ioremap_resource(&pdev->dev, &r); // base = -EBUSY Fixes: 00ab027a3b82 ("RISC-V: Add kernel image sections to the resource tree") Reported-by: Huaming Jiang <jianghuaming.jhm@alibaba-inc.com> Reviewed-by: Guo Ren <guoren@kernel.org> CC: Nick Kossifidis <mick@ics.forth.gr> Signed-off-by: Xianting Tian <xianting.tian@linux.alibaba.com> --- arch/riscv/kernel/setup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)