All of 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: 26+ 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 12:52   ` Dan Carpenter
2020-12-16 12:52   ` Dan Carpenter
2020-12-16 13:00 ` kernel test robot
2020-12-16 13:00   ` kernel test robot
2020-12-16 12:12 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.