[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] cpu: implementing victim TLB for QEMU system
From: |
Max Filippov |
Subject: |
Re: [Qemu-devel] [PATCH v2] cpu: implementing victim TLB for QEMU system emulated TLB |
Date: |
Fri, 24 Jan 2014 01:46:46 +0400 |
On Fri, Jan 24, 2014 at 1:29 AM, Xin Tong <address@hidden> wrote:
>>> +/* swap the 2 given TLB entries as well as their corresponding IOTLB */
>>> +inline void swap_tlb(CPUTLBEntry *te, CPUTLBEntry *se, hwaddr *iote,
>>> + hwaddr *iose)
>>> +{
>>> + hwaddr iotmp;
>>> + CPUTLBEntry t;
>>> + /* swap iotlb */
>>> + iotmp = *iote;
>>> + *iote = *iose;
>>> + *iose = iotmp;
>>> + /* swap tlb */
>>> + memcpy(&t, te, sizeof(CPUTLBEntry));
>>> + memcpy(te, se, sizeof(CPUTLBEntry));
>>> + memcpy(se, &t, sizeof(CPUTLBEntry));
>>
>> You could use assignment operator, it works:
>> t = *te;
>> *te = *se;
>> *se = t;
>>
> i see, will fix in patch v3.
>
>> Also all users of this function are inline functions from softmmu_template.h
>> Could this function be a static inline function in that file too?
>
> I am following where tlb_fill and tlb_flush are placed. Also, the
> softmmu_template.h is included more than once, would put the swap_tlb
> in softmmu_template.h result in multiple definitions ?
You're right. Could it be put into a file that is included by softmmu_template.h
and has include guards in it, or could it be turned into macro?
[...]
>>> - tlb_fill(env, addr, READ_ACCESS_TYPE, mmu_idx, retaddr);
>>> + /* we are about to do a page table walk. our last hope is the
>>> + * victim tlb. try to refill from the victim tlb before walking the
>>> + * page table. */
>>> + for (vtlb_idx = CPU_VTLB_SIZE; vtlb_idx >= 0; --vtlb_idx) {
>>> + if (env->tlb_v_table[mmu_idx][vtlb_idx].ADDR_READ
>>
>> Is it ADDR_READ or addr_read, as it is called in other places?
>
> I am following target_ulong tlb_addr =
> env->tlb_table[mmu_idx][index].ADDR_READ; in the same function. I am
> not sure i get what you mean.
Ok, I see ADDR_READ is a macro defined depending on
SOFTMMU_CODE_ACCESS. I got a bit confused by addr_read used
in other places vs. ADDR_READ used here vs. addr_write used below.
Sorry for the noise.
[...]
>>> - tlb_fill(env, addr, 1, mmu_idx, retaddr);
>>> + /* we are about to do a page table walk. our last hope is the
>>> + * victim tlb. try to refill from the victim tlb before walking the
>>> + * page table. */
>>> + for (vtlb_idx = CPU_VTLB_SIZE; vtlb_idx >= 0; --vtlb_idx) {
>>> + if (env->tlb_v_table[mmu_idx][vtlb_idx].addr_write
>>> + == (addr & TARGET_PAGE_MASK)) {
>>> + /* found entry in victim tlb */
>>> + swap_tlb(&env->tlb_table[mmu_idx][index],
>>> + &env->tlb_v_table[mmu_idx][vtlb_idx],
>>> + &env->iotlb[mmu_idx][index],
>>> + &env->iotlb_v[mmu_idx][vtlb_idx]);
>>> + break;
>>> + }
>>> + }
>>
>> It could be a good inline function too.
>
> One way is to define the victim tlb lookup code in a C function which
> gets called before tlb_fill. but i need the ADDR_READ and addr_write
> macros.
It could be a single macro then, taking ADDR_READ or addr_write
as a parameter.
--
Thanks.
-- Max