On 06/06/19 09:56, Laszlo Ersek wrote: > On 06/06/19 09:42, Marc W Chen wrote: >> Query the supported attributes firstly, then bitwise AND (&) both VGA_IO >> and VGA_IO_16. Since the supported attributes should only have one of >> VGA_IO or VGA_IO_16 set, the result of bitwise AND (&) is either VGA_IO >> or IO_16. Then the result can be passed to PciIo->Attributes() to set the >> attributes. >> >> Device driver should consider both since the mReserveVgaAliases in >> PciBusDxe driver is default FALSE(implies that device driver can only set >> VGA_IO_16 to PCI_ROOT_BRIDGE), and Platform code may not return >> EFI_RESERVE_VGA_IO_ALIAS in GetPlatformPolicy of PciPlatformProtocol to >> make mReserveVgaAliases become TRUE(implies that device driver can only >> set VGA_IO to PCI_ROOT_BRIDGE), Currently OvmfPkg doesn't have problem >> due to it has hard code value for PCI_ROOT_BRIDGE's attributes field, so >> an IO access by PciIoProtocol will be successed due to >> RootBridgeIoCheckParameter of PciRootBridgeIo.c will always get pass >> result for legacy IO access. >> >> Usually the attributes field of PCI_ROOT_BRIDGE should be 0, in that case >> it will have issue since the VGA_IO may not be able to be enabled, then >> IO access by PciIoProtocol will be failed, hence the QemuVideoDxe driver >> will not work fine. >> >> Signed-off-by: Marc Chen <[email protected]> >> Cc: Jordan Justen <[email protected]> >> Cc: Laszlo Ersek <[email protected]> >> Cc: Ard Biesheuvel <[email protected]> >> Cc: Anthony Perard <[email protected]> >> Cc: Julien Grall <[email protected]> >> Cc: Marc-André Lureau <[email protected]> >> Cc: Stefan Berger <[email protected]> >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1880 >> --- >> OvmfPkg/QemuVideoDxe/Driver.c | 22 +++++++++++++++++++++- >> 1 file changed, 21 insertions(+), 1 deletion(-) >> >> diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c >> index e8a613ef33..522110ef4e 100644 >> --- a/OvmfPkg/QemuVideoDxe/Driver.c >> +++ b/OvmfPkg/QemuVideoDxe/Driver.c >> @@ -201,6 +201,7 @@ QemuVideoControllerDriverStart ( >> PCI_TYPE00 Pci; >> QEMU_VIDEO_CARD *Card; >> EFI_PCI_IO_PROTOCOL *ChildPciIo; >> + UINT64 SupportedVgaIo; >> >> OldTpl = gBS->RaiseTPL (TPL_CALLBACK); >> >> @@ -277,13 +278,32 @@ QemuVideoControllerDriverStart ( >> goto ClosePciIo; >> } >> >> + // >> + // Get supported PCI attributes >> + // >> + Status = Private->PciIo->Attributes ( >> + Private->PciIo, >> + EfiPciIoAttributeOperationSupported, >> + 0, >> + &SupportedVgaIo >> + ); >> + if (EFI_ERROR (Status)) { >> + goto ClosePciIo; >> + } >> + >> + SupportedVgaIo &= (UINT64)(EFI_PCI_IO_ATTRIBUTE_VGA_IO | >> EFI_PCI_IO_ATTRIBUTE_VGA_IO_16); >> + if (SupportedVgaIo == 0) { >> + Status = EFI_UNSUPPORTED; >> + goto ClosePciIo; >> + } >> + >> // >> // Set new PCI attributes >> // >> Status = Private->PciIo->Attributes ( >> Private->PciIo, >> EfiPciIoAttributeOperationEnable, >> - EFI_PCI_DEVICE_ENABLE | >> EFI_PCI_IO_ATTRIBUTE_VGA_MEMORY | EFI_PCI_IO_ATTRIBUTE_VGA_IO, >> + EFI_PCI_DEVICE_ENABLE | >> EFI_PCI_IO_ATTRIBUTE_VGA_MEMORY | SupportedVgaIo, >> NULL >> ); >> if (EFI_ERROR (Status)) { >> > > Reviewed-by: Laszlo Ersek <[email protected]> > > I will keep this patch tagged on my end, so that I can push it once > edk2-stable201905 has been tagged. Should I forget, please ping me.
Pushed as commit 662bd0da7fd7. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#41997): https://edk2.groups.io/g/devel/message/41997 Mute This Topic: https://groups.io/mt/31948697/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
