search

genodelabs / genode

Genode OS Framework
https://genode.org/
Other
1.04k stars 249 forks source link

DDE Linux struct page object aliasing #4809

Closed cnuke closed 7 months ago

cnuke commented 1 year ago

Due to the way the memory-allocation code is currently implemented and the struct page object handling is integrate aliasing that has unexpected side-effects can occur. Since struct page objects are created for a page-size aligned address [1] when requested, allocations that fall within the same virtual page address range share the same object.

Such objects are created lazy and and life-time management is only performed via ref-counting when the page API [2] (alloc_pages, free_pages etc.) is used. Since the current implementation removes struct page objects [3] when performing allocations still referenced objects can become invalid and can be corrupted as the memory is re-used.

[1] https://github.com/genodelabs/genode/blob/73771669f1c3a2c01555ebc01c7741b5a37b3d51/repos/dde_linux/src/lib/lx_emul/virt_to_page.c#L20-L37

[2] shadow/mmc/page_alloc.c

[3] https://github.com/genodelabs/genode/blob/73771669f1c3a2c01555ebc01c7741b5a37b3d51/repos/dde_linux/src/lib/lx_emul/alloc.cc#L20-L25

nfeske commented 7 months ago

Fixed in Genode 23.11.

chelmuth commented 7 months ago

The real fix is still not merged.

chelmuth commented 7 months ago

Commit cfe7915573a5a7c998c07f7c346b456dfee46b21 implements page-struct management per DMA buffer. Repositories for allwinner, imx, rpi, and zynq were also adapted for the change. NOw I cross fingers for nightly. Also, any test results for Pinephone and MNT Reform are much appreciated.

cnuke commented 7 months ago

@chelmuth please merge 1dadb8f and b337484 to the corresponding staging branches as they contained the needed adaptation to the used_apis file of the graphics drivers.

cnuke commented 7 months ago

I briefly tested the glmark2 preset on the PinePhone with the current staging branch. It works with the page-struct commits but after the sixth scenario the gpu driver fails to upgrade the Platform session and issues a RAM resource request of 16M. When I revert cfe7915 and 06b00b5 it finishes successfully without any resource request.

I can look into that later this week.

cnuke commented 7 months ago

@chelmuth commit 74c561f appears to solve the unexpected memory consumption. I'll will give the MNT Reform 2 a spin next.

cnuke commented 7 months ago

The MNT Reform 2 also plays ball if the aforementioned fixup is applied (otherwise the imx8mq_gpu_drvrequests more caps).