LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [5.6.0-rc7] Kernel crash while running ndctl tests
From: Baoquan He @ 2020-03-24  7:07 UTC (permalink / raw)
  To: Sachin Sant; +Cc: linuxppc-dev, LKML, linux-nvdimm
In-Reply-To: <CF20E440-4DCB-4EFD-88B6-0AB98DC7FBD4@linux.vnet.ibm.com>

Hi Sachin,

On 03/24/20 at 11:25am, Sachin Sant wrote:
> While running ndctl[1] tests against 5.6.0-rc7 following crash is encountered.
> 
> Bisect leads me to  commit d41e2f3bd546 
> mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case
> 
> Reverting this commit helps and the tests complete without any crash.

Could you paste your kernel config and the boot log?

If it's confidential, private attachment is also OK.

Thanks
Baoquan

> 
> pmem0: detected capacity change from 0 to 10720641024
> BUG: Kernel NULL pointer dereference on read at 0x00000000
> Faulting instruction address: 0xc000000000c3447c
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> Dumping ftrace buffer:
>    (ftrace buffer empty)
> Modules linked in: dm_mod nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c ip6_tables nft_compat ip_set rfkill nf_tables nfnetlink sunrpc sg pseries_rng papr_scm uio_pdrv_genirq uio sch_fq_codel ip_tables sd_mod t10_pi ibmvscsi scsi_transport_srp ibmveth
> CPU: 11 PID: 7519 Comm: lt-ndctl Not tainted 5.6.0-rc7-autotest #1
> NIP:  c000000000c3447c LR: c000000000088354 CTR: c00000000018e990
> REGS: c0000006223fb630 TRAP: 0300   Not tainted  (5.6.0-rc7-autotest)
> MSR:  800000000280b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 24048888  XER: 00000000
> CFAR: c00000000000dec4 DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0 
> GPR00: c0000000003c5820 c0000006223fb8c0 c000000001684900 0000000004000000 
> GPR04: c00c000101000000 0000000007ffffff c00000067ff20900 c00c000000000000 
> GPR08: 0000000000000000 c00c000100000000 0000000000000000 c000000003f00000 
> GPR12: 0000000000008000 c00000001ec70200 00007fffc102f9e8 000000001002e088 
> GPR16: 0000000000000000 0000000010050d88 000000001002f778 000000001002f770 
> GPR20: 0000000000000000 0000000000000100 0000000000000001 0000000000001000 
> GPR24: 0000000000000008 0000000000000000 0000000004000000 c00c000100004000 
> GPR28: c000000003101aa0 c00c000100000000 0000000001000000 0000000004000100 
> NIP [c000000000c3447c] vmemmap_populated+0x98/0xc0
> LR [c000000000088354] vmemmap_free+0x144/0x320
> Call Trace:
> [c0000006223fb8c0] [c0000006223fb960] 0xc0000006223fb960 (unreliable)
> [c0000006223fb980] [c0000000003c5820] section_deactivate+0x220/0x240
> [c0000006223fba30] [c0000000003dc1d8] __remove_pages+0x118/0x170
> [c0000006223fba80] [c000000000086e5c] arch_remove_memory+0x3c/0x150
> [c0000006223fbb00] [c00000000041a3bc] memunmap_pages+0x1cc/0x2f0
> [c0000006223fbb80] [c0000000007d6d00] devm_action_release+0x30/0x50
> [c0000006223fbba0] [c0000000007d7de8] release_nodes+0x2f8/0x3e0
> [c0000006223fbc50] [c0000000007d0b38] device_release_driver_internal+0x168/0x270
> [c0000006223fbc90] [c0000000007ccf50] unbind_store+0x130/0x170
> [c0000006223fbcd0] [c0000000007cc0b4] drv_attr_store+0x44/0x60
> [c0000006223fbcf0] [c00000000051fdb8] sysfs_kf_write+0x68/0x80
> [c0000006223fbd10] [c00000000051f200] kernfs_fop_write+0x100/0x290
> [c0000006223fbd60] [c00000000042037c] __vfs_write+0x3c/0x70
> [c0000006223fbd80] [c00000000042404c] vfs_write+0xcc/0x240
> [c0000006223fbdd0] [c00000000042442c] ksys_write+0x7c/0x140
> [c0000006223fbe20] [c00000000000b278] system_call+0x5c/0x68
> Instruction dump:
> 2ea80000 4196003c 794a2428 7d685215 41820030 7d48502a 71480002 41820024 
> 714a0008 4082002c e90b0008 786adf62 <e8680000> 7c635436 70630001 4c820020 
> ---[ end trace 579b48162da1b890 ]—
> 
> Thanks
> -Sachin
> 
> [1] https://github.com/avocado-framework-tests/avocado-misc-tests/blob/master/memory/ndctl.py
> 


^ permalink raw reply

* Re: [PATCH] arch/powerpc/mm: Enable compound page check for both THP and HugeTLB
From: Aneesh Kumar K.V @ 2020-03-24  6:46 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev, paulus
In-Reply-To: <87eetixnom.fsf@mpe.ellerman.id.au>

On 3/24/20 12:08 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> THP config can result in compound pages. Make sure kernel enables the
>> PageCompound() check when only THP is enabled.
> 
> Or else what happens ... nothing, rampant data corruption, something in
> between?
> 

We can get a stale icache that can result in undefined behavior?



> And "when only THP is enabled" is not very clear, AFAIK there is no
> relation between CONFIG_TRANSPARENT_HUGEPAGE and CONFIG_HUGETLB_PAGE.
> 


yes, there is no relation between them. But the way current code is 
enabled if we have both enabled, we will find that
if (PageCompound(page)) check present which will handle THP case too.

Now with current code if we have CONFIG_HUGETLB_PAGE disabled, we 
compile out if (pageCompound(page)) check and hence don't invalidate 
compound pages correctly (THP create compound pages here)

> You mean when either or both of THP or HUGETLB is enabled right?
> 
> cheers
> 
> 
>> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
>> index 9b4f5fb719e0..b03cbddf9054 100644
>> --- a/arch/powerpc/mm/mem.c
>> +++ b/arch/powerpc/mm/mem.c
>> @@ -485,7 +485,7 @@ EXPORT_SYMBOL(flush_dcache_page);
>>   
>>   void flush_dcache_icache_page(struct page *page)
>>   {
>> -#ifdef CONFIG_HUGETLB_PAGE
>> +#if defined(CONFIG_TRANSPARENT_HUGEPAGE) || defined(CONFIG_HUGETLB_PAGE)
>>   	if (PageCompound(page)) {
>>   		flush_dcache_icache_hugepage(page);
>>   		return;
>> -- 
>> 2.25.1


^ permalink raw reply

* Re: [PATCH] arch/powerpc/mm: Enable compound page check for both THP and HugeTLB
From: Michael Ellerman @ 2020-03-24  6:38 UTC (permalink / raw)
  To: Aneesh Kumar K.V, linuxppc-dev, paulus; +Cc: Aneesh Kumar K.V
In-Reply-To: <20200320103256.229365-1-aneesh.kumar@linux.ibm.com>

"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> THP config can result in compound pages. Make sure kernel enables the
> PageCompound() check when only THP is enabled.

Or else what happens ... nothing, rampant data corruption, something in
between?

And "when only THP is enabled" is not very clear, AFAIK there is no
relation between CONFIG_TRANSPARENT_HUGEPAGE and CONFIG_HUGETLB_PAGE.

You mean when either or both of THP or HUGETLB is enabled right?

cheers


> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 9b4f5fb719e0..b03cbddf9054 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -485,7 +485,7 @@ EXPORT_SYMBOL(flush_dcache_page);
>  
>  void flush_dcache_icache_page(struct page *page)
>  {
> -#ifdef CONFIG_HUGETLB_PAGE
> +#if defined(CONFIG_TRANSPARENT_HUGEPAGE) || defined(CONFIG_HUGETLB_PAGE)
>  	if (PageCompound(page)) {
>  		flush_dcache_icache_hugepage(page);
>  		return;
> -- 
> 2.25.1

^ permalink raw reply

* Re: [PATCH] cpufreq: powernv: Fix frame-size-overflow in powernv_cpufreq_work_fn
From: Michael Ellerman @ 2020-03-24  6:34 UTC (permalink / raw)
  To: Rafael J. Wysocki, Pratik Rajesh Sampat
  Cc: ego, pratik.r.sampat, linux-pm, linux-kernel, linuxppc-dev, dja
In-Reply-To: <1921198.IfoiWgUDIW@kreacher>

"Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
> On Monday, March 16, 2020 2:57:43 PM CET Pratik Rajesh Sampat wrote:
>> The patch avoids allocating cpufreq_policy on stack hence fixing frame
>> size overflow in 'powernv_cpufreq_work_fn'
>> 
>> Fixes: 227942809b52 ("cpufreq: powernv: Restore cpu frequency to policy->cur on unthrottling")
>> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
>
> Any objections or concerns here?
>
> If not, I'll queue it up.

I have it in my testing branch, but if you pick it up I can drop it.

cheers

>> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
>> index 56f4bc0d209e..20ee0661555a 100644
>> --- a/drivers/cpufreq/powernv-cpufreq.c
>> +++ b/drivers/cpufreq/powernv-cpufreq.c
>> @@ -902,6 +902,7 @@ static struct notifier_block powernv_cpufreq_reboot_nb = {
>>  void powernv_cpufreq_work_fn(struct work_struct *work)
>>  {
>>  	struct chip *chip = container_of(work, struct chip, throttle);
>> +	struct cpufreq_policy *policy;
>>  	unsigned int cpu;
>>  	cpumask_t mask;
>>  
>> @@ -916,12 +917,14 @@ void powernv_cpufreq_work_fn(struct work_struct *work)
>>  	chip->restore = false;
>>  	for_each_cpu(cpu, &mask) {
>>  		int index;
>> -		struct cpufreq_policy policy;
>>  
>> -		cpufreq_get_policy(&policy, cpu);
>> -		index = cpufreq_table_find_index_c(&policy, policy.cur);
>> -		powernv_cpufreq_target_index(&policy, index);
>> -		cpumask_andnot(&mask, &mask, policy.cpus);
>> +		policy = cpufreq_cpu_get(cpu);
>> +		if (!policy)
>> +			continue;
>> +		index = cpufreq_table_find_index_c(policy, policy->cur);
>> +		powernv_cpufreq_target_index(policy, index);
>> +		cpumask_andnot(&mask, &mask, policy->cpus);
>> +		cpufreq_cpu_put(policy);
>>  	}
>>  out:
>>  	put_online_cpus();
>> 

^ permalink raw reply

* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Aneesh Kumar K.V @ 2020-03-24  6:30 UTC (permalink / raw)
  To: Alexey Kardashevskiy, Christoph Hellwig
  Cc: Greg Kroah-Hartman, Joerg Roedel, linuxppc-dev, linux-kernel,
	iommu, Robin Murphy, Lu Baolu
In-Reply-To: <ffce1af6-a215-dee8-7b5c-2111f43accfd@ozlabs.ru>

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> On 24/03/2020 04:22, Christoph Hellwig wrote:
>> On Mon, Mar 23, 2020 at 09:07:38PM +0530, Aneesh Kumar K.V wrote:
>>>
>>> This is what I was trying, but considering I am new to DMA subsystem, I
>>> am not sure I got all the details correct. The idea is to look at the
>>> cpu addr and see if that can be used in direct map fashion(is
>>> bus_dma_limit the right restriction here?) if not fallback to dynamic
>>> IOMMU mapping.
>> 
>> I don't think we can throw all these complications into the dma
>> mapping code.  At some point I also wonder what the point is,
>> especially for scatterlist mappings, where the iommu can coalesce.
>
> This is for persistent memory which you can DMA to/from but yet it does
> not appear in the system as a normal memory and therefore requires
> special handling anyway (O_DIRECT or DAX, I do not know the exact
> mechanics). All other devices in the system should just run as usual,
> i.e. use 1:1 mapping if possible.

This is O_DIRECT with a user buffer that is actually mmap from a dax
mounted file system.

What we really need is something that will falback to iommu_map_page
based on dma_addr. ie. Something equivalent to current
dma_direct_map_page(), but instead of fallback to swiotlb_map page we
should fallback to iommu_map_page().

Something like?

dma_addr_t dma_direct_map_page(struct device *dev, struct page *page,
		unsigned long offset, size_t size, enum dma_data_direction dir,
		unsigned long attrs)
{
	phys_addr_t phys = page_to_phys(page) + offset;
	dma_addr_t dma_addr = phys_to_dma(dev, phys);

	if (unlikely(!dma_capable(dev, dma_addr, size, true))) {
			return iommu_map(dev, phys, size, dir, attrs);

		return DMA_MAPPING_ERROR;
	}

....
...


-aneesh

^ permalink raw reply

* Re: [PATCH v5 10/13] powerpc/ptrace: split out ADV_DEBUG_REGS related functions.
From: Michael Ellerman @ 2020-03-24  6:23 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras, mikey
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <25a7f050-f241-6035-e778-16b1ca9928f3@c-s.fr>

Christophe Leroy <christophe.leroy@c-s.fr> writes:
> On 03/20/2020 02:12 AM, Michael Ellerman wrote:
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>> Move ADV_DEBUG_REGS functions out of ptrace.c, into
>>> ptrace-adv.c and ptrace-noadv.c
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>> ---
>>> v4: Leave hw_breakpoint.h for ptrace.c
>>> ---
>>>   arch/powerpc/kernel/ptrace/Makefile       |   4 +
>>>   arch/powerpc/kernel/ptrace/ptrace-adv.c   | 468 ++++++++++++++++
>>>   arch/powerpc/kernel/ptrace/ptrace-decl.h  |   5 +
>>>   arch/powerpc/kernel/ptrace/ptrace-noadv.c | 236 ++++++++
>>>   arch/powerpc/kernel/ptrace/ptrace.c       | 650 ----------------------
>>>   5 files changed, 713 insertions(+), 650 deletions(-)
>>>   create mode 100644 arch/powerpc/kernel/ptrace/ptrace-adv.c
>>>   create mode 100644 arch/powerpc/kernel/ptrace/ptrace-noadv.c
>> 
>> This is somehow breaking the ptrace-hwbreak selftest on Power8:
>> 
>>    test: ptrace-hwbreak
>>    tags: git_version:v5.6-rc6-892-g7a285a6067d6
>>    PTRACE_SET_DEBUGREG, WO, len: 1: Ok
>>    PTRACE_SET_DEBUGREG, WO, len: 2: Ok
>>    PTRACE_SET_DEBUGREG, WO, len: 4: Ok
>>    PTRACE_SET_DEBUGREG, WO, len: 8: Ok
>>    PTRACE_SET_DEBUGREG, RO, len: 1: Ok
>>    PTRACE_SET_DEBUGREG, RO, len: 2: Ok
>>    PTRACE_SET_DEBUGREG, RO, len: 4: Ok
>>    PTRACE_SET_DEBUGREG, RO, len: 8: Ok
>>    PTRACE_SET_DEBUGREG, RW, len: 1: Ok
>>    PTRACE_SET_DEBUGREG, RW, len: 2: Ok
>>    PTRACE_SET_DEBUGREG, RW, len: 4: Ok
>>    PTRACE_SET_DEBUGREG, RW, len: 8: Ok
>>    PPC_PTRACE_SETHWDEBUG, MODE_EXACT, WO, len: 1: Ok
>>    PPC_PTRACE_SETHWDEBUG, MODE_EXACT, RO, len: 1: Ok
>>    PPC_PTRACE_SETHWDEBUG, MODE_EXACT, RW, len: 1: Ok
>>    PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, WO, len: 6: Ok
>>    PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, RO, len: 6: Ok
>>    PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, RW, len: 6: Ok
>>    PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, WO, len: 6: Ok
>>    PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, RO, len: 6: Fail
>>    failure: ptrace-hwbreak
>> 
>> I haven't had time to work out why yet.
>> 
>
> A (big) part of commit c3f68b0478e7 ("powerpc/watchpoint: Fix ptrace 
> code that muck around with address/len") was lost during rebase.
>
> I'll send a fix, up to you to squash it in or commit it as is.

Thanks.

cheers

^ permalink raw reply

* Re: [PATCH v4 08/16] powerpc: Use an accessor for word instructions
From: Balamuruhan S @ 2020-03-24  6:22 UTC (permalink / raw)
  To: Jordan Niethe
  Cc: Alistair Popple, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <CACzsE9rXmCfPV0-0m99i7YgYzBABuVo=t2GExfZpu6-9dL0Ryw@mail.gmail.com>

On Tue, 2020-03-24 at 14:18 +1100, Jordan Niethe wrote:
> On Mon, Mar 23, 2020 at 10:13 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
> > On Fri, 2020-03-20 at 16:18 +1100, Jordan Niethe wrote:
> > > In preparation for prefixed instructions where all instructions are no
> > > longer words, use an accessor for getting a word instruction as a u32
> > > from the instruction data type.
> > > 
> > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > > ---
> > > v4: New to series
> > > ---
> > >  arch/powerpc/kernel/align.c          |   2 +-
> > >  arch/powerpc/kernel/kprobes.c        |   2 +-
> > >  arch/powerpc/kernel/trace/ftrace.c   |  16 +-
> > >  arch/powerpc/lib/code-patching.c     |   2 +-
> > >  arch/powerpc/lib/feature-fixups.c    |   4 +-
> > >  arch/powerpc/lib/sstep.c             | 270 ++++++++++++++-------------
> > >  arch/powerpc/lib/test_emulate_step.c |   4 +-
> > >  arch/powerpc/xmon/xmon.c             |   4 +-
> > >  8 files changed, 153 insertions(+), 151 deletions(-)
> > > 
> > > diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> > > index 77c49dfdc1b4..b246ca124931 100644
> > > --- a/arch/powerpc/kernel/align.c
> > > +++ b/arch/powerpc/kernel/align.c
> > > @@ -309,7 +309,7 @@ int fix_alignment(struct pt_regs *regs)
> > >               /* We don't handle PPC little-endian any more... */
> > >               if (cpu_has_feature(CPU_FTR_PPC_LE))
> > >                       return -EIO;
> > > -             instr = PPC_INST(swab32(instr));
> > > +             instr = PPC_INST(swab32(ppc_inst_word(instr)));
> > >       }
> > > 
> > >  #ifdef CONFIG_SPE
> > > diff --git a/arch/powerpc/kernel/kprobes.c
> > > b/arch/powerpc/kernel/kprobes.c
> > > index 4c2b656615a6..0c600b6e4ead 100644
> > > --- a/arch/powerpc/kernel/kprobes.c
> > > +++ b/arch/powerpc/kernel/kprobes.c
> > > @@ -242,7 +242,7 @@ static int try_to_emulate(struct kprobe *p, struct
> > > pt_regs *regs)
> > >                * So, we should never get here... but, its still
> > >                * good to catch them, just in case...
> > >                */
> > > -             printk("Can't step on instruction %x\n", insn);
> > > +             printk("Can't step on instruction %x\n",
> > > ppc_inst_word(insn));
> > >               BUG();
> > >       } else {
> > >               /*
> > > diff --git a/arch/powerpc/kernel/trace/ftrace.c
> > > b/arch/powerpc/kernel/trace/ftrace.c
> > > index b3645b664819..7614a9f537fd 100644
> > > --- a/arch/powerpc/kernel/trace/ftrace.c
> > > +++ b/arch/powerpc/kernel/trace/ftrace.c
> > > @@ -74,7 +74,7 @@ ftrace_modify_code(unsigned long ip, ppc_inst old,
> > > ppc_inst
> > > new)
> > >       /* Make sure it is what we expect it to be */
> > >       if (!ppc_inst_equal(replaced, old)) {
> > >               pr_err("%p: replaced (%#x) != old (%#x)",
> > > -             (void *)ip, replaced, old);
> > > +             (void *)ip, ppc_inst_word(replaced), ppc_inst_word(old));
> > >               return -EINVAL;
> > >       }
> > > 
> > > @@ -136,7 +136,7 @@ __ftrace_make_nop(struct module *mod,
> > > 
> > >       /* Make sure that that this is still a 24bit jump */
> > >       if (!is_bl_op(op)) {
> > > -             pr_err("Not expected bl: opcode is %x\n", op);
> > > +             pr_err("Not expected bl: opcode is %x\n",
> > > ppc_inst_word(op));
> > >               return -EINVAL;
> > >       }
> > > 
> > > @@ -171,7 +171,7 @@ __ftrace_make_nop(struct module *mod,
> > >       /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */
> > >       if (!ppc_inst_equal(op, PPC_INST(PPC_INST_MFLR)) &&
> > >           !ppc_inst_equal(op, PPC_INST(PPC_INST_STD_LR))) {
> > > -             pr_err("Unexpected instruction %08x around bl _mcount\n",
> > > op);
> > > +             pr_err("Unexpected instruction %08x around bl _mcount\n",
> > > ppc_inst_word(op));
> > >               return -EINVAL;
> > >       }
> > >  #else
> > > @@ -201,7 +201,7 @@ __ftrace_make_nop(struct module *mod,
> > >       }
> > > 
> > >       if (!ppc_inst_equal(op,  PPC_INST(PPC_INST_LD_TOC))) {
> > > -             pr_err("Expected %08x found %08x\n", PPC_INST_LD_TOC, op);
> > > +             pr_err("Expected %08x found %08x\n", PPC_INST_LD_TOC,
> > > ppc_inst_word(op));
> > >               return -EINVAL;
> > >       }
> > >  #endif /* CONFIG_MPROFILE_KERNEL */
> > > @@ -401,7 +401,7 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace
> > > *rec, unsigned long addr)
> > > 
> > >       /* Make sure that that this is still a 24bit jump */
> > >       if (!is_bl_op(op)) {
> > > -             pr_err("Not expected bl: opcode is %x\n", op);
> > > +             pr_err("Not expected bl: opcode is %x\n",
> > > ppc_inst_word(op));
> > >               return -EINVAL;
> > >       }
> > > 
> > > @@ -525,7 +525,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned
> > > long
> > > addr)
> > > 
> > >       if (!expected_nop_sequence(ip, op[0], op[1])) {
> > >               pr_err("Unexpected call sequence at %p: %x %x\n",
> > > -             ip, op[0], op[1]);
> > > +             ip, ppc_inst_word(op[0]), ppc_inst_word(op[1]));
> > >               return -EINVAL;
> > >       }
> > > 
> > > @@ -644,7 +644,7 @@ static int __ftrace_make_call_kernel(struct
> > > dyn_ftrace
> > > *rec, unsigned long addr)
> > >       }
> > > 
> > >       if (!ppc_inst_equal(op, PPC_INST(PPC_INST_NOP))) {
> > > -             pr_err("Unexpected call sequence at %p: %x\n", ip, op);
> > > +             pr_err("Unexpected call sequence at %p: %x\n", ip,
> > > ppc_inst_word(op));
> > >               return -EINVAL;
> > >       }
> > > 
> > > @@ -723,7 +723,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned
> > > long old_addr,
> > > 
> > >       /* Make sure that that this is still a 24bit jump */
> > >       if (!is_bl_op(op)) {
> > > -             pr_err("Not expected bl: opcode is %x\n", op);
> > > +             pr_err("Not expected bl: opcode is %x\n",
> > > ppc_inst_word(op));
> > >               return -EINVAL;
> > >       }
> > > 
> > > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> > > patching.c
> > > index ec3abe1a6927..849eee63df3d 100644
> > > --- a/arch/powerpc/lib/code-patching.c
> > > +++ b/arch/powerpc/lib/code-patching.c
> > > @@ -233,7 +233,7 @@ bool is_conditional_branch(ppc_inst instr)
> > >       if (opcode == 16)       /* bc, bca, bcl, bcla */
> > >               return true;
> > >       if (opcode == 19) {
> > > -             switch ((instr >> 1) & 0x3ff) {
> > > +             switch ((ppc_inst_word(instr) >> 1) & 0x3ff) {
> > >               case 16:        /* bclr, bclrl */
> > >               case 528:       /* bcctr, bcctrl */
> > >               case 560:       /* bctar, bctarl */
> > > diff --git a/arch/powerpc/lib/feature-fixups.c
> > > b/arch/powerpc/lib/feature-
> > > fixups.c
> > > index 552106d1f64a..fe8ec099aa96 100644
> > > --- a/arch/powerpc/lib/feature-fixups.c
> > > +++ b/arch/powerpc/lib/feature-fixups.c
> > > @@ -54,8 +54,8 @@ static int patch_alt_instruction(unsigned int *src,
> > > unsigned int *dest,
> > > 
> > >               /* Branch within the section doesn't need translating */
> > >               if (target < alt_start || target > alt_end) {
> > > -                     instr = translate_branch(dest, src);
> > > -                     if (ppc_inst_null(instr))
> > > +                     instr = ppc_inst_word(translate_branch((ppc_inst
> > > *)dest, (ppc_inst *)src));
> > > +                     if (!instr)
> > >                               return 1;
> > >               }
> > >       }
> > > diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> > > index 1d9c766a89fe..bae878a83fa5 100644
> > > --- a/arch/powerpc/lib/sstep.c
> > > +++ b/arch/powerpc/lib/sstep.c
> > > @@ -1169,26 +1169,28 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >       unsigned long int imm;
> > >       unsigned long int val, val2;
> > >       unsigned int mb, me, sh;
> > > +     unsigned int word;
> > >       long ival;
> > > 
> > > +     word = ppc_inst_word(instr);
> > >       op->type = COMPUTE;
> > > 
> > > -     opcode = instr >> 26;
> > > +     opcode = word >> 26;
> > >       switch (opcode) {
> > >       case 16:        /* bc */
> > >               op->type = BRANCH;
> > > -             imm = (signed short)(instr & 0xfffc);
> > > -             if ((instr & 2) == 0)
> > > +             imm = (signed short)(word & 0xfffc);
> > > +             if ((word & 2) == 0)
> > >                       imm += regs->nip;
> > >               op->val = truncate_if_32bit(regs->msr, imm);
> > > -             if (instr & 1)
> > > +             if (word & 1)
> > >                       op->type |= SETLK;
> > > -             if (branch_taken(instr, regs, op))
> > > +             if (branch_taken(word, regs, op))
> > >                       op->type |= BRTAKEN;
> > >               return 1;
> > >  #ifdef CONFIG_PPC64
> > >       case 17:        /* sc */
> > > -             if ((instr & 0xfe2) == 2)
> > > +             if ((word & 0xfe2) == 2)
> > >                       op->type = SYSCALL;
> > >               else
> > >                       op->type = UNKNOWN;
> > > @@ -1196,21 +1198,21 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >  #endif
> > >       case 18:        /* b */
> > >               op->type = BRANCH | BRTAKEN;
> > > -             imm = instr & 0x03fffffc;
> > > +             imm = word & 0x03fffffc;
> > >               if (imm & 0x02000000)
> > >                       imm -= 0x04000000;
> > > -             if ((instr & 2) == 0)
> > > +             if ((word & 2) == 0)
> > >                       imm += regs->nip;
> > >               op->val = truncate_if_32bit(regs->msr, imm);
> > > -             if (instr & 1)
> > > +             if (word & 1)
> > >                       op->type |= SETLK;
> > >               return 1;
> > >       case 19:
> > > -             switch ((instr >> 1) & 0x3ff) {
> > > +             switch ((word >> 1) & 0x3ff) {
> > >               case 0:         /* mcrf */
> > >                       op->type = COMPUTE + SETCC;
> > > -                     rd = 7 - ((instr >> 23) & 0x7);
> > > -                     ra = 7 - ((instr >> 18) & 0x7);
> > > +                     rd = 7 - ((word >> 23) & 0x7);
> > > +                     ra = 7 - ((word >> 18) & 0x7);
> > >                       rd *= 4;
> > >                       ra *= 4;
> > >                       val = (regs->ccr >> ra) & 0xf;
> > > @@ -1220,11 +1222,11 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >               case 16:        /* bclr */
> > >               case 528:       /* bcctr */
> > >                       op->type = BRANCH;
> > > -                     imm = (instr & 0x400)? regs->ctr: regs->link;
> > > +                     imm = (word & 0x400)? regs->ctr: regs->link;
> > >                       op->val = truncate_if_32bit(regs->msr, imm);
> > > -                     if (instr & 1)
> > > +                     if (word & 1)
> > >                               op->type |= SETLK;
> > > -                     if (branch_taken(instr, regs, op))
> > > +                     if (branch_taken(word, regs, op))
> > >                               op->type |= BRTAKEN;
> > >                       return 1;
> > > 
> > > @@ -1247,23 +1249,23 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >               case 417:       /* crorc */
> > >               case 449:       /* cror */
> > >                       op->type = COMPUTE + SETCC;
> > > -                     ra = (instr >> 16) & 0x1f;
> > > -                     rb = (instr >> 11) & 0x1f;
> > > -                     rd = (instr >> 21) & 0x1f;
> > > +                     ra = (word >> 16) & 0x1f;
> > > +                     rb = (word >> 11) & 0x1f;
> > > +                     rd = (word >> 21) & 0x1f;
> > 
> > can't we use your accessors for all these operations ?
> I felt that since we are doing so many bit operations here it was
> simpliest to just leave these as uints.

Okay :+1:

-- Bala
> > >                       ra = (regs->ccr >> (31 - ra)) & 1;
> > >                       rb = (regs->ccr >> (31 - rb)) & 1;
> > > -                     val = (instr >> (6 + ra * 2 + rb)) & 1;
> > > +                     val = (word >> (6 + ra * 2 + rb)) & 1;
> > >                       op->ccval = (regs->ccr & ~(1UL << (31 - rd))) |
> > >                               (val << (31 - rd));
> > >                       return 1;
> > >               }
> > >               break;
> > >       case 31:
> > > -             switch ((instr >> 1) & 0x3ff) {
> > > +             switch ((word >> 1) & 0x3ff) {
> > >               case 598:       /* sync */
> > >                       op->type = BARRIER + BARRIER_SYNC;
> > >  #ifdef __powerpc64__
> > > -                     switch ((instr >> 21) & 3) {
> > > +                     switch ((word >> 21) & 3) {
> > >                       case 1:         /* lwsync */
> > >                               op->type = BARRIER + BARRIER_LWSYNC;
> > >                               break;
> > > @@ -1285,20 +1287,20 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >       if (!FULL_REGS(regs))
> > >               return -1;
> > > 
> > > -     rd = (instr >> 21) & 0x1f;
> > > -     ra = (instr >> 16) & 0x1f;
> > > -     rb = (instr >> 11) & 0x1f;
> > > -     rc = (instr >> 6) & 0x1f;
> > > +     rd = (word >> 21) & 0x1f;
> > > +     ra = (word >> 16) & 0x1f;
> > > +     rb = (word >> 11) & 0x1f;
> > > +     rc = (word >> 6) & 0x1f;
> > 
> > same here and in similar such places.
> > 
> > -- Bala
> > >       switch (opcode) {
> > >  #ifdef __powerpc64__
> > >       case 2:         /* tdi */
> > > -             if (rd & trap_compare(regs->gpr[ra], (short) instr))
> > > +             if (rd & trap_compare(regs->gpr[ra], (short) word))
> > >                       goto trap;
> > >               return 1;
> > >  #endif
> > >       case 3:         /* twi */
> > > -             if (rd & trap_compare((int)regs->gpr[ra], (short) instr))
> > > +             if (rd & trap_compare((int)regs->gpr[ra], (short) word))
> > >                       goto trap;
> > >               return 1;
> > > 
> > > @@ -1307,7 +1309,7 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > >               if (!cpu_has_feature(CPU_FTR_ARCH_300))
> > >                       return -1;
> > > 
> > > -             switch (instr & 0x3f) {
> > > +             switch (word & 0x3f) {
> > >               case 48:        /* maddhd */
> > >                       asm volatile(PPC_MADDHD(%0, %1, %2, %3) :
> > >                                    "=r" (op->val) : "r" (regs->gpr[ra]),
> > > @@ -1335,16 +1337,16 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >  #endif
> > > 
> > >       case 7:         /* mulli */
> > > -             op->val = regs->gpr[ra] * (short) instr;
> > > +             op->val = regs->gpr[ra] * (short) word;
> > >               goto compute_done;
> > > 
> > >       case 8:         /* subfic */
> > > -             imm = (short) instr;
> > > +             imm = (short) word;
> > >               add_with_carry(regs, op, rd, ~regs->gpr[ra], imm, 1);
> > >               return 1;
> > > 
> > >       case 10:        /* cmpli */
> > > -             imm = (unsigned short) instr;
> > > +             imm = (unsigned short) word;
> > >               val = regs->gpr[ra];
> > >  #ifdef __powerpc64__
> > >               if ((rd & 1) == 0)
> > > @@ -1354,7 +1356,7 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > >               return 1;
> > > 
> > >       case 11:        /* cmpi */
> > > -             imm = (short) instr;
> > > +             imm = (short) word;
> > >               val = regs->gpr[ra];
> > >  #ifdef __powerpc64__
> > >               if ((rd & 1) == 0)
> > > @@ -1364,35 +1366,35 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >               return 1;
> > > 
> > >       case 12:        /* addic */
> > > -             imm = (short) instr;
> > > +             imm = (short) word;
> > >               add_with_carry(regs, op, rd, regs->gpr[ra], imm, 0);
> > >               return 1;
> > > 
> > >       case 13:        /* addic. */
> > > -             imm = (short) instr;
> > > +             imm = (short) word;
> > >               add_with_carry(regs, op, rd, regs->gpr[ra], imm, 0);
> > >               set_cr0(regs, op);
> > >               return 1;
> > > 
> > >       case 14:        /* addi */
> > > -             imm = (short) instr;
> > > +             imm = (short) word;
> > >               if (ra)
> > >                       imm += regs->gpr[ra];
> > >               op->val = imm;
> > >               goto compute_done;
> > > 
> > >       case 15:        /* addis */
> > > -             imm = ((short) instr) << 16;
> > > +             imm = ((short) word) << 16;
> > >               if (ra)
> > >                       imm += regs->gpr[ra];
> > >               op->val = imm;
> > >               goto compute_done;
> > > 
> > >       case 19:
> > > -             if (((instr >> 1) & 0x1f) == 2) {
> > > +             if (((word >> 1) & 0x1f) == 2) {
> > >                       /* addpcis */
> > > -                     imm = (short) (instr & 0xffc1); /* d0 + d2 fields
> > > */
> > > -                     imm |= (instr >> 15) & 0x3e;    /* d1 field */
> > > +                     imm = (short) (word & 0xffc1);  /* d0 + d2 fields
> > > */
> > > +                     imm |= (word >> 15) & 0x3e;     /* d1 field */
> > >                       op->val = regs->nip + (imm << 16) + 4;
> > >                       goto compute_done;
> > >               }
> > > @@ -1400,65 +1402,65 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >               return 0;
> > > 
> > >       case 20:        /* rlwimi */
> > > -             mb = (instr >> 6) & 0x1f;
> > > -             me = (instr >> 1) & 0x1f;
> > > +             mb = (word >> 6) & 0x1f;
> > > +             me = (word >> 1) & 0x1f;
> > >               val = DATA32(regs->gpr[rd]);
> > >               imm = MASK32(mb, me);
> > >               op->val = (regs->gpr[ra] & ~imm) | (ROTATE(val, rb) & imm);
> > >               goto logical_done;
> > > 
> > >       case 21:        /* rlwinm */
> > > -             mb = (instr >> 6) & 0x1f;
> > > -             me = (instr >> 1) & 0x1f;
> > > +             mb = (word >> 6) & 0x1f;
> > > +             me = (word >> 1) & 0x1f;
> > >               val = DATA32(regs->gpr[rd]);
> > >               op->val = ROTATE(val, rb) & MASK32(mb, me);
> > >               goto logical_done;
> > > 
> > >       case 23:        /* rlwnm */
> > > -             mb = (instr >> 6) & 0x1f;
> > > -             me = (instr >> 1) & 0x1f;
> > > +             mb = (word >> 6) & 0x1f;
> > > +             me = (word >> 1) & 0x1f;
> > >               rb = regs->gpr[rb] & 0x1f;
> > >               val = DATA32(regs->gpr[rd]);
> > >               op->val = ROTATE(val, rb) & MASK32(mb, me);
> > >               goto logical_done;
> > > 
> > >       case 24:        /* ori */
> > > -             op->val = regs->gpr[rd] | (unsigned short) instr;
> > > +             op->val = regs->gpr[rd] | (unsigned short) word;
> > >               goto logical_done_nocc;
> > > 
> > >       case 25:        /* oris */
> > > -             imm = (unsigned short) instr;
> > > +             imm = (unsigned short) word;
> > >               op->val = regs->gpr[rd] | (imm << 16);
> > >               goto logical_done_nocc;
> > > 
> > >       case 26:        /* xori */
> > > -             op->val = regs->gpr[rd] ^ (unsigned short) instr;
> > > +             op->val = regs->gpr[rd] ^ (unsigned short) word;
> > >               goto logical_done_nocc;
> > > 
> > >       case 27:        /* xoris */
> > > -             imm = (unsigned short) instr;
> > > +             imm = (unsigned short) word;
> > >               op->val = regs->gpr[rd] ^ (imm << 16);
> > >               goto logical_done_nocc;
> > > 
> > >       case 28:        /* andi. */
> > > -             op->val = regs->gpr[rd] & (unsigned short) instr;
> > > +             op->val = regs->gpr[rd] & (unsigned short) word;
> > >               set_cr0(regs, op);
> > >               goto logical_done_nocc;
> > > 
> > >       case 29:        /* andis. */
> > > -             imm = (unsigned short) instr;
> > > +             imm = (unsigned short) word;
> > >               op->val = regs->gpr[rd] & (imm << 16);
> > >               set_cr0(regs, op);
> > >               goto logical_done_nocc;
> > > 
> > >  #ifdef __powerpc64__
> > >       case 30:        /* rld* */
> > > -             mb = ((instr >> 6) & 0x1f) | (instr & 0x20);
> > > +             mb = ((word >> 6) & 0x1f) | (word & 0x20);
> > >               val = regs->gpr[rd];
> > > -             if ((instr & 0x10) == 0) {
> > > -                     sh = rb | ((instr & 2) << 4);
> > > +             if ((word & 0x10) == 0) {
> > > +                     sh = rb | ((word & 2) << 4);
> > >                       val = ROTATE(val, sh);
> > > -                     switch ((instr >> 2) & 3) {
> > > +                     switch ((word >> 2) & 3) {
> > >                       case 0:         /* rldicl */
> > >                               val &= MASK64_L(mb);
> > >                               break;
> > > @@ -1478,7 +1480,7 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > >               } else {
> > >                       sh = regs->gpr[rb] & 0x3f;
> > >                       val = ROTATE(val, sh);
> > > -                     switch ((instr >> 1) & 7) {
> > > +                     switch ((word >> 1) & 7) {
> > >                       case 0:         /* rldcl */
> > >                               op->val = val & MASK64_L(mb);
> > >                               goto logical_done;
> > > @@ -1493,8 +1495,8 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > > 
> > >       case 31:
> > >               /* isel occupies 32 minor opcodes */
> > > -             if (((instr >> 1) & 0x1f) == 15) {
> > > -                     mb = (instr >> 6) & 0x1f; /* bc field */
> > > +             if (((word >> 1) & 0x1f) == 15) {
> > > +                     mb = (word >> 6) & 0x1f; /* bc field */
> > >                       val = (regs->ccr >> (31 - mb)) & 1;
> > >                       val2 = (ra) ? regs->gpr[ra] : 0;
> > > 
> > > @@ -1502,7 +1504,7 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > >                       goto compute_done;
> > >               }
> > > 
> > > -             switch ((instr >> 1) & 0x3ff) {
> > > +             switch ((word >> 1) & 0x3ff) {
> > >               case 4:         /* tw */
> > >                       if (rd == 0x1f ||
> > >                           (rd & trap_compare((int)regs->gpr[ra],
> > > @@ -1536,17 +1538,17 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >                       op->reg = rd;
> > >                       /* only MSR_EE and MSR_RI get changed if bit 15 set
> > > */
> > >                       /* mtmsrd doesn't change MSR_HV, MSR_ME or MSR_LE
> > > */
> > > -                     imm = (instr & 0x10000)? 0x8002:
> > > 0xefffffffffffeffeUL;
> > > +                     imm = (word & 0x10000)? 0x8002:
> > > 0xefffffffffffeffeUL;
> > >                       op->val = imm;
> > >                       return 0;
> > >  #endif
> > > 
> > >               case 19:        /* mfcr */
> > >                       imm = 0xffffffffUL;
> > > -                     if ((instr >> 20) & 1) {
> > > +                     if ((word >> 20) & 1) {
> > >                               imm = 0xf0000000UL;
> > >                               for (sh = 0; sh < 8; ++sh) {
> > > -                                     if (instr & (0x80000 >> sh))
> > > +                                     if (word & (0x80000 >> sh))
> > >                                               break;
> > >                                       imm >>= 4;
> > >                               }
> > > @@ -1560,7 +1562,7 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > >                       val = regs->gpr[rd];
> > >                       op->ccval = regs->ccr;
> > >                       for (sh = 0; sh < 8; ++sh) {
> > > -                             if (instr & (0x80000 >> sh))
> > > +                             if (word & (0x80000 >> sh))
> > >                                       op->ccval = (op->ccval & ~imm) |
> > >                                               (val & imm);
> > >                               imm >>= 4;
> > > @@ -1568,7 +1570,7 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > >                       return 1;
> > > 
> > >               case 339:       /* mfspr */
> > > -                     spr = ((instr >> 16) & 0x1f) | ((instr >> 6) &
> > > 0x3e0);
> > > +                     spr = ((word >> 16) & 0x1f) | ((word >> 6) &
> > > 0x3e0);
> > >                       op->type = MFSPR;
> > >                       op->reg = rd;
> > >                       op->spr = spr;
> > > @@ -1578,7 +1580,7 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > >                       return 0;
> > > 
> > >               case 467:       /* mtspr */
> > > -                     spr = ((instr >> 16) & 0x1f) | ((instr >> 6) &
> > > 0x3e0);
> > > +                     spr = ((word >> 16) & 0x1f) | ((word >> 6) &
> > > 0x3e0);
> > >                       op->type = MTSPR;
> > >                       op->val = regs->gpr[rd];
> > >                       op->spr = spr;
> > > @@ -1948,7 +1950,7 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > >               case 826:       /* sradi with sh_5 = 0 */
> > >               case 827:       /* sradi with sh_5 = 1 */
> > >                       op->type = COMPUTE + SETREG + SETXER;
> > > -                     sh = rb | ((instr & 2) << 4);
> > > +                     sh = rb | ((word & 2) << 4);
> > >                       ival = (signed long int) regs->gpr[rd];
> > >                       op->val = ival >> sh;
> > >                       op->xerval = regs->xer;
> > > @@ -1964,7 +1966,7 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > >                       if (!cpu_has_feature(CPU_FTR_ARCH_300))
> > >                               return -1;
> > >                       op->type = COMPUTE + SETREG;
> > > -                     sh = rb | ((instr & 2) << 4);
> > > +                     sh = rb | ((word & 2) << 4);
> > >                       val = (signed int) regs->gpr[rd];
> > >                       if (sh)
> > >                               op->val = ROTATE(val, sh) & MASK64(0, 63 -
> > > sh);
> > > @@ -1979,34 +1981,34 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >   */
> > >               case 54:        /* dcbst */
> > >                       op->type = MKOP(CACHEOP, DCBST, 0);
> > > -                     op->ea = xform_ea(instr, regs);
> > > +                     op->ea = xform_ea(word, regs);
> > >                       return 0;
> > > 
> > >               case 86:        /* dcbf */
> > >                       op->type = MKOP(CACHEOP, DCBF, 0);
> > > -                     op->ea = xform_ea(instr, regs);
> > > +                     op->ea = xform_ea(word, regs);
> > >                       return 0;
> > > 
> > >               case 246:       /* dcbtst */
> > >                       op->type = MKOP(CACHEOP, DCBTST, 0);
> > > -                     op->ea = xform_ea(instr, regs);
> > > +                     op->ea = xform_ea(word, regs);
> > >                       op->reg = rd;
> > >                       return 0;
> > > 
> > >               case 278:       /* dcbt */
> > >                       op->type = MKOP(CACHEOP, DCBTST, 0);
> > > -                     op->ea = xform_ea(instr, regs);
> > > +                     op->ea = xform_ea(word, regs);
> > >                       op->reg = rd;
> > >                       return 0;
> > > 
> > >               case 982:       /* icbi */
> > >                       op->type = MKOP(CACHEOP, ICBI, 0);
> > > -                     op->ea = xform_ea(instr, regs);
> > > +                     op->ea = xform_ea(word, regs);
> > >                       return 0;
> > > 
> > >               case 1014:      /* dcbz */
> > >                       op->type = MKOP(CACHEOP, DCBZ, 0);
> > > -                     op->ea = xform_ea(instr, regs);
> > > +                     op->ea = xform_ea(word, regs);
> > >                       return 0;
> > >               }
> > >               break;
> > > @@ -2019,14 +2021,14 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >       op->update_reg = ra;
> > >       op->reg = rd;
> > >       op->val = regs->gpr[rd];
> > > -     u = (instr >> 20) & UPDATE;
> > > +     u = (word >> 20) & UPDATE;
> > >       op->vsx_flags = 0;
> > > 
> > >       switch (opcode) {
> > >       case 31:
> > > -             u = instr & UPDATE;
> > > -             op->ea = xform_ea(instr, regs);
> > > -             switch ((instr >> 1) & 0x3ff) {
> > > +             u = word & UPDATE;
> > > +             op->ea = xform_ea(word, regs);
> > > +             switch ((word >> 1) & 0x3ff) {
> > >               case 20:        /* lwarx */
> > >                       op->type = MKOP(LARX, 0, 4);
> > >                       break;
> > > @@ -2271,25 +2273,25 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > > 
> > >  #ifdef CONFIG_VSX
> > >               case 12:        /* lxsiwzx */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(LOAD_VSX, 0, 4);
> > >                       op->element_size = 8;
> > >                       break;
> > > 
> > >               case 76:        /* lxsiwax */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(LOAD_VSX, SIGNEXT, 4);
> > >                       op->element_size = 8;
> > >                       break;
> > > 
> > >               case 140:       /* stxsiwx */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(STORE_VSX, 0, 4);
> > >                       op->element_size = 8;
> > >                       break;
> > > 
> > >               case 268:       /* lxvx */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(LOAD_VSX, 0, 16);
> > >                       op->element_size = 16;
> > >                       op->vsx_flags = VSX_CHECK_VEC;
> > > @@ -2298,33 +2300,33 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >               case 269:       /* lxvl */
> > >               case 301: {     /* lxvll */
> > >                       int nb;
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->ea = ra ? regs->gpr[ra] : 0;
> > >                       nb = regs->gpr[rb] & 0xff;
> > >                       if (nb > 16)
> > >                               nb = 16;
> > >                       op->type = MKOP(LOAD_VSX, 0, nb);
> > >                       op->element_size = 16;
> > > -                     op->vsx_flags = ((instr & 0x20) ? VSX_LDLEFT : 0) |
> > > +                     op->vsx_flags = ((word & 0x20) ? VSX_LDLEFT : 0) |
> > >                               VSX_CHECK_VEC;
> > >                       break;
> > >               }
> > >               case 332:       /* lxvdsx */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(LOAD_VSX, 0, 8);
> > >                       op->element_size = 8;
> > >                       op->vsx_flags = VSX_SPLAT;
> > >                       break;
> > > 
> > >               case 364:       /* lxvwsx */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(LOAD_VSX, 0, 4);
> > >                       op->element_size = 4;
> > >                       op->vsx_flags = VSX_SPLAT | VSX_CHECK_VEC;
> > >                       break;
> > > 
> > >               case 396:       /* stxvx */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(STORE_VSX, 0, 16);
> > >                       op->element_size = 16;
> > >                       op->vsx_flags = VSX_CHECK_VEC;
> > > @@ -2333,118 +2335,118 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >               case 397:       /* stxvl */
> > >               case 429: {     /* stxvll */
> > >                       int nb;
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->ea = ra ? regs->gpr[ra] : 0;
> > >                       nb = regs->gpr[rb] & 0xff;
> > >                       if (nb > 16)
> > >                               nb = 16;
> > >                       op->type = MKOP(STORE_VSX, 0, nb);
> > >                       op->element_size = 16;
> > > -                     op->vsx_flags = ((instr & 0x20) ? VSX_LDLEFT : 0) |
> > > +                     op->vsx_flags = ((word & 0x20) ? VSX_LDLEFT : 0) |
> > >                               VSX_CHECK_VEC;
> > >                       break;
> > >               }
> > >               case 524:       /* lxsspx */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(LOAD_VSX, 0, 4);
> > >                       op->element_size = 8;
> > >                       op->vsx_flags = VSX_FPCONV;
> > >                       break;
> > > 
> > >               case 588:       /* lxsdx */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(LOAD_VSX, 0, 8);
> > >                       op->element_size = 8;
> > >                       break;
> > > 
> > >               case 652:       /* stxsspx */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(STORE_VSX, 0, 4);
> > >                       op->element_size = 8;
> > >                       op->vsx_flags = VSX_FPCONV;
> > >                       break;
> > > 
> > >               case 716:       /* stxsdx */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(STORE_VSX, 0, 8);
> > >                       op->element_size = 8;
> > >                       break;
> > > 
> > >               case 780:       /* lxvw4x */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(LOAD_VSX, 0, 16);
> > >                       op->element_size = 4;
> > >                       break;
> > > 
> > >               case 781:       /* lxsibzx */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(LOAD_VSX, 0, 1);
> > >                       op->element_size = 8;
> > >                       op->vsx_flags = VSX_CHECK_VEC;
> > >                       break;
> > > 
> > >               case 812:       /* lxvh8x */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(LOAD_VSX, 0, 16);
> > >                       op->element_size = 2;
> > >                       op->vsx_flags = VSX_CHECK_VEC;
> > >                       break;
> > > 
> > >               case 813:       /* lxsihzx */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(LOAD_VSX, 0, 2);
> > >                       op->element_size = 8;
> > >                       op->vsx_flags = VSX_CHECK_VEC;
> > >                       break;
> > > 
> > >               case 844:       /* lxvd2x */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(LOAD_VSX, 0, 16);
> > >                       op->element_size = 8;
> > >                       break;
> > > 
> > >               case 876:       /* lxvb16x */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(LOAD_VSX, 0, 16);
> > >                       op->element_size = 1;
> > >                       op->vsx_flags = VSX_CHECK_VEC;
> > >                       break;
> > > 
> > >               case 908:       /* stxvw4x */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(STORE_VSX, 0, 16);
> > >                       op->element_size = 4;
> > >                       break;
> > > 
> > >               case 909:       /* stxsibx */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(STORE_VSX, 0, 1);
> > >                       op->element_size = 8;
> > >                       op->vsx_flags = VSX_CHECK_VEC;
> > >                       break;
> > > 
> > >               case 940:       /* stxvh8x */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(STORE_VSX, 0, 16);
> > >                       op->element_size = 2;
> > >                       op->vsx_flags = VSX_CHECK_VEC;
> > >                       break;
> > > 
> > >               case 941:       /* stxsihx */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(STORE_VSX, 0, 2);
> > >                       op->element_size = 8;
> > >                       op->vsx_flags = VSX_CHECK_VEC;
> > >                       break;
> > > 
> > >               case 972:       /* stxvd2x */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(STORE_VSX, 0, 16);
> > >                       op->element_size = 8;
> > >                       break;
> > > 
> > >               case 1004:      /* stxvb16x */
> > > -                     op->reg = rd | ((instr & 1) << 5);
> > > +                     op->reg = rd | ((word & 1) << 5);
> > >                       op->type = MKOP(STORE_VSX, 0, 16);
> > >                       op->element_size = 1;
> > >                       op->vsx_flags = VSX_CHECK_VEC;
> > > @@ -2457,80 +2459,80 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >       case 32:        /* lwz */
> > >       case 33:        /* lwzu */
> > >               op->type = MKOP(LOAD, u, 4);
> > > -             op->ea = dform_ea(instr, regs);
> > > +             op->ea = dform_ea(word, regs);
> > >               break;
> > > 
> > >       case 34:        /* lbz */
> > >       case 35:        /* lbzu */
> > >               op->type = MKOP(LOAD, u, 1);
> > > -             op->ea = dform_ea(instr, regs);
> > > +             op->ea = dform_ea(word, regs);
> > >               break;
> > > 
> > >       case 36:        /* stw */
> > >       case 37:        /* stwu */
> > >               op->type = MKOP(STORE, u, 4);
> > > -             op->ea = dform_ea(instr, regs);
> > > +             op->ea = dform_ea(word, regs);
> > >               break;
> > > 
> > >       case 38:        /* stb */
> > >       case 39:        /* stbu */
> > >               op->type = MKOP(STORE, u, 1);
> > > -             op->ea = dform_ea(instr, regs);
> > > +             op->ea = dform_ea(word, regs);
> > >               break;
> > > 
> > >       case 40:        /* lhz */
> > >       case 41:        /* lhzu */
> > >               op->type = MKOP(LOAD, u, 2);
> > > -             op->ea = dform_ea(instr, regs);
> > > +             op->ea = dform_ea(word, regs);
> > >               break;
> > > 
> > >       case 42:        /* lha */
> > >       case 43:        /* lhau */
> > >               op->type = MKOP(LOAD, SIGNEXT | u, 2);
> > > -             op->ea = dform_ea(instr, regs);
> > > +             op->ea = dform_ea(word, regs);
> > >               break;
> > > 
> > >       case 44:        /* sth */
> > >       case 45:        /* sthu */
> > >               op->type = MKOP(STORE, u, 2);
> > > -             op->ea = dform_ea(instr, regs);
> > > +             op->ea = dform_ea(word, regs);
> > >               break;
> > > 
> > >       case 46:        /* lmw */
> > >               if (ra >= rd)
> > >                       break;          /* invalid form, ra in range to
> > > load
> > > */
> > >               op->type = MKOP(LOAD_MULTI, 0, 4 * (32 - rd));
> > > -             op->ea = dform_ea(instr, regs);
> > > +             op->ea = dform_ea(word, regs);
> > >               break;
> > > 
> > >       case 47:        /* stmw */
> > >               op->type = MKOP(STORE_MULTI, 0, 4 * (32 - rd));
> > > -             op->ea = dform_ea(instr, regs);
> > > +             op->ea = dform_ea(word, regs);
> > >               break;
> > > 
> > >  #ifdef CONFIG_PPC_FPU
> > >       case 48:        /* lfs */
> > >       case 49:        /* lfsu */
> > >               op->type = MKOP(LOAD_FP, u | FPCONV, 4);
> > > -             op->ea = dform_ea(instr, regs);
> > > +             op->ea = dform_ea(word, regs);
> > >               break;
> > > 
> > >       case 50:        /* lfd */
> > >       case 51:        /* lfdu */
> > >               op->type = MKOP(LOAD_FP, u, 8);
> > > -             op->ea = dform_ea(instr, regs);
> > > +             op->ea = dform_ea(word, regs);
> > >               break;
> > > 
> > >       case 52:        /* stfs */
> > >       case 53:        /* stfsu */
> > >               op->type = MKOP(STORE_FP, u | FPCONV, 4);
> > > -             op->ea = dform_ea(instr, regs);
> > > +             op->ea = dform_ea(word, regs);
> > >               break;
> > > 
> > >       case 54:        /* stfd */
> > >       case 55:        /* stfdu */
> > >               op->type = MKOP(STORE_FP, u, 8);
> > > -             op->ea = dform_ea(instr, regs);
> > > +             op->ea = dform_ea(word, regs);
> > >               break;
> > >  #endif
> > > 
> > > @@ -2538,14 +2540,14 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > >       case 56:        /* lq */
> > >               if (!((rd & 1) || (rd == ra)))
> > >                       op->type = MKOP(LOAD, 0, 16);
> > > -             op->ea = dqform_ea(instr, regs);
> > > +             op->ea = dqform_ea(word, regs);
> > >               break;
> > >  #endif
> > > 
> > >  #ifdef CONFIG_VSX
> > >       case 57:        /* lfdp, lxsd, lxssp */
> > > -             op->ea = dsform_ea(instr, regs);
> > > -             switch (instr & 3) {
> > > +             op->ea = dsform_ea(word, regs);
> > > +             switch (word & 3) {
> > >               case 0:         /* lfdp */
> > >                       if (rd & 1)
> > >                               break;          /* reg must be even */
> > > @@ -2569,8 +2571,8 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > > 
> > >  #ifdef __powerpc64__
> > >       case 58:        /* ld[u], lwa */
> > > -             op->ea = dsform_ea(instr, regs);
> > > -             switch (instr & 3) {
> > > +             op->ea = dsform_ea(word, regs);
> > > +             switch (word & 3) {
> > >               case 0:         /* ld */
> > >                       op->type = MKOP(LOAD, 0, 8);
> > >                       break;
> > > @@ -2586,16 +2588,16 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > > 
> > >  #ifdef CONFIG_VSX
> > >       case 61:        /* stfdp, lxv, stxsd, stxssp, stxv */
> > > -             switch (instr & 7) {
> > > +             switch (word & 7) {
> > >               case 0:         /* stfdp with LSB of DS field = 0 */
> > >               case 4:         /* stfdp with LSB of DS field = 1 */
> > > -                     op->ea = dsform_ea(instr, regs);
> > > +                     op->ea = dsform_ea(word, regs);
> > >                       op->type = MKOP(STORE_FP, 0, 16);
> > >                       break;
> > > 
> > >               case 1:         /* lxv */
> > > -                     op->ea = dqform_ea(instr, regs);
> > > -                     if (instr & 8)
> > > +                     op->ea = dqform_ea(word, regs);
> > > +                     if (word & 8)
> > >                               op->reg = rd + 32;
> > >                       op->type = MKOP(LOAD_VSX, 0, 16);
> > >                       op->element_size = 16;
> > > @@ -2604,7 +2606,7 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > > 
> > >               case 2:         /* stxsd with LSB of DS field = 0 */
> > >               case 6:         /* stxsd with LSB of DS field = 1 */
> > > -                     op->ea = dsform_ea(instr, regs);
> > > +                     op->ea = dsform_ea(word, regs);
> > >                       op->reg = rd + 32;
> > >                       op->type = MKOP(STORE_VSX, 0, 8);
> > >                       op->element_size = 8;
> > > @@ -2613,7 +2615,7 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > > 
> > >               case 3:         /* stxssp with LSB of DS field = 0 */
> > >               case 7:         /* stxssp with LSB of DS field = 1 */
> > > -                     op->ea = dsform_ea(instr, regs);
> > > +                     op->ea = dsform_ea(word, regs);
> > >                       op->reg = rd + 32;
> > >                       op->type = MKOP(STORE_VSX, 0, 4);
> > >                       op->element_size = 8;
> > > @@ -2621,8 +2623,8 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > >                       break;
> > > 
> > >               case 5:         /* stxv */
> > > -                     op->ea = dqform_ea(instr, regs);
> > > -                     if (instr & 8)
> > > +                     op->ea = dqform_ea(word, regs);
> > > +                     if (word & 8)
> > >                               op->reg = rd + 32;
> > >                       op->type = MKOP(STORE_VSX, 0, 16);
> > >                       op->element_size = 16;
> > > @@ -2634,8 +2636,8 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > > 
> > >  #ifdef __powerpc64__
> > >       case 62:        /* std[u] */
> > > -             op->ea = dsform_ea(instr, regs);
> > > -             switch (instr & 3) {
> > > +             op->ea = dsform_ea(word, regs);
> > > +             switch (word & 3) {
> > >               case 0:         /* std */
> > >                       op->type = MKOP(STORE, 0, 8);
> > >                       break;
> > > @@ -2663,7 +2665,7 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > >       return 0;
> > > 
> > >   logical_done:
> > > -     if (instr & 1)
> > > +     if (word & 1)
> > >               set_cr0(regs, op);
> > >   logical_done_nocc:
> > >       op->reg = ra;
> > > @@ -2671,7 +2673,7 @@ int analyse_instr(struct instruction_op *op, const
> > > struct pt_regs *regs,
> > >       return 1;
> > > 
> > >   arith_done:
> > > -     if (instr & 1)
> > > +     if (word & 1)
> > >               set_cr0(regs, op);
> > >   compute_done:
> > >       op->reg = rd;
> > > diff --git a/arch/powerpc/lib/test_emulate_step.c
> > > b/arch/powerpc/lib/test_emulate_step.c
> > > index 486e057e5be1..d6275a9b8ce6 100644
> > > --- a/arch/powerpc/lib/test_emulate_step.c
> > > +++ b/arch/powerpc/lib/test_emulate_step.c
> > > @@ -851,7 +851,7 @@ static int __init emulate_compute_instr(struct
> > > pt_regs
> > > *regs,
> > > 
> > >       if (analyse_instr(&op, regs, instr) != 1 ||
> > >           GETTYPE(op.type) != COMPUTE) {
> > > -             pr_info("emulation failed, instruction = 0x%08x\n", instr);
> > > +             pr_info("emulation failed, instruction = 0x%08x\n",
> > > ppc_inst_word(instr));
> > >               return -EFAULT;
> > >       }
> > > 
> > > @@ -871,7 +871,7 @@ static int __init execute_compute_instr(struct
> > > pt_regs
> > > *regs,
> > >       /* Patch the NOP with the actual instruction */
> > >       patch_instruction_site(&patch__exec_instr, instr);
> > >       if (exec_instr(regs)) {
> > > -             pr_info("execution failed, instruction = 0x%08x\n", instr);
> > > +             pr_info("execution failed, instruction = 0x%08x\n",
> > > ppc_inst_word(instr));
> > >               return -EFAULT;
> > >       }
> > > 
> > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > > index d045e583f1c9..dec522fa8201 100644
> > > --- a/arch/powerpc/xmon/xmon.c
> > > +++ b/arch/powerpc/xmon/xmon.c
> > > @@ -2871,9 +2871,9 @@ generic_inst_dump(unsigned long adr, long count,
> > > int
> > > praddr,
> > >               dotted = 0;
> > >               last_inst = inst;
> > >               if (praddr)
> > > -                     printf(REG"  %.8x", adr, inst);
> > > +                     printf(REG"  %.8x", adr, ppc_inst_word(inst));
> > >               printf("\t");
> > > -             dump_func(inst, adr);
> > > +             dump_func(ppc_inst_word(inst), adr);
> > >               printf("\n");
> > >       }
> > >       return adr - first_adr;


^ permalink raw reply

* Re: [PATCH] powerpc xmon: drop the option `i` in cacheflush
From: Balamuruhan S @ 2020-03-24  6:15 UTC (permalink / raw)
  To: Michael Ellerman, Naveen N. Rao, Segher Boessenkool
  Cc: jniethe5, linuxppc-dev, sandipan, paulus, ravi.bangoria
In-Reply-To: <87wo7axvd6.fsf@mpe.ellerman.id.au>

On Tue, 2020-03-24 at 14:52 +1100, Michael Ellerman wrote:
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> > Segher Boessenkool wrote:
> > > On Mon, Mar 23, 2020 at 04:55:48PM +0530, Balamuruhan S wrote:
> > > > Data Cache Block Invalidate (dcbi) instruction implemented in 32-bit
> > > > designs prior to PowerPC architecture version 2.01 and got obsolete
> > > > from version 2.01.
> 
> We still support 32-bit ...

Okay, got it.

> 
> > > It was added back in 2.03.  It also exists in 64-bit designs (using
> > > category embedded), in 2.07 still even.
> > 
> > Indeed, it has been part of Book3e.
> > 
> > It isn't clear if this is still useful in this context (xmon) though, 
> > since 'dcbf' seems to be equivalent in most respects. At the very least, 
> > we should restrict this to Book3e, if it is of value there.
> 
> Looking at the ISA it looks like dcbf is more or less equivalent and we
> could probably drop the explicit invalidate command.
> 
> But the simplest option is probably to just ifdef it out for
> PPC_BOOK3S_64.

Sure, I will make the changes as suggested.

Thank you!

-- Bala
> 
> cheers


^ permalink raw reply

* Re: [PATCH] powerpc xmon: drop the option `i` in cacheflush
From: Balamuruhan S @ 2020-03-24  6:08 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: ravi.bangoria, paulus, sandipan, jniethe5, naveen.n.rao,
	linuxppc-dev
In-Reply-To: <20200323124630.GP22482@gate.crashing.org>

On Mon, 2020-03-23 at 07:46 -0500, Segher Boessenkool wrote:
> On Mon, Mar 23, 2020 at 04:55:48PM +0530, Balamuruhan S wrote:
> > Data Cache Block Invalidate (dcbi) instruction implemented in 32-bit
> > designs prior to PowerPC architecture version 2.01 and got obsolete
> > from version 2.01.
> 
> It was added back in 2.03.  It also exists in 64-bit designs (using
> category embedded), in 2.07 still even.

I got to know about the version from,
https://wiki.alcf.anl.gov/images/f/fb/PowerPC_-_Assembly_-_IBM_Programming_Environment_2.3.pdf

please correct me if I was looking into wrong one.

-- Bala 
> 
> 
> Segher


^ permalink raw reply

* [5.6.0-rc7] Kernel crash while running ndctl tests
From: Sachin Sant @ 2020-03-24  5:55 UTC (permalink / raw)
  To: LKML, linuxppc-dev; +Cc: Baoquan He, linux-nvdimm

While running ndctl[1] tests against 5.6.0-rc7 following crash is encountered.

Bisect leads me to  commit d41e2f3bd546 
mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case

Reverting this commit helps and the tests complete without any crash.

pmem0: detected capacity change from 0 to 10720641024
BUG: Kernel NULL pointer dereference on read at 0x00000000
Faulting instruction address: 0xc000000000c3447c
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Dumping ftrace buffer:
   (ftrace buffer empty)
Modules linked in: dm_mod nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c ip6_tables nft_compat ip_set rfkill nf_tables nfnetlink sunrpc sg pseries_rng papr_scm uio_pdrv_genirq uio sch_fq_codel ip_tables sd_mod t10_pi ibmvscsi scsi_transport_srp ibmveth
CPU: 11 PID: 7519 Comm: lt-ndctl Not tainted 5.6.0-rc7-autotest #1
NIP:  c000000000c3447c LR: c000000000088354 CTR: c00000000018e990
REGS: c0000006223fb630 TRAP: 0300   Not tainted  (5.6.0-rc7-autotest)
MSR:  800000000280b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 24048888  XER: 00000000
CFAR: c00000000000dec4 DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0 
GPR00: c0000000003c5820 c0000006223fb8c0 c000000001684900 0000000004000000 
GPR04: c00c000101000000 0000000007ffffff c00000067ff20900 c00c000000000000 
GPR08: 0000000000000000 c00c000100000000 0000000000000000 c000000003f00000 
GPR12: 0000000000008000 c00000001ec70200 00007fffc102f9e8 000000001002e088 
GPR16: 0000000000000000 0000000010050d88 000000001002f778 000000001002f770 
GPR20: 0000000000000000 0000000000000100 0000000000000001 0000000000001000 
GPR24: 0000000000000008 0000000000000000 0000000004000000 c00c000100004000 
GPR28: c000000003101aa0 c00c000100000000 0000000001000000 0000000004000100 
NIP [c000000000c3447c] vmemmap_populated+0x98/0xc0
LR [c000000000088354] vmemmap_free+0x144/0x320
Call Trace:
[c0000006223fb8c0] [c0000006223fb960] 0xc0000006223fb960 (unreliable)
[c0000006223fb980] [c0000000003c5820] section_deactivate+0x220/0x240
[c0000006223fba30] [c0000000003dc1d8] __remove_pages+0x118/0x170
[c0000006223fba80] [c000000000086e5c] arch_remove_memory+0x3c/0x150
[c0000006223fbb00] [c00000000041a3bc] memunmap_pages+0x1cc/0x2f0
[c0000006223fbb80] [c0000000007d6d00] devm_action_release+0x30/0x50
[c0000006223fbba0] [c0000000007d7de8] release_nodes+0x2f8/0x3e0
[c0000006223fbc50] [c0000000007d0b38] device_release_driver_internal+0x168/0x270
[c0000006223fbc90] [c0000000007ccf50] unbind_store+0x130/0x170
[c0000006223fbcd0] [c0000000007cc0b4] drv_attr_store+0x44/0x60
[c0000006223fbcf0] [c00000000051fdb8] sysfs_kf_write+0x68/0x80
[c0000006223fbd10] [c00000000051f200] kernfs_fop_write+0x100/0x290
[c0000006223fbd60] [c00000000042037c] __vfs_write+0x3c/0x70
[c0000006223fbd80] [c00000000042404c] vfs_write+0xcc/0x240
[c0000006223fbdd0] [c00000000042442c] ksys_write+0x7c/0x140
[c0000006223fbe20] [c00000000000b278] system_call+0x5c/0x68
Instruction dump:
2ea80000 4196003c 794a2428 7d685215 41820030 7d48502a 71480002 41820024 
714a0008 4082002c e90b0008 786adf62 <e8680000> 7c635436 70630001 4c820020 
---[ end trace 579b48162da1b890 ]—

Thanks
-Sachin

[1] https://github.com/avocado-framework-tests/avocado-misc-tests/blob/master/memory/ndctl.py

^ permalink raw reply

* Re: [PATCH v4 00/16] Initial Prefixed Instruction support
From: Nicholas Piggin @ 2020-03-24  5:44 UTC (permalink / raw)
  To: Jordan Niethe; +Cc: Alistair Popple, Balamuruhan S, linuxppc-dev, Daniel Axtens
In-Reply-To: <CACzsE9qrNpfvoLKfdeXths4rKJ8jQcUic3=dFZ57ntogdeaMug@mail.gmail.com>

Jordan Niethe's on March 24, 2020 12:54 pm:
> On Mon, Mar 23, 2020 at 9:21 PM Nicholas Piggin <npiggin@gmail.com> wrote:
>>
>> Jordan Niethe's on March 23, 2020 7:25 pm:
>> > On Mon, Mar 23, 2020 at 5:22 PM Nicholas Piggin <npiggin@gmail.com> wrote:
>> >>
>> >> Jordan Niethe's on March 20, 2020 3:17 pm:
>> >> > A future revision of the ISA will introduce prefixed instructions. A
>> >> > prefixed instruction is composed of a 4-byte prefix followed by a
>> >> > 4-byte suffix.
>> >> >
>> >> > All prefixes have the major opcode 1. A prefix will never be a valid
>> >> > word instruction. A suffix may be an existing word instruction or a
>> >> > new instruction.
>> >> >
>> >> > This series enables prefixed instructions and extends the instruction
>> >> > emulation to support them. Then the places where prefixed instructions
>> >> > might need to be emulated are updated.
>> >> >
>> >> > The series is based on top of:
>> >> > https://patchwork.ozlabs.org/patch/1232619/ as this will effect
>> >> > kprobes.
>> >> >
>> >> > v4 is based on feedback from Nick Piggins, Christophe Leroy and Daniel Axtens.
>> >> > The major changes:
>> >> >     - Move xmon breakpoints from data section to text section
>> >> >     - Introduce a data type for instructions on powerpc
>> >>
>> >> Thanks for doing this, looks like a lot of work, I hope it works out :)
>> >>
>> > Yes it did end up touching a lot of places. I started thinking that
>> > that maybe it would be simpler to just use a u64 instead of the struct
>> > for  instructions.
>> > If we always keep the word instruction / prefix in the lower bytes,
>> > all of the current masking should still work and we can use operators
>> > again instead of ppc_inst_equal(), etc.
>>
>> Yeah.. I think now that you've done it, I prefer it this way.
> Sorry, just to be clear which way do you mean?

With the struct, not u64 scalar. mpe's preferred way is fine by me.

>> We'll want to adopt some convention for displaying prefixed
>> instruction bytes, but I don't know what what works best. I wonder
>> if binutils or any userspace tools have a convention.
> binutils-gdb upstream has supports disassembling prefixed instructions.
> Here is what objdump looks like:
>   44:    00 00 00 60     nop
>   48:    00 00 00 07     pnop
>   4c:    00 00 00 00
>   50:    01 00 20 39     li      r9,1
>   54:    00 00 00 06     paddi   r4,r9,3
>   58:    03 00 89 38
>   5c:    00 00 62 3c     addis   r3,r2,0
>>
>> Which reminds me, you might have missed show_instructions()?
>> Although maybe you don't need that until we start using them in
>> the kernel.
> You are right I missed that here.

So binutils doesn't do anything special, I guess you can make something
up.

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH v4 14/16] powerpc64: Add prefixed instructions to instruction data type
From: Nicholas Piggin @ 2020-03-24  5:40 UTC (permalink / raw)
  To: Jordan Niethe; +Cc: Alistair Popple, Balamuruhan S, linuxppc-dev, Daniel Axtens
In-Reply-To: <CACzsE9rbxV6HErxhwseMEJu7APezvRu4pKOx5YkepEnUWtpzqw@mail.gmail.com>

Jordan Niethe's on March 24, 2020 9:45 am:
> On Mon, Mar 23, 2020 at 6:37 PM Nicholas Piggin <npiggin@gmail.com> wrote:
>>
>> Jordan Niethe's on March 20, 2020 3:18 pm:
>> I'm a bit against using partially constructed opaque type for things
>> like this, even if it is in the code that knows about the type. We
>> could modify ppc_inst_prefixed() to assert that pad is equal to zero
>> (or some poisoned value) if it's not prefixed. Or do some validation
>> on the suffix if it is.
> Okay what about something like:
> +static inline ppc_inst ppc_inst_read(const void *ptr)
> +{
> +     u32 prefix, suffix;
> +     prefix = *(u32 *)ptr;
> +     if (prefix >> 26 == 1)
> +             suffix = *((u32 *)ptr + 1);
> +     else
> +             suffix = 0;
> +     return PPC_INST_PREFIX(prefix, suffix);
> +}

Sure, if that's the best way to test prefix.

>> Although there's proably no real performance or atomicity issues here,
>> I'd be pleased if we could do a case for prefixed and a case for non
>> prefixed, and store the non-prefixed with "std". Just for the principle
>> of not having half-written instructions in the image.
> Do you mean store the prefixed with std?

Oops, yes.

>> > @@ -881,7 +882,6 @@ static struct bpt *new_breakpoint(unsigned long a)
>> >               if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
>> >                       bp->address = a;
>> >                       bp->instr = bpt_table + ((bp - bpts) * BPT_WORDS);
>> > -                     patch_instruction(bp->instr + 1, PPC_INST(bpinstr));
>> >                       return bp;
>> >               }
>> >       }
>>
>> Why is this okay to remove?
> When we only had word instructions the bpt was just patched in here
> once and that was that.
> With prefixed instructions bp->instr + 1 might be the suffix. So I
> moved putting the breakpoint to insert_bpts():
> patch_instruction(bp->instr + ppc_inst_len(instr), PPC_INST(bpinstr));

Ah okay.

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH v3] powerpc/kprobes: Ignore traps that happened in real mode
From: Michael Ellerman @ 2020-03-24  5:28 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <f28a0219-abf1-07d4-b98b-b19db4af0f12@c-s.fr>

Christophe Leroy <christophe.leroy@c-s.fr> writes:
> ping
>
>
> Le 18/02/2020 à 20:38, Christophe Leroy a écrit :
>> When a program check exception happens while MMU translation is
>> disabled, following Oops happens in kprobe_handler() in the following
>> code:
>
> Michael, we have several traps in assembly while MMU is still disabled 
> (TRACE_IRQFLAGS, KUAP DEBUG, syscall from kernel, machine check in RTAS, 
> ...).
> Without this fix, all of them trigger an Oops when CONFIG_KPROBE is set.

Only on 32-bit.

But I guess this fix is good, if someone really wants to handle kprobes
in real mode they can tell us and do the work to make it solid.

cheers

>> 		} else if (*addr != BREAKPOINT_INSTRUCTION) {
>> 
>> [   33.098554] BUG: Unable to handle kernel data access on read at 0x0000e268
>> [   33.105091] Faulting instruction address: 0xc000ec34
>> [   33.110010] Oops: Kernel access of bad area, sig: 11 [#1]
>> [   33.115348] BE PAGE_SIZE=16K PREEMPT CMPC885
>> [   33.119540] Modules linked in:
>> [   33.122591] CPU: 0 PID: 429 Comm: cat Not tainted 5.6.0-rc1-s3k-dev-00824-g84195dc6c58a #3267
>> [   33.131005] NIP:  c000ec34 LR: c000ecd8 CTR: c019cab8
>> [   33.136002] REGS: ca4d3b58 TRAP: 0300   Not tainted  (5.6.0-rc1-s3k-dev-00824-g84195dc6c58a)
>> [   33.144324] MSR:  00001032 <ME,IR,DR,RI>  CR: 2a4d3c52  XER: 00000000
>> [   33.150699] DAR: 0000e268 DSISR: c0000000
>> [   33.150699] GPR00: c000b09c ca4d3c10 c66d0620 00000000 ca4d3c60 00000000 00009032 00000000
>> [   33.150699] GPR08: 00020000 00000000 c087de44 c000afe0 c66d0ad0 100d3dd6 fffffff3 00000000
>> [   33.150699] GPR16: 00000000 00000041 00000000 ca4d3d70 00000000 00000000 0000416d 00000000
>> [   33.150699] GPR24: 00000004 c53b6128 00000000 0000e268 00000000 c07c0000 c07bb6fc ca4d3c60
>> [   33.188015] NIP [c000ec34] kprobe_handler+0x128/0x290
>> [   33.192989] LR [c000ecd8] kprobe_handler+0x1cc/0x290
>> [   33.197854] Call Trace:
>> [   33.200340] [ca4d3c30] [c000b09c] program_check_exception+0xbc/0x6fc
>> [   33.206590] [ca4d3c50] [c000e43c] ret_from_except_full+0x0/0x4
>> [   33.212392] --- interrupt: 700 at 0xe268
>> [   33.270401] Instruction dump:
>> [   33.273335] 913e0008 81220000 38600001 3929ffff 91220000 80010024 bb410008 7c0803a6
>> [   33.280992] 38210020 4e800020 38600000 4e800020 <813b0000> 6d2a7fe0 2f8a0008 419e0154
>> [   33.288841] ---[ end trace 5b9152d4cdadd06d ]---
>> 
>> kprobe is not prepared to handle events in real mode and functions
>> running in real mode should have been blacklisted, so kprobe_handler()
>> can safely bail out telling 'this trap is not mine' for any trap that
>> happened while in real-mode.
>> 
>> If the trap happened with MSR_IR or MSR_DR cleared, return 0 immediately.
>> 
>> Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
>> Fixes: 6cc89bad60a6 ("powerpc/kprobes: Invoke handlers directly")
>> Cc: stable@vger.kernel.org
>> Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>> Cc: Masami Hiramatsu <mhiramat@kernel.org>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> 
>> ---
>> v3: Also bail out if MSR_DR is cleared.
>> 
>> Resending v2 with a more appropriate name
>> 
>> v2: bailing out instead of converting real-time address to virtual and continuing.
>> 
>> The bug might have existed even before that commit from Naveen.
>> 
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>>   arch/powerpc/kernel/kprobes.c | 3 +++
>>   1 file changed, 3 insertions(+)
>> 
>> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
>> index 2d27ec4feee4..9b340af02c38 100644
>> --- a/arch/powerpc/kernel/kprobes.c
>> +++ b/arch/powerpc/kernel/kprobes.c
>> @@ -264,6 +264,9 @@ int kprobe_handler(struct pt_regs *regs)
>>   	if (user_mode(regs))
>>   		return 0;
>>   
>> +	if (!(regs->msr & MSR_IR) || !(regs->msr & MSR_DR))
>> +		return 0;
>> +
>>   	/*
>>   	 * We don't want to be preempted for the entire
>>   	 * duration of kprobe processing
>> 

^ permalink raw reply

* Re: [PATCH v3] powerpc/pseries: Handle UE event for memcpy_mcsafe
From: Michael Ellerman @ 2020-03-24  5:27 UTC (permalink / raw)
  To: Ganesh Goudar, linuxppc-dev; +Cc: mahesh, santosh, Ganesh Goudar
In-Reply-To: <20200322160525.7624-1-ganeshgr@linux.ibm.com>

Ganesh Goudar <ganeshgr@linux.ibm.com> writes:
> If we hit UE at an instruction with a fixup entry, flag to
> ignore the event and set nip to continue execution at the
> fixup entry.

You don't explain why we would want to do that. Or what the consequences
are if we *don't* do it.

As such it's unclear if this is an important fix or just a nice-to-have.

> For powernv these changes are already made by
> commit 895e3dceeb97 ("powerpc/mce: Handle UE event for memcpy_mcsafe")

We have masses of code that supposedly abstracts the MCE logic. How did
we end up in the situation where we're having to write the same fix
twice for different platforms?

cheers

> Reviewed-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> Reviewed-by: Santosh S <santosh@fossix.org>
> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
> ---
> V2: Fixes a trivial checkpatch error in commit msg.
> V3: Use proper subject prefix.
> ---
>  arch/powerpc/platforms/pseries/ras.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
> index 43710b69e09e..58e2483fbb1a 100644
> --- a/arch/powerpc/platforms/pseries/ras.c
> +++ b/arch/powerpc/platforms/pseries/ras.c
> @@ -10,6 +10,7 @@
>  #include <linux/fs.h>
>  #include <linux/reboot.h>
>  #include <linux/irq_work.h>
> +#include <linux/extable.h>
>  
>  #include <asm/machdep.h>
>  #include <asm/rtas.h>
> @@ -505,6 +506,7 @@ static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
>  	int initiator = rtas_error_initiator(errp);
>  	int severity = rtas_error_severity(errp);
>  	u8 error_type, err_sub_type;
> +	const struct exception_table_entry *entry;
>  
>  	if (initiator == RTAS_INITIATOR_UNKNOWN)
>  		mce_err.initiator = MCE_INITIATOR_UNKNOWN;
> @@ -558,6 +560,12 @@ static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
>  	switch (mce_log->error_type) {
>  	case MC_ERROR_TYPE_UE:
>  		mce_err.error_type = MCE_ERROR_TYPE_UE;
> +		entry = search_kernel_exception_table(regs->nip);
> +		if (entry) {
> +			mce_err.ignore_event = true;
> +			regs->nip = extable_fixup(entry);
> +			disposition = RTAS_DISP_FULLY_RECOVERED;
> +		}
>  		switch (err_sub_type) {
>  		case MC_ERROR_UE_IFETCH:
>  			mce_err.u.ue_error_type = MCE_UE_ERROR_IFETCH;
> -- 
> 2.17.2

^ permalink raw reply

* [PATCH V2 2/3] mm/debug: Add tests validating arch advanced page table helpers
From: Anshuman Khandual @ 2020-03-24  5:22 UTC (permalink / raw)
  To: linux-mm
  Cc: Heiko Carstens, Paul Mackerras, H. Peter Anvin, linux-riscv,
	Will Deacon, linux-arch, linux-s390, x86, Mike Rapoport,
	Christian Borntraeger, Ingo Molnar, Catalin Marinas,
	linux-snps-arc, Vasily Gorbik, Anshuman Khandual, Borislav Petkov,
	Paul Walmsley, Kirill A . Shutemov, Thomas Gleixner,
	linux-arm-kernel, Vineet Gupta, linux-kernel, Palmer Dabbelt,
	Andrew Morton, linuxppc-dev
In-Reply-To: <1585027375-9997-1-git-send-email-anshuman.khandual@arm.com>

This adds new tests validating for these following arch advanced page table
helpers. These tests create and test specific mapping types at various page
table levels.

1. pxxp_set_wrprotect()
2. pxxp_get_and_clear()
3. pxxp_set_access_flags()
4. pxxp_get_and_clear_full()
5. pxxp_test_and_clear_young()
6. pxx_leaf()
7. pxx_set_huge()
8. pxx_(clear|mk)_savedwrite()
9. huge_pxxp_xxx()

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: linux-riscv@lists.infradead.org
Cc: x86@kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 mm/debug_vm_pgtable.c | 290 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 290 insertions(+)

diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 15055a8f6478..87b4b495333b 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -29,6 +29,7 @@
 #include <linux/sched/mm.h>
 #include <asm/pgalloc.h>
 #include <asm/pgtable.h>
+#include <asm/tlbflush.h>
 
 /*
  * Basic operations
@@ -68,6 +69,54 @@ static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot)
 	WARN_ON(pte_write(pte_wrprotect(pte_mkwrite(pte))));
 }
 
+static void __init pte_advanced_tests(struct mm_struct *mm,
+			struct vm_area_struct *vma, pte_t *ptep,
+			unsigned long pfn, unsigned long vaddr, pgprot_t prot)
+{
+	pte_t pte = pfn_pte(pfn, prot);
+
+	pte = pfn_pte(pfn, prot);
+	set_pte_at(mm, vaddr, ptep, pte);
+	ptep_set_wrprotect(mm, vaddr, ptep);
+	pte = READ_ONCE(*ptep);
+	WARN_ON(pte_write(pte));
+
+	pte = pfn_pte(pfn, prot);
+	set_pte_at(mm, vaddr, ptep, pte);
+	ptep_get_and_clear(mm, vaddr, ptep);
+	pte = READ_ONCE(*ptep);
+	WARN_ON(!pte_none(pte));
+
+	pte = pfn_pte(pfn, prot);
+	pte = pte_wrprotect(pte);
+	pte = pte_mkclean(pte);
+	set_pte_at(mm, vaddr, ptep, pte);
+	pte = pte_mkwrite(pte);
+	pte = pte_mkdirty(pte);
+	ptep_set_access_flags(vma, vaddr, ptep, pte, 1);
+	pte = READ_ONCE(*ptep);
+	WARN_ON(!(pte_write(pte) && pte_dirty(pte)));
+
+	pte = pfn_pte(pfn, prot);
+	set_pte_at(mm, vaddr, ptep, pte);
+	ptep_get_and_clear_full(mm, vaddr, ptep, 1);
+	pte = READ_ONCE(*ptep);
+	WARN_ON(!pte_none(pte));
+
+	pte = pte_mkyoung(pte);
+	set_pte_at(mm, vaddr, ptep, pte);
+	ptep_test_and_clear_young(vma, vaddr, ptep);
+	pte = READ_ONCE(*ptep);
+	WARN_ON(pte_young(pte));
+}
+
+static void __init pte_savedwrite_tests(unsigned long pfn, pgprot_t prot)
+{
+	pte_t pte = pfn_pte(pfn, prot);
+
+	WARN_ON(!pte_savedwrite(pte_mk_savedwrite(pte_clear_savedwrite(pte))));
+	WARN_ON(pte_savedwrite(pte_clear_savedwrite(pte_mk_savedwrite(pte))));
+}
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
 {
@@ -87,6 +136,83 @@ static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
 	WARN_ON(!pmd_bad(pmd_mkhuge(pmd)));
 }
 
+static void __init pmd_advanced_tests(struct mm_struct *mm,
+		struct vm_area_struct *vma, pmd_t *pmdp,
+		unsigned long pfn, unsigned long vaddr, pgprot_t prot)
+{
+	pmd_t pmd = pfn_pmd(pfn, prot);
+
+	/* Align the address wrt HPAGE_PMD_SIZE */
+	vaddr = (vaddr & HPAGE_PMD_MASK) + HPAGE_PMD_SIZE;
+
+	pmd = pfn_pmd(pfn, prot);
+	set_pmd_at(mm, vaddr, pmdp, pmd);
+	pmdp_set_wrprotect(mm, vaddr, pmdp);
+	pmd = READ_ONCE(*pmdp);
+	WARN_ON(pmd_write(pmd));
+
+	pmd = pfn_pmd(pfn, prot);
+	set_pmd_at(mm, vaddr, pmdp, pmd);
+	pmdp_huge_get_and_clear(mm, vaddr, pmdp);
+	pmd = READ_ONCE(*pmdp);
+	WARN_ON(!pmd_none(pmd));
+
+	pmd = pfn_pmd(pfn, prot);
+	pmd = pmd_wrprotect(pmd);
+	pmd = pmd_mkclean(pmd);
+	set_pmd_at(mm, vaddr, pmdp, pmd);
+	pmd = pmd_mkwrite(pmd);
+	pmd = pmd_mkdirty(pmd);
+	pmdp_set_access_flags(vma, vaddr, pmdp, pmd, 1);
+	pmd = READ_ONCE(*pmdp);
+	WARN_ON(!(pmd_write(pmd) && pmd_dirty(pmd)));
+
+	pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
+	set_pmd_at(mm, vaddr, pmdp, pmd);
+	pmdp_huge_get_and_clear_full(mm, vaddr, pmdp, 1);
+	pmd = READ_ONCE(*pmdp);
+	WARN_ON(!pmd_none(pmd));
+
+	pmd = pmd_mkyoung(pmd);
+	set_pmd_at(mm, vaddr, pmdp, pmd);
+	pmdp_test_and_clear_young(vma, vaddr, pmdp);
+	pmd = READ_ONCE(*pmdp);
+	WARN_ON(pmd_young(pmd));
+}
+
+static void __init pmd_leaf_tests(unsigned long pfn, pgprot_t prot)
+{
+	pmd_t pmd = pfn_pmd(pfn, prot);
+
+	/*
+	 * PMD based THP is a leaf entry.
+	 */
+	pmd = pmd_mkhuge(pmd);
+	WARN_ON(!pmd_leaf(pmd));
+}
+
+static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
+{
+	pmd_t pmd;
+
+	/*
+	 * X86 defined pmd_set_huge() verifies that the given
+	 * PMD is not a populated non-leaf entry.
+	 */
+	WRITE_ONCE(*pmdp, __pmd(0));
+	WARN_ON(!pmd_set_huge(pmdp, __pfn_to_phys(pfn), prot));
+	WARN_ON(!pmd_clear_huge(pmdp));
+	pmd = READ_ONCE(*pmdp);
+	WARN_ON(!pmd_none(pmd));
+}
+
+static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot)
+{
+	pmd_t pmd = pfn_pmd(pfn, prot);
+
+	WARN_ON(!pmd_savedwrite(pmd_mk_savedwrite(pmd_clear_savedwrite(pmd))));
+	WARN_ON(pmd_savedwrite(pmd_clear_savedwrite(pmd_mk_savedwrite(pmd))));
+}
 #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
 static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot)
 {
@@ -107,12 +233,110 @@ static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot)
 	 */
 	WARN_ON(!pud_bad(pud_mkhuge(pud)));
 }
+
+static void pud_advanced_tests(struct mm_struct *mm,
+		struct vm_area_struct *vma, pud_t *pudp,
+		unsigned long pfn, unsigned long vaddr, pgprot_t prot)
+{
+	pud_t pud = pfn_pud(pfn, prot);
+
+	/* Align the address wrt HPAGE_PUD_SIZE */
+	vaddr = (vaddr & HPAGE_PUD_MASK) + HPAGE_PUD_SIZE;
+
+	set_pud_at(mm, vaddr, pudp, pud);
+	pudp_set_wrprotect(mm, vaddr, pudp);
+	pud = READ_ONCE(*pudp);
+	WARN_ON(pud_write(pud));
+
+#ifndef __PAGETABLE_PMD_FOLDED
+	pud = pfn_pud(pfn, prot);
+	set_pud_at(mm, vaddr, pudp, pud);
+	pudp_huge_get_and_clear(mm, vaddr, pudp);
+	pud = READ_ONCE(*pudp);
+	WARN_ON(!pud_none(pud));
+
+	pud = pfn_pud(pfn, prot);
+	set_pud_at(mm, vaddr, pudp, pud);
+	pudp_huge_get_and_clear_full(mm, vaddr, pudp, 1);
+	pud = READ_ONCE(*pudp);
+	WARN_ON(!pud_none(pud));
+#endif
+	pud = pfn_pud(pfn, prot);
+	pud = pud_wrprotect(pud);
+	pud = pud_mkclean(pud);
+	set_pud_at(mm, vaddr, pudp, pud);
+	pud = pud_mkwrite(pud);
+	pud = pud_mkdirty(pud);
+	pudp_set_access_flags(vma, vaddr, pudp, pud, 1);
+	pud = READ_ONCE(*pudp);
+	WARN_ON(!(pud_write(pud) && pud_dirty(pud)));
+
+	pud = pud_mkyoung(pud);
+	set_pud_at(mm, vaddr, pudp, pud);
+	pudp_test_and_clear_young(vma, vaddr, pudp);
+	pud = READ_ONCE(*pudp);
+	WARN_ON(pud_young(pud));
+}
+
+static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot)
+{
+	pud_t pud = pfn_pud(pfn, prot);
+
+	/*
+	 * PUD based THP is a leaf entry.
+	 */
+	pud = pud_mkhuge(pud);
+	WARN_ON(!pud_leaf(pud));
+}
+
+static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
+{
+	pud_t pud;
+
+	/*
+	 * X86 defined pud_set_huge() verifies that the given
+	 * PUD is not a populated non-leaf entry.
+	 */
+	WRITE_ONCE(*pudp, __pud(0));
+	WARN_ON(!pud_set_huge(pudp, __pfn_to_phys(pfn), prot));
+	WARN_ON(!pud_clear_huge(pudp));
+	pud = READ_ONCE(*pudp);
+	WARN_ON(!pud_none(pud));
+}
 #else
 static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
+static void pud_advanced_tests(struct mm_struct *mm,
+		struct vm_area_struct *vma, pud_t *pudp,
+		unsigned long pfn, unsigned long vaddr, pgprot_t prot)
+{
+}
+static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot) { }
+static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
+{
+}
 #endif
 #else
 static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) { }
 static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
+static void __init pmd_advanced_tests(struct mm_struct *mm,
+		struct vm_area_struct *vma, pmd_t *pmdp,
+		unsigned long pfn, unsigned long vaddr, pgprot_t prot)
+{
+}
+static void __init pud_advanced_tests(struct mm_struct *mm,
+		struct vm_area_struct *vma, pud_t *pudp,
+		unsigned long pfn, unsigned long vaddr, pgprot_t prot)
+{
+}
+static void __init pmd_leaf_tests(unsigned long pfn, pgprot_t prot) { }
+static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot) { }
+static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
+{
+}
+static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
+{
+}
+static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot) { }
 #endif
 
 static void __init p4d_basic_tests(unsigned long pfn, pgprot_t prot)
@@ -500,8 +724,52 @@ static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot)
 	WARN_ON(!pte_huge(pte_mkhuge(pte)));
 #endif
 }
+
+static void __init hugetlb_advanced_tests(struct mm_struct *mm,
+					  struct vm_area_struct *vma,
+					  pte_t *ptep, unsigned long pfn,
+					  unsigned long vaddr, pgprot_t prot)
+{
+	struct page *page = pfn_to_page(pfn);
+	pte_t pte = READ_ONCE(*ptep);
+
+	pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
+	set_huge_pte_at(mm, vaddr, ptep, pte);
+	barrier();
+	WARN_ON(!pte_same(pte, huge_ptep_get(ptep)));
+	huge_pte_clear(mm, vaddr, ptep, PMD_SIZE);
+	pte = READ_ONCE(*ptep);
+	WARN_ON(!huge_pte_none(pte));
+
+	pte = mk_huge_pte(page, prot);
+	set_huge_pte_at(mm, vaddr, ptep, pte);
+	huge_ptep_set_wrprotect(mm, vaddr, ptep);
+	pte = READ_ONCE(*ptep);
+	WARN_ON(huge_pte_write(pte));
+
+	pte = mk_huge_pte(page, prot);
+	set_huge_pte_at(mm, vaddr, ptep, pte);
+	huge_ptep_get_and_clear(mm, vaddr, ptep);
+	pte = READ_ONCE(*ptep);
+	WARN_ON(!huge_pte_none(pte));
+
+	pte = mk_huge_pte(page, prot);
+	pte = huge_pte_wrprotect(pte);
+	set_huge_pte_at(mm, vaddr, ptep, pte);
+	pte = huge_pte_mkwrite(pte);
+	pte = huge_pte_mkdirty(pte);
+	huge_ptep_set_access_flags(vma, vaddr, ptep, pte, 1);
+	pte = READ_ONCE(*ptep);
+	WARN_ON(!(huge_pte_write(pte) && huge_pte_dirty(pte)));
+}
 #else
 static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot) { }
+static void __init hugetlb_advanced_tests(struct mm_struct *mm,
+					  struct vm_area_struct *vma,
+					  pte_t *ptep, unsigned long pfn,
+					  unsigned long vaddr, pgprot_t prot)
+{
+}
 #endif
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
@@ -564,6 +832,7 @@ static unsigned long __init get_random_vaddr(void)
 
 void __init debug_vm_pgtable(void)
 {
+	struct vm_area_struct *vma;
 	struct mm_struct *mm;
 	pgd_t *pgdp;
 	p4d_t *p4dp, *saved_p4dp;
@@ -592,6 +861,12 @@ void __init debug_vm_pgtable(void)
 	 */
 	protnone = __P000;
 
+	vma = vm_area_alloc(mm);
+	if (!vma) {
+		pr_err("vma allocation failed\n");
+		return;
+	}
+
 	/*
 	 * PFN for mapping at PTE level is determined from a standard kernel
 	 * text symbol. But pfns for higher page table levels are derived by
@@ -640,6 +915,20 @@ void __init debug_vm_pgtable(void)
 	p4d_clear_tests(mm, p4dp);
 	pgd_clear_tests(mm, pgdp);
 
+	pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
+	pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
+	pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
+	hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
+
+	pmd_leaf_tests(pmd_aligned, prot);
+	pud_leaf_tests(pud_aligned, prot);
+
+	pmd_huge_tests(pmdp, pmd_aligned, prot);
+	pud_huge_tests(pudp, pud_aligned, prot);
+
+	pte_savedwrite_tests(pte_aligned, prot);
+	pmd_savedwrite_tests(pmd_aligned, prot);
+
 	pte_unmap_unlock(ptep, ptl);
 
 	pmd_populate_tests(mm, pmdp, saved_ptep);
@@ -674,6 +963,7 @@ void __init debug_vm_pgtable(void)
 	pmd_free(mm, saved_pmdp);
 	pte_free(mm, saved_ptep);
 
+	vm_area_free(vma);
 	mm_dec_nr_puds(mm);
 	mm_dec_nr_pmds(mm);
 	mm_dec_nr_ptes(mm);
-- 
2.20.1


^ permalink raw reply related

* [PATCH V2 1/3] mm/debug: Add tests validating arch page table helpers for core features
From: Anshuman Khandual @ 2020-03-24  5:22 UTC (permalink / raw)
  To: linux-mm
  Cc: Heiko Carstens, Paul Mackerras, H. Peter Anvin, linux-riscv,
	Will Deacon, linux-arch, linux-s390, x86, Mike Rapoport,
	Christian Borntraeger, Ingo Molnar, Catalin Marinas,
	linux-snps-arc, Vasily Gorbik, Anshuman Khandual, Borislav Petkov,
	Paul Walmsley, Kirill A . Shutemov, Thomas Gleixner,
	linux-arm-kernel, Vineet Gupta, linux-kernel, Palmer Dabbelt,
	Andrew Morton, linuxppc-dev
In-Reply-To: <1585027375-9997-1-git-send-email-anshuman.khandual@arm.com>

This adds new tests validating arch page table helpers for these following
core memory features. These tests create and test specific mapping types at
various page table levels.

1. SPECIAL mapping
2. PROTNONE mapping
3. DEVMAP mapping
4. SOFTDIRTY mapping
5. SWAP mapping
6. MIGRATION mapping
7. HUGETLB mapping
8. THP mapping

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: linux-riscv@lists.infradead.org
Cc: x86@kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 mm/debug_vm_pgtable.c | 291 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 290 insertions(+), 1 deletion(-)

diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 98990a515268..15055a8f6478 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -289,6 +289,267 @@ static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp,
 	WARN_ON(pmd_bad(pmd));
 }
 
+static void __init pte_special_tests(unsigned long pfn, pgprot_t prot)
+{
+	pte_t pte = pfn_pte(pfn, prot);
+
+	if (!IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL))
+		return;
+
+	WARN_ON(!pte_special(pte_mkspecial(pte)));
+}
+
+static void __init pte_protnone_tests(unsigned long pfn, pgprot_t prot)
+{
+	pte_t pte = pfn_pte(pfn, prot);
+
+	if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
+		return;
+
+	WARN_ON(!pte_protnone(pte));
+	WARN_ON(!pte_present(pte));
+}
+
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+static void __init pmd_protnone_tests(unsigned long pfn, pgprot_t prot)
+{
+	pmd_t pmd = pfn_pmd(pfn, prot);
+
+	if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
+		return;
+
+	WARN_ON(!pmd_protnone(pmd));
+	WARN_ON(!pmd_present(pmd));
+}
+#else
+static void __init pmd_protnone_tests(unsigned long pfn, pgprot_t prot) { }
+#endif
+
+#ifdef CONFIG_ARCH_HAS_PTE_DEVMAP
+static void __init pte_devmap_tests(unsigned long pfn, pgprot_t prot)
+{
+	pte_t pte = pfn_pte(pfn, prot);
+
+	WARN_ON(!pte_devmap(pte_mkdevmap(pte)));
+}
+
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+static void __init pmd_devmap_tests(unsigned long pfn, pgprot_t prot)
+{
+	pmd_t pmd = pfn_pmd(pfn, prot);
+
+	WARN_ON(!pmd_devmap(pmd_mkdevmap(pmd)));
+}
+
+#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
+static void __init pud_devmap_tests(unsigned long pfn, pgprot_t prot)
+{
+	pud_t pud = pfn_pud(pfn, prot);
+
+	WARN_ON(!pud_devmap(pud_mkdevmap(pud)));
+}
+#else
+static void __init pud_devmap_tests(unsigned long pfn, pgprot_t prot) { }
+#endif
+#else
+static void __init pmd_devmap_tests(unsigned long pfn, pgprot_t prot) { }
+static void __init pud_devmap_tests(unsigned long pfn, pgprot_t prot) { }
+#endif
+#else
+static void __init pte_devmap_tests(unsigned long pfn, pgprot_t prot) { }
+static void __init pmd_devmap_tests(unsigned long pfn, pgprot_t prot) { }
+static void __init pud_devmap_tests(unsigned long pfn, pgprot_t prot) { }
+#endif
+
+static void __init pte_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
+{
+	pte_t pte = pfn_pte(pfn, prot);
+
+	if (!IS_ENABLED(CONFIG_HAVE_ARCH_SOFT_DIRTY))
+		return;
+
+	WARN_ON(!pte_soft_dirty(pte_mksoft_dirty(pte)));
+	WARN_ON(pte_soft_dirty(pte_clear_soft_dirty(pte)));
+}
+
+static void __init pte_swap_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
+{
+	pte_t pte = pfn_pte(pfn, prot);
+
+	if (!IS_ENABLED(CONFIG_HAVE_ARCH_SOFT_DIRTY))
+		return;
+
+	WARN_ON(!pte_swp_soft_dirty(pte_swp_mksoft_dirty(pte)));
+	WARN_ON(pte_swp_soft_dirty(pte_swp_clear_soft_dirty(pte)));
+}
+
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+static void __init pmd_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
+{
+	pmd_t pmd = pfn_pmd(pfn, prot);
+
+	if (!IS_ENABLED(CONFIG_HAVE_ARCH_SOFT_DIRTY))
+		return;
+
+	WARN_ON(!pmd_soft_dirty(pmd_mksoft_dirty(pmd)));
+	WARN_ON(pmd_soft_dirty(pmd_clear_soft_dirty(pmd)));
+}
+
+static void __init pmd_swap_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
+{
+	pmd_t pmd = pfn_pmd(pfn, prot);
+
+	if (!IS_ENABLED(CONFIG_HAVE_ARCH_SOFT_DIRTY) ||
+		!IS_ENABLED(CONFIG_ARCH_ENABLE_THP_MIGRATION))
+		return;
+
+	WARN_ON(!pmd_swp_soft_dirty(pmd_swp_mksoft_dirty(pmd)));
+	WARN_ON(pmd_swp_soft_dirty(pmd_swp_clear_soft_dirty(pmd)));
+}
+#else
+static void __init pmd_soft_dirty_tests(unsigned long pfn, pgprot_t prot) { }
+static void __init pmd_swap_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
+{
+}
+#endif
+
+static void __init pte_swap_tests(unsigned long pfn, pgprot_t prot)
+{
+	swp_entry_t swp;
+	pte_t pte;
+
+	pte = pfn_pte(pfn, prot);
+	swp = __pte_to_swp_entry(pte);
+	WARN_ON(!pte_same(pte, __swp_entry_to_pte(swp)));
+}
+
+#ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
+static void __init pmd_swap_tests(unsigned long pfn, pgprot_t prot)
+{
+	swp_entry_t swp;
+	pmd_t pmd;
+
+	pmd = pfn_pmd(pfn, prot);
+	swp = __pmd_to_swp_entry(pmd);
+	WARN_ON(!pmd_same(pmd, __swp_entry_to_pmd(swp)));
+}
+#else
+static void __init pmd_swap_tests(unsigned long pfn, pgprot_t prot) { }
+#endif
+
+static void __init swap_migration_tests(void)
+{
+	struct page *page;
+	swp_entry_t swp;
+
+	if (!IS_ENABLED(CONFIG_MIGRATION))
+		return;
+	/*
+	 * swap_migration_tests() requires a dedicated page as it needs to
+	 * be locked before creating a migration entry from it. Locking the
+	 * page that actually maps kernel text ('start_kernel') can be real
+	 * problematic. Lets allocate a dedicated page explicitly for this
+	 * purpose that will be freed subsequently.
+	 */
+	page = alloc_page(GFP_KERNEL);
+	if (!page) {
+		pr_err("page allocation failed\n");
+		return;
+	}
+
+	/*
+	 * make_migration_entry() expects given page to be
+	 * locked, otherwise it stumbles upon a BUG_ON().
+	 */
+	__SetPageLocked(page);
+	swp = make_migration_entry(page, 1);
+	WARN_ON(!is_migration_entry(swp));
+	WARN_ON(!is_write_migration_entry(swp));
+
+	make_migration_entry_read(&swp);
+	WARN_ON(!is_migration_entry(swp));
+	WARN_ON(is_write_migration_entry(swp));
+
+	swp = make_migration_entry(page, 0);
+	WARN_ON(!is_migration_entry(swp));
+	WARN_ON(is_write_migration_entry(swp));
+	__ClearPageLocked(page);
+	__free_page(page);
+}
+
+#ifdef CONFIG_HUGETLB_PAGE
+static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot)
+{
+	struct page *page;
+	pte_t pte;
+
+	/*
+	 * Accessing the page associated with the pfn is safe here,
+	 * as it was previously derived from a real kernel symbol.
+	 */
+	page = pfn_to_page(pfn);
+	pte = mk_huge_pte(page, prot);
+
+	WARN_ON(!huge_pte_dirty(huge_pte_mkdirty(pte)));
+	WARN_ON(!huge_pte_write(huge_pte_mkwrite(huge_pte_wrprotect(pte))));
+	WARN_ON(huge_pte_write(huge_pte_wrprotect(huge_pte_mkwrite(pte))));
+
+#ifdef CONFIG_ARCH_WANT_GENERAL_HUGETLB
+	pte = pfn_pte(pfn, prot);
+
+	WARN_ON(!pte_huge(pte_mkhuge(pte)));
+#endif
+}
+#else
+static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot) { }
+#endif
+
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+static void __init pmd_thp_tests(unsigned long pfn, pgprot_t prot)
+{
+	pmd_t pmd;
+
+	/*
+	 * pmd_trans_huge() and pmd_present() must return positive
+	 * after MMU invalidation with pmd_mknotpresent().
+	 */
+	pmd = pfn_pmd(pfn, prot);
+	WARN_ON(!pmd_trans_huge(pmd_mkhuge(pmd)));
+
+#ifndef __HAVE_ARCH_PMDP_INVALIDATE
+	WARN_ON(!pmd_trans_huge(pmd_mknotpresent(pmd_mkhuge(pmd))));
+	WARN_ON(!pmd_present(pmd_mknotpresent(pmd_mkhuge(pmd))));
+#endif
+}
+
+#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
+static void __init pud_thp_tests(unsigned long pfn, pgprot_t prot)
+{
+	pud_t pud;
+
+	/*
+	 * pud_trans_huge() and pud_present() must return positive
+	 * after MMU invalidation with pud_mknotpresent().
+	 */
+	pud = pfn_pud(pfn, prot);
+	WARN_ON(!pud_trans_huge(pud_mkhuge(pud)));
+
+	/*
+	 * pud_mknotpresent() has been dropped for now. Enable back
+	 * these tests when it comes back with a modified pud_present().
+	 *
+	 * WARN_ON(!pud_trans_huge(pud_mknotpresent(pud_mkhuge(pud))));
+	 * WARN_ON(!pud_present(pud_mknotpresent(pud_mkhuge(pud))));
+	 */
+}
+#else
+static void __init pud_thp_tests(unsigned long pfn, pgprot_t prot) { }
+#endif
+#else
+static void __init pmd_thp_tests(unsigned long pfn, pgprot_t prot) { }
+static void __init pud_thp_tests(unsigned long pfn, pgprot_t prot) { }
+#endif
+
 static unsigned long __init get_random_vaddr(void)
 {
 	unsigned long random_vaddr, random_pages, total_user_pages;
@@ -310,7 +571,7 @@ void __init debug_vm_pgtable(void)
 	pmd_t *pmdp, *saved_pmdp, pmd;
 	pte_t *ptep;
 	pgtable_t saved_ptep;
-	pgprot_t prot;
+	pgprot_t prot, protnone;
 	phys_addr_t paddr;
 	unsigned long vaddr, pte_aligned, pmd_aligned;
 	unsigned long pud_aligned, p4d_aligned, pgd_aligned;
@@ -325,6 +586,12 @@ void __init debug_vm_pgtable(void)
 		return;
 	}
 
+	/*
+	 * __P000 (or even __S000) will help create page table entries with
+	 * PROT_NONE permission as required for pxx_protnone_tests().
+	 */
+	protnone = __P000;
+
 	/*
 	 * PFN for mapping at PTE level is determined from a standard kernel
 	 * text symbol. But pfns for higher page table levels are derived by
@@ -380,6 +647,28 @@ void __init debug_vm_pgtable(void)
 	p4d_populate_tests(mm, p4dp, saved_pudp);
 	pgd_populate_tests(mm, pgdp, saved_p4dp);
 
+	pte_special_tests(pte_aligned, prot);
+	pte_protnone_tests(pte_aligned, protnone);
+	pmd_protnone_tests(pmd_aligned, protnone);
+
+	pte_devmap_tests(pte_aligned, prot);
+	pmd_devmap_tests(pmd_aligned, prot);
+	pud_devmap_tests(pud_aligned, prot);
+
+	pte_soft_dirty_tests(pte_aligned, prot);
+	pmd_soft_dirty_tests(pmd_aligned, prot);
+	pte_swap_soft_dirty_tests(pte_aligned, prot);
+	pmd_swap_soft_dirty_tests(pmd_aligned, prot);
+
+	pte_swap_tests(pte_aligned, prot);
+	pmd_swap_tests(pmd_aligned, prot);
+
+	swap_migration_tests();
+	hugetlb_basic_tests(pte_aligned, prot);
+
+	pmd_thp_tests(pmd_aligned, prot);
+	pud_thp_tests(pud_aligned, prot);
+
 	p4d_free(mm, saved_p4dp);
 	pud_free(mm, saved_pudp);
 	pmd_free(mm, saved_pmdp);
-- 
2.20.1


^ permalink raw reply related

* [PATCH V2 0/3] mm/debug: Add more arch page table helper tests
From: Anshuman Khandual @ 2020-03-24  5:22 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-doc, Heiko Carstens, Paul Mackerras, H. Peter Anvin,
	linux-riscv, Will Deacon, linux-arch, linux-s390, Jonathan Corbet,
	x86, Mike Rapoport, Christian Borntraeger, Ingo Molnar,
	Catalin Marinas, linux-snps-arc, Vasily Gorbik, Anshuman Khandual,
	Borislav Petkov, Paul Walmsley, Kirill A . Shutemov,
	Thomas Gleixner, linux-arm-kernel, Vineet Gupta, linux-kernel,
	Palmer Dabbelt, Andrew Morton, linuxppc-dev

This series adds more arch page table helper tests. The new tests here are
either related to core memory functions and advanced arch pgtable helpers.
This also creates a documentation file enlisting all expected semantics as
suggested by Mike Rapoport (https://lkml.org/lkml/2020/1/30/40).

This series has been tested on arm64 and x86 platforms. There is just one
expected failure on arm64 that will be fixed when we enable THP migration.

[   21.741634] WARNING: CPU: 0 PID: 1 at mm/debug_vm_pgtable.c:782

which corresponds to

WARN_ON(!pmd_present(pmd_mknotpresent(pmd_mkhuge(pmd))))

There are many TRANSPARENT_HUGEPAGE and ARCH_HAS_TRANSPARENT_HUGEPAGE_PUD
ifdefs scattered across the test. But consolidating all the fallback stubs
is not very straight forward because ARCH_HAS_TRANSPARENT_HUGEPAGE_PUD is
not explicitly dependent on ARCH_HAS_TRANSPARENT_HUGEPAGE.

This series has been build tested on many platforms including the ones that
subscribe the test through ARCH_HAS_DEBUG_VM_PGTABLE.

This series is based on v5.6-rc7 after applying these following patches.

1. https://patchwork.kernel.org/patch/11431277/
2. https://patchwork.kernel.org/patch/11452185/

Changes in V2:

- Dropped CONFIG_ARCH_HAS_PTE_SPECIAL per Christophe
- Dropped CONFIG_NUMA_BALANCING per Christophe
- Dropped CONFIG_HAVE_ARCH_SOFT_DIRTY per Christophe
- Dropped CONFIG_MIGRATION per Christophe
- Replaced CONFIG_S390 with __HAVE_ARCH_PMDP_INVALIDATE
- Moved page allocation & free inside swap_migration_tests() per Christophe
- Added CONFIG_TRANSPARENT_HUGEPAGE to protect pfn_pmd()
- Added CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD to protect pfn_pud()
- Added a patch for other arch advanced page table helper tests
- Added a patch creating a documentation for page table helper semantics

Changes in V1: (https://patchwork.kernel.org/patch/11408253/)

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: linux-riscv@lists.infradead.org
Cc: x86@kernel.org
Cc: linux-doc@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Anshuman Khandual (3):
  mm/debug: Add tests validating arch page table helpers for core features
  mm/debug: Add tests validating arch advanced page table helpers
  Documentation/mm: Add descriptions for arch page table helpers

 Documentation/vm/arch_pgtable_helpers.rst | 256 ++++++++++
 mm/debug_vm_pgtable.c                     | 594 +++++++++++++++++++++-
 2 files changed, 839 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/vm/arch_pgtable_helpers.rst

-- 
2.20.1


^ permalink raw reply

* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Alexey Kardashevskiy @ 2020-03-24  4:55 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Greg Kroah-Hartman, Joerg Roedel, linuxppc-dev, linux-kernel,
	iommu, Aneesh Kumar K.V, Robin Murphy, Lu Baolu
In-Reply-To: <d4bf6058-aa77-d0bc-8196-f4c27fb21b74@ozlabs.ru>



On 24/03/2020 14:37, Alexey Kardashevskiy wrote:
> 
> 
> On 24/03/2020 04:20, Christoph Hellwig wrote:
>> On Mon, Mar 23, 2020 at 07:58:01PM +1100, Alexey Kardashevskiy wrote:
>>>>> 0x100.0000.0000 .. 0x101.0000.0000
>>>>>
>>>>> 2x4G, each is 1TB aligned. And we can map directly only the first 4GB
>>>>> (because of the maximum IOMMU table size) but not the other. And 1:1 on
>>>>> that "pseries" is done with offset=0x0800.0000.0000.0000.
>>>>>
>>>>> So we want to check every bus address against dev->bus_dma_limit, not
>>>>> dev->coherent_dma_mask. In the example above I'd set bus_dma_limit to
>>>>> 0x0800.0001.0000.0000 and 1:1 mapping for the second 4GB would not be
>>>>> tried. Does this sound reasonable? Thanks,
>>>>
>>>> bus_dma_limit is just another limiting factor applied on top of
>>>> coherent_dma_mask or dma_mask respectively.
>>>
>>> This is not enough for the task: in my example, I'd set bus limit to
>>> 0x0800.0001.0000.0000 but this would disable bypass for all RAM
>>> addresses - the first and the second 4GB blocks.
>>
>> So what about something like the version here:
>>
>> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-bypass.3
> 
> 
> dma_alloc_direct() and dma_map_direct() do the same thing now which is
> good, did I miss anything else?
> 
> This lets us disable bypass automatically if this weird memory appears
> in the system but does not let us have 1:1 after that even for normal
> RAM. Thanks,

Ah no, does not help much, simple setting dma_ops_bypass will though.


But eventually, in this function:

static inline bool dma_map_direct(struct device *dev,
               const struct dma_map_ops *ops)
{
       if (likely(!ops))
               return true;
       if (!dev->dma_ops_bypass)
               return false;

       return min_not_zero(*dev->dma_mask, dev->bus_dma_limit) >=
                           dma_direct_get_required_mask(dev);
}


we rather want it to take a dma handle and a size, and add

if (dev->bus_dma_limit)
	return dev->bus_dma_limit > dma_handle + size;


where dma_handle=phys_to_dma(dev, phys) (I am not doing it here as unmap
needs the same test and it does not receive phys as a parameter).




-- 
Alexey

^ permalink raw reply

* Re: Argh, can't find dcache properties !
From: Michael Ellerman @ 2020-03-24  4:47 UTC (permalink / raw)
  To: Chris Packham, christophe.leroy@c-s.fr, paulus@samba.org,
	benh@kernel.crashing.org, tglx@linutronix.de, cai@lca.pw
  Cc: Hamish Martin, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <be8c123a90f6d1664a902b6ad6c754b9f3d9e567.camel@alliedtelesis.co.nz>

Chris Packham <Chris.Packham@alliedtelesis.co.nz> writes:
> Hi All,
>
> Just booting up v5.5.11 on a Freescale T2080RDB and I'm seeing the
> following mesage.
>
> kern.warning linuxbox kernel: Argh, can't find dcache properties !
> kern.warning linuxbox kernel: Argh, can't find icache properties !
>
> This was changed from DBG() to pr_warn() in commit 3b9176e9a874
> ("powerpc/setup_64: fix -Wempty-body warnings") but the message seems
> to be much older than that. So it's probably been an issue on the T2080
> (and other QorIQ SoCs) for a while.

That's an e6500 I think? So 64-bit Book3E.

You'll be getting the default values, which is 64 bytes so I guess that
works in practice.

> Looking at the code the t208x doesn't specifiy any of the d-cache-
> size/i-cache-size properties. Should I add them to silence the warning
> or switch it to pr_debug()/pr_info()?

Yeah ideally you'd add them to the device tree(s) for those boards.

cheers

^ permalink raw reply

* Re: [PATCH 4/4] hugetlbfs: clean up command line processing
From: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) @ 2020-03-24  3:47 UTC (permalink / raw)
  To: Mina Almasry, Mike Kravetz
  Cc: linux-doc, Catalin Marinas, Dave Hansen, Heiko Carstens, Linux-MM,
	Paul Mackerras, sparclinux, linux-riscv, Will Deacon, linux-s390,
	Jonathan Corbet, Christian Borntraeger, Ingo Molnar, Albert Ou,
	Vasily Gorbik, Paul Walmsley, Thomas Gleixner, linux-arm-kernel,
	open list, Palmer Dabbelt, Andrew Morton, linuxppc-dev,
	David S . Miller
In-Reply-To: <CAHS8izOhjvNVDXsx_SqP_oUQhCw-i_xcG9hxbvV86fFDeY_SAw@mail.gmail.com>



On 2020/3/24 8:43, Mina Almasry wrote:
> On Wed, Mar 18, 2020 at 3:07 PM Mike Kravetz <mike.kravetz@oracle.com> wrote:
>>
>> With all hugetlb page processing done in a single file clean up code.
> 
> Now that all hugepage page processing is done in a single file, clean
> up the code.
> 
>> - Make code match desired semantics
>>   - Update documentation with semantics
>> - Make all warnings and errors messages start with 'HugeTLB:'.
>> - Consistently name command line parsing routines.
>> - Add comments to code
>>   - Describe some of the subtle interactions
>>   - Describe semantics of command line arguments
>>
>> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
>> ---
>>  Documentation/admin-guide/mm/hugetlbpage.rst | 26 +++++++
>>  mm/hugetlb.c                                 | 78 +++++++++++++++-----
>>  2 files changed, 87 insertions(+), 17 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/mm/hugetlbpage.rst b/Documentation/admin-guide/mm/hugetlbpage.rst
>> index 1cc0bc78d10e..afc8888f33c3 100644
>> --- a/Documentation/admin-guide/mm/hugetlbpage.rst
>> +++ b/Documentation/admin-guide/mm/hugetlbpage.rst
>> @@ -100,6 +100,32 @@ with a huge page size selection parameter "hugepagesz=<size>".  <size> must
>>  be specified in bytes with optional scale suffix [kKmMgG].  The default huge
>>  page size may be selected with the "default_hugepagesz=<size>" boot parameter.
>>
>> +Hugetlb boot command line parameter semantics
>> +hugepagesz - Specify a huge page size.  Used in conjunction with hugepages
>> +       parameter to preallocate a number of huge pages of the specified
>> +       size.  Hence, hugepagesz and hugepages are typically specified in
>> +       pairs such as:
>> +               hugepagesz=2M hugepages=512
>> +       hugepagesz can only be specified once on the command line for a
>> +       specific huge page size.  Valid huge page sizes are architecture
>> +       dependent.
>> +hugepages - Specify the number of huge pages to preallocate.  This typically
>> +       follows a valid hugepagesz parameter.  However, if hugepages is the
>> +       first or only hugetlb command line parameter it specifies the number
>> +       of huge pages of default size to allocate.  The number of huge pages
>> +       of default size specified in this manner can be overwritten by a
>> +       hugepagesz,hugepages parameter pair for the default size.
>> +       For example, on an architecture with 2M default huge page size:
>> +               hugepages=256 hugepagesz=2M hugepages=512
>> +       will result in 512 2M huge pages being allocated.  If a hugepages
>> +       parameter is preceded by an invalid hugepagesz parameter, it will
>> +       be ignored.
>> +default_hugepagesz - Specify the default huge page size.  This parameter can
>> +       only be specified on the command line.  No other hugetlb command line
>> +       parameter is associated with default_hugepagesz.  Therefore, it can
>> +       appear anywhere on the command line.  Valid default huge page size is
>> +       architecture dependent.
> 
> Maybe specify what happens/should happen in a case like:
> 
> hugepages=100 default_hugepagesz=1G
> 
> Does that allocate 100 2MB pages or 100 1G pages? Assuming the default
> size is 2MB.
> 
> Also, regarding Randy's comment. It may be nice to keep these docs in
> one place only, so we don't have to maintain 2 docs in sync.
> 
> 
>> +
>>  When multiple huge page sizes are supported, ``/proc/sys/vm/nr_hugepages``
>>  indicates the current number of pre-allocated huge pages of the default size.
>>  Thus, one can use the following command to dynamically allocate/deallocate
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index cc85b4f156ca..2b9bf01db2b6 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -2954,7 +2954,7 @@ static void __init hugetlb_sysfs_init(void)
>>                 err = hugetlb_sysfs_add_hstate(h, hugepages_kobj,
>>                                          hstate_kobjs, &hstate_attr_group);
>>                 if (err)
>> -                       pr_err("Hugetlb: Unable to add hstate %s", h->name);
>> +                       pr_err("HugeTLB: Unable to add hstate %s", h->name);
>>         }
>>  }
>>
>> @@ -3058,7 +3058,7 @@ static void hugetlb_register_node(struct node *node)
>>                                                 nhs->hstate_kobjs,
>>                                                 &per_node_hstate_attr_group);
>>                 if (err) {
>> -                       pr_err("Hugetlb: Unable to add hstate %s for node %d\n",
>> +                       pr_err("HugeTLB: Unable to add hstate %s for node %d\n",
>>                                 h->name, node->dev.id);
>>                         hugetlb_unregister_node(node);
>>                         break;
>> @@ -3109,19 +3109,35 @@ static int __init hugetlb_init(void)
>>         if (!hugepages_supported())
>>                 return 0;
>>
>> -       if (!size_to_hstate(default_hstate_size)) {
>> -               if (default_hstate_size != 0) {
>> -                       pr_err("HugeTLB: unsupported default_hugepagesz %lu. Reverting to %lu\n",
>> -                              default_hstate_size, HPAGE_SIZE);
>> -               }
>> -
>> +       /*
>> +        * Make sure HPAGE_SIZE (HUGETLB_PAGE_ORDER) hstate exists.  Some
>> +        * architectures depend on setup being done here.
>> +        *
>> +        * If a valid default huge page size was specified on the command line,
>> +        * add associated hstate if necessary.  If not, set default_hstate_size
>> +        * to default size.  default_hstate_idx is used at runtime to identify
>> +        * the default huge page size/hstate.
>> +        */
>> +       hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
>> +       if (default_hstate_size)
>> +               hugetlb_add_hstate(ilog2(default_hstate_size) - PAGE_SHIFT);
>> +       else
>>                 default_hstate_size = HPAGE_SIZE;
>> -               hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
>> -       }
>>         default_hstate_idx = hstate_index(size_to_hstate(default_hstate_size));
>> +
>> +       /*
>> +        * default_hstate_max_huge_pages != 0 indicates a count (hugepages=)
>> +        * specified before a size (hugepagesz=).  Use this count for the
>> +        * default huge page size, unless a specific value was specified for
>> +        * this size in a hugepagesz/hugepages pair.
>> +        */
>>         if (default_hstate_max_huge_pages) {
>>                 if (!default_hstate.max_huge_pages)
>> -                       default_hstate.max_huge_pages = default_hstate_max_huge_pages;
>> +                       default_hstate.max_huge_pages =
>> +                               default_hstate_max_huge_pages;
>> +               else
>> +                       pr_warn("HugeTLB: First hugepages=%lu kB ignored\n",
>> +                               default_hstate_max_huge_pages);
>>         }
>>
>>         hugetlb_init_hstates();
>> @@ -3174,20 +3190,27 @@ void __init hugetlb_add_hstate(unsigned int order)
>>         parsed_hstate = h;
>>  }
>>
>> -static int __init hugetlb_nrpages_setup(char *s)
>> +/*
>> + * hugepages command line processing
>> + * hugepages must normally follows a valid hugepagsz specification.  If not,
> 
> 'hugepages must' or 'hugepages normally follows'
>> + * ignore the hugepages value.  hugepages can also be the first huge page
>> + * command line option in which case it specifies the number of huge pages
>> + * for the default size.
>> + */
>> +static int __init hugepages_setup(char *s)
>>  {
>>         unsigned long *mhp;
>>         static unsigned long *last_mhp;
>>
>>         if (!parsed_valid_hugepagesz) {
>> -               pr_warn("hugepages = %s preceded by "
>> +               pr_warn("HugeTLB: hugepages = %s preceded by "
>>                         "an unsupported hugepagesz, ignoring\n", s);
>>                 parsed_valid_hugepagesz = true;
>>                 return 1;
>>         }
>>         /*
>> -        * !hugetlb_max_hstate means we haven't parsed a hugepagesz= parameter yet,
>> -        * so this hugepages= parameter goes to the "default hstate".
>> +        * !hugetlb_max_hstate means we haven't parsed a hugepagesz= parameter
>> +        * yet, so this hugepages= parameter goes to the "default hstate".
>>          */
>>         else if (!hugetlb_max_hstate)
>>                 mhp = &default_hstate_max_huge_pages;
> 
> We don't set parsed_valid_hugepagesz to false at the end of this
> function, shouldn't we? Parsing a hugepages= value should 'consume' a
> previously defined hugepagesz= value, so that this is invalid IIUC:
> 
> hugepagesz=x hugepages=z hugepages=y
> 
In this case, we'll get:
"HugeTLB: hugepages= specified twice without interleaving hugepagesz=, ignoring
hugepages=y"

>> @@ -3195,7 +3218,8 @@ static int __init hugetlb_nrpages_setup(char *s)
>>                 mhp = &parsed_hstate->max_huge_pages;
>>
>>         if (mhp == last_mhp) {
>> -               pr_warn("hugepages= specified twice without interleaving hugepagesz=, ignoring\n");
>> +               pr_warn("HugeTLB: hugepages= specified twice without interleaving hugepagesz=, ignoring hugepages=%s\n",
>> +                       s);
>>                 return 1;
>>         }
>>
>> @@ -3214,8 +3238,15 @@ static int __init hugetlb_nrpages_setup(char *s)
>>
>>         return 1;
>>  }
>> -__setup("hugepages=", hugetlb_nrpages_setup);
>> +__setup("hugepages=", hugepages_setup);
>>
>> +/*
>> + * hugepagesz command line processing
>> + * A specific huge page size can only be specified once with hugepagesz.
>> + * hugepagesz is followed by hugepages on the commnad line.  The global
>> + * variable 'parsed_valid_hugepagesz' is used to determine if prior
>> + * hugepagesz argument was valid.
>> + */
>>  static int __init hugepagesz_setup(char *s)
>>  {
>>         unsigned long long size;
>> @@ -3230,16 +3261,23 @@ static int __init hugepagesz_setup(char *s)
>>         }
>>
>>         if (size_to_hstate(size)) {
>> +               parsed_valid_hugepagesz = false;
>>                 pr_warn("HugeTLB: hugepagesz %s specified twice, ignoring\n",
>>                         saved_s);
>>                 return 0;
>>         }
>>
>> +       parsed_valid_hugepagesz = true;
>>         hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
>>         return 1;
>>  }
>>  __setup("hugepagesz=", hugepagesz_setup);
>>
>> +/*
>> + * default_hugepagesz command line input
>> + * Only one instance of default_hugepagesz allowed on command line.  Do not
>> + * add hstate here as that will confuse hugepagesz/hugepages processing.
>> + */
>>  static int __init default_hugepagesz_setup(char *s)
>>  {
>>         unsigned long long size;
>> @@ -3252,6 +3290,12 @@ static int __init default_hugepagesz_setup(char *s)
>>                 return 0;
>>         }
>>
>> +       if (default_hstate_size) {
>> +               pr_err("HugeTLB: default_hugepagesz previously specified, ignoring %s\n",
>> +                       saved_s);
>> +               return 0;
>> +       }
>> +
>>         default_hstate_size = size;
>>         return 1;
>>  }
>> --
>> 2.24.1
>>
>>
> .
>
---
Regards,
Longpeng(Mike)

^ permalink raw reply

* Re: [PATCH] powerpc xmon: drop the option `i` in cacheflush
From: Michael Ellerman @ 2020-03-24  3:52 UTC (permalink / raw)
  To: Naveen N. Rao, Balamuruhan S, Segher Boessenkool
  Cc: jniethe5, linuxppc-dev, sandipan, paulus, ravi.bangoria
In-Reply-To: <1584968594.gguy53jubt.naveen@linux.ibm.com>

"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> Segher Boessenkool wrote:
>> On Mon, Mar 23, 2020 at 04:55:48PM +0530, Balamuruhan S wrote:
>>> Data Cache Block Invalidate (dcbi) instruction implemented in 32-bit
>>> designs prior to PowerPC architecture version 2.01 and got obsolete
>>> from version 2.01.

We still support 32-bit ...

>> It was added back in 2.03.  It also exists in 64-bit designs (using
>> category embedded), in 2.07 still even.
>
> Indeed, it has been part of Book3e.
>
> It isn't clear if this is still useful in this context (xmon) though, 
> since 'dcbf' seems to be equivalent in most respects. At the very least, 
> we should restrict this to Book3e, if it is of value there.

Looking at the ISA it looks like dcbf is more or less equivalent and we
could probably drop the explicit invalidate command.

But the simplest option is probably to just ifdef it out for
PPC_BOOK3S_64.

cheers

^ permalink raw reply

* Re: [PATCH v4 3/9] powerpc/vas: Add VAS user space API
From: Michael Ellerman @ 2020-03-24  3:41 UTC (permalink / raw)
  To: Daniel Axtens, Haren Myneni, herbert
  Cc: mikey, sukadev, linuxppc-dev, linux-crypto, npiggin
In-Reply-To: <875zevw61j.fsf@dja-thinkpad.axtens.net>

Daniel Axtens <dja@axtens.net> writes:
> Michael Ellerman <mpe@ellerman.id.au> writes:
>> Daniel Axtens <dja@axtens.net> writes:
>>> Haren Myneni <haren@linux.ibm.com> writes:
>>>> diff --git a/arch/powerpc/platforms/powernv/vas-api.c b/arch/powerpc/platforms/powernv/vas-api.c
>>>> new file mode 100644
>>>> index 0000000..7d049af
>>>> --- /dev/null
>>>> +++ b/arch/powerpc/platforms/powernv/vas-api.c
>>>> @@ -0,0 +1,257 @@
>> ...
>>>> +
>>>> +static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
>>>> +{
>>>> +	struct vas_window *txwin = fp->private_data;
>>>> +	unsigned long pfn;
>>>> +	u64 paste_addr;
>>>> +	pgprot_t prot;
>>>> +	int rc;
>>>> +
>>>> +	if ((vma->vm_end - vma->vm_start) > PAGE_SIZE) {
>>>
>>> I think you said this should be 4096 rather than 64k, regardless of what
>>> PAGE_SIZE you are compiled with?
>>
>> You can't mmap less than a page, a page is PAGE_SIZE bytes.
>>
>> So if that checked for 4K explicitly it would prevent mmap on 64K
>> kernels always, which seems like not what you want?
>
> Ah. My bad. Carry on then :)

Well you were just quoting something from Haren, so I think it's over to
him.

cheers

^ permalink raw reply

* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Alexey Kardashevskiy @ 2020-03-24  3:37 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Greg Kroah-Hartman, Joerg Roedel, linuxppc-dev, linux-kernel,
	iommu, Aneesh Kumar K.V, Robin Murphy, Lu Baolu
In-Reply-To: <20200323172014.GA31269@lst.de>



On 24/03/2020 04:20, Christoph Hellwig wrote:
> On Mon, Mar 23, 2020 at 07:58:01PM +1100, Alexey Kardashevskiy wrote:
>>>> 0x100.0000.0000 .. 0x101.0000.0000
>>>>
>>>> 2x4G, each is 1TB aligned. And we can map directly only the first 4GB
>>>> (because of the maximum IOMMU table size) but not the other. And 1:1 on
>>>> that "pseries" is done with offset=0x0800.0000.0000.0000.
>>>>
>>>> So we want to check every bus address against dev->bus_dma_limit, not
>>>> dev->coherent_dma_mask. In the example above I'd set bus_dma_limit to
>>>> 0x0800.0001.0000.0000 and 1:1 mapping for the second 4GB would not be
>>>> tried. Does this sound reasonable? Thanks,
>>>
>>> bus_dma_limit is just another limiting factor applied on top of
>>> coherent_dma_mask or dma_mask respectively.
>>
>> This is not enough for the task: in my example, I'd set bus limit to
>> 0x0800.0001.0000.0000 but this would disable bypass for all RAM
>> addresses - the first and the second 4GB blocks.
> 
> So what about something like the version here:
> 
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-bypass.3


dma_alloc_direct() and dma_map_direct() do the same thing now which is
good, did I miss anything else?

This lets us disable bypass automatically if this weird memory appears
in the system but does not let us have 1:1 after that even for normal
RAM. Thanks,


-- 
Alexey

^ permalink raw reply

* Re: [PATCH v4 03/16] powerpc: Use a datatype for instructions
From: Jordan Niethe @ 2020-03-24  3:21 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Alistair Popple, Nicholas Piggin, Balamuruhan S, linuxppc-dev,
	Daniel Axtens
In-Reply-To: <87369yzces.fsf@mpe.ellerman.id.au>

On Tue, Mar 24, 2020 at 1:58 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Nicholas Piggin <npiggin@gmail.com> writes:
> > Jordan Niethe's on March 23, 2020 7:28 pm:
> >> On Mon, Mar 23, 2020 at 5:27 PM Nicholas Piggin <npiggin@gmail.com> wrote:
> >>> Jordan Niethe's on March 20, 2020 3:17 pm:
> >>> > Currently unsigned ints are used to represent instructions on powerpc.
> >>> > This has worked well as instructions have always been 4 byte words.
> >>> > However, a future ISA version will introduce some changes to
> >>> > instructions that mean this scheme will no longer work as well. This
> >>> > change is Prefixed Instructions. A prefixed instruction is made up of a
> >>> > word prefix followed by a word suffix to make an 8 byte double word
> >>> > instruction. No matter the endianess of the system the prefix always
> >>> > comes first. Prefixed instructions are only planned for powerpc64.
> >>> >
> >>> > Introduce a ppc_inst type to represent both prefixed and word
> >>> > instructions on powerpc64 while keeping it possible to exclusively have
> >>> > word instructions on powerpc32, A latter patch will expand the type to
> >>> > include prefixed instructions but for now just typedef it to a u32.
> >>> >
> >>> > Later patches will introduce helper functions and macros for
> >>> > manipulating the instructions so that powerpc64 and powerpc32 might
> >>> > maintain separate type definitions.
> >>>
> >>> ppc_inst_t I would slightly prefer for a typedef like this.
> >> Are _t types meant to be reserved?
> >
> > No, just convention that structs are not normally typedefed unless
> > they are a pervasive interface that gets passed around a lot but
> > does not get accessed without accessor functions much. When you do
> > typedef them, add a _t (or less frequently _s/_u/etc). pte_t,
> > cpumask_t, atomic_t.
>
> Ideally we wouldn't use a typedef, we'd just have:
>
> struct ppc_inst {
>         u32 val;
> #ifdef CONFIG_PPC64
>         u32 suffix;
> #endif
> };
>
> That may make the conversion harder though, because you more or less
> have to update all usages at once.
Okay I will give that a try.
>
> cheers

^ permalink raw reply

* Re: [PATCH v4 08/16] powerpc: Use an accessor for word instructions
From: Jordan Niethe @ 2020-03-24  3:18 UTC (permalink / raw)
  To: Balamuruhan S
  Cc: Alistair Popple, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <ffad7904022c281493da7e56036689e70d312c0b.camel@linux.ibm.com>

On Mon, Mar 23, 2020 at 10:13 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
>
> On Fri, 2020-03-20 at 16:18 +1100, Jordan Niethe wrote:
> > In preparation for prefixed instructions where all instructions are no
> > longer words, use an accessor for getting a word instruction as a u32
> > from the instruction data type.
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v4: New to series
> > ---
> >  arch/powerpc/kernel/align.c          |   2 +-
> >  arch/powerpc/kernel/kprobes.c        |   2 +-
> >  arch/powerpc/kernel/trace/ftrace.c   |  16 +-
> >  arch/powerpc/lib/code-patching.c     |   2 +-
> >  arch/powerpc/lib/feature-fixups.c    |   4 +-
> >  arch/powerpc/lib/sstep.c             | 270 ++++++++++++++-------------
> >  arch/powerpc/lib/test_emulate_step.c |   4 +-
> >  arch/powerpc/xmon/xmon.c             |   4 +-
> >  8 files changed, 153 insertions(+), 151 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> > index 77c49dfdc1b4..b246ca124931 100644
> > --- a/arch/powerpc/kernel/align.c
> > +++ b/arch/powerpc/kernel/align.c
> > @@ -309,7 +309,7 @@ int fix_alignment(struct pt_regs *regs)
> >               /* We don't handle PPC little-endian any more... */
> >               if (cpu_has_feature(CPU_FTR_PPC_LE))
> >                       return -EIO;
> > -             instr = PPC_INST(swab32(instr));
> > +             instr = PPC_INST(swab32(ppc_inst_word(instr)));
> >       }
> >
> >  #ifdef CONFIG_SPE
> > diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> > index 4c2b656615a6..0c600b6e4ead 100644
> > --- a/arch/powerpc/kernel/kprobes.c
> > +++ b/arch/powerpc/kernel/kprobes.c
> > @@ -242,7 +242,7 @@ static int try_to_emulate(struct kprobe *p, struct
> > pt_regs *regs)
> >                * So, we should never get here... but, its still
> >                * good to catch them, just in case...
> >                */
> > -             printk("Can't step on instruction %x\n", insn);
> > +             printk("Can't step on instruction %x\n", ppc_inst_word(insn));
> >               BUG();
> >       } else {
> >               /*
> > diff --git a/arch/powerpc/kernel/trace/ftrace.c
> > b/arch/powerpc/kernel/trace/ftrace.c
> > index b3645b664819..7614a9f537fd 100644
> > --- a/arch/powerpc/kernel/trace/ftrace.c
> > +++ b/arch/powerpc/kernel/trace/ftrace.c
> > @@ -74,7 +74,7 @@ ftrace_modify_code(unsigned long ip, ppc_inst old, ppc_inst
> > new)
> >       /* Make sure it is what we expect it to be */
> >       if (!ppc_inst_equal(replaced, old)) {
> >               pr_err("%p: replaced (%#x) != old (%#x)",
> > -             (void *)ip, replaced, old);
> > +             (void *)ip, ppc_inst_word(replaced), ppc_inst_word(old));
> >               return -EINVAL;
> >       }
> >
> > @@ -136,7 +136,7 @@ __ftrace_make_nop(struct module *mod,
> >
> >       /* Make sure that that this is still a 24bit jump */
> >       if (!is_bl_op(op)) {
> > -             pr_err("Not expected bl: opcode is %x\n", op);
> > +             pr_err("Not expected bl: opcode is %x\n", ppc_inst_word(op));
> >               return -EINVAL;
> >       }
> >
> > @@ -171,7 +171,7 @@ __ftrace_make_nop(struct module *mod,
> >       /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */
> >       if (!ppc_inst_equal(op, PPC_INST(PPC_INST_MFLR)) &&
> >           !ppc_inst_equal(op, PPC_INST(PPC_INST_STD_LR))) {
> > -             pr_err("Unexpected instruction %08x around bl _mcount\n", op);
> > +             pr_err("Unexpected instruction %08x around bl _mcount\n",
> > ppc_inst_word(op));
> >               return -EINVAL;
> >       }
> >  #else
> > @@ -201,7 +201,7 @@ __ftrace_make_nop(struct module *mod,
> >       }
> >
> >       if (!ppc_inst_equal(op,  PPC_INST(PPC_INST_LD_TOC))) {
> > -             pr_err("Expected %08x found %08x\n", PPC_INST_LD_TOC, op);
> > +             pr_err("Expected %08x found %08x\n", PPC_INST_LD_TOC,
> > ppc_inst_word(op));
> >               return -EINVAL;
> >       }
> >  #endif /* CONFIG_MPROFILE_KERNEL */
> > @@ -401,7 +401,7 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace
> > *rec, unsigned long addr)
> >
> >       /* Make sure that that this is still a 24bit jump */
> >       if (!is_bl_op(op)) {
> > -             pr_err("Not expected bl: opcode is %x\n", op);
> > +             pr_err("Not expected bl: opcode is %x\n", ppc_inst_word(op));
> >               return -EINVAL;
> >       }
> >
> > @@ -525,7 +525,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> > addr)
> >
> >       if (!expected_nop_sequence(ip, op[0], op[1])) {
> >               pr_err("Unexpected call sequence at %p: %x %x\n",
> > -             ip, op[0], op[1]);
> > +             ip, ppc_inst_word(op[0]), ppc_inst_word(op[1]));
> >               return -EINVAL;
> >       }
> >
> > @@ -644,7 +644,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace
> > *rec, unsigned long addr)
> >       }
> >
> >       if (!ppc_inst_equal(op, PPC_INST(PPC_INST_NOP))) {
> > -             pr_err("Unexpected call sequence at %p: %x\n", ip, op);
> > +             pr_err("Unexpected call sequence at %p: %x\n", ip,
> > ppc_inst_word(op));
> >               return -EINVAL;
> >       }
> >
> > @@ -723,7 +723,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned
> > long old_addr,
> >
> >       /* Make sure that that this is still a 24bit jump */
> >       if (!is_bl_op(op)) {
> > -             pr_err("Not expected bl: opcode is %x\n", op);
> > +             pr_err("Not expected bl: opcode is %x\n", ppc_inst_word(op));
> >               return -EINVAL;
> >       }
> >
> > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> > patching.c
> > index ec3abe1a6927..849eee63df3d 100644
> > --- a/arch/powerpc/lib/code-patching.c
> > +++ b/arch/powerpc/lib/code-patching.c
> > @@ -233,7 +233,7 @@ bool is_conditional_branch(ppc_inst instr)
> >       if (opcode == 16)       /* bc, bca, bcl, bcla */
> >               return true;
> >       if (opcode == 19) {
> > -             switch ((instr >> 1) & 0x3ff) {
> > +             switch ((ppc_inst_word(instr) >> 1) & 0x3ff) {
> >               case 16:        /* bclr, bclrl */
> >               case 528:       /* bcctr, bcctrl */
> >               case 560:       /* bctar, bctarl */
> > diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-
> > fixups.c
> > index 552106d1f64a..fe8ec099aa96 100644
> > --- a/arch/powerpc/lib/feature-fixups.c
> > +++ b/arch/powerpc/lib/feature-fixups.c
> > @@ -54,8 +54,8 @@ static int patch_alt_instruction(unsigned int *src,
> > unsigned int *dest,
> >
> >               /* Branch within the section doesn't need translating */
> >               if (target < alt_start || target > alt_end) {
> > -                     instr = translate_branch(dest, src);
> > -                     if (ppc_inst_null(instr))
> > +                     instr = ppc_inst_word(translate_branch((ppc_inst
> > *)dest, (ppc_inst *)src));
> > +                     if (!instr)
> >                               return 1;
> >               }
> >       }
> > diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> > index 1d9c766a89fe..bae878a83fa5 100644
> > --- a/arch/powerpc/lib/sstep.c
> > +++ b/arch/powerpc/lib/sstep.c
> > @@ -1169,26 +1169,28 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >       unsigned long int imm;
> >       unsigned long int val, val2;
> >       unsigned int mb, me, sh;
> > +     unsigned int word;
> >       long ival;
> >
> > +     word = ppc_inst_word(instr);
> >       op->type = COMPUTE;
> >
> > -     opcode = instr >> 26;
> > +     opcode = word >> 26;
> >       switch (opcode) {
> >       case 16:        /* bc */
> >               op->type = BRANCH;
> > -             imm = (signed short)(instr & 0xfffc);
> > -             if ((instr & 2) == 0)
> > +             imm = (signed short)(word & 0xfffc);
> > +             if ((word & 2) == 0)
> >                       imm += regs->nip;
> >               op->val = truncate_if_32bit(regs->msr, imm);
> > -             if (instr & 1)
> > +             if (word & 1)
> >                       op->type |= SETLK;
> > -             if (branch_taken(instr, regs, op))
> > +             if (branch_taken(word, regs, op))
> >                       op->type |= BRTAKEN;
> >               return 1;
> >  #ifdef CONFIG_PPC64
> >       case 17:        /* sc */
> > -             if ((instr & 0xfe2) == 2)
> > +             if ((word & 0xfe2) == 2)
> >                       op->type = SYSCALL;
> >               else
> >                       op->type = UNKNOWN;
> > @@ -1196,21 +1198,21 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >  #endif
> >       case 18:        /* b */
> >               op->type = BRANCH | BRTAKEN;
> > -             imm = instr & 0x03fffffc;
> > +             imm = word & 0x03fffffc;
> >               if (imm & 0x02000000)
> >                       imm -= 0x04000000;
> > -             if ((instr & 2) == 0)
> > +             if ((word & 2) == 0)
> >                       imm += regs->nip;
> >               op->val = truncate_if_32bit(regs->msr, imm);
> > -             if (instr & 1)
> > +             if (word & 1)
> >                       op->type |= SETLK;
> >               return 1;
> >       case 19:
> > -             switch ((instr >> 1) & 0x3ff) {
> > +             switch ((word >> 1) & 0x3ff) {
> >               case 0:         /* mcrf */
> >                       op->type = COMPUTE + SETCC;
> > -                     rd = 7 - ((instr >> 23) & 0x7);
> > -                     ra = 7 - ((instr >> 18) & 0x7);
> > +                     rd = 7 - ((word >> 23) & 0x7);
> > +                     ra = 7 - ((word >> 18) & 0x7);
> >                       rd *= 4;
> >                       ra *= 4;
> >                       val = (regs->ccr >> ra) & 0xf;
> > @@ -1220,11 +1222,11 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >               case 16:        /* bclr */
> >               case 528:       /* bcctr */
> >                       op->type = BRANCH;
> > -                     imm = (instr & 0x400)? regs->ctr: regs->link;
> > +                     imm = (word & 0x400)? regs->ctr: regs->link;
> >                       op->val = truncate_if_32bit(regs->msr, imm);
> > -                     if (instr & 1)
> > +                     if (word & 1)
> >                               op->type |= SETLK;
> > -                     if (branch_taken(instr, regs, op))
> > +                     if (branch_taken(word, regs, op))
> >                               op->type |= BRTAKEN;
> >                       return 1;
> >
> > @@ -1247,23 +1249,23 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >               case 417:       /* crorc */
> >               case 449:       /* cror */
> >                       op->type = COMPUTE + SETCC;
> > -                     ra = (instr >> 16) & 0x1f;
> > -                     rb = (instr >> 11) & 0x1f;
> > -                     rd = (instr >> 21) & 0x1f;
> > +                     ra = (word >> 16) & 0x1f;
> > +                     rb = (word >> 11) & 0x1f;
> > +                     rd = (word >> 21) & 0x1f;
>
> can't we use your accessors for all these operations ?
I felt that since we are doing so many bit operations here it was
simpliest to just leave these as uints.
>
> >                       ra = (regs->ccr >> (31 - ra)) & 1;
> >                       rb = (regs->ccr >> (31 - rb)) & 1;
> > -                     val = (instr >> (6 + ra * 2 + rb)) & 1;
> > +                     val = (word >> (6 + ra * 2 + rb)) & 1;
> >                       op->ccval = (regs->ccr & ~(1UL << (31 - rd))) |
> >                               (val << (31 - rd));
> >                       return 1;
> >               }
> >               break;
> >       case 31:
> > -             switch ((instr >> 1) & 0x3ff) {
> > +             switch ((word >> 1) & 0x3ff) {
> >               case 598:       /* sync */
> >                       op->type = BARRIER + BARRIER_SYNC;
> >  #ifdef __powerpc64__
> > -                     switch ((instr >> 21) & 3) {
> > +                     switch ((word >> 21) & 3) {
> >                       case 1:         /* lwsync */
> >                               op->type = BARRIER + BARRIER_LWSYNC;
> >                               break;
> > @@ -1285,20 +1287,20 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >       if (!FULL_REGS(regs))
> >               return -1;
> >
> > -     rd = (instr >> 21) & 0x1f;
> > -     ra = (instr >> 16) & 0x1f;
> > -     rb = (instr >> 11) & 0x1f;
> > -     rc = (instr >> 6) & 0x1f;
> > +     rd = (word >> 21) & 0x1f;
> > +     ra = (word >> 16) & 0x1f;
> > +     rb = (word >> 11) & 0x1f;
> > +     rc = (word >> 6) & 0x1f;
>
> same here and in similar such places.
>
> -- Bala
> >
> >       switch (opcode) {
> >  #ifdef __powerpc64__
> >       case 2:         /* tdi */
> > -             if (rd & trap_compare(regs->gpr[ra], (short) instr))
> > +             if (rd & trap_compare(regs->gpr[ra], (short) word))
> >                       goto trap;
> >               return 1;
> >  #endif
> >       case 3:         /* twi */
> > -             if (rd & trap_compare((int)regs->gpr[ra], (short) instr))
> > +             if (rd & trap_compare((int)regs->gpr[ra], (short) word))
> >                       goto trap;
> >               return 1;
> >
> > @@ -1307,7 +1309,7 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >               if (!cpu_has_feature(CPU_FTR_ARCH_300))
> >                       return -1;
> >
> > -             switch (instr & 0x3f) {
> > +             switch (word & 0x3f) {
> >               case 48:        /* maddhd */
> >                       asm volatile(PPC_MADDHD(%0, %1, %2, %3) :
> >                                    "=r" (op->val) : "r" (regs->gpr[ra]),
> > @@ -1335,16 +1337,16 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >  #endif
> >
> >       case 7:         /* mulli */
> > -             op->val = regs->gpr[ra] * (short) instr;
> > +             op->val = regs->gpr[ra] * (short) word;
> >               goto compute_done;
> >
> >       case 8:         /* subfic */
> > -             imm = (short) instr;
> > +             imm = (short) word;
> >               add_with_carry(regs, op, rd, ~regs->gpr[ra], imm, 1);
> >               return 1;
> >
> >       case 10:        /* cmpli */
> > -             imm = (unsigned short) instr;
> > +             imm = (unsigned short) word;
> >               val = regs->gpr[ra];
> >  #ifdef __powerpc64__
> >               if ((rd & 1) == 0)
> > @@ -1354,7 +1356,7 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >               return 1;
> >
> >       case 11:        /* cmpi */
> > -             imm = (short) instr;
> > +             imm = (short) word;
> >               val = regs->gpr[ra];
> >  #ifdef __powerpc64__
> >               if ((rd & 1) == 0)
> > @@ -1364,35 +1366,35 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >               return 1;
> >
> >       case 12:        /* addic */
> > -             imm = (short) instr;
> > +             imm = (short) word;
> >               add_with_carry(regs, op, rd, regs->gpr[ra], imm, 0);
> >               return 1;
> >
> >       case 13:        /* addic. */
> > -             imm = (short) instr;
> > +             imm = (short) word;
> >               add_with_carry(regs, op, rd, regs->gpr[ra], imm, 0);
> >               set_cr0(regs, op);
> >               return 1;
> >
> >       case 14:        /* addi */
> > -             imm = (short) instr;
> > +             imm = (short) word;
> >               if (ra)
> >                       imm += regs->gpr[ra];
> >               op->val = imm;
> >               goto compute_done;
> >
> >       case 15:        /* addis */
> > -             imm = ((short) instr) << 16;
> > +             imm = ((short) word) << 16;
> >               if (ra)
> >                       imm += regs->gpr[ra];
> >               op->val = imm;
> >               goto compute_done;
> >
> >       case 19:
> > -             if (((instr >> 1) & 0x1f) == 2) {
> > +             if (((word >> 1) & 0x1f) == 2) {
> >                       /* addpcis */
> > -                     imm = (short) (instr & 0xffc1); /* d0 + d2 fields */
> > -                     imm |= (instr >> 15) & 0x3e;    /* d1 field */
> > +                     imm = (short) (word & 0xffc1);  /* d0 + d2 fields */
> > +                     imm |= (word >> 15) & 0x3e;     /* d1 field */
> >                       op->val = regs->nip + (imm << 16) + 4;
> >                       goto compute_done;
> >               }
> > @@ -1400,65 +1402,65 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >               return 0;
> >
> >       case 20:        /* rlwimi */
> > -             mb = (instr >> 6) & 0x1f;
> > -             me = (instr >> 1) & 0x1f;
> > +             mb = (word >> 6) & 0x1f;
> > +             me = (word >> 1) & 0x1f;
> >               val = DATA32(regs->gpr[rd]);
> >               imm = MASK32(mb, me);
> >               op->val = (regs->gpr[ra] & ~imm) | (ROTATE(val, rb) & imm);
> >               goto logical_done;
> >
> >       case 21:        /* rlwinm */
> > -             mb = (instr >> 6) & 0x1f;
> > -             me = (instr >> 1) & 0x1f;
> > +             mb = (word >> 6) & 0x1f;
> > +             me = (word >> 1) & 0x1f;
> >               val = DATA32(regs->gpr[rd]);
> >               op->val = ROTATE(val, rb) & MASK32(mb, me);
> >               goto logical_done;
> >
> >       case 23:        /* rlwnm */
> > -             mb = (instr >> 6) & 0x1f;
> > -             me = (instr >> 1) & 0x1f;
> > +             mb = (word >> 6) & 0x1f;
> > +             me = (word >> 1) & 0x1f;
> >               rb = regs->gpr[rb] & 0x1f;
> >               val = DATA32(regs->gpr[rd]);
> >               op->val = ROTATE(val, rb) & MASK32(mb, me);
> >               goto logical_done;
> >
> >       case 24:        /* ori */
> > -             op->val = regs->gpr[rd] | (unsigned short) instr;
> > +             op->val = regs->gpr[rd] | (unsigned short) word;
> >               goto logical_done_nocc;
> >
> >       case 25:        /* oris */
> > -             imm = (unsigned short) instr;
> > +             imm = (unsigned short) word;
> >               op->val = regs->gpr[rd] | (imm << 16);
> >               goto logical_done_nocc;
> >
> >       case 26:        /* xori */
> > -             op->val = regs->gpr[rd] ^ (unsigned short) instr;
> > +             op->val = regs->gpr[rd] ^ (unsigned short) word;
> >               goto logical_done_nocc;
> >
> >       case 27:        /* xoris */
> > -             imm = (unsigned short) instr;
> > +             imm = (unsigned short) word;
> >               op->val = regs->gpr[rd] ^ (imm << 16);
> >               goto logical_done_nocc;
> >
> >       case 28:        /* andi. */
> > -             op->val = regs->gpr[rd] & (unsigned short) instr;
> > +             op->val = regs->gpr[rd] & (unsigned short) word;
> >               set_cr0(regs, op);
> >               goto logical_done_nocc;
> >
> >       case 29:        /* andis. */
> > -             imm = (unsigned short) instr;
> > +             imm = (unsigned short) word;
> >               op->val = regs->gpr[rd] & (imm << 16);
> >               set_cr0(regs, op);
> >               goto logical_done_nocc;
> >
> >  #ifdef __powerpc64__
> >       case 30:        /* rld* */
> > -             mb = ((instr >> 6) & 0x1f) | (instr & 0x20);
> > +             mb = ((word >> 6) & 0x1f) | (word & 0x20);
> >               val = regs->gpr[rd];
> > -             if ((instr & 0x10) == 0) {
> > -                     sh = rb | ((instr & 2) << 4);
> > +             if ((word & 0x10) == 0) {
> > +                     sh = rb | ((word & 2) << 4);
> >                       val = ROTATE(val, sh);
> > -                     switch ((instr >> 2) & 3) {
> > +                     switch ((word >> 2) & 3) {
> >                       case 0:         /* rldicl */
> >                               val &= MASK64_L(mb);
> >                               break;
> > @@ -1478,7 +1480,7 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >               } else {
> >                       sh = regs->gpr[rb] & 0x3f;
> >                       val = ROTATE(val, sh);
> > -                     switch ((instr >> 1) & 7) {
> > +                     switch ((word >> 1) & 7) {
> >                       case 0:         /* rldcl */
> >                               op->val = val & MASK64_L(mb);
> >                               goto logical_done;
> > @@ -1493,8 +1495,8 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >
> >       case 31:
> >               /* isel occupies 32 minor opcodes */
> > -             if (((instr >> 1) & 0x1f) == 15) {
> > -                     mb = (instr >> 6) & 0x1f; /* bc field */
> > +             if (((word >> 1) & 0x1f) == 15) {
> > +                     mb = (word >> 6) & 0x1f; /* bc field */
> >                       val = (regs->ccr >> (31 - mb)) & 1;
> >                       val2 = (ra) ? regs->gpr[ra] : 0;
> >
> > @@ -1502,7 +1504,7 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >                       goto compute_done;
> >               }
> >
> > -             switch ((instr >> 1) & 0x3ff) {
> > +             switch ((word >> 1) & 0x3ff) {
> >               case 4:         /* tw */
> >                       if (rd == 0x1f ||
> >                           (rd & trap_compare((int)regs->gpr[ra],
> > @@ -1536,17 +1538,17 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >                       op->reg = rd;
> >                       /* only MSR_EE and MSR_RI get changed if bit 15 set */
> >                       /* mtmsrd doesn't change MSR_HV, MSR_ME or MSR_LE */
> > -                     imm = (instr & 0x10000)? 0x8002: 0xefffffffffffeffeUL;
> > +                     imm = (word & 0x10000)? 0x8002: 0xefffffffffffeffeUL;
> >                       op->val = imm;
> >                       return 0;
> >  #endif
> >
> >               case 19:        /* mfcr */
> >                       imm = 0xffffffffUL;
> > -                     if ((instr >> 20) & 1) {
> > +                     if ((word >> 20) & 1) {
> >                               imm = 0xf0000000UL;
> >                               for (sh = 0; sh < 8; ++sh) {
> > -                                     if (instr & (0x80000 >> sh))
> > +                                     if (word & (0x80000 >> sh))
> >                                               break;
> >                                       imm >>= 4;
> >                               }
> > @@ -1560,7 +1562,7 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >                       val = regs->gpr[rd];
> >                       op->ccval = regs->ccr;
> >                       for (sh = 0; sh < 8; ++sh) {
> > -                             if (instr & (0x80000 >> sh))
> > +                             if (word & (0x80000 >> sh))
> >                                       op->ccval = (op->ccval & ~imm) |
> >                                               (val & imm);
> >                               imm >>= 4;
> > @@ -1568,7 +1570,7 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >                       return 1;
> >
> >               case 339:       /* mfspr */
> > -                     spr = ((instr >> 16) & 0x1f) | ((instr >> 6) & 0x3e0);
> > +                     spr = ((word >> 16) & 0x1f) | ((word >> 6) & 0x3e0);
> >                       op->type = MFSPR;
> >                       op->reg = rd;
> >                       op->spr = spr;
> > @@ -1578,7 +1580,7 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >                       return 0;
> >
> >               case 467:       /* mtspr */
> > -                     spr = ((instr >> 16) & 0x1f) | ((instr >> 6) & 0x3e0);
> > +                     spr = ((word >> 16) & 0x1f) | ((word >> 6) & 0x3e0);
> >                       op->type = MTSPR;
> >                       op->val = regs->gpr[rd];
> >                       op->spr = spr;
> > @@ -1948,7 +1950,7 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >               case 826:       /* sradi with sh_5 = 0 */
> >               case 827:       /* sradi with sh_5 = 1 */
> >                       op->type = COMPUTE + SETREG + SETXER;
> > -                     sh = rb | ((instr & 2) << 4);
> > +                     sh = rb | ((word & 2) << 4);
> >                       ival = (signed long int) regs->gpr[rd];
> >                       op->val = ival >> sh;
> >                       op->xerval = regs->xer;
> > @@ -1964,7 +1966,7 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >                       if (!cpu_has_feature(CPU_FTR_ARCH_300))
> >                               return -1;
> >                       op->type = COMPUTE + SETREG;
> > -                     sh = rb | ((instr & 2) << 4);
> > +                     sh = rb | ((word & 2) << 4);
> >                       val = (signed int) regs->gpr[rd];
> >                       if (sh)
> >                               op->val = ROTATE(val, sh) & MASK64(0, 63 - sh);
> > @@ -1979,34 +1981,34 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >   */
> >               case 54:        /* dcbst */
> >                       op->type = MKOP(CACHEOP, DCBST, 0);
> > -                     op->ea = xform_ea(instr, regs);
> > +                     op->ea = xform_ea(word, regs);
> >                       return 0;
> >
> >               case 86:        /* dcbf */
> >                       op->type = MKOP(CACHEOP, DCBF, 0);
> > -                     op->ea = xform_ea(instr, regs);
> > +                     op->ea = xform_ea(word, regs);
> >                       return 0;
> >
> >               case 246:       /* dcbtst */
> >                       op->type = MKOP(CACHEOP, DCBTST, 0);
> > -                     op->ea = xform_ea(instr, regs);
> > +                     op->ea = xform_ea(word, regs);
> >                       op->reg = rd;
> >                       return 0;
> >
> >               case 278:       /* dcbt */
> >                       op->type = MKOP(CACHEOP, DCBTST, 0);
> > -                     op->ea = xform_ea(instr, regs);
> > +                     op->ea = xform_ea(word, regs);
> >                       op->reg = rd;
> >                       return 0;
> >
> >               case 982:       /* icbi */
> >                       op->type = MKOP(CACHEOP, ICBI, 0);
> > -                     op->ea = xform_ea(instr, regs);
> > +                     op->ea = xform_ea(word, regs);
> >                       return 0;
> >
> >               case 1014:      /* dcbz */
> >                       op->type = MKOP(CACHEOP, DCBZ, 0);
> > -                     op->ea = xform_ea(instr, regs);
> > +                     op->ea = xform_ea(word, regs);
> >                       return 0;
> >               }
> >               break;
> > @@ -2019,14 +2021,14 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >       op->update_reg = ra;
> >       op->reg = rd;
> >       op->val = regs->gpr[rd];
> > -     u = (instr >> 20) & UPDATE;
> > +     u = (word >> 20) & UPDATE;
> >       op->vsx_flags = 0;
> >
> >       switch (opcode) {
> >       case 31:
> > -             u = instr & UPDATE;
> > -             op->ea = xform_ea(instr, regs);
> > -             switch ((instr >> 1) & 0x3ff) {
> > +             u = word & UPDATE;
> > +             op->ea = xform_ea(word, regs);
> > +             switch ((word >> 1) & 0x3ff) {
> >               case 20:        /* lwarx */
> >                       op->type = MKOP(LARX, 0, 4);
> >                       break;
> > @@ -2271,25 +2273,25 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >
> >  #ifdef CONFIG_VSX
> >               case 12:        /* lxsiwzx */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(LOAD_VSX, 0, 4);
> >                       op->element_size = 8;
> >                       break;
> >
> >               case 76:        /* lxsiwax */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(LOAD_VSX, SIGNEXT, 4);
> >                       op->element_size = 8;
> >                       break;
> >
> >               case 140:       /* stxsiwx */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(STORE_VSX, 0, 4);
> >                       op->element_size = 8;
> >                       break;
> >
> >               case 268:       /* lxvx */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(LOAD_VSX, 0, 16);
> >                       op->element_size = 16;
> >                       op->vsx_flags = VSX_CHECK_VEC;
> > @@ -2298,33 +2300,33 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >               case 269:       /* lxvl */
> >               case 301: {     /* lxvll */
> >                       int nb;
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->ea = ra ? regs->gpr[ra] : 0;
> >                       nb = regs->gpr[rb] & 0xff;
> >                       if (nb > 16)
> >                               nb = 16;
> >                       op->type = MKOP(LOAD_VSX, 0, nb);
> >                       op->element_size = 16;
> > -                     op->vsx_flags = ((instr & 0x20) ? VSX_LDLEFT : 0) |
> > +                     op->vsx_flags = ((word & 0x20) ? VSX_LDLEFT : 0) |
> >                               VSX_CHECK_VEC;
> >                       break;
> >               }
> >               case 332:       /* lxvdsx */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(LOAD_VSX, 0, 8);
> >                       op->element_size = 8;
> >                       op->vsx_flags = VSX_SPLAT;
> >                       break;
> >
> >               case 364:       /* lxvwsx */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(LOAD_VSX, 0, 4);
> >                       op->element_size = 4;
> >                       op->vsx_flags = VSX_SPLAT | VSX_CHECK_VEC;
> >                       break;
> >
> >               case 396:       /* stxvx */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(STORE_VSX, 0, 16);
> >                       op->element_size = 16;
> >                       op->vsx_flags = VSX_CHECK_VEC;
> > @@ -2333,118 +2335,118 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >               case 397:       /* stxvl */
> >               case 429: {     /* stxvll */
> >                       int nb;
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->ea = ra ? regs->gpr[ra] : 0;
> >                       nb = regs->gpr[rb] & 0xff;
> >                       if (nb > 16)
> >                               nb = 16;
> >                       op->type = MKOP(STORE_VSX, 0, nb);
> >                       op->element_size = 16;
> > -                     op->vsx_flags = ((instr & 0x20) ? VSX_LDLEFT : 0) |
> > +                     op->vsx_flags = ((word & 0x20) ? VSX_LDLEFT : 0) |
> >                               VSX_CHECK_VEC;
> >                       break;
> >               }
> >               case 524:       /* lxsspx */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(LOAD_VSX, 0, 4);
> >                       op->element_size = 8;
> >                       op->vsx_flags = VSX_FPCONV;
> >                       break;
> >
> >               case 588:       /* lxsdx */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(LOAD_VSX, 0, 8);
> >                       op->element_size = 8;
> >                       break;
> >
> >               case 652:       /* stxsspx */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(STORE_VSX, 0, 4);
> >                       op->element_size = 8;
> >                       op->vsx_flags = VSX_FPCONV;
> >                       break;
> >
> >               case 716:       /* stxsdx */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(STORE_VSX, 0, 8);
> >                       op->element_size = 8;
> >                       break;
> >
> >               case 780:       /* lxvw4x */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(LOAD_VSX, 0, 16);
> >                       op->element_size = 4;
> >                       break;
> >
> >               case 781:       /* lxsibzx */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(LOAD_VSX, 0, 1);
> >                       op->element_size = 8;
> >                       op->vsx_flags = VSX_CHECK_VEC;
> >                       break;
> >
> >               case 812:       /* lxvh8x */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(LOAD_VSX, 0, 16);
> >                       op->element_size = 2;
> >                       op->vsx_flags = VSX_CHECK_VEC;
> >                       break;
> >
> >               case 813:       /* lxsihzx */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(LOAD_VSX, 0, 2);
> >                       op->element_size = 8;
> >                       op->vsx_flags = VSX_CHECK_VEC;
> >                       break;
> >
> >               case 844:       /* lxvd2x */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(LOAD_VSX, 0, 16);
> >                       op->element_size = 8;
> >                       break;
> >
> >               case 876:       /* lxvb16x */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(LOAD_VSX, 0, 16);
> >                       op->element_size = 1;
> >                       op->vsx_flags = VSX_CHECK_VEC;
> >                       break;
> >
> >               case 908:       /* stxvw4x */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(STORE_VSX, 0, 16);
> >                       op->element_size = 4;
> >                       break;
> >
> >               case 909:       /* stxsibx */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(STORE_VSX, 0, 1);
> >                       op->element_size = 8;
> >                       op->vsx_flags = VSX_CHECK_VEC;
> >                       break;
> >
> >               case 940:       /* stxvh8x */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(STORE_VSX, 0, 16);
> >                       op->element_size = 2;
> >                       op->vsx_flags = VSX_CHECK_VEC;
> >                       break;
> >
> >               case 941:       /* stxsihx */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(STORE_VSX, 0, 2);
> >                       op->element_size = 8;
> >                       op->vsx_flags = VSX_CHECK_VEC;
> >                       break;
> >
> >               case 972:       /* stxvd2x */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(STORE_VSX, 0, 16);
> >                       op->element_size = 8;
> >                       break;
> >
> >               case 1004:      /* stxvb16x */
> > -                     op->reg = rd | ((instr & 1) << 5);
> > +                     op->reg = rd | ((word & 1) << 5);
> >                       op->type = MKOP(STORE_VSX, 0, 16);
> >                       op->element_size = 1;
> >                       op->vsx_flags = VSX_CHECK_VEC;
> > @@ -2457,80 +2459,80 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >       case 32:        /* lwz */
> >       case 33:        /* lwzu */
> >               op->type = MKOP(LOAD, u, 4);
> > -             op->ea = dform_ea(instr, regs);
> > +             op->ea = dform_ea(word, regs);
> >               break;
> >
> >       case 34:        /* lbz */
> >       case 35:        /* lbzu */
> >               op->type = MKOP(LOAD, u, 1);
> > -             op->ea = dform_ea(instr, regs);
> > +             op->ea = dform_ea(word, regs);
> >               break;
> >
> >       case 36:        /* stw */
> >       case 37:        /* stwu */
> >               op->type = MKOP(STORE, u, 4);
> > -             op->ea = dform_ea(instr, regs);
> > +             op->ea = dform_ea(word, regs);
> >               break;
> >
> >       case 38:        /* stb */
> >       case 39:        /* stbu */
> >               op->type = MKOP(STORE, u, 1);
> > -             op->ea = dform_ea(instr, regs);
> > +             op->ea = dform_ea(word, regs);
> >               break;
> >
> >       case 40:        /* lhz */
> >       case 41:        /* lhzu */
> >               op->type = MKOP(LOAD, u, 2);
> > -             op->ea = dform_ea(instr, regs);
> > +             op->ea = dform_ea(word, regs);
> >               break;
> >
> >       case 42:        /* lha */
> >       case 43:        /* lhau */
> >               op->type = MKOP(LOAD, SIGNEXT | u, 2);
> > -             op->ea = dform_ea(instr, regs);
> > +             op->ea = dform_ea(word, regs);
> >               break;
> >
> >       case 44:        /* sth */
> >       case 45:        /* sthu */
> >               op->type = MKOP(STORE, u, 2);
> > -             op->ea = dform_ea(instr, regs);
> > +             op->ea = dform_ea(word, regs);
> >               break;
> >
> >       case 46:        /* lmw */
> >               if (ra >= rd)
> >                       break;          /* invalid form, ra in range to load
> > */
> >               op->type = MKOP(LOAD_MULTI, 0, 4 * (32 - rd));
> > -             op->ea = dform_ea(instr, regs);
> > +             op->ea = dform_ea(word, regs);
> >               break;
> >
> >       case 47:        /* stmw */
> >               op->type = MKOP(STORE_MULTI, 0, 4 * (32 - rd));
> > -             op->ea = dform_ea(instr, regs);
> > +             op->ea = dform_ea(word, regs);
> >               break;
> >
> >  #ifdef CONFIG_PPC_FPU
> >       case 48:        /* lfs */
> >       case 49:        /* lfsu */
> >               op->type = MKOP(LOAD_FP, u | FPCONV, 4);
> > -             op->ea = dform_ea(instr, regs);
> > +             op->ea = dform_ea(word, regs);
> >               break;
> >
> >       case 50:        /* lfd */
> >       case 51:        /* lfdu */
> >               op->type = MKOP(LOAD_FP, u, 8);
> > -             op->ea = dform_ea(instr, regs);
> > +             op->ea = dform_ea(word, regs);
> >               break;
> >
> >       case 52:        /* stfs */
> >       case 53:        /* stfsu */
> >               op->type = MKOP(STORE_FP, u | FPCONV, 4);
> > -             op->ea = dform_ea(instr, regs);
> > +             op->ea = dform_ea(word, regs);
> >               break;
> >
> >       case 54:        /* stfd */
> >       case 55:        /* stfdu */
> >               op->type = MKOP(STORE_FP, u, 8);
> > -             op->ea = dform_ea(instr, regs);
> > +             op->ea = dform_ea(word, regs);
> >               break;
> >  #endif
> >
> > @@ -2538,14 +2540,14 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >       case 56:        /* lq */
> >               if (!((rd & 1) || (rd == ra)))
> >                       op->type = MKOP(LOAD, 0, 16);
> > -             op->ea = dqform_ea(instr, regs);
> > +             op->ea = dqform_ea(word, regs);
> >               break;
> >  #endif
> >
> >  #ifdef CONFIG_VSX
> >       case 57:        /* lfdp, lxsd, lxssp */
> > -             op->ea = dsform_ea(instr, regs);
> > -             switch (instr & 3) {
> > +             op->ea = dsform_ea(word, regs);
> > +             switch (word & 3) {
> >               case 0:         /* lfdp */
> >                       if (rd & 1)
> >                               break;          /* reg must be even */
> > @@ -2569,8 +2571,8 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >
> >  #ifdef __powerpc64__
> >       case 58:        /* ld[u], lwa */
> > -             op->ea = dsform_ea(instr, regs);
> > -             switch (instr & 3) {
> > +             op->ea = dsform_ea(word, regs);
> > +             switch (word & 3) {
> >               case 0:         /* ld */
> >                       op->type = MKOP(LOAD, 0, 8);
> >                       break;
> > @@ -2586,16 +2588,16 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >
> >  #ifdef CONFIG_VSX
> >       case 61:        /* stfdp, lxv, stxsd, stxssp, stxv */
> > -             switch (instr & 7) {
> > +             switch (word & 7) {
> >               case 0:         /* stfdp with LSB of DS field = 0 */
> >               case 4:         /* stfdp with LSB of DS field = 1 */
> > -                     op->ea = dsform_ea(instr, regs);
> > +                     op->ea = dsform_ea(word, regs);
> >                       op->type = MKOP(STORE_FP, 0, 16);
> >                       break;
> >
> >               case 1:         /* lxv */
> > -                     op->ea = dqform_ea(instr, regs);
> > -                     if (instr & 8)
> > +                     op->ea = dqform_ea(word, regs);
> > +                     if (word & 8)
> >                               op->reg = rd + 32;
> >                       op->type = MKOP(LOAD_VSX, 0, 16);
> >                       op->element_size = 16;
> > @@ -2604,7 +2606,7 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >
> >               case 2:         /* stxsd with LSB of DS field = 0 */
> >               case 6:         /* stxsd with LSB of DS field = 1 */
> > -                     op->ea = dsform_ea(instr, regs);
> > +                     op->ea = dsform_ea(word, regs);
> >                       op->reg = rd + 32;
> >                       op->type = MKOP(STORE_VSX, 0, 8);
> >                       op->element_size = 8;
> > @@ -2613,7 +2615,7 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >
> >               case 3:         /* stxssp with LSB of DS field = 0 */
> >               case 7:         /* stxssp with LSB of DS field = 1 */
> > -                     op->ea = dsform_ea(instr, regs);
> > +                     op->ea = dsform_ea(word, regs);
> >                       op->reg = rd + 32;
> >                       op->type = MKOP(STORE_VSX, 0, 4);
> >                       op->element_size = 8;
> > @@ -2621,8 +2623,8 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >                       break;
> >
> >               case 5:         /* stxv */
> > -                     op->ea = dqform_ea(instr, regs);
> > -                     if (instr & 8)
> > +                     op->ea = dqform_ea(word, regs);
> > +                     if (word & 8)
> >                               op->reg = rd + 32;
> >                       op->type = MKOP(STORE_VSX, 0, 16);
> >                       op->element_size = 16;
> > @@ -2634,8 +2636,8 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >
> >  #ifdef __powerpc64__
> >       case 62:        /* std[u] */
> > -             op->ea = dsform_ea(instr, regs);
> > -             switch (instr & 3) {
> > +             op->ea = dsform_ea(word, regs);
> > +             switch (word & 3) {
> >               case 0:         /* std */
> >                       op->type = MKOP(STORE, 0, 8);
> >                       break;
> > @@ -2663,7 +2665,7 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >       return 0;
> >
> >   logical_done:
> > -     if (instr & 1)
> > +     if (word & 1)
> >               set_cr0(regs, op);
> >   logical_done_nocc:
> >       op->reg = ra;
> > @@ -2671,7 +2673,7 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> >       return 1;
> >
> >   arith_done:
> > -     if (instr & 1)
> > +     if (word & 1)
> >               set_cr0(regs, op);
> >   compute_done:
> >       op->reg = rd;
> > diff --git a/arch/powerpc/lib/test_emulate_step.c
> > b/arch/powerpc/lib/test_emulate_step.c
> > index 486e057e5be1..d6275a9b8ce6 100644
> > --- a/arch/powerpc/lib/test_emulate_step.c
> > +++ b/arch/powerpc/lib/test_emulate_step.c
> > @@ -851,7 +851,7 @@ static int __init emulate_compute_instr(struct pt_regs
> > *regs,
> >
> >       if (analyse_instr(&op, regs, instr) != 1 ||
> >           GETTYPE(op.type) != COMPUTE) {
> > -             pr_info("emulation failed, instruction = 0x%08x\n", instr);
> > +             pr_info("emulation failed, instruction = 0x%08x\n",
> > ppc_inst_word(instr));
> >               return -EFAULT;
> >       }
> >
> > @@ -871,7 +871,7 @@ static int __init execute_compute_instr(struct pt_regs
> > *regs,
> >       /* Patch the NOP with the actual instruction */
> >       patch_instruction_site(&patch__exec_instr, instr);
> >       if (exec_instr(regs)) {
> > -             pr_info("execution failed, instruction = 0x%08x\n", instr);
> > +             pr_info("execution failed, instruction = 0x%08x\n",
> > ppc_inst_word(instr));
> >               return -EFAULT;
> >       }
> >
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index d045e583f1c9..dec522fa8201 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -2871,9 +2871,9 @@ generic_inst_dump(unsigned long adr, long count, int
> > praddr,
> >               dotted = 0;
> >               last_inst = inst;
> >               if (praddr)
> > -                     printf(REG"  %.8x", adr, inst);
> > +                     printf(REG"  %.8x", adr, ppc_inst_word(inst));
> >               printf("\t");
> > -             dump_func(inst, adr);
> > +             dump_func(ppc_inst_word(inst), adr);
> >               printf("\n");
> >       }
> >       return adr - first_adr;
>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox