Re: [Qemu-devel] [RFC 3/5] softmmu: Add helpers for a new slow-path

[alvise rigo]

On Thu, May 7, 2015 at 7:56 PM, Richard Henderson <address@hidden> wrote:
> On 05/06/2015 08:38 AM, Alvise Rigo wrote:
>> +#define DATA_SIZE (1 << SHIFT)
>> +
>> +#if DATA_SIZE == 8
>> +#define SUFFIX q
>> +#define LSUFFIX q
>> +#define SDATA_TYPE  int64_t
>> +#define DATA_TYPE  uint64_t
>
> Duplicating all of the stuff from softmmu_template.h is Just Wrong.
>
>> +/* For the benefit of TCG generated code, we want to avoid the complication
>> +   of ABI-specific return type promotion and always return a value extended
>> +   to the register size of the host.  This is tcg_target_long, except in the
>> +   case of a 32-bit host and 64-bit data, and for that we always have
>> +   uint64_t.  Don't bother with this widened value for SOFTMMU_CODE_ACCESS. 
>>  */
>
> ...
>
>> +uint32_t helper_le_stcond_name(CPUArchState *env, target_ulong addr,
>> +                               DATA_TYPE val, int mmu_idx, uintptr_t 
>> retaddr)
>
> You didn't even read the comment above re the return type.

This part has to be reworked quite a bit. Thank you for pointing this out.

>
>> +        /* Another vCPU has accessed the memory after the LoadLink or not 
>> link
>> +         * has been previously set. */
>> +        ret = 1;
>> +        goto out;
>> +    }
>> +
>> +    helper_st_legacy(env, addr, val, mmu_idx, retaddr);
>> +
>> +    /* Set the page as dirty to avoid the creation of TLB entries with the
>> +     * TLB_EXCL bit set. */
>> +    cpu_physical_memory_set_excl_dirty(ram_addr);
>> +
>> +    /* The StoreConditional succeeded */
>> +    ret = 0;
>
> How did you come up with 0 == success, 1 = failure?   Is this an arm-ism?

Yes, but it made sense to me to keep in that way.
I will add a comment on top of the method explaining the return values.

Thank you,
alvise

>
>
> r~