public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Re: [PATCH v3 4/4] OvmfPkg: don't assign PCI BARs above 4GiB when CSM enabled
@ 2019-06-27 16:36 graf
  2019-06-27 18:43 ` Laszlo Ersek
  0 siblings, 1 reply; 6+ messages in thread
From: graf @ 2019-06-27 16:36 UTC (permalink / raw)
  To: lersek; +Cc: devel, David Woodhouse, Ard Biesheuvel

Hi David and Laszlo,

(with broken threading because gmane still mirrors the old ML ...)

> Mostly, this is only necessary for devices that the CSM might have
> native support for, such as VirtIO and NVMe; PciBusDxe will already
> degrade devices to 32-bit if they have an OpROM.
> 
> However, there doesn't seem to be a generic way of requesting PciBusDxe
> to downgrade specific devices.
> 
> There's IncompatiblePciDeviceSupportProtocol but that doesn't provide
> the PCI class information or a handle to the device itself, so there's
> no simple way to just match on all NVMe devices, for example.
> 
> Just leave gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size set to zero for
> CSM builds, until/unless that can be fixed.
> 
> Signed-off-by: David Woodhouse <dwmw2@...>
> Reviewed-by: Laszlo Ersek <lersek@...>
> ---
>  OvmfPkg/OvmfPkgIa32X64.dsc | 4 ++++
>  OvmfPkg/OvmfPkgX64.dsc     | 4 ++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 639e33cb285f..ad20531ceb8b 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -543,7 +543,11 @@ [PcdsDynamicDefault]
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Base|0x0
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Size|0x0
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base|0x0
> +!ifdef $(CSM_ENABLE)
> +  gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x0
> +!else
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x800000000
> +!endif
>  
>    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
>  
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 69a3497c2c9e..0542ac2235b4 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -542,7 +542,11 @@ [PcdsDynamicDefault]
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Base|0x0
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Size|0x0
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base|0x0
> +!ifdef $(CSM_ENABLE)
> +  gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x0

IIRC x86 Linux just takes firmware provided BAR maps as they are and 
doesn't map on its own. Or does it map if a BAR was previously unmapped?

In the former case, wouldn't that mean that we're breaking GPU 
passthrough (*big* BARs) for OVMF if the OVMF version happens to support 
CSM? So if a distro decides to turn on CSM, that would be a very subtle 
regression.

Would it be possible to change the PCI mapping logic to just simply 
*prefer* low BAR space if there's some available and the BAR is not big 
(<64MB for example)?

That way we could have CSM enabled OVMF for everyone ;)


Alex


> +!else
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x800000000
> +!endif
>  
>    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
>  
> -- 
> 2.21.0

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v3 0/4] OvmfPkg: CSM boot fixes
@ 2019-06-26 11:37 David Woodhouse
  2019-06-26 11:37 ` [PATCH v3 4/4] OvmfPkg: don't assign PCI BARs above 4GiB when CSM enabled David Woodhouse
  0 siblings, 1 reply; 6+ messages in thread
From: David Woodhouse @ 2019-06-26 11:37 UTC (permalink / raw)
  To: devel; +Cc: Laszlo Ersek

For v3, leaving out the cosmetic parts that touch code outside OvmfPkg. 
This series is now purely the correctness fixes within OvmfPkg which are 
required to make CSM boots work properly again.

The first two patches allow NVMe and VirtIO disks to be used as Legacy
boot targets, since nobody really uses IDE any more.

The third avoids using QemuVideoDxe when we have CSM, as the INT 10h 
shim installed by QemuVideoDxe conflicts with a real legacy video BIOS
being installed.

Finally, avoid placing PCI BARs above 4GiB. Strictly speaking we only 
need this for PCI devices which might be natively supported by the CSM 
BIOS, like NVMe. Devices with an OpRom already get special-cased to stay 
below 4GiB. But an IncompatiblePciDeviceSupportProtocol implementation 
doesn't get to see the PCI device class; only the vendor/device IDs so 
we can't use it for that purpose to downgrade more selectively. Instead, 
just default to putting everything below 4GiB.


David Woodhouse (4):
  OvmfPkg/LegacyBios: set NumberBbsEntries to the size of BbsTable
  OvmfPkg/LegacyBbs: Add boot entries for VirtIO and NVME devices
  OvmfPkg: Don't build in QemuVideoDxe when we have CSM
  OvmfPkg: don't assign PCI BARs above 4GiB when CSM enabled

 OvmfPkg/Csm/LegacyBiosDxe/LegacyBbs.c  | 157 ++++++++++++++++++++++++-
 OvmfPkg/Csm/LegacyBiosDxe/LegacyBios.c |   7 +-
 OvmfPkg/OvmfPkgIa32.dsc                |   2 +
 OvmfPkg/OvmfPkgIa32.fdf                |   5 +-
 OvmfPkg/OvmfPkgIa32X64.dsc             |   6 +
 OvmfPkg/OvmfPkgIa32X64.fdf             |   5 +-
 OvmfPkg/OvmfPkgX64.dsc                 |   6 +
 OvmfPkg/OvmfPkgX64.fdf                 |   5 +-
 8 files changed, 179 insertions(+), 14 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-06-28 13:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-27 16:36 [PATCH v3 4/4] OvmfPkg: don't assign PCI BARs above 4GiB when CSM enabled graf
2019-06-27 18:43 ` Laszlo Ersek
2019-06-27 19:01   ` Laszlo Ersek
2019-06-27 19:16     ` David Woodhouse
2019-06-28 13:12       ` Laszlo Ersek
  -- strict thread matches above, loose matches on Subject: below --
2019-06-26 11:37 [PATCH v3 0/4] OvmfPkg: CSM boot fixes David Woodhouse
2019-06-26 11:37 ` [PATCH v3 4/4] OvmfPkg: don't assign PCI BARs above 4GiB when CSM enabled David Woodhouse

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox