On Mon, Apr 12, 2021 at 7:12 AM Alex Ghiti <alex@xxxxxxxx> wrote: > > Le 4/9/21 à 10:42 AM, Vitaly Wool a écrit : > > On Fri, Apr 9, 2021 at 3:59 PM Mike Rapoport <rppt@xxxxxxxxxxxxx> wrote: > >> > >> On Fri, Apr 09, 2021 at 02:46:17PM +0200, David Hildenbrand wrote: > >>>>>> Also, will that memory properly be exposed in the resource tree as > >>>>>> System RAM (e.g., /proc/iomem) ? Otherwise some things (/proc/kcore) > >>>>>> won't work as expected - the kernel won't be included in a dump. > >>>> Do we really need a XIP kernel to included in kdump? > >>>> And does not it sound weird to expose flash as System RAM in /proc/iomem? ;-) > >>> > >>> See my other mail, maybe we actually want something different. > >>> > >>>> > >>>>> I have just checked and it does not appear in /proc/iomem. > >>>>> > >>>>> Ok your conclusion would be to have struct page, I'm going to implement this > >>>>> version then using memblock as you described. > >>>> > >>>> I'm not sure this is required. With XIP kernel text never gets into RAM, so > >>>> it does not seem to require struct page. > >>>> > >>>> XIP by definition has some limitations relatively to "normal" operation, > >>>> so lack of kdump could be one of them. > >>> > >>> I agree. > >>> > >>>> > >>>> I might be wrong, but IMHO, artificially creating a memory map for part of > >>>> flash would cause more problems in the long run. > >>> > >>> Can you elaborate? > >> > >> Nothing particular, just a gut feeling. Usually, when you force something > >> it comes out the wrong way later. > > > > It's possible still that MTD_XIP is implemented allowing to write to > > the flash used for XIP. While flash is being written, memory map > > doesn't make sense at all. I can't come up with a real life example > > when it can actually lead to problems but it is indeed weird when > > System RAM suddenly becomes unreadable. I really don't think exposing > > it in /proc/iomem is a good idea. > > > >>>> BTW, how does XIP account the kernel text on other architectures that > >>>> implement it? > >>> > >>> Interesting point, I thought XIP would be something new on RISC-V (well, at > >>> least to me :) ). If that concept exists already, we better mimic what > >>> existing implementations do. > >> > >> I had quick glance at ARM, it seems that kernel text does not have memory > >> map and does not show up in System RAM. > > > > Exactly, and I believe ARM64 won't do that too when it gets its own > > XIP support (which is underway). > > > > > memmap does not seem necessary and ARM/ARM64 do not use it. > > But if someone tries to get a struct page from a physical address that > lies in flash, as mentioned by David, that could lead to silent > corruptions if something exists at the address where the struct page > should be. And it is hard to know which features in the kernel depends > on that. > > Regarding SPARSEMEM, the vmemmap lies in its own region so that's > unlikely to happen, so we will catch those invalid accesses (and that's > what I observed on riscv). > > But for FLATMEM, memmap is in the linear mapping, then that could very > likely happen silently. > > Could a simple solution be to force SPARSEMEM for those XIP kernels ? > Then wrong things could happen, but we would see those and avoid > spending hours to debug :) > > I will at least send a v8 to remove the pfn_valid modifications for > FLATMEM that now returns true to pfn in flash. That sounds good to me. I am not very keen on spending 200K on struct pages for flash (we can think of this as of an option but I would definitely like to have the option to compile it out in the end), so let's remove pfn_valid and fix things that will eventually break, if some. Best regards, Vitaly
