[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [Xen-devel] [QEMU] Correct PAGE_MASK definition


  • To: "Ian Jackson" <Ian.Jackson@xxxxxxxxxxxxx>
  • From: "Han, Weidong" <weidong.han@xxxxxxxxx>
  • Date: Tue, 13 Nov 2007 19:54:43 +0800
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
  • Delivery-date: Tue, 13 Nov 2007 03:55:44 -0800
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: Acgl4KAniQii0+D5TwOMsduiSLcekgACbIvg
  • Thread-topic: [Xen-devel] [QEMU] Correct PAGE_MASK definition

Ian Jackson wrote:
> Han, Weidong writes ("[Xen-devel] [QEMU] Correct PAGE_MASK
> definition"): 
>> In tools/ioemu/hw/iommu.c, I found #define PAGE_MASK  (PAGE_SIZE -
>> 1), I think it's not correct.
> 
> Well, yes, but just changing the definition isn't correct either.
> This particular definition is used in only one place, in
> iommu_translate_pa:
> 
>     pa = ((pa & IOPTE_PAGE) << 4) + (addr & PAGE_MASK);
> 
> So here it is used as `the part of the address which specifies the
> location within the page' rather than `which specifies the page'.
> This is both confusing and inconsistent with most of the other things
> called *PAGE_MASK.
> 
> grepping xen-unstable shows similar confusion here:
>  extras/mini-os/include/ia64/efi.h:#define EFI_PAGE_MASK   0xFFF
>  tools/libxc/xc_ptrace.h:#define BSD_PAGE_MASK (PAGE_SIZE-1)
> 
> This confusion seems to be present in linux-2.6.18-xen.hg too (and I
> imagine these came from upstream):
>  arch/mips/boot/addinitrd.c:#define MIPS_PAGE_MASK    
>  (MIPS_PAGE_SIZE-1) drivers/media/dvb/b2c2/flexcop-usb.h:#define
>  V8_MEMORY_PAGE_MASK     0x7FFF drivers/mtd/nand/nand_base.c:#define
>  BBT_PAGE_MASK    0xffffff3f drivers/w1/slaves/w1_ds2433.c:#define
> W1_PAGE_MASK            0x1F 
> 
> Under these circumstances it may be best to just leave things be
> rather than try to make this consistent.  In any case if you change
> the definition you must change the point of use too.
> 

Agree, but inconsistent usage is not good after all.

-- Weidong

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.