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

[Sam Eiderman]

Links to latest commits from archive.
You can see all changes in the cover letter.

[SeaBIOS] [PATCH v4 0/5] Add Qemu to SeaBIOS LCHS interface
https://mail.coreboot.org/hyperkitty/list/address@hidden/message/VLNFBEERTWLEUO6LM5BYLBNVIFCTP46M/
[SeaBIOS] [PATCH v4 1/5] geometry: Read LCHS from fw_cfg
https://mail.coreboot.org/hyperkitty/list/address@hidden/message/B3IPD3HH4UPDYJWFE4KX3HXUCNW5GPEW/
[SeaBIOS] [PATCH v4 2/5] boot: Reorder functions in boot.c
https://mail.coreboot.org/hyperkitty/list/address@hidden/message/YDVU3WIGOSKZ2RQSMR5UVQNZ66K4IG65/
[SeaBIOS] [PATCH v4 3/5] boot: Build ata and scsi paths in function
https://mail.coreboot.org/hyperkitty/list/address@hidden/message/RY33DRZZ3UK3UMQ3Q6BY2KUCHRRW4MRK/
[SeaBIOS] [PATCH v4 4/5] geometry: Add boot_lchs_find_*() utility functions
https://mail.coreboot.org/hyperkitty/list/address@hidden/message/DAJOULWFK24DX4DY3VS6WWOOQNWW3GSG/
[SeaBIOS] [PATCH v4 5/5] geometry: Apply LCHS values for boot devices
https://mail.coreboot.org/hyperkitty/list/address@hidden/message/UUCTPPJ4PTS5CUTCFLOH3YOEXGC6HQ4T/

Sam

On Wed, Nov 13, 2019 at 6:35 PM Sam Eiderman <address@hidden> wrote:
>
> 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?
> >