From: Michael Kelley <mhkli...@outlook.com>
Current defio code works only for framebuffer memory that is allocated
with vmalloc(). The code assumes that the underlying page refcount can
be used by the mm subsystem to manage each framebuffer page's lifecycle,
including freeing the page if the refcount goes to 0. This approach is
consistent with vmalloc'ed memory, but not with contiguous kernel memory
allocated via alloc_pages() or similar. The latter such memory pages
usually have a refcount of 0 when allocated, and would be incorrectly
freed page-by-page if used with defio. That free'ing corrupts the memory
free lists and Linux eventually panics. Simply bumping the refcount after
allocation doesn’t work because when the framebuffer memory is freed,
__free_pages() complains about non-zero refcounts.
Commit 37b4837959cb ("video: deferred io with physically contiguous
memory") from the year 2008 purported to add support for contiguous
kernel memory framebuffers. The motivating device, sh_mobile_lcdcfb, uses
dma_alloc_coherent() to allocate framebuffer memory, which is likely to
use alloc_pages(). It's unclear to me how this commit actually worked at
the time, unless dma_alloc_coherent() was pulling from a CMA pool instead
of alloc_pages(). Or perhaps alloc_pages() worked differently or on the
arm32 architecture on which sh_mobile_lcdcfb is used.
In any case, for x86 and arm64 today, commit 37b4837959cb9 is not
sufficient to support contiguous kernel memory framebuffers. The problem
can be seen with the hyperv_fb driver, which may allocate the framebuffer
memory using vmalloc() or alloc_pages(), depending on the configuration
of the Hyper-V guest VM (Gen 1 vs. Gen 2) and the size of the framebuffer.
Fix this limitation by adding defio support for contiguous kernel memory
framebuffers. A driver with a framebuffer allocated from contiguous
kernel memory must set the FBINFO_KMEMFB flag to indicate such.
Tested with the hyperv_fb driver in both configurations -- with a vmalloc()
framebuffer and with an alloc_pages() framebuffer on x86. Also verified a
vmalloc() framebuffer on arm64. Hardware is not available to me to verify
that the older arm32 devices still work correctly, but the path for
vmalloc() framebuffers is essentially unchanged.
Even with these changes, defio does not support framebuffers in MMIO
space, as defio code depends on framebuffer memory pages having
corresponding 'struct page's.
Fixes: 3a6fb6c4255c ("video: hyperv: hyperv_fb: Use physical memory for fb on HyperV
Gen 1 VMs.")
Signed-off-by: Michael Kelley <mhkli...@outlook.com>
---
Changes in v3:
* Moved definition of FBINFO_KMEMFB flag to a separate patch
preceeding this one in the patch set [Helge Deller]
Changes in v2:
* Tweaked code comments regarding framebuffers allocated with
dma_alloc_coherent() [Christoph Hellwig]
drivers/video/fbdev/core/fb_defio.c | 128 +++++++++++++++++++++++-----
1 file changed, 108 insertions(+), 20 deletions(-)
diff --git a/drivers/video/fbdev/core/fb_defio.c
b/drivers/video/fbdev/core/fb_defio.c
index 4fc93f253e06..f8ae91a1c4df 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -8,11 +8,40 @@
* for more details.
*/
+/*
+ * Deferred I/O ("defio") allows framebuffers that are mmap()'ed to user space
+ * to batch user space writes into periodic updates to the underlying
+ * framebuffer hardware or other implementation (such as with a virtualized
+ * framebuffer in a VM). At each batch interval, a callback is invoked in the
+ * framebuffer's kernel driver, and the callback is supplied with a list of
+ * pages that have been modified in the preceding interval. The callback can
+ * use this information to update the framebuffer hardware as necessary. The
+ * batching can improve performance and reduce the overhead of updating the
+ * hardware.
+ *
+ * Defio is supported on framebuffers allocated using vmalloc() and allocated
+ * as contiguous kernel memory using alloc_pages() or kmalloc(). These
+ * memory allocations all have corresponding "struct page"s. Framebuffers
+ * allocated using dma_alloc_coherent() should not be used with defio.
+ * Such allocations should be treated as a black box owned by the DMA
+ * layer, and should not be deconstructed into individual pages as defio
+ * does. Framebuffers in MMIO space are *not* supported because MMIO space
+ * does not have corrresponding "struct page"s.
+ *
+ * For framebuffers allocated using vmalloc(), struct fb_info must have
+ * "screen_buffer" set to the vmalloc address of the framebuffer. For
+ * framebuffers allocated from contiguous kernel memory, FBINFO_KMEMFB must
+ * be set, and "fix.smem_start" must be set to the physical address of the
+ * frame buffer. In both cases, "fix.smem_len" must be set to the framebuffer
+ * size in bytes.
+ */
+
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/errno.h>
#include <linux/string.h>
#include <linux/mm.h>
+#include <linux/pfn_t.h>
#include <linux/vmalloc.h>
#include <linux/delay.h>
#include <linux/interrupt.h>
@@ -37,7 +66,7 @@ static struct page *fb_deferred_io_get_page(struct fb_info
*info, unsigned long
else if (info->fix.smem_start)
page = pfn_to_page((info->fix.smem_start + offs) >> PAGE_SHIFT);
- if (page)
+ if (page && !(info->flags & FBINFO_KMEMFB))
get_page(page);
return page;
@@ -137,6 +166,15 @@ static vm_fault_t fb_deferred_io_fault(struct vm_fault
*vmf)
BUG_ON(!info->fbdefio->mapping);
+ if (info->flags & FBINFO_KMEMFB)
+ /*
+ * In this path, the VMA is marked VM_PFNMAP, so mm assumes
+ * there is no struct page associated with the page. The
+ * PFN must be directly inserted and the created PTE will be
+ * marked "special".
+ */
+ return vmf_insert_pfn(vmf->vma, vmf->address,
page_to_pfn(page));
+
vmf->page = page;
return 0;
}
@@ -163,13 +201,14 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_fsync);
/*
* Adds a page to the dirty list. Call this from struct
- * vm_operations_struct.page_mkwrite.
+ * vm_operations_struct.page_mkwrite or .pfn_mkwrite.
*/
-static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned
long offset,
+static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, struct
vm_fault *vmf,
struct page *page)
{
struct fb_deferred_io *fbdefio = info->fbdefio;
struct fb_deferred_io_pageref *pageref;
+ unsigned long offset = vmf->pgoff << PAGE_SHIFT;
vm_fault_t ret;
/* protect against the workqueue changing the page list */
@@ -182,20 +221,34 @@ static vm_fault_t fb_deferred_io_track_page(struct
fb_info *info, unsigned long
}
/*
- * We want the page to remain locked from ->page_mkwrite until
- * the PTE is marked dirty to avoid mapping_wrprotect_range()
- * being called before the PTE is updated, which would leave
- * the page ignored by defio.
- * Do this by locking the page here and informing the caller
- * about it with VM_FAULT_LOCKED.
+ * The PTE must be marked writable before the defio deferred work runs
+ * again and potentially marks the PTE write-protected. If the order
+ * should be switched, the PTE would become writable without defio
+ * tracking the page, leaving the page forever ignored by defio.
+ *
+ * For vmalloc() framebuffers, the associated struct page is locked
+ * before releasing the defio lock. mm will later mark the PTE writaable
+ * and release the struct page lock. The struct page lock prevents
+ * the page from being prematurely being marked write-protected.
+ *
+ * For FBINFO_KMEMFB framebuffers, mm assumes there is no struct page,
+ * so the PTE must be marked writable while the defio lock is held.
*/
- lock_page(pageref->page);
+ if (info->flags & FBINFO_KMEMFB) {
+ unsigned long pfn = page_to_pfn(pageref->page);
+
+ ret = vmf_insert_mixed_mkwrite(vmf->vma, vmf->address,
+ __pfn_to_pfn_t(pfn,
PFN_SPECIAL));
