linux-kernel - Re: [PATCH v2] drm/qxl: fix qxl can't use in arm64

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMwc25r3ER2Mi4jo2W=VtZcusNhpsyhQ0Mjx+qHhmZAK-fSjZQ@mail.gmail.com>
Date:   Wed, 30 Mar 2022 20:43:18 +1000
From:   David Airlie <airlied@...hat.com>
To:     Cong Liu <liucong2@...inos.cn>
Cc:     Christian König <christian.koenig@....com>,
        airlied <airlied@...ux.ie>, "Hoffmann, Gerd" <kraxel@...hat.com>,
        robin.murphy@....com,
        "open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE" 
        <virtualization@...ts.linux-foundation.org>,
        "Vetter, Daniel" <daniel@...ll.ch>,
        dri-devel <dri-devel@...ts.freedesktop.org>, ray.huang@....com,
        "open list:DRM DRIVER FOR QXL VIRTUAL GPU" 
        <spice-devel@...ts.freedesktop.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] drm/qxl: fix qxl can't use in arm64

I'd like to make sure this has no side effects on x86 guests, it
probably is safe, but keep an eye for regression reports.

Reviewed-by: Dave Airlie <airlied@...hat.com>
Dave.

On Wed, Mar 30, 2022 at 8:20 PM Cong Liu <liucong2@...inos.cn> wrote:
>
> any suggestions or extra test I can do now?
>
> Regards,
> Cong
>
> On 2022/3/25 15:45, Christian König wrote:
> > Am 24.03.22 um 11:49 schrieb Cong Liu:
> >> qxl use ioremap to map ram_header and rom, in the arm64 implementation,
> >> the device is mapped as DEVICE_nGnRE, it can not support unaligned
> >> access. and qxl is a virtual device, it can be treated more like RAM
> >> than actual MMIO registers. use ioremap_wc() replace it.
> >>
> >> Signed-off-by: Cong Liu <liucong2@...inos.cn>
> >
> > Looks sane to me, but I'm really not involved enough to fully judge.
> >
> > Acked-by: Christian König <christian.koenig@....com>
> >
> >> ---
> >>   drivers/gpu/drm/qxl/qxl_kms.c | 4 ++--
> >>   drivers/gpu/drm/qxl/qxl_ttm.c | 4 ++--
> >>   2 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c
> >> b/drivers/gpu/drm/qxl/qxl_kms.c
> >> index 4dc5ad13f12c..a054e4a00fe8 100644
> >> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> >> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> >> @@ -165,7 +165,7 @@ int qxl_device_init(struct qxl_device *qdev,
> >>            (int)qdev->surfaceram_size / 1024,
> >>            (sb == 4) ? "64bit" : "32bit");
> >> -    qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
> >> +    qdev->rom = ioremap_wc(qdev->rom_base, qdev->rom_size);
> >>       if (!qdev->rom) {
> >>           pr_err("Unable to ioremap ROM\n");
> >>           r = -ENOMEM;
> >> @@ -183,7 +183,7 @@ int qxl_device_init(struct qxl_device *qdev,
> >>           goto rom_unmap;
> >>       }
> >> -    qdev->ram_header = ioremap(qdev->vram_base +
> >> +    qdev->ram_header = ioremap_wc(qdev->vram_base +
> >>                      qdev->rom->ram_header_offset,
> >>                      sizeof(*qdev->ram_header));
> >>       if (!qdev->ram_header) {
> >> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c
> >> b/drivers/gpu/drm/qxl/qxl_ttm.c
> >> index b2e33d5ba5d0..95df5750f47f 100644
> >> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> >> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> >> @@ -82,13 +82,13 @@ int qxl_ttm_io_mem_reserve(struct ttm_device *bdev,
> >>       case TTM_PL_VRAM:
> >>           mem->bus.is_iomem = true;
> >>           mem->bus.offset = (mem->start << PAGE_SHIFT) + qdev->vram_base;
> >> -        mem->bus.caching = ttm_cached;
> >> +        mem->bus.caching = ttm_write_combined;
> >>           break;
> >>       case TTM_PL_PRIV:
> >>           mem->bus.is_iomem = true;
> >>           mem->bus.offset = (mem->start << PAGE_SHIFT) +
> >>               qdev->surfaceram_base;
> >> -        mem->bus.caching = ttm_cached;
> >> +        mem->bus.caching = ttm_write_combined;
> >>           break;
> >>       default:
> >>           return -EINVAL;
> >
>