From: Gerd Hoffmann <kraxel@redhat.com>
To: "Thomas Hellström (VMware)" <thomas_os@shipmail.org>
Cc: dri-devel@lists.freedesktop.org, Guillaume.Gardet@arm.com,
David Airlie <airlied@linux.ie>,
open list <linux-kernel@vger.kernel.org>,
stable@vger.kernel.org, gurchetansingh@chromium.org,
tzimmermann@suse.de
Subject: Re: [PATCH v5 1/3] drm/shmem: add support for per object caching flags.
Date: Thu, 27 Feb 2020 08:53:21 +0100 [thread overview]
Message-ID: <20200227075321.ki74hfjpnsqv2yx2@sirius.home.kraxel.org> (raw)
In-Reply-To: <f1afba4b-9c06-48a3-42c7-046695947e91@shipmail.org>
Hi,
> > + if (!shmem->map_cached)
> > + prot = pgprot_writecombine(prot);
> > shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT,
> > - VM_MAP, pgprot_writecombine(PAGE_KERNEL));
> > + VM_MAP, prot)
>
>
> Wouldn't a vmap with pgprot_writecombine() create conflicting mappings with
> the linear kernel map which is not write-combined?
I think so, yes.
> Or do you change the linear kernel map of the shmem pages somewhere?
Havn't seen anything doing so while browsing the code.
> vmap bypassess at least the
> x86 PAT core mapping consistency check and this could potentially cause
> spuriously overwritten memory.
Well, I don't think the linear kernel map is ever used to access the
shmem gem objects. So while this isn't exactly clean it shouldn't
cause problems in practice.
Suggestions how to fix that?
The reason I need cachable gem object mappings for virtio-gpu is because
we have a inconsistency between host (cached) and guest (wc) otherwise.
> > + }
> > if (!shmem->vaddr) {
> > DRM_DEBUG_KMS("Failed to vmap pages\n");
> > @@ -540,7 +545,9 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> > }
> > vma->vm_flags |= VM_MIXEDMAP | VM_DONTEXPAND;
> > - vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> > + vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> > + if (!shmem->map_cached)
> > + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
>
> Same thing here. Note that vmf_insert_page() which is used by the fault
> handler also bypasses the x86 PAT consistency check, whereas
> vmf_insert_mixed() doesn't.
vmap + mmap are consistent though, so this likewise shouldn't cause
issues in practice.
> > vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
>
> At least with SME or SEV encryption, where shmem memory has its kernel map
> set to encrypted, creating conflicting mappings is explicitly disallowed.
> BTW, why is mmap mapping decrypted while vmap isn't?
Ok, that sounds like a real problem. Have to check.
cheers,
Gerd
PS: Given we are discussing pre-existing issues in the code I think the
series can be merged nevertheless.
next prev parent reply other threads:[~2020-02-27 7:53 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200226154752.24328-1-kraxel@redhat.com>
2020-02-26 15:47 ` [PATCH v5 1/3] drm/shmem: add support for per object caching flags Gerd Hoffmann
2020-02-26 16:51 ` Guillaume Gardet
2020-02-26 18:24 ` Thomas Hellström (VMware)
2020-02-27 0:02 ` Chia-I Wu
2020-02-27 7:16 ` Thomas Hellström (VMware)
2020-02-27 7:53 ` Gerd Hoffmann [this message]
2020-02-27 8:10 ` Thomas Hellström (VMware)
2020-02-27 10:56 ` Gerd Hoffmann
2020-02-27 12:16 ` Thomas Hellström (VMware)
2020-02-27 13:21 ` Gerd Hoffmann
2020-02-27 13:44 ` Thomas Hellström (VMware)
2020-02-27 13:49 ` Thomas Hellström (VMware)
2020-02-28 9:49 ` Gerd Hoffmann
2020-02-28 9:54 ` Thomas Hellström (VMware)
2020-02-28 10:46 ` Gerd Hoffmann
2020-03-02 1:56 ` Qiang Yu
2020-02-26 15:47 ` [PATCH v5 2/3] drm/virtio: fix mmap page attributes Gerd Hoffmann
2020-02-26 16:52 ` Guillaume Gardet
2020-02-26 15:47 ` [PATCH v5 3/3] drm/udl: simplify gem object mapping Gerd Hoffmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200227075321.ki74hfjpnsqv2yx2@sirius.home.kraxel.org \
--to=kraxel@redhat.com \
--cc=Guillaume.Gardet@arm.com \
--cc=airlied@linux.ie \
--cc=dri-devel@lists.freedesktop.org \
--cc=gurchetansingh@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=thomas_os@shipmail.org \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).