Re: [Xen-devel] [PATCH V2 27/46] xen: arm: arm64 trap handling.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-02-21 at 15:10 +0000, Tim Deegan wrote:
> At 16:47 +0000 on 14 Feb (1360860461), Ian Campbell wrote:
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > ---
> > v2: Call leave_hypervisor_tail on exit back to guest, disable interrupts 
> > while
> >     restoring state.
> 
> You don't seem to have addressed my other comments on v1:

I've got them in v3, I noted that I hadn't addresses you comment on this
patch in the #0/46.

> > --- a/xen/arch/arm/arm64/Makefile
> > +++ b/xen/arch/arm/arm64/Makefile
> > @@ -1,5 +1,7 @@
> >  subdir-y += lib
> >  
> > +obj-y += entry.o
> >  obj-y += mode_switch.o
> >  
> > +obj-y += traps.o
> >  obj-y += domain.o
> 
> Alphabetical order, please.

I kept this the same order as arm32/Makefile on purpose.

> > +#define __L2(_x)  (((_x) & 0x00000002) ?   1 : 0)
> > +#define __L4(_x)  (((_x) & 0x0000000c) ? ( 2 + __L2( (_x)>> 2)) : __L2( 
> > _x))
> > +#define __L8(_x)  (((_x) & 0x000000f0) ? ( 4 + __L4( (_x)>> 4)) : __L4( 
> > _x))
> > +#define __L16(_x) (((_x) & 0x0000ff00) ? ( 8 + __L8( (_x)>> 8)) : __L8( 
> > _x))
> > +#define LOG_2(_x) (((_x) & 0xffff0000) ? (16 + __L16((_x)>>16)) : 
> > __L16(_x))
> 
> This is now replicated in three places.  Maybe it should live in, say,
> xen/bitops.h?
[...]
> Keep the array type?  uint8_t[] would do, or define up something the
> right size. 

I've got both of these in my tree already for v3.

Ian.


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