Hello Serge, Thomas,
On 11/11/2020 15:52, Serge Semin wrote:
>>> Could you send a patch, which removes check_kernel_section_mem completly ?
finally I think you are right and this would be the right way.
>> this will expose one issue:
>> platforms usually do it in a sane way, like it was done last 15 years, namely
>> add kernel image without non-complete pages on the boundaries.
>> This will lead to the situation, that request_resource() will fail at least
>> for .bss section of the kernel and it will not be properly displayed under
>> /proc/iomem (and probably same problem will appear, which initially motivated
>> the creation of check_kernel_section_mem()).
>
> Are you saying that some old platforms rely on the
> check_kernel_section_mem() method adding the memory occupied by the
> kernel to the system? If so, do you have an example of such?
Initially I was confused why the below patch didn't solve the issue on Octeon:
@@ -532,8 +532,8 @@ static void __init request_crashkernel(struct resource *res)
static void __init check_kernel_sections_mem(void)
{
- phys_addr_t start = PFN_PHYS(PFN_DOWN(__pa_symbol(&_text)));
- phys_addr_t size = PFN_PHYS(PFN_UP(__pa_symbol(&_end))) - start;
+ phys_addr_t start = __pa_symbol(&_text);
+ phys_addr_t size = __pa_symbol(&_end) - start;
... and finally I understood, that the reason was in fact that I tested on Linux
v5.4, which still had this code to reserve RAM resources:
for_each_memblock(memory, region) {
phys_addr_t start = PFN_PHYS(memblock_region_memory_base_pfn(region));
phys_addr_t end = PFN_PHYS(memblock_region_memory_end_pfn(region)) - 1;
struct resource *res;
...
res->start = start;
res->end = end;
res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
res->name = "System RAM";
request_resource(&iomem_resource, res);
so I suppose that's where this evil truncation happened. Nowdays this is different
and I believe we can try to remove check_kernel_sections_mem() completely and
this will solve the memory corruption on Octeon.
> So IMHO what could be the best conclusion in the framework of this patch:
> 1) As Thomas said any platform-specific reservation should be done in the
> platform-specific code. That means if octeon needs some memory behind
> the kernel being reserved, then it should be done for example in
> prom_init().
> 2) The check_kernel_sections_mem() method can be removed. But it
> should be done carefully. We at least need to try to find all the
> platforms, which rely on its functionality.
Thanks for looking into this! I agree with your analysis, I'll try to rework,
removing check_kernel_sections_mem().
--
Best regards,
Alexander Sverdlin.
