linux-kernel - Re: [PATCH] arm64: fpsimd: Added API to manage fpsimd state inside kernel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20200615103024.GI25945@arm.com>
Date:   Mon, 15 Jun 2020 11:30:27 +0100
From:   Dave Martin <Dave.Martin@....com>
To:     Catalin Marinas <catalin.marinas@....com>
Cc:     Wooyeon Kim <wooy88.kim@...sung.com>,
        'Mark Rutland' <mark.rutland@....com>,
        'Bhupesh Sharma' <bhsharma@...hat.com>,
        'Julien Grall' <julien.grall@....com>,
        'Vincenzo Frascino' <vincenzo.frascino@....com>,
        'Will Deacon' <will@...nel.org>, yhwan.joo@...sung.com,
        'Anisse Astier' <aastier@...ebox.fr>,
        'Marc Zyngier' <maz@...nel.org>,
        'Allison Randal' <allison@...utok.net>,
        'Sanghoon Lee' <shoon114.lee@...sung.com>,
        'Wooki Min' <wooki.min@...sung.com>,
        'Kees Cook' <keescook@...omium.org>,
        'Suzuki K Poulose' <suzuki.poulose@....com>,
        jihun.kim@...sung.com,
        'Kristina Martsenko' <kristina.martsenko@....com>,
        'Jeongtae Park' <jtp.park@...sung.com>,
        'Thomas Gleixner' <tglx@...utronix.de>,
        linux-arm-kernel@...ts.infradead.org,
        'Steve Capper' <steve.capper@....com>,
        'Greg Kroah-Hartman' <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, 'James Morse' <james.morse@....com>,
        'Sudeep Holla' <sudeep.holla@....com>, dh.han@...sung.com
Subject: Re: [PATCH] arm64: fpsimd: Added API to manage fpsimd state inside
 kernel

On Thu, Jun 11, 2020 at 03:11:02PM +0100, Catalin Marinas wrote:
> On Thu, Jun 11, 2020 at 06:42:12PM +0900, Wooyeon Kim wrote:
> > I am in charge of camera driver development in Samsung S.LSI division.
> > 
> > In order to guarantee real time processing such as Camera 3A algorithm in
> > current or ongoing projects, prebuilt binary is loaded and used in kernel
> > space, rather than user space.
> 
> Thanks for the additional details.

I have to ask: there are other camera drivers in existence already.
What makes your hardware so different that it requires all this data
processing to be done inside the kernel?

> If you do such intensive processing in an IRQ context you'd probably
> introduce additional IRQ latency. Wouldn't offloading such work to a
> real-time (user) thread help? In a non-preempt-rt kernel, I don't think
> you can get much in terms of (soft) guarantees for IRQ latency anyway.
> 
> > Because the binary is built with other standard library which could use
> > FPSIMD register, kernel API should keep the original FPSIMD state for other
> > user tasks.
> 
> Can you not recompile those libraries not to use FP?
> 
> As Mark said, for a kernel API we require at least an in-kernel,
> upstreamed, user of that functionality.
> 
> > In the case of the kernel_neon_begin / kernel_neon_end that you mentioned,
> > there is a limitation that cannot be used in hardirq context.
> > Also, if another kernel task switching occurs while kernel API is being
> > used, fpsimd register corruption may occur.
> 
> kernel_neon_begin/end disable preemption, so you can't have a task
> switch (you can have interrupts though but we don't allow FPSIMD in IRQ
> context).

Note, the decision not to support kernel_neon_begin / kernel_neon_end in
hardirq context was deliberate.  hardirq handlers shouldn't usually do
anything at all except ensure that something responds to the hardware
event, by waking some other thread or scheduling a workqueue item for
example.  An IRQ handler that only does that has no need to do any data
processing, and gains no advantage from using FPSIMD.

Doing additional work in hardirq context will harm interrupt latency for
the rest of the system.

So, you should move the data processing work out of the hardirq handler.
Is there a reason why this is not possible?

Secondly, there is the question of whether FPSIMD can be used by kernel
threads.  Currently this is only supported in a limited way.  Again,
this a deliberate decision, for now.

Can you split the processing work into small blocks using
kernel_neon_begin/kernel_neon_end, similarly to the arm64 crypto
drivers?

This is the current accepted way of doing number crunching inside the
kernel without harming preemption latency too much.  Even so, it's
primarily intended for things that affect critical paths inside the
kernel, such as crypto or checksumming in the filesysem and network
subsystems.

Cheers
---Dave