[SeaBIOS] [PATCH RFC] x86: use volatile asm for read/write{b, w, l} implementations
Vitaly Kuznetsov
vkuznets at redhat.com
Wed Jan 3 14:41:15 CET 2018
QEMU/KVM guests running nested on top of Hyper-V fail to boot with
virtio-blk-pci disks, the debug log ends with
Booting from Hard Disk...
call32_smm 0x000edd01 e97a0
handle_smi cmd=b5 smbase=0x000a0000
vp notify fe007000 (2) -- 0x0
vp read fe005000 (1) -> 0x0
handle_smi cmd=b5 smbase=0x000a0000
call32_smm done 0x000edd01 0
Booting from 0000:7c00
call32_smm 0x000edd01 e97a4
handle_smi cmd=b5 smbase=0x000a0000
vp notify fe007000 (2) -- 0x0
In resume (status=0)
In 32bit resume
Attempting a hard reboot
...
I bisected the breakage to the following commit:
commit f46739b1a819750c63fb5849844d99cc2ab001e8
Author: Kevin O'Connor <kevin at koconnor.net>
Date: Tue Feb 2 22:34:27 2016 -0500
virtio: Convert to new PCI BAR helper functions
But the commit itself appears to be correct. The problem is in how
writew() function compiles into vp_notify(). For example, if we drop
'volatile' qualifier from the current writew() implementation everything
starts to work. If we disassemble these two versions (as of f46739b1a)
the difference will be:
00000000 <vp_notify>:
With 'volatile' (current) Without 'volatile'
0: push %ebx 0: push %ebx
1: mov %eax,%ecx 1: mov %eax,%ecx
3: mov 0x1518(%edx),%eax 3: mov 0x1518(%edx),%eax
9: cmpb $0x0,0x2c(%ecx) 9: cmpb $0x0,0x2c(%ecx)
d: je 2f <vp_notify+0x2f> d: je 2e <vp_notify+0x2e>
f: mov 0x151c(%edx),%edx f: mov 0x151c(%edx),%edx
15: mov 0x28(%ecx),%ebx
18: imul %edx,%ebx 15: imul 0x28(%ecx),%edx
1b: mov 0x8(%ecx),%edx 19: mov 0x8(%ecx),%ebx
1e: add %ebx,%edx
20: cmpb $0x0,0xe(%ecx) 1c: cmpb $0x0,0xe(%ecx)
24: je 2a <vp_notify+0x2a> 20: je 28 <vp_notify+0x28>
22: add %ebx,%edx
26: out %ax,(%dx) 24: out %ax,(%dx)
28: jmp 48 <vp_notify+0x48> 26: jmp 47 <vp_notify+0x47>
2a: mov %ax,(%edx) 28: mov %ax,(%ebx,%edx,1)
2d: jmp 48 <vp_notify+0x48> 2c: jmp 47 <vp_notify+0x47>
2f: lea 0x20(%ecx),%ebx 2e: lea 0x20(%ecx),%ebx
32: cltd 31: cltd
33: push %edx 32: push %edx
34: push %eax 33: push %eax
35: mov $0x2,%ecx 34: mov $0x2,%ecx
3a: mov $0x10,%edx 39: mov $0x10,%edx
3f: mov %ebx,%eax 3e: mov %ebx,%eax
41: call 42 <vp_notify+0x42> 40: call 41 <vp_notify+0x41>
46: pop %eax 45: pop %eax
47: pop %edx 46: pop %edx
48: pop %ebx 47: pop %ebx
49: ret 48: ret
My eyes fail to see an obvious compiler flaw here but probably the
mov difference (at '2a' old, '28' new) is to blame. Doing some other
subtle changes (e.g. adding local variables to the function) help in
some cases too. At this point I got a bit lost with my debug so I
looked at how Linux does this stuff and it seems we're not using
'*(volatile u16) = ' there. Rewriting write/read{b,w,l} with volatile
asm help.
Signed-off-by: Vitaly Kuznetsov <vkuznets at redhat.com>
---
RFC: This is rather an ongoing debug as I'm not able to point finger
at the real culprit yet, I'd be grateful for any help and suggestions.
In particular, I don't quite understand why nested virtualization
makes a difference here.
---
src/x86.h | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/src/x86.h b/src/x86.h
index 53378e9..d45122c 100644
--- a/src/x86.h
+++ b/src/x86.h
@@ -199,30 +199,27 @@ static inline void smp_wmb(void) {
}
static inline void writel(void *addr, u32 val) {
- barrier();
- *(volatile u32 *)addr = val;
+ asm volatile("movl %0, %1" : : "d"(val), "m"(*(u32 *)addr) : "memory");
}
static inline void writew(void *addr, u16 val) {
- barrier();
- *(volatile u16 *)addr = val;
+ asm volatile("movw %0, %1" : : "d"(val), "m"(*(u16 *)addr) : "memory");
}
static inline void writeb(void *addr, u8 val) {
- barrier();
- *(volatile u8 *)addr = val;
+ asm volatile("movb %0, %1" : : "d"(val), "m"(*(u8 *)addr) : "memory");
}
static inline u32 readl(const void *addr) {
- u32 val = *(volatile const u32 *)addr;
- barrier();
+ u32 val;
+ asm volatile("movl %1, %0" : "=d"(val) : "m"(*(u32 *)addr) : "memory");
return val;
}
static inline u16 readw(const void *addr) {
- u16 val = *(volatile const u16 *)addr;
- barrier();
+ u16 val;
+ asm volatile("movw %1, %0" : "=d"(val) : "m"(*(u16 *)addr) : "memory");
return val;
}
static inline u8 readb(const void *addr) {
- u8 val = *(volatile const u8 *)addr;
- barrier();
+ u8 val;
+ asm volatile("movb %1, %0" : "=d"(val) : "m"(*(u8 *)addr) : "memory");
return val;
}
--
2.14.3
More information about the SeaBIOS
mailing list