[PATCH] x86: create array based interface to change page attribute - Kernel

This is a discussion on [PATCH] x86: create array based interface to change page attribute - Kernel ; Thomas Hellström wrote: > Arjan van de Ven wrote: >> Thomas Hellström wrote: >>> Given this problem, the previously mentioned use-case, and the fact >>> that we mostly really use user-space mappings, >>> Is there a possibility we could add ...

+ Reply to Thread
Page 2 of 2 FirstFirst 1 2
Results 21 to 39 of 39

Thread: [PATCH] x86: create array based interface to change page attribute

  1. Re: [PATCH] x86: create array based interface to change page attribute

    Thomas Hellström wrote:
    > Arjan van de Ven wrote:
    >> Thomas Hellström wrote:
    >>> Given this problem, the previously mentioned use-case, and the fact
    >>> that we mostly really use user-space mappings,
    >>> Is there a possibility we could add the following functions to Dave's
    >>> patch (provided they would work as intended, of course, namely
    >>> invalidate / bring back the kernel mapping).

    >>
    >> sadly there are multiple mappings, both in theory and practice.
    >> Especially the _np / _p functions specifically work on only the
    >> mapping you specify.
    >>
    >> For this to work we would need to somehow make a "mark all mappings
    >> NP, but please only do the kernel ones" kind of thing.
    >> The semantics of that are... lets say messy at best.

    > Hmm, I'm not sure I follow you here. Are you saying that it's illegal to
    > have an NP mapping of a page (which, If I understand it correctly, means
    > no mapping at all) at the same time as you have a, say user-space WC
    > mapping pointing to the same physical page?


    no.
    What I'm saying is that even if you make one mapping NP, it's hard to know you got all of them,
    even just the kernel one.

    >
    > I was under the impression that calling CPA on the kernel mapping of
    > that page would do the rest?


    this is not a correct assumption in general, especially not for things like
    "present".
    If a page is mapped 3 times in the kernel, and you set one to "np", the others
    WILL stay mapped.

    (and CPA itself no longer exists period
    --
    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] x86: create array based interface to change page attribute


    On Mon, 2008-03-31 at 13:46 +0200, Andi Kleen wrote:
    > On Mon, Mar 31, 2008 at 09:21:19PM +1000, Dave Airlie wrote:
    > > On Mon, Mar 31, 2008 at 5:25 PM, Andi Kleen wrote:
    > > > Dave Airlie writes:
    > > > >
    > > > > +#define CPA_FLUSHTLB 1
    > > > > +#define CPA_ARRAY 2
    > > >
    > > > I don't think CPA_ARRAY should be a separate case. Rather single
    > > > page flushing should be an array with only a single entry. pageattr
    > > > is already very complex, no need to make add more special cases.

    > >
    > > I thought about this but the current interface takes a start address
    > > and number of pages from that point to cpa,
    > > the array interface takes an array of page sized pages.
    > >
    > > I don't really think we need to generate an array in the first case
    > > with all the pages in it..

    >
    > Just put the length into the array members too.


    I can only think to do this by maybe stealing some bits, and having the
    caller provide a pfn + length in one 64-bit dword.

    As otherwise I'm going to end up with the largest use case for this
    having to allocate a large array of 64-bit dwords all containing 1.

    This isn't worth it IMHO and the array doesn't add that much more
    complexity.

    Dave.

    --
    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] x86: create array based interface to change page attribute

    Arjan van de Ven wrote:
    > Thomas Hellström wrote:
    >> Arjan van de Ven wrote:
    >>> Thomas Hellström wrote:
    >>>> Given this problem, the previously mentioned use-case, and the fact
    >>>> that we mostly really use user-space mappings,
    >>>> Is there a possibility we could add the following functions to
    >>>> Dave's patch (provided they would work as intended, of course,
    >>>> namely invalidate / bring back the kernel mapping).
    >>>
    >>> sadly there are multiple mappings, both in theory and practice.
    >>> Especially the _np / _p functions specifically work on only the
    >>> mapping you specify.
    >>>
    >>> For this to work we would need to somehow make a "mark all mappings
    >>> NP, but please only do the kernel ones" kind of thing.
    >>> The semantics of that are... lets say messy at best.

    >> Hmm, I'm not sure I follow you here. Are you saying that it's illegal
    >> to have an NP mapping of a page (which, If I understand it correctly,
    >> means no mapping at all) at the same time as you have a, say
    >> user-space WC mapping pointing to the same physical page?

    >
    > no.
    > What I'm saying is that even if you make one mapping NP, it's hard to
    > know you got all of them,
    > even just the kernel one.
    >
    >>
    >> I was under the impression that calling CPA on the kernel mapping of
    >> that page would do the rest?

    >
    > this is not a correct assumption in general, especially not for things
    > like
    > "present".
    > If a page is mapped 3 times in the kernel, and you set one to "np",
    > the others
    > WILL stay mapped.
    >
    > (and CPA itself no longer exists period

    But what mappings are there, immediately after alloc_page(), that
    set_memory_np won't catch? Certainly kmap_atomic() used to leave some
    stale mappings floating around, but that has been fixed as far as I can
    tell, and I guess we must be able to assume that a driver keeps track of
    its own kernel mappings and kill them before calling set_memory_np.

    Drivers relying on set_memory_uc touching all mappings the driver hasn't
    set up itself must then have the same problem and needs to be fixed;
    referring in particular to agpgart for which driver the old CPA
    functionality was once created, IIRC.

    We should probably get this right as soon as possible, as the agpgart
    driver is (and has for the last couple of years been) relying on wc / uc
    aliasing to be legal. If we can fix this with set_memory_np /
    set_memory_p that would be great, and would have some additional
    benefits for the drm memory manager as well.

    /Thomas






    --
    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] x86: create array based interface to change page attribute

    Thomas Hellström wrote:

    > But what mappings are there, immediately after alloc_page(), that
    > set_memory_np won't catch?


    For example on x86 64bit, the kernel text is mapped (to allow relocatable) in another
    space as well.. and to allow 2Mb tlbs for this, that second mapping is bigger than it strictly needs to be.
    So your alloc_page() could get a page from the free pool that comes from the pages that have a second mapping
    due to this rounding.
    (and more fun, since it's close to frequently accessed memory, the hw prefetchers may actually just decide to pull
    such pages into the cache preemptively)


    > Drivers relying on set_memory_uc touching all mappings the driver hasn't
    > set up itself must then have the same problem and needs to be fixed;
    > referring in particular to agpgart for which driver the old CPA
    > functionality was once created, IIRC.


    "uc" is different than "np"; for "uc" the implementation, if your cpu needs this, will fix
    up the shadow text mappings already today.

    for "present" this doesn't make sense, since no cpu needs this.
    --
    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] x86: create array based interface to change page attribute

    Arjan van de Ven wrote:
    > Thomas Hellström wrote:
    >
    >> But what mappings are there, immediately after alloc_page(), that
    >> set_memory_np won't catch?

    >
    > For example on x86 64bit, the kernel text is mapped (to allow
    > relocatable) in another
    > space as well.. and to allow 2Mb tlbs for this, that second mapping is
    > bigger than it strictly needs to be.
    > So your alloc_page() could get a page from the free pool that comes
    > from the pages that have a second mapping
    > due to this rounding.
    > (and more fun, since it's close to frequently accessed memory, the hw
    > prefetchers may actually just decide to pull
    > such pages into the cache preemptively)
    >
    >
    >> Drivers relying on set_memory_uc touching all mappings the driver
    >> hasn't set up itself must then have the same problem and needs to be
    >> fixed; referring in particular to agpgart for which driver the old
    >> CPA functionality was once created, IIRC.

    >
    > "uc" is different than "np"; for "uc" the implementation, if your cpu
    > needs this, will fix
    > up the shadow text mappings already today.
    >
    > for "present" this doesn't make sense, since no cpu needs this.

    If this is the checkalias() thingy in x86/pageattr.c, it looks like it
    doesn't handle np different from the uc case. It does obviously skip NX
    bit manipulations, though.

    Anyway, a more direct question: If we were to fix whatever's missing for
    the code to fixup the x86-64 shadow text mapping, would you be opposing
    this way to fix the long standing uc/wc aliasing issue, provided we
    don't hit any other problems, like clflush() refusing to run on an NP page?

    /Thomas



    --
    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. Re: [PATCH] x86: create array based interface to change page attribute

    Thomas Hellström wrote:
    > Arjan van de Ven wrote:
    >> Thomas Hellström wrote:
    >>
    >>> But what mappings are there, immediately after alloc_page(), that
    >>> set_memory_np won't catch?

    >>
    >> For example on x86 64bit, the kernel text is mapped (to allow
    >> relocatable) in another
    >> space as well.. and to allow 2Mb tlbs for this, that second mapping is
    >> bigger than it strictly needs to be.
    >> So your alloc_page() could get a page from the free pool that comes
    >> from the pages that have a second mapping
    >> due to this rounding.
    >> (and more fun, since it's close to frequently accessed memory, the hw
    >> prefetchers may actually just decide to pull
    >> such pages into the cache preemptively)
    >>
    >>
    >>> Drivers relying on set_memory_uc touching all mappings the driver
    >>> hasn't set up itself must then have the same problem and needs to be
    >>> fixed; referring in particular to agpgart for which driver the old
    >>> CPA functionality was once created, IIRC.

    >>
    >> "uc" is different than "np"; for "uc" the implementation, if your cpu
    >> needs this, will fix
    >> up the shadow text mappings already today.
    >>
    >> for "present" this doesn't make sense, since no cpu needs this.

    > If this is the checkalias() thingy in x86/pageattr.c, it looks like it
    > doesn't handle np different from the uc case. It does obviously skip NX
    > bit manipulations, though.
    >
    > Anyway, a more direct question: If we were to fix whatever's missing for
    > the code to fixup the x86-64 shadow text mapping, would you be opposing
    > this way


    I do really oppose set_memory_np/p to work on anything but the mapping it is handled.
    that _uc and co touch other mappings is a cpu specific property and an implementation
    detail to the API. If/When CPUs exist that don't have issues with aliases, those CPUs
    will not change the other mappings.

    > to fix the long standing uc/wc aliasing issue, provided we


    I'm not opposed to a real fix. I am opposed to a bad hack.

    > don't hit any other problems, like clflush() refusing to run on an NP page?


    yes clflush doesn't work on not present pages
    --
    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. Re: [PATCH] x86: create array based interface to change page attribute

    Arjan van de Ven wrote:
    > Thomas Hellström wrote:
    >
    > > to fix the long standing uc/wc aliasing issue, provided we

    >
    > I'm not opposed to a real fix. I am opposed to a bad hack.
    >

    Great. So a real clean fix involves setting all "default" kernel
    mappings either to WC (which will require PAT) or
    Unmapped, for a pool of pages used in the graphics tables.

    To reduce the number of attribute changes for mappings that are
    frequently switched, and also to reduce the number of clflushes, and to
    avoid waiting for upcoming wc versions of set_memory_xx, I have a strong
    preference for unmapping the pages.

    Now is where I need some guidance, because interface design is not my
    strong side. I see three possible ways to do this.

    1) Use set_memory_np(). Not desirable since we want to be able to use
    that function on a single mapping, and not imply other semantics.
    2) Have the driver try to find out which "default" mappings the kernel
    has set up on a page and call set_memory_np() on each one of them. This
    seems very fragile and ugly even to me.
    3) Have code in x86/pageattr.c decide which "default" mappings are
    present on the given pages and set them all as non-present.
    In fact, there is already such a function in pageattr.c:

    kernel_map_pages(struct page *pages, int numpages, bool enable);

    But it's for debugging purposes only, could we use and export a variant
    of this?

    I guess I need a hint as to what's considered allowable here, to avoid
    spending a lot of time on something that will in the end get rejected
    anyway.

    Thanks,

    Thomas





    --
    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] x86: create array based interface to change page attribute

    On Wednesday, April 02, 2008 10:57 am Thomas Hellström wrote:
    > Arjan van de Ven wrote:
    > > Thomas Hellström wrote:
    > > > to fix the long standing uc/wc aliasing issue, provided we

    > >
    > > I'm not opposed to a real fix. I am opposed to a bad hack.

    >
    > Great. So a real clean fix involves setting all "default" kernel
    > mappings either to WC (which will require PAT) or
    > Unmapped, for a pool of pages used in the graphics tables.
    >
    > To reduce the number of attribute changes for mappings that are
    > frequently switched, and also to reduce the number of clflushes, and to
    > avoid waiting for upcoming wc versions of set_memory_xx, I have a strong
    > preference for unmapping the pages.


    Hopefully the WC stuff will be upstream right after 2.6.25 comes out. Any
    reason why we shouldn't keep the pages mapped in the kernel as WC assuming
    the interface is there?

    And we really should be keeping pools of pages around with the right type--we
    don't want to change attributes any more than absolutely necessary (the ia64
    uncached allocator does this right already, and in the DRM we actually keep
    the mappings around right now afaict). We can allocate & free large chunks
    at a time to deal with memory pressure one way or another...

    > 3) Have code in x86/pageattr.c decide which "default" mappings are
    > present on the given pages and set them all as non-present.
    > In fact, there is already such a function in pageattr.c:
    >
    > kernel_map_pages(struct page *pages, int numpages, bool enable);
    >
    > But it's for debugging purposes only, could we use and export a variant
    > of this?
    >
    > I guess I need a hint as to what's considered allowable here, to avoid
    > spending a lot of time on something that will in the end get rejected
    > anyway.


    I think we do want an interface like this, even if only for graphics memory
    (though I suspect some other device might like it as well). We'll also want
    to do it at runtime periodically to allocate new hunks of memory for graphics
    use, so a boot-time only thing won't work.

    Also, to make the API readable, we'd probably want to split the function into
    kernel_map_pages(..., enum memory_type type) and kernel_unmap_pages(...)
    (though like I said I think we really should be mapping them WC not umapping
    them altogether, since we do want to hit the ring buffer from the kernel with
    the WC type for example).

    Question is, will kernel_map_pages catch all the various kernel mappings
    (regular identity map, large page text map,e tc.), perform the proper
    flushing, and generally make sure we don't machine check on all platforms?

    Jesse
    --
    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] x86: create array based interface to change page attribute

    Jesse Barnes wrote:
    > On Wednesday, April 02, 2008 10:57 am Thomas Hellström wrote:
    >
    >> Arjan van de Ven wrote:
    >>
    >>> Thomas Hellström wrote:
    >>>
    >>>> to fix the long standing uc/wc aliasing issue, provided we
    >>>>
    >>> I'm not opposed to a real fix. I am opposed to a bad hack.
    >>>

    >> Great. So a real clean fix involves setting all "default" kernel
    >> mappings either to WC (which will require PAT) or
    >> Unmapped, for a pool of pages used in the graphics tables.
    >>
    >> To reduce the number of attribute changes for mappings that are
    >> frequently switched, and also to reduce the number of clflushes, and to
    >> avoid waiting for upcoming wc versions of set_memory_xx, I have a strong
    >> preference for unmapping the pages.
    >>

    >
    > Hopefully the WC stuff will be upstream right after 2.6.25 comes out. Any
    > reason why we shouldn't keep the pages mapped in the kernel as WC assuming
    > the interface is there?
    >

    If the pages are unmapped, we can get reasonable speed doing
    unbind-read-bind operations, kernel accesses to the memory will need to
    use an iomap_atomic_prot_pfn() type of operation.
    No IPI global tlb flushes needed for kernel mapping changes during
    unbind-read-bind and no cache flushes needed either if we write-protect
    the user-space mappings properly, or very limited cache flushes if we
    keep dirty-written-while-cached flags for each page.

    If the pages are wc-d we'll need two extra IPI global tlb flushes and a
    buffer-size cache flush every time we do unbind-read-bind, but OTOH we
    don't need the iomap_atomic_prot_pfn() to access single pages from the
    kernel.

    iomap_atomic_prot_pfn() should be really fast. It requires a
    single-page-single-processor tlb flush per map-unmap operation. For
    long-term and larger buffer maps we'll either use ioremap() or vmap()
    depending on memory type.
    > And we really should be keeping pools of pages around with the right type--we
    > don't want to change attributes any more than absolutely necessary (the ia64
    > uncached allocator does this right already, and in the DRM we actually keep
    > the mappings around right now afaict). We can allocate & free large chunks
    > at a time to deal with memory pressure one way or another...
    >
    >

    Agreed.
    >> 3) Have code in x86/pageattr.c decide which "default" mappings are
    >> present on the given pages and set them all as non-present.
    >> In fact, there is already such a function in pageattr.c:
    >>
    >> kernel_map_pages(struct page *pages, int numpages, bool enable);
    >>
    >> But it's for debugging purposes only, could we use and export a variant
    >> of this?
    >>
    >> I guess I need a hint as to what's considered allowable here, to avoid
    >> spending a lot of time on something that will in the end get rejected
    >> anyway.
    >>

    >
    > I think we do want an interface like this, even if only for graphics memory
    > (though I suspect some other device might like it as well). We'll also want
    > to do it at runtime periodically to allocate new hunks of memory for graphics
    > use, so a boot-time only thing won't work.
    >
    > Also, to make the API readable, we'd probably want to split the function into
    > kernel_map_pages(..., enum memory_type type) and kernel_unmap_pages(...)
    > (though like I said I think we really should be mapping them WC not umapping
    > them altogether, since we do want to hit the ring buffer from the kernel with
    > the WC type for example).
    >

    I think ring-buffers are using ioremap() or vmap() already today. We can
    use these to get WC-type access also in the future. The only time we use
    the linear kernel mapping today is for single page access while patching
    up command buffers.

    So it's really a tradeoff between slightly faster single-page access and
    really fast unbind-read-bind operations. That's really why I'm
    suggesting unmapped pages.

    > Question is, will kernel_map_pages catch all the various kernel mappings
    > (regular identity map, large page text map,e tc.), perform the proper
    > flushing, and generally make sure we don't machine check on all platforms?
    >
    >

    Probably not yet. OTOH, it seems like x86 is the only platform today
    that tries to do something about the AGP page aliasing.
    > Jesse
    >

    /Thomas



    --
    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] x86: create array based interface to change page attribute

    On Monday, April 07, 2008 12:51 pm Thomas Hellström wrote:
    > > Hopefully the WC stuff will be upstream right after 2.6.25 comes out.
    > > Any reason why we shouldn't keep the pages mapped in the kernel as WC
    > > assuming the interface is there?

    >
    > If the pages are unmapped, we can get reasonable speed doing
    > unbind-read-bind operations, kernel accesses to the memory will need to
    > use an iomap_atomic_prot_pfn() type of operation.
    > No IPI global tlb flushes needed for kernel mapping changes during
    > unbind-read-bind and no cache flushes needed either if we write-protect
    > the user-space mappings properly, or very limited cache flushes if we
    > keep dirty-written-while-cached flags for each page.
    >
    > If the pages are wc-d we'll need two extra IPI global tlb flushes and a
    > buffer-size cache flush every time we do unbind-read-bind, but OTOH we
    > don't need the iomap_atomic_prot_pfn() to access single pages from the
    > kernel.


    Why would we need to flush at all at unbind-read-bind time? We should be able
    to leave pages in the WC state even when we unbind them, then when we need to
    bind them back into the GTT they'll be ready, but maybe I'm misunderstanding
    you here...

    > > Also, to make the API readable, we'd probably want to split the function
    > > into kernel_map_pages(..., enum memory_type type) and
    > > kernel_unmap_pages(...) (though like I said I think we really should be
    > > mapping them WC not umapping them altogether, since we do want to hit the
    > > ring buffer from the kernel with the WC type for example).

    >
    > I think ring-buffers are using ioremap() or vmap() already today. We can
    > use these to get WC-type access also in the future. The only time we use
    > the linear kernel mapping today is for single page access while patching
    > up command buffers.


    Yeah, they're ioremapped now, but that's a problem since with the PAT patches
    they'll be mapped hard UC (right now it just happens to work).

    Thanks,
    Jesse
    --
    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] x86: create array based interface to change page attribute

    Jesse Barnes wrote:
    > On Monday, April 07, 2008 12:51 pm Thomas Hellström wrote:
    >
    >>> Hopefully the WC stuff will be upstream right after 2.6.25 comes out.
    >>> Any reason why we shouldn't keep the pages mapped in the kernel as WC
    >>> assuming the interface is there?
    >>>

    >> If the pages are unmapped, we can get reasonable speed doing
    >> unbind-read-bind operations, kernel accesses to the memory will need to
    >> use an iomap_atomic_prot_pfn() type of operation.
    >> No IPI global tlb flushes needed for kernel mapping changes during
    >> unbind-read-bind and no cache flushes needed either if we write-protect
    >> the user-space mappings properly, or very limited cache flushes if we
    >> keep dirty-written-while-cached flags for each page.
    >>
    >> If the pages are wc-d we'll need two extra IPI global tlb flushes and a
    >> buffer-size cache flush every time we do unbind-read-bind, but OTOH we
    >> don't need the iomap_atomic_prot_pfn() to access single pages from the
    >> kernel.
    >>

    >
    > Why would we need to flush at all at unbind-read-bind time? We should be able
    > to leave pages in the WC state even when we unbind them, then when we need to
    > bind them back into the GTT they'll be ready, but maybe I'm misunderstanding
    > you here...
    >
    >

    We want to make the user-space mapping cache-coherent after unbind
    during read, to have any serious read-speed, and the linear kernel map
    has to follow, unless it's non-present. Even if it's non present, we
    need to flush whatever was written through the user-space mapping from
    the cache when rebinding. Having the user-space mapping read-only when
    possible will help avoid this.
    >>> Also, to make the API readable, we'd probably want to split the function
    >>> into kernel_map_pages(..., enum memory_type type) and
    >>> kernel_unmap_pages(...) (though like I said I think we really should be
    >>> mapping them WC not umapping them altogether, since we do want to hit the
    >>> ring buffer from the kernel with the WC type for example).
    >>>

    >> I think ring-buffers are using ioremap() or vmap() already today. We can
    >> use these to get WC-type access also in the future. The only time we use
    >> the linear kernel mapping today is for single page access while patching
    >> up command buffers.
    >>

    >
    > Yeah, they're ioremapped now, but that's a problem since with the PAT patches
    > they'll be mapped hard UC (right now it just happens to work).
    >

    Ouch, so we'll be needing an ioremap_wc(), I guess. We probably
    shouldn't use the linear kernel map for this anyway, since that would
    require a chipset flush for each ring commit. We can actually use
    vmap() with a wc page protection for that, but an ioremap_wc() would
    certainly save us a lot of trouble.
    > Thanks,
    > Jesse
    >

    /Thomas




    --
    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] x86: create array based interface to change page attribute

    Jesse Barnes wrote:
    >
    > Yeah, they're ioremapped now, but that's a problem since with the PAT patches
    > they'll be mapped hard UC (right now it just happens to work).
    >

    clearly this needs to use ioremap_wc(), I was assuming nothing else....
    (code mapping things uncached and then frobbing the MTRR's underneath really shouldn't
    be allowed to live from 2.6.26 and forward. Esp for the 3D drivers we KNOW that we
    want _wc so that is a clear case of just fixing the code to do the right thing)
    --
    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] x86: create array based interface to change page attribute

    Thomas Hellström wrote:
    > Ouch, so we'll be needing an ioremap_wc(), I guess. We probably
    > shouldn't use the linear kernel map for this anyway, since that would
    > require a chipset flush for each ring commit. We can actually use
    > vmap() with a wc page protection for that, but an ioremap_wc() would
    > certainly save us a lot of trouble.


    please don't use vmap() as a hack for mapping IO memory when you should use ioremap_wc()....
    Lets get this thing right rather than finding whichever hack of the day
    happens to work with the implementation from yesterday.
    --
    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] x86: create array based interface to change page attribute

    On Monday, April 07, 2008 1:46 pm Thomas Hellström wrote:
    > > Why would we need to flush at all at unbind-read-bind time? We should be
    > > able to leave pages in the WC state even when we unbind them, then when
    > > we need to bind them back into the GTT they'll be ready, but maybe I'm
    > > misunderstanding you here...

    >
    > We want to make the user-space mapping cache-coherent after unbind
    > during read, to have any serious read-speed, and the linear kernel map
    > has to follow, unless it's non-present. Even if it's non present, we
    > need to flush whatever was written through the user-space mapping from
    > the cache when rebinding. Having the user-space mapping read-only when
    > possible will help avoid this.


    Ah, you actually want to *read* from memory? Yeah that would be really slow
    if we left it UC or WC. But I thought that was really only necessary for
    relocation, and keithp dealt with that with the "presumed offset" stuff? Are
    you seeing other cases where we need to read back frequently?

    > > Yeah, they're ioremapped now, but that's a problem since with the PAT
    > > patches they'll be mapped hard UC (right now it just happens to work).

    >
    > Ouch, so we'll be needing an ioremap_wc(), I guess. We probably
    > shouldn't use the linear kernel map for this anyway, since that would
    > require a chipset flush for each ring commit. We can actually use
    > vmap() with a wc page protection for that, but an ioremap_wc() would
    > certainly save us a lot of trouble.


    Yeah, ioremap_wc is probably the best thing to use in the DRM. And in AGP
    we'll need to clarify things more, since some drivers can do cacheable
    memory, some want UC, etc.

    Thanks,
    Jesse
    --
    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/

  15. Re: [PATCH] x86: create array based interface to change page attribute

    On Monday, April 07, 2008 1:56 pm Arjan van de Ven wrote:
    > Jesse Barnes wrote:
    > > Yeah, they're ioremapped now, but that's a problem since with the PAT
    > > patches they'll be mapped hard UC (right now it just happens to work).

    >
    > clearly this needs to use ioremap_wc(), I was assuming nothing else....
    > (code mapping things uncached and then frobbing the MTRR's underneath
    > really shouldn't be allowed to live from 2.6.26 and forward. Esp for the 3D
    > drivers we KNOW that we want _wc so that is a clear case of just fixing the
    > code to do the right thing)


    Totally agreed, the AGP code needs to be specific about what it wants...

    Jesse
    --
    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/

  16. Re: [PATCH] x86: create array based interface to change page attribute

    On Monday, April 07, 2008 2:02 pm Jesse Barnes wrote:
    > On Monday, April 07, 2008 1:56 pm Arjan van de Ven wrote:
    > > Jesse Barnes wrote:
    > > > Yeah, they're ioremapped now, but that's a problem since with the PAT
    > > > patches they'll be mapped hard UC (right now it just happens to work).

    > >
    > > clearly this needs to use ioremap_wc(), I was assuming nothing else....
    > > (code mapping things uncached and then frobbing the MTRR's underneath
    > > really shouldn't be allowed to live from 2.6.26 and forward. Esp for the
    > > 3D drivers we KNOW that we want _wc so that is a clear case of just
    > > fixing the code to do the right thing)

    >
    > Totally agreed, the AGP code needs to be specific about what it wants...


    Err DRM code (but AGP does too).

    Jesse
    --
    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/

  17. Re: [PATCH] x86: create array based interface to change page attribute

    Arjan van de Ven wrote:
    > Thomas Hellström wrote:
    >> Ouch, so we'll be needing an ioremap_wc(), I guess. We probably
    >> shouldn't use the linear kernel map for this anyway, since that would
    >> require a chipset flush for each ring commit. We can actually use
    >> vmap() with a wc page protection for that, but an ioremap_wc() would
    >> certainly save us a lot of trouble.

    >
    > please don't use vmap() as a hack for mapping IO memory when you
    > should use ioremap_wc()....

    I think you've misinterpreted. We must use vmap() if we need to map the
    cpu side of the aperture, (real pages) and ioremap() if we map the
    device side, which is io memory. Some devices accept both, but what I
    was saying was we should try to avoid the vmap() alternative, since it,
    like using the kernel mapping directly, requires a chipset flush after
    update.
    > Lets get this thing right rather than finding whichever hack of the day
    > happens to work with the implementation from yesterday.

    /Thomas



    --
    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/

  18. Re: [PATCH] x86: create array based interface to change page attribute

    Jesse Barnes wrote:
    > On Monday, April 07, 2008 1:46 pm Thomas Hellström wrote:
    >
    >>> Why would we need to flush at all at unbind-read-bind time? We should be
    >>> able to leave pages in the WC state even when we unbind them, then when
    >>> we need to bind them back into the GTT they'll be ready, but maybe I'm
    >>> misunderstanding you here...
    >>>

    >> We want to make the user-space mapping cache-coherent after unbind
    >> during read, to have any serious read-speed, and the linear kernel map
    >> has to follow, unless it's non-present. Even if it's non present, we
    >> need to flush whatever was written through the user-space mapping from
    >> the cache when rebinding. Having the user-space mapping read-only when
    >> possible will help avoid this.
    >>

    >
    > Ah, you actually want to *read* from memory? Yeah that would be really slow
    > if we left it UC or WC. But I thought that was really only necessary for
    > relocation, and keithp dealt with that with the "presumed offset" stuff? Are
    > you seeing other cases where we need to read back frequently?
    >

    While keithp's "presumed offset" is really good stuff, we should never
    need to read from device memory during relocations, I think. I was
    thinking of EXA and OpenGL software fallback cases, "download from
    screen" and "readpixels" types of operation, as well as
    pixel-buffer-object map in read mode.

    /Thomas



    --
    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/

  19. Re: [PATCH] x86: create array based interface to change page attribute

    On Monday, April 07, 2008 11:21 pm Thomas Hellström wrote:
    > Jesse Barnes wrote:
    > > On Monday, April 07, 2008 1:46 pm Thomas Hellström wrote:
    > >>> Why would we need to flush at all at unbind-read-bind time? We should
    > >>> be able to leave pages in the WC state even when we unbind them, then
    > >>> when we need to bind them back into the GTT they'll be ready, but maybe
    > >>> I'm misunderstanding you here...
    > >>
    > >> We want to make the user-space mapping cache-coherent after unbind
    > >> during read, to have any serious read-speed, and the linear kernel map
    > >> has to follow, unless it's non-present. Even if it's non present, we
    > >> need to flush whatever was written through the user-space mapping from
    > >> the cache when rebinding. Having the user-space mapping read-only when
    > >> possible will help avoid this.

    > >
    > > Ah, you actually want to *read* from memory? Yeah that would be really
    > > slow if we left it UC or WC. But I thought that was really only
    > > necessary for relocation, and keithp dealt with that with the "presumed
    > > offset" stuff? Are you seeing other cases where we need to read back
    > > frequently?

    >
    > While keithp's "presumed offset" is really good stuff, we should never
    > need to read from device memory during relocations, I think. I was
    > thinking of EXA and OpenGL software fallback cases, "download from
    > screen" and "readpixels" types of operation, as well as
    > pixel-buffer-object map in read mode.


    Hm, yeah, anholt was telling me about some of these cases offline too. This
    is what makes things really difficult...

    Jesse
    --
    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
Page 2 of 2 FirstFirst 1 2