From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Thu, 06 Jun 2019 00:56:38 -0700 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AD1A12F8BF5; Thu, 6 Jun 2019 07:56:30 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-196.ams2.redhat.com [10.36.116.196]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8E4C368D60; Thu, 6 Jun 2019 07:56:21 +0000 (UTC) Subject: Re: [PATCH v2] OvmfPkg/QemuVideoDxe: Shouldn't assume system in VGA alias mode. To: Marc W Chen , devel@edk2.groups.io Cc: Jordan Justen , Ard Biesheuvel , Anthony Perard , Julien Grall , =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= , Stefan Berger References: <20190606074237.81492-1-marc.w.chen@intel.com> From: "Laszlo Ersek" Message-ID: <91129f61-d413-925a-7d26-71ad3bda3d12@redhat.com> Date: Thu, 6 Jun 2019 09:56:10 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190606074237.81492-1-marc.w.chen@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Thu, 06 Jun 2019 07:56:38 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 06/06/19 09:42, Marc W Chen wrote: > Query the supported attributes firstly, then bitwise AND (&) both VGA_I= O > 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 t= he > attributes. >=20 > Device driver should consider both since the mReserveVgaAliases in > PciBusDxe driver is default FALSE(implies that device driver can only s= et > 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, s= o > an IO access by PciIoProtocol will be successed due to > RootBridgeIoCheckParameter of PciRootBridgeIo.c will always get pass > result for legacy IO access. >=20 > Usually the attributes field of PCI_ROOT_BRIDGE should be 0, in that ca= se > 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 drive= r > will not work fine. >=20 > Signed-off-by: Marc Chen > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Anthony Perard > Cc: Julien Grall > Cc: Marc-Andr=C3=A9 Lureau > Cc: Stefan Berger > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1880 > --- > OvmfPkg/QemuVideoDxe/Driver.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) >=20 > diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Drive= r.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; > =20 > OldTpl =3D gBS->RaiseTPL (TPL_CALLBACK); > =20 > @@ -277,13 +278,32 @@ QemuVideoControllerDriverStart ( > goto ClosePciIo; > } > =20 > + // > + // Get supported PCI attributes > + // > + Status =3D Private->PciIo->Attributes ( > + Private->PciIo, > + EfiPciIoAttributeOperationSupported, > + 0, > + &SupportedVgaIo > + ); > + if (EFI_ERROR (Status)) { > + goto ClosePciIo; > + } > + > + SupportedVgaIo &=3D (UINT64)(EFI_PCI_IO_ATTRIBUTE_VGA_IO | EFI_PCI_I= O_ATTRIBUTE_VGA_IO_16); > + if (SupportedVgaIo =3D=3D 0) { > + Status =3D EFI_UNSUPPORTED; > + goto ClosePciIo; > + } > + > // > // Set new PCI attributes > // > Status =3D Private->PciIo->Attributes ( > Private->PciIo, > EfiPciIoAttributeOperationEnable, > - EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBU= TE_VGA_MEMORY | EFI_PCI_IO_ATTRIBUTE_VGA_IO, > + EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBU= TE_VGA_MEMORY | SupportedVgaIo, > NULL > ); > if (EFI_ERROR (Status)) { >=20 Reviewed-by: Laszlo Ersek 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. Thanks Laszlo