qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 16/28] s390x/tcg: Fault-safe memmove


From: Richard Henderson
Subject: Re: [Qemu-devel] [PATCH v2 16/28] s390x/tcg: Fault-safe memmove
Date: Wed, 11 Sep 2019 18:03:55 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0

On 9/6/19 3:57 AM, David Hildenbrand wrote:
> +static void access_memmove_idx(CPUS390XState *env, vaddr dest, vaddr src,
> +                               int size, int dest_idx, int src_idx,
> +                               uintptr_t ra)
> +{
> +    S390Access srca = access_prepare_idx(env, src, size, MMU_DATA_LOAD, 
> src_idx,
> +                                         ra);
> +    S390Access desta = access_prepare_idx(env, dest, size, MMU_DATA_STORE,
> +                                          dest_idx, ra);

I was just thinking that it might be worth passing in these Access structures.
 It seems that usually we wind up computing them in several locations.

Hoisting it up it would make MVC look like

    midx = cpu_mmu_index(env);
    srca = access_prepare_idx(env, src, size, LOAD, midx, ra);
    dsta = access_prepare_idx(env, dst, size, STORE, midx, ra);

    if (dst == src + 1) {
        uint8_t x = access_get_byte(env, &srca, 0, ra);
        access_memset(env, &dsta, x, size, ra);
    } else if (!is_destructive_overlap(env, dst, src, size)) {
        access_memmove(env, &dsta, &srca, size, ra);
    } else {
        // byte by byte loop, but still need srca, dsta.
    }

which seems even More Correct, since the current access_memset path does not
check for read watchpoints or faults on all of [src, src+size-1].


r~



reply via email to

[Prev in Thread] Current Thread [Next in Thread]