[v3,13/34] tcg: Add atomic helpers

Commit Message

Nikunj A. Dadhania March 27, 2017, 11:56 a.m. UTC

Alex Bennée <[e-mail]> writes:

> Nikunj A Dadhania <[e-mail]> writes:
>
>> Richard Henderson <[e-mail]> writes:
>>
>>> On 09/12/2016 06:47 AM, Alex Bennée wrote:
>>>>> > +    /* Notice an IO access, or a notdirty page.  */
>>>>> > +    if (unlikely(tlb_addr & ~TARGET_PAGE_MASK)) {
>>>>> > +        /* There's really nothing that can be done to
>>>>> > +           support this apart from stop-the-world.  */
>>>>> > +        goto stop_the_world;
>>>> We are also triggering on TLB_NOTDIRTY here in the case where a
>>>> conditional write is the first write to a page. I don't know if a
>>>> stop_the_world is required at this point but we will need to ensure we
>>>> clear bits as notdirty_mem_write() does.
>>>>
>>>
>>> You're quite right that we could probably special-case TLB_NOTDIRTY here such
>>> that (1) we needn't leave the cpu loop, and (2) needn't utilize the actual
>>> "write" part of notdirty_mem_write; just set the bits then fall through to the
>>> actual atomic instruction below.
>>
>> I do hit this case with ppc64, where I see that its the first write to
>> the page and it exits from this every time, causing the kernel to print
>> soft-lockups.
>>
>> Can we add the special case here for NOTDIRTY and set the page as dirty
>> and return successfully?
>
> Does the atomic step fall-back not work for you?

Looked further and I do see that EXCP_ATOMIC does get executed in
qemu_tcg_cpu_thread_fn(), but I am not sure what is going wrong there.

Following snippet fixes the issue for me:


Regards
Nikunj

Patch

diff --git a/cputlb.c b/cputlb.c
index f5d056c..743776a 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -930,7 +930,13 @@  static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
         tlb_addr = tlbe->addr_write;
     }
 
-    /* Notice an IO access, or a notdirty page.  */
+    /* Check notdirty */
+    if (unlikely(tlb_addr & TLB_NOTDIRTY)) {
+        tlb_set_dirty(ENV_GET_CPU(env), addr);
+        tlb_addr = tlb_addr & ~TLB_NOTDIRTY;
+    }
+
+    /* Notice an IO access  */
     if (unlikely(tlb_addr & ~TARGET_PAGE_MASK)) {
         /* There's really nothing that can be done to
            support this apart from stop-the-world.  */