]>
Commit | Line | Data |
---|---|---|
64f24c1c GKH |
1 | From de53e9caa4c6149ef4a78c2f83d7f5b655848767 Mon Sep 17 00:00:00 2001 |
2 | From: Stephan Schreiber <info@fs-driver.org> | |
3 | Date: Tue, 19 Mar 2013 15:27:12 -0700 | |
4 | Subject: Wrong asm register contraints in the kvm implementation | |
5 | ||
6 | From: Stephan Schreiber <info@fs-driver.org> | |
7 | ||
8 | commit de53e9caa4c6149ef4a78c2f83d7f5b655848767 upstream. | |
9 | ||
10 | The Linux Kernel contains some inline assembly source code which has | |
11 | wrong asm register constraints in arch/ia64/kvm/vtlb.c. | |
12 | ||
13 | I observed this on Kernel 3.2.35 but it is also true on the most | |
14 | recent Kernel 3.9-rc1. | |
15 | ||
16 | File arch/ia64/kvm/vtlb.c: | |
17 | ||
18 | u64 guest_vhpt_lookup(u64 iha, u64 *pte) | |
19 | { | |
20 | u64 ret; | |
21 | struct thash_data *data; | |
22 | ||
23 | data = __vtr_lookup(current_vcpu, iha, D_TLB); | |
24 | if (data != NULL) | |
25 | thash_vhpt_insert(current_vcpu, data->page_flags, | |
26 | data->itir, iha, D_TLB); | |
27 | ||
28 | asm volatile ( | |
29 | "rsm psr.ic|psr.i;;" | |
30 | "srlz.d;;" | |
31 | "ld8.s r9=[%1];;" | |
32 | "tnat.nz p6,p7=r9;;" | |
33 | "(p6) mov %0=1;" | |
34 | "(p6) mov r9=r0;" | |
35 | "(p7) extr.u r9=r9,0,53;;" | |
36 | "(p7) mov %0=r0;" | |
37 | "(p7) st8 [%2]=r9;;" | |
38 | "ssm psr.ic;;" | |
39 | "srlz.d;;" | |
40 | "ssm psr.i;;" | |
41 | "srlz.d;;" | |
42 | : "=r"(ret) : "r"(iha), "r"(pte):"memory"); | |
43 | ||
44 | return ret; | |
45 | } | |
46 | ||
47 | The list of output registers is | |
48 | : "=r"(ret) : "r"(iha), "r"(pte):"memory"); | |
49 | The constraint "=r" means that the GCC has to maintain that these vars | |
50 | are in registers and contain valid info when the program flow leaves | |
51 | the assembly block (output registers). | |
52 | But "=r" also means that GCC can put them in registers that are used | |
53 | as input registers. Input registers are iha, pte on the example. | |
54 | If the predicate p7 is true, the 8th assembly instruction | |
55 | "(p7) mov %0=r0;" | |
56 | is the first one which writes to a register which is maintained by the | |
57 | register constraints; it sets %0. %0 means the first register operand; | |
58 | it is ret here. | |
59 | This instruction might overwrite the %2 register (pte) which is needed | |
60 | by the next instruction: | |
61 | "(p7) st8 [%2]=r9;;" | |
62 | Whether it really happens depends on how GCC decides what registers it | |
63 | uses and how it optimizes the code. | |
64 | ||
65 | The attached patch fixes the register operand constraints in | |
66 | arch/ia64/kvm/vtlb.c. | |
67 | The register constraints should be | |
68 | : "=&r"(ret) : "r"(iha), "r"(pte):"memory"); | |
69 | The & means that GCC must not use any of the input registers to place | |
70 | this output register in. | |
71 | ||
72 | This is Debian bug#702639 | |
73 | (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=702639). | |
74 | ||
75 | The patch is applicable on Kernel 3.9-rc1, 3.2.35 and many other versions. | |
76 | ||
77 | Signed-off-by: Stephan Schreiber <info@fs-driver.org> | |
78 | Signed-off-by: Tony Luck <tony.luck@intel.com> | |
79 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
80 | ||
81 | --- | |
82 | arch/ia64/kvm/vtlb.c | 2 +- | |
83 | 1 file changed, 1 insertion(+), 1 deletion(-) | |
84 | ||
85 | --- a/arch/ia64/kvm/vtlb.c | |
86 | +++ b/arch/ia64/kvm/vtlb.c | |
87 | @@ -256,7 +256,7 @@ u64 guest_vhpt_lookup(u64 iha, u64 *pte) | |
88 | "srlz.d;;" | |
89 | "ssm psr.i;;" | |
90 | "srlz.d;;" | |
91 | - : "=r"(ret) : "r"(iha), "r"(pte):"memory"); | |
92 | + : "=&r"(ret) : "r"(iha), "r"(pte) : "memory"); | |
93 | ||
94 | return ret; | |
95 | } |