Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
Hi Alex,
On 14 June 2018 at 13:32, Alexander Graf wrote:
>
>
> On 14.06.18 21:02, Simon Glass wrote:
>> Hi Alex,
>>
>> On 14 June 2018 at 12:05, Alexander Graf wrote:
>>>
>>>
>>>
>>> On 14.06.18 19:15, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 11:08, Alexander Graf wrote:
>
> On 06/14/2018 06:55 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 14 June 2018 at 10:42, Alexander Graf wrote:
>>>
>>> On 06/14/2018 06:33 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 10:26, Alexander Graf wrote:
>
> On 06/14/2018 06:13 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 14 June 2018 at 10:07, Alexander Graf wrote:
>>>
>>> On 06/14/2018 05:53 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 09:47, Alexander Graf wrote:
>
>
>> Am 14.06.2018 um 17:43 schrieb Simon Glass :
>>
>> Hi Alex,
>>
>>> On 14 June 2018 at 08:22, Alexander Graf wrote:
>>>
>>>
>>>
Am 14.06.2018 um 16:12 schrieb Simon Glass :
Hi Alex,
>> On 14 June 2018 at 07:41, Alexander Graf
>> wrote:
>> On 06/14/2018 02:58 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
On 14 June 2018 at 04:12, Alexander Graf
wrote:
On 06/13/2018 04:37 AM, Simon Glass wrote:
With sandbox the U-Boot code is not mapped into the sandbox
memory
range
so does not need to be excluded when allocating EFI memory.
Update
the
EFI
memory init code to take account of that.
Also use mapmem instead of a cast to convert a memory
address
to
a
pointer.
Signed-off-by: Simon Glass
---
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Update to use mapmem instead of a cast
lib/efi_loader/efi_memory.c | 31
++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c
b/lib/efi_loader/efi_memory.c
index ec66af98ea..210e49ee8b 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int
pool_type,
efi_uintn_t size, void **buffer)
);
if (r == EFI_SUCCESS) {
- struct efi_pool_allocation *alloc = (void
*)(uintptr_t)t;
+ struct efi_pool_allocation *alloc =
map_sysmem(t,
size);
>>>
>>>
>>> This is still the wrong spot. We don't want the conversion
>>> to
>>> happen when
>>> going from an EFI internal address to an allocation, but
>>> when
>>> determining
>>> which addresses are usable in the first place.
>>
>> There seem to be two ways to do this:
>>
>> 1. Record addresses (ulong) in the EFI tables and use
>> map_sysmem()
>> before returning an address in the allocator
>> 2. Store pointers in the EFI tables using map_sysmem(), then
>> do
>> no
>> mapping in the allocator
>>
>> I've gone with option 1 since:
>>
>> - the EFI addresses are not pointers
>> - it makes sandbox consistent with other architectures which
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
On 14.06.18 21:02, Simon Glass wrote:
> Hi Alex,
>
> On 14 June 2018 at 12:05, Alexander Graf wrote:
>>
>>
>>
>> On 14.06.18 19:15, Simon Glass wrote:
>>> Hi Alex,
>>>
>>> On 14 June 2018 at 11:08, Alexander Graf wrote:
On 06/14/2018 06:55 PM, Simon Glass wrote:
>
> Hi Alex,
>
> On 14 June 2018 at 10:42, Alexander Graf wrote:
>>
>> On 06/14/2018 06:33 PM, Simon Glass wrote:
>>>
>>> Hi Alex,
>>>
>>> On 14 June 2018 at 10:26, Alexander Graf wrote:
On 06/14/2018 06:13 PM, Simon Glass wrote:
>
> Hi Alex,
>
> On 14 June 2018 at 10:07, Alexander Graf wrote:
>>
>> On 06/14/2018 05:53 PM, Simon Glass wrote:
>>>
>>> Hi Alex,
>>>
>>> On 14 June 2018 at 09:47, Alexander Graf wrote:
> Am 14.06.2018 um 17:43 schrieb Simon Glass :
>
> Hi Alex,
>
>> On 14 June 2018 at 08:22, Alexander Graf wrote:
>>
>>
>>
>>> Am 14.06.2018 um 16:12 schrieb Simon Glass :
>>>
>>> Hi Alex,
>>>
> On 14 June 2018 at 07:41, Alexander Graf
> wrote:
> On 06/14/2018 02:58 PM, Simon Glass wrote:
>
> Hi Alex,
>
>>> On 14 June 2018 at 04:12, Alexander Graf
>>> wrote:
>>>
>>> On 06/13/2018 04:37 AM, Simon Glass wrote:
>>>
>>> With sandbox the U-Boot code is not mapped into the sandbox
>>> memory
>>> range
>>> so does not need to be excluded when allocating EFI memory.
>>> Update
>>> the
>>> EFI
>>> memory init code to take account of that.
>>>
>>> Also use mapmem instead of a cast to convert a memory
>>> address
>>> to
>>> a
>>> pointer.
>>>
>>> Signed-off-by: Simon Glass
>>> ---
>>>
>>> Changes in v6: None
>>> Changes in v5: None
>>> Changes in v4: None
>>> Changes in v3: None
>>> Changes in v2:
>>> - Update to use mapmem instead of a cast
>>>
>>> lib/efi_loader/efi_memory.c | 31
>>> ++-
>>> 1 file changed, 18 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/lib/efi_loader/efi_memory.c
>>> b/lib/efi_loader/efi_memory.c
>>> index ec66af98ea..210e49ee8b 100644
>>> --- a/lib/efi_loader/efi_memory.c
>>> +++ b/lib/efi_loader/efi_memory.c
>>> @@ -9,6 +9,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> #include
>>> #include
>>> @@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int
>>> pool_type,
>>> efi_uintn_t size, void **buffer)
>>> );
>>>if (r == EFI_SUCCESS) {
>>> - struct efi_pool_allocation *alloc = (void
>>> *)(uintptr_t)t;
>>> + struct efi_pool_allocation *alloc =
>>> map_sysmem(t,
>>> size);
>>
>>
>> This is still the wrong spot. We don't want the conversion to
>> happen when
>> going from an EFI internal address to an allocation, but when
>> determining
>> which addresses are usable in the first place.
>
> There seem to be two ways to do this:
>
> 1. Record addresses (ulong) in the EFI tables and use
> map_sysmem()
> before returning an address in the allocator
> 2. Store pointers in the EFI tables using map_sysmem(), then
> do
> no
> mapping in the allocator
>
> I've gone with option 1 since:
>
> - the EFI addresses are not pointers
> - it makes sandbox consistent with other architectures which
> use
> an
> address rather than a pointer in EFI tables
> - we don't normally do pointer arithmetic on the results of
> map_sysmem()
> - we normally
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
Hi Alex,
On 14 June 2018 at 12:05, Alexander Graf wrote:
>
>
>
> On 14.06.18 19:15, Simon Glass wrote:
> > Hi Alex,
> >
> > On 14 June 2018 at 11:08, Alexander Graf wrote:
> >>
> >> On 06/14/2018 06:55 PM, Simon Glass wrote:
> >>>
> >>> Hi Alex,
> >>>
> >>> On 14 June 2018 at 10:42, Alexander Graf wrote:
>
> On 06/14/2018 06:33 PM, Simon Glass wrote:
> >
> > Hi Alex,
> >
> > On 14 June 2018 at 10:26, Alexander Graf wrote:
> >>
> >> On 06/14/2018 06:13 PM, Simon Glass wrote:
> >>>
> >>> Hi Alex,
> >>>
> >>> On 14 June 2018 at 10:07, Alexander Graf wrote:
>
> On 06/14/2018 05:53 PM, Simon Glass wrote:
> >
> > Hi Alex,
> >
> > On 14 June 2018 at 09:47, Alexander Graf wrote:
> >>
> >>
> >>> Am 14.06.2018 um 17:43 schrieb Simon Glass :
> >>>
> >>> Hi Alex,
> >>>
> On 14 June 2018 at 08:22, Alexander Graf wrote:
>
>
>
> > Am 14.06.2018 um 16:12 schrieb Simon Glass :
> >
> > Hi Alex,
> >
> >>> On 14 June 2018 at 07:41, Alexander Graf
> >>> wrote:
> >>> On 06/14/2018 02:58 PM, Simon Glass wrote:
> >>>
> >>> Hi Alex,
> >>>
> > On 14 June 2018 at 04:12, Alexander Graf
> > wrote:
> >
> > On 06/13/2018 04:37 AM, Simon Glass wrote:
> >
> > With sandbox the U-Boot code is not mapped into the sandbox
> > memory
> > range
> > so does not need to be excluded when allocating EFI memory.
> > Update
> > the
> > EFI
> > memory init code to take account of that.
> >
> > Also use mapmem instead of a cast to convert a memory
> > address
> > to
> > a
> > pointer.
> >
> > Signed-off-by: Simon Glass
> > ---
> >
> > Changes in v6: None
> > Changes in v5: None
> > Changes in v4: None
> > Changes in v3: None
> > Changes in v2:
> > - Update to use mapmem instead of a cast
> >
> > lib/efi_loader/efi_memory.c | 31
> > ++-
> > 1 file changed, 18 insertions(+), 13 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_memory.c
> > b/lib/efi_loader/efi_memory.c
> > index ec66af98ea..210e49ee8b 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -9,6 +9,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > @@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int
> > pool_type,
> > efi_uintn_t size, void **buffer)
> > );
> >if (r == EFI_SUCCESS) {
> > - struct efi_pool_allocation *alloc = (void
> > *)(uintptr_t)t;
> > + struct efi_pool_allocation *alloc =
> > map_sysmem(t,
> > size);
>
>
> This is still the wrong spot. We don't want the conversion to
> happen when
> going from an EFI internal address to an allocation, but when
> determining
> which addresses are usable in the first place.
> >>>
> >>> There seem to be two ways to do this:
> >>>
> >>> 1. Record addresses (ulong) in the EFI tables and use
> >>> map_sysmem()
> >>> before returning an address in the allocator
> >>> 2. Store pointers in the EFI tables using map_sysmem(), then
> >>> do
> >>> no
> >>> mapping in the allocator
> >>>
> >>> I've gone with option 1 since:
> >>>
> >>> - the EFI addresses are not pointers
> >>> - it makes sandbox consistent with other architectures which
> >>> use
> >>> an
> >>> address rather than a pointer in EFI tables
> >>> - we don't normally do pointer arithmetic on the results of
> >>> map_sysmem()
> >>> - we normally map the memory when it is used rather than when
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
On 14.06.18 19:15, Simon Glass wrote:
> Hi Alex,
>
> On 14 June 2018 at 11:08, Alexander Graf wrote:
>>
>> On 06/14/2018 06:55 PM, Simon Glass wrote:
>>>
>>> Hi Alex,
>>>
>>> On 14 June 2018 at 10:42, Alexander Graf wrote:
On 06/14/2018 06:33 PM, Simon Glass wrote:
>
> Hi Alex,
>
> On 14 June 2018 at 10:26, Alexander Graf wrote:
>>
>> On 06/14/2018 06:13 PM, Simon Glass wrote:
>>>
>>> Hi Alex,
>>>
>>> On 14 June 2018 at 10:07, Alexander Graf wrote:
On 06/14/2018 05:53 PM, Simon Glass wrote:
>
> Hi Alex,
>
> On 14 June 2018 at 09:47, Alexander Graf wrote:
>>
>>
>>> Am 14.06.2018 um 17:43 schrieb Simon Glass :
>>>
>>> Hi Alex,
>>>
On 14 June 2018 at 08:22, Alexander Graf wrote:
> Am 14.06.2018 um 16:12 schrieb Simon Glass :
>
> Hi Alex,
>
>>> On 14 June 2018 at 07:41, Alexander Graf wrote:
>>> On 06/14/2018 02:58 PM, Simon Glass wrote:
>>>
>>> Hi Alex,
>>>
> On 14 June 2018 at 04:12, Alexander Graf
> wrote:
>
> On 06/13/2018 04:37 AM, Simon Glass wrote:
>
> With sandbox the U-Boot code is not mapped into the sandbox
> memory
> range
> so does not need to be excluded when allocating EFI memory.
> Update
> the
> EFI
> memory init code to take account of that.
>
> Also use mapmem instead of a cast to convert a memory address
> to
> a
> pointer.
>
> Signed-off-by: Simon Glass
> ---
>
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2:
> - Update to use mapmem instead of a cast
>
> lib/efi_loader/efi_memory.c | 31
> ++-
> 1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/lib/efi_loader/efi_memory.c
> b/lib/efi_loader/efi_memory.c
> index ec66af98ea..210e49ee8b 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -9,6 +9,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> @@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int
> pool_type,
> efi_uintn_t size, void **buffer)
> );
>if (r == EFI_SUCCESS) {
> - struct efi_pool_allocation *alloc = (void
> *)(uintptr_t)t;
> + struct efi_pool_allocation *alloc =
> map_sysmem(t,
> size);
This is still the wrong spot. We don't want the conversion to
happen when
going from an EFI internal address to an allocation, but when
determining
which addresses are usable in the first place.
>>>
>>> There seem to be two ways to do this:
>>>
>>> 1. Record addresses (ulong) in the EFI tables and use
>>> map_sysmem()
>>> before returning an address in the allocator
>>> 2. Store pointers in the EFI tables using map_sysmem(), then do
>>> no
>>> mapping in the allocator
>>>
>>> I've gone with option 1 since:
>>>
>>> - the EFI addresses are not pointers
>>> - it makes sandbox consistent with other architectures which use
>>> an
>>> address rather than a pointer in EFI tables
>>> - we don't normally do pointer arithmetic on the results of
>>> map_sysmem()
>>> - we normally map the memory when it is used rather than when it
>>> is
>>> set up
>>>
>>> I think you are asking for option 2. I think that would get very
>>> confusing. The addresses where things actually end up in sandbox
>>> are
>>> best kept to sandbox.
>>>
>>> Overall I feel that you are either missing the point of sandbox
>>>
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
Hi Alex,
On 14 June 2018 at 11:08, Alexander Graf wrote:
>
> On 06/14/2018 06:55 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 14 June 2018 at 10:42, Alexander Graf wrote:
>>>
>>> On 06/14/2018 06:33 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 10:26, Alexander Graf wrote:
>
> On 06/14/2018 06:13 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 14 June 2018 at 10:07, Alexander Graf wrote:
>>>
>>> On 06/14/2018 05:53 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 09:47, Alexander Graf wrote:
>
>
>> Am 14.06.2018 um 17:43 schrieb Simon Glass :
>>
>> Hi Alex,
>>
>>> On 14 June 2018 at 08:22, Alexander Graf wrote:
>>>
>>>
>>>
Am 14.06.2018 um 16:12 schrieb Simon Glass :
Hi Alex,
>> On 14 June 2018 at 07:41, Alexander Graf wrote:
>> On 06/14/2018 02:58 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
On 14 June 2018 at 04:12, Alexander Graf
wrote:
On 06/13/2018 04:37 AM, Simon Glass wrote:
With sandbox the U-Boot code is not mapped into the sandbox
memory
range
so does not need to be excluded when allocating EFI memory.
Update
the
EFI
memory init code to take account of that.
Also use mapmem instead of a cast to convert a memory address
to
a
pointer.
Signed-off-by: Simon Glass
---
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Update to use mapmem instead of a cast
lib/efi_loader/efi_memory.c | 31
++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c
b/lib/efi_loader/efi_memory.c
index ec66af98ea..210e49ee8b 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int
pool_type,
efi_uintn_t size, void **buffer)
);
if (r == EFI_SUCCESS) {
- struct efi_pool_allocation *alloc = (void
*)(uintptr_t)t;
+ struct efi_pool_allocation *alloc =
map_sysmem(t,
size);
>>>
>>>
>>> This is still the wrong spot. We don't want the conversion to
>>> happen when
>>> going from an EFI internal address to an allocation, but when
>>> determining
>>> which addresses are usable in the first place.
>>
>> There seem to be two ways to do this:
>>
>> 1. Record addresses (ulong) in the EFI tables and use
>> map_sysmem()
>> before returning an address in the allocator
>> 2. Store pointers in the EFI tables using map_sysmem(), then do
>> no
>> mapping in the allocator
>>
>> I've gone with option 1 since:
>>
>> - the EFI addresses are not pointers
>> - it makes sandbox consistent with other architectures which use
>> an
>> address rather than a pointer in EFI tables
>> - we don't normally do pointer arithmetic on the results of
>> map_sysmem()
>> - we normally map the memory when it is used rather than when it
>> is
>> set up
>>
>> I think you are asking for option 2. I think that would get very
>> confusing. The addresses where things actually end up in sandbox
>> are
>> best kept to sandbox.
>>
>> Overall I feel that you are either missing the point of sandbox
>> addressing, or don't agree with how it is done. But it does work
>> pretty well and we don't get a lot of confusion with sandbox
>> pointers
>>
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
On 06/14/2018 06:55 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 10:42, Alexander Graf wrote:
On 06/14/2018 06:33 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 10:26, Alexander Graf wrote:
On 06/14/2018 06:13 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 10:07, Alexander Graf wrote:
On 06/14/2018 05:53 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 09:47, Alexander Graf wrote:
Am 14.06.2018 um 17:43 schrieb Simon Glass :
Hi Alex,
On 14 June 2018 at 08:22, Alexander Graf wrote:
Am 14.06.2018 um 16:12 schrieb Simon Glass :
Hi Alex,
On 14 June 2018 at 07:41, Alexander Graf wrote:
On 06/14/2018 02:58 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 04:12, Alexander Graf
wrote:
On 06/13/2018 04:37 AM, Simon Glass wrote:
With sandbox the U-Boot code is not mapped into the sandbox
memory
range
so does not need to be excluded when allocating EFI memory.
Update
the
EFI
memory init code to take account of that.
Also use mapmem instead of a cast to convert a memory address
to
a
pointer.
Signed-off-by: Simon Glass
---
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Update to use mapmem instead of a cast
lib/efi_loader/efi_memory.c | 31
++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c
b/lib/efi_loader/efi_memory.c
index ec66af98ea..210e49ee8b 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int
pool_type,
efi_uintn_t size, void **buffer)
);
if (r == EFI_SUCCESS) {
- struct efi_pool_allocation *alloc = (void
*)(uintptr_t)t;
+ struct efi_pool_allocation *alloc =
map_sysmem(t,
size);
This is still the wrong spot. We don't want the conversion to
happen when
going from an EFI internal address to an allocation, but when
determining
which addresses are usable in the first place.
There seem to be two ways to do this:
1. Record addresses (ulong) in the EFI tables and use
map_sysmem()
before returning an address in the allocator
2. Store pointers in the EFI tables using map_sysmem(), then do
no
mapping in the allocator
I've gone with option 1 since:
- the EFI addresses are not pointers
- it makes sandbox consistent with other architectures which use
an
address rather than a pointer in EFI tables
- we don't normally do pointer arithmetic on the results of
map_sysmem()
- we normally map the memory when it is used rather than when it
is
set up
I think you are asking for option 2. I think that would get very
confusing. The addresses where things actually end up in sandbox
are
best kept to sandbox.
Overall I feel that you are either missing the point of sandbox
addressing, or don't agree with how it is done. But it does work
pretty well and we don't get a lot of confusion with sandbox
pointers
since we typically use the address until the last moment.
I've assembled a quick tree for version 2. With that I'm able to
run
a
simple hello world efi application. Grub refuses to start because
it
wants
memory in the low 32bit and also emits random PIO accessing
functions, which
obviously don't work work from user space.
But overall, I think this is the right path to tackle this:
https://github.com/agraf/u-boot/tree/efi-sandbox
What do you mean by version 2?
Option 2 is what you called it. It's the only option we have to
make
efi binaries work.
It looks like you've added one patch,
so will you send that to the list?
It's more than 1 patch. And yes, I'll send them.
Anyway, I hope I can convince you of the above, the way sandbox
memory
works.
I still dislike option 1 :)
The reason is simple: The efi memory map is available to efi
payloads.
It's perfectly legal for them to do a static allocation at a
particular
address. We also share a lot of (host) pointers for callbacks and
structs
already with efi applications, so there is no real point to have a
split
brain situation between u-boot and host pointers.
OK so you mean they don't have to allocate memory before using it?
How
then do you make sure that there is no conflict? I thought EFI was
an
API?
You allocate it, but payloads expect that the address you pass in as
address you allocate at is the pointer/address that gets returned.
Can you please point me to that? Are you referring to this call?
static efi_status_t EFIAPI efi_get_memory_map_ext(
efi_uintn_t *memory_map_size,
struct efi_mem_desc *memory_map,
efi_uintn_t *map_key,
efi_uintn_t *descriptor_size,
uint32_t *descriptor_version)
If so, we can still support that.
In a nutshell, what you're proposing is that the efi quivalent of
mmap(addr, MAP_FIXED) returns something != addr. That will break
things.
I
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
Hi Alex,
On 14 June 2018 at 10:42, Alexander Graf wrote:
> On 06/14/2018 06:33 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 14 June 2018 at 10:26, Alexander Graf wrote:
>>>
>>> On 06/14/2018 06:13 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 10:07, Alexander Graf wrote:
>
> On 06/14/2018 05:53 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 14 June 2018 at 09:47, Alexander Graf wrote:
>>>
>>>
Am 14.06.2018 um 17:43 schrieb Simon Glass :
Hi Alex,
> On 14 June 2018 at 08:22, Alexander Graf wrote:
>
>
>
>> Am 14.06.2018 um 16:12 schrieb Simon Glass :
>>
>> Hi Alex,
>>
On 14 June 2018 at 07:41, Alexander Graf wrote:
On 06/14/2018 02:58 PM, Simon Glass wrote:
Hi Alex,
>> On 14 June 2018 at 04:12, Alexander Graf
>> wrote:
>>
>> On 06/13/2018 04:37 AM, Simon Glass wrote:
>>
>> With sandbox the U-Boot code is not mapped into the sandbox
>> memory
>> range
>> so does not need to be excluded when allocating EFI memory.
>> Update
>> the
>> EFI
>> memory init code to take account of that.
>>
>> Also use mapmem instead of a cast to convert a memory address
>> to
>> a
>> pointer.
>>
>> Signed-off-by: Simon Glass
>> ---
>>
>> Changes in v6: None
>> Changes in v5: None
>> Changes in v4: None
>> Changes in v3: None
>> Changes in v2:
>> - Update to use mapmem instead of a cast
>>
>> lib/efi_loader/efi_memory.c | 31
>> ++-
>> 1 file changed, 18 insertions(+), 13 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_memory.c
>> b/lib/efi_loader/efi_memory.c
>> index ec66af98ea..210e49ee8b 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -9,6 +9,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> @@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int
>> pool_type,
>> efi_uintn_t size, void **buffer)
>> );
>> if (r == EFI_SUCCESS) {
>> - struct efi_pool_allocation *alloc = (void
>> *)(uintptr_t)t;
>> + struct efi_pool_allocation *alloc =
>> map_sysmem(t,
>> size);
>
>
> This is still the wrong spot. We don't want the conversion to
> happen when
> going from an EFI internal address to an allocation, but when
> determining
> which addresses are usable in the first place.
There seem to be two ways to do this:
1. Record addresses (ulong) in the EFI tables and use
map_sysmem()
before returning an address in the allocator
2. Store pointers in the EFI tables using map_sysmem(), then do
no
mapping in the allocator
I've gone with option 1 since:
- the EFI addresses are not pointers
- it makes sandbox consistent with other architectures which use
an
address rather than a pointer in EFI tables
- we don't normally do pointer arithmetic on the results of
map_sysmem()
- we normally map the memory when it is used rather than when it
is
set up
I think you are asking for option 2. I think that would get very
confusing. The addresses where things actually end up in sandbox
are
best kept to sandbox.
Overall I feel that you are either missing the point of sandbox
addressing, or don't agree with how it is done. But it does work
pretty well and we don't get a lot of confusion with sandbox
pointers
since we typically use the address until the last moment.
>>>
>>>
>>> I've assembled a quick tree for version 2. With that I'm able to
>>> run
>>> a
>>> simple hello world efi application. Grub refuses to start because
>>> it
>>> wants
>>> memory in the low 32bit and also emits random PIO accessing
>>> functions,
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
On 06/14/2018 06:33 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 10:26, Alexander Graf wrote:
On 06/14/2018 06:13 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 10:07, Alexander Graf wrote:
On 06/14/2018 05:53 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 09:47, Alexander Graf wrote:
Am 14.06.2018 um 17:43 schrieb Simon Glass :
Hi Alex,
On 14 June 2018 at 08:22, Alexander Graf wrote:
Am 14.06.2018 um 16:12 schrieb Simon Glass :
Hi Alex,
On 14 June 2018 at 07:41, Alexander Graf wrote:
On 06/14/2018 02:58 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 04:12, Alexander Graf wrote:
On 06/13/2018 04:37 AM, Simon Glass wrote:
With sandbox the U-Boot code is not mapped into the sandbox
memory
range
so does not need to be excluded when allocating EFI memory.
Update
the
EFI
memory init code to take account of that.
Also use mapmem instead of a cast to convert a memory address to
a
pointer.
Signed-off-by: Simon Glass
---
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Update to use mapmem instead of a cast
lib/efi_loader/efi_memory.c | 31
++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c
b/lib/efi_loader/efi_memory.c
index ec66af98ea..210e49ee8b 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int
pool_type,
efi_uintn_t size, void **buffer)
);
if (r == EFI_SUCCESS) {
- struct efi_pool_allocation *alloc = (void
*)(uintptr_t)t;
+ struct efi_pool_allocation *alloc =
map_sysmem(t,
size);
This is still the wrong spot. We don't want the conversion to
happen when
going from an EFI internal address to an allocation, but when
determining
which addresses are usable in the first place.
There seem to be two ways to do this:
1. Record addresses (ulong) in the EFI tables and use map_sysmem()
before returning an address in the allocator
2. Store pointers in the EFI tables using map_sysmem(), then do no
mapping in the allocator
I've gone with option 1 since:
- the EFI addresses are not pointers
- it makes sandbox consistent with other architectures which use
an
address rather than a pointer in EFI tables
- we don't normally do pointer arithmetic on the results of
map_sysmem()
- we normally map the memory when it is used rather than when it
is
set up
I think you are asking for option 2. I think that would get very
confusing. The addresses where things actually end up in sandbox
are
best kept to sandbox.
Overall I feel that you are either missing the point of sandbox
addressing, or don't agree with how it is done. But it does work
pretty well and we don't get a lot of confusion with sandbox
pointers
since we typically use the address until the last moment.
I've assembled a quick tree for version 2. With that I'm able to
run
a
simple hello world efi application. Grub refuses to start because
it
wants
memory in the low 32bit and also emits random PIO accessing
functions, which
obviously don't work work from user space.
But overall, I think this is the right path to tackle this:
https://github.com/agraf/u-boot/tree/efi-sandbox
What do you mean by version 2?
Option 2 is what you called it. It's the only option we have to make
efi binaries work.
It looks like you've added one patch,
so will you send that to the list?
It's more than 1 patch. And yes, I'll send them.
Anyway, I hope I can convince you of the above, the way sandbox
memory
works.
I still dislike option 1 :)
The reason is simple: The efi memory map is available to efi
payloads.
It's perfectly legal for them to do a static allocation at a
particular
address. We also share a lot of (host) pointers for callbacks and
structs
already with efi applications, so there is no real point to have a
split
brain situation between u-boot and host pointers.
OK so you mean they don't have to allocate memory before using it? How
then do you make sure that there is no conflict? I thought EFI was an
API?
You allocate it, but payloads expect that the address you pass in as
address you allocate at is the pointer/address that gets returned.
Can you please point me to that? Are you referring to this call?
static efi_status_t EFIAPI efi_get_memory_map_ext(
efi_uintn_t *memory_map_size,
struct efi_mem_desc *memory_map,
efi_uintn_t *map_key,
efi_uintn_t *descriptor_size,
uint32_t *descriptor_version)
If so, we can still support that.
In a nutshell, what you're proposing is that the efi quivalent of
mmap(addr, MAP_FIXED) returns something != addr. That will break
things.
I see this function:
efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void
**buffer)
So there appears to
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
Hi Alex,
On 14 June 2018 at 10:26, Alexander Graf wrote:
> On 06/14/2018 06:13 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 14 June 2018 at 10:07, Alexander Graf wrote:
>>>
>>> On 06/14/2018 05:53 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 09:47, Alexander Graf wrote:
>
>
>> Am 14.06.2018 um 17:43 schrieb Simon Glass :
>>
>> Hi Alex,
>>
>>> On 14 June 2018 at 08:22, Alexander Graf wrote:
>>>
>>>
>>>
Am 14.06.2018 um 16:12 schrieb Simon Glass :
Hi Alex,
>> On 14 June 2018 at 07:41, Alexander Graf wrote:
>> On 06/14/2018 02:58 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
On 14 June 2018 at 04:12, Alexander Graf wrote:
On 06/13/2018 04:37 AM, Simon Glass wrote:
With sandbox the U-Boot code is not mapped into the sandbox
memory
range
so does not need to be excluded when allocating EFI memory.
Update
the
EFI
memory init code to take account of that.
Also use mapmem instead of a cast to convert a memory address to
a
pointer.
Signed-off-by: Simon Glass
---
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Update to use mapmem instead of a cast
lib/efi_loader/efi_memory.c | 31
++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c
b/lib/efi_loader/efi_memory.c
index ec66af98ea..210e49ee8b 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int
pool_type,
efi_uintn_t size, void **buffer)
);
if (r == EFI_SUCCESS) {
- struct efi_pool_allocation *alloc = (void
*)(uintptr_t)t;
+ struct efi_pool_allocation *alloc =
map_sysmem(t,
size);
>>>
>>>
>>> This is still the wrong spot. We don't want the conversion to
>>> happen when
>>> going from an EFI internal address to an allocation, but when
>>> determining
>>> which addresses are usable in the first place.
>>
>> There seem to be two ways to do this:
>>
>> 1. Record addresses (ulong) in the EFI tables and use map_sysmem()
>> before returning an address in the allocator
>> 2. Store pointers in the EFI tables using map_sysmem(), then do no
>> mapping in the allocator
>>
>> I've gone with option 1 since:
>>
>> - the EFI addresses are not pointers
>> - it makes sandbox consistent with other architectures which use
>> an
>> address rather than a pointer in EFI tables
>> - we don't normally do pointer arithmetic on the results of
>> map_sysmem()
>> - we normally map the memory when it is used rather than when it
>> is
>> set up
>>
>> I think you are asking for option 2. I think that would get very
>> confusing. The addresses where things actually end up in sandbox
>> are
>> best kept to sandbox.
>>
>> Overall I feel that you are either missing the point of sandbox
>> addressing, or don't agree with how it is done. But it does work
>> pretty well and we don't get a lot of confusion with sandbox
>> pointers
>> since we typically use the address until the last moment.
>
>
> I've assembled a quick tree for version 2. With that I'm able to
> run
> a
> simple hello world efi application. Grub refuses to start because
> it
> wants
> memory in the low 32bit and also emits random PIO accessing
> functions, which
> obviously don't work work from user space.
>
> But overall, I think this is the right path to tackle this:
>
> https://github.com/agraf/u-boot/tree/efi-sandbox
What do you mean by version 2?
>>>
>>> Option 2 is what you called it. It's the only option we have to make
>>> efi binaries work.
>>>
It looks like you've added one patch,
so will you
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
On 06/14/2018 06:13 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 10:07, Alexander Graf wrote:
On 06/14/2018 05:53 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 09:47, Alexander Graf wrote:
Am 14.06.2018 um 17:43 schrieb Simon Glass :
Hi Alex,
On 14 June 2018 at 08:22, Alexander Graf wrote:
Am 14.06.2018 um 16:12 schrieb Simon Glass :
Hi Alex,
On 14 June 2018 at 07:41, Alexander Graf wrote:
On 06/14/2018 02:58 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 04:12, Alexander Graf wrote:
On 06/13/2018 04:37 AM, Simon Glass wrote:
With sandbox the U-Boot code is not mapped into the sandbox memory
range
so does not need to be excluded when allocating EFI memory. Update
the
EFI
memory init code to take account of that.
Also use mapmem instead of a cast to convert a memory address to a
pointer.
Signed-off-by: Simon Glass
---
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Update to use mapmem instead of a cast
lib/efi_loader/efi_memory.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c
b/lib/efi_loader/efi_memory.c
index ec66af98ea..210e49ee8b 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int
pool_type,
efi_uintn_t size, void **buffer)
);
if (r == EFI_SUCCESS) {
- struct efi_pool_allocation *alloc = (void
*)(uintptr_t)t;
+ struct efi_pool_allocation *alloc = map_sysmem(t,
size);
This is still the wrong spot. We don't want the conversion to
happen when
going from an EFI internal address to an allocation, but when
determining
which addresses are usable in the first place.
There seem to be two ways to do this:
1. Record addresses (ulong) in the EFI tables and use map_sysmem()
before returning an address in the allocator
2. Store pointers in the EFI tables using map_sysmem(), then do no
mapping in the allocator
I've gone with option 1 since:
- the EFI addresses are not pointers
- it makes sandbox consistent with other architectures which use an
address rather than a pointer in EFI tables
- we don't normally do pointer arithmetic on the results of
map_sysmem()
- we normally map the memory when it is used rather than when it is
set up
I think you are asking for option 2. I think that would get very
confusing. The addresses where things actually end up in sandbox are
best kept to sandbox.
Overall I feel that you are either missing the point of sandbox
addressing, or don't agree with how it is done. But it does work
pretty well and we don't get a lot of confusion with sandbox
pointers
since we typically use the address until the last moment.
I've assembled a quick tree for version 2. With that I'm able to run
a
simple hello world efi application. Grub refuses to start because it
wants
memory in the low 32bit and also emits random PIO accessing
functions, which
obviously don't work work from user space.
But overall, I think this is the right path to tackle this:
https://github.com/agraf/u-boot/tree/efi-sandbox
What do you mean by version 2?
Option 2 is what you called it. It's the only option we have to make
efi binaries work.
It looks like you've added one patch,
so will you send that to the list?
It's more than 1 patch. And yes, I'll send them.
Anyway, I hope I can convince you of the above, the way sandbox memory
works.
I still dislike option 1 :)
The reason is simple: The efi memory map is available to efi payloads.
It's perfectly legal for them to do a static allocation at a particular
address. We also share a lot of (host) pointers for callbacks and structs
already with efi applications, so there is no real point to have a split
brain situation between u-boot and host pointers.
OK so you mean they don't have to allocate memory before using it? How
then do you make sure that there is no conflict? I thought EFI was an
API?
You allocate it, but payloads expect that the address you pass in as
address you allocate at is the pointer/address that gets returned.
Can you please point me to that? Are you referring to this call?
static efi_status_t EFIAPI efi_get_memory_map_ext(
efi_uintn_t *memory_map_size,
struct efi_mem_desc *memory_map,
efi_uintn_t *map_key,
efi_uintn_t *descriptor_size,
uint32_t *descriptor_version)
If so, we can still support that.
In a nutshell, what you're proposing is that the efi quivalent of
mmap(addr, MAP_FIXED) returns something != addr. That will break things.
I see this function:
efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void
**buffer)
So there appears to be no way for people to specify the address they
want. Is there another call somewhere?
You can call
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
Hi Alex,
On 14 June 2018 at 10:07, Alexander Graf wrote:
> On 06/14/2018 05:53 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 14 June 2018 at 09:47, Alexander Graf wrote:
>>>
>>>
Am 14.06.2018 um 17:43 schrieb Simon Glass :
Hi Alex,
> On 14 June 2018 at 08:22, Alexander Graf wrote:
>
>
>
>> Am 14.06.2018 um 16:12 schrieb Simon Glass :
>>
>> Hi Alex,
>>
On 14 June 2018 at 07:41, Alexander Graf wrote:
On 06/14/2018 02:58 PM, Simon Glass wrote:
Hi Alex,
>> On 14 June 2018 at 04:12, Alexander Graf wrote:
>>
>> On 06/13/2018 04:37 AM, Simon Glass wrote:
>>
>> With sandbox the U-Boot code is not mapped into the sandbox memory
>> range
>> so does not need to be excluded when allocating EFI memory. Update
>> the
>> EFI
>> memory init code to take account of that.
>>
>> Also use mapmem instead of a cast to convert a memory address to a
>> pointer.
>>
>> Signed-off-by: Simon Glass
>> ---
>>
>> Changes in v6: None
>> Changes in v5: None
>> Changes in v4: None
>> Changes in v3: None
>> Changes in v2:
>> - Update to use mapmem instead of a cast
>>
>> lib/efi_loader/efi_memory.c | 31 ++-
>> 1 file changed, 18 insertions(+), 13 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_memory.c
>> b/lib/efi_loader/efi_memory.c
>> index ec66af98ea..210e49ee8b 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -9,6 +9,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> @@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int
>> pool_type,
>> efi_uintn_t size, void **buffer)
>>);
>> if (r == EFI_SUCCESS) {
>> - struct efi_pool_allocation *alloc = (void
>> *)(uintptr_t)t;
>> + struct efi_pool_allocation *alloc = map_sysmem(t,
>> size);
>
>
> This is still the wrong spot. We don't want the conversion to
> happen when
> going from an EFI internal address to an allocation, but when
> determining
> which addresses are usable in the first place.
There seem to be two ways to do this:
1. Record addresses (ulong) in the EFI tables and use map_sysmem()
before returning an address in the allocator
2. Store pointers in the EFI tables using map_sysmem(), then do no
mapping in the allocator
I've gone with option 1 since:
- the EFI addresses are not pointers
- it makes sandbox consistent with other architectures which use an
address rather than a pointer in EFI tables
- we don't normally do pointer arithmetic on the results of
map_sysmem()
- we normally map the memory when it is used rather than when it is
set up
I think you are asking for option 2. I think that would get very
confusing. The addresses where things actually end up in sandbox are
best kept to sandbox.
Overall I feel that you are either missing the point of sandbox
addressing, or don't agree with how it is done. But it does work
pretty well and we don't get a lot of confusion with sandbox
pointers
since we typically use the address until the last moment.
>>>
>>>
>>> I've assembled a quick tree for version 2. With that I'm able to run
>>> a
>>> simple hello world efi application. Grub refuses to start because it
>>> wants
>>> memory in the low 32bit and also emits random PIO accessing
>>> functions, which
>>> obviously don't work work from user space.
>>>
>>> But overall, I think this is the right path to tackle this:
>>>
>>> https://github.com/agraf/u-boot/tree/efi-sandbox
>>
>> What do you mean by version 2?
>
> Option 2 is what you called it. It's the only option we have to make
> efi binaries work.
>
>> It looks like you've added one patch,
>> so will you send that to the list?
>
> It's more than 1 patch. And yes, I'll send them.
>
>> Anyway, I hope I can convince you of the above, the way sandbox memory
>> works.
>
> I still dislike option 1 :)
>
> The reason is simple: The efi memory map is available to efi payloads.
> It's perfectly legal for them to do a static allocation at a particular
> address. We also share a lot of (host) pointers for callbacks and structs
> already with efi applications,
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
On 06/14/2018 05:53 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 09:47, Alexander Graf wrote:
Am 14.06.2018 um 17:43 schrieb Simon Glass :
Hi Alex,
On 14 June 2018 at 08:22, Alexander Graf wrote:
Am 14.06.2018 um 16:12 schrieb Simon Glass :
Hi Alex,
On 14 June 2018 at 07:41, Alexander Graf wrote:
On 06/14/2018 02:58 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 04:12, Alexander Graf wrote:
On 06/13/2018 04:37 AM, Simon Glass wrote:
With sandbox the U-Boot code is not mapped into the sandbox memory range
so does not need to be excluded when allocating EFI memory. Update the
EFI
memory init code to take account of that.
Also use mapmem instead of a cast to convert a memory address to a
pointer.
Signed-off-by: Simon Glass
---
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Update to use mapmem instead of a cast
lib/efi_loader/efi_memory.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index ec66af98ea..210e49ee8b 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int pool_type,
efi_uintn_t size, void **buffer)
);
if (r == EFI_SUCCESS) {
- struct efi_pool_allocation *alloc = (void
*)(uintptr_t)t;
+ struct efi_pool_allocation *alloc = map_sysmem(t, size);
This is still the wrong spot. We don't want the conversion to happen when
going from an EFI internal address to an allocation, but when determining
which addresses are usable in the first place.
There seem to be two ways to do this:
1. Record addresses (ulong) in the EFI tables and use map_sysmem()
before returning an address in the allocator
2. Store pointers in the EFI tables using map_sysmem(), then do no
mapping in the allocator
I've gone with option 1 since:
- the EFI addresses are not pointers
- it makes sandbox consistent with other architectures which use an
address rather than a pointer in EFI tables
- we don't normally do pointer arithmetic on the results of map_sysmem()
- we normally map the memory when it is used rather than when it is set up
I think you are asking for option 2. I think that would get very
confusing. The addresses where things actually end up in sandbox are
best kept to sandbox.
Overall I feel that you are either missing the point of sandbox
addressing, or don't agree with how it is done. But it does work
pretty well and we don't get a lot of confusion with sandbox pointers
since we typically use the address until the last moment.
I've assembled a quick tree for version 2. With that I'm able to run a
simple hello world efi application. Grub refuses to start because it wants
memory in the low 32bit and also emits random PIO accessing functions, which
obviously don't work work from user space.
But overall, I think this is the right path to tackle this:
https://github.com/agraf/u-boot/tree/efi-sandbox
What do you mean by version 2?
Option 2 is what you called it. It's the only option we have to make efi
binaries work.
It looks like you've added one patch,
so will you send that to the list?
It's more than 1 patch. And yes, I'll send them.
Anyway, I hope I can convince you of the above, the way sandbox memory works.
I still dislike option 1 :)
The reason is simple: The efi memory map is available to efi payloads. It's
perfectly legal for them to do a static allocation at a particular address. We
also share a lot of (host) pointers for callbacks and structs already with efi
applications, so there is no real point to have a split brain situation between
u-boot and host pointers.
OK so you mean they don't have to allocate memory before using it? How
then do you make sure that there is no conflict? I thought EFI was an
API?
You allocate it, but payloads expect that the address you pass in as address
you allocate at is the pointer/address that gets returned.
Can you please point me to that? Are you referring to this call?
static efi_status_t EFIAPI efi_get_memory_map_ext(
efi_uintn_t *memory_map_size,
struct efi_mem_desc *memory_map,
efi_uintn_t *map_key,
efi_uintn_t *descriptor_size,
uint32_t *descriptor_version)
If so, we can still support that.
In a nutshell, what you're proposing is that the efi quivalent of mmap(addr,
MAP_FIXED) returns something != addr. That will break things.
I see this function:
efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer)
So there appears to be no way for people to specify the address they
want. Is there another call somewhere?
You can call efi_allocate_pages() with EFI_ALLOCATE_ADDRESS or even
EFI_ALLOCATE_MAX_ADDRESS which you usually base on information you
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
Hi Alex,
On 14 June 2018 at 09:47, Alexander Graf wrote:
>
>
>> Am 14.06.2018 um 17:43 schrieb Simon Glass :
>>
>> Hi Alex,
>>
>>> On 14 June 2018 at 08:22, Alexander Graf wrote:
>>>
>>>
>>>
Am 14.06.2018 um 16:12 schrieb Simon Glass :
Hi Alex,
>> On 14 June 2018 at 07:41, Alexander Graf wrote:
>> On 06/14/2018 02:58 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
On 14 June 2018 at 04:12, Alexander Graf wrote:
On 06/13/2018 04:37 AM, Simon Glass wrote:
With sandbox the U-Boot code is not mapped into the sandbox memory
range
so does not need to be excluded when allocating EFI memory. Update the
EFI
memory init code to take account of that.
Also use mapmem instead of a cast to convert a memory address to a
pointer.
Signed-off-by: Simon Glass
---
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Update to use mapmem instead of a cast
lib/efi_loader/efi_memory.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index ec66af98ea..210e49ee8b 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int pool_type,
efi_uintn_t size, void **buffer)
);
if (r == EFI_SUCCESS) {
- struct efi_pool_allocation *alloc = (void
*)(uintptr_t)t;
+ struct efi_pool_allocation *alloc = map_sysmem(t,
size);
>>>
>>>
>>> This is still the wrong spot. We don't want the conversion to happen
>>> when
>>> going from an EFI internal address to an allocation, but when
>>> determining
>>> which addresses are usable in the first place.
>>
>> There seem to be two ways to do this:
>>
>> 1. Record addresses (ulong) in the EFI tables and use map_sysmem()
>> before returning an address in the allocator
>> 2. Store pointers in the EFI tables using map_sysmem(), then do no
>> mapping in the allocator
>>
>> I've gone with option 1 since:
>>
>> - the EFI addresses are not pointers
>> - it makes sandbox consistent with other architectures which use an
>> address rather than a pointer in EFI tables
>> - we don't normally do pointer arithmetic on the results of map_sysmem()
>> - we normally map the memory when it is used rather than when it is set
>> up
>>
>> I think you are asking for option 2. I think that would get very
>> confusing. The addresses where things actually end up in sandbox are
>> best kept to sandbox.
>>
>> Overall I feel that you are either missing the point of sandbox
>> addressing, or don't agree with how it is done. But it does work
>> pretty well and we don't get a lot of confusion with sandbox pointers
>> since we typically use the address until the last moment.
>
>
> I've assembled a quick tree for version 2. With that I'm able to run a
> simple hello world efi application. Grub refuses to start because it wants
> memory in the low 32bit and also emits random PIO accessing functions,
> which
> obviously don't work work from user space.
>
> But overall, I think this is the right path to tackle this:
>
> https://github.com/agraf/u-boot/tree/efi-sandbox
What do you mean by version 2?
>>>
>>> Option 2 is what you called it. It's the only option we have to make efi
>>> binaries work.
>>>
It looks like you've added one patch,
so will you send that to the list?
>>>
>>> It's more than 1 patch. And yes, I'll send them.
>>>
Anyway, I hope I can convince you of the above, the way sandbox memory
works.
>>>
>>> I still dislike option 1 :)
>>>
>>> The reason is simple: The efi memory map is available to efi payloads. It's
>>> perfectly legal for them to do a static allocation at a particular address.
>>> We also share a lot of (host) pointers for callbacks and structs already
>>> with efi applications, so there is no real point to have a split brain
>>> situation between u-boot and host pointers.
>>
>> OK so you mean they don't have to allocate memory before using it? How
>> then do you make sure that there is no conflict? I thought EFI was an
>> API?
>
> You allocate it, but payloads expect that the address you pass in as address
> you allocate at is the pointer/address that gets returned.
>
Can you please point me to
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
> Am 14.06.2018 um 17:43 schrieb Simon Glass :
>
> Hi Alex,
>
>> On 14 June 2018 at 08:22, Alexander Graf wrote:
>>
>>
>>
>>> Am 14.06.2018 um 16:12 schrieb Simon Glass :
>>>
>>> Hi Alex,
>>>
> On 14 June 2018 at 07:41, Alexander Graf wrote:
> On 06/14/2018 02:58 PM, Simon Glass wrote:
>
> Hi Alex,
>
>>> On 14 June 2018 at 04:12, Alexander Graf wrote:
>>>
>>> On 06/13/2018 04:37 AM, Simon Glass wrote:
>>>
>>> With sandbox the U-Boot code is not mapped into the sandbox memory range
>>> so does not need to be excluded when allocating EFI memory. Update the
>>> EFI
>>> memory init code to take account of that.
>>>
>>> Also use mapmem instead of a cast to convert a memory address to a
>>> pointer.
>>>
>>> Signed-off-by: Simon Glass
>>> ---
>>>
>>> Changes in v6: None
>>> Changes in v5: None
>>> Changes in v4: None
>>> Changes in v3: None
>>> Changes in v2:
>>> - Update to use mapmem instead of a cast
>>>
>>> lib/efi_loader/efi_memory.c | 31 ++-
>>> 1 file changed, 18 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>> index ec66af98ea..210e49ee8b 100644
>>> --- a/lib/efi_loader/efi_memory.c
>>> +++ b/lib/efi_loader/efi_memory.c
>>> @@ -9,6 +9,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> #include
>>> #include
>>> @@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int pool_type,
>>> efi_uintn_t size, void **buffer)
>>> );
>>>if (r == EFI_SUCCESS) {
>>> - struct efi_pool_allocation *alloc = (void
>>> *)(uintptr_t)t;
>>> + struct efi_pool_allocation *alloc = map_sysmem(t, size);
>>
>>
>> This is still the wrong spot. We don't want the conversion to happen when
>> going from an EFI internal address to an allocation, but when determining
>> which addresses are usable in the first place.
>
> There seem to be two ways to do this:
>
> 1. Record addresses (ulong) in the EFI tables and use map_sysmem()
> before returning an address in the allocator
> 2. Store pointers in the EFI tables using map_sysmem(), then do no
> mapping in the allocator
>
> I've gone with option 1 since:
>
> - the EFI addresses are not pointers
> - it makes sandbox consistent with other architectures which use an
> address rather than a pointer in EFI tables
> - we don't normally do pointer arithmetic on the results of map_sysmem()
> - we normally map the memory when it is used rather than when it is set up
>
> I think you are asking for option 2. I think that would get very
> confusing. The addresses where things actually end up in sandbox are
> best kept to sandbox.
>
> Overall I feel that you are either missing the point of sandbox
> addressing, or don't agree with how it is done. But it does work
> pretty well and we don't get a lot of confusion with sandbox pointers
> since we typically use the address until the last moment.
I've assembled a quick tree for version 2. With that I'm able to run a
simple hello world efi application. Grub refuses to start because it wants
memory in the low 32bit and also emits random PIO accessing functions,
which
obviously don't work work from user space.
But overall, I think this is the right path to tackle this:
https://github.com/agraf/u-boot/tree/efi-sandbox
>>>
>>> What do you mean by version 2?
>>
>> Option 2 is what you called it. It's the only option we have to make efi
>> binaries work.
>>
>>> It looks like you've added one patch,
>>> so will you send that to the list?
>>
>> It's more than 1 patch. And yes, I'll send them.
>>
>>>
>>> Anyway, I hope I can convince you of the above, the way sandbox memory
>>> works.
>>
>> I still dislike option 1 :)
>>
>> The reason is simple: The efi memory map is available to efi payloads. It's
>> perfectly legal for them to do a static allocation at a particular address.
>> We also share a lot of (host) pointers for callbacks and structs already
>> with efi applications, so there is no real point to have a split brain
>> situation between u-boot and host pointers.
>
> OK so you mean they don't have to allocate memory before using it? How
> then do you make sure that there is no conflict? I thought EFI was an
> API?
You allocate it, but payloads expect that the address you pass in as address
you allocate at is the pointer/address that gets returned.
In a nutshell, what you're proposing is that the efi quivalent of mmap(addr,
MAP_FIXED) returns something != addr. That will break things.
Alex
>
> Regards,
> Simon
___
U-Boot mailing
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
Hi Alex,
On 14 June 2018 at 08:22, Alexander Graf wrote:
>
>
>
> > Am 14.06.2018 um 16:12 schrieb Simon Glass :
> >
> > Hi Alex,
> >
> >> On 14 June 2018 at 07:41, Alexander Graf wrote:
> >>> On 06/14/2018 02:58 PM, Simon Glass wrote:
> >>>
> >>> Hi Alex,
> >>>
> On 14 June 2018 at 04:12, Alexander Graf wrote:
>
> > On 06/13/2018 04:37 AM, Simon Glass wrote:
> >
> > With sandbox the U-Boot code is not mapped into the sandbox memory range
> > so does not need to be excluded when allocating EFI memory. Update the
> > EFI
> > memory init code to take account of that.
> >
> > Also use mapmem instead of a cast to convert a memory address to a
> > pointer.
> >
> > Signed-off-by: Simon Glass
> > ---
> >
> > Changes in v6: None
> > Changes in v5: None
> > Changes in v4: None
> > Changes in v3: None
> > Changes in v2:
> > - Update to use mapmem instead of a cast
> >
> > lib/efi_loader/efi_memory.c | 31 ++-
> > 1 file changed, 18 insertions(+), 13 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > index ec66af98ea..210e49ee8b 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -9,6 +9,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > @@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int pool_type,
> > efi_uintn_t size, void **buffer)
> >);
> > if (r == EFI_SUCCESS) {
> > - struct efi_pool_allocation *alloc = (void
> > *)(uintptr_t)t;
> > + struct efi_pool_allocation *alloc = map_sysmem(t, size);
>
>
> This is still the wrong spot. We don't want the conversion to happen when
> going from an EFI internal address to an allocation, but when determining
> which addresses are usable in the first place.
> >>>
> >>> There seem to be two ways to do this:
> >>>
> >>> 1. Record addresses (ulong) in the EFI tables and use map_sysmem()
> >>> before returning an address in the allocator
> >>> 2. Store pointers in the EFI tables using map_sysmem(), then do no
> >>> mapping in the allocator
> >>>
> >>> I've gone with option 1 since:
> >>>
> >>> - the EFI addresses are not pointers
> >>> - it makes sandbox consistent with other architectures which use an
> >>> address rather than a pointer in EFI tables
> >>> - we don't normally do pointer arithmetic on the results of map_sysmem()
> >>> - we normally map the memory when it is used rather than when it is set up
> >>>
> >>> I think you are asking for option 2. I think that would get very
> >>> confusing. The addresses where things actually end up in sandbox are
> >>> best kept to sandbox.
> >>>
> >>> Overall I feel that you are either missing the point of sandbox
> >>> addressing, or don't agree with how it is done. But it does work
> >>> pretty well and we don't get a lot of confusion with sandbox pointers
> >>> since we typically use the address until the last moment.
> >>
> >>
> >> I've assembled a quick tree for version 2. With that I'm able to run a
> >> simple hello world efi application. Grub refuses to start because it wants
> >> memory in the low 32bit and also emits random PIO accessing functions,
> >> which
> >> obviously don't work work from user space.
> >>
> >> But overall, I think this is the right path to tackle this:
> >>
> >> https://github.com/agraf/u-boot/tree/efi-sandbox
> >
> > What do you mean by version 2?
>
> Option 2 is what you called it. It's the only option we have to make efi
> binaries work.
>
> > It looks like you've added one patch,
> > so will you send that to the list?
>
> It's more than 1 patch. And yes, I'll send them.
>
> >
> > Anyway, I hope I can convince you of the above, the way sandbox memory
> > works.
>
> I still dislike option 1 :)
>
> The reason is simple: The efi memory map is available to efi payloads. It's
> perfectly legal for them to do a static allocation at a particular address.
> We also share a lot of (host) pointers for callbacks and structs already with
> efi applications, so there is no real point to have a split brain situation
> between u-boot and host pointers.
OK so you mean they don't have to allocate memory before using it? How
then do you make sure that there is no conflict? I thought EFI was an
API?
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
> Am 14.06.2018 um 16:12 schrieb Simon Glass :
>
> Hi Alex,
>
>> On 14 June 2018 at 07:41, Alexander Graf wrote:
>>> On 06/14/2018 02:58 PM, Simon Glass wrote:
>>>
>>> Hi Alex,
>>>
On 14 June 2018 at 04:12, Alexander Graf wrote:
> On 06/13/2018 04:37 AM, Simon Glass wrote:
>
> With sandbox the U-Boot code is not mapped into the sandbox memory range
> so does not need to be excluded when allocating EFI memory. Update the
> EFI
> memory init code to take account of that.
>
> Also use mapmem instead of a cast to convert a memory address to a
> pointer.
>
> Signed-off-by: Simon Glass
> ---
>
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2:
> - Update to use mapmem instead of a cast
>
> lib/efi_loader/efi_memory.c | 31 ++-
> 1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index ec66af98ea..210e49ee8b 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -9,6 +9,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> @@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int pool_type,
> efi_uintn_t size, void **buffer)
>);
> if (r == EFI_SUCCESS) {
> - struct efi_pool_allocation *alloc = (void
> *)(uintptr_t)t;
> + struct efi_pool_allocation *alloc = map_sysmem(t, size);
This is still the wrong spot. We don't want the conversion to happen when
going from an EFI internal address to an allocation, but when determining
which addresses are usable in the first place.
>>>
>>> There seem to be two ways to do this:
>>>
>>> 1. Record addresses (ulong) in the EFI tables and use map_sysmem()
>>> before returning an address in the allocator
>>> 2. Store pointers in the EFI tables using map_sysmem(), then do no
>>> mapping in the allocator
>>>
>>> I've gone with option 1 since:
>>>
>>> - the EFI addresses are not pointers
>>> - it makes sandbox consistent with other architectures which use an
>>> address rather than a pointer in EFI tables
>>> - we don't normally do pointer arithmetic on the results of map_sysmem()
>>> - we normally map the memory when it is used rather than when it is set up
>>>
>>> I think you are asking for option 2. I think that would get very
>>> confusing. The addresses where things actually end up in sandbox are
>>> best kept to sandbox.
>>>
>>> Overall I feel that you are either missing the point of sandbox
>>> addressing, or don't agree with how it is done. But it does work
>>> pretty well and we don't get a lot of confusion with sandbox pointers
>>> since we typically use the address until the last moment.
>>
>>
>> I've assembled a quick tree for version 2. With that I'm able to run a
>> simple hello world efi application. Grub refuses to start because it wants
>> memory in the low 32bit and also emits random PIO accessing functions, which
>> obviously don't work work from user space.
>>
>> But overall, I think this is the right path to tackle this:
>>
>> https://github.com/agraf/u-boot/tree/efi-sandbox
>
> What do you mean by version 2?
Option 2 is what you called it. It's the only option we have to make efi
binaries work.
> It looks like you've added one patch,
> so will you send that to the list?
It's more than 1 patch. And yes, I'll send them.
>
> Anyway, I hope I can convince you of the above, the way sandbox memory works.
I still dislike option 1 :)
The reason is simple: The efi memory map is available to efi payloads. It's
perfectly legal for them to do a static allocation at a particular address. We
also share a lot of (host) pointers for callbacks and structs already with efi
applications, so there is no real point to have a split brain situation between
u-boot and host pointers.
Alex
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
Hi Alex,
On 14 June 2018 at 07:41, Alexander Graf wrote:
> On 06/14/2018 02:58 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 14 June 2018 at 04:12, Alexander Graf wrote:
>>>
>>> On 06/13/2018 04:37 AM, Simon Glass wrote:
With sandbox the U-Boot code is not mapped into the sandbox memory range
so does not need to be excluded when allocating EFI memory. Update the
EFI
memory init code to take account of that.
Also use mapmem instead of a cast to convert a memory address to a
pointer.
Signed-off-by: Simon Glass
---
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Update to use mapmem instead of a cast
lib/efi_loader/efi_memory.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index ec66af98ea..210e49ee8b 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int pool_type,
efi_uintn_t size, void **buffer)
);
if (r == EFI_SUCCESS) {
- struct efi_pool_allocation *alloc = (void
*)(uintptr_t)t;
+ struct efi_pool_allocation *alloc = map_sysmem(t, size);
>>>
>>>
>>> This is still the wrong spot. We don't want the conversion to happen when
>>> going from an EFI internal address to an allocation, but when determining
>>> which addresses are usable in the first place.
>>
>> There seem to be two ways to do this:
>>
>> 1. Record addresses (ulong) in the EFI tables and use map_sysmem()
>> before returning an address in the allocator
>> 2. Store pointers in the EFI tables using map_sysmem(), then do no
>> mapping in the allocator
>>
>> I've gone with option 1 since:
>>
>> - the EFI addresses are not pointers
>> - it makes sandbox consistent with other architectures which use an
>> address rather than a pointer in EFI tables
>> - we don't normally do pointer arithmetic on the results of map_sysmem()
>> - we normally map the memory when it is used rather than when it is set up
>>
>> I think you are asking for option 2. I think that would get very
>> confusing. The addresses where things actually end up in sandbox are
>> best kept to sandbox.
>>
>> Overall I feel that you are either missing the point of sandbox
>> addressing, or don't agree with how it is done. But it does work
>> pretty well and we don't get a lot of confusion with sandbox pointers
>> since we typically use the address until the last moment.
>
>
> I've assembled a quick tree for version 2. With that I'm able to run a
> simple hello world efi application. Grub refuses to start because it wants
> memory in the low 32bit and also emits random PIO accessing functions, which
> obviously don't work work from user space.
>
> But overall, I think this is the right path to tackle this:
>
> https://github.com/agraf/u-boot/tree/efi-sandbox
What do you mean by version 2? It looks like you've added one patch,
so will you send that to the list?
Anyway, I hope I can convince you of the above, the way sandbox memory works.
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
On 06/14/2018 02:58 PM, Simon Glass wrote:
Hi Alex,
On 14 June 2018 at 04:12, Alexander Graf wrote:
On 06/13/2018 04:37 AM, Simon Glass wrote:
With sandbox the U-Boot code is not mapped into the sandbox memory range
so does not need to be excluded when allocating EFI memory. Update the EFI
memory init code to take account of that.
Also use mapmem instead of a cast to convert a memory address to a
pointer.
Signed-off-by: Simon Glass
---
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Update to use mapmem instead of a cast
lib/efi_loader/efi_memory.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index ec66af98ea..210e49ee8b 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int pool_type,
efi_uintn_t size, void **buffer)
);
if (r == EFI_SUCCESS) {
- struct efi_pool_allocation *alloc = (void *)(uintptr_t)t;
+ struct efi_pool_allocation *alloc = map_sysmem(t, size);
This is still the wrong spot. We don't want the conversion to happen when
going from an EFI internal address to an allocation, but when determining
which addresses are usable in the first place.
There seem to be two ways to do this:
1. Record addresses (ulong) in the EFI tables and use map_sysmem()
before returning an address in the allocator
2. Store pointers in the EFI tables using map_sysmem(), then do no
mapping in the allocator
I've gone with option 1 since:
- the EFI addresses are not pointers
- it makes sandbox consistent with other architectures which use an
address rather than a pointer in EFI tables
- we don't normally do pointer arithmetic on the results of map_sysmem()
- we normally map the memory when it is used rather than when it is set up
I think you are asking for option 2. I think that would get very
confusing. The addresses where things actually end up in sandbox are
best kept to sandbox.
Overall I feel that you are either missing the point of sandbox
addressing, or don't agree with how it is done. But it does work
pretty well and we don't get a lot of confusion with sandbox pointers
since we typically use the address until the last moment.
I've assembled a quick tree for version 2. With that I'm able to run a
simple hello world efi application. Grub refuses to start because it
wants memory in the low 32bit and also emits random PIO accessing
functions, which obviously don't work work from user space.
But overall, I think this is the right path to tackle this:
https://github.com/agraf/u-boot/tree/efi-sandbox
Alex
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
Hi Alex,
On 14 June 2018 at 04:12, Alexander Graf wrote:
> On 06/13/2018 04:37 AM, Simon Glass wrote:
>>
>> With sandbox the U-Boot code is not mapped into the sandbox memory range
>> so does not need to be excluded when allocating EFI memory. Update the EFI
>> memory init code to take account of that.
>>
>> Also use mapmem instead of a cast to convert a memory address to a
>> pointer.
>>
>> Signed-off-by: Simon Glass
>> ---
>>
>> Changes in v6: None
>> Changes in v5: None
>> Changes in v4: None
>> Changes in v3: None
>> Changes in v2:
>> - Update to use mapmem instead of a cast
>>
>> lib/efi_loader/efi_memory.c | 31 ++-
>> 1 file changed, 18 insertions(+), 13 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index ec66af98ea..210e49ee8b 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -9,6 +9,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> @@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int pool_type,
>> efi_uintn_t size, void **buffer)
>>);
>> if (r == EFI_SUCCESS) {
>> - struct efi_pool_allocation *alloc = (void *)(uintptr_t)t;
>> + struct efi_pool_allocation *alloc = map_sysmem(t, size);
>
>
> This is still the wrong spot. We don't want the conversion to happen when
> going from an EFI internal address to an allocation, but when determining
> which addresses are usable in the first place.
There seem to be two ways to do this:
1. Record addresses (ulong) in the EFI tables and use map_sysmem()
before returning an address in the allocator
2. Store pointers in the EFI tables using map_sysmem(), then do no
mapping in the allocator
I've gone with option 1 since:
- the EFI addresses are not pointers
- it makes sandbox consistent with other architectures which use an
address rather than a pointer in EFI tables
- we don't normally do pointer arithmetic on the results of map_sysmem()
- we normally map the memory when it is used rather than when it is set up
I think you are asking for option 2. I think that would get very
confusing. The addresses where things actually end up in sandbox are
best kept to sandbox.
Overall I feel that you are either missing the point of sandbox
addressing, or don't agree with how it is done. But it does work
pretty well and we don't get a lot of confusion with sandbox pointers
since we typically use the address until the last moment.
>
>> alloc->num_pages = num_pages;
>> *buffer = alloc->data;
>> }
>> @@ -504,18 +505,22 @@ int efi_memory_init(void)
>> efi_add_known_memory();
>> - /* Add U-Boot */
>> - uboot_start = (gd->start_addr_sp - uboot_stack_size) &
>> ~EFI_PAGE_MASK;
>> - uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
>> - efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA,
>> false);
>> -
>> - /* Add Runtime Services */
>> - runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
>> - runtime_end = (ulong)&__efi_runtime_stop;
>> - runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
>> - runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
>> - efi_add_memory_map(runtime_start, runtime_pages,
>> - EFI_RUNTIME_SERVICES_CODE, false);
>> + if (!IS_ENABLED(CONFIG_SANDBOX)) {
>> + /* Add U-Boot */
>> + uboot_start = (gd->start_addr_sp - uboot_stack_size) &
>> + ~EFI_PAGE_MASK;
>> + uboot_pages = (gd->ram_top - uboot_start) >>
>> EFI_PAGE_SHIFT;
>> + efi_add_memory_map(uboot_start, uboot_pages,
>> EFI_LOADER_DATA,
>> + false);
>
>
> Didn't we want to have these in a function?
Yes but I forgot. Will do for v7.
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
On 06/13/2018 04:37 AM, Simon Glass wrote:
With sandbox the U-Boot code is not mapped into the sandbox memory range
so does not need to be excluded when allocating EFI memory. Update the EFI
memory init code to take account of that.
Also use mapmem instead of a cast to convert a memory address to a
pointer.
Signed-off-by: Simon Glass
---
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Update to use mapmem instead of a cast
lib/efi_loader/efi_memory.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index ec66af98ea..210e49ee8b 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer)
);
if (r == EFI_SUCCESS) {
- struct efi_pool_allocation *alloc = (void *)(uintptr_t)t;
+ struct efi_pool_allocation *alloc = map_sysmem(t, size);
This is still the wrong spot. We don't want the conversion to happen
when going from an EFI internal address to an allocation, but when
determining which addresses are usable in the first place.
alloc->num_pages = num_pages;
*buffer = alloc->data;
}
@@ -504,18 +505,22 @@ int efi_memory_init(void)
efi_add_known_memory();
- /* Add U-Boot */
- uboot_start = (gd->start_addr_sp - uboot_stack_size) & ~EFI_PAGE_MASK;
- uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
- efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA, false);
-
- /* Add Runtime Services */
- runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
- runtime_end = (ulong)&__efi_runtime_stop;
- runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
- runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
- efi_add_memory_map(runtime_start, runtime_pages,
- EFI_RUNTIME_SERVICES_CODE, false);
+ if (!IS_ENABLED(CONFIG_SANDBOX)) {
+ /* Add U-Boot */
+ uboot_start = (gd->start_addr_sp - uboot_stack_size) &
+ ~EFI_PAGE_MASK;
+ uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
+ efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA,
+ false);
Didn't we want to have these in a function?
Alex
+
+ /* Add Runtime Services */
+ runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
+ runtime_end = (ulong)&__efi_runtime_stop;
+ runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
+ runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
+ efi_add_memory_map(runtime_start, runtime_pages,
+ EFI_RUNTIME_SERVICES_CODE, false);
+ }
#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
/* Request a 32bit 64MB bounce buffer region */
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox
With sandbox the U-Boot code is not mapped into the sandbox memory range
so does not need to be excluded when allocating EFI memory. Update the EFI
memory init code to take account of that.
Also use mapmem instead of a cast to convert a memory address to a
pointer.
Signed-off-by: Simon Glass
---
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Update to use mapmem instead of a cast
lib/efi_loader/efi_memory.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index ec66af98ea..210e49ee8b 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t
size, void **buffer)
);
if (r == EFI_SUCCESS) {
- struct efi_pool_allocation *alloc = (void *)(uintptr_t)t;
+ struct efi_pool_allocation *alloc = map_sysmem(t, size);
alloc->num_pages = num_pages;
*buffer = alloc->data;
}
@@ -504,18 +505,22 @@ int efi_memory_init(void)
efi_add_known_memory();
- /* Add U-Boot */
- uboot_start = (gd->start_addr_sp - uboot_stack_size) & ~EFI_PAGE_MASK;
- uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
- efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA, false);
-
- /* Add Runtime Services */
- runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
- runtime_end = (ulong)&__efi_runtime_stop;
- runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
- runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
- efi_add_memory_map(runtime_start, runtime_pages,
- EFI_RUNTIME_SERVICES_CODE, false);
+ if (!IS_ENABLED(CONFIG_SANDBOX)) {
+ /* Add U-Boot */
+ uboot_start = (gd->start_addr_sp - uboot_stack_size) &
+ ~EFI_PAGE_MASK;
+ uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
+ efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA,
+ false);
+
+ /* Add Runtime Services */
+ runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
+ runtime_end = (ulong)&__efi_runtime_stop;
+ runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
+ runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
+ efi_add_memory_map(runtime_start, runtime_pages,
+ EFI_RUNTIME_SERVICES_CODE, false);
+ }
#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
/* Request a 32bit 64MB bounce buffer region */
--
2.18.0.rc1.244.gcf134e6275-goog
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
