| Message ID | 54649253.9000609@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Nov 13, 2014 at 12:13:23PM +0100, Laszlo Ersek wrote: > Anthony, > > On 11/06/14 12:24, Anthony PERARD wrote: > > The BaseTools/Scripts/ConvertMasmToNasm.py script was used to convert > > X64/TestAndClearBit.asm to X64/TestAndClearBit.nasm > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > > --- > > OvmfPkg/XenBusDxe/X64/TestAndClearBit.S | 12 ------------ > > .../X64/{TestAndClearBit.asm => TestAndClearBit.nasm} | 8 ++++---- > > OvmfPkg/XenBusDxe/XenBusDxe.inf | 3 +-- > > 3 files changed, 5 insertions(+), 18 deletions(-) > > delete mode 100644 OvmfPkg/XenBusDxe/X64/TestAndClearBit.S > > rename OvmfPkg/XenBusDxe/X64/{TestAndClearBit.asm => TestAndClearBit.nasm} (63%) > > > > diff --git a/OvmfPkg/XenBusDxe/X64/TestAndClearBit.S b/OvmfPkg/XenBusDxe/X64/TestAndClearBit.S > > deleted file mode 100644 > > index 0372e83..0000000 > > --- a/OvmfPkg/XenBusDxe/X64/TestAndClearBit.S > > +++ /dev/null > > @@ -1,12 +0,0 @@ > > -# INT32 > > -# EFIAPI > > -# TestAndClearBit ( > > -# IN INT32 Bit, // rcx > > -# IN volatile VOID* Address // rdx > > -# ); > > -ASM_GLOBAL ASM_PFX(TestAndClearBit) > > -ASM_PFX(TestAndClearBit): > > - lock > > - btrl %ecx, (%rdx) > > - sbbl %eax, %eax > > - ret > > diff --git a/OvmfPkg/XenBusDxe/X64/TestAndClearBit.asm b/OvmfPkg/XenBusDxe/X64/TestAndClearBit.nasm > > similarity index 63% > > rename from OvmfPkg/XenBusDxe/X64/TestAndClearBit.asm > > rename to OvmfPkg/XenBusDxe/X64/TestAndClearBit.nasm > > index 3a25879..38ac549 100644 > > --- a/OvmfPkg/XenBusDxe/X64/TestAndClearBit.asm > > +++ b/OvmfPkg/XenBusDxe/X64/TestAndClearBit.nasm > > @@ -1,4 +1,5 @@ > > -.code > > +DEFAULT REL > > +SECTION .text > > > > ; INT32 > > ; EFIAPI > > @@ -6,11 +7,10 @@ > > ; IN INT32 Bit, // rcx > > ; IN volatile VOID* Address // rdx > > ; ); > > -TestAndClearBit PROC > > +global ASM_PFX(TestAndClearBit) > > +ASM_PFX(TestAndClearBit): > > lock > > btr [rdx], ecx > > sbb eax, eax > > ret > > -TestAndClearBit ENDP > > > > -END > > diff --git a/OvmfPkg/XenBusDxe/XenBusDxe.inf b/OvmfPkg/XenBusDxe/XenBusDxe.inf > > index 61f7568..4ce4743 100644 > > --- a/OvmfPkg/XenBusDxe/XenBusDxe.inf > > +++ b/OvmfPkg/XenBusDxe/XenBusDxe.inf > > @@ -56,8 +56,7 @@ > > [Sources.X64] > > X64/hypercall.nasm > > X64/InterlockedCompareExchange16.nasm > > - X64/TestAndClearBit.S > > - X64/TestAndClearBit.asm > > + X64/TestAndClearBit.nasm > > > > [LibraryClasses] > > UefiDriverEntryPoint > > > > when building this, my nasm reports: > > Build/OvmfX64/DEBUG_GCC48/X64/OvmfPkg/XenBusDxe/XenBusDxe/OUTPUT/X64/TestAndClearBit.iii:12: warning: instruction is not lockable > > Looking at the "insns.dat" file in the "nasm" source code, I can see > > BTR mem,reg64 [mr: hle o64 0f b3 /r] X64,SM,LOCK > > So apparently the problem is that you don't pass the bit position in a 64-bit register (rcx); you pass it in a 32-bit register (ecx). I tried the following patch: > > diff --git a/OvmfPkg/XenBusDxe/X64/TestAndClearBit.nasm b/OvmfPkg/XenBusDxe/X64/TestAndClearBit.nasm > index 38ac549..ce49d7f 100644 > --- a/OvmfPkg/XenBusDxe/X64/TestAndClearBit.nasm > +++ b/OvmfPkg/XenBusDxe/X64/TestAndClearBit.nasm > @@ -10,7 +10,7 @@ SECTION .text > global ASM_PFX(TestAndClearBit) > ASM_PFX(TestAndClearBit): > lock > - btr [rdx], ecx > + btr [rdx], rcx > sbb eax, eax > ret > > But nasm emits the same warning. Any ideas? Yes, put lock and btr on the same line ... ASM_PFX(TestAndClearBit): - lock - btr [rdx], ecx + lock btr [rdx], ecx sbb eax, eax This small patch appear to work, no more warning from nasm. > The warning is a bit strange though, because the disassembly confirms the BTR is locked: > > Disassembly of section .text: > > 0000000000000000 <TestAndClearBit>: > 0: f0 48 0f b3 0a lock btr %rcx,(%rdx) > 5: 19 c0 sbb %eax,%eax > 7: c3 retq > > So maybe it's a nasm bug.
On 11/13/14 18:55, Anthony PERARD wrote: > On Thu, Nov 13, 2014 at 12:13:23PM +0100, Laszlo Ersek wrote: >> But nasm emits the same warning. Any ideas? > > Yes, put lock and btr on the same line ... > ASM_PFX(TestAndClearBit): > - lock > - btr [rdx], ecx > + lock btr [rdx], ecx > sbb eax, eax > > This small patch appear to work, no more warning from nasm. Thanks. Can you please tack it as a standalone patch to your recent series, as 5/4? http://thread.gmane.org/gmane.comp.bios.tianocore.devel/11211 Cheers Laszlo ------------------------------------------------------------------------------ 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
On Thu, Nov 13, 2014 at 07:00:18PM +0100, Laszlo Ersek wrote: > On 11/13/14 18:55, Anthony PERARD wrote: > > On Thu, Nov 13, 2014 at 12:13:23PM +0100, Laszlo Ersek wrote: > > >> But nasm emits the same warning. Any ideas? > > > > Yes, put lock and btr on the same line ... > > ASM_PFX(TestAndClearBit): > > - lock > > - btr [rdx], ecx > > + lock btr [rdx], ecx > > sbb eax, eax > > > > This small patch appear to work, no more warning from nasm. > > Thanks. Can you please tack it as a standalone patch to your recent > series, as 5/4? yes. > http://thread.gmane.org/gmane.comp.bios.tianocore.devel/11211 > > Cheers > Laszlo >
diff --git a/OvmfPkg/XenBusDxe/X64/TestAndClearBit.nasm b/OvmfPkg/XenBusDxe/X64/TestAndClearBit.nasm index 38ac549..ce49d7f 100644 --- a/OvmfPkg/XenBusDxe/X64/TestAndClearBit.nasm +++ b/OvmfPkg/XenBusDxe/X64/TestAndClearBit.nasm @@ -10,7 +10,7 @@ SECTION .text global ASM_PFX(TestAndClearBit) ASM_PFX(TestAndClearBit): lock - btr [rdx], ecx + btr [rdx], rcx sbb eax, eax ret