From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1aW4tU-0006SI-Io for mharc-grub-devel@gnu.org; Wed, 17 Feb 2016 11:24:00 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50538) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aW4tP-0006RF-EL for grub-devel@gnu.org; Wed, 17 Feb 2016 11:23:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aW4tN-0006qA-RN for grub-devel@gnu.org; Wed, 17 Feb 2016 11:23:55 -0500 Received: from mail-wm0-x233.google.com ([2a00:1450:400c:c09::233]:35422) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aW4tN-0006ps-Hb for grub-devel@gnu.org; Wed, 17 Feb 2016 11:23:53 -0500 Received: by mail-wm0-x233.google.com with SMTP id c200so222287734wme.0 for ; Wed, 17 Feb 2016 08:23:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-type; bh=L5P5dYLcGsxB9oUGjgrKLFyHmzqNvd0YS/+guDMmGmo=; b=tQqQIsLf6vxDw2/kTwcjcQjXBMv0O5a/Mi+tijEFoi+3Rq7Hd3MNJuGtD156Q9m2eu pXLsx+Z4CY/jk7zE00qrSs0IUv9RF9H79cRSSOKGCsLok3d+SxjHPBuKTY/2a2GMdULS 84QLC90e/aYBJq1AepVjAsOjm2tpB+cjv0Rkqzc5f5DL7DjaEnCdtMA91d0kFtaQ2h68 CFBMsDlhI1BDdwLmABcf2QE4DI03EZ3rMWbchGFF+iEjJIAvwklBOpcd7bbHktfnU3EX haZvCN3vGBw3kspbfADr3cD5NGndm1PDMmkIZMTfj+jfWF3CGK1p1SsuPFJASWQ7XMQj yZJw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-type; bh=L5P5dYLcGsxB9oUGjgrKLFyHmzqNvd0YS/+guDMmGmo=; b=Z88TDUlrsJ3ooa7DlyQSfXNBXZvNcdl8lS1oHfbUVozE1YLgiri5d14XWTYBsEOoUn 1twWEN/Jy4hrTx8HxWA6w7LbT/CH+rE7bTqvfNqKLqm+Q5e2w/7W9ofeureYdJMbutyS y76LGUxjDGLSeWnpE1uBm0v9hIZVD/fnrv37CKSks/qAaLQK0PSiuRbEwieNfN8IeIyp D6kIGkkdoX0pIfMsOVDR8X1ylTifMXODbgL2niZuEivrRzp5qS6t75x3KWdSk8B1Wy2r yM5DM2EN2+gdrupHz9dTxFZKO9kSuJlYO/LcWprcFWeDnsd65sxcDcZQEtAzkgN/fxBb k9kw== X-Gm-Message-State: AG10YOTARosukV00yAsp8VzspuSXqRUb0vNjLUvYN06Eaa4AdtI88bSJM/P5/KBUbXRUGg== X-Received: by 10.28.32.147 with SMTP id g141mr26158058wmg.19.1455726232664; Wed, 17 Feb 2016 08:23:52 -0800 (PST) Received: from ?IPv6:2620:0:105f:fd00:a2a8:cdff:fe64:b3b5? ([2620:0:105f:fd00:a2a8:cdff:fe64:b3b5]) by smtp.gmail.com with ESMTPSA id za6sm2257719wjc.18.2016.02.17.08.23.51 for (version=TLSv1/SSLv3 cipher=OTHER); Wed, 17 Feb 2016 08:23:51 -0800 (PST) Subject: Re: Respect EFI block-io buffer alignment To: The development of GNU GRUB References: <20160217154825.GD1159@bivouac.eciton.net> From: =?UTF-8?Q?Vladimir_'=cf=86-coder/phcoder'_Serbinenko?= X-Enigmail-Draft-Status: N1110 Message-ID: <56C49E90.3070307@gmail.com> Date: Wed, 17 Feb 2016 17:23:44 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.6.0 MIME-Version: 1.0 In-Reply-To: <20160217154825.GD1159@bivouac.eciton.net> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="OiHwF7KBVVTeJfFi1fe5b1OaJUPd4wMrH" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:400c:c09::233 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Feb 2016 16:23:59 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --OiHwF7KBVVTeJfFi1fe5b1OaJUPd4wMrH Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 17.02.2016 16:48, Leif Lindholm wrote: > This resolves a complete failure to access devices connected to the > SATA port on the ARM ltd. Juno platform (apart from a violation of the > UEFI block io protocol). >=20 > The below is a bit of a hack, but I'd like some feedback on preferred > solution before over(or under)engineering something. >=20 > As far as I can tell, a struct_disk is only ever allocated in > kern/disk.c, using grub_zalloc(). So the only reason for the horrid > ifdefs is that there is no grub_memalign for EMU. >=20 > Do I: > - Keep the ifdefs? > - Implement grub_memalign() for EMU? You could insipire by grub_osdep_dl_memalign > - Something else? >=20 The code as-is will not work. Buf is passed from external call to grub_disk_read and grub_disk_read tries to read in-place whenever possible. There are 2 cases in current codebase when we need a special buffer: usbms and ahci. In both cases corresponding disk routines allocate corresponding temporary buffer. While it would be nicer to avoid such a buffer, it will result in messy code and I doubt there is any speed benefit. So feel free to create temporary buffer or prove me wrong about speed difference. > / > Leif >=20 >>>From 2d8b7ae2dd4c639252517e9a1df783ed0564c112 Mon Sep 17 00:00:00 2001 > From: Leif Lindholm > Date: Wed, 17 Feb 2016 15:33:47 +0000 > Subject: [PATCH] efidisk: Respect block_io_protocol buffer alignment >=20 > 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. >=20 > Add an io_align field to struct grub_disk, and use it in kern/disk.c > when GRUB_MACHINE_EFI is defined. >=20 > Reported-by: Jeremy Linton > --- > grub-core/disk/efi/efidisk.c | 1 + > grub-core/kern/disk.c | 9 +++++++++ > include/grub/disk.h | 3 +++ > 3 files changed, 13 insertions(+) >=20 > diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.= c > index 1c00e3e..424ff92 100644 > --- a/grub-core/disk/efi/efidisk.c > +++ b/grub-core/disk/efi/efidisk.c > @@ -495,6 +495,7 @@ grub_efidisk_open (const char *name, struct grub_di= sk *disk) > and total sectors should be replaced with total blocks. */ > grub_dprintf ("efidisk", "m =3D %p, last block =3D %llx, block size = =3D %x\n", > m, (unsigned long long) m->last_block, m->block_size); > + disk->io_align =3D m->io_align; > disk->total_sectors =3D m->last_block + 1; > /* Don't increase this value due to bug in some EFI. */ > disk->max_agglomerate =3D 0xa0000 >> (GRUB_DISK_CACHE_BITS + GRUB_DI= SK_SECTOR_BITS); > diff --git a/grub-core/kern/disk.c b/grub-core/kern/disk.c > index 789f8c0..27bef10 100644 > --- a/grub-core/kern/disk.c > +++ b/grub-core/kern/disk.c > @@ -331,7 +331,12 @@ grub_disk_read_small_real (grub_disk_t disk, grub_= disk_addr_t sector, > } > =20 > /* Allocate a temporary buffer. */ > +#ifdef GRUB_MACHINE_EFI > + tmp_buf =3D grub_memalign (disk->io_align, > + GRUB_DISK_SECTOR_SIZE << GRUB_DISK_CACHE_BITS); > +#else > tmp_buf =3D grub_malloc (GRUB_DISK_SECTOR_SIZE << GRUB_DISK_CACHE_BI= TS); > +#endif > if (! tmp_buf) > return grub_errno; > =20 > @@ -373,7 +378,11 @@ grub_disk_read_small_real (grub_disk_t disk, grub_= disk_addr_t sector, > num =3D ((size + offset + (1ULL << (disk->log_sector_size)) > - 1) >> (disk->log_sector_size)); > =20 > +#ifdef GRUB_MACHINE_EFI > + tmp_buf =3D grub_memalign (disk->io_align, num << disk->log_sector= _size); > +#else > tmp_buf =3D grub_malloc (num << disk->log_sector_size); > +#endif > if (!tmp_buf) > return grub_errno; > =20 > diff --git a/include/grub/disk.h b/include/grub/disk.h > index b385af8..b063bf6 100644 > --- a/include/grub/disk.h > +++ b/include/grub/disk.h > @@ -126,6 +126,9 @@ struct grub_disk > /* Logarithm of sector size. */ > unsigned int log_sector_size; > =20 > + /* Minimum read/write buffer alignment */ > + unsigned int io_align; > + > /* Maximum number of sectors read divided by GRUB_DISK_CACHE_SIZE. = */ > unsigned int max_agglomerate; > =20 >=20 --OiHwF7KBVVTeJfFi1fe5b1OaJUPd4wMrH Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iF4EAREKAAYFAlbEnpcACgkQmBXlbbo5nOunUwEAovqGQi+V9K9ZHGR7lm6AUPMH SlYlczWIJxWsFk3SDoUA/jJbjchTqBLcWkOy3n1VCbWXZhfaA+zKqeePiUtlD3sn =lmqH -----END PGP SIGNATURE----- --OiHwF7KBVVTeJfFi1fe5b1OaJUPd4wMrH--