All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: mhklinux@outlook.com
Cc: simona@ffwll.ch, deller@gmx.de, haiyangz@microsoft.com,
	kys@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
	akpm@linux-foundation.org, weh@microsoft.com,
	tzimmermann@suse.de, hch@lst.de, dri-devel@lists.freedesktop.org,
	linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-hyperv@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
Date: Mon, 26 May 2025 08:54:48 +0200	[thread overview]
Message-ID: <20250526065448.GB13065@lst.de> (raw)
In-Reply-To: <20250523161522.409504-4-mhklinux@outlook.com>

On Fri, May 23, 2025 at 09:15:21AM -0700, mhkelley58@gmail.com wrote:
> 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.

That langugage is far too nice.  The existing users of fb_defio are
all gravely broken because they violate the dma API restriction to
not poke into the memory.  You can't speculate what you get from
dma_alloc_coherent and it can change behind you all the time.

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

Honestly, the right thing is to invert the flag.  What hypervs is doing
here - take kernel memory in the direct mapping or from vmalloc is fine.

What others drivers do it completely broken crap.  So add a flag
FBINFO_BROKEN_CRAP to maybe keep the guessing.  Or just disable it
because it's dangerous.


  parent reply	other threads:[~2025-05-26  6:54 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-23 16:15 [PATCH v3 0/4] fbdev: Add deferred I/O support for contiguous kernel memory framebuffers mhkelley58
2025-05-23 16:15 ` [PATCH v3 1/4] mm: Export vmf_insert_mixed_mkwrite() mhkelley58
2025-05-23 16:15 ` [PATCH v3 2/4] fbdev: Add flag indicating framebuffer is allocated from kernel memory mhkelley58
2025-05-23 16:15 ` [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers mhkelley58
2025-05-24  7:28   ` kernel test robot
2025-05-26  6:54   ` Christoph Hellwig [this message]
2025-06-02  9:47   ` David Hildenbrand
2025-06-03  1:49     ` Michael Kelley
2025-06-03  6:25       ` Thomas Zimmermann
2025-06-03 17:50         ` Michael Kelley
2025-06-04  8:12           ` Thomas Zimmermann
2025-06-04 14:45             ` Simona Vetter
2025-06-04 21:43               ` Michael Kelley
2025-06-05  7:55                 ` Thomas Zimmermann
2025-06-05 15:35                 ` Thomas Zimmermann
2025-06-05 17:38                   ` Michael Kelley
2025-06-06  7:05                     ` Thomas Zimmermann
2025-06-11 23:18                     ` Michael Kelley
2025-06-12  7:25                       ` Thomas Zimmermann
2025-06-03  7:55       ` David Hildenbrand
2025-06-03 17:24         ` Michael Kelley
2025-06-04 21:58           ` Michael Kelley
2025-06-05  8:10             ` David Hildenbrand
2025-05-23 16:15 ` [PATCH v3 4/4] fbdev: hyperv_fb: Fix mmap of framebuffers allocated using alloc_pages() mhkelley58

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250526065448.GB13065@lst.de \
    --to=hch@lst.de \
    --cc=akpm@linux-foundation.org \
    --cc=decui@microsoft.com \
    --cc=deller@gmx.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=haiyangz@microsoft.com \
    --cc=kys@microsoft.com \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhklinux@outlook.com \
    --cc=simona@ffwll.ch \
    --cc=tzimmermann@suse.de \
    --cc=weh@microsoft.com \
    --cc=wei.liu@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.