Re: [PATCH] efidisk: Breakup large reads into batches

[Olaf Hering]

Fri, 23 Feb 2024 16:53:37 + Mate Kukri :

> some UEFI envrionments

Please spell them out explicitly.


Olaf


pgpdV7W68_hjp.pgp
Description: Digitale Signatur von OpenPGP
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] efidisk: Breakup large reads into batches

[Vladimir 'phcoder' Serbinenko]

Why is setting max_agglomerate not enough to achieve the same effect?

On Fri, Feb 23, 2024 at 8:12 PM Mate Kukri  wrote:
>
> From: David F 
>
> Work around firmware bugs that cause large reads to fail from certain
> devices.
>
> Report-By: David F 
> Signed-off-by: Mate Kukri 
> ---
>  grub-core/disk/efi/efidisk.c | 43 +---
>  1 file changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
> index 3b5ed5691..079b72ab7 100644
> --- a/grub-core/disk/efi/efidisk.c
> +++ b/grub-core/disk/efi/efidisk.c
> @@ -27,6 +27,9 @@
>  #include 
>  #include 
>
> +/* break up io to prevent problems under some UEFI envrionments */
> +#define EFIDISK_C_MAXIOSECS 0x100U
> +
>  struct grub_efidisk_data
>  {
>grub_efi_handle_t handle;
> @@ -599,20 +602,34 @@ grub_efidisk_read (struct grub_disk *disk, 
> grub_disk_addr_t sector,
>grub_size_t size, char *buf)
>  {
>grub_efi_status_t status;
> +  grub_size_t sector_count;
>
> -  grub_dprintf ("efidisk",
> -   "reading 0x%lx sectors at the sector 0x%llx from %s\n",
> -   (unsigned long) size, (unsigned long long) sector, 
> disk->name);
> -
> -  status = grub_efidisk_readwrite (disk, sector, size, buf, 0);
> -
> -  if (status == GRUB_EFI_NO_MEDIA)
> -return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("no media in `%s'"), 
> disk->name);
> -  else if (status != GRUB_EFI_SUCCESS)
> -return grub_error (GRUB_ERR_READ_ERROR,
> -  N_("failure reading sector 0x%llx from `%s'"),
> -  (unsigned long long) sector,
> -  disk->name);
> +  /* break up reads to EFIDISK_C_MAXIOSECS size chunks */
> +  do
> +{
> +  /* determine number of sectors this cycle */
> +  sector_count = (size > EFIDISK_C_MAXIOSECS) ? EFIDISK_C_MAXIOSECS : 
> size;
> +
> +  /* output debug information */
> +  grub_dprintf ("efidisk",
> +   "reading 0x%lx sectors at the sector 0x%llx from %s\n",
> +   (unsigned long) sector_count, (unsigned long long) 
> sector, disk->name);
> +
> +  status = grub_efidisk_readwrite (disk, sector, sector_count, buf, 0);
> +
> +  if (status == GRUB_EFI_NO_MEDIA)
> +   return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("no media in `%s'"), 
> disk->name);
> +  if (status != GRUB_EFI_SUCCESS)
> +   return grub_error (GRUB_ERR_READ_ERROR,
> +  N_("failure reading 0x%lx sector(s) from sector 
> 0x%llx on `%s'"),
> +  (unsigned long) sector_count, (unsigned long long) 
> sector, disk->name);
> +
> +  /* next cycle */
> +  buf += (grub_efi_uintn_t) sector_count << disk->log_sector_size;;
> +  sector += sector_count;
> +  size -= sector_count;
> +}
> +  while (size);
>
>return GRUB_ERR_NONE;
>  }
> --
> 2.39.2
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] efidisk: Breakup large reads into batches

[Mate Kukri]

From: David F 

Work around firmware bugs that cause large reads to fail from certain
devices.

Report-By: David F 
Signed-off-by: Mate Kukri 
---
 grub-core/disk/efi/efidisk.c | 43 +---
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
index 3b5ed5691..079b72ab7 100644
--- a/grub-core/disk/efi/efidisk.c
+++ b/grub-core/disk/efi/efidisk.c
@@ -27,6 +27,9 @@
 #include 
 #include 
 
+/* break up io to prevent problems under some UEFI envrionments */
+#define EFIDISK_C_MAXIOSECS 0x100U
+
 struct grub_efidisk_data
 {
   grub_efi_handle_t handle;
@@ -599,20 +602,34 @@ grub_efidisk_read (struct grub_disk *disk, 
grub_disk_addr_t sector,
   grub_size_t size, char *buf)
 {
   grub_efi_status_t status;
+  grub_size_t sector_count;
 
-  grub_dprintf ("efidisk",
-   "reading 0x%lx sectors at the sector 0x%llx from %s\n",
-   (unsigned long) size, (unsigned long long) sector, disk->name);
-
-  status = grub_efidisk_readwrite (disk, sector, size, buf, 0);
-
-  if (status == GRUB_EFI_NO_MEDIA)
-return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("no media in `%s'"), 
disk->name);
-  else if (status != GRUB_EFI_SUCCESS)
-return grub_error (GRUB_ERR_READ_ERROR,
-  N_("failure reading sector 0x%llx from `%s'"),
-  (unsigned long long) sector,
-  disk->name);
+  /* break up reads to EFIDISK_C_MAXIOSECS size chunks */
+  do
+{
+  /* determine number of sectors this cycle */
+  sector_count = (size > EFIDISK_C_MAXIOSECS) ? EFIDISK_C_MAXIOSECS : size;
+
+  /* output debug information */
+  grub_dprintf ("efidisk",
+   "reading 0x%lx sectors at the sector 0x%llx from %s\n",
+   (unsigned long) sector_count, (unsigned long long) sector, 
disk->name);
+
+  status = grub_efidisk_readwrite (disk, sector, sector_count, buf, 0);
+
+  if (status == GRUB_EFI_NO_MEDIA)
+   return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("no media in `%s'"), 
disk->name);
+  if (status != GRUB_EFI_SUCCESS)
+   return grub_error (GRUB_ERR_READ_ERROR,
+  N_("failure reading 0x%lx sector(s) from sector 
0x%llx on `%s'"),
+  (unsigned long) sector_count, (unsigned long long) 
sector, disk->name);
+
+  /* next cycle */
+  buf += (grub_efi_uintn_t) sector_count << disk->log_sector_size;;
+  sector += sector_count;
+  size -= sector_count;
+}
+  while (size);
 
   return GRUB_ERR_NONE;
 }
-- 
2.39.2


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel