Re: [PATCH] Make PFN_PHYS return a properly-formed physical address - Kernel

This is a discussion on Re: [PATCH] Make PFN_PHYS return a properly-formed physical address - Kernel ; Andrew Morton wrote: > On Thu, 07 Aug 2008 14:38:08 -0700 > Jeremy Fitzhardinge wrote: > > >> PFN_PHYS, as its name suggests, turns a pfn into a physical address. >> However, it is a macro which just operates on ...

+ Reply to Thread
Results 1 to 14 of 14

Thread: Re: [PATCH] Make PFN_PHYS return a properly-formed physical address

  1. Re: [PATCH] Make PFN_PHYS return a properly-formed physical address

    Andrew Morton wrote:
    > On Thu, 07 Aug 2008 14:38:08 -0700
    > Jeremy Fitzhardinge wrote:
    >
    >
    >> PFN_PHYS, as its name suggests, turns a pfn into a physical address.
    >> However, it is a macro which just operates on its argument without
    >> modifying its type. pfns are typed unsigned long, but an unsigned
    >> long may not be long enough to hold a physical address (32-bit systems
    >> with more than 32 bits of physcial address). This means that the
    >> resulting address could be truncated if it doesn't fit within an
    >> unsigned long. This isn't generally a problem because most users end
    >> up using it for "low" memory, but there's no reason why PFN_PHYS
    >> couldn't be used for any possible pfn.
    >>

    >
    > Please copy a mailing list on patches. So you can get your titties
    > toasted off
    >


    Oops. Forgot.

    >> Fortunately, resource_size_t is the right size, and has approximately
    >> the right meaning. It's 64-bits on platforms where that's
    >> appropriate, but 32-bits where the extra bits are not needed.
    >>

    >
    > aww maaan. Hack or what?
    >


    I don't know. Is it? It's what linux/ioport.h:struct resource uses to
    hold "start" and "end", which presumably means its intended to hold
    arbitrary physical addresses.

    >> #define PFN_ALIGN(x) (((unsigned long)(x) + (PAGE_SIZE - 1)) & PAGE_MASK)
    >> #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
    >> #define PFN_DOWN(x) ((x) >> PAGE_SHIFT)
    >> -#define PFN_PHYS(x) ((x) << PAGE_SHIFT)
    >> +#define PFN_PHYS(x) ((resource_size_t)(x) << PAGE_SHIFT)
    >>

    >
    > Busted on PAE with CONFIG_RESOURCES_64BIT=n, surely?
    >


    Not an option:

    config X86_PAE
    def_bool n
    prompt "PAE (Physical Address Extension) Support"
    depends on X86_32 && !HIGHMEM4G
    select RESOURCES_64BIT


    And if you don't enable RESOURCES_64BIT, then I guess it's reasonable
    for PFN_PHYS to discount the possibility of high pages?

    > Can we please do this properly, whatever that is? Even a dumb
    > always-return-u64 would be better?
    >


    I had that originally, but someone (hpa?) suggested resource_size_t.
    The sad thing is that most users don't really care; they're either
    64-bit anyway, or immediately truncate the result to 32-bit.

    "Properly" would be to define a phys_addr_t which can always represent a
    physical address. We have one in x86-land, but I hesitate to add it for
    everyone else.

    >> printk("initrd extends beyond end of memory "
    >> - "(0x%08lx > 0x%08lx)\ndisabling initrd\n",
    >> + "(0x%08lx > 0x%08llx)\ndisabling initrd\n",
    >> INITRD_START + INITRD_SIZE,
    >> PFN_PHYS(max_low_pfn));
    >>

    >
    > that'll generate a compile warning if m32r can set CONFIG_RESOURCES_64BIT=n.
    >


    (u64) cast, I guess.

    J
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH] Make PFN_PHYS return a properly-formed physical address

    On Thu, 07 Aug 2008 15:10:11 -0700
    Jeremy Fitzhardinge wrote:

    > Andrew Morton wrote:
    > > On Thu, 07 Aug 2008 14:38:08 -0700
    > > Jeremy Fitzhardinge wrote:
    > >
    > >
    > >> PFN_PHYS, as its name suggests, turns a pfn into a physical address.
    > >> However, it is a macro which just operates on its argument without
    > >> modifying its type. pfns are typed unsigned long, but an unsigned
    > >> long may not be long enough to hold a physical address (32-bit systems
    > >> with more than 32 bits of physcial address). This means that the
    > >> resulting address could be truncated if it doesn't fit within an
    > >> unsigned long. This isn't generally a problem because most users end
    > >> up using it for "low" memory, but there's no reason why PFN_PHYS
    > >> couldn't be used for any possible pfn.
    > >>

    > >
    > > Please copy a mailing list on patches. So you can get your titties
    > > toasted off
    > >

    >
    > Oops. Forgot.
    >
    > >> Fortunately, resource_size_t is the right size, and has approximately
    > >> the right meaning. It's 64-bits on platforms where that's
    > >> appropriate, but 32-bits where the extra bits are not needed.
    > >>

    > >
    > > aww maaan. Hack or what?
    > >

    >
    > I don't know. Is it? It's what linux/ioport.h:struct resource uses to
    > hold "start" and "end", which presumably means its intended to hold
    > arbitrary physical addresses.


    Yes, but resource_size_t is for IO addressing, not for memory addressing.

    Lots of X86_32 machines can happily support 32-bit physical addresses
    for IO while needing >32 bit addresses for physical memory.


    > >> #define PFN_ALIGN(x) (((unsigned long)(x) + (PAGE_SIZE - 1)) & PAGE_MASK)
    > >> #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
    > >> #define PFN_DOWN(x) ((x) >> PAGE_SHIFT)
    > >> -#define PFN_PHYS(x) ((x) << PAGE_SHIFT)
    > >> +#define PFN_PHYS(x) ((resource_size_t)(x) << PAGE_SHIFT)
    > >>

    > >
    > > Busted on PAE with CONFIG_RESOURCES_64BIT=n, surely?
    > >

    >
    > Not an option:
    >
    > config X86_PAE
    > def_bool n
    > prompt "PAE (Physical Address Extension) Support"
    > depends on X86_32 && !HIGHMEM4G
    > select RESOURCES_64BIT
    >


    err, OK, that was a bit arbitrary of us.

    Oh well, scrub the above assertion.

    Then again, do all architectures disallow 32-bit resource_size_t on
    64-bit? And there's ppc32's CONFIG_HIGHMEM option to think about.

    > And if you don't enable RESOURCES_64BIT, then I guess it's reasonable
    > for PFN_PHYS to discount the possibility of high pages?
    >
    > > Can we please do this properly, whatever that is? Even a dumb
    > > always-return-u64 would be better?
    > >

    >
    > I had that originally, but someone (hpa?) suggested resource_size_t.
    > The sad thing is that most users don't really care; they're either
    > 64-bit anyway, or immediately truncate the result to 32-bit.
    >
    > "Properly" would be to define a phys_addr_t which can always represent a
    > physical address. We have one in x86-land, but I hesitate to add it for
    > everyone else.


    hm. It is a distinct and singular concept - it makes sense to have a
    specific type to represet "a physical address for memory".


    > >> printk("initrd extends beyond end of memory "
    > >> - "(0x%08lx > 0x%08lx)\ndisabling initrd\n",
    > >> + "(0x%08lx > 0x%08llx)\ndisabling initrd\n",
    > >> INITRD_START + INITRD_SIZE,
    > >> PFN_PHYS(max_low_pfn));
    > >>

    > >
    > > that'll generate a compile warning if m32r can set CONFIG_RESOURCES_64BIT=n.
    > >

    >
    > (u64) cast, I guess.
    >


    nope We don't know what type u64 has - some architectures use
    `unsigned long' (we might fix this soon).

    For now, a full cast to `unsigned long long' is needed.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH] Make PFN_PHYS return a properly-formed physical address

    Andrew Morton wrote:
    > Yes, but resource_size_t is for IO addressing, not for memory addressing.
    >
    > Lots of X86_32 machines can happily support 32-bit physical addresses
    > for IO while needing >32 bit addresses for physical memory.
    >


    Really? The resource tree treats normal memory as just another
    resource. Is it expected that you could have usable memory not
    represented by /proc/iomem?

    Hm, looks like memory hotplug assumes that resource_size_t is always
    64-bits, but the e820->resource conversion simply skips over-large
    addresses.

    >>>> #define PFN_ALIGN(x) (((unsigned long)(x) + (PAGE_SIZE - 1)) & PAGE_MASK)
    >>>> #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
    >>>> #define PFN_DOWN(x) ((x) >> PAGE_SHIFT)
    >>>> -#define PFN_PHYS(x) ((x) << PAGE_SHIFT)
    >>>> +#define PFN_PHYS(x) ((resource_size_t)(x) << PAGE_SHIFT)
    >>>>
    >>>>
    >>> Busted on PAE with CONFIG_RESOURCES_64BIT=n, surely?
    >>>
    >>>

    >> Not an option:
    >>
    >> config X86_PAE
    >> def_bool n
    >> prompt "PAE (Physical Address Extension) Support"
    >> depends on X86_32 && !HIGHMEM4G
    >> select RESOURCES_64BIT
    >>
    >>

    >
    > err, OK, that was a bit arbitrary of us.
    >
    > Oh well, scrub the above assertion.
    >
    > Then again, do all architectures disallow 32-bit resource_size_t on
    > 64-bit? And there's ppc32's CONFIG_HIGHMEM option to think about.
    >


    x86 and ppc were the only archs to touch it; they otherwise use the
    default of "default 64BIT".

    I didn't look at the ppc use case. I wasn't terribly concerned about
    current users of PFN_PHYS, because it presumably works OK for them.

    >> "Properly" would be to define a phys_addr_t which can always represent a
    >> physical address. We have one in x86-land, but I hesitate to add it for
    >> everyone else.
    >>

    >
    > hm. It is a distinct and singular concept - it makes sense to have a
    > specific type to represet "a physical address for memory".
    >


    Yes. We had to be particularly careful with it on x86 because of all
    the problems it's caused, but it is a generally useful thing to be able
    to talk about.

    Shall we go with just using plain u64 (or unsigned long long if we want
    a really consistent type) in the meantime, and then waffle about
    introducing a new type everywhere?

    Or we could redefine resource_size_t to be big enough to refer to any
    resource, including all memory. It's close to being that anyway.

    > nope We don't know what type u64 has - some architectures use
    > `unsigned long' (we might fix this soon).
    >
    > For now, a full cast to `unsigned long long' is needed.
    >


    Yep.

    J

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH] Make PFN_PHYS return a properly-formed physical address

    On Thu, 07 Aug 2008 16:45:12 -0700
    Jeremy Fitzhardinge wrote:

    > Shall we go with just using plain u64 (or unsigned long long if we want
    > a really consistent type) in the meantime, and then waffle about
    > introducing a new type everywhere?
    >
    > Or we could redefine resource_size_t to be big enough to refer to any
    > resource, including all memory. It's close to being that anyway.


    We could do

    typedef resource_size_t jeremy_thing_t;

    for now and worry about it later if the need arises, perhaps.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH] Make PFN_PHYS return a properly-formed physical address

    Andrew Morton wrote:
    > On Thu, 07 Aug 2008 16:45:12 -0700
    > Jeremy Fitzhardinge wrote:
    >
    >
    >> Shall we go with just using plain u64 (or unsigned long long if we want
    >> a really consistent type) in the meantime, and then waffle about
    >> introducing a new type everywhere?
    >>
    >> Or we could redefine resource_size_t to be big enough to refer to any
    >> resource, including all memory. It's close to being that anyway.
    >>

    >
    > We could do
    >
    > typedef resource_size_t jeremy_thing_t;
    >
    > for now and worry about it later if the need arises, perhaps.
    >


    OK, I'll revise and repost.

    Hm, curiously, the two arches which care most about this issue (x86 and
    powerpc) already have phys_addr_t...

    I guess we don't want CONFIG_ARCH_HAVE_PHYS_ADDR_T...

    J
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. [PATCH 2/2] make PFN_PHYS explicitly return phys_addr_t

    PFN_PHYS, as its name suggests, turns a pfn into a physical address.
    However, it is a macro which just operates on its argument without
    modifying its type. pfns are typed unsigned long, but an unsigned
    long may not be long enough to hold a physical address (32-bit systems
    with more than 32 bits of physcial address).

    Make sure we cast to phys_addr_t to return a complete result.

    Signed-off-by: Jeremy Fitzhardinge
    Cc: Eduardo Habkost
    ---
    arch/m32r/mm/discontig.c | 2 +-
    include/asm-x86/xen/page.h | 4 ++--
    include/linux/pfn.h | 6 +++++-
    3 files changed, 8 insertions(+), 4 deletions(-)

    diff -r 6b03eb6361dd arch/m32r/mm/discontig.c
    --- a/arch/m32r/mm/discontig.c Mon Aug 11 11:19:24 2008 -0700
    +++ b/arch/m32r/mm/discontig.c Mon Aug 11 11:20:07 2008 -0700
    @@ -112,9 +112,9 @@
    initrd_start, INITRD_SIZE);
    } else {
    printk("initrd extends beyond end of memory "
    - "(0x%08lx > 0x%08lx)\ndisabling initrd\n",
    + "(0x%08lx > 0x%08llx)\ndisabling initrd\n",
    INITRD_START + INITRD_SIZE,
    - PFN_PHYS(max_low_pfn));
    + (unsigned long long)PFN_PHYS(max_low_pfn));

    initrd_start = 0;
    }
    diff -r 6b03eb6361dd include/asm-x86/xen/page.h
    --- a/include/asm-x86/xen/page.h Mon Aug 11 11:19:24 2008 -0700
    +++ b/include/asm-x86/xen/page.h Mon Aug 11 11:20:07 2008 -0700
    @@ -76,13 +76,13 @@
    static inline xmaddr_t phys_to_machine(xpaddr_t phys)
    {
    unsigned offset = phys.paddr & ~PAGE_MASK;
    - return XMADDR(PFN_PHYS((u64)pfn_to_mfn(PFN_DOWN(phys.padd r))) | offset);
    + return XMADDR(PFN_PHYS(pfn_to_mfn(PFN_DOWN(phys.paddr))) | offset);
    }

    static inline xpaddr_t machine_to_phys(xmaddr_t machine)
    {
    unsigned offset = machine.maddr & ~PAGE_MASK;
    - return XPADDR(PFN_PHYS((u64)mfn_to_pfn(PFN_DOWN(machine.m addr))) | offset);
    + return XPADDR(PFN_PHYS(mfn_to_pfn(PFN_DOWN(machine.maddr) )) | offset);
    }

    /*
    diff -r 6b03eb6361dd include/linux/pfn.h
    --- a/include/linux/pfn.h Mon Aug 11 11:19:24 2008 -0700
    +++ b/include/linux/pfn.h Mon Aug 11 11:20:07 2008 -0700
    @@ -1,9 +1,13 @@
    #ifndef _LINUX_PFN_H_
    #define _LINUX_PFN_H_
    +
    +#ifndef __ASSEMBLY__
    +#include
    +#endif

    #define PFN_ALIGN(x) (((unsigned long)(x) + (PAGE_SIZE - 1)) & PAGE_MASK)
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
    #define PFN_DOWN(x) ((x) >> PAGE_SHIFT)
    -#define PFN_PHYS(x) ((x) << PAGE_SHIFT)
    +#define PFN_PHYS(x) ((phys_addr_t)(x) << PAGE_SHIFT)

    #endif


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. [PATCH 1/2] add phys_addr_t for holding physical addresses

    Add a kernel-wide "phys_addr_t" which is guaranteed to be able to hold
    any physical address. By default it equals the word size of the
    architecture, but a 32-bit architecture can set ARCH_PHYS_ADDR_T_64BIT
    if it needs a 64-bit phys_addr_t.

    Signed-off-by: Jeremy Fitzhardinge
    Cc: Paul Mackerras
    Cc: Benjamin Herrenschmidt
    diff -r 4909b40dbdc5 arch/powerpc/Kconfig
    --- a/arch/powerpc/Kconfig Fri Aug 08 13:40:52 2008 -0700
    +++ b/arch/powerpc/Kconfig Mon Aug 11 10:32:52 2008 -0700
    @@ -21,6 +21,9 @@

    config PPC_MERGE
    def_bool y
    +
    +config ARCH_PHYS_ADDR_T_64BIT
    + def_bool PPC64 || PHYS_64BIT

    config MMU
    bool
    diff -r 4909b40dbdc5 arch/x86/Kconfig
    --- a/arch/x86/Kconfig Fri Aug 08 13:40:52 2008 -0700
    +++ b/arch/x86/Kconfig Mon Aug 11 10:32:53 2008 -0700
    @@ -1002,6 +1002,9 @@
    has the cost of more pagetable lookup overhead, and also
    consumes more pagetable space per process.

    +config ARCH_PHYS_ADDR_T_64BIT
    + def_bool X86_64 || X86_PAE
    +
    # Common NUMA Features
    config NUMA
    bool "Numa Memory Allocation and Scheduler Support (EXPERIMENTAL)"
    diff -r 4909b40dbdc5 include/asm-powerpc/types.h
    --- a/include/asm-powerpc/types.h Fri Aug 08 13:40:52 2008 -0700
    +++ b/include/asm-powerpc/types.h Mon Aug 11 10:32:53 2008 -0700
    @@ -48,13 +48,6 @@

    typedef __vector128 vector128;

    -/* Physical address used by some IO functions */
    -#if defined(CONFIG_PPC64) || defined(CONFIG_PHYS_64BIT)
    -typedef u64 phys_addr_t;
    -#else
    -typedef u32 phys_addr_t;
    -#endif
    -
    #ifdef __powerpc64__
    typedef u64 dma_addr_t;
    #else
    diff -r 4909b40dbdc5 include/asm-x86/page_32.h
    --- a/include/asm-x86/page_32.h Fri Aug 08 13:40:52 2008 -0700
    +++ b/include/asm-x86/page_32.h Mon Aug 11 10:32:53 2008 -0700
    @@ -33,7 +33,6 @@
    typedef u64 pudval_t;
    typedef u64 pgdval_t;
    typedef u64 pgprotval_t;
    -typedef u64 phys_addr_t;

    typedef union {
    struct {
    @@ -54,7 +53,6 @@
    typedef unsigned long pudval_t;
    typedef unsigned long pgdval_t;
    typedef unsigned long pgprotval_t;
    -typedef unsigned long phys_addr_t;

    typedef union {
    pteval_t pte;
    diff -r 4909b40dbdc5 include/asm-x86/page_64.h
    --- a/include/asm-x86/page_64.h Fri Aug 08 13:40:52 2008 -0700
    +++ b/include/asm-x86/page_64.h Mon Aug 11 10:32:53 2008 -0700
    @@ -79,7 +79,6 @@
    typedef unsigned long pudval_t;
    typedef unsigned long pgdval_t;
    typedef unsigned long pgprotval_t;
    -typedef unsigned long phys_addr_t;

    typedef struct page *pgtable_t;

    diff -r 4909b40dbdc5 include/linux/types.h
    --- a/include/linux/types.h Fri Aug 08 13:40:52 2008 -0700
    +++ b/include/linux/types.h Mon Aug 11 10:32:53 2008 -0700
    @@ -197,6 +197,12 @@
    typedef u32 resource_size_t;
    #endif

    +#ifdef CONFIG_PHYS_ADDR_T_64BIT
    +typedef u64 phys_addr_t;
    +#else
    +typedef u32 phys_addr_t;
    +#endif
    +
    struct ustat {
    __kernel_daddr_t f_tfree;
    __kernel_ino_t f_tinode;
    diff -r 4909b40dbdc5 mm/Kconfig
    --- a/mm/Kconfig Fri Aug 08 13:40:52 2008 -0700
    +++ b/mm/Kconfig Mon Aug 11 10:32:53 2008 -0700
    @@ -190,6 +190,9 @@
    help
    This option allows memory and IO resources to be 64 bit.

    +config PHYS_ADDR_T_64BIT
    + def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT
    +
    config ZONE_DMA_FLAG
    int
    default "0" if !ZONE_DMA


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [PATCH 1/2] add phys_addr_t for holding physical addresses

    On Mon, 2008-08-11 at 12:38 -0700, Jeremy Fitzhardinge wrote:
    > Add a kernel-wide "phys_addr_t" which is guaranteed to be able to hold
    > any physical address. By default it equals the word size of the
    > architecture, but a 32-bit architecture can set ARCH_PHYS_ADDR_T_64BIT
    > if it needs a 64-bit phys_addr_t.


    I've been wondering for some time why can't we make phys_addr_t and
    resource_size_t the same thing... I don't like having two ARCH_* thing
    especially since I believe the one for resources is already what we
    want.

    Ben.

    > Signed-off-by: Jeremy Fitzhardinge
    > Cc: Paul Mackerras
    > Cc: Benjamin Herrenschmidt
    > diff -r 4909b40dbdc5 arch/powerpc/Kconfig
    > --- a/arch/powerpc/Kconfig Fri Aug 08 13:40:52 2008 -0700
    > +++ b/arch/powerpc/Kconfig Mon Aug 11 10:32:52 2008 -0700
    > @@ -21,6 +21,9 @@
    >
    > config PPC_MERGE
    > def_bool y
    > +
    > +config ARCH_PHYS_ADDR_T_64BIT
    > + def_bool PPC64 || PHYS_64BIT
    >
    > config MMU
    > bool
    > diff -r 4909b40dbdc5 arch/x86/Kconfig
    > --- a/arch/x86/Kconfig Fri Aug 08 13:40:52 2008 -0700
    > +++ b/arch/x86/Kconfig Mon Aug 11 10:32:53 2008 -0700
    > @@ -1002,6 +1002,9 @@
    > has the cost of more pagetable lookup overhead, and also
    > consumes more pagetable space per process.
    >
    > +config ARCH_PHYS_ADDR_T_64BIT
    > + def_bool X86_64 || X86_PAE
    > +
    > # Common NUMA Features
    > config NUMA
    > bool "Numa Memory Allocation and Scheduler Support (EXPERIMENTAL)"
    > diff -r 4909b40dbdc5 include/asm-powerpc/types.h
    > --- a/include/asm-powerpc/types.h Fri Aug 08 13:40:52 2008 -0700
    > +++ b/include/asm-powerpc/types.h Mon Aug 11 10:32:53 2008 -0700
    > @@ -48,13 +48,6 @@
    >
    > typedef __vector128 vector128;
    >
    > -/* Physical address used by some IO functions */
    > -#if defined(CONFIG_PPC64) || defined(CONFIG_PHYS_64BIT)
    > -typedef u64 phys_addr_t;
    > -#else
    > -typedef u32 phys_addr_t;
    > -#endif
    > -
    > #ifdef __powerpc64__
    > typedef u64 dma_addr_t;
    > #else
    > diff -r 4909b40dbdc5 include/asm-x86/page_32.h
    > --- a/include/asm-x86/page_32.h Fri Aug 08 13:40:52 2008 -0700
    > +++ b/include/asm-x86/page_32.h Mon Aug 11 10:32:53 2008 -0700
    > @@ -33,7 +33,6 @@
    > typedef u64 pudval_t;
    > typedef u64 pgdval_t;
    > typedef u64 pgprotval_t;
    > -typedef u64 phys_addr_t;
    >
    > typedef union {
    > struct {
    > @@ -54,7 +53,6 @@
    > typedef unsigned long pudval_t;
    > typedef unsigned long pgdval_t;
    > typedef unsigned long pgprotval_t;
    > -typedef unsigned long phys_addr_t;
    >
    > typedef union {
    > pteval_t pte;
    > diff -r 4909b40dbdc5 include/asm-x86/page_64.h
    > --- a/include/asm-x86/page_64.h Fri Aug 08 13:40:52 2008 -0700
    > +++ b/include/asm-x86/page_64.h Mon Aug 11 10:32:53 2008 -0700
    > @@ -79,7 +79,6 @@
    > typedef unsigned long pudval_t;
    > typedef unsigned long pgdval_t;
    > typedef unsigned long pgprotval_t;
    > -typedef unsigned long phys_addr_t;
    >
    > typedef struct page *pgtable_t;
    >
    > diff -r 4909b40dbdc5 include/linux/types.h
    > --- a/include/linux/types.h Fri Aug 08 13:40:52 2008 -0700
    > +++ b/include/linux/types.h Mon Aug 11 10:32:53 2008 -0700
    > @@ -197,6 +197,12 @@
    > typedef u32 resource_size_t;
    > #endif
    >
    > +#ifdef CONFIG_PHYS_ADDR_T_64BIT
    > +typedef u64 phys_addr_t;
    > +#else
    > +typedef u32 phys_addr_t;
    > +#endif
    > +
    > struct ustat {
    > __kernel_daddr_t f_tfree;
    > __kernel_ino_t f_tinode;
    > diff -r 4909b40dbdc5 mm/Kconfig
    > --- a/mm/Kconfig Fri Aug 08 13:40:52 2008 -0700
    > +++ b/mm/Kconfig Mon Aug 11 10:32:53 2008 -0700
    > @@ -190,6 +190,9 @@
    > help
    > This option allows memory and IO resources to be 64 bit.
    >
    > +config PHYS_ADDR_T_64BIT
    > + def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT
    > +
    > config ZONE_DMA_FLAG
    > int
    > default "0" if !ZONE_DMA
    >


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [PATCH 1/2] add phys_addr_t for holding physical addresses

    Benjamin Herrenschmidt wrote:
    > On Mon, 2008-08-11 at 12:38 -0700, Jeremy Fitzhardinge wrote:
    >
    >> Add a kernel-wide "phys_addr_t" which is guaranteed to be able to hold
    >> any physical address. By default it equals the word size of the
    >> architecture, but a 32-bit architecture can set ARCH_PHYS_ADDR_T_64BIT
    >> if it needs a 64-bit phys_addr_t.
    >>

    >
    > I've been wondering for some time why can't we make phys_addr_t and
    > resource_size_t the same thing... I don't like having two ARCH_* thing
    > especially since I believe the one for resources is already what we
    > want.
    >


    I made the same argument, but Andrew thinks they're conceptually
    distinct. It is theoretically possible you might have a system with >4G
    memory, but all io resources < 4G, so you'd have resource_size_t
    32-bits, while having 64-bit physical addresses. You can configure such
    a thing, but I don't know if it's 1) useful or 2) used.

    J
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [PATCH 1/2] add phys_addr_t for holding physical addresses

    On Mon, 2008-08-11 at 15:15 -0700, Jeremy Fitzhardinge wrote:
    > Benjamin Herrenschmidt wrote:
    > > On Mon, 2008-08-11 at 12:38 -0700, Jeremy Fitzhardinge wrote:
    > >
    > >> Add a kernel-wide "phys_addr_t" which is guaranteed to be able to hold
    > >> any physical address. By default it equals the word size of the
    > >> architecture, but a 32-bit architecture can set ARCH_PHYS_ADDR_T_64BIT
    > >> if it needs a 64-bit phys_addr_t.
    > >>

    > >
    > > I've been wondering for some time why can't we make phys_addr_t and
    > > resource_size_t the same thing... I don't like having two ARCH_* thing
    > > especially since I believe the one for resources is already what we
    > > want.
    > >

    >
    > I made the same argument, but Andrew thinks they're conceptually
    > distinct. It is theoretically possible you might have a system with >4G
    > memory, but all io resources < 4G, so you'd have resource_size_t
    > 32-bits, while having 64-bit physical addresses. You can configure such
    > a thing, but I don't know if it's 1) useful or 2) used.


    Are we sure resource_size_t is -never- used to represent memory ? I
    though it was on some platforms....

    Ben.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [PATCH 1/2] add phys_addr_t for holding physical addresses

    On Mon, 2008-08-11 at 15:50 -0700, Jeremy Fitzhardinge wrote:
    > Benjamin Herrenschmidt wrote:
    > > Are we sure resource_size_t is -never- used to represent memory ? I
    > > though it was on some platforms....

    >
    > On x86 it's optionally used to put memory in the resource tree, but if
    > the memory is larger than can be held in resource_size_t it simply skips
    > it. Don't know about elsewhere.


    That sounds like a good enough reason to not separate the two concepts..

    Ben.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: [PATCH 1/2] add phys_addr_t for holding physical addresses

    Benjamin Herrenschmidt wrote:
    > Are we sure resource_size_t is -never- used to represent memory ? I
    > though it was on some platforms....


    On x86 it's optionally used to put memory in the resource tree, but if
    the memory is larger than can be held in resource_size_t it simply skips
    it. Don't know about elsewhere.

    J

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [PATCH 1/2] add phys_addr_t for holding physical addresses

    Benjamin Herrenschmidt wrote:
    > On Mon, 2008-08-11 at 15:50 -0700, Jeremy Fitzhardinge wrote:
    >
    >> Benjamin Herrenschmidt wrote:
    >>
    >>> Are we sure resource_size_t is -never- used to represent memory ? I
    >>> though it was on some platforms....
    >>>

    >> On x86 it's optionally used to put memory in the resource tree, but if
    >> the memory is larger than can be held in resource_size_t it simply skips
    >> it. Don't know about elsewhere.
    >>

    >
    > That sounds like a good enough reason to not separate the two concepts..


    The resource_size_t situation is obscure. I'd be happy to just remove
    its config, use my patch and typedef phys_addr_t resource_size_t.

    J

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [PATCH 1/2] add phys_addr_t for holding physical addresses


    > The resource_size_t situation is obscure. I'd be happy to just remove
    > its config, use my patch and typedef phys_addr_t resource_size_t.


    I have no objection there as far as powerpc is concerned.

    Ben.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread