Re: [Qemu-devel] [PATCH v3 2/2] target-mips: Misaligned memory accesses for MSA

[Richard Henderson <rth@twiddle.net>]

On 05/14/2015 08:27 AM, Richard Henderson wrote:
> Perhaps
> 
>   void probe_read(CPUArchState *env, target_ulong addr, int mmu_idx,
>                             uintptr_t retaddr);
> 
>   void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx,
>                             uintptr_t retaddr);

Alternately, return the host address and then we're mostly overlapped with
tlb_vaddr_to_host.  Which was the function I was trying to remember earlier,
but doesn't *quite* do what I hoped.

What tlb_vaddr_to_host doesn't do is force a tlb_fill when the page in question
isn't in the tlb.  The helper dc_zva uses a subsequent store to force that.

I do wonder if the arm helper might be better written as

  uint64_t blocksize = ...
  uint64_t writesize = MIN(blocksize, TARGET_PAGE_SIZE);

  for (ofs = 0; ofs < blocklen; ofs += writesize) {
    hostaddr = probe_write(env, vaddr + ofs, mmu_idx, GETRA());
    if (hostaddr != NULL) {
        memset(hostaddr, 0, MIN(blocksize, writesize);
    } else {
        /* Since we didn't trap out of probe_write, the map is present
           and writable, but isn't RAM.  Do a series of byte writes as
           the architecture demands.  */
        for (i = 0; i < writesize; ++i) {
            helper_ret_stb_mmu(env, vaddr + ofs + i, 0, oi, GETRA());
        }
    }

Which does have different properties wrt the size of the memset in currently
unused cases of very large blocksize.  And probably the case of notdirty or
watchpointed ram as well.

For the case of MIPS under discussion, we could write this as

  baddr = probe_write(env, addr, mmu_idx, GETRA());
  eaddr = probe_write(env, addr + 15, mmu_idx, GETRA());

  /* We know both pages are present and writable.  */
  if (eaddr == baddr + 15) {
      /* Consecutive pages in RAM.  */
      memcpy(baddr, register, 16);
  } else {
      /* Someone's doing an MSA store to device memory.  */
      for (i = 0; i < 2; ++i) {
          helper_ret_stq_mmu(env, vaddr + i*8, register.d[0],
                             make_memop_idx(MO_UNALN | MO_TEQ, mmu_idx),
                             GETRA());
      }
  }

Thoughts?


r~