Re: [RFC v5 32/38] powerpc: capture the violated protection key on fault

[Ram Pai]

On Mon, Jul 10, 2017 at 08:40:19AM +0530, Anshuman Khandual wrote:
> On 07/06/2017 02:52 AM, Ram Pai wrote:
> > Capture the protection key that got violated in paca.
> > This value will be used by used to inform the signal
> > handler.
> > 
> > Signed-off-by: Ram Pai <linuxram@xxxxxxxxxx>
> > ---
> >  arch/powerpc/include/asm/paca.h   |    1 +
> >  arch/powerpc/kernel/asm-offsets.c |    1 +
> >  arch/powerpc/mm/fault.c           |    3 +++
> >  3 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
> > index c8bd1fc..0c06188 100644
> > --- a/arch/powerpc/include/asm/paca.h
> > +++ b/arch/powerpc/include/asm/paca.h
> > @@ -94,6 +94,7 @@ struct paca_struct {
> >  	u64 dscr_default;		/* per-CPU default DSCR */
> >  #ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> >  	u64 paca_amr;			/* value of amr at exception */
> > +	u16 paca_pkey;                  /* exception causing pkey */
> >  #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> >  
> >  #ifdef CONFIG_PPC_STD_MMU_64
> > diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> > index 17f5d8a..7dff862 100644
> > --- a/arch/powerpc/kernel/asm-offsets.c
> > +++ b/arch/powerpc/kernel/asm-offsets.c
> > @@ -244,6 +244,7 @@ int main(void)
> >  
> >  #ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> >  	OFFSET(PACA_AMR, paca_struct, paca_amr);
> > +	OFFSET(PACA_PKEY, paca_struct, paca_pkey);
> >  #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> >  
> >  	OFFSET(ACCOUNT_STARTTIME, paca_struct, accounting.starttime);
> > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> > index a6710f5..c8674a7 100644
> > --- a/arch/powerpc/mm/fault.c
> > +++ b/arch/powerpc/mm/fault.c
> > @@ -265,6 +265,7 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
> >  	if (error_code & DSISR_KEYFAULT) {
> >  		code = SEGV_PKUERR;
> >  		get_paca()->paca_amr = read_amr();
> > +		get_paca()->paca_pkey = get_pte_pkey(current->mm, address);
> >  		goto bad_area_nosemaphore;
> >  	}
> >  #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> > @@ -290,6 +291,7 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
> >  
> >  	perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address);
> >  
> > +
> 
> Stray empty line addition here.
> 
> >  	/*
> >  	 * We want to do this outside mmap_sem, because reading code around nip
> >  	 * can result in fault, which will cause a deadlock when called with
> > @@ -453,6 +455,7 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
> >  	if (!arch_vma_access_permitted(vma, flags & FAULT_FLAG_WRITE,
> >  			is_exec, 0)) {
> >  		get_paca()->paca_amr = read_amr();
> > +		get_paca()->paca_pkey = vma_pkey(vma);
> 
> Why not get_pte_pkey() here as well ? IIUC both these function would
> give us the same pkey, then why is the difference when we process a
> page fault for real protection key violation in HW compared to cross
> checking of VMA protection key in SW for regular page faults.

Unfortunately if we have reached here, it means the pgd-pmd-pdt-...pte
structures have not yet been totally populated for the task. Hence we
cannot walk the tree, to find the pte, to find the key. hence we have to 
depend on vma_pkey() to get the key from the vma.

RP




-- 
Ram Pai