diff mbox series

[SeaBIOS,v3,4/4] geometry: Apply LCHS values for boot devices

Message ID 20190619092352.23583-5-shmuel.eiderman@oracle.com
State New
Headers show
Series Add Qemu to SeaBIOS LCHS interface | expand

Commit Message

Sam Eiderman June 19, 2019, 9:23 a.m. UTC 
Boot devices which use overriden LCHS values are:

    * ata
    * ahci
    * scsi
        * esp
        * lsi
        * megasas
        * mpt
        * pvscsi
        * virtio
    * virtio-blk

We use these values in get_translation() and setup_translation() by
introducing a new translation type: "TRANSLATION_MACHINE".

We treat this translation as TRANSLATION_NONE in fill_ata_edd(),
although this does not really matter since now the translation between
physical and logical geometry does not exist.

Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
Reviewed-by: Arbel Moshe <arbel.moshe@oracle.com>
Signed-off-by: Sam Eiderman <shmuel.eiderman@oracle.com>
---
 src/block.c          | 21 ++++++++++++++++++++-
 src/block.h          |  1 +
 src/hw/ahci.c        |  1 +
 src/hw/ata.c         |  8 ++++++++
 src/hw/esp-scsi.c    |  2 ++
 src/hw/lsi-scsi.c    |  2 ++
 src/hw/megasas.c     |  1 +
 src/hw/mpt-scsi.c    |  2 ++
 src/hw/pvscsi.c      |  1 +
 src/hw/virtio-blk.c  |  2 ++
 src/hw/virtio-scsi.c |  2 ++
 11 files changed, 42 insertions(+), 1 deletion(-)

Comments

Gerd Hoffmann June 20, 2019, 5:42 a.m. UTC | #1 
> +static int
> +overriden_lchs_supplied(struct drive_s *drive)
> +{
> +    return drive->lchs.cylinder || drive->lchs.head || drive->lchs.sector;
> +}

> +    case TRANSLATION_MACHINE:

Hmm, why this name?  Doesn't look intuitive to me.

> +        desc = "overriden";

I'd name that "host-supplied" or "fw-cfg".

> +        cylinders = drive->lchs.cylinder;
> +        heads = drive->lchs.head;
> +        if (heads > 255)
> +            heads = 255;

I suggest to move these sanity checks to overriden_lchs_supplied(), then
ignore the override altogether when heads or sectors is out of range
instead of trying to fixup things.

The other patches look all sane to me.

cheers,
  Gerd
Sam Eiderman June 20, 2019, 8:52 a.m. UTC | #2 
> On 20 Jun 2019, at 8:42, Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
>> +static int
>> +overriden_lchs_supplied(struct drive_s *drive)
>> +{
>> +    return drive->lchs.cylinder || drive->lchs.head || drive->lchs.sector;
>> +}
> 
>> +    case TRANSLATION_MACHINE:
> 
> Hmm, why this name?  Doesn't look intuitive to me.

TRANSLATION_HOST?

> 
>> +        desc = "overriden";
> 
> I'd name that "host-supplied" or "fw-cfg”.

“host-supplied”?

> 
>> +        cylinders = drive->lchs.cylinder;
>> +        heads = drive->lchs.head;
>> +        if (heads > 255)
>> +            heads = 255;
> 
> I suggest to move these sanity checks to overriden_lchs_supplied(), then
> ignore the override altogether when heads or sectors is out of range
> instead of trying to fixup things.

Sounds reasonable.
I’ll rename to host_lchs_supplied()?

WDYT?

> 
> The other patches look all sane to me.
> 
> cheers,
>  Gerd
>
Gerd Hoffmann June 20, 2019, 11:47 a.m. UTC | #3 
On Thu, Jun 20, 2019 at 11:52:01AM +0300, Sam Eiderman wrote:
> 
> 
> > On 20 Jun 2019, at 8:42, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > 
> >> +static int
> >> +overriden_lchs_supplied(struct drive_s *drive)
> >> +{
> >> +    return drive->lchs.cylinder || drive->lchs.head || drive->lchs.sector;
> >> +}
> > 
> >> +    case TRANSLATION_MACHINE:
> > 
> > Hmm, why this name?  Doesn't look intuitive to me.
> 
> TRANSLATION_HOST?
> 
> > 
> >> +        desc = "overriden";
> > 
> > I'd name that "host-supplied" or "fw-cfg”.
> 
> “host-supplied”?
> 
> > 
> >> +        cylinders = drive->lchs.cylinder;
> >> +        heads = drive->lchs.head;
> >> +        if (heads > 255)
> >> +            heads = 255;
> > 
> > I suggest to move these sanity checks to overriden_lchs_supplied(), then
> > ignore the override altogether when heads or sectors is out of range
> > instead of trying to fixup things.
> 
> Sounds reasonable.
> I’ll rename to host_lchs_supplied()?
> 
> WDYT?

looks all good to me.

cheers,
  Gerd
diff mbox series

Patch

diff --git a/src/block.c b/src/block.c
index f73ec18c..ca23a83a 100644
--- a/src/block.c
+++ b/src/block.c
@@ -69,9 +69,17 @@  int create_bounce_buf(void)
  * Disk geometry translation
  ****************************************************************/
 
+static int
+overriden_lchs_supplied(struct drive_s *drive)
+{
+    return drive->lchs.cylinder || drive->lchs.head || drive->lchs.sector;
+}
+
 static u8
 get_translation(struct drive_s *drive)
 {
+    if (overriden_lchs_supplied(drive))
+        return TRANSLATION_MACHINE;
     u8 type = drive->type;
     if (CONFIG_QEMU && type == DTYPE_ATA) {
         // Emulators pass in the translation info via nvram.
@@ -159,6 +167,16 @@  setup_translation(struct drive_s *drive)
                 break;
         }
         break;
+    case TRANSLATION_MACHINE:
+        desc = "overriden";
+        cylinders = drive->lchs.cylinder;
+        heads = drive->lchs.head;
+        if (heads > 255)
+            heads = 255;
+        spt = drive->lchs.sector;
+        if (spt > 63)
+            spt = 63;
+        break;
     }
     // clip to 1024 cylinders in lchs
     if (cylinders > 1024)
@@ -423,7 +441,8 @@  fill_ata_edd(struct segoff_s edd, struct drive_s *drive_gf)
     u16 options = 0;
     if (GET_GLOBALFLAT(drive_gf->type) == DTYPE_ATA) {
         u8 translation = GET_GLOBALFLAT(drive_gf->translation);
-        if (translation != TRANSLATION_NONE) {
+        if ((translation != TRANSLATION_NONE) &&
+            (translation != TRANSLATION_MACHINE)) {
             options |= 1<<3; // CHS translation
             if (translation == TRANSLATION_LBA)
                 options |= 1<<9;
diff --git a/src/block.h b/src/block.h
index f64e8807..12f27eee 100644
--- a/src/block.h
+++ b/src/block.h
@@ -90,6 +90,7 @@  struct drive_s {
 #define TRANSLATION_LBA   1
 #define TRANSLATION_LARGE 2
 #define TRANSLATION_RECHS 3
+#define TRANSLATION_MACHINE 4
 
 #define EXTTYPE_FLOPPY 0
 #define EXTTYPE_HD 1
diff --git a/src/hw/ahci.c b/src/hw/ahci.c
index 1746e7a1..97a072a1 100644
--- a/src/hw/ahci.c
+++ b/src/hw/ahci.c
@@ -593,6 +593,7 @@  static int ahci_port_setup(struct ahci_port_s *port)
                               , ata_extract_version(buffer));
         port->prio = bootprio_find_ata_device(ctrl->pci_tmp, pnr, 0);
     }
+    boot_lchs_find_ata_device(ctrl->pci_tmp, pnr, 0, &(port->drive.lchs));
     return 0;
 }
 
diff --git a/src/hw/ata.c b/src/hw/ata.c
index b6e073cf..f788ce71 100644
--- a/src/hw/ata.c
+++ b/src/hw/ata.c
@@ -755,6 +755,10 @@  init_drive_atapi(struct atadrive_s *dummy, u16 *buffer)
         int prio = bootprio_find_ata_device(adrive->chan_gf->pci_tmp,
                                             adrive->chan_gf->chanid,
                                             adrive->slave);
+        boot_lchs_find_ata_device(adrive->chan_gf->pci_tmp,
+                                  adrive->chan_gf->chanid,
+                                  adrive->slave,
+                                  &(adrive->drive.lchs));
         boot_add_cd(&adrive->drive, desc, prio);
     }
 
@@ -805,6 +809,10 @@  init_drive_ata(struct atadrive_s *dummy, u16 *buffer)
     int prio = bootprio_find_ata_device(adrive->chan_gf->pci_tmp,
                                         adrive->chan_gf->chanid,
                                         adrive->slave);
+    boot_lchs_find_ata_device(adrive->chan_gf->pci_tmp,
+                              adrive->chan_gf->chanid,
+                              adrive->slave,
+                              &(adrive->drive.lchs));
     // Register with bcv system.
     boot_add_hd(&adrive->drive, desc, prio);
 
diff --git a/src/hw/esp-scsi.c b/src/hw/esp-scsi.c
index ffd86d0f..cc25f227 100644
--- a/src/hw/esp-scsi.c
+++ b/src/hw/esp-scsi.c
@@ -181,6 +181,8 @@  esp_scsi_add_lun(u32 lun, struct drive_s *tmpl_drv)
 
     char *name = znprintf(MAXDESCSIZE, "esp %pP %d:%d",
                           llun->pci, llun->target, llun->lun);
+    boot_lchs_find_scsi_device(llun->pci, llun->target, llun->lun,
+                               &(llun->drive.lchs));
     int prio = bootprio_find_scsi_device(llun->pci, llun->target, llun->lun);
     int ret = scsi_drive_setup(&llun->drive, name, prio);
     free(name);
diff --git a/src/hw/lsi-scsi.c b/src/hw/lsi-scsi.c
index d5fc3e45..cbaa2acd 100644
--- a/src/hw/lsi-scsi.c
+++ b/src/hw/lsi-scsi.c
@@ -158,6 +158,8 @@  lsi_scsi_add_lun(u32 lun, struct drive_s *tmpl_drv)
     lsi_scsi_init_lun(llun, tmpl_llun->pci, tmpl_llun->iobase,
                       tmpl_llun->target, lun);
 
+    boot_lchs_find_scsi_device(llun->pci, llun->target, llun->lun,
+                               &(llun->drive.lchs));
     char *name = znprintf(MAXDESCSIZE, "lsi %pP %d:%d",
                           llun->pci, llun->target, llun->lun);
     int prio = bootprio_find_scsi_device(llun->pci, llun->target, llun->lun);
diff --git a/src/hw/megasas.c b/src/hw/megasas.c
index d2675804..87b8beec 100644
--- a/src/hw/megasas.c
+++ b/src/hw/megasas.c
@@ -225,6 +225,7 @@  megasas_add_lun(struct pci_device *pci, u32 iobase, u8 target, u8 lun)
         free(mlun);
         return -1;
     }
+    boot_lchs_find_scsi_device(pci, target, lun, &(mlun->drive.lchs));
     name = znprintf(MAXDESCSIZE, "MegaRAID SAS (PCI %pP) LD %d:%d"
                     , pci, target, lun);
     prio = bootprio_find_scsi_device(pci, target, lun);
diff --git a/src/hw/mpt-scsi.c b/src/hw/mpt-scsi.c
index 1faede6a..570b2126 100644
--- a/src/hw/mpt-scsi.c
+++ b/src/hw/mpt-scsi.c
@@ -221,6 +221,8 @@  mpt_scsi_add_lun(u32 lun, struct drive_s *tmpl_drv)
     mpt_scsi_init_lun(llun, tmpl_llun->pci, tmpl_llun->iobase,
                       tmpl_llun->target, lun);
 
+    boot_lchs_find_scsi_device(llun->pci, llun->target, llun->lun,
+                               &(llun->drive.lchs));
     char *name = znprintf(MAXDESCSIZE, "mpt %pP %d:%d",
                           llun->pci, llun->target, llun->lun);
     int prio = bootprio_find_scsi_device(llun->pci, llun->target, llun->lun);
diff --git a/src/hw/pvscsi.c b/src/hw/pvscsi.c
index 9d7d68d8..3e5171ad 100644
--- a/src/hw/pvscsi.c
+++ b/src/hw/pvscsi.c
@@ -273,6 +273,7 @@  pvscsi_add_lun(struct pci_device *pci, void *iobase,
     plun->iobase = iobase;
     plun->ring_dsc = ring_dsc;
 
+    boot_lchs_find_scsi_device(pci, target, lun, &(plun->drive.lchs));
     char *name = znprintf(MAXDESCSIZE, "pvscsi %pP %d:%d", pci, target, lun);
     int prio = bootprio_find_scsi_device(pci, target, lun);
     int ret = scsi_drive_setup(&plun->drive, name, prio);
diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c
index 88d7e54a..3e615b26 100644
--- a/src/hw/virtio-blk.c
+++ b/src/hw/virtio-blk.c
@@ -183,6 +183,8 @@  init_virtio_blk(void *data)
 
     status |= VIRTIO_CONFIG_S_DRIVER_OK;
     vp_set_status(&vdrive->vp, status);
+
+    boot_lchs_find_pci_device(pci, &vdrive->drive.lchs);
     return;
 
 fail:
diff --git a/src/hw/virtio-scsi.c b/src/hw/virtio-scsi.c
index a87cad88..e1e2f5d4 100644
--- a/src/hw/virtio-scsi.c
+++ b/src/hw/virtio-scsi.c
@@ -121,6 +121,8 @@  virtio_scsi_add_lun(u32 lun, struct drive_s *tmpl_drv)
     virtio_scsi_init_lun(vlun, tmpl_vlun->pci, tmpl_vlun->vp, tmpl_vlun->vq,
                          tmpl_vlun->target, lun);
 
+    boot_lchs_find_scsi_device(vlun->pci, vlun->target, vlun->lun,
+                               &(vlun->drive.lchs));
     int prio = bootprio_find_scsi_device(vlun->pci, vlun->target, vlun->lun);
     int ret = scsi_drive_setup(&vlun->drive, "virtio-scsi", prio);
     if (ret)