Commit Graph

1368 Commits

Author SHA1 Message Date
Laszlo Ersek 39b9a5ffe6 OvmfPkg/PlatformPei: fix MTRR for low-RAM sizes that have many bits clear
Assume that we boot OVMF in a QEMU guest with 1025 MB of RAM. The
following assertion will fire:

> ASSERT_EFI_ERROR (Status = Out of Resources)
> ASSERT OvmfPkg/PlatformPei/MemDetect.c(696): !EFI_ERROR (Status)

That's because the range [1025 MB, 4 GB) that we try to mark as
uncacheable with MTRRs has size 3071 MB:

   0x1_0000_0000
  -0x0_4010_0000
  --------------
   0x0_BFF0_0000

The integer that stands for the uncacheable area size has 11 (eleven) bits
set to 1. As a result, covering this size requires 11 variable MTRRs (each
MTRR must cover a naturally aligned, power-of-two sized area). But, if we
need more variable MTRRs than the CPU can muster (such as 8), then
MtrrSetMemoryAttribute() fails, and we refuse to continue booting (which
is justified, in itself).

Unfortunately, this is not difficult to trigger, and the error message is
well-hidden from end-users, in the OVMF debug log. The following
mitigation is inspired by SeaBIOS:

Truncate the uncacheable area size to a power-of-two, while keeping the
end fixed at 4 GB. Such an interval can be covered by just one variable
MTRR.

This may leave such an MMIO gap, between the end of low-RAM and the start
of the uncacheable area, that is marked as WB (through the MTRR default).
Raise the base of the 32-bit PCI MMIO aperture accordingly -- the gap will
not be used for anything.

On Q35, the minimal 32-bit PCI MMIO aperture (triggered by RAM size 2815
MB) shrinks from

  0xE000_0000 - 0xAFF0_0000 = 769 MB

to

  0xE000_0000 - 0xC000_0000 = 512 MB

On i440fx, the minimal 32-bit PCI MMIO aperture (triggered by RAM size
3583 MB) shrinks from

  0xFC00_0000 - 0xDFF0_0000 = 449 MB

to

  0xFC00_0000 - 0xE000_0000 = 448 MB

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1814
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1666941
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1701710
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
2019-05-16 21:18:58 +02:00
Laszlo Ersek 75136b2954 OvmfPkg/PlatformPei: reorder the 32-bit PCI window vs. the PCIEXBAR on q35
Commit 7b8fe63561 ("OvmfPkg: PlatformPei: enable PCIEXBAR (aka MMCONFIG
/ ECAM) on Q35", 2016-03-10) claimed that,

  On Q35 machine types that QEMU intends to support in the long term, QEMU
  never lets the RAM below 4 GB exceed 2 GB.

Alas, this statement came from a misunderstanding that occurred while we
worked out the interface contract. In fact QEMU does allow the 32-bit RAM
extend up to 0xB000_0000 (exclusive), in case the RAM size falls in the
range (0x8000_0000, 0xB000_0000) (i.e., the RAM size is greater than
2048MB and smaller than 2816MB).

In turn, such a RAM size (justifiedly) triggers

  ASSERT (TopOfLowRam <= PciExBarBase);

in MemMapInitialization(), because we placed the 256MB PCIEXBAR at
0x8000_0000 (2GB) exactly, relying on the interface contract. (And, the
32-bit PCI window would follow the PCIEXBAR, covering the [0x9000_0000,
0xFC00_0000) range.)

In order to fix this, reorder the 32-bit PCI window against the PCIEXBAR,
as follows:

- start the 32-bit PCI window where it starts on i440fx as well, that is,
  at 2GB or TopOfLowRam, whichever is higher;

- unlike on i440fx, where the 32-bit PCI window extends up to 0xFC00_0000,
  stop it at 0xE000_0000 on q35,

- place the PCIEXBAR at 0xE000_0000.

(We cannot place the PCIEXBAR at 0xF000_0000 because the 256MB MMIO area
that starts there is not entirely free.)

Before this patch, the 32-bit PCI window used to only *end* at the same
spot (namely, 0xFC00_0000) between i440fx and q35; now it will only
*start* at the same spot (namely, 2GB or TopOfLowRam, whichever is higher)
between both boards.

On q35, the maximal window shrinks from

  0xFC00_0000 - 0x9000_0000 = 0x6C00_0000 == 1728 MB

to

  0xE000_0000 - 0x8000_0000 == 1536 MB.

We lose 192 MB of the aperture; however, the aperture is now aligned at
1GB, rather than 256 MB, and so it could fit a 1GB BAR even.

Regarding the minimal window (triggered by RAM size 2815MB), its size is

  0xE000_0000 - 0xAFF0_0000 = 769 MB

which is not great, but probably better than a failed ASSERT.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1814
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1666941
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1701710
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
2019-05-16 21:18:54 +02:00
Laszlo Ersek 9a2e8d7c65 OvmfPkg/PlatformPei: hoist PciBase assignment above the i440fx/q35 branching
In the MemMapInitialization() function, we currently assign PciBase
different values, on both branches of the board type check. Hoist the
PciBase assignment from the i440fx branch in front of the "if". This is a
no-op for the i440fx branch. On the q35 branch, we overwrite this value,
hence the change is a no-op on q35 as well.

This is another refactoring for simplifying the rest of this series.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1814
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1666941
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1701710
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
2019-05-16 21:18:52 +02:00
Laszlo Ersek 60e95bf509 OvmfPkg/PlatformPei: assign PciSize on both i440fx/q35 branches explicitly
In the MemMapInitialization() function, we currently have a common
PciSize assignment, shared between i440fx and q35. In order to simplify
the rest of this series, lift and duplicate the assignment identically to
both board-specific branches.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1814
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1666941
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1701710
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
2019-05-16 21:18:41 +02:00
Fu Siyuan 631195044f OvmfPkg: Update DSC/FDF to use NetworkPkg's include fragment file.
This patch updates the platform DSC/FDF files to use the include fragment
files provided by NetworkPkg.
The feature enabling flags in [Defines] section have been updated to use
the NetworkPkg's terms, and the value has been overridden with the original
default value on this platform.

v2:1.Make the comments before Network definition align other parts.
   2.Set NETWORK_ALLOW_HTTP_CONNECTIONS true.
   3.Remove TcpIoLib in lib classes section.
   4.Withdraw the removal of [PcdsFixedAtBuild.X64].

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien.grall@linaro.org>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
Message-Id: <20190516081810.27840-2-shenglei.zhang@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1293
[lersek@redhat.com: add TianoCore Bugzilla reference]
2019-05-16 16:28:49 +02:00
Gary Lin 89d7c543cf OvmfPkg/EnrollDefaultKeys: import the non-default key into db
For QA test and development, we may need to test Secure Boot with a
devel key instead of UEFI CA.

This commit adds an argument, "--no-default", to EnrollDefaultKeys.efi.
With the argument, the key from SMBIOS Type 11 will also be enrolled
into db. Besides, the keys in AuthData.c, i.e. Microsoft KEK CA,
Microsoft PCA, and Microsoft UEFI CA will be excluded, so the developer
can easily create a varstore template for a specific key.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Signed-off-by: Gary Lin <glin@suse.com>
Message-Id: <20190516030834.12203-1-glin@suse.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2019-05-16 15:36:04 +02:00
Zhang, Shenglei a11d371ef6 OvmfPkg: Remove duplicated library classes in dxe drivers section in DSC
NetLib,IpIoLib,UdpIoLib and DpcLib already exist in [LibraryClasses].
So the ones in [LibraryClasses.common.DXE_DRIVER] are duplicated,
which should be removed.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien.grall@linaro.org>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
Message-Id: <20190515072419.23792-1-shenglei.zhang@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1293
[lersek@redhat.com: add TianoCore BZ reference]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2019-05-15 11:08:50 +02:00
Michael D Kinney cd5147734c OvmfPkg/QemuVideoDxe: Remove dependency on OptionRomPkg
Update the QemuVideoDxe driver to not depend on the
OptionRomPkg to support moving OptionRomPkg to the
edk2-platforms repository.

The only dependency on the OptionRomPkg is the use of
PcdDriverSupportedEfiVersion to set the version value in the
EFI Driver Supported EFI Version Protocol.

Quoting the UEFI-2.8 spec, "This protocol is required for EFI
drivers that are *on* PCI and other plug in cards" (emphasis ours).
However, QemuVideoDxe is always part of the OVMF platform firmware,
and is never read by PciBusDxe from the PCI ROM BAR of QEMU's
emulated graphics cards.

Since this protocol does not apply to QEMU environment, remove
both the PCD and the installation of the EFI Driver Supported EFI
Version Protocol.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2019-05-09 20:03:26 -07:00
Laszlo Ersek 727d7ebaa9 OvmfPkg/EnrollDefaultKeys: suppress incorrect compiler warning
When building OvmfPkg/EnrollDefaultKeys for IA32 with gcc-4.8 for the
DEBUG target (and possibly under other build configurations too), the
compiler incorrectly reports,

> OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c: In function
> 'ShellAppMain':
> OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c:631:10: error:
> 'SizeOfPkKek1' may be used uninitialized in this function
> [-Werror=maybe-uninitialized]
>    Status = EnrollListOfCerts (
>           ^
> OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c:703:12: error: 'PkKek1'
> may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    FreePool (PkKek1);
>             ^
> cc1: all warnings being treated as errors

Suppress this warning, in the style suggested under
<https://bugzilla.tianocore.org/show_bug.cgi?id=607>.

Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Fixes: be9470b3c9
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
2019-05-01 08:30:35 +02:00
Igor Druzhinin 0c40c9c925 OvmfPkg/XenSupport: turn off address decoding before BAR sizing
On Xen, hvmloader firmware leaves address decoding enabled for
enumerated PCI device before jumping into OVMF. OVMF seems to
expect it to be disabled and tries to size PCI BARs in several places
without disabling it which causes BAR64, for example, being
incorrectly placed by QEMU.

Fix it by disabling PCI address decoding explicitly before the
first attempt to size BARs on Xen.

Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2019-04-30 18:42:35 +02:00
Igor Druzhinin d63610df60 OvmfPkg/XenSupport: use a correct PCI host bridge aperture for BAR64
In case BAR64 is placed below 4G choose the correct aperture.
This fixes a failed assertion down the code path.

Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
2019-04-30 18:41:57 +02:00
Igor Druzhinin 27e4d6ae25 OvmfPkg/XenSupport: remove usage of prefetchable PCI host bridge aperture
This aperture doesn't exist in QEMU-XEN and hvmloader places BARs
in arbitrary order disregarding prefetchable bit. This makes
prefetchable and non-prefetchable BARs to follow each other that's
quite likely with PCI passthrough devices. In that case, the existing
code, that tries to work out aperture boundaries by reading hvmloader
BAR placement, will report a bogus prefetchable aperture which overlaps
with the regular one. It will eventually trigger an assertion in
DXE PCI initialization code.

Do the same thing as OVMF on QEMU-KVM and pass a non-existing aperture
there. It's not necessary to pass additional allocation flags as we set
ResourceAssigned flag on the root bridge which means they will be ignored.

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
2019-04-30 18:41:57 +02:00
Laszlo Ersek 9fb2ce2f46 OvmfPkg/EnrollDefaultKeys: remove Red Hat's hard-coded PK/KEK1
The certificate

  "Red Hat Secure Boot (PK/KEK key 1)/emailAddress=secalert@redhat.com"
  SHA1: fd:fc:7f:3c:7e:f3:e0:57:76:ad:d7:98:78:21:6c:9b:e0:e1:95:97

is no longer referenced; remove it.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:49 +02:00
Laszlo Ersek be9470b3c9 OvmfPkg/EnrollDefaultKeys: enroll PK/KEK1 from the Type 11 SMBIOS table
Disconnect the certificate that is enrolled as both Platform Key and first
Key Exchange Key from Red Hat: expect the hypervisor to specify it, as
part of SMBIOS.

Example usage with QEMU:

* Generate self-signed X509 certificate:

  openssl req \
    -x509 \
    -newkey rsa:2048 \
    -outform PEM \
    -keyout PkKek1.private.key \
    -out PkKek1.pem

  (where "PEM" simply means "DER + base64 + header + footer").

* Strip the header, footer, and newline characters; prepend the
  application prefix:

  sed \
    -e 's/^-----BEGIN CERTIFICATE-----$/4e32566d-8e9e-4f52-81d3-5bb9715f9727:/' \
    -e '/^-----END CERTIFICATE-----$/d' \
    PkKek1.pem \
  | tr -d '\n' \
  > PkKek1.oemstr

* Pass the certificate to EnrollDefaultKeys with the following QEMU
  option:

  -smbios type=11,value="$(< PkKek1.oemstr)"

  (Note: for the above option to work correctly, a QEMU version is needed
  that includes commit 950c4e6c94b1 ("opts: don't silently truncate long
  option values", 2018-05-09). The first upstream release with that commit
  was v3.0.0.

  Once <https://bugs.launchpad.net/qemu/+bug/1826200> is fixed, QEMU will
  learn to read the file directly; passing the blob on the command will be
  necessary no more.)

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:46 +02:00
Laszlo Ersek 392404167f OvmfPkg: introduce OVMF_PK_KEK1_APP_PREFIX_GUID
For the EnrollDefaultKeys application, the hypervisor is expected to add a
string entry to the "OEM Strings" (Type 11) SMBIOS table, with the
following format:

4e32566d-8e9e-4f52-81d3-5bb9715f9727:<Base64 X509 cert for PK and first KEK>

The string representation of the GUID at the front is the "application
prefix", in terms of QEMU commit
<https://git.qemu.org/?p=qemu.git;a=commit;h=2d6dcbf93fb0>.

Introduce this GUID in the usual manner.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:45 +02:00
Laszlo Ersek c9727ff1df OvmfPkg/EnrollDefaultKeys: document the steps of the entry point function
The entry point function of EnrollDefaultKeys finishes with a sanity
check, verifying the values of the Secure Boot-related "control"
variables. Add a diagram to explain why we expect the values we do.

While at it, write comments on the rest of the entry point function.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:43 +02:00
Laszlo Ersek 86bf2672a3 OvmfPkg/EnrollDefaultKeys: describe functions with leading comment blocks
The GetExact(), GetSettings(), PrintSettings(), and ShellAppMain()
functions lack leading comment blocks. Supply those.

While at it, make sure that every such comment block is preceded by two
blank lines.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:42 +02:00
Laszlo Ersek 7eeaa7585a OvmfPkg/EnrollDefaultKeys: extract MICROSOFT_VENDOR_GUID
The GUID

  77FA9ABD-0359-4D32-BD60-28F4E78F784B

is specified in MSDN, at
<https://msdn.microsoft.com/en-us/ie/dn932805(v=vs.94)>, therefore it
deserves an entry in the package DEC file, and a header file under
"Include/Guid".

(Arguably, this GUID declaration / definition could even live under
SecurityPkg, but the edk2 tradition has been to hoist GUIDs,
protocols/PPIs, and lib classes from OvmfPkg to a core package only when
dependent C code is added to the core package.)

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:40 +02:00
Laszlo Ersek a79b115ade OvmfPkg/EnrollDefaultKeys: split out certificate and signature constants
A large portion of "EnrollDefaultKeys.c" is hex-encoded X509 certificates,
GUIDs, and signatures. These objects are constants, unlikely to see
changes anytime soon. Move them out of the way, to "AuthData.c", so we can
more easily work on functions in "EnrollDefaultKeys.c".

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:39 +02:00
Laszlo Ersek 1c9418fcaf OvmfPkg/EnrollDefaultKeys: extract typedefs to a header file
"EnrollDefaultKeys.c" defines three structure types: SINGLE_HEADER,
REPEATING_HEADER, and SETTINGS. The definitions are scattered over the C
file, and lack high-level summary comments.

Extract the structures to "EnrollDefaultKeys.h", and add the missing
comments.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:37 +02:00
Laszlo Ersek a2491a6c82 OvmfPkg/EnrollDefaultKeys: remove unneeded EFIAPI call. conv. specifiers
The GetExact(), GetSettings(), PrintSettings() functions are only called
from within "EnrollDefaultKeys.c", and none of them take variable
arguments. Drop their EFIAPI calling convention specifiers.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:35 +02:00
Laszlo Ersek e0d612356e OvmfPkg/EnrollDefaultKeys: clean up acronym capitalization in identifiers
According to the edk2 coding standards, "[w]hen all letters in an acronym
are capitalized, it makes the prior and subsequent words visually
difficult to distinguish".

Fix the spellings of three acronyms, accordingly:
- "KEK" (Key Exchange Key) should be written as "Kek", in "mMicrosoftKEK",
- "CA" (Certificate Authority) should be written as "Ca", in
  "mMicrosoftUefiCA",
- "PCA" (Production Certificate Authority) should be written as "Pca", in
  "mMicrosoftPCA".

Generate the changes with:

  sed --regexp-extended --in-place \
    --expression='s,\<mMicrosoftKEK\>,mMicrosoftKek,g' \
    --expression='s,\<mMicrosoftUefiCA\>,mMicrosoftUefiCa,g' \
    --expression='s,\<mMicrosoftPCA\>,mMicrosoftPca,g' \
    OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:33 +02:00
Laszlo Ersek 32d1440aad OvmfPkg/EnrollDefaultKeys: clean up global variable name prefixes
In edk2, we should start the names of module-global variables with "m".
Rename the "RedHatPkKek1", "MicrosoftKEK", "MicrosoftPCA",
"MicrosoftUefiCA" variables accordingly, with the following command:

  sed --regexp-extended --in-place \
    --expression='s,\<(RedHatPkKek1|Microsoft(KEK|PCA|UefiCA))\>,m\1,g' \
    OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:31 +02:00
Laszlo Ersek 85d96998a4 OvmfPkg/EnrollDefaultKeys: clean up minor whitespace wart
In edk2, we should spell "#pragma pack(...)" with a space character in
front of the opening parenthesis. Fix up locations that suggest otherwise.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:29 +02:00
Laszlo Ersek 05d7f3c3fc OvmfPkg/EnrollDefaultKeys: refresh INF file
Bump INF_VERSION to the latest edk2 INF specification.

Regenerate FILE_GUID to distinguish this application from downstream-only
versions.

Remove the VALID_ARCHITECTURES comment as there is nothing ISA or platform
specific in the application.

Ensure all sections apart from [Defines] are alphabetically sorted.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:25 +02:00
Laszlo Ersek 3defea06b2 OvmfPkg/EnrollDefaultKeys: update @file comment blocks
Refresh the sentence that states the purpose of the application.

Extend the copyright notice to the year 2019.

Replace the 2-clause BSD License banner with the BSD-2-Clause-Patent SPDX
ID.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:24 +02:00
Laszlo Ersek b11636239f OvmfPkg: introduce EnrollDefaultKeys application
Add the OvmfPkg/EnrollDefaultKeys shell application source as it is at the
"edk2-20190308git89910a39dcfd-2.el8" tag in RHEL8's downstream "edk2"
package.

Further patches in this series will replace Red Hat-specific bits in the
application, with a conduit to pass in parameters directly from the host
side, on both QEMU and Xen.

(Note: at the moment, Xen doesn't support Secure Boot, due to lacking a
standards-conformant variable driver stack. However, that could change
soon
<https://fosdem.org/2019/schedule/event/vai_implementing_uefi_variable_services_in_qemu/>,
and then this facility will become useful on Xen too.)

The use case for this application (including why it is only being added to
the DSC files) is explained in detail in
<https://bugzilla.tianocore.org/show_bug.cgi?id=1747#c0>.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Gary Lin <glin@suse.com>
2019-04-30 14:26:11 +02:00
Shenglei Zhang a8a8b408b7 OvmfPkg/PlatformBootManagerLib: Remove dependency on Mps.h
Mps.h is included in BdsPlatform.h but not actually used.
So remove it.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
2019-04-28 09:50:13 +08:00
Zhichao Gao 84c0b80de7 OvmfPkg/ResetSystemLib: Add new API ResetSystem
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1460

Add a new API ResetSystem to this ResetSystemLib instance.
It only adds the basic functions from ResetSystemRuntimeDxe.
Lacking of this interface may cause link error, if some drivers
use this new API and link to this library instance.
Notes:
This library API only provide a basic function of reset. Full
function should use the instance in the MdeModulePkg and make
sure the depex driver is dispatched.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2019-04-28 09:40:18 +08:00
Laszlo Ersek b9d4847ec2 OvmfPkg/Sec: fix out-of-bounds reads
RH covscan justifiedly reports that accessing "EFI_FFS_FILE_HEADER.Size"
and "EFI_COMMON_SECTION_HEADER.Size", which both are of type UINT8[3],
through (UINT32*), is undefined behavior:

> Error: OVERRUN (CWE-119):
> edk2-89910a39dcfd/OvmfPkg/Sec/SecMain.c:283: overrun-local: Overrunning
> array of 3 bytes at byte offset 3 by dereferencing pointer
> "(UINT32 *)File->Size".
> #  281|
> #  282|       File = (EFI_FFS_FILE_HEADER*)(UINTN) CurrentAddress;
> #  283|->     Size = *(UINT32*) File->Size & 0xffffff;
> #  284|       if (Size < (sizeof (*File) + sizeof (EFI_COMMON_SECTION_HEADER))) {
> #  285|         return EFI_VOLUME_CORRUPTED;
>
> Error: OVERRUN (CWE-119):
> edk2-89910a39dcfd/OvmfPkg/Sec/SecMain.c:614: overrun-local: Overrunning
> array of 3 bytes at byte offset 3 by dereferencing pointer
> "(UINT32 *)File->Size".
> #  612|
> #  613|       File = (EFI_FFS_FILE_HEADER*)(UINTN) CurrentAddress;
> #  614|->     Size = *(UINT32*) File->Size & 0xffffff;
> #  615|       if (Size < sizeof (*File)) {
> #  616|         return EFI_NOT_FOUND;
>
> Error: OVERRUN (CWE-119):
> edk2-89910a39dcfd/OvmfPkg/Sec/SecMain.c:639: overrun-local: Overrunning
> array of 3 bytes at byte offset 3 by dereferencing pointer
> "(UINT32 *)Section->Size".
> #  637|         Section = (EFI_COMMON_SECTION_HEADER*)(UINTN) CurrentAddress;
> #  638|
> #  639|->       Size = *(UINT32*) Section->Size & 0xffffff;
> #  640|         if (Size < sizeof (*Section)) {
> #  641|           return EFI_NOT_FOUND;

Fix these by invoking the FFS_FILE_SIZE() and SECTION_SIZE() macros, which
by now have been fixed too.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710
Issue: scan-1008.txt
Issue: scan-1009.txt
Issue: scan-1010.txt
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
2019-04-24 17:31:02 +02:00
Laszlo Ersek c2f643479e OvmfPkg/BasePciCapLib: suppress invalid "nullptr deref" warning
RH covscan reports the following "nullptr deref" warning:

> Error: CLANG_WARNING:
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:312:5:
> warning: Dereference of null pointer
> #    InstanceZero->NumInstancesUnion.NumInstances++;
> #    ^
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:509:7:
> note: Assuming 'OutCapList' is not equal to NULL
> #  if (OutCapList == NULL) {
> #      ^~~~~~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:509:3:
> note: Taking false branch
> #  if (OutCapList == NULL) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:518:7:
> note: Assuming the condition is false
> #  if (OutCapList->Capabilities == NULL) {
> #      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:518:3:
> note: Taking false branch
> #  if (OutCapList->Capabilities == NULL) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:529:7:
> note: Assuming 'CapHdrOffsets' is not equal to NULL
> #  if (CapHdrOffsets == NULL) {
> #      ^~~~~~~~~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:529:3:
> note: Taking false branch
> #  if (CapHdrOffsets == NULL) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:546:3:
> note: Taking false branch
> #  if (RETURN_ERROR (Status)) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:549:7:
> note: Assuming the condition is true
> #  if ((PciStatusReg & EFI_PCI_STATUS_CAPABILITY) != 0) {
> #      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:549:3:
> note: Taking true branch
> #  if ((PciStatusReg & EFI_PCI_STATUS_CAPABILITY) != 0) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:557:5:
> note: Taking false branch
> #    if (RETURN_ERROR (Status)) {
> #    ^
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:565:12:
> note: Assuming 'NormalCapHdrOffset' is > 0
> #    while (NormalCapHdrOffset > 0) {
> #           ^~~~~~~~~~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:565:5:
> note: Loop condition is true.  Entering loop body
> #    while (NormalCapHdrOffset > 0) {
> #    ^
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:570:7:
> note: Taking false branch
> #      if (RETURN_ERROR (Status)) {
> #      ^
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:574:16:
> note: Calling 'InsertPciCap'
> #      Status = InsertPciCap (OutCapList, CapHdrOffsets, PciCapNormal,
> #               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:235:3:
> note: Null pointer value stored to 'InstanceZero'
> #  InstanceZero = NULL;
> #  ^~~~~~~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:243:7:
> note: Assuming 'PciCap' is not equal to NULL
> #  if (PciCap == NULL) {
> #      ^~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:243:3:
> note: Taking false branch
> #  if (PciCap == NULL) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:259:3:
> note: Taking false branch
> #  if (RETURN_ERROR (Status)) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:297:3:
> note: Taking false branch
> #  if (RETURN_ERROR (Status)) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:311:7:
> note: Assuming the condition is true
> #  if (PciCap->Key.Instance > 0) {
> #      ^~~~~~~~~~~~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:311:3:
> note: Taking true branch
> #  if (PciCap->Key.Instance > 0) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:312:5:
> note: Dereference of null pointer
> #    InstanceZero->NumInstancesUnion.NumInstances++;
> #    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> #  310|     //
> #  311|     if (PciCap->Key.Instance > 0) {
> #  312|->     InstanceZero->NumInstancesUnion.NumInstances++;
> #  313|     }
> #  314|     return RETURN_SUCCESS;

The warning is invalid: the flagged dereferencing of "InstanceZero" is
gated by a condition that is only satisfied if we dereference
"InstanceZero" *first*.

(Perhaps the analyzer assumes that the OrderedCollectionInsert() call,
just before line 259, can change the value of "PciCap->Key.Instance" via
the last argument:

   254    //
   255    // Add PciCap to CapList.
   256    //
   257    Status = OrderedCollectionInsert (CapList->Capabilities, &PciCapEntry,
   258               PciCap);
   259    if (RETURN_ERROR (Status)) {

That assumption is incorrect.)

Add a comment and an ASSERT().

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710
Issue: scan-0994.txt
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
2019-04-18 16:06:04 +02:00
Laszlo Ersek e30991740d OvmfPkg/AcpiPlatformDxe: catch theoretical nullptr deref in Xen code
RH covscan justifiedly reports a path through InstallXenTables() where
DsdtTable can technically remain NULL.

If this occurs in practice, then the guest and the VMM are out of sync on
the interface contract. Catch the situation with a code snippet that halts
in RELEASE builds, and in DEBUG builds lets the platform DSC control the
assert disposition first (i.e. CPU exception, deadloop, or nothing).

> Error: CLANG_WARNING:
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:309:14: warning: Access
> to field 'Length' results in a dereference of a null pointer (loaded
> from variable 'DsdtTable')
> #             DsdtTable->Length,
> #             ^~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:154:3: note: Null
> pointer value stored to 'DsdtTable'
> #  DsdtTable   = NULL;
> #  ^~~~~~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:162:3: note: Taking
> false branch
> #  if (EFI_ERROR (Status)) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:170:7: note: Assuming
> the condition is false
> #  if (XenAcpiRsdpStructurePtr->XsdtAddress) {
> #      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:170:3: note: Taking
> false branch
> #  if (XenAcpiRsdpStructurePtr->XsdtAddress) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:220:12: note: Assuming
> the condition is false
> #  else if (XenAcpiRsdpStructurePtr->RsdtAddress) {
> #           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:220:8: note: Taking
> false branch
> #  else if (XenAcpiRsdpStructurePtr->RsdtAddress) {
> #       ^
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:274:3: note: Taking
> false branch
> #  if (Fadt2Table) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:288:8: note: Taking
> false branch
> #  else if (Fadt1Table) {
> #       ^
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:309:14: note: Access to
> field 'Length' results in a dereference of a null pointer (loaded from
> variable 'DsdtTable')
> #             DsdtTable->Length,
> #             ^~~~~~~~~
> #  307|                AcpiProtocol,
> #  308|                DsdtTable,
> #  309|->              DsdtTable->Length,
> #  310|                &TableHandle
> #  311|                );

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710
Issue: scan-0993.txt
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
2019-04-18 16:05:24 +02:00
Laszlo Ersek dc5bbf1074 OvmfPkg/AcpiPlatformDxe: suppress invalid "deref of undef pointer" warning
RH covscan emits the following false positive:

> Error: CLANG_WARNING:
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:182:14:
> warning: Dereference of undefined pointer value
> #    Status = FwVol->ReadSection (
> #             ^~~~~~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:164:7: note:
> Assuming the condition is false
> #  if (QemuDetected ()) {
> #      ^~~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:164:3: note:
> Taking false branch
> #  if (QemuDetected ()) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:174:3: note:
> Taking false branch
> #  if (EFI_ERROR (Status)) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:180:10: note:
> Assuming 'Status' is equal to EFI_SUCCESS
> #  while (Status == EFI_SUCCESS) {
> #         ^~~~~~~~~~~~~~~~~~~~~
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:180:3: note:
> Loop condition is true.  Entering loop body
> #  while (Status == EFI_SUCCESS) {
> #  ^
> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:182:14: note:
> Dereference of undefined pointer value
> #    Status = FwVol->ReadSection (
> #             ^~~~~~~~~~~~~~~~~~
> #  180|     while (Status == EFI_SUCCESS) {
> #  181|
> #  182|->     Status = FwVol->ReadSection (
> #  183|                         FwVol,
> #  184|                         (EFI_GUID*)PcdGetPtr (PcdAcpiTableStorageFile),

This is invalid because LocateFvInstanceWithTables() sets FwVol on
success.

Suppress the message by:
- assigning FwVol NULL first (this would replace the original report with
  "nullptr deref"),
- asserting that FwVol is no longer NULL, on success.

What's important here is that ASSERT() ends with ANALYZER_UNREACHABLE() on
failure.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710
Issue: scan-0991.txt
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
2019-04-18 16:04:18 +02:00
Laszlo Ersek 52d229238b OvmfPkg/QemuVideoDxe: avoid arithmetic on null pointer
The real mode interrupt vector table, which we modify for the sake of
Windows 7, starts at address 0, which happens to be the representation of
null pointers on all edk2 architectures. A null pointer may never undergo
pointer arithmetic, and RH covscan justifiedly reports:

> Error: CPPCHECK_WARNING (CWE-682):
> edk2-89910a39dcfd/OvmfPkg/QemuVideoDxe/VbeShim.c:105:
> error[nullPointerArithmetic]: Pointer addition with NULL pointer.
> #  103|     //
> #  104|     Segment0Pages = 1;
> #  105|->   Int0x10       = (IVT_ENTRY *)(UINTN)Segment0 + 0x10;
> #  106|     Segment0AllocationStatus = gBS->AllocatePages (
> #  107|                                       AllocateAddress,

Fix this by calculating the EFI_PHYSICAL_ADDRESS of IVT entry 0x10 first,
and by casting the address to the right type second.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710
Issue: scan-1002.txt
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
2019-04-18 16:03:38 +02:00
Laszlo Ersek 34131e1b5c OvmfPkg/XenBusDxe: replace MIT license text with SPDX ID
The "XenBus.c", "XenStore.c", "XenStore.h" files under "OvmfPkg/XenBusDxe"
include the MIT License text. Replace those license blocks with
"SPDX-License-Identifier: MIT" (which refers to
<https://spdx.org/licenses/MIT.html>) for easier parsing by machines (and
humans).

This follows the tree-wide adoption of "SPDX-License-Identifier"s, made
for <https://bugzilla.tianocore.org/show_bug.cgi?id=1373>.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Lars Kurth <lars.kurth@citrix.com>
Cc: xen-devel@lists.xenproject.org
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1654
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2019-04-12 12:04:38 +02:00
Laszlo Ersek 6f21d772aa OvmfPkg/IndustryStandard/Xen: replace MIT license text with SPDX ID
The header files under "OvmfPkg/Include/IndustryStandard/Xen" include the
MIT License text. Replace those license blocks with
"SPDX-License-Identifier: MIT" (which refers to
<https://spdx.org/licenses/MIT.html>) for easier parsing by machines (and
humans).

This follows the tree-wide adoption of "SPDX-License-Identifier"s, made
for <https://bugzilla.tianocore.org/show_bug.cgi?id=1373>.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Lars Kurth <lars.kurth@citrix.com>
Cc: xen-devel@lists.xenproject.org
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1654
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2019-04-12 12:04:36 +02:00
Laszlo Ersek 07a0f27b0e OvmfPkg/License.txt: refresh the MIT license text and include the SPDX ID
Refresh the MIT license text from <https://spdx.org/licenses/MIT.html>,
and add "SPDX-License-Identifier: MIT" for easier parsing by machines.

This follows the tree-wide adoption of "SPDX-License-Identifier"s, made
for <https://bugzilla.tianocore.org/show_bug.cgi?id=1373>; in particular:

- commit 304bff7223 ("edk2: Change License.txt from 2-Clause BSD to
  BSD+Patent", 2019-04-09),

- commit 75fae505b7 ("OvmfPkg: Change License.txt from 2-Clause BSD to
  BSD+Patent", 2019-04-09).

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Lars Kurth <lars.kurth@citrix.com>
Cc: xen-devel@lists.xenproject.org
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1654
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2019-04-12 12:04:33 +02:00
Laszlo Ersek cff96d4373 OvmfPkg/License.txt: remove XenPvBlkDxe from the MIT licensed dir list
All files under "OvmfPkg/XenPvBlkDxe" carry

  SPDX-License-Identifier: BSD-2-Clause-Patent

None of them are MIT-licensed.

Update "OvmfPkg/License.txt" accordingly.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Lars Kurth <lars.kurth@citrix.com>
Cc: xen-devel@lists.xenproject.org
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1654
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2019-04-12 12:04:30 +02:00
Hao Wu 6d70ade90c OvmfPkg: Update DSC/FDF files to consume 8259/8254 drivers in OvmfPkg
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1496

This commit updates the OVMF DSC/FDF files to consume the copied
8259InterruptControllerDxe and 8254TimerDxe drivers within OvmfPkg.

The unconsumed PCD:
gPcAtChipsetPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel

is removed from DSC files as well.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2019-04-11 08:57:30 +08:00
Hao Wu e3c787af8c OvmfPkg/8254TimerDxe: Update to make it build for OVMF
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1496

This commit will remove the IntelFrameworkPkg DEC file dependency in the
driver INF file.

A new GUID has been updated for the INF file.

Corresponding changes have been made in OVMF DSC files as well in order to
verify the build.

Please note that a subsequent commit (final patch of the series) will:

A) Add OvmfPkg/8254TimerDxe driver in OVMF FDF files;
B) Remove the consumption of PcAtChipsetPkg/8254TimerDxe driver in OVMF
DSC/FDF files.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2019-04-11 08:57:29 +08:00
Hao Wu 1a3ffdff82 OvmfPkg: Copy 8254TimerDxe driver from PcAtChipsetPkg
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1496

This commit copies the exact 8254TimerDxe driver from PcAtChipsetPkg to
OvmfPkg.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2019-04-11 08:57:29 +08:00
Hao Wu 682888a6d2 OvmfPkg/AcpiPlatformDxe: Consume the 8259 PCD defined in OvmfPkg
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1496

Several updates have been made to the OvmfPkg/AcpiPlatformDxe driver to
drop its dependency on PcAtChipsetPkg:

A) Consumes the PCD 'Pcd8259LegacyModeEdgeLevel' defined within OvmfPkg;
B) Remove the PcAtChipsetPkg DEC file dependency in the driver INF file.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2019-04-11 08:57:28 +08:00
Hao Wu c01852433f OvmfPkg/8259InterruptControllerDxe: Update to make it build for OVMF
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1496

Several updates have been made to the
OvmfPkg/8259InterruptControllerDxe driver to make it build under OvmfPkg:

A) Update the driver INF file to consume PCDs defined within OvmfPkg;
B) Remove the unnecessary dependency on the IntelFrameworkPkg header file
   'FrameworkDxe.h';
C) Remove the IntelFrameworkPkg & PcAtChipsetPkg DEC files dependency in
   the driver INF file.

A new GUID has been updated for the INF file.

Corresponding changes have been made in OVMF DSC files as well in order to
verify the build.

Please note that a subsequent commit (final patch of the series) will:

A) Add OvmfPkg/8259InterruptControllerDxe driver in OVMF FDF files;
B) Remove the consumption of PcAtChipsetPkg/8259InterruptControllerDxe
   driver in OVMF DSC/FDF files;
C) Remove the consumption of
   gPcAtChipsetPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel in OVMF DSC
   files.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2019-04-11 08:57:28 +08:00
Hao Wu 460ffed242 OvmfPkg/OvmfPkg.dec: Add 8259-related PCDs in OVMF DEC file
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1496

According to the DEC file in PcAtChipsetPkg, this commit adds the two
8259-driver-related PCDs into the OvmfPkg DEC file.

Please note that, instead of adding the two PCDs under section:
[PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx, PcdsPatchableInModule]

as in PcAtChipsetPkg.dec file, they are added in section:
[PcdsFixedAtBuild]

in OvmfPkg.dec instead.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2019-04-11 08:57:27 +08:00
Hao Wu 10fa47e538 OvmfPkg: Copy Legacy8259 protocol definitions from IntelFrameworkPkg
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1496

This commit copies the exact Legacy8259 protocol header file from
IntelFrameworkPkg to OvmfPkg. Also, the protocol GUID definition is
duplicated in the OvmfPkg DEC file.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2019-04-11 08:57:26 +08:00
Hao Wu 9a86fd33e9 OvmfPkg: Copy 8259InterruptControllerDxe driver from PcAtChipsetPkg
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1496

This commit copies the exact 8259InterruptControllerDxe driver from
PcAtChipsetPkg to OvmfPkg.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2019-04-11 08:57:26 +08:00
Michael D Kinney 75fae505b7 OvmfPkg: Change License.txt from 2-Clause BSD to BSD+Patent
Change contents of License.txt in OvmfPkg of the edk2 repository
from the 2-Clause BSD License to the BSD+Patent License for the
default license type.  Preserve the contents related to the use
of the MIT license in this file.

https://bugzilla.tianocore.org/show_bug.cgi?id=1373

This change is based on the following emails:

  https://lists.01.org/pipermail/edk2-devel/2019-February/036260.html
  https://lists.01.org/pipermail/edk2-devel/2018-October/030385.html

RFCs with detailed process for the license change:

  V3: https://lists.01.org/pipermail/edk2-devel/2019-March/038116.html
  V2: https://lists.01.org/pipermail/edk2-devel/2019-March/037669.html
  V1: https://lists.01.org/pipermail/edk2-devel/2019-March/037500.html

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2019-04-09 10:58:34 -07:00
Michael D Kinney b26f0cf9ee OvmfPkg: Replace BSD License with BSD+Patent License
https://bugzilla.tianocore.org/show_bug.cgi?id=1373

Replace BSD 2-Clause License with BSD+Patent License.  This change is
based on the following emails:

  https://lists.01.org/pipermail/edk2-devel/2019-February/036260.html
  https://lists.01.org/pipermail/edk2-devel/2018-October/030385.html

RFCs with detailed process for the license change:

  V3: https://lists.01.org/pipermail/edk2-devel/2019-March/038116.html
  V2: https://lists.01.org/pipermail/edk2-devel/2019-March/037669.html
  V1: https://lists.01.org/pipermail/edk2-devel/2019-March/037500.html

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2019-04-09 10:58:19 -07:00
Bret Barkelew 2fe5f2f529 OvmfPkg/PlatformDebugLibIoPort: Add new APIs
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add new APIs' implementation (DebugVPrint, DebugBPrint)
in the DebugLib instance. These APIs would expose print
routines with VaList parameter and BaseList parameter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
2019-04-02 12:49:28 +08:00
Hao Wu a068102296 OvmfPkg: Add a build flag to select ISA driver stack
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1495

This commit will add a static build flag 'USE_LEGACY_ISA_STACK' to select
the ISA driver stack.

If the flag is set to TRUE, the below driver stack will be used:
  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf

If the flag is set to FALSE, the below driver stack will be used:
  OvmfPkg/SioBusDxe/SioBusDxe.inf
  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf

The default value is set to FALSE in OVMF DSC files.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ray Ni <ray.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Anthony PERARD <anthony.perard@citrix.com>
2019-03-27 13:13:58 +08:00