All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: Chen Li <chenli@uniontech.com>
Cc: "Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/[amdgpu|radeon]: fix memset on io mem
Date: Fri, 18 Dec 2020 14:42:35 +0000	[thread overview]
Message-ID: <90a89839-651d-71b0-b0eb-5535b6f6f4f5@arm.com> (raw)
In-Reply-To: <87wnxfy71f.wl-chenli@uniontech.com>

On 2020-12-18 06:14, Chen Li wrote:
[...]
>>> 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?

Because even Device-GRE is a bit too restrictive to expose to userspace 
that's likely to expect it to behave as regular memory, so, for better 
or worse, we use MT_NORMAL_MC for pgrprot_writecombine().

>> 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).

As I said it was an assumption. I guessed at it being more likely to be 
an MMU fault than an external abort, and given the size and the fact 
that it's a variable initialisation guessed at it being slightly more 
likely to hit the ZVA special-case rather than being unaligned. Looking 
again, I guess starting at an odd-numbered 32-bit element might lead to 
an unaligned store of XZR, but either way it doesn't really matter - 
what it showed is it clearly *could* be an MMU fault because TTM seems 
to be a bit careless with iomem pointers.

That said, if you're also getting external aborts from your host bridge 
not liking 128-bit transactions, then as Christian says you're probably 
going to have a bad time on this platform either way.

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

  reply	other threads:[~2020-12-18 14:42 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
2020-12-18 14:42               ` Robin Murphy [this message]
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=90a89839-651d-71b0-b0eb-5535b6f6f4f5@arm.com \
    --to=robin.murphy@arm.com \
    --cc=alexander.deucher@amd.com \
    --cc=chenli@uniontech.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    /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.