Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lersek queue 2020-Apr-01 #491

Merged
merged 5 commits into from
Apr 1, 2020
Merged

Conversation

lersek
Copy link
Member

@lersek lersek commented Apr 1, 2020

Vitaly Cheptsov and others added 5 commits April 1, 2020 15:42
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2642

This patch resolves the problem of using memory protection
attributes when OVMF firmware is compiled with XCODE5 and CLANGPDB.

Cc: Andrew Fish <afish@apple.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Marvin Häuser <mhaeuser@outlook.de>
Signed-off-by: Vitaly Cheptsov <vit9696@protonmail.com>
Message-Id: <20200329132158.35259-2-cheptsov@ispras.ru>
Acked-by: Laszlo Ersek <lersek@redhat.com>
[lersek@redhat.com: fix whitespace issues reported by git-am]
[lersek@redhat.com: replace "CC:" tags with "Cc:" ones for PatchCheck.py]
When DHCP is misconfigured on a network segment, such that two DHCP
servers attempt to reply to requests (and therefore race with each other),
the edk2 PXE client can confuse itself.

In PxeBcDhcp4BootInfo() / PxeBcDhcp6BootInfo(), the client may refer to a
DHCP reply packet as an "earlier" packet from the "same" DHCP server, when
in reality both packets are unrelated, and arrive from different DHCP
servers.

While the edk2 PXE client can do nothing to fix this, it should at least
not ASSERT() -- ASSERT() is for catching programming errors (violations of
invariants that are under the control of the programmer). ASSERT()s should
in particular not refer to external data (such as network packets). What's
more, in RELEASE builds, we get NULL pointer references.

Check the problem conditions with actual "if"s, and return
EFI_PROTOCOL_ERROR. This will trickle out to PxeBcLoadBootFile(), and be
reported as "PXE-E99: Unexpected network error".

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200331004749.16128-1-lersek@redhat.com>
Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Laszlo suggested that as I have contributed the OvmfPkg PVSCSI driver, I
will also register myself as a reviewer in Maintainers.txt.

In addition, as Nikita have assisted the development of the PVSCSI
driver and have developed another similar OvmfPkg SCSI driver, add him
as a reviewer to PVSCSI driver as-well.

Cc: Nikita Leshenko <nikita.leshchenko@oracle.com>
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
Message-Id: <20200331110244.51409-1-liran.alon@oracle.com>
Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
[lersek@redhat.com: fixup "F:" pattern by appending slash character]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Sean reported that VS2019 build produce the following build error:
INFO - PvScsi.c
INFO - Generating code
INFO - d:\a\1\s\OvmfPkg\PvScsiDxe\PvScsi.c(459): error C2220: the
       following warning is treated as an error
INFO - d:\a\1\s\OvmfPkg\PvScsiDxe\PvScsi.c(459): warning C4244: '=':
       conversion from 'const UINT16' to 'UINT8', possible loss of data

This result from an implicit cast from PVSCSI Response->ScsiStatus
(Which is UINT16) to Packet->TargetResponse (Which is UINT8).

Fix this issue by adding an appropriate explicit cast and verify with
assert that this truncation do not result in loss of data.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2651
Reported-by: Sean Brogan <sean.brogan@microsoft.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
Message-Id: <20200331110452.51992-1-liran.alon@oracle.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
[lersek@redhat.com: rewrap VS2019 diags in commit msg for PatchCheck.py]
Previous to this change, PvScsiFreeRings() was not undoing all
operations that was done by PvScsiInitRings().
This is because PvScsiInitRings() was both preparing rings (Allocate
memory and map it for device DMA) and setup the rings against device by
issueing a device command. While PvScsiFreeRings() only unmaps the rings
and free their memory.

Driver do not have a functional error as it makes sure to reset device
before every call site to PvScsiFreeRings(). However, this is not
intuitive.

Therefore, prefer to refactor the setup of the ring against device to a
separate function than PvScsiInitRings().

Signed-off-by: Liran Alon <liran.alon@oracle.com>
Message-Id: <20200331225637.123318-1-liran.alon@oracle.com>
[lersek@redhat.com: rename FreeDMACommBuffer label to FreeDmaCommBuffer]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
@lersek lersek added the push Auto push patch series in PR if all checks pass label Apr 1, 2020
@mergify mergify bot merged commit e210fc1 into tianocore:master Apr 1, 2020
@lersek lersek deleted the queue-2020-04-01 branch April 1, 2020 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
push Auto push patch series in PR if all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant