* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: Michael S. Tsirkin @ 2026-06-10 21:18 UTC (permalink / raw)
To: Miaohe Lin
Cc: Zi Yan, David Hildenbrand (Arm), Andrew Morton, linux-kernel,
Jason Wang, Xuan Zhuo, Eugenio Pérez, Muchun Song,
Oscar Salvador, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Baolin Wang, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Hugh Dickins, Matthew Brost, Joshua Hahn,
Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi
In-Reply-To: <dd8d89be-9c73-8ff7-7d44-9705bddeda4d@huawei.com>
On Wed, Jun 10, 2026 at 03:24:30PM +0800, Miaohe Lin wrote:
> On 2026/6/10 5:00, Michael S. Tsirkin wrote:
> > On Tue, Jun 09, 2026 at 04:54:01PM -0400, Zi Yan wrote:
> >> On 9 Jun 2026, at 16:34, Michael S. Tsirkin wrote:
> >>
> >>> On Tue, Jun 09, 2026 at 02:52:47PM -0400, Zi Yan wrote:
> >>>> On 9 Jun 2026, at 14:39, Zi Yan wrote:
> >>>>
> >>>>> On 9 Jun 2026, at 14:38, David Hildenbrand (Arm) wrote:
> >>>>>
> >>>>>> On 6/9/26 20:10, Andrew Morton wrote:
> >>>>>>> On Tue, 9 Jun 2026 06:12:49 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >>>>>>>
> >>>>>>>> TestSetPageHWPoison() is called without zone->lock, so its atomic
> >>>>>>>> update to page->flags can race with non-atomic flag operations
> >>>>>>>> that run under zone->lock in the buddy allocator.
> >>>>>>>>
> >>>>>>>> In particular, __free_pages_prepare() does:
> >>>>>>>>
> >>>>>>>> page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
> >>>>>>>>
> >>>>>>>> This non-atomic read-modify-write, while correctly excluding
> >>>>>>>> __PG_HWPOISON from the mask, can still lose a concurrent
> >>>>>>>> TestSetPageHWPoison if the read happens before the poison bit
> >>>>>>>> is set and the write happens after. Will only get worse if/when
> >>>>>>>> we add more non-atomic flag operations.
> >>>>>>>>
> >>>>>>>> Fix by acquiring zone->lock around TestSetPageHWPoison and
> >>>>>>>> around ClearPageHWPoison in the retry path. This
> >>>>>>>> serializes with all buddy flag manipulation. The cost is
> >>>>>>>> negligible: one lock/unlock in an extremely rare path
> >>>>>>>> (hardware memory errors).
> >>>>>>>>
> >>>>>>>> Note: SetPageHWPoison and TestClearPageHWPoison calls elsewhere
> >>>>>>>> in this file operate on pages already removed from the buddy
> >>>>>>>> allocator or on non-buddy pages (DAX, hugetlb), so they do not
> >>>>>>>> need zone->lock protection.
> >>>>>>>
> >>>>>>> Sashiko is saying this doesn't do anything "Because
> >>>>>>> __free_pages_prepare() executes entirely locklessly". Did it goof?
> >>>>>>>
> >>>>>>> https://sashiko.dev/#/patchset/df06b66fe4ff8e925ee0714955abc2183a727b90.1780998980.git.mst@redhat.com
> >>>>>>
> >>>>>> Battle of the bots: it's right.
> >>>>>
> >>>>> Yep, __free_pages_prepare() changes the page flag without holding
> >>>>> zone->lock.
> >>>>
> >>>> __free_pages_prepare() works on frozen pages and assumes no one else
> >>>> touches the input page. To avoid this race, memory_failure() might
> >>>> want to try_get_page() before TestClearPageHWPoison(), but I am not
> >>>> sure if that works along with memory failure flow.
> >>>>
> >>>> Best Regards,
> >>>> Yan, Zi
> >>>
> >>>
> >>>
> >>> Actually memory failure already plays with this down the road no?
> >>>
> >>> So maybe it's enough to just SetPageHWPoison afterwards again?
> >>>
> >>>
> >>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> >>> index ee42d4361309..4758fea94a96 100644
> >>> --- a/mm/memory-failure.c
> >>> +++ b/mm/memory-failure.c
> >>> @@ -2415,6 +2415,7 @@ int memory_failure(unsigned long pfn, int flags)
> >>> if (!res) {
> >>> if (is_free_buddy_page(p)) {
> >>> if (take_page_off_buddy(p)) {
> >>> + SetPageHWPoison(p);
> >>> page_ref_inc(p);
> >>> res = MF_RECOVERED;
> >>> } else {
> >>>
> >>>
> >>> and maybe in a bunch of other places in there?
> >>
> >> You mean for fear of losing HWPoison flag in the earlier TestSetPageHWPoison(),
> >> just set it again here?
> >
> > Yea.
> >
> >> Why not do it after get_hwpoison_page(), since that
> >> is the expected page flag?
> >
> > It's still in the buddy at that point right? I'm worried buddy might
> > poke at flags.
>
> Since __free_pages_prepare() executes entirely locklessly, the only way to ensure
> HWPoison flag won't be lost might be only set hwpoison flag iff we can make sure
> pages are not on the way to buddy...
>
> Thanks.
> .
To clarify do you not agree repeating SetPageHWPoison is enough for
this? And if not, do you have suggestions on how to fix this race?
Thanks a lot,
--
MST
^ permalink raw reply
* Re: [PATCH 04/11] treewide: Convert struct kernel_param_ops initializers to DEFINE_KERNEL_PARAM_OPS
From: jim.cromie @ 2026-06-10 21:06 UTC (permalink / raw)
To: Petr Pavlu
Cc: Kees Cook, Luis Chamberlain, Pengpeng Hou, Richard Weinberger,
Anton Ivanov, Johannes Berg, Rafael J. Wysocki, Len Brown,
Corey Minyard, Gabriel Somlo, Michael S. Tsirkin, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
Simona Vetter, Bart Van Assche, Jason Gunthorpe, Leon Romanovsky,
Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Bjorn Helgaas, Hannes Reinecke, James E.J. Bottomley,
Martin K. Petersen, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Greg Kroah-Hartman, Jiri Slaby, Alan Stern, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Jason Baron, Tiwei Bie, Benjamin Berg,
Ilpo Järvinen, David E. Box, Maciej W. Rozycki,
Srinivas Pandruvada, Peter Zijlstra, Heiko Carstens,
Vasily Gorbik, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Vinod Koul, Frank Li, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Alexander Potapenko, Marco Elver, Dmitry Vyukov,
Andrew Morton, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andy Shevchenko, Georgia Garcia, kvm, dmaengine,
linux-modules, kasan-dev, linux-mm, apparmor,
linux-security-module, linux-um, linux-acpi, openipmi-developer,
qemu-devel, intel-gfx, dri-devel, linux-rdma, linux-media,
linux-pci, linux-scsi, linux-pm, linuxppc-dev, linux-serial,
linux-usb, usb-storage, virtualization, linux-kernel, linux-arch,
netdev, linux-fsdevel, linux-hardening
In-Reply-To: <da358ae1-91b4-4a16-ac76-ffab99c230b9@suse.com>
On Mon, May 25, 2026 at 7:35 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> On 5/21/26 3:33 PM, Kees Cook wrote:
> > Using Coccinelle, rewrite every struct kernel_param_ops initializer that
> > sets .get into a DEFINE_KERNEL_PARAM_OPS-family macro invocation,
> > for example:
> >
> > @@
> > declarer name DEFINE_KERNEL_PARAM_OPS;
> > identifier OPS;
> > expression SET, GET;
> > @@
> > - const struct kernel_param_ops OPS = {
> > - .set = SET,
> > - .get = GET,
> > - };
> > + DEFINE_KERNEL_PARAM_OPS(OPS, SET, GET);
> >
> > Using the macro for initialization means future changes can manipulate
> > the struct layout and callback prototypes without having to change every
> > initializer.
>
> Nit: For consistency, I suggest also converting the few remaining
> kernel_param_ops instances that specify only .set and no .get, such as
> simdisk_param_ops_filename.
>
> --
> Thanks,
> Petr
for the dynamic-debug changes
Reviewed-by: Jim Cromie <jim.cromie@gmail.com>
^ permalink raw reply
* Re: [PATCH] drm: Consistently define pci_device_ids using named initializers
From: Patrik Jakobsson @ 2026-06-10 19:24 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Gerd Hoffmann, Markus Schneider-Pargmann,
Jianmin Lv, Qianhai Wu, Huacai Chen, Mingcong Bai, Xi Ruoyao,
Icenowy Zheng, Dave Airlie, Jocelyn Falempe, dri-devel,
linux-kernel, virtualization, spice-devel
In-Reply-To: <20260504150537.2136760-2-u.kleine-koenig@baylibre.com>
On Mon, May 4, 2026 at 5:05 PM Uwe Kleine-König (The Capable Hub)
<u.kleine-koenig@baylibre.com> wrote:
>
> The .driver_data member of the various struct pci_device_id arrays were
> initialized by list expressions. This isn't easily readable if you're
> not into PCI. Using the PCI_DEVICE macro and named initializers is more
> explicit and thus easier to parse. Also skip explicit assignments of 0
> (which the compiler then takes care of).
>
> This change doesn't introduce changes to the compiled pci_device_id
> arrays. Tested on x86 and arm64.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Since it touches multiple drivers, perhaps a drm-misc maintainer can take this?
-Patrik
> ---
> Hello,
>
> The secret plan is to make struct pci_device_id::driver_data an
> anonymous union (similar to
> https://lore.kernel.org/all/cover.1776579304.git.u.kleine-koenig@baylibre.com/)
> and that requires named initializers. But IMHO it's also a nice cleanup
> on its own.
>
> The anonymous union will allow changes like the following:
>
> - { PCI_DEVICE(0x8086, 0x8108), .driver_data = (long) &psb_chip_ops },
> + { PCI_DEVICE(0x8086, 0x8108), .driver_data_ptr = &psb_chip_ops },
>
> (together with the respective change in the code when the value is
> used). This gets rid of a bunch of casts and thus slightly improves
> type safety.
>
> Best regards
> Uwe
>
> drivers/gpu/drm/gma500/psb_drv.c | 56 +++++++++++++--------------
> drivers/gpu/drm/loongson/lsdc_drv.c | 4 +-
> drivers/gpu/drm/mgag200/mgag200_drv.c | 24 ++++++------
> drivers/gpu/drm/qxl/qxl_drv.c | 15 ++++---
> 4 files changed, 52 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
> index 005ab7f5355f..039da26ef24d 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.c
> +++ b/drivers/gpu/drm/gma500/psb_drv.c
> @@ -56,36 +56,36 @@ static int psb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
> */
> static const struct pci_device_id pciidlist[] = {
> /* Poulsbo */
> - { 0x8086, 0x8108, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &psb_chip_ops },
> - { 0x8086, 0x8109, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &psb_chip_ops },
> + { PCI_DEVICE(0x8086, 0x8108), .driver_data = (long) &psb_chip_ops },
> + { PCI_DEVICE(0x8086, 0x8109), .driver_data = (long) &psb_chip_ops },
> /* Oak Trail */
> - { 0x8086, 0x4100, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4101, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4103, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4104, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4105, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4106, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4107, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> - { 0x8086, 0x4108, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4100), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4101), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4102), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4103), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4104), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4105), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4106), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4107), .driver_data = (long) &oaktrail_chip_ops },
> + { PCI_DEVICE(0x8086, 0x4108), .driver_data = (long) &oaktrail_chip_ops },
> /* Cedar Trail */
> - { 0x8086, 0x0be0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be1, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be2, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be3, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be4, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be5, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be6, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be7, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be8, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0be9, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0bea, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0beb, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0bec, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0bed, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0bee, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0x8086, 0x0bef, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },
> - { 0, }
> + { PCI_DEVICE(0x8086, 0x0be0), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be1), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be2), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be3), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be4), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be5), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be6), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be7), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be8), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0be9), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0bea), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0beb), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0bec), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0bed), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0bee), .driver_data = (long) &cdv_chip_ops },
> + { PCI_DEVICE(0x8086, 0x0bef), .driver_data = (long) &cdv_chip_ops },
> + { }
> };
> MODULE_DEVICE_TABLE(pci, pciidlist);
>
> diff --git a/drivers/gpu/drm/loongson/lsdc_drv.c b/drivers/gpu/drm/loongson/lsdc_drv.c
> index 1ece1ea42f78..f9f7271ddbff 100644
> --- a/drivers/gpu/drm/loongson/lsdc_drv.c
> +++ b/drivers/gpu/drm/loongson/lsdc_drv.c
> @@ -444,8 +444,8 @@ static const struct dev_pm_ops lsdc_pm_ops = {
> };
>
> static const struct pci_device_id lsdc_pciid_list[] = {
> - {PCI_VDEVICE(LOONGSON, 0x7a06), CHIP_LS7A1000},
> - {PCI_VDEVICE(LOONGSON, 0x7a36), CHIP_LS7A2000},
> + { PCI_VDEVICE(LOONGSON, 0x7a06), .driver_data = CHIP_LS7A1000 },
> + { PCI_VDEVICE(LOONGSON, 0x7a36), .driver_data = CHIP_LS7A2000 },
> { }
> };
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index a32be27c39e8..8ad4ddb60ee6 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -205,18 +205,18 @@ int mgag200_device_init(struct mga_device *mdev,
> */
>
> static const struct pci_device_id mgag200_pciidlist[] = {
> - { PCI_VENDOR_ID_MATROX, 0x520, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_PCI },
> - { PCI_VENDOR_ID_MATROX, 0x521, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_AGP },
> - { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A },
> - { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B },
> - { PCI_VENDOR_ID_MATROX, 0x530, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EV },
> - { PCI_VENDOR_ID_MATROX, 0x532, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_WB },
> - { PCI_VENDOR_ID_MATROX, 0x533, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EH },
> - { PCI_VENDOR_ID_MATROX, 0x534, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_ER },
> - { PCI_VENDOR_ID_MATROX, 0x536, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EW3 },
> - { PCI_VENDOR_ID_MATROX, 0x538, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EH3 },
> - { PCI_VENDOR_ID_MATROX, 0x53a, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EH5 },
> - {0,}
> + { PCI_VDEVICE(MATROX, 0x0520), .driver_data = G200_PCI },
> + { PCI_VDEVICE(MATROX, 0x0521), .driver_data = G200_AGP },
> + { PCI_VDEVICE(MATROX, 0x0522), .driver_data = G200_SE_A },
> + { PCI_VDEVICE(MATROX, 0x0524), .driver_data = G200_SE_B },
> + { PCI_VDEVICE(MATROX, 0x0530), .driver_data = G200_EV },
> + { PCI_VDEVICE(MATROX, 0x0532), .driver_data = G200_WB },
> + { PCI_VDEVICE(MATROX, 0x0533), .driver_data = G200_EH },
> + { PCI_VDEVICE(MATROX, 0x0534), .driver_data = G200_ER },
> + { PCI_VDEVICE(MATROX, 0x0536), .driver_data = G200_EW3 },
> + { PCI_VDEVICE(MATROX, 0x0538), .driver_data = G200_EH3 },
> + { PCI_VDEVICE(MATROX, 0x053a), .driver_data = G200_EH5 },
> + { }
> };
>
> MODULE_DEVICE_TABLE(pci, mgag200_pciidlist);
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 2bbb1168a3ff..6c3c309b8e4d 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -50,11 +50,16 @@
> #include "qxl_object.h"
>
> static const struct pci_device_id pciidlist[] = {
> - { 0x1b36, 0x100, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8,
> - 0xffff00, 0 },
> - { 0x1b36, 0x100, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_OTHER << 8,
> - 0xffff00, 0 },
> - { 0, 0, 0 },
> + {
> + PCI_DEVICE(0x1b36, 0x0100),
> + .class = PCI_CLASS_DISPLAY_VGA << 8,
> + .class_mask = 0xffff00
> + }, {
> + PCI_DEVICE(0x1b36, 0x0100),
> + .class = PCI_CLASS_DISPLAY_OTHER << 8,
> + .class_mask = 0xffff00
> + },
> + { },
> };
> MODULE_DEVICE_TABLE(pci, pciidlist);
>
>
> base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
> --
> 2.47.3
>
^ permalink raw reply
* [PATCH] vduse: Fix error around jumping over a __cleanup() variable
From: Nathan Chancellor @ 2026-06-10 19:16 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez
Cc: virtualization, linux-kernel, llvm, Nathan Chancellor
When building with clang, there is an error in vduse_vq_kick() from
attempting to jump over a variable declared with the cleanup attribute
using goto:
drivers/vdpa/vdpa_user/vduse_dev.c:566:3: error: cannot jump from this goto statement to its label
566 | goto unlock;
| ^
drivers/vdpa/vdpa_user/vduse_dev.c:568:2: note: jump bypasses initialization of variable with __attribute__((cleanup))
568 | guard(rwsem_read)(&vq->dev->rwsem);
| ^
Jumping over a variable declared with the cleanup attribute does not
prevent the cleanup function from running, it would just result in the
variable being passed uninitialized to the cleanup function .clang
errors instead of generating the invalid code, unlike GCC.
The jump is only present to call spin_unlock(), so convert the
spin_lock() and spin_unlock() calls to an equivalent guard() to avoid
the jump while leaving runtime behavior unchanged, clearing up the
warning.
Fixes: 6c141c034c1b ("vduse: Add suspend")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
drivers/vdpa/vdpa_user/vduse_dev.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index a257fdcb77b7..0500da043761 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -561,9 +561,9 @@ static int vduse_vdpa_set_vq_address(struct vdpa_device *vdpa, u16 idx,
static void vduse_vq_kick(struct vduse_virtqueue *vq)
{
- spin_lock(&vq->kick_lock);
+ guard(spinlock)(&vq->kick_lock);
if (!vq->ready)
- goto unlock;
+ return;
guard(rwsem_read)(&vq->dev->rwsem);
if (vq->dev->suspended)
@@ -573,8 +573,6 @@ static void vduse_vq_kick(struct vduse_virtqueue *vq)
eventfd_signal(vq->kickfd);
else
vq->kicked = true;
-unlock:
- spin_unlock(&vq->kick_lock);
}
static void vduse_vq_kick_work(struct work_struct *work)
---
base-commit: 6c141c034c1b0b74a2ca4dd3d6fbb6d9054f6e46
change-id: 20260610-vduse_vq_kick-fix-guard-usage-d1037c331419
Best regards,
--
Cheers,
Nathan
^ permalink raw reply related
* Re: [PATCH v3] vduse: Add suspend
From: kernel test robot @ 2026-06-10 19:10 UTC (permalink / raw)
To: Eugenio Pérez, Michael S . Tsirkin
Cc: llvm, oe-kbuild-all, virtualization, Jason Wang, Cindy Lu,
Xuan Zhuo, Stefano Garzarella, linux-kernel, Laurent Vivier,
Yongji Xie, Eugenio Pérez, Maxime Coquelin
In-Reply-To: <20260610083452.477759-1-eperezma@redhat.com>
Hi Eugenio,
kernel test robot noticed the following build errors:
[auto build test ERROR on next-20260609]
[cannot apply to linus/master v7.1-rc7 v7.1-rc6 v7.1-rc5 v7.1-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Eugenio-P-rez/vduse-Add-suspend/20260610-164534
base: next-20260609
patch link: https://lore.kernel.org/r/20260610083452.477759-1-eperezma%40redhat.com
patch subject: [PATCH v3] vduse: Add suspend
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20260611/202606110222.CJtRWzQl-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 7917772d7d61384696c61102c08c2ea158e610fa)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260611/202606110222.CJtRWzQl-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202606110222.CJtRWzQl-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/vdpa/vdpa_user/vduse_dev.c:566:3: error: cannot jump from this goto statement to its label
566 | goto unlock;
| ^
drivers/vdpa/vdpa_user/vduse_dev.c:568:2: note: jump bypasses initialization of variable with __attribute__((cleanup))
568 | guard(rwsem_read)(&vq->dev->rwsem);
| ^
include/linux/cleanup.h:423:2: note: expanded from macro 'guard'
423 | CLASS(_name, __UNIQUE_ID(guard))
| ^
include/linux/cleanup.h:303:3: note: expanded from macro 'CLASS'
303 | class_##_name##_constructor
| ^
<scratch space>:132:1: note: expanded from here
132 | class_rwsem_read_constructor
| ^
note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
include/linux/compiler_types.h:16:23: note: expanded from macro '__PASTE'
16 | #define __PASTE(a, b) ___PASTE(a, b)
| ^
include/linux/compiler_types.h:15:24: note: expanded from macro '___PASTE'
15 | #define ___PASTE(a, b) a##b
| ^
<scratch space>:138:1: note: expanded from here
138 | __UNIQUE_ID_unlock_770
| ^
drivers/vdpa/vdpa_user/vduse_dev.c:568:2: note: jump bypasses initialization of variable with __attribute__((cleanup))
include/linux/cleanup.h:423:15: note: expanded from macro 'guard'
423 | CLASS(_name, __UNIQUE_ID(guard))
| ^
include/linux/compiler.h:165:2: note: expanded from macro '__UNIQUE_ID'
165 | __PASTE(__UNIQUE_ID_, \
| ^
include/linux/compiler_types.h:16:23: note: expanded from macro '__PASTE'
16 | #define __PASTE(a, b) ___PASTE(a, b)
| ^
include/linux/compiler_types.h:15:24: note: expanded from macro '___PASTE'
15 | #define ___PASTE(a, b) a##b
| ^
<scratch space>:126:1: note: expanded from here
126 | __UNIQUE_ID_guard_769
| ^
1 error generated.
vim +566 drivers/vdpa/vdpa_user/vduse_dev.c
c8a6153b6c59d9 Xie Yongji 2021-08-31 561
c8a6153b6c59d9 Xie Yongji 2021-08-31 562 static void vduse_vq_kick(struct vduse_virtqueue *vq)
c8a6153b6c59d9 Xie Yongji 2021-08-31 563 {
c8a6153b6c59d9 Xie Yongji 2021-08-31 564 spin_lock(&vq->kick_lock);
c8a6153b6c59d9 Xie Yongji 2021-08-31 565 if (!vq->ready)
c8a6153b6c59d9 Xie Yongji 2021-08-31 @566 goto unlock;
c8a6153b6c59d9 Xie Yongji 2021-08-31 567
9c4307e82fa1dc Eugenio Pérez 2026-06-10 568 guard(rwsem_read)(&vq->dev->rwsem);
9c4307e82fa1dc Eugenio Pérez 2026-06-10 569 if (vq->dev->suspended)
9c4307e82fa1dc Eugenio Pérez 2026-06-10 570 return;
9c4307e82fa1dc Eugenio Pérez 2026-06-10 571
c8a6153b6c59d9 Xie Yongji 2021-08-31 572 if (vq->kickfd)
3652117f854819 Christian Brauner 2023-11-22 573 eventfd_signal(vq->kickfd);
c8a6153b6c59d9 Xie Yongji 2021-08-31 574 else
c8a6153b6c59d9 Xie Yongji 2021-08-31 575 vq->kicked = true;
c8a6153b6c59d9 Xie Yongji 2021-08-31 576 unlock:
c8a6153b6c59d9 Xie Yongji 2021-08-31 577 spin_unlock(&vq->kick_lock);
c8a6153b6c59d9 Xie Yongji 2021-08-31 578 }
c8a6153b6c59d9 Xie Yongji 2021-08-31 579
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v5] i2c: virtio: retain xfer with kref to fix UAF on interrupted wait
From: Michael S. Tsirkin @ 2026-06-10 18:29 UTC (permalink / raw)
To: Gavin Li
Cc: linux-i2c, viresh.kumar, Chen, Jian Jun, andi.shyti,
virtualization
In-Reply-To: <CAKvMnUXk0GrYNNYOHvME6FYRdKik-7fV5B-NgvRM+i5yYKTmvw@mail.gmail.com>
On Wed, Jun 10, 2026 at 01:34:10PM -0400, Gavin Li wrote:
> Another option I am considering:
>
> - Uninterruptible wait with timeout, timeout based on adap->timeout
> - Upon timeout, reset the device with
> virtio_i2c_del_vqs() + virtio_i2c_setup_vqs() + virtio_device_ready()
>
> This behavior more closely mirrors what other I2C bus drivers do.
> The device should be completely quiesced when virtio_i2c_del_vqs()
> returns, avoiding the UAF.
I don't know enough about i2c.
Maybe
1. revert
2. some other hack
--
MST
^ permalink raw reply
* Re: [PATCH v5] i2c: virtio: retain xfer with kref to fix UAF on interrupted wait
From: Michael S. Tsirkin @ 2026-06-10 18:27 UTC (permalink / raw)
To: Gavin Li
Cc: linux-i2c, viresh.kumar, Chen, Jian Jun, andi.shyti,
virtualization
In-Reply-To: <CAKvMnUW-3b=cqHcozydnc6K1vD06FGk1ZO_q7S6SqhA5d8eR2g@mail.gmail.com>
On Wed, Jun 10, 2026 at 12:32:42PM -0400, Gavin Li wrote:
> On qemu, queue reset is only supported by virtio-net.
Not hard to fix.
> If a queue reset
> is requested, the vhost backend is never notified, and as a result it's
> still at the device's discretion to write to the potentially freed buffer.
>
> As for device reset, I really don't want to initiate a device reset just
> because a userspace process was signaled (it seems a little extreme).
> I can implement this if you think it is the best path forward.
>
> Compared to the original patch of making the wait uninterruptible,
> I feel like this patch has become much larger than I originally wanted.
> The commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
> completion wait") that introduced the UAF mentioned that it was originally
> done because a transfer could hang, but IMO this should really be fixed
> in the vhost backend rather than in the driver, mostly since virtio-i2c
> doesn't provide a way to cancel an in-flight request.
Maybe the 1st step is to revert that then. Up to i2c maintainers.
^ permalink raw reply
* Re: [PATCH v5] i2c: virtio: retain xfer with kref to fix UAF on interrupted wait
From: Gavin Li @ 2026-06-10 17:34 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-i2c, viresh.kumar, Chen, Jian Jun, andi.shyti,
virtualization
In-Reply-To: <CAKvMnUW-3b=cqHcozydnc6K1vD06FGk1ZO_q7S6SqhA5d8eR2g@mail.gmail.com>
Another option I am considering:
- Uninterruptible wait with timeout, timeout based on adap->timeout
- Upon timeout, reset the device with
virtio_i2c_del_vqs() + virtio_i2c_setup_vqs() + virtio_device_ready()
This behavior more closely mirrors what other I2C bus drivers do.
The device should be completely quiesced when virtio_i2c_del_vqs()
returns, avoiding the UAF.
On Wed, Jun 10, 2026 at 12:32 PM Gavin Li <gavin.li@samsara.com> wrote:
>
> On qemu, queue reset is only supported by virtio-net. If a queue reset
> is requested, the vhost backend is never notified, and as a result it's
> still at the device's discretion to write to the potentially freed buffer.
>
> As for device reset, I really don't want to initiate a device reset just
> because a userspace process was signaled (it seems a little extreme).
> I can implement this if you think it is the best path forward.
>
> Compared to the original patch of making the wait uninterruptible,
> I feel like this patch has become much larger than I originally wanted.
> The commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
> completion wait") that introduced the UAF mentioned that it was originally
> done because a transfer could hang, but IMO this should really be fixed
> in the vhost backend rather than in the driver, mostly since virtio-i2c
> doesn't provide a way to cancel an in-flight request.
>
> On Wed, Jun 10, 2026 at 12:07 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Wed, Jun 10, 2026 at 11:58:34AM -0400, Gavin Li wrote:
> > > commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
> > > completion wait") switched virtio_i2c_complete_reqs() to
> > > wait_for_completion_interruptible() so a stuck device cannot hang a
> > > task forever. That left a use-after-free: if the wait returns early on
> > > a signal, virtio_i2c_xfer() frees reqs and DMA bounce buffers while the
> > > device may still hold virtqueue tokens pointing at &reqs[i] and DMA
> > > into read buffers. When those requests complete later,
> > > virtio_i2c_msg_done() calls complete() on freed memory.
> > >
> > > Waiting uninterruptibly for every completion before freeing avoids the
> > > UAF but can hang the caller indefinitely if the virtio side never
> > > completes the request. Performing a queue reset / device reset is
> > > possible in this scenario, but adds complexity.
> > >
> > > This commit manages the freeing of the xfer allocations via kref, and
> > > ensures that each in-flight request holds a reference. This fixes the
> > > use-after-free by ensuring that the virtio device has a valid location
> > > to write to until the request completes. This will cause a memory leak
> > > in cases where the device hangs, but that is much preferable to memory
> > > corruption.
> > >
> > > Additionally, force usage of a bounce buffer even if the i2c_msg buf is
> > > DMA-safe, since the buffer passed to the virtqueue must remain valid
> > > even if the transfer is interrupted. Remove usage of
> > > i2c_get_dma_safe_msg_buf() since it may pass through msg->buf directly.
> > > All bounce buffers are part of the single xfer allocation, so there is
> > > no additional allocation overhead.
> > >
> > > Signed-off-by: Gavin Li <gavin.li@samsara.com>
> >
> > But maybe, if queue reset is supported, we could use that?
> >
> > Is that problematic?
> >
> >
> > And maybe fallback on device reset on interrupt even?
> >
> > > ---
> > >
> > > Changes in v5:
> > > - DMA-align all bounce buffers
> > >
> > > Changes in v4:
> > > - Pack bounce buffers into a single allocation after reqs[]
> > > - Remove per-request buf pointer and xfer->num
> > > - Remove req.msg, track message direction with req->read
> > > - Simplify xfer release to a single kfree()
> > > - Restore using j to track successful transfers in _complete_xfer()
> > > ---
> > > drivers/i2c/busses/i2c-virtio.c | 135 +++++++++++++++++++++++++-------
> > > 1 file changed, 108 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> > > index 5da6fef92bec3..a5602865102d9 100644
> > > --- a/drivers/i2c/busses/i2c-virtio.c
> > > +++ b/drivers/i2c/busses/i2c-virtio.c
> > > @@ -10,10 +10,13 @@
> > >
> > > #include <linux/acpi.h>
> > > #include <linux/completion.h>
> > > +#include <linux/dma-mapping.h>
> > > #include <linux/err.h>
> > > #include <linux/i2c.h>
> > > #include <linux/kernel.h>
> > > +#include <linux/kref.h>
> > > #include <linux/module.h>
> > > +#include <linux/overflow.h>
> > > #include <linux/virtio.h>
> > > #include <linux/virtio_ids.h>
> > > #include <linux/virtio_config.h>
> > > @@ -31,39 +34,80 @@ struct virtio_i2c {
> > > struct virtqueue *vq;
> > > };
> > >
> > > +struct virtio_i2c_xfer;
> > > +
> > > /**
> > > * struct virtio_i2c_req - the virtio I2C request structure
> > > + * @xfer: owning transfer
> > > * @completion: completion of virtio I2C message
> > > + * @read: true if this is a read message (I2C_M_RD is set)
> > > * @out_hdr: the OUT header of the virtio I2C message
> > > - * @buf: the buffer into which data is read, or from which it's written
> > > * @in_hdr: the IN header of the virtio I2C message
> > > */
> > > struct virtio_i2c_req {
> > > + struct virtio_i2c_xfer *xfer;
> > > struct completion completion;
> > > + bool read;
> > > struct virtio_i2c_out_hdr out_hdr ____cacheline_aligned;
> > > - uint8_t *buf ____cacheline_aligned;
> > > struct virtio_i2c_in_hdr in_hdr ____cacheline_aligned;
> > > };
> > >
> > > +/**
> > > + * struct virtio_i2c_xfer - a queued I2C transfer
> > > + * @ref: one ref for the caller, plus one per in-flight virtqueue request
> > > + * @bounce_buf_base: start of bounce buffer region
> > > + * @reqs: the virtio I2C requests
> > > + *
> > > + * Allocation layout:
> > > + * - struct virtio_i2c_xfer xfer
> > > + * - struct virtio_i2c_req reqs[num]
> > > + * - padding to dma_get_cache_alignment()
> > > + * - u8 bounce_buf[virtio_i2c_bounce_size(msgs[0].len)]
> > > + * ...
> > > + * - u8 bounce_buf[virtio_i2c_bounce_size(msgs[num-1].len)]
> > > + */
> > > +struct virtio_i2c_xfer {
> > > + struct kref ref;
> > > + u8 *bounce_buf_base;
> > > + struct virtio_i2c_req reqs[];
> > > +};
> > > +
> > > +static size_t virtio_i2c_bounce_size(unsigned int len)
> > > +{
> > > + return ALIGN(len, dma_get_cache_alignment());
> > > +}
> > > +
> > > +static void virtio_i2c_xfer_release(struct kref *ref)
> > > +{
> > > + struct virtio_i2c_xfer *xfer = container_of(ref, struct virtio_i2c_xfer, ref);
> > > + kfree(xfer);
> > > +}
> > > +
> > > static void virtio_i2c_msg_done(struct virtqueue *vq)
> > > {
> > > struct virtio_i2c_req *req;
> > > unsigned int len;
> > >
> > > - while ((req = virtqueue_get_buf(vq, &len)))
> > > + while ((req = virtqueue_get_buf(vq, &len))) {
> > > complete(&req->completion);
> > > + kref_put(&req->xfer->ref, virtio_i2c_xfer_release);
> > > + }
> > > }
> > >
> > > -static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > > - struct virtio_i2c_req *reqs,
> > > +static int virtio_i2c_prepare_xfer(struct virtqueue *vq,
> > > + struct virtio_i2c_xfer *xfer,
> > > struct i2c_msg *msgs, int num)
> > > {
> > > struct scatterlist *sgs[3], out_hdr, msg_buf, in_hdr;
> > > + struct virtio_i2c_req *reqs = xfer->reqs;
> > > + u8 *bounce_buf = xfer->bounce_buf_base;
> > > int i;
> > >
> > > for (i = 0; i < num; i++) {
> > > int outcnt = 0, incnt = 0;
> > >
> > > + reqs[i].xfer = xfer;
> > > + reqs[i].read = !!(msgs[i].flags & I2C_M_RD);
> > > init_completion(&reqs[i].completion);
> > >
> > > /*
> > > @@ -82,23 +126,31 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > > sgs[outcnt++] = &out_hdr;
> > >
> > > if (msgs[i].len) {
> > > - reqs[i].buf = i2c_get_dma_safe_msg_buf(&msgs[i], 1);
> > > - if (!reqs[i].buf)
> > > - break;
> > > + /*
> > > + * Even if msg->flags has I2C_M_DMA_SAFE set, a bounce
> > > + * buffer is required because the transfer may be
> > > + * interrupted, after which msg->buf is no longer valid.
> > > + */
> > > + if (!(msgs[i].flags & I2C_M_RD))
> > > + memcpy(bounce_buf, msgs[i].buf, msgs[i].len);
> > >
> > > - sg_init_one(&msg_buf, reqs[i].buf, msgs[i].len);
> > > + sg_init_one(&msg_buf, bounce_buf, msgs[i].len);
> > >
> > > if (msgs[i].flags & I2C_M_RD)
> > > sgs[outcnt + incnt++] = &msg_buf;
> > > else
> > > sgs[outcnt++] = &msg_buf;
> > > }
> > > + bounce_buf += virtio_i2c_bounce_size(msgs[i].len);
> > >
> > > sg_init_one(&in_hdr, &reqs[i].in_hdr, sizeof(reqs[i].in_hdr));
> > > sgs[outcnt + incnt++] = &in_hdr;
> > >
> > > + /* This reference is released in virtio_i2c_msg_done(). */
> > > + kref_get(&xfer->ref);
> > > +
> > > if (virtqueue_add_sgs(vq, sgs, outcnt, incnt, &reqs[i], GFP_KERNEL)) {
> > > - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], false);
> > > + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> > > break;
> > > }
> > > }
> > > @@ -106,26 +158,38 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > > return i;
> > > }
> > >
> > > -static int virtio_i2c_complete_reqs(struct virtqueue *vq,
> > > - struct virtio_i2c_req *reqs,
> > > - struct i2c_msg *msgs, int num)
> > > +static int virtio_i2c_complete_xfer(struct virtio_i2c_xfer *xfer,
> > > + struct i2c_msg *msgs,
> > > + int num)
> > > {
> > > + struct virtio_i2c_req *reqs = xfer->reqs;
> > > + u8 *bounce_buf = xfer->bounce_buf_base;
> > > bool failed = false;
> > > int i, j = 0;
> > >
> > > for (i = 0; i < num; i++) {
> > > struct virtio_i2c_req *req = &reqs[i];
> > > + struct i2c_msg *msg = &msgs[i];
> > > +
> > > + if (wait_for_completion_interruptible(&req->completion))
> > > + return -EINTR;
> > > +
> > > + if (req->in_hdr.status != VIRTIO_I2C_MSG_OK) {
> > > + /*
> > > + * Don't break yet. Try to wait until all requests
> > > + * complete to ensure that the virtqueue has enough
> > > + * descriptor slots for the next transfer.
> > > + */
> > > + failed = true;
> > > + }
> > >
> > > if (!failed) {
> > > - if (wait_for_completion_interruptible(&req->completion))
> > > - failed = true;
> > > - else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> > > - failed = true;
> > > - else
> > > - j++;
> > > + if (req->read)
> > > + memcpy(msg->buf, bounce_buf, msg->len);
> > > + j++;
> > > }
> > >
> > > - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
> > > + bounce_buf += virtio_i2c_bounce_size(msg->len);
> > > }
> > >
> > > return j;
> > > @@ -136,14 +200,31 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> > > {
> > > struct virtio_i2c *vi = i2c_get_adapdata(adap);
> > > struct virtqueue *vq = vi->vq;
> > > - struct virtio_i2c_req *reqs;
> > > - int count;
> > > + struct virtio_i2c_xfer *xfer;
> > > + size_t alloc_size;
> > > + int i, count;
> > > +
> > > + alloc_size = struct_size(xfer, reqs, num);
> > > + if (check_add_overflow(alloc_size,
> > > + dma_get_cache_alignment() - 1,
> > > + &alloc_size)) /* padding for PTR_ALIGN() */
> > > + return -EOVERFLOW;
> > > + for (i = 0; i < num; i++) {
> > > + if (check_add_overflow(alloc_size,
> > > + virtio_i2c_bounce_size(msgs[i].len),
> > > + &alloc_size))
> > > + return -EOVERFLOW;
> > > + }
> > >
> > > - reqs = kzalloc_objs(*reqs, num);
> > > - if (!reqs)
> > > + xfer = kzalloc(alloc_size, GFP_KERNEL);
> > > + if (!xfer)
> > > return -ENOMEM;
> > >
> > > - count = virtio_i2c_prepare_reqs(vq, reqs, msgs, num);
> > > + kref_init(&xfer->ref);
> > > + xfer->bounce_buf_base = PTR_ALIGN((u8 *)(xfer->reqs + num),
> > > + dma_get_cache_alignment());
> > > +
> > > + count = virtio_i2c_prepare_xfer(vq, xfer, msgs, num);
> > > if (!count)
> > > goto err_free;
> > >
> > > @@ -157,10 +238,10 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> > > */
> > > virtqueue_kick(vq);
> > >
> > > - count = virtio_i2c_complete_reqs(vq, reqs, msgs, count);
> > > + count = virtio_i2c_complete_xfer(xfer, msgs, count);
> > >
> > > err_free:
> > > - kfree(reqs);
> > > + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> > > return count;
> > > }
> > >
> > > --
> > > 2.54.0
> >
^ permalink raw reply
* Re: [PATCH] drm: Consistently define pci_device_ids using named initializers
From: Uwe Kleine-König (The Capable Hub) @ 2026-06-10 16:57 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Gerd Hoffmann
Cc: Markus Schneider-Pargmann, Patrik Jakobsson, Jianmin Lv,
Qianhai Wu, Huacai Chen, Mingcong Bai, Xi Ruoyao, Icenowy Zheng,
Dave Airlie, Jocelyn Falempe, dri-devel, linux-kernel,
virtualization, spice-devel
In-Reply-To: <20260504150537.2136760-2-u.kleine-koenig@baylibre.com>
[-- Attachment #1: Type: text/plain, Size: 730 bytes --]
On Mon, May 04, 2026 at 05:05:37PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> The .driver_data member of the various struct pci_device_id arrays were
> initialized by list expressions. This isn't easily readable if you're
> not into PCI. Using the PCI_DEVICE macro and named initializers is more
> explicit and thus easier to parse. Also skip explicit assignments of 0
> (which the compiler then takes care of).
>
> This change doesn't introduce changes to the compiled pci_device_id
> arrays. Tested on x86 and arm64.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Does someone feel responsible to pick up this patch? (Or give review
feedback?)
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v3 3/4] block: drop shared-tag fairness throttling
From: Keith Busch @ 2026-06-10 16:35 UTC (permalink / raw)
To: Sumit Saxena
Cc: Christoph Hellwig, Martin K . Petersen, Jens Axboe,
James E . J . Bottomley, linux-scsi, linux-block, Adam Radford,
Khalid Aziz, Adaptec OEM Raid Solutions, Matthew Wilcox,
Hannes Reinecke, Juergen E . Fischer, Russell King,
linux-arm-kernel, Finn Thain, Michael Schmitz, Anil Gurumurthy,
Sudarsana Kalluru, Oliver Neukum, Ali Akcaagac, Jamie Lenehan,
Ram Vegesna, target-devel, Bradley Grove, Satish Kharat,
Sesidhar Baddela, Karan Tilak Kumar, Yihang Li, Don Brace,
storagedev, HighPoint Linux Team, Tyrel Datwyler,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, linuxppc-dev, Brian King, Lee Duncan,
Chris Leech, Mike Christie, open-iscsi, Justin Tee, Paul Ely,
Kashyap Desai, Shivasharan S, Chandrakanth Patil,
megaraidlinux.pdl, Sathya Prakash Veerichetty, Sreekanth Reddy,
mpi3mr-linuxdrv.pdl, Suganath Prabu Subramani, Ranjan Kumar,
MPT-FusionLinux.pdl, Daniel Palmer, GOTO Masanori, YOKOTA Hiroshi,
Jack Wang, Geoff Levand, Michael Reed, Nilesh Javali,
GR-QLogic-Storage-Upstream, Narsimhulu Musini, K . Y . Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, linux-hyperv,
Michael S . Tsirkin, Jason Wang, Paolo Bonzini, Stefan Hajnoczi,
Eugenio Perez, virtualization, Vishal Bhakta,
bcm-kernel-feedback-list, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, xen-devel, Bart Van Assche
In-Reply-To: <CAL2rwxr1uGshb1o=jvP2OnBffNz2cKXj8tHuAUCN5HFuy2vB_g@mail.gmail.com>
On Wed, Jun 10, 2026 at 09:16:11PM +0530, Sumit Saxena wrote:
> The motivation for this change stems from performance issue we
> encountered due to false sharing of the 'nr_active_requests_shared_tags'
> counter
> on certain CPU architectures. I initially submitted a patch to move that
> counter to
> its own cache line to avoid conflicts with 'nr_requests' and other hot
> fields
> (see:
> https://patchwork.kernel.org/project/linux-scsi/patch/20260402074637.92417-3-sumit.saxena@broadcom.com/
> ).
>
> During the review, Bart shared his work, which eliminates the
> counter entirely by removing the fairness throttling. My testing confirmed
> that
> this approach resolved the performance issues and improved IOPS.
> This patch is part of a larger set, and I have reported the cumulative
> performance
> improvements in the cover letter.
So the problem is just the atomic operation accounting overhead? I
previously thought the device just really needed to consume all the tags
to hit performance.
^ permalink raw reply
* Re: [PATCH v5] i2c: virtio: retain xfer with kref to fix UAF on interrupted wait
From: Gavin Li @ 2026-06-10 16:32 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-i2c, viresh.kumar, Chen, Jian Jun, andi.shyti,
virtualization
In-Reply-To: <20260610120606-mutt-send-email-mst@kernel.org>
On qemu, queue reset is only supported by virtio-net. If a queue reset
is requested, the vhost backend is never notified, and as a result it's
still at the device's discretion to write to the potentially freed buffer.
As for device reset, I really don't want to initiate a device reset just
because a userspace process was signaled (it seems a little extreme).
I can implement this if you think it is the best path forward.
Compared to the original patch of making the wait uninterruptible,
I feel like this patch has become much larger than I originally wanted.
The commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
completion wait") that introduced the UAF mentioned that it was originally
done because a transfer could hang, but IMO this should really be fixed
in the vhost backend rather than in the driver, mostly since virtio-i2c
doesn't provide a way to cancel an in-flight request.
On Wed, Jun 10, 2026 at 12:07 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jun 10, 2026 at 11:58:34AM -0400, Gavin Li wrote:
> > commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
> > completion wait") switched virtio_i2c_complete_reqs() to
> > wait_for_completion_interruptible() so a stuck device cannot hang a
> > task forever. That left a use-after-free: if the wait returns early on
> > a signal, virtio_i2c_xfer() frees reqs and DMA bounce buffers while the
> > device may still hold virtqueue tokens pointing at &reqs[i] and DMA
> > into read buffers. When those requests complete later,
> > virtio_i2c_msg_done() calls complete() on freed memory.
> >
> > Waiting uninterruptibly for every completion before freeing avoids the
> > UAF but can hang the caller indefinitely if the virtio side never
> > completes the request. Performing a queue reset / device reset is
> > possible in this scenario, but adds complexity.
> >
> > This commit manages the freeing of the xfer allocations via kref, and
> > ensures that each in-flight request holds a reference. This fixes the
> > use-after-free by ensuring that the virtio device has a valid location
> > to write to until the request completes. This will cause a memory leak
> > in cases where the device hangs, but that is much preferable to memory
> > corruption.
> >
> > Additionally, force usage of a bounce buffer even if the i2c_msg buf is
> > DMA-safe, since the buffer passed to the virtqueue must remain valid
> > even if the transfer is interrupted. Remove usage of
> > i2c_get_dma_safe_msg_buf() since it may pass through msg->buf directly.
> > All bounce buffers are part of the single xfer allocation, so there is
> > no additional allocation overhead.
> >
> > Signed-off-by: Gavin Li <gavin.li@samsara.com>
>
> But maybe, if queue reset is supported, we could use that?
>
> Is that problematic?
>
>
> And maybe fallback on device reset on interrupt even?
>
> > ---
> >
> > Changes in v5:
> > - DMA-align all bounce buffers
> >
> > Changes in v4:
> > - Pack bounce buffers into a single allocation after reqs[]
> > - Remove per-request buf pointer and xfer->num
> > - Remove req.msg, track message direction with req->read
> > - Simplify xfer release to a single kfree()
> > - Restore using j to track successful transfers in _complete_xfer()
> > ---
> > drivers/i2c/busses/i2c-virtio.c | 135 +++++++++++++++++++++++++-------
> > 1 file changed, 108 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> > index 5da6fef92bec3..a5602865102d9 100644
> > --- a/drivers/i2c/busses/i2c-virtio.c
> > +++ b/drivers/i2c/busses/i2c-virtio.c
> > @@ -10,10 +10,13 @@
> >
> > #include <linux/acpi.h>
> > #include <linux/completion.h>
> > +#include <linux/dma-mapping.h>
> > #include <linux/err.h>
> > #include <linux/i2c.h>
> > #include <linux/kernel.h>
> > +#include <linux/kref.h>
> > #include <linux/module.h>
> > +#include <linux/overflow.h>
> > #include <linux/virtio.h>
> > #include <linux/virtio_ids.h>
> > #include <linux/virtio_config.h>
> > @@ -31,39 +34,80 @@ struct virtio_i2c {
> > struct virtqueue *vq;
> > };
> >
> > +struct virtio_i2c_xfer;
> > +
> > /**
> > * struct virtio_i2c_req - the virtio I2C request structure
> > + * @xfer: owning transfer
> > * @completion: completion of virtio I2C message
> > + * @read: true if this is a read message (I2C_M_RD is set)
> > * @out_hdr: the OUT header of the virtio I2C message
> > - * @buf: the buffer into which data is read, or from which it's written
> > * @in_hdr: the IN header of the virtio I2C message
> > */
> > struct virtio_i2c_req {
> > + struct virtio_i2c_xfer *xfer;
> > struct completion completion;
> > + bool read;
> > struct virtio_i2c_out_hdr out_hdr ____cacheline_aligned;
> > - uint8_t *buf ____cacheline_aligned;
> > struct virtio_i2c_in_hdr in_hdr ____cacheline_aligned;
> > };
> >
> > +/**
> > + * struct virtio_i2c_xfer - a queued I2C transfer
> > + * @ref: one ref for the caller, plus one per in-flight virtqueue request
> > + * @bounce_buf_base: start of bounce buffer region
> > + * @reqs: the virtio I2C requests
> > + *
> > + * Allocation layout:
> > + * - struct virtio_i2c_xfer xfer
> > + * - struct virtio_i2c_req reqs[num]
> > + * - padding to dma_get_cache_alignment()
> > + * - u8 bounce_buf[virtio_i2c_bounce_size(msgs[0].len)]
> > + * ...
> > + * - u8 bounce_buf[virtio_i2c_bounce_size(msgs[num-1].len)]
> > + */
> > +struct virtio_i2c_xfer {
> > + struct kref ref;
> > + u8 *bounce_buf_base;
> > + struct virtio_i2c_req reqs[];
> > +};
> > +
> > +static size_t virtio_i2c_bounce_size(unsigned int len)
> > +{
> > + return ALIGN(len, dma_get_cache_alignment());
> > +}
> > +
> > +static void virtio_i2c_xfer_release(struct kref *ref)
> > +{
> > + struct virtio_i2c_xfer *xfer = container_of(ref, struct virtio_i2c_xfer, ref);
> > + kfree(xfer);
> > +}
> > +
> > static void virtio_i2c_msg_done(struct virtqueue *vq)
> > {
> > struct virtio_i2c_req *req;
> > unsigned int len;
> >
> > - while ((req = virtqueue_get_buf(vq, &len)))
> > + while ((req = virtqueue_get_buf(vq, &len))) {
> > complete(&req->completion);
> > + kref_put(&req->xfer->ref, virtio_i2c_xfer_release);
> > + }
> > }
> >
> > -static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > - struct virtio_i2c_req *reqs,
> > +static int virtio_i2c_prepare_xfer(struct virtqueue *vq,
> > + struct virtio_i2c_xfer *xfer,
> > struct i2c_msg *msgs, int num)
> > {
> > struct scatterlist *sgs[3], out_hdr, msg_buf, in_hdr;
> > + struct virtio_i2c_req *reqs = xfer->reqs;
> > + u8 *bounce_buf = xfer->bounce_buf_base;
> > int i;
> >
> > for (i = 0; i < num; i++) {
> > int outcnt = 0, incnt = 0;
> >
> > + reqs[i].xfer = xfer;
> > + reqs[i].read = !!(msgs[i].flags & I2C_M_RD);
> > init_completion(&reqs[i].completion);
> >
> > /*
> > @@ -82,23 +126,31 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > sgs[outcnt++] = &out_hdr;
> >
> > if (msgs[i].len) {
> > - reqs[i].buf = i2c_get_dma_safe_msg_buf(&msgs[i], 1);
> > - if (!reqs[i].buf)
> > - break;
> > + /*
> > + * Even if msg->flags has I2C_M_DMA_SAFE set, a bounce
> > + * buffer is required because the transfer may be
> > + * interrupted, after which msg->buf is no longer valid.
> > + */
> > + if (!(msgs[i].flags & I2C_M_RD))
> > + memcpy(bounce_buf, msgs[i].buf, msgs[i].len);
> >
> > - sg_init_one(&msg_buf, reqs[i].buf, msgs[i].len);
> > + sg_init_one(&msg_buf, bounce_buf, msgs[i].len);
> >
> > if (msgs[i].flags & I2C_M_RD)
> > sgs[outcnt + incnt++] = &msg_buf;
> > else
> > sgs[outcnt++] = &msg_buf;
> > }
> > + bounce_buf += virtio_i2c_bounce_size(msgs[i].len);
> >
> > sg_init_one(&in_hdr, &reqs[i].in_hdr, sizeof(reqs[i].in_hdr));
> > sgs[outcnt + incnt++] = &in_hdr;
> >
> > + /* This reference is released in virtio_i2c_msg_done(). */
> > + kref_get(&xfer->ref);
> > +
> > if (virtqueue_add_sgs(vq, sgs, outcnt, incnt, &reqs[i], GFP_KERNEL)) {
> > - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], false);
> > + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> > break;
> > }
> > }
> > @@ -106,26 +158,38 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > return i;
> > }
> >
> > -static int virtio_i2c_complete_reqs(struct virtqueue *vq,
> > - struct virtio_i2c_req *reqs,
> > - struct i2c_msg *msgs, int num)
> > +static int virtio_i2c_complete_xfer(struct virtio_i2c_xfer *xfer,
> > + struct i2c_msg *msgs,
> > + int num)
> > {
> > + struct virtio_i2c_req *reqs = xfer->reqs;
> > + u8 *bounce_buf = xfer->bounce_buf_base;
> > bool failed = false;
> > int i, j = 0;
> >
> > for (i = 0; i < num; i++) {
> > struct virtio_i2c_req *req = &reqs[i];
> > + struct i2c_msg *msg = &msgs[i];
> > +
> > + if (wait_for_completion_interruptible(&req->completion))
> > + return -EINTR;
> > +
> > + if (req->in_hdr.status != VIRTIO_I2C_MSG_OK) {
> > + /*
> > + * Don't break yet. Try to wait until all requests
> > + * complete to ensure that the virtqueue has enough
> > + * descriptor slots for the next transfer.
> > + */
> > + failed = true;
> > + }
> >
> > if (!failed) {
> > - if (wait_for_completion_interruptible(&req->completion))
> > - failed = true;
> > - else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> > - failed = true;
> > - else
> > - j++;
> > + if (req->read)
> > + memcpy(msg->buf, bounce_buf, msg->len);
> > + j++;
> > }
> >
> > - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
> > + bounce_buf += virtio_i2c_bounce_size(msg->len);
> > }
> >
> > return j;
> > @@ -136,14 +200,31 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> > {
> > struct virtio_i2c *vi = i2c_get_adapdata(adap);
> > struct virtqueue *vq = vi->vq;
> > - struct virtio_i2c_req *reqs;
> > - int count;
> > + struct virtio_i2c_xfer *xfer;
> > + size_t alloc_size;
> > + int i, count;
> > +
> > + alloc_size = struct_size(xfer, reqs, num);
> > + if (check_add_overflow(alloc_size,
> > + dma_get_cache_alignment() - 1,
> > + &alloc_size)) /* padding for PTR_ALIGN() */
> > + return -EOVERFLOW;
> > + for (i = 0; i < num; i++) {
> > + if (check_add_overflow(alloc_size,
> > + virtio_i2c_bounce_size(msgs[i].len),
> > + &alloc_size))
> > + return -EOVERFLOW;
> > + }
> >
> > - reqs = kzalloc_objs(*reqs, num);
> > - if (!reqs)
> > + xfer = kzalloc(alloc_size, GFP_KERNEL);
> > + if (!xfer)
> > return -ENOMEM;
> >
> > - count = virtio_i2c_prepare_reqs(vq, reqs, msgs, num);
> > + kref_init(&xfer->ref);
> > + xfer->bounce_buf_base = PTR_ALIGN((u8 *)(xfer->reqs + num),
> > + dma_get_cache_alignment());
> > +
> > + count = virtio_i2c_prepare_xfer(vq, xfer, msgs, num);
> > if (!count)
> > goto err_free;
> >
> > @@ -157,10 +238,10 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> > */
> > virtqueue_kick(vq);
> >
> > - count = virtio_i2c_complete_reqs(vq, reqs, msgs, count);
> > + count = virtio_i2c_complete_xfer(xfer, msgs, count);
> >
> > err_free:
> > - kfree(reqs);
> > + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> > return count;
> > }
> >
> > --
> > 2.54.0
>
^ permalink raw reply
* Re: [PATCH] 9p/trans_virtio: bound mount_tag show copy to one page
From: Christian Schoenebeck @ 2026-06-10 15:46 UTC (permalink / raw)
To: Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Michael Bommarito
Cc: v9fs, virtualization, linux-kernel, stable
In-Reply-To: <20260610114206.3749904-1-michael.bommarito@gmail.com>
On Wednesday, 10 June 2026 13:42:06 CEST Michael Bommarito wrote:
> p9_mount_tag_show() copies strlen(chan->tag) + 1 bytes into the
> single-page buffer the sysfs core provides, with no upper bound. The
> mount tag length comes from virtio_9p_config.tag_len, a 16-bit field read
> from the device at probe in p9_virtio_probe() with no cap. Under the
> confidential-computing threat model, where the guest does not trust the
> host, a malicious or compromised host can present a 65535-byte tag with
> no embedded NUL. A read of the world-readable /sys/.../mount_tag
> attribute (udev reads it at probe) then copies ~64 KiB into the 4 KiB
> sysfs page, a slab-out-of-bounds write of host-controlled content.
>
> Bound the copy to the page size in the show handler.
>
> Fixes: 179a5bc4b8cb ("net/9p: use memcpy() instead of snprintf() in
> p9_mount_tag_show()") Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> net/9p/trans_virtio.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
> index 4cdab7094b273..b62aa7b309f1c 100644
> --- a/net/9p/trans_virtio.c
> +++ b/net/9p/trans_virtio.c
> @@ -573,7 +573,11 @@ static ssize_t p9_mount_tag_show(struct device *dev,
> chan = vdev->priv;
> tag_len = strlen(chan->tag);
>
> - memcpy(buf, chan->tag, tag_len + 1);
> + if (tag_len > PAGE_SIZE - 2)
> + tag_len = PAGE_SIZE - 2;
> +
> + memcpy(buf, chan->tag, tag_len);
> + buf[tag_len] = '\0';
>
> return tag_len + 1;
> }
Given that this has already seen some rotations, it is worth to think a bit
more about it:
1. Destination buffer being PAGE_SIZE large is an implementation detail of
seq_file. If the latter is changed, this code breaks (again) silently without
anybody noticing.
2. memcpy() was introduced by 179a5bc4b8cb because chan->tag was not NULL
terminated. However since edcd9d977354 it *is* NULL terminated.
Given those two, an alternative would be:
len = sysfs_emit(buf, "%s", chan->tag);
As it already handles the PAGE_SIZE limit inside its implementation.
However, still ...
3. Is it a good idea to just *silenty* truncate a very long tag to something
else just to avoid an OOB? It would at least break auto mount rules, as the
truncated tag would not match device's real tag.
4. And most importantly: Would a sane 9p device *ever* use a 64k tag, if yes
what for?
I would say no, a 64k tag is at least suspicious, if not even hostile.
Therefore: what about simply rejecting the device at probe time if its tag is
beyond a certain length?
/Christian
^ permalink raw reply
* Re: [PATCH v2] mm: page_reporting: allow driver to set batch capacity
From: Zi Yan @ 2026-06-10 16:21 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Miaohe Lin, David Hildenbrand (Arm), Jason Wang,
Xuan Zhuo, Eugenio Perez, Muchun Song, Oscar Salvador,
Andrew Morton, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Baolin Wang, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Hugh Dickins, Matthew Brost, Joshua Hahn,
Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi,
Alexander Duyck
In-Reply-To: <cb43adc61d2ed3069b2fe428f3e051dbdc4cc28d.1781097156.git.mst@redhat.com>
On 10 Jun 2026, at 9:17, Michael S. Tsirkin wrote:
> At the moment, if a virtio balloon device has a page reporting vq but
> its size is < PAGE_REPORTING_CAPACITY (32), the balloon driver fails
> probe.
>
> But, there's no way for host to know this value, so it can easily
> create a smaller vq and suddenly adding the reporting capability
> to the device makes all of the driver fail. Not pretty.
>
> Add a capacity field to page_reporting_dev_info so drivers can
> control the maximum number of pages per report batch.
>
> In virtio-balloon, set the capacity to the reporting virtqueue size,
> letting page_reporting adapt to whatever the device provides.
>
> Capacity need not be a power of two. Code previously called out
> division by PAGE_REPORTING_CAPACITY as cheap since it was a power
> of 2, but no performance difference was observed with non-power-of-2
> values.
>
> If capacity is 0 or exceeds PAGE_REPORTING_CAPACITY, it defaults
> to PAGE_REPORTING_CAPACITY. The 0 check and the clamping is done in
> page_reporting_register(), before the reporting work is scheduled,
> so we never get division by 0.
>
> Fixes: b0c504f15471 ("virtio-balloon: add support for providing free page reports to host")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> ---
> Changes v1->v2:
> - Document capacity=0 as default in commit log
> - Document that capacity need not be a power of two
> - Drop unnecessary comment about integer division cost
> - Update comment on capacity field: "0 (default) means PAGE_REPORTING_CAPACITY"
>
> drivers/virtio/virtio_balloon.c | 5 +----
> include/linux/page_reporting.h | 3 +++
> mm/page_reporting.c | 24 ++++++++++++------------
> 3 files changed, 16 insertions(+), 16 deletions(-)
>
LGTM. With David's concern addressed, feel free to add:
Acked-by: Zi Yan <ziy@nvidia.com>
--
Best Regards,
Yan, Zi
^ permalink raw reply
* Re: [PATCH v5] i2c: virtio: retain xfer with kref to fix UAF on interrupted wait
From: Michael S. Tsirkin @ 2026-06-10 16:07 UTC (permalink / raw)
To: Gavin Li
Cc: linux-i2c, viresh.kumar, Chen, Jian Jun, andi.shyti,
virtualization
In-Reply-To: <20260610155834.79207-1-gavin.li@samsara.com>
On Wed, Jun 10, 2026 at 11:58:34AM -0400, Gavin Li wrote:
> commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
> completion wait") switched virtio_i2c_complete_reqs() to
> wait_for_completion_interruptible() so a stuck device cannot hang a
> task forever. That left a use-after-free: if the wait returns early on
> a signal, virtio_i2c_xfer() frees reqs and DMA bounce buffers while the
> device may still hold virtqueue tokens pointing at &reqs[i] and DMA
> into read buffers. When those requests complete later,
> virtio_i2c_msg_done() calls complete() on freed memory.
>
> Waiting uninterruptibly for every completion before freeing avoids the
> UAF but can hang the caller indefinitely if the virtio side never
> completes the request. Performing a queue reset / device reset is
> possible in this scenario, but adds complexity.
>
> This commit manages the freeing of the xfer allocations via kref, and
> ensures that each in-flight request holds a reference. This fixes the
> use-after-free by ensuring that the virtio device has a valid location
> to write to until the request completes. This will cause a memory leak
> in cases where the device hangs, but that is much preferable to memory
> corruption.
>
> Additionally, force usage of a bounce buffer even if the i2c_msg buf is
> DMA-safe, since the buffer passed to the virtqueue must remain valid
> even if the transfer is interrupted. Remove usage of
> i2c_get_dma_safe_msg_buf() since it may pass through msg->buf directly.
> All bounce buffers are part of the single xfer allocation, so there is
> no additional allocation overhead.
>
> Signed-off-by: Gavin Li <gavin.li@samsara.com>
But maybe, if queue reset is supported, we could use that?
Is that problematic?
And maybe fallback on device reset on interrupt even?
> ---
>
> Changes in v5:
> - DMA-align all bounce buffers
>
> Changes in v4:
> - Pack bounce buffers into a single allocation after reqs[]
> - Remove per-request buf pointer and xfer->num
> - Remove req.msg, track message direction with req->read
> - Simplify xfer release to a single kfree()
> - Restore using j to track successful transfers in _complete_xfer()
> ---
> drivers/i2c/busses/i2c-virtio.c | 135 +++++++++++++++++++++++++-------
> 1 file changed, 108 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> index 5da6fef92bec3..a5602865102d9 100644
> --- a/drivers/i2c/busses/i2c-virtio.c
> +++ b/drivers/i2c/busses/i2c-virtio.c
> @@ -10,10 +10,13 @@
>
> #include <linux/acpi.h>
> #include <linux/completion.h>
> +#include <linux/dma-mapping.h>
> #include <linux/err.h>
> #include <linux/i2c.h>
> #include <linux/kernel.h>
> +#include <linux/kref.h>
> #include <linux/module.h>
> +#include <linux/overflow.h>
> #include <linux/virtio.h>
> #include <linux/virtio_ids.h>
> #include <linux/virtio_config.h>
> @@ -31,39 +34,80 @@ struct virtio_i2c {
> struct virtqueue *vq;
> };
>
> +struct virtio_i2c_xfer;
> +
> /**
> * struct virtio_i2c_req - the virtio I2C request structure
> + * @xfer: owning transfer
> * @completion: completion of virtio I2C message
> + * @read: true if this is a read message (I2C_M_RD is set)
> * @out_hdr: the OUT header of the virtio I2C message
> - * @buf: the buffer into which data is read, or from which it's written
> * @in_hdr: the IN header of the virtio I2C message
> */
> struct virtio_i2c_req {
> + struct virtio_i2c_xfer *xfer;
> struct completion completion;
> + bool read;
> struct virtio_i2c_out_hdr out_hdr ____cacheline_aligned;
> - uint8_t *buf ____cacheline_aligned;
> struct virtio_i2c_in_hdr in_hdr ____cacheline_aligned;
> };
>
> +/**
> + * struct virtio_i2c_xfer - a queued I2C transfer
> + * @ref: one ref for the caller, plus one per in-flight virtqueue request
> + * @bounce_buf_base: start of bounce buffer region
> + * @reqs: the virtio I2C requests
> + *
> + * Allocation layout:
> + * - struct virtio_i2c_xfer xfer
> + * - struct virtio_i2c_req reqs[num]
> + * - padding to dma_get_cache_alignment()
> + * - u8 bounce_buf[virtio_i2c_bounce_size(msgs[0].len)]
> + * ...
> + * - u8 bounce_buf[virtio_i2c_bounce_size(msgs[num-1].len)]
> + */
> +struct virtio_i2c_xfer {
> + struct kref ref;
> + u8 *bounce_buf_base;
> + struct virtio_i2c_req reqs[];
> +};
> +
> +static size_t virtio_i2c_bounce_size(unsigned int len)
> +{
> + return ALIGN(len, dma_get_cache_alignment());
> +}
> +
> +static void virtio_i2c_xfer_release(struct kref *ref)
> +{
> + struct virtio_i2c_xfer *xfer = container_of(ref, struct virtio_i2c_xfer, ref);
> + kfree(xfer);
> +}
> +
> static void virtio_i2c_msg_done(struct virtqueue *vq)
> {
> struct virtio_i2c_req *req;
> unsigned int len;
>
> - while ((req = virtqueue_get_buf(vq, &len)))
> + while ((req = virtqueue_get_buf(vq, &len))) {
> complete(&req->completion);
> + kref_put(&req->xfer->ref, virtio_i2c_xfer_release);
> + }
> }
>
> -static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> - struct virtio_i2c_req *reqs,
> +static int virtio_i2c_prepare_xfer(struct virtqueue *vq,
> + struct virtio_i2c_xfer *xfer,
> struct i2c_msg *msgs, int num)
> {
> struct scatterlist *sgs[3], out_hdr, msg_buf, in_hdr;
> + struct virtio_i2c_req *reqs = xfer->reqs;
> + u8 *bounce_buf = xfer->bounce_buf_base;
> int i;
>
> for (i = 0; i < num; i++) {
> int outcnt = 0, incnt = 0;
>
> + reqs[i].xfer = xfer;
> + reqs[i].read = !!(msgs[i].flags & I2C_M_RD);
> init_completion(&reqs[i].completion);
>
> /*
> @@ -82,23 +126,31 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> sgs[outcnt++] = &out_hdr;
>
> if (msgs[i].len) {
> - reqs[i].buf = i2c_get_dma_safe_msg_buf(&msgs[i], 1);
> - if (!reqs[i].buf)
> - break;
> + /*
> + * Even if msg->flags has I2C_M_DMA_SAFE set, a bounce
> + * buffer is required because the transfer may be
> + * interrupted, after which msg->buf is no longer valid.
> + */
> + if (!(msgs[i].flags & I2C_M_RD))
> + memcpy(bounce_buf, msgs[i].buf, msgs[i].len);
>
> - sg_init_one(&msg_buf, reqs[i].buf, msgs[i].len);
> + sg_init_one(&msg_buf, bounce_buf, msgs[i].len);
>
> if (msgs[i].flags & I2C_M_RD)
> sgs[outcnt + incnt++] = &msg_buf;
> else
> sgs[outcnt++] = &msg_buf;
> }
> + bounce_buf += virtio_i2c_bounce_size(msgs[i].len);
>
> sg_init_one(&in_hdr, &reqs[i].in_hdr, sizeof(reqs[i].in_hdr));
> sgs[outcnt + incnt++] = &in_hdr;
>
> + /* This reference is released in virtio_i2c_msg_done(). */
> + kref_get(&xfer->ref);
> +
> if (virtqueue_add_sgs(vq, sgs, outcnt, incnt, &reqs[i], GFP_KERNEL)) {
> - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], false);
> + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> break;
> }
> }
> @@ -106,26 +158,38 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> return i;
> }
>
> -static int virtio_i2c_complete_reqs(struct virtqueue *vq,
> - struct virtio_i2c_req *reqs,
> - struct i2c_msg *msgs, int num)
> +static int virtio_i2c_complete_xfer(struct virtio_i2c_xfer *xfer,
> + struct i2c_msg *msgs,
> + int num)
> {
> + struct virtio_i2c_req *reqs = xfer->reqs;
> + u8 *bounce_buf = xfer->bounce_buf_base;
> bool failed = false;
> int i, j = 0;
>
> for (i = 0; i < num; i++) {
> struct virtio_i2c_req *req = &reqs[i];
> + struct i2c_msg *msg = &msgs[i];
> +
> + if (wait_for_completion_interruptible(&req->completion))
> + return -EINTR;
> +
> + if (req->in_hdr.status != VIRTIO_I2C_MSG_OK) {
> + /*
> + * Don't break yet. Try to wait until all requests
> + * complete to ensure that the virtqueue has enough
> + * descriptor slots for the next transfer.
> + */
> + failed = true;
> + }
>
> if (!failed) {
> - if (wait_for_completion_interruptible(&req->completion))
> - failed = true;
> - else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> - failed = true;
> - else
> - j++;
> + if (req->read)
> + memcpy(msg->buf, bounce_buf, msg->len);
> + j++;
> }
>
> - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
> + bounce_buf += virtio_i2c_bounce_size(msg->len);
> }
>
> return j;
> @@ -136,14 +200,31 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> {
> struct virtio_i2c *vi = i2c_get_adapdata(adap);
> struct virtqueue *vq = vi->vq;
> - struct virtio_i2c_req *reqs;
> - int count;
> + struct virtio_i2c_xfer *xfer;
> + size_t alloc_size;
> + int i, count;
> +
> + alloc_size = struct_size(xfer, reqs, num);
> + if (check_add_overflow(alloc_size,
> + dma_get_cache_alignment() - 1,
> + &alloc_size)) /* padding for PTR_ALIGN() */
> + return -EOVERFLOW;
> + for (i = 0; i < num; i++) {
> + if (check_add_overflow(alloc_size,
> + virtio_i2c_bounce_size(msgs[i].len),
> + &alloc_size))
> + return -EOVERFLOW;
> + }
>
> - reqs = kzalloc_objs(*reqs, num);
> - if (!reqs)
> + xfer = kzalloc(alloc_size, GFP_KERNEL);
> + if (!xfer)
> return -ENOMEM;
>
> - count = virtio_i2c_prepare_reqs(vq, reqs, msgs, num);
> + kref_init(&xfer->ref);
> + xfer->bounce_buf_base = PTR_ALIGN((u8 *)(xfer->reqs + num),
> + dma_get_cache_alignment());
> +
> + count = virtio_i2c_prepare_xfer(vq, xfer, msgs, num);
> if (!count)
> goto err_free;
>
> @@ -157,10 +238,10 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> */
> virtqueue_kick(vq);
>
> - count = virtio_i2c_complete_reqs(vq, reqs, msgs, count);
> + count = virtio_i2c_complete_xfer(xfer, msgs, count);
>
> err_free:
> - kfree(reqs);
> + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> return count;
> }
>
> --
> 2.54.0
^ permalink raw reply
* Re: [PATCH v4] i2c: virtio: retain xfer with kref to fix UAF on interrupted wait
From: Gavin Li @ 2026-06-10 16:02 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-i2c, viresh.kumar, Chen, Jian Jun, andi.shyti,
virtualization
In-Reply-To: <20260610110532-mutt-send-email-mst@kernel.org>
Thanks for pointing that out.
The v5 patch fixes the DMA alignment. I've tested it with
CONFIG_DMA_API_DEBUG=y and CONFIG_DMA_API_DEBUG_SG=y
and everything seems to be working with no log warnings.
On Wed, Jun 10, 2026 at 11:08 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jun 10, 2026 at 11:02:32AM -0400, Gavin Li wrote:
> > commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
> > completion wait") switched virtio_i2c_complete_reqs() to
> > wait_for_completion_interruptible() so a stuck device cannot hang a
> > task forever. That left a use-after-free: if the wait returns early on
> > a signal, virtio_i2c_xfer() frees reqs and DMA bounce buffers while the
> > device may still hold virtqueue tokens pointing at &reqs[i] and DMA
> > into read buffers. When those requests complete later,
> > virtio_i2c_msg_done() calls complete() on freed memory.
> >
> > Waiting uninterruptibly for every completion before freeing avoids the
> > UAF but can hang the caller indefinitely if the virtio side never
> > completes the request. Performing a queue reset / device reset is
> > possible in this scenario, but adds complexity.
> >
> > This commit manages the freeing of the xfer allocations via kref, and
> > ensures that each in-flight request holds a reference. This fixes the
> > use-after-free by ensuring that the virtio device has a valid location
> > to write to until the request completes. This will cause a memory leak
> > in cases where the device hangs, but that is much preferable to memory
> > corruption.
> >
> > Additionally, force usage of a bounce buffer even if the i2c_msg buf is
> > DMA-safe, since the buffer passed to the virtqueue must remain valid
> > even if the transfer is interrupted. Remove usage of
> > i2c_get_dma_safe_msg_buf() since it may pass through msg->buf directly.
> > All bounce buffers are part of the single xfer allocation, so there is
> > no additional allocation overhead.
> >
> > Signed-off-by: Gavin Li <gavin.li@samsara.com>
> >
> > ---
> >
> > Changes in v4:
> > - Pack bounce buffers into a single allocation after reqs[]
> > - Remove per-request buf pointer and xfer->num
> > - Remove req.msg, track message direction with req->read
> > - Simplify xfer release to a single kfree()
> > - Restore using j to track successful transfers in _complete_xfer()
> > ---
> > drivers/i2c/busses/i2c-virtio.c | 121 +++++++++++++++++++++++++-------
> > 1 file changed, 94 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> > index 5da6fef92bec3..33b469ca39d4b 100644
> > --- a/drivers/i2c/busses/i2c-virtio.c
> > +++ b/drivers/i2c/busses/i2c-virtio.c
> > @@ -13,7 +13,9 @@
> > #include <linux/err.h>
> > #include <linux/i2c.h>
> > #include <linux/kernel.h>
> > +#include <linux/kref.h>
> > #include <linux/module.h>
> > +#include <linux/overflow.h>
> > #include <linux/virtio.h>
> > #include <linux/virtio_ids.h>
> > #include <linux/virtio_config.h>
> > @@ -31,39 +33,74 @@ struct virtio_i2c {
> > struct virtqueue *vq;
> > };
> >
> > +struct virtio_i2c_xfer;
> > +
> > /**
> > * struct virtio_i2c_req - the virtio I2C request structure
> > + * @xfer: owning transfer
> > * @completion: completion of virtio I2C message
> > + * @read: true if this is a read message (I2C_M_RD is set)
> > * @out_hdr: the OUT header of the virtio I2C message
> > - * @buf: the buffer into which data is read, or from which it's written
> > * @in_hdr: the IN header of the virtio I2C message
> > */
> > struct virtio_i2c_req {
> > + struct virtio_i2c_xfer *xfer;
> > struct completion completion;
> > + bool read;
> > struct virtio_i2c_out_hdr out_hdr ____cacheline_aligned;
> > - uint8_t *buf ____cacheline_aligned;
> > struct virtio_i2c_in_hdr in_hdr ____cacheline_aligned;
> > };
> >
> > +/**
> > + * struct virtio_i2c_xfer - a queued I2C transfer
> > + * @ref: one ref for the caller, plus one per in-flight virtqueue request
> > + * @bounce_buf_base: start of bounce buffer region
> > + * @reqs: the virtio I2C requests
> > + *
> > + * Allocation layout:
> > + * - struct virtio_i2c_xfer xfer
> > + * - struct virtio_i2c_req reqs[num]
> > + * - u8 bounce_buf[msgs[0].len]
> > + * ...
> > + * - u8 bounce_buf[msgs[num-1].len]
>
> You seems to be ignoring DMA alignment requirements here.
> Did you try running with DMA debugging enabled?
>
>
> > + */
> > +struct virtio_i2c_xfer {
> > + struct kref ref;
> > + u8 *bounce_buf_base;
> > + struct virtio_i2c_req reqs[];
> > +};
> > +
> > +static void virtio_i2c_xfer_release(struct kref *ref)
> > +{
> > + struct virtio_i2c_xfer *xfer = container_of(ref, struct virtio_i2c_xfer, ref);
> > + kfree(xfer);
> > +}
> > +
> > static void virtio_i2c_msg_done(struct virtqueue *vq)
> > {
> > struct virtio_i2c_req *req;
> > unsigned int len;
> >
> > - while ((req = virtqueue_get_buf(vq, &len)))
> > + while ((req = virtqueue_get_buf(vq, &len))) {
> > complete(&req->completion);
> > + kref_put(&req->xfer->ref, virtio_i2c_xfer_release);
> > + }
> > }
> >
> > -static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > - struct virtio_i2c_req *reqs,
> > +static int virtio_i2c_prepare_xfer(struct virtqueue *vq,
> > + struct virtio_i2c_xfer *xfer,
> > struct i2c_msg *msgs, int num)
> > {
> > struct scatterlist *sgs[3], out_hdr, msg_buf, in_hdr;
> > + struct virtio_i2c_req *reqs = xfer->reqs;
> > + u8 *bounce_buf = xfer->bounce_buf_base;
> > int i;
> >
> > for (i = 0; i < num; i++) {
> > int outcnt = 0, incnt = 0;
> >
> > + reqs[i].xfer = xfer;
> > + reqs[i].read = !!(msgs[i].flags & I2C_M_RD);
> > init_completion(&reqs[i].completion);
> >
> > /*
> > @@ -82,23 +119,31 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > sgs[outcnt++] = &out_hdr;
> >
> > if (msgs[i].len) {
> > - reqs[i].buf = i2c_get_dma_safe_msg_buf(&msgs[i], 1);
> > - if (!reqs[i].buf)
> > - break;
> > + /*
> > + * Even if msg->flags has I2C_M_DMA_SAFE set, a bounce
> > + * buffer is required because the transfer may be
> > + * interrupted, after which msg->buf is no longer valid.
> > + */
> > + if (!(msgs[i].flags & I2C_M_RD))
> > + memcpy(bounce_buf, msgs[i].buf, msgs[i].len);
> >
> > - sg_init_one(&msg_buf, reqs[i].buf, msgs[i].len);
> > + sg_init_one(&msg_buf, bounce_buf, msgs[i].len);
> >
> > if (msgs[i].flags & I2C_M_RD)
> > sgs[outcnt + incnt++] = &msg_buf;
> > else
> > sgs[outcnt++] = &msg_buf;
> > }
> > + bounce_buf += msgs[i].len;
> >
> > sg_init_one(&in_hdr, &reqs[i].in_hdr, sizeof(reqs[i].in_hdr));
> > sgs[outcnt + incnt++] = &in_hdr;
> >
> > + /* This reference is released in virtio_i2c_msg_done(). */
> > + kref_get(&xfer->ref);
> > +
> > if (virtqueue_add_sgs(vq, sgs, outcnt, incnt, &reqs[i], GFP_KERNEL)) {
> > - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], false);
> > + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> > break;
> > }
> > }
> > @@ -106,26 +151,38 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> > return i;
> > }
> >
> > -static int virtio_i2c_complete_reqs(struct virtqueue *vq,
> > - struct virtio_i2c_req *reqs,
> > - struct i2c_msg *msgs, int num)
> > +static int virtio_i2c_complete_xfer(struct virtio_i2c_xfer *xfer,
> > + struct i2c_msg *msgs,
> > + int num)
> > {
> > + struct virtio_i2c_req *reqs = xfer->reqs;
> > + u8 *bounce_buf = xfer->bounce_buf_base;
> > bool failed = false;
> > int i, j = 0;
> >
> > for (i = 0; i < num; i++) {
> > struct virtio_i2c_req *req = &reqs[i];
> > + struct i2c_msg *msg = &msgs[i];
> > +
> > + if (wait_for_completion_interruptible(&req->completion))
> > + return -EINTR;
> > +
> > + if (req->in_hdr.status != VIRTIO_I2C_MSG_OK) {
> > + /*
> > + * Don't break yet. Try to wait until all requests
> > + * complete to ensure that the virtqueue has enough
> > + * descriptor slots for the next transfer.
> > + */
> > + failed = true;
> > + }
> >
> > if (!failed) {
> > - if (wait_for_completion_interruptible(&req->completion))
> > - failed = true;
> > - else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> > - failed = true;
> > - else
> > - j++;
> > + if (req->read)
> > + memcpy(msg->buf, bounce_buf, msg->len);
> > + j++;
> > }
> >
> > - i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
> > + bounce_buf += msg->len;
> > }
> >
> > return j;
> > @@ -136,14 +193,24 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> > {
> > struct virtio_i2c *vi = i2c_get_adapdata(adap);
> > struct virtqueue *vq = vi->vq;
> > - struct virtio_i2c_req *reqs;
> > - int count;
> > + struct virtio_i2c_xfer *xfer;
> > + size_t alloc_size;
> > + int i, count;
> >
> > - reqs = kzalloc_objs(*reqs, num);
> > - if (!reqs)
> > + alloc_size = struct_size(xfer, reqs, num);
> > + for (i = 0; i < num; i++) {
> > + if (check_add_overflow(alloc_size, msgs[i].len, &alloc_size))
> > + return -EOVERFLOW;
> > + }
> > +
> > + xfer = kzalloc(alloc_size, GFP_KERNEL);
> > + if (!xfer)
> > return -ENOMEM;
> >
> > - count = virtio_i2c_prepare_reqs(vq, reqs, msgs, num);
> > + kref_init(&xfer->ref);
> > + xfer->bounce_buf_base = (u8 *)(xfer->reqs + num);
> > +
> > + count = virtio_i2c_prepare_xfer(vq, xfer, msgs, num);
> > if (!count)
> > goto err_free;
> >
> > @@ -157,10 +224,10 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> > */
> > virtqueue_kick(vq);
> >
> > - count = virtio_i2c_complete_reqs(vq, reqs, msgs, count);
> > + count = virtio_i2c_complete_xfer(xfer, msgs, count);
> >
> > err_free:
> > - kfree(reqs);
> > + kref_put(&xfer->ref, virtio_i2c_xfer_release);
> > return count;
> > }
> >
> > --
> > 2.54.0
>
^ permalink raw reply
* Re: [PATCH] 9p/trans_virtio: bound mount_tag show copy to one page
From: Michael Bommarito @ 2026-06-10 16:00 UTC (permalink / raw)
To: Christian Schoenebeck
Cc: Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet, v9fs,
virtualization, linux-kernel, stable
In-Reply-To: <1962500.CQOukoFCf9@weasel>
On Wed, Jun 10, 2026 at 11:46 AM Christian Schoenebeck
<linux_oss@crudebyte.com> wrote:
> I would say no, a 64k tag is at least suspicious, if not even hostile.
>
> Therefore: what about simply rejecting the device at probe time if its tag is
> beyond a certain length?
I think that's much cleaner if the user base really can't think of any
legitimate purpose here (or in userspace utils).
FWIW, on a similar Xen issue, we're also talking about blacklisting,
which is one step even further.
Thanks,
Mike
^ permalink raw reply
* [PATCH v5] i2c: virtio: retain xfer with kref to fix UAF on interrupted wait
From: Gavin Li @ 2026-06-10 15:58 UTC (permalink / raw)
To: linux-i2c, viresh.kumar, Michael S. Tsirkin
Cc: Chen, Jian Jun, andi.shyti, virtualization, Gavin Li
commit a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
completion wait") switched virtio_i2c_complete_reqs() to
wait_for_completion_interruptible() so a stuck device cannot hang a
task forever. That left a use-after-free: if the wait returns early on
a signal, virtio_i2c_xfer() frees reqs and DMA bounce buffers while the
device may still hold virtqueue tokens pointing at &reqs[i] and DMA
into read buffers. When those requests complete later,
virtio_i2c_msg_done() calls complete() on freed memory.
Waiting uninterruptibly for every completion before freeing avoids the
UAF but can hang the caller indefinitely if the virtio side never
completes the request. Performing a queue reset / device reset is
possible in this scenario, but adds complexity.
This commit manages the freeing of the xfer allocations via kref, and
ensures that each in-flight request holds a reference. This fixes the
use-after-free by ensuring that the virtio device has a valid location
to write to until the request completes. This will cause a memory leak
in cases where the device hangs, but that is much preferable to memory
corruption.
Additionally, force usage of a bounce buffer even if the i2c_msg buf is
DMA-safe, since the buffer passed to the virtqueue must remain valid
even if the transfer is interrupted. Remove usage of
i2c_get_dma_safe_msg_buf() since it may pass through msg->buf directly.
All bounce buffers are part of the single xfer allocation, so there is
no additional allocation overhead.
Signed-off-by: Gavin Li <gavin.li@samsara.com>
---
Changes in v5:
- DMA-align all bounce buffers
Changes in v4:
- Pack bounce buffers into a single allocation after reqs[]
- Remove per-request buf pointer and xfer->num
- Remove req.msg, track message direction with req->read
- Simplify xfer release to a single kfree()
- Restore using j to track successful transfers in _complete_xfer()
---
drivers/i2c/busses/i2c-virtio.c | 135 +++++++++++++++++++++++++-------
1 file changed, 108 insertions(+), 27 deletions(-)
diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
index 5da6fef92bec3..a5602865102d9 100644
--- a/drivers/i2c/busses/i2c-virtio.c
+++ b/drivers/i2c/busses/i2c-virtio.c
@@ -10,10 +10,13 @@
#include <linux/acpi.h>
#include <linux/completion.h>
+#include <linux/dma-mapping.h>
#include <linux/err.h>
#include <linux/i2c.h>
#include <linux/kernel.h>
+#include <linux/kref.h>
#include <linux/module.h>
+#include <linux/overflow.h>
#include <linux/virtio.h>
#include <linux/virtio_ids.h>
#include <linux/virtio_config.h>
@@ -31,39 +34,80 @@ struct virtio_i2c {
struct virtqueue *vq;
};
+struct virtio_i2c_xfer;
+
/**
* struct virtio_i2c_req - the virtio I2C request structure
+ * @xfer: owning transfer
* @completion: completion of virtio I2C message
+ * @read: true if this is a read message (I2C_M_RD is set)
* @out_hdr: the OUT header of the virtio I2C message
- * @buf: the buffer into which data is read, or from which it's written
* @in_hdr: the IN header of the virtio I2C message
*/
struct virtio_i2c_req {
+ struct virtio_i2c_xfer *xfer;
struct completion completion;
+ bool read;
struct virtio_i2c_out_hdr out_hdr ____cacheline_aligned;
- uint8_t *buf ____cacheline_aligned;
struct virtio_i2c_in_hdr in_hdr ____cacheline_aligned;
};
+/**
+ * struct virtio_i2c_xfer - a queued I2C transfer
+ * @ref: one ref for the caller, plus one per in-flight virtqueue request
+ * @bounce_buf_base: start of bounce buffer region
+ * @reqs: the virtio I2C requests
+ *
+ * Allocation layout:
+ * - struct virtio_i2c_xfer xfer
+ * - struct virtio_i2c_req reqs[num]
+ * - padding to dma_get_cache_alignment()
+ * - u8 bounce_buf[virtio_i2c_bounce_size(msgs[0].len)]
+ * ...
+ * - u8 bounce_buf[virtio_i2c_bounce_size(msgs[num-1].len)]
+ */
+struct virtio_i2c_xfer {
+ struct kref ref;
+ u8 *bounce_buf_base;
+ struct virtio_i2c_req reqs[];
+};
+
+static size_t virtio_i2c_bounce_size(unsigned int len)
+{
+ return ALIGN(len, dma_get_cache_alignment());
+}
+
+static void virtio_i2c_xfer_release(struct kref *ref)
+{
+ struct virtio_i2c_xfer *xfer = container_of(ref, struct virtio_i2c_xfer, ref);
+ kfree(xfer);
+}
+
static void virtio_i2c_msg_done(struct virtqueue *vq)
{
struct virtio_i2c_req *req;
unsigned int len;
- while ((req = virtqueue_get_buf(vq, &len)))
+ while ((req = virtqueue_get_buf(vq, &len))) {
complete(&req->completion);
+ kref_put(&req->xfer->ref, virtio_i2c_xfer_release);
+ }
}
-static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
- struct virtio_i2c_req *reqs,
+static int virtio_i2c_prepare_xfer(struct virtqueue *vq,
+ struct virtio_i2c_xfer *xfer,
struct i2c_msg *msgs, int num)
{
struct scatterlist *sgs[3], out_hdr, msg_buf, in_hdr;
+ struct virtio_i2c_req *reqs = xfer->reqs;
+ u8 *bounce_buf = xfer->bounce_buf_base;
int i;
for (i = 0; i < num; i++) {
int outcnt = 0, incnt = 0;
+ reqs[i].xfer = xfer;
+ reqs[i].read = !!(msgs[i].flags & I2C_M_RD);
init_completion(&reqs[i].completion);
/*
@@ -82,23 +126,31 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
sgs[outcnt++] = &out_hdr;
if (msgs[i].len) {
- reqs[i].buf = i2c_get_dma_safe_msg_buf(&msgs[i], 1);
- if (!reqs[i].buf)
- break;
+ /*
+ * Even if msg->flags has I2C_M_DMA_SAFE set, a bounce
+ * buffer is required because the transfer may be
+ * interrupted, after which msg->buf is no longer valid.
+ */
+ if (!(msgs[i].flags & I2C_M_RD))
+ memcpy(bounce_buf, msgs[i].buf, msgs[i].len);
- sg_init_one(&msg_buf, reqs[i].buf, msgs[i].len);
+ sg_init_one(&msg_buf, bounce_buf, msgs[i].len);
if (msgs[i].flags & I2C_M_RD)
sgs[outcnt + incnt++] = &msg_buf;
else
sgs[outcnt++] = &msg_buf;
}
+ bounce_buf += virtio_i2c_bounce_size(msgs[i].len);
sg_init_one(&in_hdr, &reqs[i].in_hdr, sizeof(reqs[i].in_hdr));
sgs[outcnt + incnt++] = &in_hdr;
+ /* This reference is released in virtio_i2c_msg_done(). */
+ kref_get(&xfer->ref);
+
if (virtqueue_add_sgs(vq, sgs, outcnt, incnt, &reqs[i], GFP_KERNEL)) {
- i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], false);
+ kref_put(&xfer->ref, virtio_i2c_xfer_release);
break;
}
}
@@ -106,26 +158,38 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
return i;
}
-static int virtio_i2c_complete_reqs(struct virtqueue *vq,
- struct virtio_i2c_req *reqs,
- struct i2c_msg *msgs, int num)
+static int virtio_i2c_complete_xfer(struct virtio_i2c_xfer *xfer,
+ struct i2c_msg *msgs,
+ int num)
{
+ struct virtio_i2c_req *reqs = xfer->reqs;
+ u8 *bounce_buf = xfer->bounce_buf_base;
bool failed = false;
int i, j = 0;
for (i = 0; i < num; i++) {
struct virtio_i2c_req *req = &reqs[i];
+ struct i2c_msg *msg = &msgs[i];
+
+ if (wait_for_completion_interruptible(&req->completion))
+ return -EINTR;
+
+ if (req->in_hdr.status != VIRTIO_I2C_MSG_OK) {
+ /*
+ * Don't break yet. Try to wait until all requests
+ * complete to ensure that the virtqueue has enough
+ * descriptor slots for the next transfer.
+ */
+ failed = true;
+ }
if (!failed) {
- if (wait_for_completion_interruptible(&req->completion))
- failed = true;
- else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
- failed = true;
- else
- j++;
+ if (req->read)
+ memcpy(msg->buf, bounce_buf, msg->len);
+ j++;
}
- i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
+ bounce_buf += virtio_i2c_bounce_size(msg->len);
}
return j;
@@ -136,14 +200,31 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
{
struct virtio_i2c *vi = i2c_get_adapdata(adap);
struct virtqueue *vq = vi->vq;
- struct virtio_i2c_req *reqs;
- int count;
+ struct virtio_i2c_xfer *xfer;
+ size_t alloc_size;
+ int i, count;
+
+ alloc_size = struct_size(xfer, reqs, num);
+ if (check_add_overflow(alloc_size,
+ dma_get_cache_alignment() - 1,
+ &alloc_size)) /* padding for PTR_ALIGN() */
+ return -EOVERFLOW;
+ for (i = 0; i < num; i++) {
+ if (check_add_overflow(alloc_size,
+ virtio_i2c_bounce_size(msgs[i].len),
+ &alloc_size))
+ return -EOVERFLOW;
+ }
- reqs = kzalloc_objs(*reqs, num);
- if (!reqs)
+ xfer = kzalloc(alloc_size, GFP_KERNEL);
+ if (!xfer)
return -ENOMEM;
- count = virtio_i2c_prepare_reqs(vq, reqs, msgs, num);
+ kref_init(&xfer->ref);
+ xfer->bounce_buf_base = PTR_ALIGN((u8 *)(xfer->reqs + num),
+ dma_get_cache_alignment());
+
+ count = virtio_i2c_prepare_xfer(vq, xfer, msgs, num);
if (!count)
goto err_free;
@@ -157,10 +238,10 @@ static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
*/
virtqueue_kick(vq);
- count = virtio_i2c_complete_reqs(vq, reqs, msgs, count);
+ count = virtio_i2c_complete_xfer(xfer, msgs, count);
err_free:
- kfree(reqs);
+ kref_put(&xfer->ref, virtio_i2c_xfer_release);
return count;
}
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v3 3/4] block: drop shared-tag fairness throttling
From: Sumit Saxena @ 2026-06-10 15:46 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Martin K . Petersen, Jens Axboe, James E . J . Bottomley,
linux-scsi, linux-block, Adam Radford, Khalid Aziz,
Adaptec OEM Raid Solutions, Matthew Wilcox, Hannes Reinecke,
Juergen E . Fischer, Russell King, linux-arm-kernel, Finn Thain,
Michael Schmitz, Anil Gurumurthy, Sudarsana Kalluru,
Oliver Neukum, Ali Akcaagac, Jamie Lenehan, Ram Vegesna,
target-devel, Bradley Grove, Satish Kharat, Sesidhar Baddela,
Karan Tilak Kumar, Yihang Li, Don Brace, storagedev,
HighPoint Linux Team, Tyrel Datwyler, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy, linuxppc-dev,
Brian King, Lee Duncan, Chris Leech, Mike Christie, open-iscsi,
Justin Tee, Paul Ely, Kashyap Desai, Shivasharan S,
Chandrakanth Patil, megaraidlinux.pdl, Sathya Prakash Veerichetty,
Sreekanth Reddy, mpi3mr-linuxdrv.pdl, Suganath Prabu Subramani,
Ranjan Kumar, MPT-FusionLinux.pdl, Daniel Palmer, GOTO Masanori,
YOKOTA Hiroshi, Jack Wang, Geoff Levand, Michael Reed,
Nilesh Javali, GR-QLogic-Storage-Upstream, Narsimhulu Musini,
K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
linux-hyperv, Michael S . Tsirkin, Jason Wang, Paolo Bonzini,
Stefan Hajnoczi, Eugenio Perez, virtualization, Vishal Bhakta,
bcm-kernel-feedback-list, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, xen-devel, Bart Van Assche
In-Reply-To: <aikAs4X-2NWTuwCc@infradead.org>
[-- Attachment #1.1: Type: text/plain, Size: 1889 bytes --]
On Wed, Jun 10, 2026 at 11:44 AM Christoph Hellwig <hch@infradead.org>
wrote:
>
> Just dropping the fairness was rejected before and there is no
> explanation here on why any of that has changed.
I missed the fact that this patch had been previously rejected.
The motivation for this change stems from performance issue we
encountered due to false sharing of the 'nr_active_requests_shared_tags'
counter
on certain CPU architectures. I initially submitted a patch to move that
counter to
its own cache line to avoid conflicts with 'nr_requests' and other hot
fields
(see:
https://patchwork.kernel.org/project/linux-scsi/patch/20260402074637.92417-3-sumit.saxena@broadcom.com/
).
During the review, Bart shared his work, which eliminates the
counter entirely by removing the fairness throttling. My testing confirmed
that
this approach resolved the performance issues and improved IOPS.
This patch is part of a larger set, and I have reported the cumulative
performance
improvements in the cover letter.
Thanks,
Sumit
>
> On Tue, Jun 09, 2026 at 05:48:02PM +0530, Sumit Saxena wrote:
> > From: Bart Van Assche <bvanassche@acm.org>
> >
> > Original patch [1] by Bart Van Assche; this version is rebased onto the
> > current tree. In testing it improves IOPS by roughly 16-18% by removing
> > the fair-sharing throttle on shared tag queues.
> >
> > This patch removes the following code and structure members:
> > - The function hctx_may_queue().
> > - blk_mq_hw_ctx.nr_active and
request_queue.nr_active_requests_shared_tags
> > and also all the code that modifies these two member variables.
>
> .. and besides that, this commit message is still entirely useless
> as it doesn't explain any of the thoughts of why this change is safe
> and desirable. While the mechanics above are totally obvious from
> the code change itself.
>
[-- Attachment #1.2: Type: text/html, Size: 2485 bytes --]
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]
^ permalink raw reply
* Re: [PATCH v2] mm: page_reporting: allow driver to set batch capacity
From: Gregory Price @ 2026-06-10 15:30 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Miaohe Lin, David Hildenbrand (Arm), Jason Wang,
Xuan Zhuo, Eugenio Perez, Muchun Song, Oscar Salvador,
Andrew Morton, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi,
Alexander Duyck
In-Reply-To: <cb43adc61d2ed3069b2fe428f3e051dbdc4cc28d.1781097156.git.mst@redhat.com>
On Wed, Jun 10, 2026 at 09:17:08AM -0400, Michael S. Tsirkin wrote:
> At the moment, if a virtio balloon device has a page reporting vq but
> its size is < PAGE_REPORTING_CAPACITY (32), the balloon driver fails
> probe.
>
> But, there's no way for host to know this value, so it can easily
> create a smaller vq and suddenly adding the reporting capability
> to the device makes all of the driver fail. Not pretty.
>
> Add a capacity field to page_reporting_dev_info so drivers can
> control the maximum number of pages per report batch.
>
> In virtio-balloon, set the capacity to the reporting virtqueue size,
> letting page_reporting adapt to whatever the device provides.
>
> Capacity need not be a power of two. Code previously called out
> division by PAGE_REPORTING_CAPACITY as cheap since it was a power
> of 2, but no performance difference was observed with non-power-of-2
> values.
>
> If capacity is 0 or exceeds PAGE_REPORTING_CAPACITY, it defaults
> to PAGE_REPORTING_CAPACITY. The 0 check and the clamping is done in
> page_reporting_register(), before the reporting work is scheduled,
> so we never get division by 0.
>
> Fixes: b0c504f15471 ("virtio-balloon: add support for providing free page reports to host")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
lgtm asside from David's comment request
Reviewed-by: Gregory Price <gourry@gourry.net>
> ---
> Changes v1->v2:
> - Document capacity=0 as default in commit log
> - Document that capacity need not be a power of two
> - Drop unnecessary comment about integer division cost
> - Update comment on capacity field: "0 (default) means PAGE_REPORTING_CAPACITY"
>
> drivers/virtio/virtio_balloon.c | 5 +----
> include/linux/page_reporting.h | 3 +++
> mm/page_reporting.c | 24 ++++++++++++------------
> 3 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index f6c2dff33f8a..6a1a610c2cb1 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -1017,10 +1017,6 @@ static int virtballoon_probe(struct virtio_device *vdev)
> unsigned int capacity;
>
> capacity = virtqueue_get_vring_size(vb->reporting_vq);
> - if (capacity < PAGE_REPORTING_CAPACITY) {
> - err = -ENOSPC;
> - goto out_unregister_oom;
> - }
>
> vb->pr_dev_info.order = PAGE_REPORTING_ORDER_UNSPECIFIED;
>
> @@ -1041,6 +1037,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
> vb->pr_dev_info.order = 5;
> #endif
>
> + vb->pr_dev_info.capacity = capacity;
> err = page_reporting_register(&vb->pr_dev_info);
> if (err)
> goto out_unregister_oom;
> diff --git a/include/linux/page_reporting.h b/include/linux/page_reporting.h
> index 9d4ca5c218a0..048578118a4b 100644
> --- a/include/linux/page_reporting.h
> +++ b/include/linux/page_reporting.h
> @@ -22,6 +22,9 @@ struct page_reporting_dev_info {
>
> /* Minimal order of page reporting */
> unsigned int order;
> +
> + /* Max pages per report batch; 0 (default) means PAGE_REPORTING_CAPACITY */
> + unsigned int capacity;
> };
>
> /* Tear-down and bring-up for page reporting devices */
> diff --git a/mm/page_reporting.c b/mm/page_reporting.c
> index 7418f2e500bb..942e84b6908a 100644
> --- a/mm/page_reporting.c
> +++ b/mm/page_reporting.c
> @@ -173,11 +173,8 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
> * any pages that may have already been present from the previous
> * list processed. This should result in us reporting all pages on
> * an idle system in about 30 seconds.
> - *
> - * The division here should be cheap since PAGE_REPORTING_CAPACITY
> - * should always be a power of 2.
> */
> - budget = DIV_ROUND_UP(area->nr_free, PAGE_REPORTING_CAPACITY * 16);
> + budget = DIV_ROUND_UP(area->nr_free, prdev->capacity * 16);
>
> /* loop through free list adding unreported pages to sg list */
> list_for_each_entry_safe(page, next, list, lru) {
> @@ -222,10 +219,10 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
> spin_unlock_irq(&zone->lock);
>
> /* begin processing pages in local list */
> - err = prdev->report(prdev, sgl, PAGE_REPORTING_CAPACITY);
> + err = prdev->report(prdev, sgl, prdev->capacity);
>
> /* reset offset since the full list was reported */
> - *offset = PAGE_REPORTING_CAPACITY;
> + *offset = prdev->capacity;
>
> /* update budget to reflect call to report function */
> budget--;
> @@ -234,7 +231,7 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
> spin_lock_irq(&zone->lock);
>
> /* flush reported pages from the sg list */
> - page_reporting_drain(prdev, sgl, PAGE_REPORTING_CAPACITY, !err);
> + page_reporting_drain(prdev, sgl, prdev->capacity, !err);
>
> /*
> * Reset next to first entry, the old next isn't valid
> @@ -260,13 +257,13 @@ static int
> page_reporting_process_zone(struct page_reporting_dev_info *prdev,
> struct scatterlist *sgl, struct zone *zone)
> {
> - unsigned int order, mt, leftover, offset = PAGE_REPORTING_CAPACITY;
> + unsigned int order, mt, leftover, offset = prdev->capacity;
> unsigned long watermark;
> int err = 0;
>
> /* Generate minimum watermark to be able to guarantee progress */
> watermark = low_wmark_pages(zone) +
> - (PAGE_REPORTING_CAPACITY << page_reporting_order);
> + (prdev->capacity << page_reporting_order);
>
> /*
> * Cancel request if insufficient free memory or if we failed
> @@ -290,7 +287,7 @@ page_reporting_process_zone(struct page_reporting_dev_info *prdev,
> }
>
> /* report the leftover pages before going idle */
> - leftover = PAGE_REPORTING_CAPACITY - offset;
> + leftover = prdev->capacity - offset;
> if (leftover) {
> sgl = &sgl[offset];
> err = prdev->report(prdev, sgl, leftover);
> @@ -322,11 +319,11 @@ static void page_reporting_process(struct work_struct *work)
> atomic_set(&prdev->state, state);
>
> /* allocate scatterlist to store pages being reported on */
> - sgl = kmalloc_objs(*sgl, PAGE_REPORTING_CAPACITY);
> + sgl = kmalloc_objs(*sgl, prdev->capacity);
> if (!sgl)
> goto err_out;
>
> - sg_init_table(sgl, PAGE_REPORTING_CAPACITY);
> + sg_init_table(sgl, prdev->capacity);
>
> for_each_zone(zone) {
> err = page_reporting_process_zone(prdev, sgl, zone);
> @@ -377,6 +374,9 @@ int page_reporting_register(struct page_reporting_dev_info *prdev)
> page_reporting_order = pageblock_order;
> }
>
> + if (!prdev->capacity || prdev->capacity > PAGE_REPORTING_CAPACITY)
> + prdev->capacity = PAGE_REPORTING_CAPACITY;
> +
> /* initialize state and work structures */
> atomic_set(&prdev->state, PAGE_REPORTING_IDLE);
> INIT_DELAYED_WORK(&prdev->work, &page_reporting_process);
> --
> MST
>
^ permalink raw reply
* [PATCH v5 15/15] drm/vmwgfx: Remove unused field struct vmwgfx_du_update_plane.old_state
From: Thomas Zimmermann @ 2026-06-10 15:18 UTC (permalink / raw)
To: mripard, maarten.lankhorst, airlied, airlied, simona, admin,
gargaditya08, paul, jani.nikula, mhklkml, zack.rusin,
bcm-kernel-feedback-list, harry.wentland, sunpeng.li, siqueira,
alexander.deucher, rodrigo.vivi, joonas.lahtinen, tursulin,
javierm, dmitry.osipenko, gurchetansingh, olvaffe
Cc: dri-devel, linux-hyperv, intel-gfx, intel-xe, linux-mips,
virtualization, amd-gfx, Thomas Zimmermann
In-Reply-To: <20260610152505.260172-1-tzimmermann@suse.de>
Plane updates no longer require the old plane state. Remove the field
from struct vmwgfx_du_update_plane and fix all callers.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Zack Rusin <zack.rusin@broadcom.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 --
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 12 ++----------
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 11 ++---------
3 files changed, 4 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 2224d7d91d1b..8c2072b82062 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -19,7 +19,6 @@
/**
* struct vmw_du_update_plane - Closure structure for vmw_du_helper_plane_update
* @plane: Plane which is being updated.
- * @old_state: Old state of plane.
* @dev_priv: Device private.
* @du: Display unit on which to update the plane.
* @vfb: Framebuffer which is blitted to display unit.
@@ -102,7 +101,6 @@ struct vmw_du_update_plane {
struct drm_rect *bb);
struct drm_plane *plane;
- struct drm_plane_state *old_state;
struct vmw_private *dev_priv;
struct vmw_display_unit *du;
struct vmw_framebuffer *vfb;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index c83061cf7455..fa84bc7ab5bb 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -530,7 +530,6 @@ static uint32_t vmw_stud_bo_post_clip(struct vmw_du_update_plane *update,
*/
static int vmw_sou_plane_update_bo(struct vmw_private *dev_priv,
struct drm_plane *plane,
- struct drm_plane_state *old_state,
struct vmw_framebuffer *vfb,
struct vmw_fence_obj **out_fence)
{
@@ -538,7 +537,6 @@ static int vmw_sou_plane_update_bo(struct vmw_private *dev_priv,
memset(&bo_update, 0, sizeof(struct vmw_du_update_plane_buffer));
bo_update.base.plane = plane;
- bo_update.base.old_state = old_state;
bo_update.base.dev_priv = dev_priv;
bo_update.base.du = vmw_crtc_to_du(plane->state->crtc);
bo_update.base.vfb = vfb;
@@ -692,7 +690,6 @@ static uint32_t vmw_sou_surface_post_clip(struct vmw_du_update_plane *update,
*/
static int vmw_sou_plane_update_surface(struct vmw_private *dev_priv,
struct drm_plane *plane,
- struct drm_plane_state *old_state,
struct vmw_framebuffer *vfb,
struct vmw_fence_obj **out_fence)
{
@@ -700,7 +697,6 @@ static int vmw_sou_plane_update_surface(struct vmw_private *dev_priv,
memset(&srf_update, 0, sizeof(struct vmw_du_update_plane_surface));
srf_update.base.plane = plane;
- srf_update.base.old_state = old_state;
srf_update.base.dev_priv = dev_priv;
srf_update.base.du = vmw_crtc_to_du(plane->state->crtc);
srf_update.base.vfb = vfb;
@@ -721,7 +717,6 @@ static void
vmw_sou_primary_plane_atomic_update(struct drm_plane *plane,
struct drm_atomic_commit *state)
{
- struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(state, plane);
struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state, plane);
struct drm_crtc *crtc = new_state->crtc;
struct vmw_fence_obj *fence = NULL;
@@ -734,12 +729,9 @@ vmw_sou_primary_plane_atomic_update(struct drm_plane *plane,
vmw_framebuffer_to_vfb(new_state->fb);
if (vfb->bo)
- ret = vmw_sou_plane_update_bo(dev_priv, plane,
- old_state, vfb, &fence);
+ ret = vmw_sou_plane_update_bo(dev_priv, plane, vfb, &fence);
else
- ret = vmw_sou_plane_update_surface(dev_priv, plane,
- old_state, vfb,
- &fence);
+ ret = vmw_sou_plane_update_surface(dev_priv, plane, vfb, &fence);
if (ret != 0)
DRM_ERROR("Failed to update screen.\n");
} else {
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index f0df2b1c8465..474e3badb80f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -1240,7 +1240,6 @@ vmw_stdu_bo_populate_update_cpu(struct vmw_du_update_plane *update, void *cmd,
*/
static int vmw_stdu_plane_update_bo(struct vmw_private *dev_priv,
struct drm_plane *plane,
- struct drm_plane_state *old_state,
struct vmw_framebuffer *vfb,
struct vmw_fence_obj **out_fence)
{
@@ -1248,7 +1247,6 @@ static int vmw_stdu_plane_update_bo(struct vmw_private *dev_priv,
memset(&bo_update, 0, sizeof(struct vmw_du_update_plane_buffer));
bo_update.base.plane = plane;
- bo_update.base.old_state = old_state;
bo_update.base.dev_priv = dev_priv;
bo_update.base.du = vmw_crtc_to_du(plane->state->crtc);
bo_update.base.vfb = vfb;
@@ -1350,7 +1348,6 @@ vmw_stdu_surface_populate_update(struct vmw_du_update_plane *update, void *cmd,
*/
static int vmw_stdu_plane_update_surface(struct vmw_private *dev_priv,
struct drm_plane *plane,
- struct drm_plane_state *old_state,
struct vmw_framebuffer *vfb,
struct vmw_fence_obj **out_fence)
{
@@ -1363,7 +1360,6 @@ static int vmw_stdu_plane_update_surface(struct vmw_private *dev_priv,
memset(&srf_update, 0, sizeof(struct vmw_du_update_plane));
srf_update.plane = plane;
- srf_update.old_state = old_state;
srf_update.dev_priv = dev_priv;
srf_update.du = vmw_crtc_to_du(plane->state->crtc);
srf_update.vfb = vfb;
@@ -1424,12 +1420,9 @@ vmw_stdu_primary_plane_atomic_update(struct drm_plane *plane,
DRM_ERROR("Failed to bind surface to STDU.\n");
if (vfb->bo)
- ret = vmw_stdu_plane_update_bo(dev_priv, plane,
- old_state, vfb, &fence);
+ ret = vmw_stdu_plane_update_bo(dev_priv, plane, vfb, &fence);
else
- ret = vmw_stdu_plane_update_surface(dev_priv, plane,
- old_state, vfb,
- &fence);
+ ret = vmw_stdu_plane_update_surface(dev_priv, plane, vfb, &fence);
if (ret)
DRM_ERROR("Failed to update STDU.\n");
} else {
--
2.54.0
^ permalink raw reply related
* [PATCH v5 11/15] drm/damage-helper: Test src coord in drm_atomic_helper_check_plane_damage()
From: Thomas Zimmermann @ 2026-06-10 15:18 UTC (permalink / raw)
To: mripard, maarten.lankhorst, airlied, airlied, simona, admin,
gargaditya08, paul, jani.nikula, mhklkml, zack.rusin,
bcm-kernel-feedback-list, harry.wentland, sunpeng.li, siqueira,
alexander.deucher, rodrigo.vivi, joonas.lahtinen, tursulin,
javierm, dmitry.osipenko, gurchetansingh, olvaffe
Cc: dri-devel, linux-hyperv, intel-gfx, intel-xe, linux-mips,
virtualization, amd-gfx, Thomas Zimmermann
In-Reply-To: <20260610152505.260172-1-tzimmermann@suse.de>
Planes require a full update if the source coordinates change across
atomic commits. Evaluate this during the atomic-check and set the flag
ignore_damage_clips in the plane state, if so. Remove the check from
drm_atomic_helper_damage_iter_init().
This will help with removing the old state from the atomic-commit phase
and simplify atomic_update helpers a bit.
Several unit tests check against the change of the src coordinate. Drop
them as they do no longer serve a purpose. If the src coordinate changes
across commits, atomic helpers will set the plane state's
ignore_damage_clips flag, for which a separate unit test exists.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Zack Rusin <zack.rusin@broadcom.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 2 +-
drivers/gpu/drm/drm_damage_helper.c | 20 ++-
.../gpu/drm/tests/drm_damage_helper_test.c | 151 ------------------
include/drm/drm_damage_helper.h | 3 +-
4 files changed, 15 insertions(+), 161 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index d9cb9b3f8490..9c95fbc978fc 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1070,7 +1070,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
}
for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
- drm_atomic_helper_check_plane_damage(state, new_plane_state);
+ drm_atomic_helper_check_plane_damage(state, old_plane_state, new_plane_state);
}
return ret;
diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c
index 945fac8dc27b..f492a59edbeb 100644
--- a/drivers/gpu/drm/drm_damage_helper.c
+++ b/drivers/gpu/drm/drm_damage_helper.c
@@ -55,7 +55,8 @@ static void convert_clip_rect_to_rect(const struct drm_clip_rect *src,
/**
* drm_atomic_helper_check_plane_damage - Verify plane damage on atomic_check.
* @state: The driver state object.
- * @plane_state: Plane state for which to verify damage.
+ * @old_plane_state: Old plane state to verify against.
+ * @new_plane_state: Plane state for which to verify damage.
*
* This helper function makes sure that damage from plane state is discarded
* for full modeset. If there are more reasons a driver would want to do a full
@@ -67,19 +68,23 @@ static void convert_clip_rect_to_rect(const struct drm_clip_rect *src,
* &drm_plane_state.src as damage.
*/
void drm_atomic_helper_check_plane_damage(struct drm_atomic_commit *state,
- struct drm_plane_state *plane_state)
+ const struct drm_plane_state *old_plane_state,
+ struct drm_plane_state *new_plane_state)
{
struct drm_crtc_state *crtc_state;
- if (plane_state->crtc) {
+ if (!drm_rect_equals(&new_plane_state->src, &old_plane_state->src))
+ new_plane_state->ignore_damage_clips = true;
+
+ if (new_plane_state->crtc) {
crtc_state = drm_atomic_get_new_crtc_state(state,
- plane_state->crtc);
+ new_plane_state->crtc);
if (WARN_ON(!crtc_state))
return;
if (drm_atomic_crtc_needs_modeset(crtc_state))
- plane_state->ignore_damage_clips = true;
+ new_plane_state->ignore_damage_clips = true;
}
}
EXPORT_SYMBOL(drm_atomic_helper_check_plane_damage);
@@ -204,7 +209,7 @@ EXPORT_SYMBOL(drm_atomic_helper_dirtyfb);
/**
* drm_atomic_helper_damage_iter_init - Initialize the damage iterator.
* @iter: The iterator to initialize.
- * @old_state: Old plane state for validation.
+ * @old_state: Unused, pass NULL.
* @state: Plane state from which to iterate the damage clips.
*
* Initialize an iterator, which clips plane damage
@@ -241,8 +246,7 @@ drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
iter->plane_src.x2 = (src.x2 >> 16) + !!(src.x2 & 0xFFFF);
iter->plane_src.y2 = (src.y2 >> 16) + !!(src.y2 & 0xFFFF);
- if (!iter->clips || state->ignore_damage_clips ||
- !drm_rect_equals(&state->src, &old_state->src)) {
+ if (!iter->clips || state->ignore_damage_clips) {
iter->clips = NULL;
iter->num_clips = 0;
iter->full_update = true;
diff --git a/drivers/gpu/drm/tests/drm_damage_helper_test.c b/drivers/gpu/drm/tests/drm_damage_helper_test.c
index 64f038a62ffe..ef931497baf9 100644
--- a/drivers/gpu/drm/tests/drm_damage_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_damage_helper_test.c
@@ -155,45 +155,6 @@ static void drm_test_damage_iter_no_damage_fractional_src(struct kunit *test)
check_damage_clip(test, &clip, 3, 3, 1028, 772);
}
-static void drm_test_damage_iter_no_damage_src_moved(struct kunit *test)
-{
- struct drm_damage_mock *mock = test->priv;
- struct drm_atomic_helper_damage_iter iter;
- struct drm_rect clip;
- u32 num_hits = 0;
-
- /* Plane src moved since old plane state. */
- set_plane_src(&mock->old_state, 0, 0, 1024 << 16, 768 << 16);
- set_plane_src(&mock->state, 10 << 16, 10 << 16,
- (10 + 1024) << 16, (10 + 768) << 16);
- drm_atomic_helper_damage_iter_init(&iter, &mock->old_state, &mock->state);
- drm_atomic_for_each_plane_damage(&iter, &clip)
- num_hits++;
-
- KUNIT_EXPECT_EQ_MSG(test, num_hits, 1, "Should return plane src as damage.");
- check_damage_clip(test, &clip, 10, 10, 1034, 778);
-}
-
-static void drm_test_damage_iter_no_damage_fractional_src_moved(struct kunit *test)
-{
- struct drm_damage_mock *mock = test->priv;
- struct drm_atomic_helper_damage_iter iter;
- struct drm_rect clip;
- u32 num_hits = 0;
-
- /* Plane src has fractional part and it moved since old plane state. */
- set_plane_src(&mock->old_state, 0x3fffe, 0x3fffe,
- 0x3fffe + (1024 << 16), 0x3fffe + (768 << 16));
- set_plane_src(&mock->state, 0x40002, 0x40002,
- 0x40002 + (1024 << 16), 0x40002 + (768 << 16));
- drm_atomic_helper_damage_iter_init(&iter, &mock->old_state, &mock->state);
- drm_atomic_for_each_plane_damage(&iter, &clip)
- num_hits++;
-
- KUNIT_EXPECT_EQ_MSG(test, num_hits, 1, "Should return plane src as damage.");
- check_damage_clip(test, &clip, 4, 4, 1029, 773);
-}
-
static void drm_test_damage_iter_no_damage_not_visible(struct kunit *test)
{
struct drm_damage_mock *mock = test->priv;
@@ -415,58 +376,6 @@ static void drm_test_damage_iter_single_damage_outside_fractional_src(struct kun
KUNIT_EXPECT_EQ_MSG(test, num_hits, 0, "Should have no damage.");
}
-static void drm_test_damage_iter_single_damage_src_moved(struct kunit *test)
-{
- struct drm_damage_mock *mock = test->priv;
- struct drm_atomic_helper_damage_iter iter;
- struct drm_property_blob damage_blob;
- struct drm_mode_rect damage;
- struct drm_rect clip;
- u32 num_hits = 0;
-
- /* Plane src moved since old plane state. */
- set_plane_src(&mock->old_state, 0, 0, 1024 << 16, 768 << 16);
- set_plane_src(&mock->state, 10 << 16, 10 << 16,
- (10 + 1024) << 16, (10 + 768) << 16);
- set_damage_clip(&damage, 20, 30, 256, 256);
- set_damage_blob(&damage_blob, &damage, sizeof(damage));
- set_plane_damage(&mock->state, &damage_blob);
- drm_atomic_helper_damage_iter_init(&iter, &mock->old_state, &mock->state);
- drm_atomic_for_each_plane_damage(&iter, &clip)
- num_hits++;
-
- KUNIT_EXPECT_EQ_MSG(test, num_hits, 1,
- "Should return plane src as damage.");
- check_damage_clip(test, &clip, 10, 10, 1034, 778);
-}
-
-static void drm_test_damage_iter_single_damage_fractional_src_moved(struct kunit *test)
-{
- struct drm_damage_mock *mock = test->priv;
- struct drm_atomic_helper_damage_iter iter;
- struct drm_property_blob damage_blob;
- struct drm_mode_rect damage;
- struct drm_rect clip;
- u32 num_hits = 0;
-
- /* Plane src with fractional part moved since old plane state. */
- set_plane_src(&mock->old_state, 0x3fffe, 0x3fffe,
- 0x3fffe + (1024 << 16), 0x3fffe + (768 << 16));
- set_plane_src(&mock->state, 0x40002, 0x40002,
- 0x40002 + (1024 << 16), 0x40002 + (768 << 16));
- /* Damage intersect with plane src. */
- set_damage_clip(&damage, 20, 30, 1360, 256);
- set_damage_blob(&damage_blob, &damage, sizeof(damage));
- set_plane_damage(&mock->state, &damage_blob);
- drm_atomic_helper_damage_iter_init(&iter, &mock->old_state, &mock->state);
- drm_atomic_for_each_plane_damage(&iter, &clip)
- num_hits++;
-
- KUNIT_EXPECT_EQ_MSG(test, num_hits, 1,
- "Should return rounded off plane as damage.");
- check_damage_clip(test, &clip, 4, 4, 1029, 773);
-}
-
static void drm_test_damage_iter_damage(struct kunit *test)
{
struct drm_damage_mock *mock = test->priv;
@@ -549,60 +458,6 @@ static void drm_test_damage_iter_damage_one_outside(struct kunit *test)
check_damage_clip(test, &clip, 240, 200, 280, 250);
}
-static void drm_test_damage_iter_damage_src_moved(struct kunit *test)
-{
- struct drm_damage_mock *mock = test->priv;
- struct drm_atomic_helper_damage_iter iter;
- struct drm_property_blob damage_blob;
- struct drm_mode_rect damage[2];
- struct drm_rect clip;
- u32 num_hits = 0;
-
- set_plane_src(&mock->old_state, 0x40002, 0x40002,
- 0x40002 + (1024 << 16), 0x40002 + (768 << 16));
- set_plane_src(&mock->state, 0x3fffe, 0x3fffe,
- 0x3fffe + (1024 << 16), 0x3fffe + (768 << 16));
- /* 2 damage clips, one outside plane src. */
- set_damage_clip(&damage[0], 1360, 1360, 1380, 1380);
- set_damage_clip(&damage[1], 240, 200, 280, 250);
- set_damage_blob(&damage_blob, &damage[0], sizeof(damage));
- set_plane_damage(&mock->state, &damage_blob);
- drm_atomic_helper_damage_iter_init(&iter, &mock->old_state, &mock->state);
- drm_atomic_for_each_plane_damage(&iter, &clip)
- num_hits++;
-
- KUNIT_EXPECT_EQ_MSG(test, num_hits, 1,
- "Should return round off plane src as damage.");
- check_damage_clip(test, &clip, 3, 3, 1028, 772);
-}
-
-static void drm_test_damage_iter_damage_not_visible(struct kunit *test)
-{
- struct drm_damage_mock *mock = test->priv;
- struct drm_atomic_helper_damage_iter iter;
- struct drm_property_blob damage_blob;
- struct drm_mode_rect damage[2];
- struct drm_rect clip;
- u32 num_hits = 0;
-
- mock->state.visible = false;
-
- set_plane_src(&mock->old_state, 0x40002, 0x40002,
- 0x40002 + (1024 << 16), 0x40002 + (768 << 16));
- set_plane_src(&mock->state, 0x3fffe, 0x3fffe,
- 0x3fffe + (1024 << 16), 0x3fffe + (768 << 16));
- /* 2 damage clips, one outside plane src. */
- set_damage_clip(&damage[0], 1360, 1360, 1380, 1380);
- set_damage_clip(&damage[1], 240, 200, 280, 250);
- set_damage_blob(&damage_blob, &damage[0], sizeof(damage));
- set_plane_damage(&mock->state, &damage_blob);
- drm_atomic_helper_damage_iter_init(&iter, &mock->old_state, &mock->state);
- drm_atomic_for_each_plane_damage(&iter, &clip)
- num_hits++;
-
- KUNIT_EXPECT_EQ_MSG(test, num_hits, 0, "Should not return any damage.");
-}
-
static void drm_test_damage_iter_damage_ignore(struct kunit *test)
{
struct drm_damage_mock *mock = test->priv;
@@ -633,8 +488,6 @@ static void drm_test_damage_iter_damage_ignore(struct kunit *test)
static struct kunit_case drm_damage_helper_tests[] = {
KUNIT_CASE(drm_test_damage_iter_no_damage),
KUNIT_CASE(drm_test_damage_iter_no_damage_fractional_src),
- KUNIT_CASE(drm_test_damage_iter_no_damage_src_moved),
- KUNIT_CASE(drm_test_damage_iter_no_damage_fractional_src_moved),
KUNIT_CASE(drm_test_damage_iter_no_damage_not_visible),
KUNIT_CASE(drm_test_damage_iter_no_damage_no_crtc),
KUNIT_CASE(drm_test_damage_iter_no_damage_no_fb),
@@ -645,13 +498,9 @@ static struct kunit_case drm_damage_helper_tests[] = {
KUNIT_CASE(drm_test_damage_iter_single_damage_fractional_src),
KUNIT_CASE(drm_test_damage_iter_single_damage_intersect_fractional_src),
KUNIT_CASE(drm_test_damage_iter_single_damage_outside_fractional_src),
- KUNIT_CASE(drm_test_damage_iter_single_damage_src_moved),
- KUNIT_CASE(drm_test_damage_iter_single_damage_fractional_src_moved),
KUNIT_CASE(drm_test_damage_iter_damage),
KUNIT_CASE(drm_test_damage_iter_damage_one_intersect),
KUNIT_CASE(drm_test_damage_iter_damage_one_outside),
- KUNIT_CASE(drm_test_damage_iter_damage_src_moved),
- KUNIT_CASE(drm_test_damage_iter_damage_not_visible),
KUNIT_CASE(drm_test_damage_iter_damage_ignore),
{ }
};
diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h
index 3661aeab2cd3..e93eaa0fbcb6 100644
--- a/include/drm/drm_damage_helper.h
+++ b/include/drm/drm_damage_helper.h
@@ -65,7 +65,8 @@ struct drm_atomic_helper_damage_iter {
};
void drm_atomic_helper_check_plane_damage(struct drm_atomic_commit *state,
- struct drm_plane_state *plane_state);
+ const struct drm_plane_state *old_plane_state,
+ struct drm_plane_state *new_plane_state);
int drm_atomic_helper_dirtyfb(struct drm_framebuffer *fb,
struct drm_file *file_priv, unsigned int flags,
unsigned int color, struct drm_clip_rect *clips,
--
2.54.0
^ permalink raw reply related
* [PATCH v5 10/15] drm/atomic_helper: Do not evaluate plane damage before atomic_check
From: Thomas Zimmermann @ 2026-06-10 15:18 UTC (permalink / raw)
To: mripard, maarten.lankhorst, airlied, airlied, simona, admin,
gargaditya08, paul, jani.nikula, mhklkml, zack.rusin,
bcm-kernel-feedback-list, harry.wentland, sunpeng.li, siqueira,
alexander.deucher, rodrigo.vivi, joonas.lahtinen, tursulin,
javierm, dmitry.osipenko, gurchetansingh, olvaffe
Cc: dri-devel, linux-hyperv, intel-gfx, intel-xe, linux-mips,
virtualization, amd-gfx, Thomas Zimmermann
In-Reply-To: <20260610152505.260172-1-tzimmermann@suse.de>
Remove the call to drm_atomic_helper_check_plane_damage() from before
calling the atomic_check helpers. The call has no longer any purpose,
as the actual evaluation happens after running atomic_check.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Zack Rusin <zack.rusin@broadcom.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 8e080a42aec4..d9cb9b3f8490 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1040,8 +1040,6 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
drm_atomic_helper_plane_changed(state, old_plane_state, new_plane_state, plane);
- drm_atomic_helper_check_plane_damage(state, new_plane_state);
-
if (!funcs || !funcs->atomic_check)
continue;
--
2.54.0
^ permalink raw reply related
* [PATCH v5 14/15] drm/damage-helper: Rename state parameters in damage helpers
From: Thomas Zimmermann @ 2026-06-10 15:18 UTC (permalink / raw)
To: mripard, maarten.lankhorst, airlied, airlied, simona, admin,
gargaditya08, paul, jani.nikula, mhklkml, zack.rusin,
bcm-kernel-feedback-list, harry.wentland, sunpeng.li, siqueira,
alexander.deucher, rodrigo.vivi, joonas.lahtinen, tursulin,
javierm, dmitry.osipenko, gurchetansingh, olvaffe
Cc: dri-devel, linux-hyperv, intel-gfx, intel-xe, linux-mips,
virtualization, amd-gfx, Thomas Zimmermann
In-Reply-To: <20260610152505.260172-1-tzimmermann@suse.de>
Rename some of the state parameters of the damage-helper functions to
align them with each other and other helpers. No functional changes.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Zack Rusin <zack.rusin@broadcom.com>
---
drivers/gpu/drm/drm_damage_helper.c | 20 ++++++++++----------
include/drm/drm_damage_helper.h | 4 ++--
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c
index 28b847636253..23701e5c51b7 100644
--- a/drivers/gpu/drm/drm_damage_helper.c
+++ b/drivers/gpu/drm/drm_damage_helper.c
@@ -209,7 +209,7 @@ EXPORT_SYMBOL(drm_atomic_helper_dirtyfb);
/**
* drm_atomic_helper_damage_iter_init - Initialize the damage iterator.
* @iter: The iterator to initialize.
- * @state: Plane state from which to iterate the damage clips.
+ * @plane_state: Plane state from which to iterate the damage clips.
*
* Initialize an iterator, which clips plane damage
* &drm_plane_state.fb_damage_clips to plane &drm_plane_state.src. This iterator
@@ -225,26 +225,26 @@ EXPORT_SYMBOL(drm_atomic_helper_dirtyfb);
*/
void
drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
- const struct drm_plane_state *state)
+ const struct drm_plane_state *plane_state)
{
struct drm_rect src;
memset(iter, 0, sizeof(*iter));
- if (!state || !state->crtc || !state->fb || !state->visible)
+ if (!plane_state || !plane_state->crtc || !plane_state->fb || !plane_state->visible)
return;
- iter->clips = (struct drm_rect *)drm_plane_get_damage_clips(state);
- iter->num_clips = drm_plane_get_damage_clips_count(state);
+ iter->clips = (struct drm_rect *)drm_plane_get_damage_clips(plane_state);
+ iter->num_clips = drm_plane_get_damage_clips_count(plane_state);
/* Round down for x1/y1 and round up for x2/y2 to catch all pixels */
- src = drm_plane_state_src(state);
+ src = drm_plane_state_src(plane_state);
iter->plane_src.x1 = src.x1 >> 16;
iter->plane_src.y1 = src.y1 >> 16;
iter->plane_src.x2 = (src.x2 >> 16) + !!(src.x2 & 0xFFFF);
iter->plane_src.y2 = (src.y2 >> 16) + !!(src.y2 & 0xFFFF);
- if (!iter->clips || state->ignore_damage_clips) {
+ if (!iter->clips || plane_state->ignore_damage_clips) {
iter->clips = NULL;
iter->num_clips = 0;
iter->full_update = true;
@@ -296,7 +296,7 @@ EXPORT_SYMBOL(drm_atomic_helper_damage_iter_next);
/**
* drm_atomic_helper_damage_merged - Merged plane damage
- * @state: Plane state from which to iterate the damage clips.
+ * @plane_state: Plane state from which to iterate the damage clips.
* @rect: Returns the merged damage rectangle
*
* This function merges any valid plane damage clips into one rectangle and
@@ -308,7 +308,7 @@ EXPORT_SYMBOL(drm_atomic_helper_damage_iter_next);
* Returns:
* True if there is valid plane damage otherwise false.
*/
-bool drm_atomic_helper_damage_merged(const struct drm_plane_state *state,
+bool drm_atomic_helper_damage_merged(const struct drm_plane_state *plane_state,
struct drm_rect *rect)
{
struct drm_atomic_helper_damage_iter iter;
@@ -320,7 +320,7 @@ bool drm_atomic_helper_damage_merged(const struct drm_plane_state *state,
rect->x2 = 0;
rect->y2 = 0;
- drm_atomic_helper_damage_iter_init(&iter, state);
+ drm_atomic_helper_damage_iter_init(&iter, plane_state);
drm_atomic_for_each_plane_damage(&iter, &clip) {
rect->x1 = min(rect->x1, clip.x1);
rect->y1 = min(rect->y1, clip.y1);
diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h
index b5a4de779db6..4a1ac47b9051 100644
--- a/include/drm/drm_damage_helper.h
+++ b/include/drm/drm_damage_helper.h
@@ -73,11 +73,11 @@ int drm_atomic_helper_dirtyfb(struct drm_framebuffer *fb,
unsigned int num_clips);
void
drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
- const struct drm_plane_state *state);
+ const struct drm_plane_state *plane_state);
bool
drm_atomic_helper_damage_iter_next(struct drm_atomic_helper_damage_iter *iter,
struct drm_rect *rect);
-bool drm_atomic_helper_damage_merged(const struct drm_plane_state *state,
+bool drm_atomic_helper_damage_merged(const struct drm_plane_state *plane_state,
struct drm_rect *rect);
#endif
--
2.54.0
^ permalink raw reply related
* [PATCH v5 09/15] drm/ingenic: Remove calls to drm_atomic_helper_check_plane_damage()
From: Thomas Zimmermann @ 2026-06-10 15:18 UTC (permalink / raw)
To: mripard, maarten.lankhorst, airlied, airlied, simona, admin,
gargaditya08, paul, jani.nikula, mhklkml, zack.rusin,
bcm-kernel-feedback-list, harry.wentland, sunpeng.li, siqueira,
alexander.deucher, rodrigo.vivi, joonas.lahtinen, tursulin,
javierm, dmitry.osipenko, gurchetansingh, olvaffe
Cc: dri-devel, linux-hyperv, intel-gfx, intel-xe, linux-mips,
virtualization, amd-gfx, Thomas Zimmermann
In-Reply-To: <20260610152505.260172-1-tzimmermann@suse.de>
Atomic helpers call drm_atomic_helper_check_plane_damage() after the
atomic_check anyway. See atomic_helper_check_planes(). Remove the calls
from the planes' atomic_check.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Zack Rusin <zack.rusin@broadcom.com>
---
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 3 ---
drivers/gpu/drm/ingenic/ingenic-ipu.c | 8 ++------
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 42c86f195c66..e99b44e3ac92 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -519,9 +519,6 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane,
old_plane_state->fb->format->format != new_plane_state->fb->format->format))
crtc_state->mode_changed = true;
- if (priv->soc_info->map_noncoherent)
- drm_atomic_helper_check_plane_damage(state, new_plane_state);
-
return 0;
}
diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c
index 56143a191f36..fd17c642c7ac 100644
--- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
+++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
@@ -594,7 +594,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
if (!new_plane_state->crtc ||
!crtc_state->mode.hdisplay || !crtc_state->mode.vdisplay)
- goto out_check_damage;
+ return 0;
/* Plane must be fully visible */
if (new_plane_state->crtc_x < 0 || new_plane_state->crtc_y < 0 ||
@@ -611,7 +611,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
if (!osd_changed(new_plane_state, old_plane_state))
- goto out_check_damage;
+ return 0;
crtc_state->mode_changed = true;
@@ -645,10 +645,6 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
ipu_state->denom_w = denom_w;
ipu_state->denom_h = denom_h;
-out_check_damage:
- if (ingenic_drm_map_noncoherent(ipu->master))
- drm_atomic_helper_check_plane_damage(state, new_plane_state);
-
return 0;
}
--
2.54.0
^ permalink raw reply related
* [PATCH v5 13/15] drm/damage-helper: Remove old state from drm_atomic_helper_damage_merged()
From: Thomas Zimmermann @ 2026-06-10 15:18 UTC (permalink / raw)
To: mripard, maarten.lankhorst, airlied, airlied, simona, admin,
gargaditya08, paul, jani.nikula, mhklkml, zack.rusin,
bcm-kernel-feedback-list, harry.wentland, sunpeng.li, siqueira,
alexander.deucher, rodrigo.vivi, joonas.lahtinen, tursulin,
javierm, dmitry.osipenko, gurchetansingh, olvaffe
Cc: dri-devel, linux-hyperv, intel-gfx, intel-xe, linux-mips,
virtualization, amd-gfx, Thomas Zimmermann
In-Reply-To: <20260610152505.260172-1-tzimmermann@suse.de>
Nothing in drm_atomic_helper_damage_merged() requires the old
plane state. Remove the parameter and mass-convert callers.
Most callers now no longer require the old plane state in their plane's
atomic_update helper. Remove it as well.
v5:
- also handle i915
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Zack Rusin <zack.rusin@broadcom.com>
---
drivers/gpu/drm/ast/ast_cursor.c | 3 +--
drivers/gpu/drm/drm_damage_helper.c | 4 +---
drivers/gpu/drm/drm_mipi_dbi.c | 3 +--
drivers/gpu/drm/i915/display/intel_plane.c | 11 ++---------
drivers/gpu/drm/i915/display/intel_psr.c | 7 ++++---
drivers/gpu/drm/sitronix/st7586.c | 3 +--
drivers/gpu/drm/tiny/gm12u320.c | 2 +-
drivers/gpu/drm/tiny/ili9225.c | 3 +--
drivers/gpu/drm/tiny/repaper.c | 2 +-
drivers/gpu/drm/tiny/sharp-memory.c | 3 +--
drivers/gpu/drm/virtio/virtgpu_plane.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 4 +---
include/drm/drm_damage_helper.h | 3 +--
13 files changed, 17 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
index fd19c45f2abe..12d5f93eec5f 100644
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -251,7 +251,6 @@ static void ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
struct drm_framebuffer *fb = plane_state->fb;
- struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
struct ast_device *ast = to_ast_device(plane->dev);
struct drm_rect damage;
u64 dst_off = ast_plane->offset;
@@ -266,7 +265,7 @@ static void ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
* engine to the offset.
*/
- if (drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage)) {
+ if (drm_atomic_helper_damage_merged(plane_state, &damage)) {
const u8 *argb4444 = ast_cursor_plane_get_argb4444(ast_cursor_plane,
shadow_plane_state,
&damage);
diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c
index 28f26234523d..28b847636253 100644
--- a/drivers/gpu/drm/drm_damage_helper.c
+++ b/drivers/gpu/drm/drm_damage_helper.c
@@ -296,7 +296,6 @@ EXPORT_SYMBOL(drm_atomic_helper_damage_iter_next);
/**
* drm_atomic_helper_damage_merged - Merged plane damage
- * @old_state: Old plane state for validation.
* @state: Plane state from which to iterate the damage clips.
* @rect: Returns the merged damage rectangle
*
@@ -309,8 +308,7 @@ EXPORT_SYMBOL(drm_atomic_helper_damage_iter_next);
* Returns:
* True if there is valid plane damage otherwise false.
*/
-bool drm_atomic_helper_damage_merged(const struct drm_plane_state *old_state,
- const struct drm_plane_state *state,
+bool drm_atomic_helper_damage_merged(const struct drm_plane_state *state,
struct drm_rect *rect)
{
struct drm_atomic_helper_damage_iter iter;
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 25cf04d029f7..4da201c38c93 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -380,7 +380,6 @@ void drm_mipi_dbi_plane_helper_atomic_update(struct drm_plane *plane,
struct drm_plane_state *plane_state = plane->state;
struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
struct drm_framebuffer *fb = plane_state->fb;
- struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
struct drm_rect rect;
int idx;
@@ -388,7 +387,7 @@ void drm_mipi_dbi_plane_helper_atomic_update(struct drm_plane *plane,
return;
if (drm_dev_enter(plane->dev, &idx)) {
- if (drm_atomic_helper_damage_merged(old_plane_state, plane_state, &rect))
+ if (drm_atomic_helper_damage_merged(plane_state, &rect))
mipi_dbi_fb_dirty(&shadow_plane_state->data[0], fb, &rect,
&shadow_plane_state->fmtcnv_state);
drm_dev_exit(idx);
diff --git a/drivers/gpu/drm/i915/display/intel_plane.c b/drivers/gpu/drm/i915/display/intel_plane.c
index 3eaf82477f49..a75f89e16f08 100644
--- a/drivers/gpu/drm/i915/display/intel_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_plane.c
@@ -373,7 +373,6 @@ static void intel_plane_clear_hw_state(struct intel_plane_state *plane_state)
static void
intel_plane_copy_uapi_plane_damage(struct intel_plane_state *new_plane_state,
- const struct intel_plane_state *old_uapi_plane_state,
const struct intel_plane_state *new_uapi_plane_state)
{
struct intel_display *display = to_intel_display(new_plane_state);
@@ -383,10 +382,9 @@ intel_plane_copy_uapi_plane_damage(struct intel_plane_state *new_plane_state,
if (DISPLAY_VER(display) < 12)
return;
- if (!drm_atomic_helper_damage_merged(&old_uapi_plane_state->uapi,
- &new_uapi_plane_state->uapi,
+ if (!drm_atomic_helper_damage_merged(&new_uapi_plane_state->uapi,
damage))
- /* Incase helper fails, mark whole plane region as damage */
+ /* In case the helper fails, mark whole plane region as damage */
*damage = drm_plane_state_src(&new_uapi_plane_state->uapi);
}
@@ -851,7 +849,6 @@ static int plane_atomic_check(struct intel_atomic_state *state,
const struct intel_plane_state *old_plane_state =
intel_atomic_get_old_plane_state(state, plane);
const struct intel_plane_state *new_primary_crtc_plane_state;
- const struct intel_plane_state *old_primary_crtc_plane_state;
struct intel_crtc *crtc = intel_crtc_for_pipe(display, plane->pipe);
const struct intel_crtc_state *old_crtc_state =
intel_atomic_get_old_crtc_state(state, crtc);
@@ -866,15 +863,11 @@ static int plane_atomic_check(struct intel_atomic_state *state,
new_primary_crtc_plane_state =
intel_atomic_get_new_plane_state(state, primary_crtc_plane);
- old_primary_crtc_plane_state =
- intel_atomic_get_old_plane_state(state, primary_crtc_plane);
} else {
new_primary_crtc_plane_state = new_plane_state;
- old_primary_crtc_plane_state = old_plane_state;
}
intel_plane_copy_uapi_plane_damage(new_plane_state,
- old_primary_crtc_plane_state,
new_primary_crtc_plane_state);
intel_plane_copy_uapi_to_hw_state(state,
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index e4f43eb5bd72..6581b7de53bb 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -3019,10 +3019,11 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
drm_rect_fp_to_int(&src, &src);
/* Prepare plane-damage state before using it */
- drm_atomic_helper_check_plane_damage(&state->base, &new_plane_state->uapi);
+ drm_atomic_helper_check_plane_damage(&state->base,
+ &old_plane_state->uapi,
+ &new_plane_state->uapi);
- if (!drm_atomic_helper_damage_merged(&old_plane_state->uapi,
- &new_plane_state->uapi, &damaged_area))
+ if (!drm_atomic_helper_damage_merged(&new_plane_state->uapi, &damaged_area))
continue;
damaged_area.y1 += new_plane_state->uapi.dst.y1 - src.y1;
diff --git a/drivers/gpu/drm/sitronix/st7586.c b/drivers/gpu/drm/sitronix/st7586.c
index 28b2245f6b79..2cc0312595a4 100644
--- a/drivers/gpu/drm/sitronix/st7586.c
+++ b/drivers/gpu/drm/sitronix/st7586.c
@@ -176,7 +176,6 @@ static void st7586_plane_helper_atomic_update(struct drm_plane *plane,
struct drm_plane_state *plane_state = plane->state;
struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
struct drm_framebuffer *fb = plane_state->fb;
- struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
struct drm_rect rect;
int idx;
@@ -186,7 +185,7 @@ static void st7586_plane_helper_atomic_update(struct drm_plane *plane,
if (!drm_dev_enter(plane->dev, &idx))
return;
- if (drm_atomic_helper_damage_merged(old_plane_state, plane_state, &rect))
+ if (drm_atomic_helper_damage_merged(plane_state, &rect))
st7586_fb_dirty(&shadow_plane_state->data[0], fb, &rect,
&shadow_plane_state->fmtcnv_state);
diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
index 4ad074337af0..dcc1767c645d 100644
--- a/drivers/gpu/drm/tiny/gm12u320.c
+++ b/drivers/gpu/drm/tiny/gm12u320.c
@@ -583,7 +583,7 @@ static void gm12u320_pipe_update(struct drm_simple_display_pipe *pipe,
struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
struct drm_rect rect;
- if (drm_atomic_helper_damage_merged(old_state, state, &rect))
+ if (drm_atomic_helper_damage_merged(state, &rect))
gm12u320_fb_mark_dirty(state->fb, &shadow_plane_state->data[0], &rect);
}
diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index 5bf52a8fd75b..d821a659a585 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -185,7 +185,6 @@ static void ili9225_plane_helper_atomic_update(struct drm_plane *plane,
struct drm_plane_state *plane_state = plane->state;
struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
struct drm_framebuffer *fb = plane_state->fb;
- struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
struct drm_rect rect;
int idx;
@@ -195,7 +194,7 @@ static void ili9225_plane_helper_atomic_update(struct drm_plane *plane,
if (!drm_dev_enter(drm, &idx))
return;
- if (drm_atomic_helper_damage_merged(old_plane_state, plane_state, &rect))
+ if (drm_atomic_helper_damage_merged(plane_state, &rect))
ili9225_fb_dirty(&shadow_plane_state->data[0], fb, &rect,
&shadow_plane_state->fmtcnv_state);
diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
index c8270591afc7..531831d2b73f 100644
--- a/drivers/gpu/drm/tiny/repaper.c
+++ b/drivers/gpu/drm/tiny/repaper.c
@@ -837,7 +837,7 @@ static void repaper_pipe_update(struct drm_simple_display_pipe *pipe,
if (!pipe->crtc.state->active)
return;
- if (drm_atomic_helper_damage_merged(old_state, state, &rect))
+ if (drm_atomic_helper_damage_merged(state, &rect))
repaper_fb_dirty(state->fb, shadow_plane_state->data,
&shadow_plane_state->fmtcnv_state);
}
diff --git a/drivers/gpu/drm/tiny/sharp-memory.c b/drivers/gpu/drm/tiny/sharp-memory.c
index 506e6432e70d..1dacd41ddbaa 100644
--- a/drivers/gpu/drm/tiny/sharp-memory.c
+++ b/drivers/gpu/drm/tiny/sharp-memory.c
@@ -241,7 +241,6 @@ static int sharp_memory_plane_atomic_check(struct drm_plane *plane,
static void sharp_memory_plane_atomic_update(struct drm_plane *plane,
struct drm_atomic_commit *state)
{
- struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(state, plane);
struct drm_plane_state *plane_state = plane->state;
struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
struct sharp_memory_device *smd;
@@ -251,7 +250,7 @@ static void sharp_memory_plane_atomic_update(struct drm_plane *plane,
if (!smd->crtc.state->active)
return;
- if (drm_atomic_helper_damage_merged(old_state, plane_state, &rect))
+ if (drm_atomic_helper_damage_merged(plane_state, &rect))
sharp_memory_fb_dirty(plane_state->fb, shadow_plane_state->data,
&rect, &shadow_plane_state->fmtcnv_state);
}
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 1d1b27ece62a..4728047315a2 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -260,7 +260,7 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane,
return;
}
- if (!drm_atomic_helper_damage_merged(old_state, plane->state, &rect))
+ if (!drm_atomic_helper_damage_merged(plane->state, &rect))
return;
bo = gem_to_virtio_gpu_obj(plane->state->fb->obj[0]);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 4139837f4caf..f0df2b1c8465 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -977,7 +977,6 @@ vmw_stdu_primary_plane_prepare_fb(struct drm_plane *plane,
enum stdu_content_type new_content_type;
struct vmw_framebuffer_surface *new_vfbs;
uint32_t hdisplay = new_state->crtc_w, vdisplay = new_state->crtc_h;
- struct drm_plane_state *old_state = plane->state;
struct drm_rect rect;
int ret;
@@ -1101,8 +1100,7 @@ vmw_stdu_primary_plane_prepare_fb(struct drm_plane *plane,
struct vmw_surface *surf = vmw_user_object_surface(&vps->uo);
struct vmw_resource *res = &surf->res;
- if (!res->res_dirty && drm_atomic_helper_damage_merged(old_state,
- new_state,
+ if (!res->res_dirty && drm_atomic_helper_damage_merged(new_state,
&rect)) {
/*
* At some point it might be useful to actually translate
diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h
index fafe29b50fc6..b5a4de779db6 100644
--- a/include/drm/drm_damage_helper.h
+++ b/include/drm/drm_damage_helper.h
@@ -77,8 +77,7 @@ drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
bool
drm_atomic_helper_damage_iter_next(struct drm_atomic_helper_damage_iter *iter,
struct drm_rect *rect);
-bool drm_atomic_helper_damage_merged(const struct drm_plane_state *old_state,
- const struct drm_plane_state *state,
+bool drm_atomic_helper_damage_merged(const struct drm_plane_state *state,
struct drm_rect *rect);
#endif
--
2.54.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox