Gabriel Somlo [mailto:gso...@gmail.com] wrote:

]On Sun, Nov 09, 2014 at 05:00:38PM -0800, Jordan Justen wrote:
]> On 2014-11-08 14:01:56, Gabriel L. Somlo wrote:
]> > @@ -228,6 +229,11 @@ MiscInitialization (
]> >    VOID
]> >    )
]> >  {
]> > +  UINT16 HostBridgeDevId;
]> > +  UINTN  PmCmd;
]> > +  UINTN  Pmba;
]> > +  UINTN  PmRegMisc;
]> > +
]> >    //
]> >    // Disable A20 Mask
]> >    //
]> > @@ -239,33 +245,47 @@ MiscInitialization (
]> >    BuildCpuHob (36, 16);
]> >  
]> >    //
]> > +  // Query Host Bridge DID to determine platform type
]> > +  //
]> > +  HostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
]> > +  switch (HostBridgeDevId) {
]> > +    case INTEL_82441_DEVICE_ID:
]> > +      PmCmd     = POWER_MGMT_REGISTER_PIIX4 (PCI_COMMAND_OFFSET);
]> > +      Pmba      = POWER_MGMT_REGISTER_PIIX4 (0x40);
]> > +      PmRegMisc = POWER_MGMT_REGISTER_PIIX4 (0x80);
]> > +      break;
]> > +    case INTEL_Q35_MCH_DEVICE_ID:
]> > +      PmCmd     = POWER_MGMT_REGISTER_Q35 (PCI_COMMAND_OFFSET);
]> > +      Pmba      = POWER_MGMT_REGISTER_Q35 (0x40);
]> > +      PmRegMisc = POWER_MGMT_REGISTER_Q35 (0x80);
]> > +      break;
]> > +    default:
]> > +      DEBUG ((EFI_D_ERROR, "%a: Unknown Host Bridge Device ID: 0x%04x\n",
]> > +        __FUNCTION__, HostBridgeDevId));
]> > +      ASSERT (FALSE);
]> 
]> When I build IA32 (OvmfPkg/build.sh -a IA32) I get warnings about
]> possibly uninitialized variables.
]> 
]> It seems to happen basically everwhere that you switch on
]> HostBridgeDevId.
]
]I wonder why it's only warning on IA32, not like its different on X64 :)


Because GCC49 IA32 is an optimized build (-Os) and GCC49 X64 is not.


]> We should default to setting variables to the PIIX4 values in this
]> case.
]> 
]> I'm not sure if we should just skip the assert and use the PIIX4
]> values.
] 
]With the understanding that I will obviously defer to your judgment
]regarding style, I actually *like* the switch statement, complete with
]ASSERT(FALSE) if something "surprising" happens that causes the world
]to end... :)
]
]Assuming we're still on the same page, the question becomes how to
]shut up gcc.


Using a phrase like 'shut up gcc' sort of implies the warning is in
the bogus category, like so many from the Microsoft compilers. But
this warning is valid, right? For example, if I run Ovmf in some
environment where PIIX4 is not present, then the code will fail
to find the ACPI timer I/O address. Why not return an error if
no supported device ID is found. That will eliminate the warning,
and prevent the code from running on the unsupported chipset.

Thanks,
Scott


] We could initialize PmCmd, Pmba, and PmRegMisc to 0 when
]we declare them:
]
]  UINTN  PmCmd     = 0;
]  UINTN  Pmba      = 0;
]  UINTN  PmRegMisc = 0;
]
]or right before the switch statement, but that feels cheesy because
]it's gratuitous, so I don't like either alternative.
]
]Right now, I'm very tempted to initialize them to 0 *after* the
]ASSERT(FALSE) with a comment:
]
]  HostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
]  switch (HostBridgeDevId) {
]    case INTEL_82441_DEVICE_ID:
]      PmCmd     = POWER_MGMT_REGISTER_PIIX4 (PCI_COMMAND_OFFSET);
]      Pmba      = POWER_MGMT_REGISTER_PIIX4 (0x40);
]      PmRegMisc = POWER_MGMT_REGISTER_PIIX4 (0x80);
]      break;
]    case INTEL_Q35_MCH_DEVICE_ID:
]      PmCmd     = POWER_MGMT_REGISTER_Q35 (PCI_COMMAND_OFFSET);
]      Pmba      = POWER_MGMT_REGISTER_Q35 (0x40);
]      PmRegMisc = POWER_MGMT_REGISTER_Q35 (0x80);
]      break;
]    default:
]      DEBUG ((EFI_D_ERROR, "%a: Unknown Host Bridge Device ID: x%04x\n",
]        __FUNCTION__, HostBridgeDevId));
]      ASSERT (FALSE);
]+     PmCmd = Pmba = PmRegMisc = 0; // suppress uninitialized use gcc warning
]  }
]
]I can retrofit this on top of each patch in which I introduced a
]switch on HostBridgeDevId, unless you really resent the way it looks :)
]
]Please let me know what you think.
]
]Thanks,
]--Gabriel


PS. Even once I fixed all of these, IA32 build is still busted right now
(for a reason unrelated to q35):

"/usr/bin/gcc" -g -fshort-wchar -fno-strict-aliasing -Wall -Werror
-Wno-array-bounds -ffunction-sections -fdata-sections -c -include
AutoGen.h -DSTRING_ARRAY_NAME=XenPvBlkDxeStrings -m32 -malign-double
-fno-stack-protector -D EFI32 -Wno-address
-Wno-unused-but-set-variable -Os -mno-mmx -mno-sse -o
/home/somlo/KVM-OSX/SCRATCH/edk2/Build/OvmfIa32/DEBUG_GCC48/IA32/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe/OUTPUT/./BlockFront.obj
-I/home/somlo/KVM-OSX/SCRATCH/edk2/OvmfPkg/XenPvBlkDxe
-I/home/somlo/KVM-OSX/SCRATCH/edk2/Build/OvmfIa32/DEBUG_GCC48/IA32/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe/DEBUG
-I/home/somlo/KVM-OSX/SCRATCH/edk2/MdePkg
-I/home/somlo/KVM-OSX/SCRATCH/edk2/MdePkg/Include
-I/home/somlo/KVM-OSX/SCRATCH/edk2/MdePkg/Include/Ia32
-I/home/somlo/KVM-OSX/SCRATCH/edk2/OvmfPkg
-I/home/somlo/KVM-OSX/SCRATCH/edk2/OvmfPkg/Include
/home/somlo/KVM-OSX/SCRATCH/edk2/OvmfPkg/XenPvBlkDxe/BlockFront.c
In file included from /usr/include/features.h:388:0,
                 from /usr/include/inttypes.h:25,
                 from
/home/somlo/KVM-OSX/SCRATCH/edk2/OvmfPkg/XenPvBlkDxe/BlockFront.c:40:
/usr/include/gnu/stubs.h:7:27: fatal error: gnu/stubs-32.h: No such
file or directory
 # include <gnu/stubs-32.h>
                           ^
compilation terminated.
make: ***
[/home/somlo/KVM-OSX/SCRATCH/edk2/Build/OvmfIa32/DEBUG_GCC48/IA32/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe/OUTPUT/BlockFront.obj]
Error 1


build.py...
 : error 7000: Failed to execute command
        make tbuild
[/home/somlo/KVM-OSX/SCRATCH/edk2/Build/OvmfIa32/DEBUG_GCC48/IA32/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe]


build.py...
 : error F002: Failed to build module
        /home/somlo/KVM-OSX/SCRATCH/edk2/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
[IA32, GCC48, DEBUG]

- Failed -


------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://pubads.g.doubleclick.net/gampad/clk?id=154624111&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://pubads.g.doubleclick.net/gampad/clk?id=154624111&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to