grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] efidisk: Respect block_io_protocol buffer alignment


From: Vladimir 'φ-coder/phcoder' Serbinenko
Subject: Re: [PATCH] efidisk: Respect block_io_protocol buffer alignment
Date: Sun, 28 Feb 2016 12:14:21 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.6.0

On 18.02.2016 19:05, Leif Lindholm wrote:
> Returned from the OpenProtocol operation, the grub_efi_block_io_media
> structure contains the io_align field, specifying the minimum alignment
> required for buffers used in any data transfers with the device.
> 
> Make grub_efidisk_readwrite() allocate a temporary buffer, aligned to
> this boundary, if the buffer passed to it does not already meet the
> requirements.
> 
> Also sanity check the io_align field in grub_efidisk_open() for
> power-of-two-ness and bail if invalid.
> 
I have committed this variant to make tests pass, so that I can release
beta3. Optimisations can go in separately after this
> Reported-by: Jeremy Linton <address@hidden>
> ---
> 
> v4 - Improved alignment checking as suggested by Vladimir, and moved
>      the error exit as requested by Andrei.
> 
>  grub-core/disk/efi/efidisk.c | 48 
> +++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
> index 1c00e3e..e4f4c25 100644
> --- a/grub-core/disk/efi/efidisk.c
> +++ b/grub-core/disk/efi/efidisk.c
> @@ -493,8 +493,15 @@ grub_efidisk_open (const char *name, struct grub_disk 
> *disk)
>    m = d->block_io->media;
>    /* FIXME: Probably it is better to store the block size in the disk,
>       and total sectors should be replaced with total blocks.  */
> -  grub_dprintf ("efidisk", "m = %p, last block = %llx, block size = %x\n",
> -             m, (unsigned long long) m->last_block, m->block_size);
> +  grub_dprintf ("efidisk",
> +             "m = %p, last block = %llx, block size = %x, io align = %x\n",
> +             m, (unsigned long long) m->last_block, m->block_size,
> +             m->io_align);
> +
> +  /* Ensure required buffer alignment is a power of two (or is zero). */
> +  if (m->io_align & (m->io_align - 1))
> +    return grub_error (GRUB_ERR_IO, "invalid buffer alignment %d", 
> m->io_align);
> +
>    disk->total_sectors = m->last_block + 1;
>    /* Don't increase this value due to bug in some EFI.  */
>    disk->max_agglomerate = 0xa0000 >> (GRUB_DISK_CACHE_BITS + 
> GRUB_DISK_SECTOR_BITS);
> @@ -524,15 +531,42 @@ grub_efidisk_readwrite (struct grub_disk *disk, 
> grub_disk_addr_t sector,
>  {
>    struct grub_efidisk_data *d;
>    grub_efi_block_io_t *bio;
> +  grub_efi_status_t status;
> +  grub_size_t io_align, num_bytes;
> +  char *aligned_buf;
>  
>    d = disk->data;
>    bio = d->block_io;
>  
> -  return efi_call_5 ((wr ? bio->write_blocks : bio->read_blocks), bio,
> -                  bio->media->media_id,
> -                  (grub_efi_uint64_t) sector,
> -                  (grub_efi_uintn_t) size << disk->log_sector_size,
> -                  buf);
> +  /* Set alignment to 1 if 0 specified */
> +  io_align = bio->media->io_align ? bio->media->io_align : 1;
> +  num_bytes = size << disk->log_sector_size;
> +
> +  if ((grub_addr_t) buf & (io_align - 1))
> +    {
> +      aligned_buf = grub_memalign (io_align, num_bytes);
> +      if (! aligned_buf)
> +     return GRUB_EFI_OUT_OF_RESOURCES;
> +      if (wr)
> +     grub_memcpy (aligned_buf, buf, num_bytes);
> +    }
> +  else
> +    {
> +      aligned_buf = buf;
> +    }
> +
> +  status =  efi_call_5 ((wr ? bio->write_blocks : bio->read_blocks), bio,
> +                     bio->media->media_id, (grub_efi_uint64_t) sector,
> +                     (grub_efi_uintn_t) num_bytes, aligned_buf);
> +
> +  if ((grub_addr_t) buf & (io_align - 1))
> +    {
> +      if (!wr)
> +     grub_memcpy (buf, aligned_buf, num_bytes);
> +      grub_free (aligned_buf);
> +    }
> +
> +  return status;
>  }
>  
>  static grub_err_t
> 


Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]