Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755158AbbDTMc2 (ORCPT ); Mon, 20 Apr 2015 08:32:28 -0400 Received: from e06smtp15.uk.ibm.com ([195.75.94.111]:37738 "EHLO e06smtp15.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754354AbbDTMc1 (ORCPT ); Mon, 20 Apr 2015 08:32:27 -0400 In-Reply-To: <552B82F9.3080108@linux.vnet.ibm.com> References: <20141203052204.9DA8F1400DD@ozlabs.org> <54947C64.4030206@linux.vnet.ibm.com> <54A50094.5070902@linux.vnet.ibm.com> <1421883597.30744.3.camel@neuling.org> <1421963049.30744.23.camel@neuling.org> <1422419289.9646.20.camel@neuling.org> <1424667110.16027.6.camel@neuling.org> <1426718702.4866.2.camel@neuling.org> <1426719027.4866.4.camel@neuling.org> <550FEC36.8080803@linux.vnet.ibm.com> <1428534695.4682.18.camel@neuling.org> <55267595.90 <5527938B.1030901@linux.vnet.ibm.com> <552B82F9.3080108@linux.vnet.ibm.com> Subject: Re: [V6,1/9] elf: Add new powerpc specifc core note sections X-KeepSent: 939465E0:3429202B-C1257E2D:0043E1D3; type=4; name=$KeepSent To: Anshuman Khandual Cc: akpm@linux-foundation.org, avagin@openvz.org, davej@redhat.com, davem@davemloft.net, dhowells@redhat.com, Edjunior Barbosa Machado , james.hogan@imgtec.com, kirjanov@gmail.com, linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org, Michael Neuling , oleg@redhat.com, palves@redhat.com, Paul.Clothier@imgtec.com, peterz@infradead.org, sam.bobroff@au1.ibm.com, shuahkh@osg.samsung.com, sukadev@linux.vnet.ibm.com, tglx@linutronix.de X-Mailer: IBM Notes Release 9.0.1FP2 August 03, 2014 Message-ID: From: Ulrich Weigand Date: Mon, 20 Apr 2015 14:27:56 +0200 X-MIMETrack: Serialize by Router on D06ML032/06/M/IBM(Release 9.0.1FP3|January 12, 2015) at 20/04/2015 14:32:18 MIME-Version: 1.0 Content-type: text/plain; charset=US-ASCII X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15042012-0021-0000-0000-000003A30490 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4602 Lines: 122 Anshuman Khandual wrote on 13.04.2015 10:48:57: > On 04/10/2015 04:03 PM, Ulrich Weigand wrote: > > - You provide checkpointed FPR and VMX registers, but there doesn't seem > > to be any way to get at the checkpointed *VSX* registers (i.e. the part > > that is neither covered by FPR or VMX, corresponding to NT_PPC_VSX). > > Will change vsr_get, vsr_set functions as we have done for fpr_get and fpr_set > functions. Also will add one more ELF core note NT_PPC_TM_CVSX to fetch the > check pointed state of VSX register while inside the transaction. OK. > > I would much prefer three separate regsets (e.g. NT_PPC_DSCR, NT_PPC_PPR, > > and NT_PPC_TAR), each of which is available and valid if and only if the > > current processor actually has the register in question. > > Thats like adding one ELF core note for every single register > because we cannot > put them in any category. Then as Michael Ellerman had pointed out to include > a lot more registers in this MISC category (which we are not doing right now > in the interest of having minimum support available before we look at the full > possible list of MISC registers), we should add one ELF core note section for > each of those individual registers ? I am not sure. This confuses me a bit. My understanding was that ptrace regsets, once defined, should never change in the future. (GDB will only check whether or not a regset is supported; if it is, it will expect the contents to be as it expects them to be.) "Including a lot more registers" would therefore seem to require adding new regsets anyway, which is one of the reasons why I disagree a "MISC" regset is particularly useful. > > - Similarly, the NT_PPC_TM_SPR regset as currently defined mixes and > > matches > > registers with different "lifetimes". The transactional memory registers > > (TFHAR, TEXASR, TFIAR) are available *always* on machines that support > > transactions. But the other registers in that regset are checkpointed > > versions that are only available/valid within a transaction. I think a > > better way to faithfully represent this would be to have the > > NT_PPC_TM_SPR > > regset only contain the transcational memory registers, and use separate > > regsets for the checkpointed registers -- those should parallel the non- > > checkpointed register regset. > > Right now, we support NT_PPC_TM_SPR only inside the transaction, so we dont > have the problem with different "lifetimes" registers accessed together. But > yes, I get your point. Since the transactional SPRs are accessible from user space outside of a transaction, it would make sense for them to accessible from ptrace as well. If the current patch set doesn't do that, I guess it would be better to change that. > > - Particularly confusing to me is the "checkpointed original MSR" which > > currently also resides in NT_PPC_TM_SPR. What exactly is this? How > > does that differ from the MSR slot in the NT_PPC_TM_CGPR regset? > > I believed it stores the check pointed MSR value which was in the register > before the transaction started. But then how it is different from the > ckpt_regs.msr, I am not sure. Mikey or Michael should be able to clarify > more on this. I can see "orig_msr" getting used in many places to hold the > check pointed value of MSR. Your other mail states that the orig_mst may be irrelevant for ptrace anyway ... that would be OK with me as well. > > In any case, it seems the ptrace set-register case currently allows user > > space to restore *any* arbitrary value into the checkpointed MSR, which > > would presumably get restored into the real MSR at some point, unless I'm > > missing something here. Do we not need a check that only safe bits are > > modified, just like with ptrace access to the real MSR? > > Where and which safe bits do we check before writing any value into the MSR > register from ptrace interface ? May be I am missing something here. All ptrace accesses to *set* the regular msr go via this routine: static int set_user_msr(struct task_struct *task, unsigned long msr) { task->thread.regs->msr &= ~MSR_DEBUGCHANGE; task->thread.regs->msr |= msr & MSR_DEBUGCHANGE; return 0; } I think we'd need to do the equivalent whenever changing the checkpointed MSR. Bye, Ulrich -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/