Re: [Qemu-devel] [PATCH qom-cpu v2 37/40] cputlb: Change tlb_flush_page() argument to CPUState

[Andreas Färber]

Am 11.03.2014 16:05, schrieb Max Filippov:
> On Mon, Mar 10, 2014 at 4:15 AM, Andreas Färber <afaer...@suse.de> wrote:
>> diff --git a/target-xtensa/op_helper.c b/target-xtensa/op_helper.c
>> index 624ef54..612d3bf 100644
>> --- a/target-xtensa/op_helper.c
>> +++ b/target-xtensa/op_helper.c
>> @@ -730,21 +730,24 @@ void xtensa_tlb_set_entry_mmu(const CPUXtensaState 
>> *env,
>>  void xtensa_tlb_set_entry(CPUXtensaState *env, bool dtlb,
>>          unsigned wi, unsigned ei, uint32_t vpn, uint32_t pte)
>>  {
>> +    XtensaCPU *cpu = xtensa_env_get_cpu(env);
>> +    CPUState *cs;
>>      xtensa_tlb_entry *entry = xtensa_tlb_get_entry(env, dtlb, wi, ei);
>>
>>      if (xtensa_option_enabled(env->config, XTENSA_OPTION_MMU)) {
>>          if (entry->variable) {
>> +            cs = CPU(cpu);
> 
> Maybe move it up and use cs everywhere, or drop cs and use CPU(cpu)
> everywhere?

Sure, if the !enabled && !variable path is of no performance concern,
I'll happily change it to:

diff --git a/target-xtensa/op_helper.c b/target-xtensa/op_helper.c
index c41d341..cae9cf4 100644
--- a/target-xtensa/op_helper.c
+++ b/target-xtensa/op_helper.c
@@ -733,12 +733,11 @@ void xtensa_tlb_set_entry(CPUXtensaState *env,
bool dtlb,
         unsigned wi, unsigned ei, uint32_t vpn, uint32_t pte)
 {
     XtensaCPU *cpu = xtensa_env_get_cpu(env);
-    CPUState *cs;
+    CPUState *cs = CPU(cpu);
     xtensa_tlb_entry *entry = xtensa_tlb_get_entry(env, dtlb, wi, ei);

     if (xtensa_option_enabled(env->config, XTENSA_OPTION_MMU)) {
         if (entry->variable) {
-            cs = CPU(cpu);
             if (entry->asid) {
                 tlb_flush_page(cs, entry->vaddr);
             }
@@ -749,7 +748,7 @@ void xtensa_tlb_set_entry(CPUXtensaState *env, bool
dtlb,
                     __func__, dtlb, wi, ei);
         }
     } else {
-        tlb_flush_page(CPU(cpu), entry->vaddr);
+        tlb_flush_page(cs, entry->vaddr);
         if (xtensa_option_enabled(env->config,
                     XTENSA_OPTION_REGION_TRANSLATION)) {
             entry->paddr = pte & REGION_PAGE_MASK;

Thanks,
Andreas

> 
>>              if (entry->asid) {
>> -                tlb_flush_page(env, entry->vaddr);
>> +                tlb_flush_page(cs, entry->vaddr);
>>              }
>>              xtensa_tlb_set_entry_mmu(env, entry, dtlb, wi, ei, vpn, pte);
>> -            tlb_flush_page(env, entry->vaddr);
>> +            tlb_flush_page(cs, entry->vaddr);
>>          } else {
>>              qemu_log("%s %d, %d, %d trying to set immutable entry\n",
>>                      __func__, dtlb, wi, ei);
>>          }
>>      } else {
>> -        tlb_flush_page(env, entry->vaddr);
>> +        tlb_flush_page(CPU(cpu), entry->vaddr);
>>          if (xtensa_option_enabled(env->config,
>>                      XTENSA_OPTION_REGION_TRANSLATION)) {
>>              entry->paddr = pte & REGION_PAGE_MASK;

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg