Re: [Qemu-devel] [PATCH v5 12/37] target-arm: A64: Implement DC ZVA

[Richard Henderson <rth@twiddle.net>]

On 03/28/2014 09:09 AM, Peter Maydell wrote:
> +            for (i = 0; i < maxidx; i++) {
> +                hostaddr[i] = tlb_vaddr_to_host(env,
> +                                                vaddr + TARGET_PAGE_SIZE * i,
> +                                                1, cpu_mmu_index(env));
> +                if (!hostaddr[i]) {
> +                    break;
> +                }
> +            }
> +            if (i == maxidx) {
> +                /* If it's all in the TLB it's fair game for just writing to;
> +                 * we know we don't need to update dirty status, etc.
> +                 */
> +                for (i = 0; i < maxidx - 1; i++) {
> +                    memset(hostaddr[i], 0, TARGET_PAGE_SIZE);
> +                }
> +                memset(hostaddr[i], 0, blocklen - (i * TARGET_PAGE_SIZE));
> +                return;
> +            }

Doesn't this fail if blocklen < TARGET_PAGE_SIZE?

Since blocklen must be a power of 4, it's either less than TARGET_PAGE_SIZE or
a multiple of TARGET_PAGE_SIZE, so that last memset looks suspect.

I think all this would be easier to follow as two cases:

    if (blocklen <= TARGET_PAGE_SIZE) {
        // One look up and no hostaddr array
    } else {
        // Multiple pages; much of what you have now, only no partial pages
    }


r~