Re: [Qemu-devel] [PATCH 2/2] target-arm: Add checks that cpreg raw accesses are handled

[Greg Bellows]

On Mon, Jan 19, 2015 at 1:03 PM, Peter Maydell <address@hidden> wrote:

On 19 January 2015 at 18:05, Greg Bellows <address@hidden> wrote:
>
>
> On Mon, Jan 19, 2015 at 9:17 AM, Peter Maydell <address@hidden>
> wrote:
>> +    if (ri->type & ARM_CP_CONST) {
>> +        return true;
>> +    }
>
>
> Had to refresh my memory on this.  It appears we changed the name (polarity)
> of the function based on our previous discussion.  However, according to the
> above description, we should return 'true' if read/write would cause an
> assertion. but in the case of CONST we would not assert, but still return
> 'true'?

Doh. I inverted the name and polarity but forgot to change the function
body. (I have no idea why that didn't blow up). Will fix (and test a
bit more thoroughly...)

​FYI, I actually ran the changes and they do unwantedly assert.​

 

>>
>> +    return (ri->raw_writefn || ri->writefn || ri->fieldoffset) &&
>> +        (ri->raw_readfn || ri->readfn || ri->fieldoffset);
>
>
> This case appears to need inverting as it will resolve to 'true' but should
> be valid.
>
> NIT: It would be cleaner to pull the ri->fieldoffset check above this check
> to simplify the conditional.

That's deliberately matching the checks in the read/write_raw_cp_reg
functions, but then I didn't do that for the CP_CONST check I guess.

-- PMM