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

["Han, Weidong" <weidong.han@xxxxxxxxx>]

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