Re: [PATCH] sparc64: OBP available region contains grub. Start at grub_phys_end

[Eric Snowberg]

> On Feb 13, 2016, at 9:06 AM, Vladimir 'phcoder' Serbinenko <address@hidden> 
> wrote:
> 
> 
> 
> Le Sat, Feb 13, 2016 à 5:03 PM, Eric Snowberg <address@hidden> a écrit :
> 
> > On Feb 12, 2016, at 10:30 PM, Andrei Borzenkov <address@hidden> 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 <address@hidden>
> >> ---
> >> 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
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel