Re: [PATCH] ARC: Improve cmpxchng syscall implementation
Hi Vineet, Peter,
On Wed, 2018-03-21 at 14:54 +0300, Alexey Brodkin wrote:
> Hi Vineet,
>
> On Mon, 2018-03-19 at 11:29 -0700, Vineet Gupta wrote:
> > On 03/19/2018 04:00 AM, Alexey Brodkin wrote:
> > > arc_usr_cmpxchg syscall is supposed to be used on platforms
> > > that lack support of Load-Locked/Store-Conditional instructions
> > > in hardware. And in that case we mimic missing hardware features
> > > with help of kernel's sycall that "atomically" checks current
> > > value in memory and then if it matches caller expectation new
> > > value is written to that same location.
> > >
> >
> > ...
> > ...
> >
> > >
> > > 2. What's worse if we're dealing with data from not yet allocated
> > > page (think of pre-copy-on-write state) we'll successfully
> > > read data but on write we'll silently return to user-space
> > > with correct result
> >
> > This is technically incorrect, even for reading, you need a page, which
> > could be
> > common zero page in certain cases.
>
> Ok I'll reword it like.
>
> >
> > (which we really read just before). That leads
> > > to very strange problems in user-space app further down the line
> > > because new value was never written to the destination.
> > >
> > > 3. Regardless of what went wrong we'll return from syscall
> > > and user-space application will continue to execute.
> > > Even if user's pointer was completely bogus.
> >
> > Again we are exaggerating (from technical correctness POV) - if user
> > pointer was
> > bogs, the read would not have worked in first place etc. So lets tone down
> > the
> > rhetoric.
>
> Ok here I may rephrase it like that:
> --->8-
> 3. Regardless of what went wrong we'll return from syscall
>and user-space application will continue to execute.
> --->8-
>
> >
> > > In case of hardware LL/SC that app would have been killed
> > > by the kernel.
> > >
> > > With that change we attempt to imrove on all 3 items above:
> > >
> > > 1. We still disable preemption around read-and-write of
> > > user's data but if we happen to fail with either of them
> > > we're enabling preemption and try to force page fault so
> > > that we have a correct mapping in the TLB. Then re-try
> > > again in "atomic" context.
> > >
> > > 2. If real page fault fails or even access_ok() returns false
> > > we send SIGSEGV to the user-space process so if something goes
> > > seriously wrong we'll know about it much earlier.
> > >
> >
> >
> > >
> > > /*
> > >* This is only for old cores lacking LLOCK/SCOND, which by
> > > defintion
> > > @@ -60,23 +62,48 @@ SYSCALL_DEFINE3(arc_usr_cmpxchg, int *, uaddr, int,
> > > expected, int, new)
> > > /* Z indicates to userspace if operation succeded */
> > > regs->status32 &= ~STATUS_Z_MASK;
> > >
> > > - if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
> > > - return -EFAULT;
> > > + ret = access_ok(VERIFY_WRITE, uaddr, sizeof(*uaddr));
> > > + if (!ret)
> > > + goto fail;
> > >
> > > +again:
> > > preempt_disable();
> > >
> > > - if (__get_user(uval, uaddr))
> > > - goto done;
> > > -
> > > - if (uval == expected) {
> > > - if (!__put_user(new, uaddr))
> > > + ret = __get_user(val, uaddr);
> > > + if (ret == -EFAULT) {
> >
> >
> > Lets see if this warrants adding complexity ! This implies that TLB entry
> > with
> > Read permissions didn't exist for reading the var and page fault handler
> > could not
> > wire up even a zero page due to preempt_disable, meaning it was something
> > not
> > touched by userspace already - sort of uninitialized variable in user code.
>
> Ok I completely missed the fact that fast path TLB miss handler is being
> executed even if we have preemption disabled. So given the mapping exist
> we do not need to retry with enabled preemption.
>
> Still maybe I'm a bit paranoid here but IMHO it's good to be ready for a
> corner-case
> when the pointer is completely bogus and there's no mapping for him.
> I understand that today we only expect this syscall to be used from libc's
> internals but as long as syscall exists nobody stops anybody from using it
> directly without libc. So maybe instead of doing get_user_pages_fast() just
> send a SIGSEGV to the process? At least user will realize there's some problem
> at earlier stage.
>
> > Otherwise it is extremely unlikely to start with a TLB entry with Read
> > permissions, followed by syscall Trap only to find the entry missing,
> > unless a
> > global TLB flush came from other cores, right in the middle. But this
> > syscall is
> > not guaranteed to work with SMP anyways, so lets ignore any SMP misdoings
> > here.
>
> Well but that's exactly the situation I was debugging: we start from data
> from read-only
> page and on attempt to write back modif
Re: [PATCH v6 04/16] arc: Switch to generic free_initrd_mem.
Hi Shea, On Sun, 2018-04-01 at 10:59 -0400, Shea Levy wrote: > The first patch in this series added a weakly-defined generic > implementation, which is functionally identical to the > architecture-specific one removed here. > > Series boot-tested on RISC-V (which now uses the generic > implementation) and x86_64 (which doesn't). > > Signed-off-by: Shea Levy Boot-tested on ARC, thus... Tested-by: Alexey Brodkin -Alexey P.S. Note Vineet is out this week so please wait for him to return next week to ack your patch. ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: tst-atomic
Hi Waldemar,
On Tue, 2018-03-13 at 20:55 +0100, Waldemar Brodkorb wrote:
> Hi Alexey,
> Alexey Brodkin wrote,
>
> > Hi Waldemar,
> >
> > I see in commit e65912f8b2a6 ("rework most tests to work as standalone
> > package")
> > "tst-atomic" and "tst-atomic-long" were marked as disabled for all
> > architectures
> > as opposed to previously (in times of test-suite being a part of uClibc)
> > disabled
> > only for selected arches (IA64, MIPS and SPARC).
> >
> > The only guess I have it was done due to inability to refer not-exported
> > "atomic.h".
> > Is that correct? But anyways I'm looking at a possibility to revive those
> > as they
> > might be of use especially in case of emulation of atomics.
> >
> > Any thoughts on how that could be done?
>
> Should we export atomic.h again?
> The header is removed since
> 536a0e3a05a0c7d9a4d741d9401c220900cf1e04.
Probably not as this will open again a can with worms.
Fortunately I was able to get my problem understood and almost solved,
I guess you're on Cc list of that discussion here:
http://lists.infradead.org/pipermail/linux-snps-arc/2018-March/003583.html
But then why don't we get rid of ./test/silly/tst-atomic*.c at all?
-Alexey
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
DRM_UDL and GPU under Xserver
Hello, We're trying to use DisplayLink USB2-to-HDMI adapter to render GPU-accelerated graphics. Hardware setup is as simple as a devboard + DisplayLink adapter. Devboards we use for this experiment are: * Wandboard Quad (based on IMX6 SoC with Vivante GPU) or * HSDK (based on Synopsys ARC HS38 SoC with Vivante GPU as well) I'm sure any other board with DRM supported GPU will work, those we just used as the very recent Linux kernels could be easily run on them both. Basically the problem is UDL needs to be explicitly notified about new data to be rendered on the screen compared to typical bit-streamers that infinitely scan a dedicated buffer in memory. In case of UDL there're just 2 ways for this notification: 1) DRM_IOCTL_MODE_PAGE_FLIP that calls drm_crtc_funcs->page_flip() 2) DRM_IOCTL_MODE_DIRTYFB that calls drm_framebuffer_funcs->dirty() But neither of IOCTLs happen when we run Xserver with xf86-video-armada driver (see http://git.arm.linux.org.uk/cgit/xf86-video-armada.git/log/?h=unstable-devel). Is it something missing in Xserver or in UDL driver? Regards, Alexey ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: DRM_UDL and GPU under Xserver
On Wed, Apr 4, 2018 at 10:06 PM, Alexey Brodkin wrote: > Hello, > > We're trying to use DisplayLink USB2-to-HDMI adapter to render > GPU-accelerated graphics. > Hardware setup is as simple as a devboard + DisplayLink adapter. > Devboards we use for this experiment are: > * Wandboard Quad (based on IMX6 SoC with Vivante GPU) or > * HSDK (based on Synopsys ARC HS38 SoC with Vivante GPU as well) > > I'm sure any other board with DRM supported GPU will work, those we just used > as the very recent Linux kernels could be easily run on them both. > > Basically the problem is UDL needs to be explicitly notified about new data > to be rendered on the screen compared to typical bit-streamers that infinitely > scan a dedicated buffer in memory. > > In case of UDL there're just 2 ways for this notification: > 1) DRM_IOCTL_MODE_PAGE_FLIP that calls drm_crtc_funcs->page_flip() > 2) DRM_IOCTL_MODE_DIRTYFB that calls drm_framebuffer_funcs->dirty() > > But neither of IOCTLs happen when we run Xserver with xf86-video-armada driver > (see > http://git.arm.linux.org.uk/cgit/xf86-video-armada.git/log/?h=unstable-devel). > > Is it something missing in Xserver or in UDL driver? Use the -modesetting driverr for UDL, that one works correctly. Kernel-driver specific X drivers are kinda deprecated, and stuff like this (and other bugfixes and improvements that don't propagate around) are the reason for that. -Daniel > > Regards, > Alexey > > > > > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
