Re: [SeaBIOS] Re: [PATCH] ahci: zero-initialize port struct

[Sam Eiderman]

Sure,

There are two issues here.

The first issue is that my commits which applied to seabios master:

* 9caa19b - geometry: Apply LCHS values for boot devices
* cb56f61 - config: Add toggle for bootdevice information
* ad29109 - geometry: Add boot_lchs_find_*() utility functions
* b3d2120 - boot: Reorder functions in boot.c
* 7c66a43 - geometry: Read LCHS from fw_cfg

Are not from the latest version which was submitted to the mailing list (v4)
* fw_cfg key name has changed
* The value and of the key has changed from binary (v1) to textual (v4)
* Other fixes and variable name changes.

So these commits need to be reverted and reapplied with the latest version (v4)

The second issue is that my commits, (in v4 too) will require this fix
that Gerd added ([PATCH] ahci: zero-initialize port struct) since they
change how SeaBIOS uses lchs.

Previously, before any of my commits, drive.lchs could contain "random
crap" since it was always set before being used in
setup_translation().

After my patches, get_translation() invokes overriden_lchs_supplied()
which checks: "return drive->lchs.cylinder || drive->lchs.head ||
drive->lchs.sector;"
So there is an assumption that "drive->lchs" is zeroed when lchs is
not supplied for the host.

This was true for all devices using "drive->lchs" (all were memset to
0) except ahci.
(I used 'git grep "drive_s * drive"' to find them all).

So Gerd fix is indeed needed and then all devices are covered
(drive->lchs is memset to 0).

Now only the first issue remains...

Sam

On Wed, Nov 13, 2019 at 6:12 PM Philippe Mathieu-Daudé
<address@hidden> wrote:
>
> Hi Sam,
>
> On 11/13/19 4:03 PM, Sam Eiderman wrote:
> > Hi,
> >
> > Does this fix a bug that actually happened?
> >
> > I just noticed that in my lchs patches I assumed that lchs struct is
> > zeroed out in all devices (not only ahci):
> >
> > 9caa19be0e53 (geometry: Apply LCHS values for boot devices)
> >
> > Seems like this is not the case but why only ahci is affected?
> >
> > The list of devices is at least:
> >
> >          * ata
> >          * ahci
> >          * scsi
> >              * esp
> >              * lsi
> >              * megasas
> >              * mpt
> >              * pvscsi
> >              * virtio
> >          * virtio-blk
> >
> > As specified in the commit message.
> >
> > Also Gerd it seems that my lchs patches were not committed in the
> > latest submitted version (v4)!!!
> > The ABI of the fw config key is completely broken.
>
> What do you mean? Can you be more specific?
>