[Top][All Lists]

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

Re: [Qemu-devel] [PATCH v2 05/17] RISC-V: add vector extension load and

From: Richard Henderson
Subject: Re: [Qemu-devel] [PATCH v2 05/17] RISC-V: add vector extension load and store instructions
Date: Wed, 8 Jan 2020 12:08:15 +1000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2

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.



reply via email to

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