All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sparc64: OBP available region contains grub. Start at grub_phys_end
@ 2016-02-12 21:35 Eric Snowberg
  2016-02-12 22:16 ` Vladimir 'φ-coder/phcoder' Serbinenko
  2016-02-13  5:30 ` Andrei Borzenkov
  0 siblings, 2 replies; 8+ messages in thread
From: Eric Snowberg @ 2016-02-12 21:35 UTC (permalink / raw)
  To: grub-devel; +Cc: Eric Snowberg, daniel.kiper

OBP available region contains grub. Start at grub_phys_end.

This prevents a problem where grub was being overwritten since
grub_phys_start does not start at a zero offset within the memory
map.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 grub-core/loader/sparc64/ieee1275/linux.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/grub-core/loader/sparc64/ieee1275/linux.c b/grub-core/loader/sparc64/ieee1275/linux.c
index d44d7a1..67ef048 100644
--- a/grub-core/loader/sparc64/ieee1275/linux.c
+++ b/grub-core/loader/sparc64/ieee1275/linux.c
@@ -203,20 +203,20 @@ alloc_phys_choose (grub_uint64_t addr, grub_uint64_t len,
   if (addr + ctx->size >= end)
     return 0;
 
-  if (addr >= grub_phys_start && addr < grub_phys_end)
-    {
-      addr = ALIGN_UP (grub_phys_end, FOUR_MB);
-      if (addr + ctx->size >= end)
-	return 0;
-    }
-  if ((addr + ctx->size) >= grub_phys_start
-      && (addr + ctx->size) < grub_phys_end)
+  /* OBP available region contains grub. Start at grub_phys_end. */
+  /* grub_phys_start does not start at the beginning of the memory region */
+  if ((grub_phys_start >= addr && grub_phys_end < end) ||
+      (addr > grub_phys_start && addr < grub_phys_end))
     {
       addr = ALIGN_UP (grub_phys_end, FOUR_MB);
       if (addr + ctx->size >= end)
 	return 0;
     }
 
+  grub_dprintf("loader",
+    "addr = 0x%lx grub_phys_start = 0x%lx grub_phys_end = 0x%lx\n",
+    addr, grub_phys_start, grub_phys_end);
+
   if (loaded)
     {
       grub_addr_t linux_end = ALIGN_UP (linux_paddr + linux_size, FOUR_MB);
-- 
1.7.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] sparc64: OBP available region contains grub. Start at grub_phys_end
  2016-02-12 21:35 [PATCH] sparc64: OBP available region contains grub. Start at grub_phys_end Eric Snowberg
@ 2016-02-12 22:16 ` Vladimir 'φ-coder/phcoder' Serbinenko
  2016-02-13  5:30 ` Andrei Borzenkov
  1 sibling, 0 replies; 8+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2016-02-12 22:16 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 1804 bytes --]

Committed, thanks
On 12.02.2016 22:35, Eric Snowberg wrote:
> OBP available region contains grub. Start at grub_phys_end.
> 
> This prevents a problem where grub was being overwritten since
> grub_phys_start does not start at a zero offset within the memory
> map.
> 
> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> ---
>  grub-core/loader/sparc64/ieee1275/linux.c |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/grub-core/loader/sparc64/ieee1275/linux.c b/grub-core/loader/sparc64/ieee1275/linux.c
> index d44d7a1..67ef048 100644
> --- a/grub-core/loader/sparc64/ieee1275/linux.c
> +++ b/grub-core/loader/sparc64/ieee1275/linux.c
> @@ -203,20 +203,20 @@ alloc_phys_choose (grub_uint64_t addr, grub_uint64_t len,
>    if (addr + ctx->size >= end)
>      return 0;
>  
> -  if (addr >= grub_phys_start && addr < grub_phys_end)
> -    {
> -      addr = ALIGN_UP (grub_phys_end, FOUR_MB);
> -      if (addr + ctx->size >= end)
> -	return 0;
> -    }
> -  if ((addr + ctx->size) >= grub_phys_start
> -      && (addr + ctx->size) < grub_phys_end)
> +  /* OBP available region contains grub. Start at grub_phys_end. */
> +  /* grub_phys_start does not start at the beginning of the memory region */
> +  if ((grub_phys_start >= addr && grub_phys_end < end) ||
> +      (addr > grub_phys_start && addr < grub_phys_end))
>      {
>        addr = ALIGN_UP (grub_phys_end, FOUR_MB);
>        if (addr + ctx->size >= end)
>  	return 0;
>      }
>  
> +  grub_dprintf("loader",
> +    "addr = 0x%lx grub_phys_start = 0x%lx grub_phys_end = 0x%lx\n",
> +    addr, grub_phys_start, grub_phys_end);
> +
>    if (loaded)
>      {
>        grub_addr_t linux_end = ALIGN_UP (linux_paddr + linux_size, FOUR_MB);
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] sparc64: OBP available region contains grub. Start at grub_phys_end
  2016-02-12 21:35 [PATCH] sparc64: OBP available region contains grub. Start at grub_phys_end Eric Snowberg
  2016-02-12 22:16 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2016-02-13  5:30 ` Andrei Borzenkov
  2016-02-13 13:56   ` Vladimir 'phcoder' Serbinenko
  2016-02-13 16:03   ` Eric Snowberg
  1 sibling, 2 replies; 8+ messages in thread
From: Andrei Borzenkov @ 2016-02-13  5:30 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Eric Snowberg, daniel.kiper

13.02.2016 00:35, Eric Snowberg пишет:
> OBP available region contains grub. Start at grub_phys_end.
> 
> This prevents a problem where grub was being overwritten since
> grub_phys_start does not start at a zero offset within the memory
> map.
> 
> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> ---
>  grub-core/loader/sparc64/ieee1275/linux.c |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/grub-core/loader/sparc64/ieee1275/linux.c b/grub-core/loader/sparc64/ieee1275/linux.c
> index d44d7a1..67ef048 100644
> --- a/grub-core/loader/sparc64/ieee1275/linux.c
> +++ b/grub-core/loader/sparc64/ieee1275/linux.c
> @@ -203,20 +203,20 @@ alloc_phys_choose (grub_uint64_t addr, grub_uint64_t len,
>    if (addr + ctx->size >= end)
>      return 0;
>  
> -  if (addr >= grub_phys_start && addr < grub_phys_end)
> -    {
> -      addr = ALIGN_UP (grub_phys_end, FOUR_MB);
> -      if (addr + ctx->size >= end)
> -	return 0;
> -    }
> -  if ((addr + ctx->size) >= grub_phys_start
> -      && (addr + ctx->size) < grub_phys_end)
> +  /* OBP available region contains grub. Start at grub_phys_end. */
> +  /* grub_phys_start does not start at the beginning of the memory region */
> +  if ((grub_phys_start >= addr && grub_phys_end < end) ||
> +      (addr > grub_phys_start && addr < grub_phys_end))

What about

a) overlapping regions?

addr < grub_phys_start < end < grub_phys_end

b) is is possible for requested region to fit before grub?

addr < end < grub_phys_start

>      {
>        addr = ALIGN_UP (grub_phys_end, FOUR_MB);
>        if (addr + ctx->size >= end)
>  	return 0;
>      }
>  
> +  grub_dprintf("loader",
> +    "addr = 0x%lx grub_phys_start = 0x%lx grub_phys_end = 0x%lx\n",
> +    addr, grub_phys_start, grub_phys_end);
> +
>    if (loaded)
>      {
>        grub_addr_t linux_end = ALIGN_UP (linux_paddr + linux_size, FOUR_MB);
> 



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] sparc64: OBP available region contains grub. Start at grub_phys_end
  2016-02-13  5:30 ` Andrei Borzenkov
@ 2016-02-13 13:56   ` Vladimir 'phcoder' Serbinenko
  2016-02-13 14:49     ` Andrei Borzenkov
  2016-02-13 16:03   ` Eric Snowberg
  1 sibling, 1 reply; 8+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2016-02-13 13:56 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Eric Snowberg, daniel.kiper


[-- Attachment #1.1: Type: text/plain, Size: 2770 bytes --]

Le sam. 13 févr. 2016 06:30, Andrei Borzenkov <arvidjaar@gmail.com> a
écrit :

> 13.02.2016 00:35, Eric Snowberg пишет:
> > OBP available region contains grub. Start at grub_phys_end.
> >
> > This prevents a problem where grub was being overwritten since
> > grub_phys_start does not start at a zero offset within the memory
> > map.
> >
> > Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> > ---
> >  grub-core/loader/sparc64/ieee1275/linux.c |   16 ++++++++--------
> >  1 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/grub-core/loader/sparc64/ieee1275/linux.c
> b/grub-core/loader/sparc64/ieee1275/linux.c
> > index d44d7a1..67ef048 100644
> > --- a/grub-core/loader/sparc64/ieee1275/linux.c
> > +++ b/grub-core/loader/sparc64/ieee1275/linux.c
> > @@ -203,20 +203,20 @@ alloc_phys_choose (grub_uint64_t addr,
> grub_uint64_t len,
> >    if (addr + ctx->size >= end)
> >      return 0;
> >
> > -  if (addr >= grub_phys_start && addr < grub_phys_end)
> > -    {
> > -      addr = ALIGN_UP (grub_phys_end, FOUR_MB);
> > -      if (addr + ctx->size >= end)
> > -     return 0;
> > -    }
> > -  if ((addr + ctx->size) >= grub_phys_start
> > -      && (addr + ctx->size) < grub_phys_end)
> > +  /* OBP available region contains grub. Start at grub_phys_end. */
> > +  /* grub_phys_start does not start at the beginning of the memory
> region */
> > +  if ((grub_phys_start >= addr && grub_phys_end < end) ||
> > +      (addr > grub_phys_start && addr < grub_phys_end))
>
> What about
>
> a) overlapping regions?
>
> addr < grub_phys_start < end < grub_phys_end
>
> Good catch. Condition should have been
grub_phys_start >= addr && grub_phys_start < end) || (addr >
grub_phys_start && addr < grub_phys_end))
My pattern matching algorithm failed and I thought it was standard check
for segment intersection but it wasn't. I'll fix this


>
> b) is is possible for requested region to fit before grub?
>
There is only 16KiB of memory before GRUB but thinking about it again, it's
16 KiB of virtual memory, not physical. What about attached patch?

>
>
> addr < end < grub_phys_start
>
> >      {
> >        addr = ALIGN_UP (grub_phys_end, FOUR_MB);
> >        if (addr + ctx->size >= end)
> >       return 0;
> >      }
> >
> > +  grub_dprintf("loader",
> > +    "addr = 0x%lx grub_phys_start = 0x%lx grub_phys_end = 0x%lx\n",
> > +    addr, grub_phys_start, grub_phys_end);
> > +
> >    if (loaded)
> >      {
> >        grub_addr_t linux_end = ALIGN_UP (linux_paddr + linux_size,
> FOUR_MB);
> >
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #1.2: Type: text/html, Size: 4343 bytes --]

[-- Attachment #2: filetree.diff (1) --]
[-- Type: application/octet-stream, Size: 838 bytes --]

diff --git a/grub-core/loader/sparc64/ieee1275/linux.c b/grub-core/loader/sparc64/ieee1275/linux.c
index 67ef048..973e49e 100644
--- a/grub-core/loader/sparc64/ieee1275/linux.c
+++ b/grub-core/loader/sparc64/ieee1275/linux.c
@@ -205,10 +205,13 @@
 
   /* OBP available region contains grub. Start at grub_phys_end. */
   /* grub_phys_start does not start at the beginning of the memory region */
-  if ((grub_phys_start >= addr && grub_phys_end < end) ||
+  if ((grub_phys_start >= addr && grub_phys_start < end) ||
       (addr > grub_phys_start && addr < grub_phys_end))
     {
-      addr = ALIGN_UP (grub_phys_end, FOUR_MB);
+      if (addr + ctx->size <= grub_phys_start)
+        end = grub_phys_start;
+      else
+        addr = ALIGN_UP (grub_phys_end, FOUR_MB);
       if (addr + ctx->size >= end)
 	return 0;
     }

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] sparc64: OBP available region contains grub. Start at grub_phys_end
  2016-02-13 13:56   ` Vladimir 'phcoder' Serbinenko
@ 2016-02-13 14:49     ` Andrei Borzenkov
  0 siblings, 0 replies; 8+ messages in thread
From: Andrei Borzenkov @ 2016-02-13 14:49 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Eric Snowberg, daniel.kiper

13.02.2016 16:56, Vladimir 'phcoder' Serbinenko пишет:
> Le sam. 13 févr. 2016 06:30, Andrei Borzenkov <arvidjaar@gmail.com> a
> écrit :
> 
>> 13.02.2016 00:35, Eric Snowberg пишет:
>>> OBP available region contains grub. Start at grub_phys_end.
>>>
>>> This prevents a problem where grub was being overwritten since
>>> grub_phys_start does not start at a zero offset within the memory
>>> map.
>>>
>>> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
>>> ---
>>>  grub-core/loader/sparc64/ieee1275/linux.c |   16 ++++++++--------
>>>  1 files changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/grub-core/loader/sparc64/ieee1275/linux.c
>> b/grub-core/loader/sparc64/ieee1275/linux.c
>>> index d44d7a1..67ef048 100644
>>> --- a/grub-core/loader/sparc64/ieee1275/linux.c
>>> +++ b/grub-core/loader/sparc64/ieee1275/linux.c
>>> @@ -203,20 +203,20 @@ alloc_phys_choose (grub_uint64_t addr,
>> grub_uint64_t len,
>>>    if (addr + ctx->size >= end)
>>>      return 0;
>>>
>>> -  if (addr >= grub_phys_start && addr < grub_phys_end)
>>> -    {
>>> -      addr = ALIGN_UP (grub_phys_end, FOUR_MB);
>>> -      if (addr + ctx->size >= end)
>>> -     return 0;
>>> -    }
>>> -  if ((addr + ctx->size) >= grub_phys_start
>>> -      && (addr + ctx->size) < grub_phys_end)
>>> +  /* OBP available region contains grub. Start at grub_phys_end. */
>>> +  /* grub_phys_start does not start at the beginning of the memory
>> region */
>>> +  if ((grub_phys_start >= addr && grub_phys_end < end) ||
>>> +      (addr > grub_phys_start && addr < grub_phys_end))
>>
>> What about
>>
>> a) overlapping regions?
>>
>> addr < grub_phys_start < end < grub_phys_end
>>
>> Good catch. Condition should have been
> grub_phys_start >= addr && grub_phys_start < end) || (addr >
> grub_phys_start && addr < grub_phys_end))
> My pattern matching algorithm failed and I thought it was standard check
> for segment intersection but it wasn't. I'll fix this
> 
> 
>>
>> b) is is possible for requested region to fit before grub?
>>
> There is only 16KiB of memory before GRUB but thinking about it again, it's
> 16 KiB of virtual memory, not physical. What about attached patch?
> 

As far as I understand what it does looks OK.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] sparc64: OBP available region contains grub. Start at grub_phys_end
  2016-02-13  5:30 ` Andrei Borzenkov
  2016-02-13 13:56   ` Vladimir 'phcoder' Serbinenko
@ 2016-02-13 16:03   ` Eric Snowberg
  2016-02-13 16:06     ` Vladimir 'phcoder' Serbinenko
  1 sibling, 1 reply; 8+ messages in thread
From: Eric Snowberg @ 2016-02-13 16:03 UTC (permalink / raw)
  To: Andrei Borzenkov; +Cc: The development of GNU GRUB, daniel.kiper


> On Feb 12, 2016, at 10:30 PM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 
> 13.02.2016 00:35, Eric Snowberg пишет:
>> OBP available region contains grub. Start at grub_phys_end.
>> 
>> This prevents a problem where grub was being overwritten since
>> grub_phys_start does not start at a zero offset within the memory
>> map.
>> 
>> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
>> ---
>> grub-core/loader/sparc64/ieee1275/linux.c |   16 ++++++++--------
>> 1 files changed, 8 insertions(+), 8 deletions(-)
>> 
>> diff --git a/grub-core/loader/sparc64/ieee1275/linux.c b/grub-core/loader/sparc64/ieee1275/linux.c
>> index d44d7a1..67ef048 100644
>> --- a/grub-core/loader/sparc64/ieee1275/linux.c
>> +++ b/grub-core/loader/sparc64/ieee1275/linux.c
>> @@ -203,20 +203,20 @@ alloc_phys_choose (grub_uint64_t addr, grub_uint64_t len,
>>   if (addr + ctx->size >= end)
>>     return 0;
>> 
>> -  if (addr >= grub_phys_start && addr < grub_phys_end)
>> -    {
>> -      addr = ALIGN_UP (grub_phys_end, FOUR_MB);
>> -      if (addr + ctx->size >= end)
>> -	return 0;
>> -    }
>> -  if ((addr + ctx->size) >= grub_phys_start
>> -      && (addr + ctx->size) < grub_phys_end)
>> +  /* OBP available region contains grub. Start at grub_phys_end. */
>> +  /* grub_phys_start does not start at the beginning of the memory region */
>> +  if ((grub_phys_start >= addr && grub_phys_end < end) ||
>> +      (addr > grub_phys_start && addr < grub_phys_end))
> 
> What about
> 
> a) overlapping regions?
> 
> addr < grub_phys_start < end < grub_phys_end
> 
> b) is is possible for requested region to fit before grub?
> 
> addr < end < grub_phys_start
> 

On SPARC, there is always an 8k offset from the beginning of memory to grub_phys_start. I had intentionally skipped this memory for the 2 cases above.


>>     {
>>       addr = ALIGN_UP (grub_phys_end, FOUR_MB);
>>       if (addr + ctx->size >= end)
>> 	return 0;
>>     }
>> 
>> +  grub_dprintf("loader",
>> +    "addr = 0x%lx grub_phys_start = 0x%lx grub_phys_end = 0x%lx\n",
>> +    addr, grub_phys_start, grub_phys_end);
>> +
>>   if (loaded)
>>     {
>>       grub_addr_t linux_end = ALIGN_UP (linux_paddr + linux_size, FOUR_MB);



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] sparc64: OBP available region contains grub. Start at grub_phys_end
  2016-02-13 16:03   ` Eric Snowberg
@ 2016-02-13 16:06     ` Vladimir 'phcoder' Serbinenko
  2016-02-13 23:26       ` Eric Snowberg
  0 siblings, 1 reply; 8+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2016-02-13 16:06 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 2790 bytes --]

Le Sat, Feb 13, 2016 à 5:03 PM, Eric Snowberg <eric.snowberg@oracle.com> a
écrit :

>
> > On Feb 12, 2016, at 10:30 PM, Andrei Borzenkov <arvidjaar@gmail.com>
> wrote:
> >
> > 13.02.2016 00:35, Eric Snowberg пишет:
> >> OBP available region contains grub. Start at grub_phys_end.
> >>
> >> This prevents a problem where grub was being overwritten since
> >> grub_phys_start does not start at a zero offset within the memory
> >> map.
> >>
> >> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> >> ---
> >> grub-core/loader/sparc64/ieee1275/linux.c |   16 ++++++++--------
> >> 1 files changed, 8 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/grub-core/loader/sparc64/ieee1275/linux.c
> b/grub-core/loader/sparc64/ieee1275/linux.c
> >> index d44d7a1..67ef048 100644
> >> --- a/grub-core/loader/sparc64/ieee1275/linux.c
> >> +++ b/grub-core/loader/sparc64/ieee1275/linux.c
> >> @@ -203,20 +203,20 @@ alloc_phys_choose (grub_uint64_t addr,
> grub_uint64_t len,
> >>   if (addr + ctx->size >= end)
> >>     return 0;
> >>
> >> -  if (addr >= grub_phys_start && addr < grub_phys_end)
> >> -    {
> >> -      addr = ALIGN_UP (grub_phys_end, FOUR_MB);
> >> -      if (addr + ctx->size >= end)
> >> -    return 0;
> >> -    }
> >> -  if ((addr + ctx->size) >= grub_phys_start
> >> -      && (addr + ctx->size) < grub_phys_end)
> >> +  /* OBP available region contains grub. Start at grub_phys_end. */
> >> +  /* grub_phys_start does not start at the beginning of the memory
> region */
> >> +  if ((grub_phys_start >= addr && grub_phys_end < end) ||
> >> +      (addr > grub_phys_start && addr < grub_phys_end))
> >
> > What about
> >
> > a) overlapping regions?
> >
> > addr < grub_phys_start < end < grub_phys_end
> >
> > b) is is possible for requested region to fit before grub?
> >
> > addr < end < grub_phys_start
> >
>
> On SPARC, there is always an 8k offset from the beginning of memory to
> grub_phys_start. I had intentionally skipped this memory for the 2 cases
> above.
>
> Isn't this an internal implementation detail of OBP? It has to be 8k in
virtual memory but why does it have to be in physical memory?

>
> >>     {
> >>       addr = ALIGN_UP (grub_phys_end, FOUR_MB);
> >>       if (addr + ctx->size >= end)
> >>      return 0;
> >>     }
> >>
> >> +  grub_dprintf("loader",
> >> +    "addr = 0x%lx grub_phys_start = 0x%lx grub_phys_end = 0x%lx\n",
> >> +    addr, grub_phys_start, grub_phys_end);
> >> +
> >>   if (loaded)
> >>     {
> >>       grub_addr_t linux_end = ALIGN_UP (linux_paddr + linux_size,
> FOUR_MB);
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 4053 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] sparc64: OBP available region contains grub. Start at grub_phys_end
  2016-02-13 16:06     ` Vladimir 'phcoder' Serbinenko
@ 2016-02-13 23:26       ` Eric Snowberg
  0 siblings, 0 replies; 8+ messages in thread
From: Eric Snowberg @ 2016-02-13 23:26 UTC (permalink / raw)
  To: The development of GNU GRUB


> On Feb 13, 2016, at 9:06 AM, Vladimir 'phcoder' Serbinenko <phcoder@gmail.com> wrote:
> 
> 
> 
> Le Sat, Feb 13, 2016 à 5:03 PM, Eric Snowberg <eric.snowberg@oracle.com> a écrit :
> 
> > On Feb 12, 2016, at 10:30 PM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> >
> > 13.02.2016 00:35, Eric Snowberg пишет:
> >> OBP available region contains grub. Start at grub_phys_end.
> >>
> >> This prevents a problem where grub was being overwritten since
> >> grub_phys_start does not start at a zero offset within the memory
> >> map.
> >>
> >> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> >> ---
> >> grub-core/loader/sparc64/ieee1275/linux.c |   16 ++++++++--------
> >> 1 files changed, 8 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/grub-core/loader/sparc64/ieee1275/linux.c b/grub-core/loader/sparc64/ieee1275/linux.c
> >> index d44d7a1..67ef048 100644
> >> --- a/grub-core/loader/sparc64/ieee1275/linux.c
> >> +++ b/grub-core/loader/sparc64/ieee1275/linux.c
> >> @@ -203,20 +203,20 @@ alloc_phys_choose (grub_uint64_t addr, grub_uint64_t len,
> >>   if (addr + ctx->size >= end)
> >>     return 0;
> >>
> >> -  if (addr >= grub_phys_start && addr < grub_phys_end)
> >> -    {
> >> -      addr = ALIGN_UP (grub_phys_end, FOUR_MB);
> >> -      if (addr + ctx->size >= end)
> >> -    return 0;
> >> -    }
> >> -  if ((addr + ctx->size) >= grub_phys_start
> >> -      && (addr + ctx->size) < grub_phys_end)
> >> +  /* OBP available region contains grub. Start at grub_phys_end. */
> >> +  /* grub_phys_start does not start at the beginning of the memory region */
> >> +  if ((grub_phys_start >= addr && grub_phys_end < end) ||
> >> +      (addr > grub_phys_start && addr < grub_phys_end))
> >
> > What about
> >
> > a) overlapping regions?
> >
> > addr < grub_phys_start < end < grub_phys_end
> >
> > b) is is possible for requested region to fit before grub?
> >
> > addr < end < grub_phys_start
> >
> 
> On SPARC, there is always an 8k offset from the beginning of memory to grub_phys_start. I had intentionally skipped this memory for the 2 cases above.
> 
> Isn't this an internal implementation detail of OBP? It has to be 8k in virtual memory but why does it have to be in physical memory? 
> 

My understanding is this is OBP’s internal implementation, I’m not sure the history behind it.  I’ve verified this is the same on 10 year old hardware as it is on current generation hardware.  This bug (that this patch fixes) will only show up on older machines though, since there is typically only one large memory map.  The newer machines don’t see this do to the way grub_machine_mmap_iterate finds available memory.  The newer machines hit the other memory bug for which I submitted a patch.

> >>     {
> >>       addr = ALIGN_UP (grub_phys_end, FOUR_MB);
> >>       if (addr + ctx->size >= end)
> >>      return 0;
> >>     }
> >>
> >> +  grub_dprintf("loader",
> >> +    "addr = 0x%lx grub_phys_start = 0x%lx grub_phys_end = 0x%lx\n",
> >> +    addr, grub_phys_start, grub_phys_end);
> >> +
> >>   if (loaded)
> >>     {
> >>       grub_addr_t linux_end = ALIGN_UP (linux_paddr + linux_size, FOUR_MB);
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-02-13 23:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-12 21:35 [PATCH] sparc64: OBP available region contains grub. Start at grub_phys_end Eric Snowberg
2016-02-12 22:16 ` Vladimir 'φ-coder/phcoder' Serbinenko
2016-02-13  5:30 ` Andrei Borzenkov
2016-02-13 13:56   ` Vladimir 'phcoder' Serbinenko
2016-02-13 14:49     ` Andrei Borzenkov
2016-02-13 16:03   ` Eric Snowberg
2016-02-13 16:06     ` Vladimir 'phcoder' Serbinenko
2016-02-13 23:26       ` Eric Snowberg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.