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

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx>

Specifically "Write only as write ignore" and "Read only as read as
zero" (essentially s/+/as/)

Is that clear enough do you think?

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel