Re: [PATCH v6] arm: Preserve the user r/w register TPIDRURW oncontext, switch and fork

[Will Deacon]

On Tue, Jun 18, 2013 at 07:58:27PM +0100, André Hentschel wrote:
> On 18.06.2013 12:07, Will Deacon wrote:
> > On Mon, Jun 17, 2013 at 11:13:48PM +0100, André Hentschel wrote:
> >> From: André Hentschel <nerv@xxxxxxxxxxx>
> >>
> >> Since commit 6a1c53124aa1 the user writeable TLS register was zeroed to
> >> prevent it from being used as a covert channel between two tasks.
> >>
> >> There are more and more applications coming to Windows RT,
> >> Wine could support them, but mostly they expect to have
> >> the thread environment block (TEB) in TPIDRURW.
> >>
> >> This patch preserves that register per thread instead of clearing it.
> >> Unlike the TPIDRURO, which is already switched, the TPIDRURW
> >> can be updated from userspace so needs careful treatment in the case that we
> >> modify TPIDRURW and call fork(). To avoid this we must always read
> >> TPIDRURW in copy_thread.
> >>
> >> Signed-off-by: André Hentschel <nerv@xxxxxxxxxxx>
> >> Signed-off-by: Will Deacon <will.deacon@xxxxxxx>
> >> Signed-off-by: Jonathan Austin <jonathan.austin@xxxxxxx> 
> >>
> >> ---
> >> This patch is against Linux 3.10-rc6 (7d132055814ef17a6c7b69f342244c410a5e000f)
> >>
> >> v2: rework and fixup of v1, based on a suggested patch by Will Deacon
> >> v3: total rework and fixup of v2
> >> v4: removed condition on assembler instruction,
> >>     adapted my code to kernel-style, both based on comments by Will Deacon
> >> v5: rebased v4 on 3.10-rc2 and adding this version history
> >> v6: moved loading the TLS registers to the macros
> >>     (fixing the "LDRD is not supported on all the CPUs we have" problem)
> > 
> > You've changed quite a lot with this version, including the way the macro
> > parameters are passed. Why not just replace the problematic ldrd with two
> > ldr instructions and be done with it? I don't think the simple build error
> > warrants an overhaul of the code we already had.
> 
> Wantig that patch to be in 3.11 i thought i should do more now to push it.

It's getting pretty late for 3.11, so generally you'd want to make the most
minimal changes possible, rather than redesign core parts of the patch.

> I'm still not that familiar with the process, but i think Russell King would
> have done this easy change himself when he would be happy with it.

That's unlikely -- you're still the author on the patch so I wouldn't expect
the gatekeeper to make any changes above merge conflicts.

> Further this patch seems cleaner and much likely performing better.

Well, it's not the approach that has been reviewed up until now and, without
numbers backing up your claims, I'd be inclined simply to make the simple
change of adding two ldr instructions.

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/