Re: [Qemu-devel] [PATCH 2/4] cputlb: move TLB handling to a separate file

[Andreas Färber]

Am 28.04.2012 13:48, schrieb Blue Swirl:
> On Sun, Apr 22, 2012 at 15:35, Blue Swirl  wrote:
>> diff --git a/cputlb.c b/cputlb.c
>> new file mode 100644
>> index 000..b7d8f07
>> --- /dev/null
>> +++ b/cputlb.c
[...]
>> +void tlb_reset_dirty_range(CPUTLBEntry *tlb_entry, uintptr_t start,
>> +   uintptr_t length)
> 
> This...
> 
>> +{
>> +uintptr_t addr;
>> +
>> +if (tlb_is_dirty_ram(tlb_entry)) {
>> +addr = (tlb_entry->addr_write & TARGET_PAGE_MASK) + 
>> tlb_entry->addend;
>> +if ((addr - start) < length) {
>> +tlb_entry->addr_write |= TLB_NOTDIRTY;
>> +}
>> +}
>> +}
[...]
>> diff --git a/cputlb.h b/cputlb.h
>> new file mode 100644
>> index 000..d16c22e
>> --- /dev/null
>> +++ b/cputlb.h
[...]
>> +void tlb_reset_dirty_range(CPUTLBEntry *tlb_entry, unsigned long start,
>> +   unsigned long length);
> 
> ... doesn't match this prototype, so build would fail on win32.
> 
> Maybe this is 1.2 material anyway.

Personally I'd like to have those big code movements sorted out earlier
than later (including any outstanding AREG0 refactorings) due to my
CPUArchState -> CPUState consolidation work.

So if you and Richard are happy with the code changes except for this
mismatch then I can check both mingw cross builds if you name a branch.

But I'd also be fine with a semiofficial "next" branch for 1.2.

Andreas

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

Re: [Qemu-devel] [PATCH 2/4] cputlb: move TLB handling to a separate file

[Blue Swirl]

On Sun, Apr 22, 2012 at 15:35, Blue Swirl  wrote:
> Move TLB handling and softmmu code load helpers to cputlb.c,
> compile only for softmmu targets.
>
> Signed-off-by: Blue Swirl 
> ---
>  Makefile.target |    2 +-
>  cputlb.c        |  362 
>  cputlb.h        |   63 +
>  exec-all.h      |   12 +-
>  exec.c          |  380 
> +--
>  5 files changed, 443 insertions(+), 376 deletions(-)
>  create mode 100644 cputlb.c
>  create mode 100644 cputlb.h
>
> diff --git a/Makefile.target b/Makefile.target
> index b6a9330..77cd0a1 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -225,7 +225,7 @@ obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/virtio-9p-device.o
>  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>  obj-$(CONFIG_NO_KVM) += kvm-stub.o
>  obj-$(CONFIG_VGA) += vga.o
> -obj-y += memory.o savevm.o
> +obj-y += memory.o savevm.o cputlb.o
>  LIBS+=-lz
>
>  obj-i386-$(CONFIG_KVM) += hyperv.o
> diff --git a/cputlb.c b/cputlb.c
> new file mode 100644
> index 000..b7d8f07
> --- /dev/null
> +++ b/cputlb.c
> @@ -0,0 +1,362 @@
> +/*
> + *  Common CPU TLB handling
> + *
> + *  Copyright (c) 2003 Fabrice Bellard
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> .
> + */
> +
> +#include "config.h"
> +#include "cpu.h"
> +#include "exec-all.h"
> +#include "memory.h"
> +
> +#include "cputlb.h"
> +
> +#define WANT_EXEC_OBSOLETE
> +#include "exec-obsolete.h"
> +
> +//#define DEBUG_TLB
> +//#define DEBUG_TLB_CHECK
> +
> +/* statistics */
> +int tlb_flush_count;
> +
> +static const CPUTLBEntry s_cputlb_empty_entry = {
> +    .addr_read  = -1,
> +    .addr_write = -1,
> +    .addr_code  = -1,
> +    .addend     = -1,
> +};
> +
> +/* NOTE:
> + * If flush_global is true (the usual case), flush all tlb entries.
> + * If flush_global is false, flush (at least) all tlb entries not
> + * marked global.
> + *
> + * Since QEMU doesn't currently implement a global/not-global flag
> + * for tlb entries, at the moment tlb_flush() will also flush all
> + * tlb entries in the flush_global == false case. This is OK because
> + * CPU architectures generally permit an implementation to drop
> + * entries from the TLB at any time, so flushing more entries than
> + * required is only an efficiency issue, not a correctness issue.
> + */
> +void tlb_flush(CPUArchState *env, int flush_global)
> +{
> +    int i;
> +
> +#if defined(DEBUG_TLB)
> +    printf("tlb_flush:\n");
> +#endif
> +    /* must reset current TB so that interrupts cannot modify the
> +       links while we are modifying them */
> +    env->current_tb = NULL;
> +
> +    for (i = 0; i < CPU_TLB_SIZE; i++) {
> +        int mmu_idx;
> +
> +        for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
> +            env->tlb_table[mmu_idx][i] = s_cputlb_empty_entry;
> +        }
> +    }
> +
> +    memset(env->tb_jmp_cache, 0, TB_JMP_CACHE_SIZE * sizeof (void *));
> +
> +    env->tlb_flush_addr = -1;
> +    env->tlb_flush_mask = 0;
> +    tlb_flush_count++;
> +}
> +
> +static inline void tlb_flush_entry(CPUTLBEntry *tlb_entry, target_ulong addr)
> +{
> +    if (addr == (tlb_entry->addr_read &
> +                 (TARGET_PAGE_MASK | TLB_INVALID_MASK)) ||
> +        addr == (tlb_entry->addr_write &
> +                 (TARGET_PAGE_MASK | TLB_INVALID_MASK)) ||
> +        addr == (tlb_entry->addr_code &
> +                 (TARGET_PAGE_MASK | TLB_INVALID_MASK))) {
> +        *tlb_entry = s_cputlb_empty_entry;
> +    }
> +}
> +
> +void tlb_flush_page(CPUArchState *env, target_ulong addr)
> +{
> +    int i;
> +    int mmu_idx;
> +
> +#if defined(DEBUG_TLB)
> +    printf("tlb_flush_page: " TARGET_FMT_lx "\n", addr);
> +#endif
> +    /* Check if we need to flush due to large pages.  */
> +    if ((addr & env->tlb_flush_mask) == env->tlb_flush_addr) {
> +#if defined(DEBUG_TLB)
> +        printf("tlb_flush_page: forced full flush ("
> +               TARGET_FMT_lx "/" TARGET_FMT_lx ")\n",
> +               env->tlb_flush_addr, env->tlb_flush_mask);
> +#endif
> +        tlb_flush(env, 1);
> +        return;
> +    }
> +    /* must reset current TB so that interrupts cannot modify the
> +       links while we are modifying them */
> +    env->current_tb = NULL;
> +
> +    addr &= TARGET_PAGE_MASK;
> +    i = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZ

Re: [Qemu-devel] [PATCH 2/4] cputlb: move TLB handling to a separate file

[Richard Henderson]

On 04/22/12 08:35, Blue Swirl wrote:
> Move TLB handling and softmmu code load helpers to cputlb.c,
> compile only for softmmu targets.
> 
> Signed-off-by: Blue Swirl 
> ---
>  Makefile.target |2 +-
>  cputlb.c|  362 
>  cputlb.h|   63 +
>  exec-all.h  |   12 +-
>  exec.c  |  380 
> +--
>  5 files changed, 443 insertions(+), 376 deletions(-)
>  create mode 100644 cputlb.c
>  create mode 100644 cputlb.h

Reviewed-by: Richard Henderson 


r~