Re: [Qemu-devel] [PATCH v1 6/6] mem: change tcg code to rcu style

[liu ping fan]

On Wed, May 29, 2013 at 3:22 PM, Paolo Bonzini <address@hidden> wrote:
> Il 29/05/2013 04:11, Liu Ping Fan ha scritto:
>> From: Liu Ping Fan <address@hidden>
>>
>> When adopting rcu style, for tcg code, need to fix two kind of path:
>>  -tlb_set_page() will cache translation info.
>>  -instruction emualation path
>>
>> Signed-off-by: Liu Ping Fan <address@hidden>
>> ----
>> Not sure about tcg code, so I took helper_st as the example.
>> ---
>>  cputlb.c                        |   25 +++++++++++++++++-
>>  exec.c                          |    2 +-
>>  include/exec/softmmu_template.h |   53 
>> +++++++++++++++++++++++++++++++-------
>>  3 files changed, 68 insertions(+), 12 deletions(-)
>>
>> diff --git a/cputlb.c b/cputlb.c
>> index 86666c8..6b55c70 100644
>> --- a/cputlb.c
>> +++ b/cputlb.c
>> @@ -249,6 +249,10 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
>>      uintptr_t addend;
>>      CPUTLBEntry *te;
>>      hwaddr iotlb, xlat, sz;
>> +    unsigned long start;
>> +    Node *map_base;
>> +    MemoryRegionSection *sections_base;
>> +    PhysPageEntry *roots_base;
>>
>>      assert(size >= TARGET_PAGE_SIZE);
>>      if (size != TARGET_PAGE_SIZE) {
>> @@ -256,8 +260,18 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
>>      }
>>
>>      sz = size;
>> +
>> +reload:
>> +    rcu_read_lock();
>
> It's really the whole of cpu_exec that has to be protected by a RCU
> critical section.
>
Ok.
>> +    start = seqlock_read_begin(&dispatch_table_lock);
>> +    map_base = cur_map_nodes;
>> +    sections_base = cur_phys_sections;
>> +    roots_base = cur_roots;
>
> Why the seqlock_read_check at the end and not here?
>
In case the tlb_flush() empty tlb entry, and we record incorrect index

> Remember that this code is running under the BQL, so there is no need to
> protect the TLB flushes otherwise.  There is also no need to do anything

Do you mean that cpu_exec() can not be out of BQL like kvm path?  If
it is true, then there is no changes needed for tcg code, right?

> as long as you ref the regions that are entered in the map, and unref
> them when you destroy the map.  Those refs will protect TCG's TLB too.
>
Worry about whether the TLB is the right index in the right
phys_sections, if the mem topology changes between tlb_set_page() and
instruction emulated.

> In the end, all of TCG's execution consists of a large RCU critical section.
>
Even stronger? If it runs with BQL protection.

Regards,
Pingfan

[...]