Re: [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers

[Julien Grall <julien.grall@citrix.com>]

Hi Ian,

On 08/05/15 11:16, Ian Campbell wrote:
> On Sat, 2015-04-25 at 22:16 +0500, Julien Grall wrote:
>> Hi Ian,
>>
>> On 17/04/2015 19:01, Ian Campbell wrote:
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>> ---
>>> v2: Move last paramter of a handle_ro_raz call to next patch where it
>>>      belongs.
>>> ---
>>>   xen/arch/arm/traps.c |   52 ++++++++++++++++++++++++++++++++------------------
>>>   1 file changed, 33 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>>> index 8b1846a..b54aef6 100644
>>> --- a/xen/arch/arm/traps.c
>>> +++ b/xen/arch/arm/traps.c
>>> @@ -1587,6 +1587,34 @@ static void handle_raz_wi(struct cpu_user_regs *regs,
>>>       advance_pc(regs, hsr);
>>>   }
>>>
>>> +/* Write only + write ignore */
>>
>> [..]
>>
>>> +/* Read only + read as zero */
>>
>> I'm not sure if we finished the discussion on those comment on v1 before 
>> you sent the v2.
> 
> I think we hadn't.
> 
>> The "+" is very confusing for me because it indicates two parts: write 
>> only and write ignore (same for the read). Both part doesn't really fit 
>> together. Although this helper clearly choose to implement WO as WI 
>> (resp. RO as RAZ).
> 
>> I think this should be clearer in order to avoid people think this can 
>> be used for RO but with a different value than 0.
> 
> For v3 I've made the change I proposed in
> <1429266891.25195.260.camel@citrix.com>
> 
> Specifically "Write only as write ignore" and "Read only as read as
> zero" (essentially s/+/as/)
> 
> Is that clear enough do you think?

Yes. Thanks.

With this changes:

Reviewed-by: Julien Grall <julien.grall@citrix.com>

Regards,

-- 
Julien Grall