x86: create array based interface to change page attribute
PATCH 1/1 x86: create array based interface to change page attribute
When cpa was refactored to the new set_memory_ interfaces, it removed a special case fast path for AGP systems, which did a lot of page by page attribute changing and held the flushes until they were finished. The DRM memory manager also required this to get useable speeds.
This introduces a new interface, which accepts an array of memory addresses to have attributes changed on and to flush once finished.
Further changes to the AGP stack to actually use this interface will be published later.
--- arch/x86/mm/pageattr-test.c | 12 ++- arch/x86/mm/pageattr.c | 164 +++++++++++++++++++++++++++++++----------- include/asm-x86/cacheflush.h | 3 + 3 files changed, 132 insertions(+), 47 deletions(-)
arch/x86/mm/pageattr-test.c +8 -4
@@ -111,6 +111,7 @@ static int pageattr_test(void)unsigned int level;int i, k;int err; +unsigned long test_addr;if (print)printk (KERN_INFO "CPA self-test:\n" );
@@ -165,8 +166,10 @@ static int pageattr_test(void)continue ; } - err =change_page_attr_clear (addr[i], len[i], -__pgprot (_PAGE_GLOBAL)); + + test_addr = addr[i]; + err =change_page_attr_clear (&test_addr, len[i], +__pgprot (_PAGE_GLOBAL),0 );if (err <0 ) {printk (KERN_ERR "CPA %d failed %d\n" , i, err); failed++;
@@ -198,8 +201,9 @@ static int pageattr_test(void) failed++;continue ; } - err =change_page_attr_set (addr[i], len[i], -__pgprot (_PAGE_GLOBAL)); + test_addr = addr[i]; + err =change_page_attr_set (&test_addr, len[i], +__pgprot (_PAGE_GLOBAL),0 );if (err <0 ) {printk (KERN_ERR "CPA reverting failed: %d\n" , err); failed++;
arch/x86/mm/pageattr.c +121 -43
337 lines not on this page · raw message
include/asm-x86/cacheflush.h +3 -0
@@ -42,6 +42,9 @@ int set_memory_ro(unsigned long addr, int numpages);int set_memory_rw (unsigned long addr,int numpages);int set_memory_np (unsigned long addr,int numpages); +int set_memory_array_uc (unsigned long *addr,int addrinarray); +int set_memory_array_wb (unsigned long *addr,int addrinarray); +void clflush_cache_range (void *addr,unsigned int size);void cpa_init (void );
Quoted 11 lines · Dave Airlie
When cpa was refactored to the new
set_memory_interfaces, it removed a special case fast path for AGP systems, which did a lot of page by page attribute changing and held the flushes until they were finished. The DRM memory manager also required this to get useable speeds.This introduces a new interface, which accepts an array of memory addresses to have attributes changed on and to flush once finished.
Further changes to the AGP stack to actually use this interface will be published later.
Quoted 7 lines
Signed-off-by Dave Airlie <[email protected]>--- arch/x86/mm/pageattr-test.c | 12 ++- arch/x86/mm/pageattr.c | 164 +++++++++++++++++++++++++++++++----------- include/asm-x86/cacheflush.h | 3 + 3 files changed, 132 insertions(+), 47 deletions(-)
...
Dave,
Nice work, but how do we handle highmem pages? I know that they don't need any attribute change since they're not in the kernel map, but both the AGP module and the DRM memory manager typically hold highmem addresses in their arrays, so the code should presumably detect those and avoid them?
Since this is an AGPgart and DRM fastpath, the interface should ideally be adapted to match the data structures used by those callers. The AGPgart module uses an array of addresses, which effectively stops it from using pages beyond the DMA32 limit. The DRM memory manager uses an array of struct page pointers, but using that was, If I understand things correctly, rejected.
So, if we, at some point, want to have an AGPgart module capable of using anything beyond the DMA32 limit we will end up with an interface that doesn't match neither AGPgart nor DRM, for which users the fastpath was originally intended.
/Thomas
Quoted 20 lines · Thomas Hellström
Quoted 11 lines · Dave Airlie
When cpa was refactored to the new
set_memory_interfaces, it removed a special case fast path for AGP systems, which did a lot of page by page attribute changing and held the flushes until they were finished. The DRM memory manager also required this to get useable speeds.This introduces a new interface, which accepts an array of memory addresses to have attributes changed on and to flush once finished.
Further changes to the AGP stack to actually use this interface will be published later.
Quoted 8 lines
Signed-off-by Dave Airlie <[email protected]>--- arch/x86/mm/pageattr-test.c | 12 ++- arch/x86/mm/pageattr.c | 164 +++++++++++++++++++++++++++++++----------- include/asm-x86/cacheflush.h | 3 + 3 files changed, 132 insertions(+), 47 deletions(-)
Quoted 3 lines
...
Dave,
Nice work, but how do we handle highmem pages?
Cache attributes fundamentally work on a mapping and not on physical memory.
(MTRR's are special there, they do work on physical memory, but that's a special case and not relevant here)
So to be honest, your question doesn't make sense; because all I can do is ask "which mapping of these pages".
Even the old interfaces prior to 2.6.24 only managed to deal with SOME of the mappings of a page. And if/when future CPUs don't require all mappings to be in sync, clearly the kernel will only change the mapping that is requested as well.
Quoted 1 line
Since this is an AGPgart and DRM fastpath, the interface should ideally be adapted to match the data structures used by those callers.
Actually, the interface has to make sense conceptually convey the right information, the implementation should not have to second guess internals of AGP/DRM because that would just be a recipe for disaster.
Quoted 1 line
The AGPgart module uses an array of addresses, which effectively stops it from using pages beyond the DMA32 limit. The DRM memory manager uses an array of struct page pointers, but using that was, If I understand things correctly, rejected.
yes because simply put, if you pass a struct page to such a function, you're not telling it which mapping or mappings you want changed....
(And if you say "only the 1:1 mapping, so why doesn't the other side calculate that"... there's no speed gain in doing the calculation for that on the other side of an interface, and that makes it zero reason to misdesign the interface to only have the "which mapping" information implicit)
Quoted 41 lines · Arjan van de Ven
Quoted 20 lines · Thomas Hellström
Quoted 11 lines · Dave Airlie
When cpa was refactored to the new
set_memory_interfaces, it removed a special case fast path for AGP systems, which did a lot of page by page attribute changing and held the flushes until they were finished. The DRM memory manager also required this to get useable speeds.This introduces a new interface, which accepts an array of memory addresses to have attributes changed on and to flush once finished.
Further changes to the AGP stack to actually use this interface will be published later.
Quoted 8 lines
Signed-off-by Dave Airlie <[email protected]>--- arch/x86/mm/pageattr-test.c | 12 ++- arch/x86/mm/pageattr.c | 164 +++++++++++++++++++++++++++++++----------- include/asm-x86/cacheflush.h | 3 + 3 files changed, 132 insertions(+), 47 deletions(-)Quoted 3 lines
...
Dave,
Nice work, but how do we handle highmem pages?Cache attributes fundamentally work on a mapping and not on physical memory.
(MTRR's are special there, they do work on physical memory, but that's a special case and not relevant here)So to be honest, your question doesn't make sense; because all I can do is ask "which mapping of these pages".
Even the old interfaces prior to 2.6.24 only managed to deal with SOME of the mappings of a page. And if/when future CPUs don't require all mappings to be in sync, clearly the kernel will only change the mapping that is requested as well.
Quoted 1 line
Since this is an AGPgart and DRM fastpath, the interface should ideally be adapted to match the data structures used by those callers.
Actually, the interface has to make sense conceptually convey the right information, the implementation should not have to second guess internals of AGP/DRM because that would just be a recipe for disaster.
Quoted 8 lines
Quoted 1 line
The AGPgart module uses an array of addresses, which effectively stops it from using pages beyond the DMA32 limit. The DRM memory manager uses an array of struct page pointers, but using that was, If I understand things correctly, rejected.
yes because simply put, if you pass a struct page to such a function, you're not telling it which mapping or mappings you want changed....
(And if you say "only the 1:1 mapping, so why doesn't the other side calculate that"... there's no speed gain in doing the calculation for that on the other side of an interface, and that makes it zero reason to misdesign the interface to only have the "which mapping" information implicit)
Hmm. I get the point. There should be ways to do reasonably efficient workarounds in the drivers.
/Thomas
arch/x86/mm/pageattr.c line 35
Quoted 3 lines · Dave Airlie
arch/x86/mm/pageattr.c +2
-0+#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.
arch/x86/mm/pageattr.c static void cpa_flush_range(unsigned long start, int numpages, int cache) · line 177
Quoted 6 lines
arch/x86/mm/pageattr.c +6
-0+ +/* +* Only flush present addresses: +*/ +if (pte && (pte_val (*pte) & _PAGE_PRESENT)) +clflush_cache_range ((void *) *addr,PAGE_SIZE );
Also it is doubtful clflush really makes sense on a large array. Just doing wbinvd might be faster then. Or perhaps better supporting Self-Snoop should be revisited, that would at least eliminate it on most Intel CPUs.
-Andi
Quoted 11 lines · Andi Kleen
arch/x86/mm/pageattr.c line 35
Quoted 4 lines · Dave Airlie
arch/x86/mm/pageattr.c +2
-0+#define CPA_FLUSHTLB 1 +#define CPA_ARRAY 2I don't think
CPA_ARRAYshould 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.
Quoted 13 lines
arch/x86/mm/pageattr.c static void cpa_flush_range(unsigned long start, int numpages, int cache) · line 177
Quoted 7 lines
arch/x86/mm/pageattr.c +6
-0+ +/* +* Only flush present addresses: +*/ +if (pte && (pte_val (*pte) & _PAGE_PRESENT)) +clflush_cache_range ((void *) *addr,PAGE_SIZE );Also it is doubtful clflush really makes sense on a large array. Just doing wbinvd might be faster then. Or perhaps better supporting Self-Snoop should be revisited, that would at least eliminate it on most Intel CPUs.
I agree that wbinvd() seems to be faster on large arrays on the processors I've tested. But isn't there a severe latency problem with that instruction, that makes people really want to avoid it in all possible cases?
Also I think we need to clarify the semantics of the c_p_a functionality. Right now both AGP and DRM relies on c_p_a doing an explicit cache flush. Otherwise the data won't appear on the device side of the aperture. If we use self-snoop, the AGP and DRM drivers can't rely on this flush being performed, and they have to do the flush themselves, and for non-self-snooping processors, the flush needs to be done twice?
/Thomas
Quoted 2 lines
-Andi
Quoted 4 lines
Also I think we need to clarify the semantics of the
c_p_afunctionality. Right now both AGP and DRM relies onc_p_adoing an explicit cache flush. Otherwise the data won't appear on the device side of the aperture.
But surely not in cpa I hope ? Or are you saying you first write data and then do cpa? If true that would be quite an abuse of CPA I would say and you should fix it ASAP.
Quoted 2 lines
If we use self-snoop, the AGP and DRM drivers can't rely on this flush being performed, and they have to do the flush themselves, and for
They definitely should flush themselves if they want data to reach the device. That is obviously required any time they reuse a page too.
-Andi
Quoted 7 lines · Andi Kleen
Quoted 2 lines
Also I think we need to clarify the semantics of the
c_p_afunctionality. Right now both AGP and DRM relies onc_p_adoing an explicit cache flush. Otherwise the data won't appear on the device side of the aperture.But surely not in cpa I hope ? Or are you saying you first write data
and then do cpa? If true that would be quite an abuse of CPA I would say and you should fix it ASAP.
As AGP buffers are moved in- and out of AGP, the caching policy changes, so yes, there may be writes to cache coherent memory that needs to be flushed at some point. Since CPA has been doing that up to now, and the codepaths involved are quite time-critical, a double cache-flush is a no-no, so if this is left to the caller, we must be able to tell CPA that any needed cache-flush has already been performed.
Quoted 7 lines
Quoted 2 lines
If we use self-snoop, the AGP and DRM drivers can't rely on this flush being performed, and they have to do the flush themselves, and for
They definitely should flush themselves if they want data to reach the device. That is obviously required any time they reuse a page too.
Understood,
but then we *must* really find a way to say "don't flush the cache again", perhaps part of Dave's array function?
/Thomas
Quoted 2 lines
-Andi
Quoted 12 lines · Thomas Hellström
Quoted 11 lines · Andi Kleen
Quoted 5 lines
Also I think we need to clarify the semantics of the
c_p_afunctionality. Right now both AGP and DRM relies onc_p_adoing an explicit cache flush. Otherwise the data won't appear on the device side of the aperture.But surely not in cpa I hope ? Or are you saying you first write data and then do cpa? If true that would be quite an abuse of CPA I would say and you should fix it ASAP.
Quoted 4 lines
As AGP buffers are moved in- and out of AGP, the caching policy changes, so yes, there may be writes to cache coherent memory that needs to be flushed at some point. Since CPA has been doing that up to now, and the codepaths involved are quite time-critical, a double cache-flush is a
That doesn't make sense. You shouldn't be using CPA in any time critical code path. It will always be slow.
For anything time critical you should keep a pool of uncached pages once set up using CPA and reuse them.
CPA really should only be used on initialization or for larger setup changes which are ok to go somewhat slower.
Quoted 2 lines
no-no, so if this is left to the caller, we must be able to tell CPA that any needed cache-flush has already been performed.
Sounds like a bad design.
Quoted 11 lines
Quoted 8 lines
Quoted 3 lines
If we use self-snoop, the AGP and DRM drivers can't rely on this flush being performed, and they have to do the flush themselves, and for
They definitely should flush themselves if they want data to reach the device. That is obviously required any time they reuse a page too.
Understood,
but then we *must* really find a way to say "don't flush the cache again", perhaps part of Dave's array function?
The cache must be flushed in CPA, there is no way around it.
If you write data into the buffers before you do CPA on them you could avoid it, but I don't think you should do that anywhere near time critical code, so it actually shouldn't matter.
-Andi
Quoted 15 lines · Andi Kleen
Quoted 5 lines · Thomas Hellström
Quoted 3 lines · Andi Kleen
Quoted 3 lines
Also I think we need to clarify the semantics of the
c_p_afunctionality. Right now both AGP and DRM relies onc_p_adoing an explicit cache flush. Otherwise the data won't appear on the device side of the aperture.Quoted 7 lines
Quoted 5 lines
But surely not in cpa I hope ? Or are you saying you first write data
and then do cpa? If true that would be quite an abuse of CPA I would say and you should fix it ASAP.As AGP buffers are moved in- and out of AGP, the caching policy changes, so yes, there may be writes to cache coherent memory that needs to be flushed at some point. Since CPA has been doing that up to now, and the codepaths involved are quite time-critical, a double cache-flush is a
Quoted 7 lines
That doesn't make sense. You shouldn't be using CPA in any time critical code path. It will always be slow.
For anything time critical you should keep a pool of uncached pages once set up using CPA and reuse them.
CPA really should only be used on initialization or for larger setup changes which are ok to go somewhat slower.
Let me rehprase. Not really time-critical but it is of some importance that CPA is done quickly.
We're dealing with the tradeoff of reading from uncached device memory vs taking the pages out of
AGP, setting up a cache-coherent mapping, read and then change back. What we'd really would like to set up is a pool of completely unmapped (like highmem) pages. Then we could, to a large extent, avoid the CPA calls.
Quoted 10 lines
The cache must be flushed in CPA, there is no way around it.
If you write data into the buffers before you do CPA on them you could avoid it, but I don't think you should do that anywhere near time critical code, so it actually shouldn't matter.
-Andi
But then we wouldn't really be discussing SS either? For DRM purposes, the more performance we can squeeze out of CPA, the better.
/Thomas
Quoted 2 lines · Thomas Hellström
Let me rehprase. Not really time-critical but it is of some importance that CPA is done quickly. We're dealing with the tradeoff of reading from uncached device memory
uncached or write combining ?
Quoted 2 lines
vs taking the pages out of
AGP, setting up a cache-coherent mapping, read and then change back. What we'd really would like to set up is a pool of completely unmapped (like highmem) pages. Then we could, to a large extent, avoid the CPA calls.
changing attributes by nature means a tlb flush and a bunch of expensive cache work. That's never going to be cheap, I guess it all depends on how much work you do on the memory for it to pay off or not...
Quoted 6 lines · Arjan van de Ven
Quoted 2 lines · Thomas Hellström
Let me rehprase. Not really time-critical but it is of some importance that CPA is done quickly. We're dealing with the tradeoff of reading from uncached device memory
uncached or write combining ?
The user-space mappings (the ones that we really use) are usually write-combined, whereas the kernel mappings are uncached. (I think this is OK since both mapping types implies no cache coherency). Even if (IIRC) write combining is theoretically prefetchable, some devices give read speeds around 9MB/s.
Quoted 7 lines
Quoted 3 lines
vs taking the pages out of
AGP, setting up a cache-coherent mapping, read and then change back. What we'd really would like to set up is a pool of completely unmapped (like highmem) pages. Then we could, to a large extent, avoid the CPA calls.changing attributes by nature means a tlb flush and a bunch of expensive cache work. That's never going to be cheap, I guess it all depends on how much work you do on the memory for it to pay off or not...
Indeed. Actually with the new non-wbinvd() CPA, We seem to benefit already if the buffer is a single page, though it's probably hard to measure the impact of repopulating the tlb.
/Thomas
Quoted 8 lines · Thomas Hellström
Quoted 6 lines · Arjan van de Ven
Quoted 2 lines · Thomas Hellström
Let me rehprase. Not really time-critical but it is of some importance that CPA is done quickly. We're dealing with the tradeoff of reading from uncached device memory
uncached or write combining ?
The user-space mappings (the ones that we really use) are usually write-combined, whereas the kernel mappings are uncached. (I think this is OK since both mapping types implies no cache coherency).
This is not officially allowed and may tripple fault your cpu.. To comply with the spec one needs to have ALL mappings the same unfortunately. (And yes, this is a hard problem)
Quoted 14 lines
Even if (IIRC) write combining is theoretically prefetchable, some devices give read speeds around 9MB/s.
Quoted 7 lines
Quoted 3 lines
vs taking the pages out of
AGP, setting up a cache-coherent mapping, read and then change back. What we'd really would like to set up is a pool of completely unmapped (like highmem) pages. Then we could, to a large extent, avoid the CPA calls.changing attributes by nature means a tlb flush and a bunch of expensive cache work. That's never going to be cheap, I guess it all depends on how much work you do on the memory for it to pay off or not...
Indeed. Actually with the new non-wbinvd() CPA, We seem to benefit already if the buffer is a single page, though it's probably hard to measure the impact of repopulating the tlb.
/Thomas
Quoted 7 lines · Arjan van de Ven
Quoted 6 lines · Thomas Hellström
Quoted 5 lines · Arjan van de Ven
Quoted 2 lines · Thomas Hellström
Let me rehprase. Not really time-critical but it is of some importance that CPA is done quickly. We're dealing with the tradeoff of reading from uncached device memory
Quoted 7 lines
Quoted 2 lines
Quoted 1 line
uncached or write combining ?
The user-space mappings (the ones that we really use) are usually write-combined, whereas the kernel mappings are uncached. (I think this is OK since both mapping types implies no cache coherency).
This is not officially allowed and may tripple fault your cpu.. To comply with the spec one needs to have ALL mappings the same unfortunately. (And yes, this is a hard problem)
Hmm...
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). Then we can set up a pool of unmapped pages, avoid frequent use of CPA and everybody's happy.
int set_memory_array_np(unsigned long *addr, int addrinarray) {
return change_page_attr_clr(addr, addrinarray,
__pgprot(_PAGE_PRESENT | _PAGE_RW), 1); }
EXPORT_SYMBOL(set_memory_array_np);
int set_memory_array_p(unsigned long *addr, int addrinarray) {
return change_page_attr_set(addr, addrinarray,
__pgprot(_PAGE_PRESENT | _PAGE_RW), 1); }
EXPORT_SYMBOL(set_memory_array_p);
/Thomas
Quoted 2 lines · Thomas Hellström
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.
Quoted 9 lines · Arjan van de Ven
Quoted 2 lines · Thomas Hellström
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?
Or are you saying that it's very hard to keep track of all mappings to a page, and change only the ones you really want to change?
If you mean the latter, for the DRM case, the DRM memory manager has already made sure all user-space mappings of a page are killed before calling CPA on it, (using unmap_mapping_range()) and they are faulted back once CPA is done.
I was under the impression that calling CPA on the kernel mapping of that page would do the rest?
/Thomas
Quoted 11 lines · Thomas Hellström
Quoted 9 lines · Arjan van de Ven
Quoted 2 lines · Thomas Hellström
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.
Quoted 2 lines
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 ;)
Quoted 18 lines · Arjan van de Ven
Quoted 11 lines · Thomas Hellström
Quoted 9 lines · Arjan van de Ven
Quoted 2 lines · Thomas Hellström
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.Quoted 1 line
Quoted 8 lines
Quoted 1 line
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
Quoted 1 line · Thomas Hellström
But what mappings are there, immediately after
alloc_page(), thatset_memory_npwon'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)
Quoted 1 line
Drivers relying on
set_memory_uctouching 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.
Quoted 12 lines · Arjan van de Ven
Quoted 1 line · Thomas Hellström
But what mappings are there, immediately after
alloc_page(), thatset_memory_npwon'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 youralloc_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)
Quoted 6 lines
Quoted 1 line
Drivers relying on
set_memory_uctouching 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
Quoted 15 lines · Thomas Hellström
Quoted 14 lines · Arjan van de Ven
Quoted 1 line · Thomas Hellström
But what mappings are there, immediately after
alloc_page(), thatset_memory_npwon'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 youralloc_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)Quoted 1 line
Drivers relying on
set_memory_uctouching 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.
Quoted 7 lines
Quoted 4 lines
"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 inx86/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.
Quoted 1 line
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.
Quoted 1 line
don't hit any other problems, like
clflush()refusing to run on an NP page?
yes clflush doesn't work on not present pages ;)
Quoted 6 lines · Arjan van de Ven
Quoted 1 line · Thomas Hellström
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
Quoted 6 lines · Thomas Hellström
Quoted 4 lines · Arjan van de Ven
Quoted 1 line · Thomas Hellström
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.
Quoted 8 lines
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...
Quoted 4 lines
3) Have code in
x86/pageattr.cdecide which "default" mappings are present on the given pages and set them all as non-present. In fact, there is already such a function inpageattr.c:
Quoted 8 lines
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
Quoted 14 lines · Jesse Barnes
Quoted 12 lines · Thomas Hellström
Quoted 6 lines · Arjan van de Ven
Quoted 2 lines · Thomas Hellström
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.
Quoted 8 lines
Quoted 5 lines
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.
Quoted 3 lines
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.
Quoted 9 lines
Quoted 9 lines
3) Have code in
x86/pageattr.cdecide which "default" mappings are present on the given pages and set them all as non-present. In fact, there is already such a function inpageattr.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?
Quoted 9 lines
Quoted 4 lines
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)andkernel_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.
Quoted 3 lines
Question is, will
kernel_map_pagescatch 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.
Quoted 2 lines
Jesse
/Thomas
Quoted 12 lines · Thomas Hellström
Quoted 3 lines
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.
Quoted 4 lines
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...
Quoted 10 lines
Quoted 5 lines
Also, to make the API readable, we'd probably want to split the function into
kernel_map_pages(..., enum memory_type type)andkernel_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()orvmap()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
Quoted 13 lines · Jesse Barnes
Quoted 11 lines · Thomas Hellström
Quoted 3 lines
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.
Quoted 9 lines
Quoted 5 lines
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.
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Also, to make the API readable, we'd probably want to split the function into
kernel_map_pages(..., enum memory_type type)andkernel_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).
Quoted 8 lines
Quoted 5 lines
I think ring-buffers are using
ioremap()orvmap()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.
Quoted 3 lines
Thanks,
Jesse
/Thomas
Quoted 1 line · Thomas Hellström
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 usevmap()with a wc page protection for that, but anioremap_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.
Quoted 4 lines · Arjan van de Ven
Quoted 1 line · Thomas Hellström
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 usevmap()with a wc page protection for that, but anioremap_wc()would certainly save us a lot of trouble.please don't use
vmap()as a hack for mapping IO memory when you should useioremap_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.
Quoted 2 lines
Lets get this thing right rather than finding whichever hack of the day happens to work with the implementation from yesterday.
/Thomas
Quoted 11 lines · Thomas Hellström
Quoted 4 lines
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?
Quoted 8 lines
Quoted 2 lines
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 usevmap()with a wc page protection for that, but anioremap_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
Quoted 15 lines · Jesse Barnes
Quoted 5 lines · Thomas Hellström
Quoted 5 lines
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...
Quoted 7 lines
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.
Quoted 2 lines
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
Quoted 20 lines · Thomas Hellström
Quoted 13 lines · Jesse Barnes
Quoted 11 lines · Thomas Hellström
Quoted 4 lines
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.
Quoted 5 lines
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?
Quoted 5 lines
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
Quoted 3 lines · Jesse Barnes
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)
Quoted 9 lines · Arjan van de Ven
Quoted 2 lines · Jesse Barnes
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_wcso 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
Quoted 5 lines · Jesse Barnes
Quoted 4 lines · Arjan van de Ven
Quoted 2 lines · Jesse Barnes
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).
Quoted 7 lines
Quoted 5 lines
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_wcso 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
Quoted 2 lines · Andi Kleen
Also it is doubtful clflush really makes sense on a large array. Just doing wbinvd might be faster then.
wbinvd is rarely a good idea; think about it... it'll flush 12Mb of cache *per socket* in one instruction. (on a modern Intel consumer grade CPU, more on the enterprise ones)
This doesn't only impact the current logical thread, but ALL of the threads in the system, since the cache coherency needs to be preserved, all have to go empty at the same time.
Forget real time... this can take really long even for non-realtime users ;)
At least clflush breaks this up into smaller pieces so total latency won't suck entirely.
Quoted 8 lines · Andi Kleen
arch/x86/mm/pageattr.c line 35
Quoted 3 lines · Dave Airlie
arch/x86/mm/pageattr.c +2
-0+#define CPA_FLUSHTLB 1 +#define CPA_ARRAY 2I don't think
CPA_ARRAYshould 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..
Dave.
Quoted 10 lines · Dave Airlie
Quoted 8 lines · Andi Kleen
arch/x86/mm/pageattr.c line 35
Quoted 3 lines · Dave Airlie
arch/x86/mm/pageattr.c +2
-0+#define CPA_FLUSHTLB 1 +#define CPA_ARRAY 2I don't think
CPA_ARRAYshould 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.
Quoted 6 lines
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.
-Andi
Quoted 11 lines · Andi Kleen
Quoted 10 lines · Dave Airlie
Quoted 8 lines · Andi Kleen
arch/x86/mm/pageattr.c line 35
Quoted 3 lines · Dave Airlie
arch/x86/mm/pageattr.c +2
-0+#define CPA_FLUSHTLB 1 +#define CPA_ARRAY 2I don't think
CPA_ARRAYshould 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.
Quoted 8 lines
Quoted 6 lines
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.
Quoted 10 lines · Dave Airlie
When cpa was refactored to the new
set_memory_interfaces, it removed a special case fast path for AGP systems, which did a lot of page by page attribute changing and held the flushes until they were finished. The DRM memory manager also required this to get useable speeds.This introduces a new interface, which accepts an array of memory addresses to have attributes changed on and to flush once finished.
Further changes to the AGP stack to actually use this interface will be published later.