diff mbox series

hw/block/hd-geometry: Do not override specified bios-chs-trans

Message ID 20220707204045.999544-1-lkujaw@member.fsf.org (mailing list archive)
State New, archived
Headers show
Series hw/block/hd-geometry: Do not override specified bios-chs-trans | expand

Commit Message

Lev Kujawski July 7, 2022, 8:40 p.m. UTC
For small disk images (<4 GiB), QEMU and SeaBIOS default to the
LARGE/ECHS disk translation method, but it is not uncommon for other
BIOS software to use LBA in these cases as well.  Some operating
system boot loaders (e.g., NT 4) do not handle LARGE translations
outside of fixed configurations.  See, e.g., Q154052:

"When starting an x86 based computer, Ntdetect.com retrieves and
stores Interrupt 13 information. . . If the disk controller is using a
32 sector/64 head translation scheme, this boundary will be 1 GB. If
the controller uses 63 sector/255 head translation [AUTHOR: i.e.,
LBA], the limit will be 4 GB."

To accommodate these situations, hd_geometry_guess() now follows the
disk translation specified by the user even when the ATA disk geometry
is guessed.

hd_geometry_guess():
* Only set the disk translation when translation is AUTO.
* Show the soon-to-be active translation (*ptrans) in the trace rather
  than what was guessed.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/56
Buglink: https://bugs.launchpad.net/qemu/+bug/1745312

Signed-off-by: Lev Kujawski <lkujaw@member.fsf.org>
---
 hw/block/hd-geometry.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Kevin Wolf Aug. 2, 2022, 9:47 a.m. UTC | #1
Am 07.07.2022 um 22:40 hat Lev Kujawski geschrieben:
> For small disk images (<4 GiB), QEMU and SeaBIOS default to the
> LARGE/ECHS disk translation method, but it is not uncommon for other
> BIOS software to use LBA in these cases as well.  Some operating
> system boot loaders (e.g., NT 4) do not handle LARGE translations
> outside of fixed configurations.  See, e.g., Q154052:

I wonder if this means that we should just always use LBA by default
instead of using LARGE for smaller disks, or if this would break other
cases that are working well with the current default.

> "When starting an x86 based computer, Ntdetect.com retrieves and
> stores Interrupt 13 information. . . If the disk controller is using a
> 32 sector/64 head translation scheme, this boundary will be 1 GB. If
> the controller uses 63 sector/255 head translation [AUTHOR: i.e.,
> LBA], the limit will be 4 GB."
> 
> To accommodate these situations, hd_geometry_guess() now follows the
> disk translation specified by the user even when the ATA disk geometry
> is guessed.
> 
> hd_geometry_guess():
> * Only set the disk translation when translation is AUTO.
> * Show the soon-to-be active translation (*ptrans) in the trace rather
>   than what was guessed.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/56
> Buglink: https://bugs.launchpad.net/qemu/+bug/1745312
> 
> Signed-off-by: Lev Kujawski <lkujaw@member.fsf.org>

Thanks, irrespective of my wondering above, the fix looks right, so I've
applied it to my block branch.

Kevin
diff mbox series

Patch

diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index dcbccee294..67462f1752 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -150,7 +150,12 @@  void hd_geometry_guess(BlockBackend *blk,
         translation = BIOS_ATA_TRANSLATION_NONE;
     }
     if (ptrans) {
-        *ptrans = translation;
+        if (*ptrans == BIOS_ATA_TRANSLATION_AUTO) {
+            *ptrans = translation;
+        } else {
+            /* Defer to the translation specified by the user.  */
+            translation = *ptrans;
+        }
     }
     trace_hd_geometry_guess(blk, *pcyls, *pheads, *psecs, translation);
 }