Re: [PATCH 0/6] rust: page: Support borrowing `struct page` and physaddr conversion

[David Hildenbrand <david@redhat.com>]

On 04.02.25 22:18, Asahi Lina wrote:
> 
> 
> On 2/5/25 5:47 AM, David Hildenbrand wrote:
>> On 04.02.25 21:41, David Hildenbrand wrote:
>>> On 04.02.25 21:26, Jason Gunthorpe wrote:
>>>> On Tue, Feb 04, 2025 at 09:05:47PM +0100, David Hildenbrand wrote:
>>>>> Fully agreed, this is going into the right direction. Dumping what's
>>>>> mapped
>>>>> is a different story. Maybe that dumping logic could simply be
>>>>> written in C
>>>>> for the time being?
>>>>
>>>> ?
>>>>
>>>> Isn't dumping just a
>>>>      decode pte -> phys_to_virt() -> for_each_u64(virt) -> printk?
>>>>
>>>
>>> IIUC, the problematic bit is that you might not have a directmap such
>>> that phys_to_virt() would tell you the whole story.
>>
>> ... but it's late and I am confused. For dumping the *page table* that
>> would not be required, only when dumping mapped page content (and at
>> this point I am not sure if that is a requirement).
>>
>> So hopefully Asahi Lina can clarify what the issue was (if there is
>> any :) ).
> 
> Yes, the crash dumper has to dump the mapped page content. In fact, I
> don't care about the page tables themselves other than PTE permission
> bits, and the page tables alone are not useful for debugging firmware
> crashes (and aren't even included in the dump verbatim, at least not the
> kernel-allocated ones). The goal of the crash dumper is to take a
> complete dump of firmware virtual memory address space (which includes
> the kinds of memory I mentioned in [1]). The output is an ELF format
> core dump with all memory that the GPU firmware can access, that the
> kernel was able to successfully dump (which should be everything except
> for MMIO if the bootloader/DT have the right reserved-memory setup).
> 
> I *think* phys_to_virt should work just fine for *this* specific use
> case on this platform, but I'm not entirely sure. I still want to use
> the various pfn check functions before doing that, to exclude ranges
> that would definitely not work.

See my other mail, in /proc/kcore we do exactly that. See 
fs/proc/kcore.c, especially the KCORE_RAM case in read_kcore_iter().

So I would hope that at least for the Devcoredump we can find an 
alternative that does not require rust bindings, and maybe not even new 
c code at all (in a perfect world where you could just reuse  /proc/kcore).

-- 
Cheers,

David / dhildenb