On 1/8/20 11:32 AM, LIU Zhiwei wrote: >>> + switch (width) { >>> + case 8: >>> + if (vector_elem_mask(env, vm, width, lmul, i)) { >>> + while (k >= 0) { >>> + read = i * (nf + 1) + k; >>> + env->vfp.vreg[dest + k * lmul].u8[j] = >>> + cpu_ldub_data(env, env->gpr[rs1] + read); >> You must not modify vreg[x] before you've recognized all possible exceptions, >> e.g. validating that a subsequent access will not trigger a page fault. >> Otherwise you will have a partially modified register value when the >> exception >> handler is entered. > There are two questions here. > > 1) How to validate access before real access to registers? > > As pointed in another comment for patchset v1, > > "instructions that perform more than one host store must probe > the entire range to be stored before performing any stores. > "
Use probe_access (or one of the probe_write/probe_read helpers). Ideally one would then use the result, which is a host address, and perform direct loads/stores using that. The result may be null, indicating that the operation needs the i/o path. But in any case, after the probe we are guaranteed that the page is mapped and readable/writable. Note that probe_* does not allow [addr, addr+size) to cross a page boundary. So you do have to be prepared for the vector operation to consist of 2 pages, and probe both of them. > I didn't see the validation of page in SVE, for example, sve_st1_r, > which directly use the helper_ret_*_mmu that may cause an page fault > exception or ovelap a watchpoint, > before probe the entire range to be stored . Yes, this is a bug in SVE that will be fixed. Note that you should not use helper_ret_* anymore. I've just introduced cpu_{ld,st}*_mmuidx_ra() that should be used instead. > 2) Why not use the cpu_ld* API? It's possible to use cpu_ld*, but then you need to store the results into a temporary, and copy the result to the register afterward. But I think it's better to probe first and avoid a second copy. > I see in SVE that ld*_p is used to directly access the host memory. And > helper_ret_*_mmu > is used to access guest memory. But from the definition of cpu_ld*, it's the > combination of > ld*_p and helper_ret_*_mmu. This is all changed now, FWIW. > I will take it. However I didn't have a big-endian host to test the feature. You can apply for a gcc compile farm account, and then you will have access to ppc64 big-endian hosts. https://cfarm.tetaneutral.net/users/new/ r~