On 05/13/14 01:22, Jordan Justen wrote:
> On Mon, May 12, 2014 at 2:35 PM, Laszlo Ersek <ler...@redhat.com> wrote:
>> On 05/12/14 22:49, Jordan Justen wrote:
>>> On Mon, May 12, 2014 at 10:41 AM, Laszlo Ersek <ler...@redhat.com> wrote:
>>>> On 05/12/14 18:03, Jordan Justen wrote:
>>>>> On Mon, May 12, 2014 at 6:18 AM, Laszlo Ersek <ler...@redhat.com>
>>>>> wrote:
>>>>>> Additionally, there's another (similarly downstream only) patch
>>>>>>
>>>>>>   OvmfPkg: Don't build in QemuVideoDxe when we have CSM
>>>>>>
>>>>>> Did you include the presence of this patch in your question?
>>>>>
>>>>> No. I want to remove this patch from being needed, and also rely upon
>>>>> your shim patch (for this OS) when CSM is enabled.
>>>>
>>>> (The question was "If CSM is enabled, we will fail to allocate, right?")
>>>>
>>>> OK, so we have:
>>>>
>>>> * "LegacyBios: Add UmbStart,UmbEnd fields to EFI_COMPATIBILITY16_TABLE"
>>>>   (I tried to replace it with David's new patch that he just posted
>>>>   <http://thread.gmane.org/gmane.comp.bios.tianocore.devel/7065>,
>>>>   but it doesn't apply for me.)
>>>> * OvmfPkg/Csm/Csm16/Csm16.bin (the SeaBIOS binary)
>>>> * -D CSM_ENABLE, resulting in
>>>>   - OvmfPkg/Csm/CsmSupportLib/CsmSupportLib.inf
>>>>     (built into IntelFrameworkModulePkg/Universal/BdsDxe/BdsDxe.inf)
>>>>   - IntelFrameworkModulePkg/Csm/BiosThunk/VideoDxe/VideoDxe.inf
>>>>   - IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
>>>> * this patch
>>>>
>>>> I'm seeing the following:
>>>>
>>>> (1) LegacyBiosDxe is loaded first. LegacyBiosInstall() -->
>>>>     AllocateLegacyMemory() succeeds and takes the page at zero.
>>>>     [IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c]
>>>>
>>>> (2) QemuVideoDxe binds the card before VideoDxe gets there (this used to
>>>> be the reason for excluding QemuVideoDxe from the CSM build). It can't
>>>> allocate the page at zero:
>>>>
>>>>   QemuVideoControllerDriverStart() [OvmfPkg/QemuVideoDxe/Driver.c]
>>>>     InstallVbeShim() [OvmfPkg/QemuVideoDxe/VbeShim.c]
>>>>       gBS->AllocatePages()
>>>>
>>>> Therefore the binding fails. (Answering your question with "yes".)
>>>
>>> I don't want to fail to start QemuVideoDxe whenever a CSM is enabled.
>>>
>>> Although it is a bit of a hack to go write to the int 0x10 vector even
>>> if QemuVideoDxe doesn't allocate page 0, I still want to consider it.
>>>
>>> I think in my patch I probed the int 0x10 vector to see if it looked
>>> like a vbios had been installed. If it didn't look like it, then I
>>> went ahead and installed the stub even if the allocation didn't
>>> succeed.
>>
>> Yes, I do recall it that way. I didn't understand why even then :)
>>
>>> I think the stub could then work for this OS while QemuVideoDxe is
>>> attached to the device (and a CSM is enabled).
>>
>> But what do you gain with it in practice? If LegacyBiosDxe has allocated
>> page 0, then all option roms (including the VGA BIOS) are its (and
>> VideoDxe's) responsibility.
>>
>> The only scenario I can imagine where all of the above hold, ie:
>> - CSM is enabled (incl. LegacyBiosDxe, some CSM binary, plus VideoDxe),
>> - hence page 0 is taken by LegacyBiosDxe,
>> - and VideoDxe does *not* POST / install a video BIOS
>>
>> is when you start qemu like this:
>>
>>   -device qxl-vga,romfile=
>>
>> or
>>
>>   -vga qxl \
>>   -global qxl-vga.romfile=
>>
>> that is, when you forcefully remove the VGABIOS oprom from the card (all
>> the while running a CSM-enabled OVMF build). Why would you do that?
>>
>> What other use case is there that I'm not seeing?
> 
> One OS doesn't properly UEFI boot using only UEFI boot services, so we
> should never run the UEFI native video driver if CSM is enabled?

I would have said "yes". (This is what the downstream-only patch does now.)

> It
> seems like too big of a hammer for the issue.

I still don't see what (of worth) you lose with it. You lose the use of
QemuVideoDxe, sure; but why would you enable the CSM if you weren't OK
with using VideoDxe over QemuVideoDxe?

But, let's consider the alternative. Suppose the following:
- LegacyBiosDxe is loaded first. LegacyBiosInstall() -->
  AllocateLegacyMemory() succeeds and takes the page at zero.
- QemuVideoDxe binds the card, and wants to install the shim. The
  allocation of page 0 fails, but we don't care, we just overwrite the
  Int10h entry, and binding succeeds.
- VideoDxe never binds the card (it has been claimed).

Isn't this a similar question? "Even though we enabled the CSM, OVMF
won't use the legacy video bios in the card's oprom; why is that?"

> I think you've got us 98% of the way to a solution that works around
> this OS boot issue. Can we use it here too?

Sure. Please tell me precisely what you'd like to see in v2 in this
regard. Or, consider answering with a small patch on top -- I can squash
it or include it as a separate patch in the v2 series.

I haven't been arguing because I wanted my patch kept intact. I honestly
don't understand what use case the proposed change is good for. But I'm
certainly willing to squash or append your proposed patch. Do you mean
something like:
- if the allocation succeeds, go ahead like now.
- if it fails, check the Int10h vector:
  - if it points into the C segment, abort. (Just the shim's
    installation, or abandon the binding completely?)
  - if it points into elsewhere (eg. it's zero), go ahead like now.
?

Thanks,
Laszlo

------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to