On Wed, 8 May 2024 at 18:09, Caleb Connolly <caleb.conno...@linaro.org> wrote: > > Hi Sam, > > On 08/05/2024 13:40, Sam Day wrote: > > Salutations Sumit, > > > > On Wednesday, 8 May 2024 at 11:14 AM, Sumit Garg <sumit.g...@linaro.org> > > wrote: > > > >> > >> > >> Hi Sam, > >> > >> On Wed, 8 May 2024 at 00:11, Sam Day m...@samcday.com wrote: > >> > >>> The newly introduced carve_out_reserved_memory causes issues when > >>> U-Boot is chained from the lk2nd bootloader. lk2nd provides a > >>> simple-framebuffer device and marks the framebuffer region as no-map in > >>> the supplied /reserved-memory. Consequently, the simple_video driver > >>> triggers a page fault when it tries to write to this region. > >> > >> > >> How does the corresponding Linux kernel driver handle this? > > > > Firstly: I'm something of a middle-man here. I would consider Caleb the > > authoritative source on the carveouts stuff (since they wrote it) and > > Nikita Travkin the authority on the simplefb handoff (since he originally > > wrote it for lk2nd to hand off to Linux simpledrm and then adapted it to > > work with U-Boot simplefb). > > > > I consulted with Nikita on your first question here. He linked me to this > > snippet: > > https://elixir.bootlin.com/linux/v6.9-rc7/source/drivers/gpu/drm/tiny/simpledrm.c#L877 > > > >> Is the > >> framebuffer region required to be mapped as normal memory or device > >> type or something else? > > > > So I guess based on the link above, it's just mapped as normal uncached > > memory. > > > > I tried to do something like this a few days ago in U-Boot, but a) it > > doesn't work and b) I have no idea what I'm doing: > > https://github.com/samcday/u-boot/commit/c100cb3711ddf5b01601691f3e6a9ec890d9a496
Can you start adding some debug prints as to what might be happening? Also, look at my proposal below to hook memory mapping properly for drivers. > > > > After talking with Caleb about this for a bit, they suggested the patch you > > see here as what I guess could be considered a "stopgap" solution that > > hopefully makes it into 2024.07. > > > >> Similarly would normal memory type work for > >> all other reserved memory regions marked as no-map? > > > > I'll let Caleb weigh in here. My understanding is that the other regions > > *should* be marked as PTE_TYPE_FAULT because otherwise drivers might > > inadvertently speculatively access regions that are very much off-limits, > > such as TZ app regions. > > Right, carving out reserved regions and having the driver handle them is > theoretically the "correct" thing to do here. But I'm not sure that the > additional complexity offers much value from a U-Boot context. > > If we can teach the armv8 PT/cache code to handle this better, and teach > all the drivers to map their regions on-demand, this would probably help > us a lot (especially as right now if you attempt to access dead space > between peripherals then you'll hang the bus...). But U-Boot is a ways > away from that. I am not sure if U-Boot is really that far away although you can say the infrastructure is not hooked up for Arm. Have a look into map_physmem() and unmap_physmem() utilized by various drivers which are actually hooked up on MIPS. So I think the right way to approach this feature on Qcom platforms is to start hooking up those APIs on mach-snapdragon to begin with via mmu_set_region_dcache_behaviour() underneath. We can always go ahead and make it more generic if needed on other platforms too. -Sumit
