On 7/7/20 10:36 AM, Laszlo Ersek wrote: > On 07/06/20 22:03, Tom Lendacky wrote: >> On 7/2/20 2:04 AM, Dong, Eric wrote: >>> Hi Tom, >> >> Hi Eric, >> >>> >>> We have root cause this Mac file format issue. The patch mail from your >>> side include extra two "=0D=0D" , and our test tool convert them to "\r\r". >>> This is Mac file line ending format. So this issue been reported. We have >>> updated our tool to handle this special case. >> >> Good to know, thanks! >> >>> >>> With that change, now I met below error when use VS2015 tool chain. Can you >>> help to fix it? >>> >>> Building ... >>> g:\edk2-open-source\edk2\MdePkg\Library\PeiCoreEntryPoint\PeiCoreEntryPoint.inf >>> [X64] >>> PeCoffLoaderEx.c >>> g:\edk2-open-source\edk2\OvmfPkg\Library\VmgExitLib\VmgExitVcHandler.c(386): >>> warning C4334: '<<': result of 32-bit shift implicitly converted to 64 >>> bits (was 64-bit shift intended?) >>> NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio >>> 14.0\Vc\bin\x86_amd64\cl.exe"' : return code '0x2' > > This is for the line > > Displacement *= (1 << Ext->Sib.Scale); > > from > > [edk2-devel] [PATCH v9 17/46] > OvmfPkg/VmgExitLib: Add support for NPF NAE events (MMIO) > >> >> Yup, looks like that needs to be a "1ULL <<" instead of "1 <<". >> I have verified that fixes the issue. > > I disagree. > > At that point, Displacement is of type INT64, and it may well be a > negative value. We definitely want to multiply it by a signed int > (values 1, 2, 4, 8). > > I commented on this before. Please see: > > (i) my comment block (10) here -- especially comment (10c): > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F60144&data=02%7C01%7Cthomas.lendacky%40amd.com%7Cec0cb2ad96694b66d8ff08d8228b7c8e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637297329772337705&sdata=g%2BGooY1Sv0G7ydr11Jh%2BTXxo4Wy6ZWcT5Mq9VmWddi8%3D&reserved=0 > > (alternative link: > <https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmid.mail-archive.com%2F169e44cb-2c1c-6d9a-342a-2a1f618e3753%40redhat.com&data=02%7C01%7Cthomas.lendacky%40amd.com%7Cec0cb2ad96694b66d8ff08d8228b7c8e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637297329772337705&sdata=6p91db%2F6oz%2FHc65Sq4fvH%2FcPmiAfdS8MImsaznaoaXA%3D&reserved=0>) > > (ii) and my comment here: > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F60146&data=02%7C01%7Cthomas.lendacky%40amd.com%7Cec0cb2ad96694b66d8ff08d8228b7c8e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637297329772337705&sdata=iNIBJCIlfEEsY37cdwUbH27tx5HvXVs3PZiOQfaGeLQ%3D&reserved=0 > > (alternative link: > <https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmid.mail-archive.com%2F139ce789-b938-c8b9-030e-c1b6c67e47ea%40redhat.com&data=02%7C01%7Cthomas.lendacky%40amd.com%7Cec0cb2ad96694b66d8ff08d8228b7c8e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637297329772337705&sdata=mWCAHqTOpp7B9nWUJjTRJ9VZ74iwdElRTOoNhEpFs%2Bc%3D&reserved=0>). > > > The compiler warning is well-meaning, but unnecessary. A 64-bit shift is > *NOT* intended. We want to end up with one of the signed int (aka INT32) > values 1, 2, 4 or 8. And then multiply the INT64 Displacement with that > value. For the multiplication, the INT32 value 1, 2, 4 or 8 will be > implicitly converted to INT64. That's entirely intentional. > > If we want to suppress the warning, while keeping the logic intact, we > should employ an explicit cast: > > Displacement *= (INT64)(1 << Ext->Sib.Scale);
Ok, that makes sense. I'll use the explicit cast. > >> >> One thing I noticed is that the 32-bit builds >> (PlatformCI_OvmfPkg_Windows_VS2019_PR, Platform_CI OVMF_IA32_NOOPT and >> Platform_CI OVMF_IA32X64_NOOPT) encounter an error: >> >> ERROR - Linker #2001 from SecMain.lib(SecMain.obj) : unresolved external >> symbol __allshl >> ERROR - Linker #1120 from >> d:\a\1\s\Build\Ovmf3264\NOOPT_VS2019\IA32\OvmfPkg\Sec\SecMain\DEBUG\SecMain.dll >> : fatal 1 unresolved externals >> ERROR - Compiler #1077 from NMAKE : fatal '"C:\Program Files >> (x86)\Microsoft Visual >> Studio\2019\Enterprise\VC\Tools\MSVC\14.26.28801\bin\Hostx86\x86\link.exe"' >> : return code '0x460' >> >> Any idea what is causing this error? > > A left-shift operator (<<) applied to a 64-bit operand is somehow > finding its way into the 32-bit SEC build. > > That is indeed wrong (for such cases, we're supposed to use LShiftU64() > from BaseLib). > > What I don't understand however is that all of the "<<" operator uses, > on 64-bit operands, should already be limited to code that is *only* > built for X64! > > For example, with this series applied, SecMain in OVMF consumes > "UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf". > And the latter consumes VmgExitLib. > > But VmgExitLib is resolved to > "UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf", in the IA32 and > IA32X64 DSC files. This Null instance contains no left-shifts. > > Therefore any << operators, applied to 64-bit operands, present in > "OvmfPkg/Library/VmgExitLib", should never be compiled for IA32 and IA32X64. > > So I don't know where the problematic "<<" comes from. It does not come > from VmgExitLib, as far as I can tell. Yes, I don't think it's coming from VmgExitLib, either. I wonder if it somehow might be coming from the MSR_SEV_ES_GHCB_REGISTER struct and the bit fields that are used within it? That code, while not executed in non-X64 builds because SEV-ES is not active, is still built and maybe the bit fields result in implicit shifts occurring, specifically in SevEsProtocolFailure()? I'll experiment with some things and see if that is the issue. Thanks, Tom > > Thanks, > Laszlo > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62168): https://edk2.groups.io/g/devel/message/62168 Mute This Topic: https://groups.io/mt/74692413/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
