On Tue, Dec 03, 2024 at 06:45:58AM -0700, Simon Glass wrote:
Hi Ilias,
On Tue, 3 Dec 2024 at 01:49, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Simon,
On Tue, 3 Dec 2024 at 02:22, Simon Glass sjg@chromium.org wrote:
Hi Tom,
On Mon, 2 Dec 2024 at 13:18, Tom Rini trini@konsulko.com wrote:
On Sun, Dec 01, 2024 at 08:24:23AM -0700, Simon Glass wrote:
Some functions are passing addresses instead of pointers to the efi_add_memory_map() function. This confusion is understandable since the function arguments indicate an address.
Make a note of the 8 places where there are problems, which would break usage in sandbox tests.
Future work will resolve these problems.
Signed-off-by: Simon Glass sjg@chromium.org
Please just resolve these rather than introducing a patch to then fix them later. This is something that should have been fixup'd before posting. Thanks.
That was deliberate, as I wanted people to see the problems. It will save discussion on later patches where the problems are fixed, if we can agree that these are actual problems. If people are happy to add review tags to the later patches then I'm happy to redo it.
I am pretty sure Heinrich has repeated this in the past. Why do we have to sprinkle around map_sysmem/unmap sysmem for sandbox? Polluting the entire u-boot to support a special platform is less than ideal. Why can't sandbox limit this internally and do whatever mappings it needs when it receives an address?
Tom, I suppose I have made my point.
Ilias, this is documented at [1] and has been the same for 10 years.
See for example the 'md' command, do_mem_md(), which shows how 'md 0' is implemented in sandbox.
As I have mentioned before, the nice thing is that you can easily make code work with sandbox just by converting casts from address-to-pointer into map_sysmem().
Back to this series, if you look at efi_allocate_pages() you'll currently see three calls (two map_to_sysmem() one map_sysmem(), but now, with this series, there is none. It also makes it a lot easier to understand what is going on, IMO.
Regards, Simon
[1] https://docs.u-boot.org/en/latest/arch/sandbox/sandbox.html#memory-emulation
This is something I was wondering about yesterday in fact. Especially these days when "everyone" is developing on machines with 8GB or more of memory, why does sandbox not just malloc 512MB/1GB/2GB/4GB (make it a switch?) and pretend that area is memory starting at some address. Especially if it's less than 4GB and starts at 0x80000000 more "memory starts at 0" bugs would be found and fixed. I swear we did something like that in my OS class 20+ years ago. I'm not sure what your link is meant for, as it doesn't say why sandbox works the way it does, merely that it works this way. Not that it can't work some other way.