Re: Hardcoded IDE Controller B/D/F (0/1/1) in BdsPlatform.c?

[Laszlo Ersek <lersek@redhat.com>]

On 11/22/18 01:06, Bryan Rosario via edk2-devel wrote:
> Hi all,
> 
> I noticed that there is apparently a hardcoded B/D/F for an IDE Controller
> in BdsPlatform.c, when setting bit 15 of a particular register:
> https://github.com/tianocore/edk2/blob/14923c1a6bf9940b48feeaf47cb5d6c662b6528c/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c#L1612-L1617
> .
> 
> Why is this hardcoded? Is it just old code that has hung around?

It's ancient code that should not exist, arguably. I've always been
afraid to rip it out, because I've never known what would break.

In general, BDS has zero business writing register of various PCI
devices. Even if it does, due to various platform glitches or hacks, BDS
should preferably not use PciLib APIs for that, but the UEFI PciIo
protocol. (I totally know that OVMF's Platform BDS does not live up to
this.)

> I noticed
> that this code is from this commit, and the surrounding code at the time
> had lots of hardcoded B/D/Fs:
> https://github.com/tianocore/edk2/commit/40f2c454343be84ab3bacf9955cc8d7842c70b5c
> .
> 
> The context of this question is that I'm trying to make this work in a
> configuration with an IDE Controller located at a different B/D/F, and so
> the hardcoded value of 0/1/1 doesn't work for me.
> 
> If I want to change this so that it's not hardcoded, what is a good
> approach?

Remove it altogether from BDS (or else move it to the specific UEFI
driver) and see if things still work. :)

> I've added some code locally to the IdeInitSetTiming function in
> SataController.c to set the bit using the PciIo structure which is opened
> specifically on the controller in question -- does that sound like the
> right approach?

Definitely. It certainly helps if you know what those register writes
actually mean. (I don't.)

Thanks,
Laszlo

> Link to the function I'm referring to:
> https://github.com/tianocore/edk2/blob/f6b0258d25a63ae3d3bc6430abe30fb625abc52a/OvmfPkg/SataControllerDxe/SataController.c#L1091-L1099