Re: [PATCH 02/15] sparc64: Add blocklist GPT support for SPARC

[Eric Snowberg]

> On May 10, 2017, at 6:40 PM, Vladimir 'phcoder' Serbinenko <address@hidden> 
> wrote:
> 
> 
> 
> On Thu, May 11, 2017, 02:37 Vladimir 'phcoder' Serbinenko <address@hidden> 
> wrote:
> 
> 
> On Thu, May 11, 2017, 01:21 Eric Snowberg <address@hidden> wrote:
> 
> > On May 10, 2017, at 4:42 PM, Vladimir 'phcoder' Serbinenko <address@hidden> 
> > wrote:
> >
> >
> >
> > On Wed, Jun 29, 2016, 23:47 Eric Snowberg <address@hidden> wrote:
> > Add block-list GPT support for SPARC.  The OBP "load" and "boot" methods
> > are partition aware and neither command can see the partition table. Also
> > neither command can address the entire physical disk. When the install 
> > happens,
> > grub generates the block-list entries based on the beginning of the physical
> > disk, not the beginning of the parition. This patch fixes the block-list
> > entries so they match what OBP expects during boot for a GPT disk.
> >
> > T5 and above now supports GPT as well as VTOC.
> >
> > This patch has been tested on T5-2 and newer SPARC systems.
> >
> > Signed-off-by: Eric Snowberg <address@hidden>
> > ---
> >  grub-core/osdep/linux/blocklist.c |    5 +++++
> >  util/setup.c                      |   12 +++++++++---
> >  2 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/grub-core/osdep/linux/blocklist.c 
> > b/grub-core/osdep/linux/blocklist.c
> > index c77d608..caf8d4e 100644
> > --- a/grub-core/osdep/linux/blocklist.c
> > +++ b/grub-core/osdep/linux/blocklist.c
> > @@ -58,6 +58,11 @@ grub_install_get_blocklist (grub_device_t root_dev,
> >    struct fiemap fie1;
> >    int fd;
> >
> > +#ifdef __sparc__
> > +  if (grub_strstr (container->partmap->name, "gpt"))
> > +    container_start = 0;
> > +#endif
> > +
> > This makes ifdef conditional on platform of the tool, not of the binaries 
> > and they can be different.
> 
> Exactly, they are different, that is why the ifdef was added for SPARC.  The 
> tool was putting the wrong value into the binary.
> No. What I mean is that someone with x86 cpu fan prepare a disk for sparc64. 
> He would compile tools for x86 and binaries for sparc64. With your code tool 
> will end up with wrong offsets.
> 
> > Also there are several implementations of blocklist retrieving for 
> > different platform. This condition needs to be detected and adjusted for in 
> > the caller (setup)
> 
> How do you recommend I adjusted it in the setup when container_start is 
> obtained from grub_partition_get_start?
> Just subtract partition offset in save_blocklists or its caller
> Sorry, I meant *not* its caller

Ok, I’ll make that change and send an updated patch.

> 
> 
> >    /* Write the first two sectors of the core image onto the disk.  */
> >    grub_util_info ("opening the core image `%s'", core_path);
> >    fd = open (core_path, O_RDONLY);
> > diff --git a/util/setup.c b/util/setup.c
> > index 8aa5a39..5908498 100644
> > --- a/util/setup.c
> > +++ b/util/setup.c
> > @@ -721,15 +721,21 @@ unable_to_embed:
> >    {
> >      char *buf, *ptr = core_img;
> >      size_t len = core_size;
> > -    grub_uint64_t blk;
> > +    grub_uint64_t blk, offset = 0;
> >      grub_partition_t container = core_dev->disk->partition;
> >      grub_err_t err;
> >
> >      core_dev->disk->partition = 0;
> > +#ifdef GRUB_SETUP_SPARC64
> > +    {
> > +      if (grub_strstr (container->partmap->name, "gpt"))
> > +        offset = grub_partition_get_start (container);
> > +    }
> > +#endif
> >
> >      buf = xmalloc (core_size);
> >      blk = bl.first_sector;
> > -    err = grub_disk_read (core_dev->disk, blk, 0, GRUB_DISK_SECTOR_SIZE, 
> > buf);
> > +    err = grub_disk_read (core_dev->disk, blk + offset, 0, 
> > GRUB_DISK_SECTOR_SIZE, buf);
> >      if (err)
> >        grub_util_error (_("cannot read `%s': %s"), core_dev->disk->name,
> >                        grub_errmsg);
> > @@ -748,7 +754,7 @@ unable_to_embed:
> >         if (cur > len)
> >           cur = len;
> >
> > -       err = grub_disk_read (core_dev->disk, blk, 0, cur, buf);
> > +       err = grub_disk_read (core_dev->disk, blk + offset, 0, cur, buf);
> >         if (err)
> >           grub_util_error (_("cannot read `%s': %s"), core_dev->disk->name,
> >                            grub_errmsg);
> > --
> > 1.7.1
> >
> >
> > _______________________________________________
> > 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
> 
> 
> _______________________________________________
> 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