Re: [Xen-devel] [PATCH v2 1/4] xen/arm: Add macro MB

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-09-26 at 16:32 +0100, Julien Grall wrote:
> On 09/26/2013 04:30 PM, Ian Campbell wrote:
> > On Thu, 2013-09-26 at 16:26 +0100, Julien Grall wrote:
> >> On 09/26/2013 03:32 PM, Ian Campbell wrote:
> >>> On Thu, 2013-09-26 at 13:56 +0100, Julien Grall wrote:
> >>>> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> >>>> ---
> >>>>  xen/include/asm-arm/config.h |    1 +
> >>>>  1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> >>>> index 604088e..2cea1ba 100644
> >>>> --- a/xen/include/asm-arm/config.h
> >>>> +++ b/xen/include/asm-arm/config.h
> >>>> @@ -119,6 +119,7 @@
> >>>>  #define BOOT_MISC_VIRT_START   _AT(vaddr_t,0x00600000)
> >>>>  
> >>>>  #define HYPERVISOR_VIRT_START  XEN_VIRT_START
> >>>> +#define MB(_mb)     (_AC(_mb, UL) << 20)
> >>>
> >>> Can you move the GB here too for consistency.
> >>>
> >>> In fact it would be worth considering moving this to
> >>> xen/include/xen/config.h and consolidating the x86 version too.
> >>
> >> I will do.
> > 
> > Thanks.
> > 
> >> I'm wondering, do we need to use ULL instead of UL in GB and MB?
> > 
> > Is it used with a paddr_t anywhere? If it's just vaddr then UL is fine.
> 
> MB will be used with paddr_t. GB not yet, but it could be used to
> replace 0x1...ULL in a such patch:
> http://permalink.gmane.org/gmane.comp.emulators.xen.devel/172128
> 

OK, maybe give it a go. I don't think there will be any critical places
which would be badly impacted on 32-bit from having to use two registers
etc where one would do.

My only other concern would be assignments complaining about being
truncated and the ball of wool effect fixing them all up -- but it
quickly be obvious to you if this is going to be a problem!

Ian.


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