[PATCH 0/2] disk: use maximum number of sectors for LBA

[PATCH 0/2] disk: use maximum number of sectors for LBA

[ValdikSS]

GRUB2 limits number of sectors read at once in LBA mode to the
number reported in CHS disk geometry by BIOS.
This is unnecessary, as IBM/MS INT13 Extensions allows reading
up to 127 sectors in a single call.

This fixes greatly increased boot times on WYSE C10LE x86 thin
client with Phoenix bios.

See: https://lists.gnu.org/archive/html/grub-devel/2023-07/msg00001.html

[PATCH 1/2] disk: read up to 63 sectors in LBA mode

[ValdikSS]

There's no need to obey CHS layout restrictions for LBA reads
on LBA disks, it only slows down booting process.

See: https://lists.gnu.org/archive/html/grub-devel/2023-07/msg00001.html
---
 grub-core/disk/i386/pc/biosdisk.c | 5 ++++-
 include/grub/disk.h               | 3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/grub-core/disk/i386/pc/biosdisk.c b/grub-core/disk/i386/pc/biosdisk.c
index f9362d17c..1d6788950 100644
--- a/grub-core/disk/i386/pc/biosdisk.c
+++ b/grub-core/disk/i386/pc/biosdisk.c
@@ -471,7 +471,7 @@ grub_biosdisk_rw (int cmd, grub_disk_t disk,
       struct grub_biosdisk_dap *dap;
 
       dap = (struct grub_biosdisk_dap *) (GRUB_MEMORY_MACHINE_SCRATCH_ADDR
-					  + (data->sectors
+					  + (GRUB_DISK_MAX_LBA_SECTORS
 					     << disk->log_sector_size));
       dap->length = sizeof (*dap);
       dap->reserved = 0;
@@ -561,6 +561,9 @@ get_safe_sectors (grub_disk_t disk, grub_disk_addr_t sector)
   struct grub_biosdisk_data *data = disk->data;
   grub_uint32_t sectors = data->sectors;
 
+  if (data->flags & GRUB_BIOSDISK_FLAG_LBA)
+    sectors = GRUB_DISK_MAX_LBA_SECTORS;
+
   /* OFFSET = SECTOR % SECTORS */
   grub_divmod64 (sector, sectors, &offset);
 
diff --git a/include/grub/disk.h b/include/grub/disk.h
index a4b5d13f3..be032a72c 100644
--- a/include/grub/disk.h
+++ b/include/grub/disk.h
@@ -190,6 +190,9 @@ typedef struct grub_disk_memberlist *grub_disk_memberlist_t;
 
 #define GRUB_DISK_MAX_MAX_AGGLOMERATE ((1 << (30 - GRUB_DISK_CACHE_BITS - GRUB_DISK_SECTOR_BITS)) - 1)
 
+/* Maximum number of sectors to read in LBA mode at once */
+#define GRUB_DISK_MAX_LBA_SECTORS 63
+
 /* Return value of grub_disk_native_sectors() in case disk size is unknown. */
 #define GRUB_DISK_SIZE_UNKNOWN	 0xffffffffffffffffULL
 
-- 
2.41.0

[PATCH 2/2] disk: increase sector size up to 127 for LBA reads

[ValdikSS]

According to Wikipedia and various sources, the recommended
value for LBA read using IBM/MS INT13 Extensions is 127 sectors.
---
 include/grub/disk.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/grub/disk.h b/include/grub/disk.h
index be032a72c..608deb034 100644
--- a/include/grub/disk.h
+++ b/include/grub/disk.h
@@ -184,14 +184,14 @@ typedef struct grub_disk_memberlist *grub_disk_memberlist_t;
 #define GRUB_MDRAID_MAX_DISKS	4096
 
 /* The size of a disk cache in 512B units. Must be at least as big as the
-   largest supported sector size, currently 16K.  */
-#define GRUB_DISK_CACHE_BITS	6
+   largest supported sector size, currently 64K.  */
+#define GRUB_DISK_CACHE_BITS	7
 #define GRUB_DISK_CACHE_SIZE	(1 << GRUB_DISK_CACHE_BITS)
 
 #define GRUB_DISK_MAX_MAX_AGGLOMERATE ((1 << (30 - GRUB_DISK_CACHE_BITS - GRUB_DISK_SECTOR_BITS)) - 1)
 
 /* Maximum number of sectors to read in LBA mode at once */
-#define GRUB_DISK_MAX_LBA_SECTORS 63
+#define GRUB_DISK_MAX_LBA_SECTORS 127
 
 /* Return value of grub_disk_native_sectors() in case disk size is unknown. */
 #define GRUB_DISK_SIZE_UNKNOWN	 0xffffffffffffffffULL
-- 
2.41.0

Re: [PATCH 0/2] disk: use maximum number of sectors for LBA

[Daniel Kiper]

On Fri, Jul 07, 2023 at 12:39:46AM +0300, ValdikSS via Grub-devel wrote:
> GRUB2 limits number of sectors read at once in LBA mode to the
> number reported in CHS disk geometry by BIOS.
> This is unnecessary, as IBM/MS INT13 Extensions allows reading
> up to 127 sectors in a single call.
>
> This fixes greatly increased boot times on WYSE C10LE x86 thin

s/increased/decresed/?

> client with Phoenix bios.

Both patches miss Signed-off-by line. Additionally, please add links to
pages to the patch #2 which say that "IBM/MS INT13 Extensions allows
reading up to 127 sectors in a single call".

Otherwise patches LGTM...

Daniel

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

Re: [PATCH 0/2] disk: use maximum number of sectors for LBA

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1.1: Type: text/plain, Size: 1077 bytes --]

This has a very high risk of breaking existing configs. "Specification
allows" doesn't mean that real-world BIOSes actually react well to it. Some
BIOS may read just reads 63 instead of 127 sectors and we end up with
garbage in this case. I fill it shouldn't come into release. After it we
can discuss. What can change my mind:
1) Proof that windows does such calls
2) Whitelist approach (but probably after release)

Le jeu. 6 juil. 2023, 23:48, ValdikSS via Grub-devel <grub-devel@gnu.org> a
écrit :

> GRUB2 limits number of sectors read at once in LBA mode to the
> number reported in CHS disk geometry by BIOS.
> This is unnecessary, as IBM/MS INT13 Extensions allows reading
> up to 127 sectors in a single call.
>
> This fixes greatly increased boot times on WYSE C10LE x86 thin
> client with Phoenix bios.
>
> See: https://lists.gnu.org/archive/html/grub-devel/2023-07/msg00001.html
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #1.2: Type: text/html, Size: 1728 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

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

Re: [PATCH 0/2] disk: use maximum number of sectors for LBA

[Glenn Washburn]

On Fri,  7 Jul 2023 00:39:46 +0300
ValdikSS via Grub-devel <grub-devel@gnu.org> wrote:

> GRUB2 limits number of sectors read at once in LBA mode to the
> number reported in CHS disk geometry by BIOS.
> This is unnecessary, as IBM/MS INT13 Extensions allows reading
> up to 127 sectors in a single call.

Please provide more documentation for this. Wikipedia[1] suggests that
IBM/MS INT13 Extensions allows reading up to 64K sectors in a single
call. But that "some Phoenix BIOSes are limited to a maximum of 127
sectors". So the above seems misleading.

> 
> This fixes greatly increased boot times on WYSE C10LE x86 thin
> client with Phoenix bios.
> 
> See: https://lists.gnu.org/archive/html/grub-devel/2023-07/msg00001.html

First, this seems risky to me to be added for the release.  Why do we
believe that some other bios doesn't have a limit less than 127?

Also, based on reading the email in the link above, am I correct in
concluding that your bios set the CHS values returned in
grub_biosdisk_rw_int13_extensions() from the values in the MBR?

Glenn

[1]
https://en.wikipedia.org/wiki/INT_13H#INT_13h_AH=42h:_Extended_Read_Sectors_From_Drive

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

Re: [PATCH 1/2] disk: read up to 63 sectors in LBA mode

[Glenn Washburn]

On Fri,  7 Jul 2023 00:39:47 +0300
ValdikSS via Grub-devel <grub-devel@gnu.org> wrote:

> There's no need to obey CHS layout restrictions for LBA reads
> on LBA disks, it only slows down booting process.
> 
> See: https://lists.gnu.org/archive/html/grub-devel/2023-07/msg00001.html

I'd suggest using this link:
  https://lore.kernel.org/grub-devel/d42a11fa-2a59-b5e7-08b1-d2c60444bb99@valdikss.org.ru/

This allows more features for those that might years later be viewing
this link.

> ---
>  grub-core/disk/i386/pc/biosdisk.c | 5 ++++-
>  include/grub/disk.h               | 3 +++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/disk/i386/pc/biosdisk.c b/grub-core/disk/i386/pc/biosdisk.c
> index f9362d17c..1d6788950 100644
> --- a/grub-core/disk/i386/pc/biosdisk.c
> +++ b/grub-core/disk/i386/pc/biosdisk.c
> @@ -471,7 +471,7 @@ grub_biosdisk_rw (int cmd, grub_disk_t disk,
>        struct grub_biosdisk_dap *dap;
>  
>        dap = (struct grub_biosdisk_dap *) (GRUB_MEMORY_MACHINE_SCRATCH_ADDR
> -					  + (data->sectors
> +					  + (GRUB_DISK_MAX_LBA_SECTORS
>  					     << disk->log_sector_size));
>        dap->length = sizeof (*dap);
>        dap->reserved = 0;
> @@ -561,6 +561,9 @@ get_safe_sectors (grub_disk_t disk, grub_disk_addr_t sector)
>    struct grub_biosdisk_data *data = disk->data;
>    grub_uint32_t sectors = data->sectors;
>  
> +  if (data->flags & GRUB_BIOSDISK_FLAG_LBA)
> +    sectors = GRUB_DISK_MAX_LBA_SECTORS;
> +
>    /* OFFSET = SECTOR % SECTORS */
>    grub_divmod64 (sector, sectors, &offset);
>  
> diff --git a/include/grub/disk.h b/include/grub/disk.h
> index a4b5d13f3..be032a72c 100644
> --- a/include/grub/disk.h
> +++ b/include/grub/disk.h
> @@ -190,6 +190,9 @@ typedef struct grub_disk_memberlist *grub_disk_memberlist_t;
>  
>  #define GRUB_DISK_MAX_MAX_AGGLOMERATE ((1 << (30 - GRUB_DISK_CACHE_BITS - GRUB_DISK_SECTOR_BITS)) - 1)
>  
> +/* Maximum number of sectors to read in LBA mode at once */
> +#define GRUB_DISK_MAX_LBA_SECTORS 63

Why is this set to 63 if we're not obeying CHS restrictions any way?

Glenn

> +
>  /* Return value of grub_disk_native_sectors() in case disk size is unknown. */
>  #define GRUB_DISK_SIZE_UNKNOWN	 0xffffffffffffffffULL
>  

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

Re: [PATCH 2/2] disk: increase sector size up to 127 for LBA reads

[Glenn Washburn]

On Fri,  7 Jul 2023 00:39:48 +0300
ValdikSS via Grub-devel <grub-devel@gnu.org> wrote:

> According to Wikipedia and various sources, the recommended
> value for LBA read using IBM/MS INT13 Extensions is 127 sectors.

Please cite references. I'm not seeing that on Wikipedia or osdev.org.
I do see where Wikipedia says that some Phoenix bioses are limited to a
maximum of 127 sectors, but I'm not sure that's a recommendation.

> ---
>  include/grub/disk.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/grub/disk.h b/include/grub/disk.h
> index be032a72c..608deb034 100644
> --- a/include/grub/disk.h
> +++ b/include/grub/disk.h
> @@ -184,14 +184,14 @@ typedef struct grub_disk_memberlist *grub_disk_memberlist_t;
>  #define GRUB_MDRAID_MAX_DISKS	4096
>  
>  /* The size of a disk cache in 512B units. Must be at least as big as the
> -   largest supported sector size, currently 16K.  */
> -#define GRUB_DISK_CACHE_BITS	6
> +   largest supported sector size, currently 64K.  */
> +#define GRUB_DISK_CACHE_BITS	7

This change is not reflected in the commit message and is not obvious
why this is changed. Is it necessary due to the change below or
additional performance improvement?

Glenn

>  #define GRUB_DISK_CACHE_SIZE	(1 << GRUB_DISK_CACHE_BITS)
>  
>  #define GRUB_DISK_MAX_MAX_AGGLOMERATE ((1 << (30 - GRUB_DISK_CACHE_BITS - GRUB_DISK_SECTOR_BITS)) - 1)
>  
>  /* Maximum number of sectors to read in LBA mode at once */
> -#define GRUB_DISK_MAX_LBA_SECTORS 63
> +#define GRUB_DISK_MAX_LBA_SECTORS 127
>  
>  /* Return value of grub_disk_native_sectors() in case disk size is unknown. */
>  #define GRUB_DISK_SIZE_UNKNOWN	 0xffffffffffffffffULL

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

Re: [PATCH 0/2] disk: use maximum number of sectors for LBA

[ValdikSS via Grub-devel]

[-- Attachment #1.1.1: Type: text/plain, Size: 1864 bytes --]

On 05.10.2023 22:09, Glenn Washburn wrote:
>> GRUB2 limits number of sectors read at once in LBA mode to the
>> number reported in CHS disk geometry by BIOS.
>> This is unnecessary, as IBM/MS INT13 Extensions allows reading
>> up to 127 sectors in a single call.
> 
> Please provide more documentation for this. Wikipedia[1] suggests that
> IBM/MS INT13 Extensions allows reading up to 64K sectors in a single
> call. But that "some Phoenix BIOSes are limited to a maximum of 127
> sectors". So the above seems misleading.

Rephrased as "upper safe value for LBA read". Check v2.

>>
>> This fixes greatly increased boot times on WYSE C10LE x86 thin
>> client with Phoenix bios.
>>
>> See: https://lists.gnu.org/archive/html/grub-devel/2023-07/msg00001.html
> 
> First, this seems risky to me to be added for the release.  Why do we
> believe that some other bios doesn't have a limit less than 127?

I've checked Wikipedia and OSDev, as well as Russian forum (don't 
remember which), and tested on two of my old machines with Phoenix and 
Award BIOS.

syslinux also limits to 127 if EBIOS is detected:

         hard_max_transfer = 63;
         ...
         if (!(oreg.eflags.l & EFLAGS_CF) &&
             oreg.ebx.w[0] == 0xaa55 && (oreg.ecx.b[0] & 1)) {
             ebios = true;
             hard_max_transfer = 127;

https://github.com/geneC/syslinux/blob/5e426532210bb830d2d7426eb8d8c154d9dfcba6/core/fs/diskio_bios.c#L349

That's why I assume it should be safe to enable.

> 
> Also, based on reading the email in the link above, am I correct in
> concluding that your bios set the CHS values returned in
> grub_biosdisk_rw_int13_extensions() from the values in the MBR?

Yes, this is correct.

> 
> Glenn
> 
> [1]
> https://en.wikipedia.org/wiki/INT_13H#INT_13h_AH=42h:_Extended_Read_Sectors_From_Drive

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

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

Re: [PATCH 1/2] disk: read up to 63 sectors in LBA mode

[ValdikSS via Grub-devel]

[-- Attachment #1.1.1: Type: text/plain, Size: 686 bytes --]

On 05.10.2023 22:10, Glenn Washburn wrote:
>> --- a/include/grub/disk.h
>> +++ b/include/grub/disk.h
>> @@ -190,6 +190,9 @@ typedef struct grub_disk_memberlist *grub_disk_memberlist_t;
>>   
>>   #define GRUB_DISK_MAX_MAX_AGGLOMERATE ((1 << (30 - GRUB_DISK_CACHE_BITS - GRUB_DISK_SECTOR_BITS)) - 1)
>>   
>> +/* Maximum number of sectors to read in LBA mode at once */
>> +#define GRUB_DISK_MAX_LBA_SECTORS 63
> 
> Why is this set to 63 if we're not obeying CHS restrictions any way?

The current overall reading limit in GRUB is up to 63 sectors. This 
patch just removes CHS layout restrictions for LBA reads, and the later 
patch increases overall limit value to 127.

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

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

Re: [PATCH 2/2] disk: increase sector size up to 127 for LBA reads

[ValdikSS via Grub-devel]

[-- Attachment #1.1.1: Type: text/plain, Size: 1745 bytes --]

On 05.10.2023 22:11, Glenn Washburn wrote:
>> According to Wikipedia and various sources, the recommended
>> value for LBA read using IBM/MS INT13 Extensions is 127 sectors.
> 
> Please cite references. I'm not seeing that on Wikipedia or osdev.org.
> I do see where Wikipedia says that some Phoenix bioses are limited to a
> maximum of 127 sectors, but I'm not sure that's a recommendation.

Done, check v2. Also checked syslinux code, they also have 127.

>> ---
>>   include/grub/disk.h | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/grub/disk.h b/include/grub/disk.h
>> index be032a72c..608deb034 100644
>> --- a/include/grub/disk.h
>> +++ b/include/grub/disk.h
>> @@ -184,14 +184,14 @@ typedef struct grub_disk_memberlist *grub_disk_memberlist_t;
>>   #define GRUB_MDRAID_MAX_DISKS	4096
>>   
>>   /* The size of a disk cache in 512B units. Must be at least as big as the
>> -   largest supported sector size, currently 16K.  */
>> -#define GRUB_DISK_CACHE_BITS	6
>> +   largest supported sector size, currently 64K.  */
>> +#define GRUB_DISK_CACHE_BITS	7
> 
> This change is not reflected in the commit message and is not obvious
> why this is changed. Is it necessary due to the change below or
> additional performance improvement?
> 

The disk cache imposes an overall limitation of a higher-layer reading 
code. The original comment regarding 16K is incorrect, it was 512<<6 = 
32768, and now it is 512<<7 = 65536.

As we're now reading up to 127 sectors of 512 bytes, we need to be able 
to put in the cache up to 65024 bytes. Without this change, GRUB 
wouldn't try to read more than 64 sectors at once (even if the lower 
reading layer allows it).

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

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

Re: [PATCH 0/2] disk: use maximum number of sectors for LBA

[Glenn Washburn]

On Fri, 6 Oct 2023 19:24:11 +0300
ValdikSS <iam@valdikss.org.ru> wrote:

> On 05.10.2023 22:09, Glenn Washburn wrote:
> >> GRUB2 limits number of sectors read at once in LBA mode to the
> >> number reported in CHS disk geometry by BIOS.
> >> This is unnecessary, as IBM/MS INT13 Extensions allows reading
> >> up to 127 sectors in a single call.
> > 
> > Please provide more documentation for this. Wikipedia[1] suggests that
> > IBM/MS INT13 Extensions allows reading up to 64K sectors in a single
> > call. But that "some Phoenix BIOSes are limited to a maximum of 127
> > sectors". So the above seems misleading.
> 
> Rephrased as "upper safe value for LBA read". Check v2.
> 
> >>
> >> This fixes greatly increased boot times on WYSE C10LE x86 thin
> >> client with Phoenix bios.
> >>
> >> See: https://lists.gnu.org/archive/html/grub-devel/2023-07/msg00001.html
> > 
> > First, this seems risky to me to be added for the release.  Why do we
> > believe that some other bios doesn't have a limit less than 127?
> 
> I've checked Wikipedia and OSDev, as well as Russian forum (don't 
> remember which), and tested on two of my old machines with Phoenix and 
> Award BIOS.

My concern isn't with Phoenix but testing for other BIOS manufacturers.
However...

> 
> syslinux also limits to 127 if EBIOS is detected:
> 
>          hard_max_transfer = 63;
>          ...
>          if (!(oreg.eflags.l & EFLAGS_CF) &&
>              oreg.ebx.w[0] == 0xaa55 && (oreg.ecx.b[0] & 1)) {
>              ebios = true;
>              hard_max_transfer = 127;
> 
> https://github.com/geneC/syslinux/blob/5e426532210bb830d2d7426eb8d8c154d9dfcba6/core/fs/diskio_bios.c#L349
> 
> That's why I assume it should be safe to enable.

This gives me more confidence in using 127, although its not clear to
me without digging in the syslinux code that 127 is actually being
used as the transfer size (or when it is). It appears to be a hard max
transfer, which means the actual transfer size could be lower. In this
series, we're using 127 as the transfer size always. So questions that
would help clear that up are: where does MaxTransfer ultimately come
from and when can it be less than 127? How is disk.maxtransfer used and
when does it not represent the actual transfer size.

> 
> > 
> > Also, based on reading the email in the link above, am I correct in
> > concluding that your bios set the CHS values returned in
> > grub_biosdisk_rw_int13_extensions() from the values in the MBR?
> 
> Yes, this is correct.

Ok, this lends more weight to not taking those values too seriously
then. Do you have an explanation of how you got an MBR with sectors ==
2? I would have a hard time believing that debian would produce that.

> 
> > 
> > Glenn
> > 
> > [1]
> > https://en.wikipedia.org/wiki/INT_13H#INT_13h_AH=42h:_Extended_Read_Sectors_From_Drive

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

Re: [PATCH 0/2] disk: use maximum number of sectors for LBA

[ValdikSS via Grub-devel]

[-- Attachment #1.1.1: Type: text/plain, Size: 1155 bytes --]

On 05.10.2023 20:53, Vladimir 'phcoder' Serbinenko wrote:
> This has a very high risk of breaking existing configs. "Specification 
> allows" doesn't mean that real-world BIOSes actually react well to it. 
> Some BIOS may read just reads 63 instead of 127 sectors and we end up 
> with garbage in this case. I fill it shouldn't come into release. After 
> it we can discuss.

I've checked syslinux code, they also use 127 sector reads in LBA mode.
https://github.com/geneC/syslinux/blob/5e426532210bb830d2d7426eb8d8c154d9dfcba6/core/fs/diskio_bios.c#L349

GRUB has this limit as well, but it is not applied due to other constraints.
https://github.com/rhboot/grub2/blob/10f8ffc133553209ec1ddaadc6f4a8a25d3dea4e/grub-core/disk/i386/pc/biosdisk.c#L434

What can change my mind:
> 1) Proof that windows does such calls

Windows XP **bootloader** uses single-sector reads with int 13h to read 
the kernel (just checked with qemu). But the kernel seem to use direct 
IDE controller method reading.

If you believe that 127 sectors could break things, then this patch 
could be ignored. It provides only marginal speed-up vs 63 sectors.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

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

Re: [PATCH 0/2] disk: use maximum number of sectors for LBA

[ValdikSS via Grub-devel]

[-- Attachment #1.1.1: Type: text/plain, Size: 1797 bytes --]

On 06.10.2023 21:33, Glenn Washburn wrote:
> This gives me more confidence in using 127, although its not clear to
> me without digging in the syslinux code that 127 is actually being
> used as the transfer size (or when it is). It appears to be a hard max
> transfer, which means the actual transfer size could be lower. In this
> series, we're using 127 as the transfer size always. So questions that
> would help clear that up are: where does MaxTransfer ultimately come
> from and when can it be less than 127? How is disk.maxtransfer used and
> when does it not represent the actual transfer size.

I haven't studied syslinux code deeply, but it seems they have a 
fallback code which decreases reading sector number on failures if 
larger reads were unsuccessful.
GRUB just falls back to CHS reading if int13_extensions read failed.

I've checked with seabios debugging capabilities in qemu.

disk_op d=0x000f3a20 lba=6544 buf=0x00010000 count=127 cmd=2
disk_op d=0x000f3a20 lba=6671 buf=0x00010000 count=127 cmd=2
disk_op d=0x000f3a20 lba=6798 buf=0x00010000 count=127 cmd=2
disk_op d=0x000f3a20 lba=6925 buf=0x00010000 count=127 cmd=2
disk_op d=0x000f3a20 lba=7052 buf=0x00010000 count=127 cmd=2
disk_op d=0x000f3a20 lba=7179 buf=0x00010000 count=127 cmd=2
disk_op d=0x000f3a20 lba=7306 buf=0x00010000 count=127 cmd=2
disk_op d=0x000f3a20 lba=7433 buf=0x00010000 count=127 cmd=2

> Ok, this lends more weight to not taking those values too seriously
> then. Do you have an explanation of how you got an MBR with sectors ==
> 2? I would have a hard time believing that debian would produce that.

That was just a regular automatic Debian installation in a VM on a 8GB 
disk size.
Check the thread
https://lists.debian.org/debian-boot/2023/07/msg00043.html

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

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

Re: [PATCH 0/2] disk: use maximum number of sectors for LBA

[ValdikSS via Grub-devel]

[-- Attachment #1.1.1: Type: text/plain, Size: 2020 bytes --]

On 06.10.2023 21:34, ValdikSS wrote:
> What can change my mind:
>> 1) Proof that windows does such calls
> 
> Windows XP **bootloader** uses single-sector reads with int 13h to read 
> the kernel (just checked with qemu). But the kernel seem to use direct 
> IDE controller method reading.

I was checking in the wrong place. Now, checking with seabios, Windows 
XP bootloader is reading by a single sector, and the kernel uses up to 
63 sectors reading.

disk_op d=0x000f3a20 lba=1145839 buf=0x00030000 count=5 cmd=2
disk_op d=0x000f3a20 lba=1145844 buf=0x00030000 count=59 cmd=2
disk_op d=0x000f3a20 lba=1145903 buf=0x00030000 count=4 cmd=2
disk_op d=0x000f3a20 lba=1145907 buf=0x00030000 count=60 cmd=2
disk_op d=0x000f3a20 lba=1145967 buf=0x00030000 count=3 cmd=2
disk_op d=0x000f3a20 lba=1145970 buf=0x00030000 count=61 cmd=2
disk_op d=0x000f3a20 lba=1146031 buf=0x00030000 count=2 cmd=2
disk_op d=0x000f3a20 lba=1146033 buf=0x00030000 count=62 cmd=2
disk_op d=0x000f3a20 lba=1146095 buf=0x00030000 count=1 cmd=2
disk_op d=0x000f3a20 lba=1146096 buf=0x00030000 count=63 cmd=2
disk_op d=0x000f3a20 lba=1146159 buf=0x00030000 count=63 cmd=2
disk_op d=0x000f3a20 lba=1146222 buf=0x00030000 count=1 cmd=2
disk_op d=0x000f3a20 lba=1146223 buf=0x00030000 count=62 cmd=2
disk_op d=0x000f3a20 lba=1146285 buf=0x00030000 count=2 cmd=2
disk_op d=0x000f3a20 lba=1146287 buf=0x00030000 count=61 cmd=2

syslinux reads by 127 all the time

disk_op d=0x000f3a20 lba=6544 buf=0x00010000 count=127 cmd=2
disk_op d=0x000f3a20 lba=6671 buf=0x00010000 count=127 cmd=2
disk_op d=0x000f3a20 lba=6798 buf=0x00010000 count=127 cmd=2
disk_op d=0x000f3a20 lba=6925 buf=0x00010000 count=127 cmd=2
disk_op d=0x000f3a20 lba=7052 buf=0x00010000 count=127 cmd=2
disk_op d=0x000f3a20 lba=7179 buf=0x00010000 count=127 cmd=2
disk_op d=0x000f3a20 lba=7306 buf=0x00010000 count=127 cmd=2
disk_op d=0x000f3a20 lba=7433 buf=0x00010000 count=127 cmd=2

Currently GRUB falls back to CHS reading if LBA has failed.

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

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

Re: [PATCH 0/2] disk: use maximum number of sectors for LBA

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1.1: Type: text/plain, Size: 2602 bytes --]

Is Windows XP slow as well?
We should avoid doing anything with firmware API that windows doesn't as
manufacturers use it as sole compatibility test and it's not unusual for
them to just not care about other OS. Like when they shipped bunch of
computers with ACPI code that is basically
if (_OSI("Linux")) {
...some stale code that will corrupt random RAM location...
}

Since then Linux doesn't identify as "Linux" anymore

Le ven. 6 oct. 2023, 22:28, ValdikSS <iam@valdikss.org.ru> a écrit :

> On 06.10.2023 21:34, ValdikSS wrote:
> > What can change my mind:
> >> 1) Proof that windows does such calls
> >
> > Windows XP **bootloader** uses single-sector reads with int 13h to read
> > the kernel (just checked with qemu). But the kernel seem to use direct
> > IDE controller method reading.
>
> I was checking in the wrong place. Now, checking with seabios, Windows
> XP bootloader is reading by a single sector, and the kernel uses up to
> 63 sectors reading.
>
> disk_op d=0x000f3a20 lba=1145839 buf=0x00030000 count=5 cmd=2
> disk_op d=0x000f3a20 lba=1145844 buf=0x00030000 count=59 cmd=2
> disk_op d=0x000f3a20 lba=1145903 buf=0x00030000 count=4 cmd=2
> disk_op d=0x000f3a20 lba=1145907 buf=0x00030000 count=60 cmd=2
> disk_op d=0x000f3a20 lba=1145967 buf=0x00030000 count=3 cmd=2
> disk_op d=0x000f3a20 lba=1145970 buf=0x00030000 count=61 cmd=2
> disk_op d=0x000f3a20 lba=1146031 buf=0x00030000 count=2 cmd=2
> disk_op d=0x000f3a20 lba=1146033 buf=0x00030000 count=62 cmd=2
> disk_op d=0x000f3a20 lba=1146095 buf=0x00030000 count=1 cmd=2
> disk_op d=0x000f3a20 lba=1146096 buf=0x00030000 count=63 cmd=2
> disk_op d=0x000f3a20 lba=1146159 buf=0x00030000 count=63 cmd=2
> disk_op d=0x000f3a20 lba=1146222 buf=0x00030000 count=1 cmd=2
> disk_op d=0x000f3a20 lba=1146223 buf=0x00030000 count=62 cmd=2
> disk_op d=0x000f3a20 lba=1146285 buf=0x00030000 count=2 cmd=2
> disk_op d=0x000f3a20 lba=1146287 buf=0x00030000 count=61 cmd=2
>
> syslinux reads by 127 all the time
>
> disk_op d=0x000f3a20 lba=6544 buf=0x00010000 count=127 cmd=2
> disk_op d=0x000f3a20 lba=6671 buf=0x00010000 count=127 cmd=2
> disk_op d=0x000f3a20 lba=6798 buf=0x00010000 count=127 cmd=2
> disk_op d=0x000f3a20 lba=6925 buf=0x00010000 count=127 cmd=2
> disk_op d=0x000f3a20 lba=7052 buf=0x00010000 count=127 cmd=2
> disk_op d=0x000f3a20 lba=7179 buf=0x00010000 count=127 cmd=2
> disk_op d=0x000f3a20 lba=7306 buf=0x00010000 count=127 cmd=2
> disk_op d=0x000f3a20 lba=7433 buf=0x00010000 count=127 cmd=2
>
> Currently GRUB falls back to CHS reading if LBA has failed.
>

[-- Attachment #1.2: Type: text/html, Size: 3123 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

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

Re: [PATCH 0/2] disk: use maximum number of sectors for LBA

[Glenn Washburn]

On Fri, 6 Oct 2023 23:27:34 +0300
ValdikSS <iam@valdikss.org.ru> wrote:

> On 06.10.2023 21:33, Glenn Washburn wrote:
> > This gives me more confidence in using 127, although its not clear to
> > me without digging in the syslinux code that 127 is actually being
> > used as the transfer size (or when it is). It appears to be a hard max
> > transfer, which means the actual transfer size could be lower. In this
> > series, we're using 127 as the transfer size always. So questions that
> > would help clear that up are: where does MaxTransfer ultimately come
> > from and when can it be less than 127? How is disk.maxtransfer used and
> > when does it not represent the actual transfer size.
> 
> I haven't studied syslinux code deeply, but it seems they have a 
> fallback code which decreases reading sector number on failures if 
> larger reads were unsuccessful.
> GRUB just falls back to CHS reading if int13_extensions read failed.
> 
> I've checked with seabios debugging capabilities in qemu.
> 
> disk_op d=0x000f3a20 lba=6544 buf=0x00010000 count=127 cmd=2
> disk_op d=0x000f3a20 lba=6671 buf=0x00010000 count=127 cmd=2
> disk_op d=0x000f3a20 lba=6798 buf=0x00010000 count=127 cmd=2
> disk_op d=0x000f3a20 lba=6925 buf=0x00010000 count=127 cmd=2
> disk_op d=0x000f3a20 lba=7052 buf=0x00010000 count=127 cmd=2
> disk_op d=0x000f3a20 lba=7179 buf=0x00010000 count=127 cmd=2
> disk_op d=0x000f3a20 lba=7306 buf=0x00010000 count=127 cmd=2
> disk_op d=0x000f3a20 lba=7433 buf=0x00010000 count=127 cmd=2
> 
> > Ok, this lends more weight to not taking those values too seriously
> > then. Do you have an explanation of how you got an MBR with sectors ==
> > 2? I would have a hard time believing that debian would produce that.
> 
> That was just a regular automatic Debian installation in a VM on a 8GB 
> disk size.
> Check the thread
> https://lists.debian.org/debian-boot/2023/07/msg00043.html

Strange, perhaps that's worth reporting as a bug to Debian.

Glenn

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

Re: [PATCH 0/2] disk: use maximum number of sectors for LBA

[ValdikSS via Grub-devel]

[-- Attachment #1.1.1: Type: text/plain, Size: 340 bytes --]

On 06.10.2023 23:49, Vladimir 'phcoder' Serbinenko wrote:
> Is Windows XP slow as well?

Nope, it's fast. The slowness is fixed by the first patch (1/2), without 
which GRUB reads by only 2 sectors at once. With the patch, GRUB reads 
up to 63 sectors.

The second patch only increases 63 to 127, which brings minor speed gain.



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

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

Re: [PATCH 0/2] disk: use maximum number of sectors for LBA

[ValdikSS via Grub-devel]

[-- Attachment #1.1.1: Type: text/plain, Size: 245 bytes --]

On 06.10.2023 23:49, Vladimir 'phcoder' Serbinenko wrote:
> Is Windows XP slow as well?
>
(windows first-stage bootloader is really tiny and these 1-sector reads 
don't feel slow compared to 2-sector reading of 4.5MB Linux kernel by GRUB).

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

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