Page 2 of 2

Re: UEFI GetMemoryMap returns INVALID_PARAMETER

Posted: Wed Mar 31, 2021 2:00 pm
by zaval
kzinti wrote:Yes this is why I am adding "descriptorSize * 10"... I am adding extra space for new entries. My point is that you don't need exponential growth here. Adding space for a few entries (10 in my case) ought to be enough. Each EFI allocation typically adds one entry to the memory map.

So my algo does this:

1) Ask the current memory map size (with no buffer)
2) Add space for 10 extra descriptors to the memory map size returned by #1
3) Get the memory map. If that fails for some reason, goto #2
I didn't notice that adding in your code. But what I add is not much different. It first adds 8 extra entries, then only if it's not enough 16 and up to 1024 entries - for the craziest fw memory allocators. :D It's just for failsafety, since if the fw is sane, it will never add more, than 8 entries on a deallocation/allocation, right? :)

Re: UEFI GetMemoryMap returns INVALID_PARAMETER

Posted: Wed Mar 31, 2021 2:39 pm
by kzinti
NucleaTNT wrote: Here you go :D I'm probably just missing something super obvious
These descriptors look good to me.
NucleaTNT wrote: I think the static variable may be getting overwritten as I've just checked and despite being initialized as 0 it has garbage values in it
Something to investigate. Make sure you are clearing the BSS, verify where that static variable is stored, etc. Maybe try moving it outside the function.
zaval wrote: I didn't notice that adding in your code. But what I add is not much different. It first adds 8 extra entries, then only if it's not enough 16 and up to 1024 entries - for the craziest fw memory allocators. :D It's just for failsafety, since if the fw is sane, it will never add more, than 8 entries on a deallocation/allocation, right? :)
Agreed.

Re: UEFI GetMemoryMap returns INVALID_PARAMETER

Posted: Wed Mar 31, 2021 2:42 pm
by NucleaTNT
kzinti wrote:These descriptors look good to me.
Yep, I've just run the memory size calculations in my kernel after trying to pass references to set the variable somewhere else etc. It seems to have calculated it fine - thanks for the help with memory mapping, I'll just try and find a way to work around the calculations now. However I am curious, do you have any clue as to why the variable linked below is getting garbled?

https://github.com/NucleaTNT/NucleaOS/b ... ory.cpp#L4

I would like to, once again, thank everyone for being so helpful and kind despite my obvious lack of knowledge haha

Re: UEFI GetMemoryMap returns INVALID_PARAMETER

Posted: Wed Mar 31, 2021 2:46 pm
by kzinti
NucleaTNT wrote:However I am curious, do you have any clue as to why the variable linked below is getting garbled?
My guess is that this variable never gets initialized. The question is why? Is it in the .bss? Are you clearing the .bss? Also maybe take a look at the disassembly of that function. Try to move the variable outside the function to see if it makes a difference. Etc.

Re: UEFI GetMemoryMap returns INVALID_PARAMETER

Posted: Wed Mar 31, 2021 10:38 pm
by sj95126
kzinti wrote:

Code: Select all

static_assert(offsetof(EFI_MEMORY_DESCRIPTOR, PhysicalStart) == 8);
Thank for posting this! I had seen something similar but forgot where it was.

I was looking for a way to ensure I was using the right offsets of structs that are defined in C but referenced in ASM using displacements. Now I can put this in my header files after the struct definition. (in C, static_assert seems to be C++):

Code: Select all

#define ASSERT_OFFSET(S, MEMBER, OFFSET) \
        _Static_assert(__builtin_offsetof(struct S, MEMBER) == OFFSET);

ASSERT_OFFSET(cpu_state, saved_rsp, CPU_STATE_SAVED_RSP);
and I'll get a compile error if the offset #defines don't match the struct position.

Thanks!

Re: UEFI GetMemoryMap returns INVALID_PARAMETER

Posted: Wed Mar 31, 2021 11:07 pm
by nullplan
sj95126 wrote:(in C, static_assert seems to be C++)
Honestly, I wasn't quite sure of the state of static_assert in C, but in the past I got a lot of mileage out of:

Code: Select all

#define static_assert(c) do { extern char sa[2 * !!(c) - 1]; } while (0)
This opens a new scope to be compatible even with C89 compilers, then declares an external variable but doesn't use it, therefore not actually creating a reference. And that variable is an array of length 1 if the condition checks out and -1 if it doesn't. Obviously the latter is a compiler error. This macro can use all compile-time constants, even those the preprocessor cannot see.

Re: UEFI GetMemoryMap returns INVALID_PARAMETER

Posted: Thu Apr 01, 2021 8:11 am
by sj95126
nullplan wrote:
sj95126 wrote:(in C, static_assert seems to be C++)
Honestly, I wasn't quite sure of the state of static_assert in C, but in the past I got a lot of mileage out of:

Code: Select all

#define static_assert(c) do { extern char sa[2 * !!(c) - 1]; } while (0)
I saw some examples similar to that but the limitation was that you couldn't use it outside functions, because it contain a statement. The advantage to this other approach is that I can put the asserts in the header file, immediately after the struct definition and offset #defines, so they're logically grouped together.

Anyway, there's multiple ways to do it, and some are certainly more portable and backwards-compatible with others, but I'm just happy to have something that will help, so I can stop worrying I'll accidentally transpose struct offsets and create weird bugs.

Re: UEFI GetMemoryMap returns INVALID_PARAMETER

Posted: Thu Apr 01, 2021 9:23 am
by nullplan
sj95126 wrote:I saw some examples similar to that but the limitation was that you couldn't use it outside functions, because it contain a statement.
OK, you can also make the macro contain only the declaration sans semi-colon. Then you can no longer use it as a statement (unless you do it in a place where declarations are OK), but then, static_assert is not a run-time thing. Yeah, getting rid of the do-while may be the correct thing to do.