Revision: 14970
http://sourceforge.net/p/edk2/code/14970
Author: jljusten
Date: 2013-12-12 17:28:05 +0000 (Thu, 12 Dec 2013)
Log Message:
-----------
OvmfPkg: Virtio drivers: fix incorrect casts in init functions
The recent patch
OvmfPkg: Make the VirtIo devices use the new VIRTIO_DEVICE_PROTOCOL
was fixed up at commit time, in order to silence warnings issued by the
Visual Studio compiler. Differences between the posted and committed
patch:
> diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
> b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
> -index 17b9f71..96a0d9f 100644
> +index 17b9f71..f09b0d1 100644
> --- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
> +++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
> @@ -23,7 +23,6 @@
> @@ -994,7 +998,7 @@
> + // step 4c -- Report GPFN (guest-physical frame number) of queue.
> + //
> + Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo,
> -+ (UINTN) Dev->Ring.Base >> EFI_PAGE_SHIFT);
> ++ (UINT32)(UINTN) Dev->Ring.Base >> EFI_PAGE_SHIFT);
> + if (EFI_ERROR (Status)) {
> + goto ReleaseQueue;
> + }
> @@ -1495,7 +1499,7 @@
> goto Exit;
> }
> diff --git a/OvmfPkg/VirtioNetDxe/SnpInitialize.c
> b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
> -index 6cee014..8dcf9da 100644
> +index 6cee014..4203fbd 100644
> --- a/OvmfPkg/VirtioNetDxe/SnpInitialize.c
> +++ b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
> @@ -57,14 +57,15 @@ VirtioNetInitRing (
> @@ -1539,7 +1543,7 @@
> - Status = VIRTIO_CFG_WRITE (Dev, Generic.VhdrQueueAddress,
> - (UINTN) Ring->Base >> EFI_PAGE_SHIFT);
> + Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo,
> -+ (UINTN) Ring->Base >> EFI_PAGE_SHIFT);
> ++ (UINT32)(UINTN) Ring->Base >> EFI_PAGE_SHIFT);
> if (EFI_ERROR (Status)) {
> - VirtioRingUninit (Ring);
> + goto ReleaseQueue;
> @@ -1721,7 +1725,7 @@
> Exit:
> gBS->RestoreTPL (OldTpl);
> diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
> b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
> -index b836fb3..bcec676 100644
> +index b836fb3..2223c9c 100644
> --- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
> +++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
> @@ -38,7 +38,6 @@
> @@ -1908,7 +1912,7 @@
> + // step 4c -- Report GPFN (guest-physical frame number) of queue.
> + //
> + Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo,
> -+ (UINTN) Dev->Ring.Base >> EFI_PAGE_SHIFT);
> ++ (UINT32)(UINTN) Dev->Ring.Base >> EFI_PAGE_SHIFT);
> if (EFI_ERROR (Status)) {
> goto ReleaseQueue;
> }
These casts are incorrect -- they throw away address bits >=32 before
shifting, which can break the drivers in guests with more than 4GB RAM.
The bug is clearly an artifact of the edk2 coding style, which requires
cast expressions to be written as
(type) expression
rather than the usual
(type)expression
The latter correctly reflects that casts have one of the strongest
bindings in C. The former actively obscures that fact. Cf.
(type) expr1 >> expr2
vs.
(type)expr1 >> expr2
Make sure we shift before we truncate.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <[email protected]>
Reviewed-by: Jordan Justen <[email protected]>
Modified Paths:
--------------
trunk/edk2/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
trunk/edk2/OvmfPkg/VirtioNetDxe/SnpInitialize.c
trunk/edk2/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
Modified: trunk/edk2/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
===================================================================
--- trunk/edk2/OvmfPkg/VirtioBlkDxe/VirtioBlk.c 2013-12-12 17:27:27 UTC (rev
14969)
+++ trunk/edk2/OvmfPkg/VirtioBlkDxe/VirtioBlk.c 2013-12-12 17:28:05 UTC (rev
14970)
@@ -700,7 +700,7 @@
// step 4c -- Report GPFN (guest-physical frame number) of queue.
//
Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo,
- (UINT32)(UINTN) Dev->Ring.Base >> EFI_PAGE_SHIFT);
+ (UINT32) ((UINTN) Dev->Ring.Base >> EFI_PAGE_SHIFT));
if (EFI_ERROR (Status)) {
goto ReleaseQueue;
}
Modified: trunk/edk2/OvmfPkg/VirtioNetDxe/SnpInitialize.c
===================================================================
--- trunk/edk2/OvmfPkg/VirtioNetDxe/SnpInitialize.c 2013-12-12 17:27:27 UTC
(rev 14969)
+++ trunk/edk2/OvmfPkg/VirtioNetDxe/SnpInitialize.c 2013-12-12 17:28:05 UTC
(rev 14970)
@@ -96,7 +96,7 @@
// step 4c -- report GPFN (guest-physical frame number) of queue
//
Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo,
- (UINT32)(UINTN) Ring->Base >> EFI_PAGE_SHIFT);
+ (UINT32) ((UINTN) Ring->Base >> EFI_PAGE_SHIFT));
if (EFI_ERROR (Status)) {
goto ReleaseQueue;
}
Modified: trunk/edk2/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
===================================================================
--- trunk/edk2/OvmfPkg/VirtioScsiDxe/VirtioScsi.c 2013-12-12 17:27:27 UTC
(rev 14969)
+++ trunk/edk2/OvmfPkg/VirtioScsiDxe/VirtioScsi.c 2013-12-12 17:28:05 UTC
(rev 14970)
@@ -842,7 +842,7 @@
// step 4c -- Report GPFN (guest-physical frame number) of queue.
//
Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo,
- (UINT32)(UINTN) Dev->Ring.Base >> EFI_PAGE_SHIFT);
+ (UINT32) ((UINTN) Dev->Ring.Base >> EFI_PAGE_SHIFT));
if (EFI_ERROR (Status)) {
goto ReleaseQueue;
}
This was sent by the SourceForge.net collaborative development platform, the
world's largest Open Source development site.
------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT
organizations don't have a clear picture of how application performance
affects their revenue. With AppDynamics, you get 100% visibility into your
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
_______________________________________________
edk2-commits mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-commits
