dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Chen Li <chenli@uniontech.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: "Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	dri-devel@lists.freedesktop.org, "Chen Li" <chenli@uniontech.com>
Subject: Re: [PATCH] drm/[amdgpu|radeon]: fix memset on io mem
Date: Fri, 18 Dec 2020 14:14:52 +0800	[thread overview]
Message-ID: <87wnxfy71f.wl-chenli@uniontech.com> (raw)
In-Reply-To: <159c72db-1316-6155-2209-8e0e9a7f5224@arm.com>

On Thu, 17 Dec 2020 21:45:06 +0800,
Robin Murphy wrote:
> 
> On 2020-12-17 10:25, Christian König wrote:
> > Am 17.12.20 um 02:07 schrieb Chen Li:
> >> On Wed, 16 Dec 2020 22:19:11 +0800,
> >> Christian König wrote:
> >>> Am 16.12.20 um 14:48 schrieb Chen Li:
> >>>> On Wed, 16 Dec 2020 15:59:37 +0800,
> >>>> Christian König wrote:
> >>>>> [SNIP]
> >>>> Hi, Christian. I'm not sure why this change is a hack here. I cannot see
> >>>> the problem and wll be grateful if you give more explainations.
> >>> __memset is supposed to work on those addresses, otherwise you can't use the
> >>> e8860 on your arm64 system.
> >> If __memset is supposed to work on those adresses, why this
> >> commit(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftorvalds%2Flinux%2Fcommit%2Fba0b2275a6781b2f3919d931d63329b5548f6d5f&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7C4ed3c075888746b7f41408d8a22811c5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637437640274023350%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=HhWxUaLo3WpzoV6hjV%2BG1HICaIOXwsoNpzv5tNMNg8A%3D&amp;reserved=0)
> >> is needed? (I also notice drm/radeon didn't take this change though) just out
> >> of curiosity.
> > 
> > We generally accept those patches as cleanup in the kernel with the hope that
> > we can find a way to work around the userspace restrictions.
> > 
> > But when you also have this issue in userspace then there isn't much we can do
> > for you.
> > 
> >>> Replacing the the direct write in the kernel with calls to writel() or
> >>> memset_io() will fix that temporary, but you have a more general problem
> >>> here.
> >> I cannot see what's the more general problem here :( u mean performance?
> > 
> > No, not performance. See standards like OpenGL, Vulkan as well as VA-API and
> > VDPAU require that you can mmap() device memory and execute memset/memcpy on
> > the memory from userspace.
> > 
> > If your ARM base board can't do that for some then you can't use the hardware
> > with that board.
> 
> If the VRAM lives in a prefetchable PCI bar then on most sane Arm-based systems
> I believe it should be able to mmap() to userspace with the Normal memory type,
> where unaligned accesses and such are allowed, as opposed to the Device memory
> type intended for MMIO mappings, which has more restrictions but stricter
> ordering guarantees.
 
Hi, Robin. I cannot understand it allow unaligned accesses. prefetchable PCI bar should also be mmio, and accesses will end with device memory, so why does this allow unaligned access?
> Regardless of what happens elsewhere though, if something is mapped *into the
> kernel* with ioremap(), then it is fundamentally wrong per the kernel memory
> model to reference that mapping directly without using I/O accessors. That is
> not specific to any individual architecture, and Sparse should be screaming
> about it already. I guess in this case the UVD code needs to pay more attention
> to whether radeon_bo_kmap() ends up going via ttm_bo_ioremap() or not.
> 
> (I'm assuming the initial fault was memset() with 0 trying to perform "DC ZVA"
> on a Device-type mapping from ioremap() - FYI a stacktrace on its own without
> the rest of the error dump showing what actually triggered it isn't overly
> useful)
> 
> Robin.
why it may be 'DC ZVA'? I'm not sure the pc in initial kernel fault memset, but I capture the userspace crash pc: stp(128bit) or str with neon(also 128bit) to render node(/dev/dri/renderD128).
 
> >>>>> For amdgpu I suggest that we allocate the UVD message in GTT instead of
> >>>>> VRAM
> >>>>> since we don't have the hardware restriction for that on the new
> >>>>> generations.
> >>>>> 
> >>>> Thanks, I will try to dig into deeper. But what's the "hardware
> >>>> restriction" meaning here? I'm not familiar with video driver stack and amd
> >>>> gpu, sorry.
> >>> On older hardware (AGP days) the buffer had to be in VRAM (MMIO) memory, but
> >>> on
> >>> modern system GTT (system memory) works as well.
> >> IIUC, e8860 can use amdgpu(I use radeon now) beause its device id 6822 is in
> >> amdgpu's table. But I cannot tell whether e8860 has iommu, and I cannot find
> >> iommu from lspci, so graphics translation table may not work here?
> > 
> > That is not related to IOMMU. IOMMU is a feature of the CPU/motherboard. This
> > is implemented using GTT, e.g. the VM page tables inside the GPU.
> > 
> > And yes it should work I will prepare a patch for it.
> > 
> >>>>> BTW: How does userspace work on arm64 then? The driver stack usually only
> >>>>> works
> >>>>> if mmio can be mapped directly.
> >>>> I also post two usespace issue on mesa, and you may be interested with
> >>>> them:
> >>>>    https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fmesa%2F-%2Fissues%2F3954&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7C4ed3c075888746b7f41408d8a22811c5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637437640274023350%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ZR7pDS%2BCLUuMjCeKcMAXfHtbczt8WdUwSeLZCuHfCHw%3D&amp;reserved=0 
> >>>>    https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fmesa%2F-%2Fissues%2F3951&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7C4ed3c075888746b7f41408d8a22811c5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637437640274033344%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=jAJo3aG2I1oIDTZXWhNgcKoKbd6tTdiAtc7vE4hJJPY%3D&amp;reserved=0 
> >>>> I paste some virtual memory map in userspace there. (and the two problems
> >>>> do bother me quite a long time.)
> >>> I don't really see a solution for those problems.
> >>> 
> >>> See it is perfectly valid for an application to memset/memcpy on mmaped MMIO
> >>> space which comes from OpenGL or Vulkan.
> >>> 
> >>> So your CPU simply won't work with the hardware. We could work around that
> >>> with
> >>> a couple of hacks, but this is a pretty much general problem.
> >>> 
> >>> Regards,
> >>> Christian.
> >> Thanks! Can you provid some details about these hacks? Should I post another
> >> issue on the mail list?
> > 
> > Adjust the kernel and/or user space to never map VRAM to the CPU.
> > 
> > This violates the OpenGL/Vulkan specification in some ways. So not sure if
> > that will work or not.
> > 
> > Regards,
> > Christian.
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2020-12-18  8:33 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-16  5:41 [PATCH] drm/[amdgpu|radeon]: fix memset on io mem Chen Li
2020-12-16  7:59 ` Christian König
2020-12-16 13:48   ` Chen Li
2020-12-16 14:19     ` Christian König
2020-12-17  1:07       ` Chen Li
2020-12-17 10:25         ` Christian König
2020-12-17 13:37           ` Chen Li
2020-12-17 14:16             ` Christian König
2020-12-18  3:51               ` Chen Li
2020-12-18  8:10                 ` Christian König
2020-12-18  8:52                   ` Chen Li
2020-12-18 10:41                     ` Christian König
2020-12-17 13:45           ` Robin Murphy
2020-12-17 14:02             ` Christian König
2020-12-17 14:18               ` Lucas Stach
2020-12-18 14:17               ` Robin Murphy
2020-12-18 14:33                 ` Christian König
2020-12-18 15:19                   ` Robin Murphy
2020-12-18  6:14             ` Chen Li [this message]
2020-12-18 14:42               ` Robin Murphy
2020-12-16 12:52 ` [kbuild] " Dan Carpenter
2020-12-16 13:00 ` kernel test robot

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=87wnxfy71f.wl-chenli@uniontech.com \
    --to=chenli@uniontech.com \
    --cc=alexander.deucher@amd.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=robin.murphy@arm.com \
    /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