LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND] macintosh: convert to i2c_new_scanned_device
From: Wolfram Sang @ 2020-02-10 17:04 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: linux-i2c, Wolfram Sang

Move from the deprecated i2c_new_probed_device() to the new
i2c_new_scanned_device(). No functional change for this driver because
it doesn't check the return code anyhow.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

I can take this via I2C tree if this makes things easier...

 drivers/macintosh/therm_windtunnel.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/macintosh/therm_windtunnel.c b/drivers/macintosh/therm_windtunnel.c
index 8c744578122a..f15fec5e1cb6 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -321,10 +321,10 @@ do_attach( struct i2c_adapter *adapter )
 
 		memset(&info, 0, sizeof(struct i2c_board_info));
 		strlcpy(info.type, "therm_ds1775", I2C_NAME_SIZE);
-		i2c_new_probed_device(adapter, &info, scan_ds1775, NULL);
+		i2c_new_scanned_device(adapter, &info, scan_ds1775, NULL);
 
 		strlcpy(info.type, "therm_adm1030", I2C_NAME_SIZE);
-		i2c_new_probed_device(adapter, &info, scan_adm1030, NULL);
+		i2c_new_scanned_device(adapter, &info, scan_adm1030, NULL);
 
 		if( x.thermostat && x.fan ) {
 			x.running = 1;
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v2] libnvdimm: Update persistence domain value for of_pmem and papr_scm device
From: Dan Williams @ 2020-02-10 18:18 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: linuxppc-dev, linux-nvdimm
In-Reply-To: <87y2ta8qy7.fsf@linux.ibm.com>

On Mon, Feb 10, 2020 at 6:20 AM Aneesh Kumar K.V
<aneesh.kumar@linux.ibm.com> wrote:
>
> Dan Williams <dan.j.williams@intel.com> writes:
>
> > On Tue, Feb 4, 2020 at 9:21 PM Aneesh Kumar K.V
> > <aneesh.kumar@linux.ibm.com> wrote:
> >>
> >> Currently, kernel shows the below values
> >>         "persistence_domain":"cpu_cache"
> >>         "persistence_domain":"memory_controller"
> >>         "persistence_domain":"unknown"
> >>
> >> "cpu_cache" indicates no extra instructions is needed to ensure the persistence
> >> of data in the pmem media on power failure.
> >>
> >> "memory_controller" indicates platform provided instructions need to be issued
> >
> > No, it does not. The only requirement implied by "memory_controller"
> > is global visibility outside the cpu cache. If there are special
> > instructions beyond that then it isn't persistent memory, at least not
> > pmem that is safe for dax. virtio-pmem is an example of pmem-like
> > memory that is not enabled for userspace flushing (MAP_SYNC disabled).
> >
>
> Can you explain this more? The way I was expecting the application to
> interpret the value was, a regular store instruction doesn't guarantee
> persistence if you find the "memory_controller" value for
> persistence_domain. Instead, we need to make sure we flush data to the
> controller at which point the platform will take care of the persistence in
> case of power loss. How we flush data to the controller will also be
> defined by the platform.

If the platform requires any flush mechanism outside of the base cpu
ISA of cache flushes and memory barriers then MAP_SYNC needs to be
explicitly disabled to force the application to call fsync()/msync().
Then those platform specific mechanisms need to be triggered through a
platform-aware driver.

>
>
> >> as per documented sequence to make sure data get flushed so that it is
> >> guaranteed to be on pmem media in case of system power loss.
> >>
> >> Based on the above use memory_controller for non volatile regions on ppc64.
> >>
> >> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> >> ---
> >>  arch/powerpc/platforms/pseries/papr_scm.c | 7 ++++++-
> >>  drivers/nvdimm/of_pmem.c                  | 4 +++-
> >>  include/linux/libnvdimm.h                 | 1 -
> >>  3 files changed, 9 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> >> index 7525635a8536..ffcd0d7a867c 100644
> >> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> >> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> >> @@ -359,8 +359,13 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
> >>
> >>         if (p->is_volatile)
> >>                 p->region = nvdimm_volatile_region_create(p->bus, &ndr_desc);
> >> -       else
> >> +       else {
> >> +               /*
> >> +                * We need to flush things correctly to guarantee persistance
> >> +                */
> >
> > There are never guarantees. If you're going to comment what does
> > software need to flush, and how?
>
> Can you explain why you say there are never guarantees? If you follow the platform
> recommended instruction sequence to flush data, we can be sure of data
> persistence in the pmem media.

Because storage can always fail. You can reduce risk, but never
eliminate it. This is similar to SSDs that use latent capacitance to
flush their write caches on driver power loss. Even if the application
successfully flushes its writes to buffers that are protected by that
capacitance that power source can still (and in practice does) fail.

>
>
> >
> >> +               set_bit(ND_REGION_PERSIST_MEMCTRL, &ndr_desc.flags);
> >>                 p->region = nvdimm_pmem_region_create(p->bus, &ndr_desc);
> >> +       }
> >>         if (!p->region) {
> >>                 dev_err(dev, "Error registering region %pR from %pOF\n",
> >>                                 ndr_desc.res, p->dn);
> >> diff --git a/drivers/nvdimm/of_pmem.c b/drivers/nvdimm/of_pmem.c
> >> index 8224d1431ea9..6826a274a1f1 100644
> >> --- a/drivers/nvdimm/of_pmem.c
> >> +++ b/drivers/nvdimm/of_pmem.c
> >> @@ -62,8 +62,10 @@ static int of_pmem_region_probe(struct platform_device *pdev)
> >>
> >>                 if (is_volatile)
> >>                         region = nvdimm_volatile_region_create(bus, &ndr_desc);
> >> -               else
> >> +               else {
> >> +                       set_bit(ND_REGION_PERSIST_MEMCTRL, &ndr_desc.flags);
> >>                         region = nvdimm_pmem_region_create(bus, &ndr_desc);
> >> +               }
> >>
> >>                 if (!region)
> >>                         dev_warn(&pdev->dev, "Unable to register region %pR from %pOF\n",
> >> diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
> >> index 0f366706b0aa..771d888a5ed7 100644
> >> --- a/include/linux/libnvdimm.h
> >> +++ b/include/linux/libnvdimm.h
> >> @@ -54,7 +54,6 @@ enum {
> >>         /*
> >>          * Platform provides mechanisms to automatically flush outstanding
> >>          * write data from memory controler to pmem on system power loss.
> >> -        * (ADR)
> >
> > I'd rather not delete critical terminology for a developer / platform
> > owner to be able to consult documentation, or their vendor. Can you
> > instead add the PowerPC equivalent term for this capability? I.e. list
> > (x86: ADR PowerPC: foo ...).
>
> Power ISA doesn't clearly call out what mechanism will be used to ensure
> that a load following power loss will return the previously flushed
> data. Hence there is no description of details like Asynchronous DRAM
> Refresh. Only details specified is with respect to flush sequence that ensures
> that a load following power loss will return the value stored.

What is this "flush sequence"?

^ permalink raw reply

* Re: [PATCH] powerpc/vdso32: mark __kernel_datapage_offset as STV_PROTECTED
From: Fāng-ruì Sòng @ 2020-02-10 18:41 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: linux-kernel, clang-built-linux, Paul Mackerras, linuxppc-dev
In-Reply-To: <87pnemzoxa.fsf@mpe.ellerman.id.au>

On Mon, Feb 10, 2020 at 3:01 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Fangrui Song <maskray@google.com> writes:
> > A PC-relative relocation (R_PPC_REL16_LO in this case) referencing a
> > preemptible symbol in a -shared link is not allowed.  GNU ld's powerpc
> > port is permissive and allows it [1], but lld will report an error after
> > https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=ec0895f08f99515194e9fcfe1338becf6f759d38
> >
> > Make the symbol protected so that it is non-preemptible but still
> > exported.
>
> "preemptible" means something different to me, and I assume we're not
> using it to mean the same thing.
>
> Can you explain it using small words that a kernel developer can
> understand? :)
>
> cheers

The term used in the ELF specification is "preemptable". I heard from
Roland McGrathr that "preemptable" was a typo. The correct term is
"preemptible".
On a random article I found, it mentions that "preemptible" is used
more than "preemptable". So now I stick with "preemptible".

The word is overloaded and has a different meaning in the kernel, but
here we refer to within the ELF binary format context.

From http://www.sco.com/developers/gabi/latest/ch4.symtab.html
"The visibility of symbols with the STV_DEFAULT attribute is as
specified by the symbol's binding type. That is, global and weak
symbols are visible outside of their defining component (executable
file or shared object). Local symbols are hidden, as described below.
Global and weak symbols are also preemptable, that is, they may by
preempted by definitions of the same name in another component."

__kernel_datapage_offset is a STB_GLOBAL STV_DEFAULT symbol. In a
-shared link, it is considered preemptible. There are some methods
that make such symbols non-preemptible but none is used in this
context.

* -Bsymbolic
* -Bsymbolic-functions if STT_FUNC
* --dynamic-list is specified but the dynamic list does not name this symbol
* A --version-script makes the symbol local

__kernel_datapage_offset is accessed via some mechanism similar to
dlsym, so it has to be exported.

Given all the above, I chose STV_PROTECTED, which is the simplest and
least intrusive approach.

> > [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=25500
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/851
> > Signed-off-by: Fangrui Song <maskray@google.com>
>
> > ---
> >  arch/powerpc/kernel/vdso32/datapage.S | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/kernel/vdso32/datapage.S b/arch/powerpc/kernel/vdso32/datapage.S
> > index 217bb630f8f9..2831a8676365 100644
> > --- a/arch/powerpc/kernel/vdso32/datapage.S
> > +++ b/arch/powerpc/kernel/vdso32/datapage.S
> > @@ -13,7 +13,8 @@
> >  #include <asm/vdso_datapage.h>
> >
> >       .text
> > -     .global __kernel_datapage_offset;
> > +     .global __kernel_datapage_offset
> > +     .protected      __kernel_datapage_offset
> >  __kernel_datapage_offset:
> >       .long   0
> >
> > --
> > 2.25.0.341.g760bfbb309-goog



-- 
宋方睿

^ permalink raw reply

* Re: linux-next: DTS build warnings
From: Stephen Rothwell @ 2020-02-11  0:19 UTC (permalink / raw)
  To: ppc-dev
In-Reply-To: <20190731153936.02083439@canb.auug.org.au>

[-- Attachment #1: Type: text/plain, Size: 19041 bytes --]

Hi all,

On Wed, 31 Jul 2019 15:39:36 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> I have been getting the following warnings from a couple of powerpc
> builds for quite a while now.  I was hoping someone might have time to
> look at them and maybe even fix them up :-)

Today's list (from an allyesconfig build) looks like this:

arch/powerpc/boot/dts/icon.dts:318.26-357.5: Warning (pci_bridge): /plb/pciex@d00000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/icon.dts:359.26-398.5: Warning (pci_bridge): /plb/pciex@d20000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/icon.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/virtex440-ml510.dts:335.37-439.6: Warning (pci_bridge): /plb@0/plbv46-pci@85e00000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/virtex440-ml510.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/o2mnt.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
  also defined at arch/powerpc/boot/dts/o2d.dtsi:32.12-43.5
arch/powerpc/boot/dts/o2mnt.dtb: Warning (spi_bus_reg): Failed prerequisite 'spi_bus_bridge'
arch/powerpc/boot/dts/makalu.dts:271.25-310.5: Warning (pci_bridge): /plb/pciex@a0000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/makalu.dts:312.25-351.5: Warning (pci_bridge): /plb/pciex@c0000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/makalu.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/mgcoge.dts:230.14-234.7: Warning (spi_bus_reg): /soc@f0000000/cpm@119c0/spi@11aa0/ds3106@1: SPI bus unit address format error, expected "0"
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/motionpro.dts:84.15-86.4
arch/powerpc/boot/dts/motionpro.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/glacier.dts:492.26-532.5: Warning (pci_bridge): /plb/pciex@d00000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/glacier.dts:534.26-574.5: Warning (pci_bridge): /plb/pciex@d20000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/glacier.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/fsl/p2020rdb.dts:251.22-254.4: Warning (pci_bridge): /pcie@ffe08000: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/fsl/p2020si-post.dtsi:43.7-68.3
arch/powerpc/boot/dts/fsl/p2020si-post.dtsi:52.9-67.4: Warning (pci_bridge): /pcie@ffe08000/pcie@0: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/fsl/p2020rdb.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/fsl/mvme7100.dts:135.22-137.4: Warning (pci_bridge): /pcie@f1008000: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi:92.7-117.3
arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi:102.9-116.4: Warning (pci_bridge): /pcie@f1008000/pcie@0: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/fsl/mvme7100.dts:139.22-141.4: Warning (pci_bridge): /pcie@f1009000: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi:119.7-144.3
arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi:129.9-143.4: Warning (pci_bridge): /pcie@f1009000/pcie@0: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/fsl/mvme7100.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/fsl/mvme7100.dts:30.11-32.6: Warning (i2c_bus_reg): /soc@f1000000/i2c@3000/rtc@68: missing or empty reg property
arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi:38.2-25: Warning (interrupts_property): /soc@fffe00000/mdio@24000/ethernet-phy@0:#interrupt-cells: size is (8), expected multiple of 16
arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi:38.2-25: Warning (interrupts_property): /soc@fffe00000/mdio@24000/ethernet-phy@1:#interrupt-cells: size is (8), expected multiple of 16
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/uc101.dts:100.15-102.4
arch/powerpc/boot/dts/uc101.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi:38.2-25: Warning (interrupts_property): /soc@fffe00000/mdio@24000/ethernet-phy@0:#interrupt-cells: size is (8), expected multiple of 16
arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi:38.2-25: Warning (interrupts_property): /soc@fffe00000/mdio@24000/ethernet-phy@1:#interrupt-cells: size is (8), expected multiple of 16
arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi:38.2-25: Warning (interrupts_property): /soc@ffe00000/mdio@24000/ethernet-phy@0:#interrupt-cells: size is (8), expected multiple of 16
arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi:38.2-25: Warning (interrupts_property): /soc@ffe00000/mdio@24000/ethernet-phy@1:#interrupt-cells: size is (8), expected multiple of 16
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/a3m071.dts:135.15-137.4
arch/powerpc/boot/dts/a3m071.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/o2d.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
  also defined at arch/powerpc/boot/dts/o2d.dtsi:32.12-43.5
arch/powerpc/boot/dts/o2d.dtb: Warning (spi_bus_reg): Failed prerequisite 'spi_bus_bridge'
arch/powerpc/boot/dts/fsl/p1020rdb-pd.dts:189.11-193.6: Warning (spi_bus_reg): /soc@ffe00000/spi@7000/slic@0: SPI bus unit address format error, expected "1"
arch/powerpc/boot/dts/fsl/p1020rdb-pd.dts:195.11-199.6: Warning (spi_bus_reg): /soc@ffe00000/spi@7000/slic@1: SPI bus unit address format error, expected "2"
arch/powerpc/boot/dts/mpc5121.dtsi:397.13-406.5: Warning (spi_bus_bridge): /soc@80000000/psc@11400: node name for SPI buses should be 'spi'
  also defined at arch/powerpc/boot/dts/ac14xx.dts:305.19-326.5
arch/powerpc/boot/dts/mpc5121.dtsi:409.13-418.5: Warning (spi_bus_bridge): /soc@80000000/psc@11500: node name for SPI buses should be 'spi'
  also defined at arch/powerpc/boot/dts/ac14xx.dts:329.19-344.5
arch/powerpc/boot/dts/ac14xx.dtb: Warning (spi_bus_reg): Failed prerequisite 'spi_bus_bridge'
arch/powerpc/boot/dts/fsl/mpc8555cds.dts:330.3-21: Warning (pci_device_bus_num): /pci@e0008000/i8259@19000:bus-range: PCI bus number 1 out of range, expected (0 - 0)
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/cm5200.dts:70.15-72.4
arch/powerpc/boot/dts/cm5200.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/mpc5121.dtsi:457.13-466.5: Warning (spi_bus_bridge): /soc@80000000/psc@11900: node name for SPI buses should be 'spi'
  also defined at arch/powerpc/boot/dts/pdm360ng.dts:172.13-185.5
arch/powerpc/boot/dts/pdm360ng.dtb: Warning (spi_bus_reg): Failed prerequisite 'spi_bus_bridge'
arch/powerpc/boot/dts/haleakala.dts:240.25-279.5: Warning (pci_bridge): /plb/pciex@a0000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/haleakala.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi:38.2-25: Warning (interrupts_property): /soc@ffe00000/mdio@24000/ethernet-phy@0:#interrupt-cells: size is (8), expected multiple of 16
arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi:38.2-25: Warning (interrupts_property): /soc@ffe00000/mdio@24000/ethernet-phy@1:#interrupt-cells: size is (8), expected multiple of 16
arch/powerpc/boot/dts/katmai.dts:322.26-361.5: Warning (pci_bridge): /plb/pciex@d00000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/katmai.dts:363.26-402.5: Warning (pci_bridge): /plb/pciex@d20000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/katmai.dts:404.26-443.5: Warning (pci_bridge): /plb/pciex@d40000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/katmai.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/bluestone.dts:328.26-368.5: Warning (pci_bridge): /plb/pciex@d00000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/bluestone.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/bluestone.dts:272.13-277.7: Warning (i2c_bus_reg): /plb/opb/i2c@ef600700/sttm@4C: I2C bus unit address format error, expected "4c"
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/o2i.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
  also defined at arch/powerpc/boot/dts/o2d.dtsi:32.12-43.5
arch/powerpc/boot/dts/o2i.dtb: Warning (spi_bus_reg): Failed prerequisite 'spi_bus_bridge'
arch/powerpc/boot/dts/fsl/gef_sbc610.dts:209.22-211.4: Warning (pci_bridge): /pcie@fef09000: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi:119.7-144.3
arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi:129.9-143.4: Warning (pci_bridge): /pcie@fef09000/pcie@0: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/fsl/gef_sbc610.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/currituck.dts:125.28-161.5: Warning (pci_bridge): /plb/pciex@10100000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/currituck.dts:163.28-198.5: Warning (pci_bridge): /plb/pciex@30100000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/currituck.dts:200.28-235.5: Warning (pci_bridge): /plb/pciex@38100000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/currituck.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/xpedite5200.dts:135.11-138.6: Warning (i2c_bus_reg): /soc@ef000000/i2c@3000/dtt@48: I2C bus unit address format error, expected "34"
arch/powerpc/boot/dts/mpc8610_hpcd.dts:420.3-21: Warning (pci_device_bus_num): /pcie@e000a000/pcie@0:bus-range: PCI bus number 0 out of range, expected (1 - 1)
arch/powerpc/boot/dts/akebono.dts:251.28-289.5: Warning (pci_bridge): /plb/pciex@10100000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/akebono.dts:291.28-329.5: Warning (pci_bridge): /plb/pciex@20100000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/akebono.dts:331.28-369.5: Warning (pci_bridge): /plb/pciex@18100000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/akebono.dts:371.28-409.5: Warning (pci_bridge): /plb/pciex@28100000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/akebono.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/canyonlands.dts:464.26-504.5: Warning (pci_bridge): /plb/pciex@d00000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/canyonlands.dts:506.26-546.5: Warning (pci_bridge): /plb/pciex@d20000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/canyonlands.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/stxssa8555.dts:329.3-21: Warning (pci_device_bus_num): /pci@e0008000/i8259@19000:bus-range: PCI bus number 1 out of range, expected (0 - 0)
arch/powerpc/boot/dts/fsl/gef_ppc9a.dts:211.22-213.4: Warning (pci_bridge): /pcie@fef09000: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi:119.7-144.3
arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi:129.9-143.4: Warning (pci_bridge): /pcie@fef09000/pcie@0: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/fsl/gef_ppc9a.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/o2dnt2.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
  also defined at arch/powerpc/boot/dts/o2d.dtsi:32.12-43.5
arch/powerpc/boot/dts/o2dnt2.dtb: Warning (spi_bus_reg): Failed prerequisite 'spi_bus_bridge'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/o3dnt.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
  also defined at arch/powerpc/boot/dts/o2d.dtsi:32.12-43.5
arch/powerpc/boot/dts/o3dnt.dtb: Warning (spi_bus_reg): Failed prerequisite 'spi_bus_bridge'
arch/powerpc/boot/dts/redwood.dts:238.26-277.5: Warning (pci_bridge): /plb/pciex@d00000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/redwood.dts:279.26-318.5: Warning (pci_bridge): /plb/pciex@d20000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/redwood.dts:320.26-359.5: Warning (pci_bridge): /plb/pciex@d40000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/redwood.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/xpedite5200_xmon.dts:139.11-142.6: Warning (i2c_bus_reg): /soc@ef000000/i2c@3000/dtt@48: I2C bus unit address format error, expected "34"
arch/powerpc/boot/dts/fsl/oca4080.dts:132.23-134.4: Warning (pci_bridge): /pcie@ffe200000: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/fsl/p4080si-post.dtsi:58.7-84.3
arch/powerpc/boot/dts/fsl/p4080si-post.dtsi:68.9-83.4: Warning (pci_bridge): /pcie@ffe200000/pcie@0: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/fsl/oca4080.dts:136.23-138.4: Warning (pci_bridge): /pcie@ffe201000: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/fsl/p4080si-post.dtsi:87.7-113.3
arch/powerpc/boot/dts/fsl/p4080si-post.dtsi:97.9-112.4: Warning (pci_bridge): /pcie@ffe201000/pcie@0: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/fsl/oca4080.dts:140.23-142.4: Warning (pci_bridge): /pcie@ffe202000: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/fsl/p4080si-post.dtsi:116.7-142.3
arch/powerpc/boot/dts/fsl/p4080si-post.dtsi:126.9-141.4: Warning (pci_bridge): /pcie@ffe202000/pcie@0: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/fsl/oca4080.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/fsl/ppa8548.dts:34.22-37.4: Warning (pci_bridge): /pci@fe0008000: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/fsl/mpc8548si-post.dtsi:43.7-51.3
arch/powerpc/boot/dts/fsl/ppa8548.dts:39.22-42.4: Warning (pci_bridge): /pci@fe0009000: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/fsl/mpc8548si-post.dtsi:54.7-62.3
arch/powerpc/boot/dts/fsl/ppa8548.dts:44.23-47.4: Warning (pci_bridge): /pcie@fe000a000: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/fsl/mpc8548si-post.dtsi:65.7-90.3
arch/powerpc/boot/dts/fsl/mpc8548si-post.dtsi:74.9-89.4: Warning (pci_bridge): /pcie@fe000a000/pcie@0: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/fsl/ppa8548.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/o2d300.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
  also defined at arch/powerpc/boot/dts/o2d.dtsi:32.12-43.5
arch/powerpc/boot/dts/o2d300.dtb: Warning (spi_bus_reg): Failed prerequisite 'spi_bus_bridge'
arch/powerpc/boot/dts/kilauea.dts:325.25-364.5: Warning (pci_bridge): /plb/pciex@a0000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/kilauea.dts:366.25-405.5: Warning (pci_bridge): /plb/pciex@c0000000: node name is not "pci" or "pcie"
arch/powerpc/boot/dts/kilauea.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/fsl/p2020rdb-pc_36b.dts:59.23-62.4: Warning (pci_bridge): /pcie@fffe08000: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/fsl/p2020si-post.dtsi:100.7-126.3
arch/powerpc/boot/dts/fsl/p2020si-post.dtsi:109.9-125.4: Warning (pci_bridge): /pcie@fffe08000/pcie@0: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/fsl/p2020rdb-pc_36b.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/fsl/p2020rdb-pc_32b.dts:59.22-62.4: Warning (pci_bridge): /pcie@ffe08000: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/fsl/p2020si-post.dtsi:100.7-126.3
arch/powerpc/boot/dts/fsl/p2020si-post.dtsi:109.9-125.4: Warning (pci_bridge): /pcie@ffe08000/pcie@0: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/fsl/p2020rdb-pc_32b.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
arch/powerpc/boot/dts/fsl/mpc8541cds.dts:330.3-21: Warning (pci_device_bus_num): /pci@e0008000/i8259@19000:bus-range: PCI bus number 1 out of range, expected (0 - 0)
arch/powerpc/boot/dts/fsl/kmcoge4.dts:196.23-198.4: Warning (pci_bridge): /pcie@ffe201000: missing ranges for PCI bridge (or not a bridge)
  also defined at arch/powerpc/boot/dts/fsl/p2041si-post.dtsi:87.7-113.3
arch/powerpc/boot/dts/fsl/p2041si-post.dtsi:97.9-112.4: Warning (pci_bridge): /pcie@ffe201000/pcie@0: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/fsl/kmcoge4.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] selftests: vm: Do not override definition of ARCH
From: Michael Ellerman @ 2020-02-11  2:49 UTC (permalink / raw)
  To: Sandipan Das, shuah, skhan, linux-kselftest
  Cc: linux-mm, kamalesh, linuxppc-dev, mhiramat, aneesh.kumar
In-Reply-To: <471a73600bf9f59a59484726f50f81d7f54f4b2f.1580367152.git.sandipan@linux.ibm.com>

Sandipan Das <sandipan@linux.ibm.com> writes:
> Independent builds of the vm selftests is currently broken
> because commit 7549b3364201 overrides the value of ARCH with
> the machine name from uname. This does not always match the
> architecture names used for tasks like header installation.
>
> E.g. for building tests on powerpc64, we need ARCH=powerpc
> and not ARCH=ppc64 or ARCH=ppc64le. Otherwise, the build
> fails as shown below.
>
>   $ uname -m
>   ppc64le
>
>   $ make -C tools/testing/selftests/vm
>   make: Entering directory '/home/sandipan/linux/tools/testing/selftests/vm'
>   make --no-builtin-rules ARCH=ppc64le -C ../../../.. headers_install
>   make[1]: Entering directory '/home/sandipan/linux'
>   Makefile:653: arch/ppc64le/Makefile: No such file or directory
>   make[1]: *** No rule to make target 'arch/ppc64le/Makefile'.  Stop.
>   make[1]: Leaving directory '/home/sandipan/linux'
>   ../lib.mk:50: recipe for target 'khdr' failed
>   make: *** [khdr] Error 2
>   make: Leaving directory '/home/sandipan/linux/tools/testing/selftests/vm'
>
> Fixes: 7549b3364201 ("selftests: vm: Build/Run 64bit tests only on 64bit arch")
> Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
> ---
>  tools/testing/selftests/vm/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Tested-by: Michael Ellerman <mpe@ellerman.id.au>

cheers

> diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
> index 7f9a8a8c31da..3f2e2f0ccbc9 100644
> --- a/tools/testing/selftests/vm/Makefile
> +++ b/tools/testing/selftests/vm/Makefile
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # Makefile for vm selftests
>  uname_M := $(shell uname -m 2>/dev/null || echo not)
> -ARCH ?= $(shell echo $(uname_M) | sed -e 's/aarch64.*/arm64/')
> +MACHINE ?= $(shell echo $(uname_M) | sed -e 's/aarch64.*/arm64/')
>  
>  CFLAGS = -Wall -I ../../../../usr/include $(EXTRA_CFLAGS)
>  LDLIBS = -lrt
> @@ -19,7 +19,7 @@ TEST_GEN_FILES += thuge-gen
>  TEST_GEN_FILES += transhuge-stress
>  TEST_GEN_FILES += userfaultfd
>  
> -ifneq (,$(filter $(ARCH),arm64 ia64 mips64 parisc64 ppc64 riscv64 s390x sh64 sparc64 x86_64))
> +ifneq (,$(filter $(MACHINE),arm64 ia64 mips64 parisc64 ppc64 riscv64 s390x sh64 sparc64 x86_64))
>  TEST_GEN_FILES += va_128TBswitch
>  TEST_GEN_FILES += virtual_address_range
>  endif
> -- 
> 2.17.1

^ permalink raw reply

* Re: [PATCH 2/2] selftests: vm: Fix 64-bit test builds for powerpc64le
From: Michael Ellerman @ 2020-02-11  2:50 UTC (permalink / raw)
  To: Sandipan Das, shuah, skhan, linux-kselftest
  Cc: linux-mm, kamalesh, linuxppc-dev, mhiramat, aneesh.kumar
In-Reply-To: <e9178d28dddfc64a23ffd51e2fecd06ad16ac92a.1580367152.git.sandipan@linux.ibm.com>

Sandipan Das <sandipan@linux.ibm.com> writes:
> Some tests are built only for 64-bit systems. This makes
> sure that these tests are built for both big and little
> endian variants of powerpc64.
>
> Fixes: 7549b3364201 ("selftests: vm: Build/Run 64bit tests only on 64bit arch")
> Reviewed-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
> Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
> ---
>  tools/testing/selftests/vm/Makefile    | 2 +-
>  tools/testing/selftests/vm/run_vmtests | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Tested-by: Michael Ellerman <mpe@ellerman.id.au>

cheers

> diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
> index 3f2e2f0ccbc9..8074340c6b3a 100644
> --- a/tools/testing/selftests/vm/Makefile
> +++ b/tools/testing/selftests/vm/Makefile
> @@ -19,7 +19,7 @@ TEST_GEN_FILES += thuge-gen
>  TEST_GEN_FILES += transhuge-stress
>  TEST_GEN_FILES += userfaultfd
>  
> -ifneq (,$(filter $(MACHINE),arm64 ia64 mips64 parisc64 ppc64 riscv64 s390x sh64 sparc64 x86_64))
> +ifneq (,$(filter $(MACHINE),arm64 ia64 mips64 parisc64 ppc64 ppc64le riscv64 s390x sh64 sparc64 x86_64))
>  TEST_GEN_FILES += va_128TBswitch
>  TEST_GEN_FILES += virtual_address_range
>  endif
> diff --git a/tools/testing/selftests/vm/run_vmtests b/tools/testing/selftests/vm/run_vmtests
> index a692ea828317..db8e0d1c7b39 100755
> --- a/tools/testing/selftests/vm/run_vmtests
> +++ b/tools/testing/selftests/vm/run_vmtests
> @@ -59,7 +59,7 @@ else
>  fi
>  
>  #filter 64bit architectures
> -ARCH64STR="arm64 ia64 mips64 parisc64 ppc64 riscv64 s390x sh64 sparc64 x86_64"
> +ARCH64STR="arm64 ia64 mips64 parisc64 ppc64 ppc64le riscv64 s390x sh64 sparc64 x86_64"
>  if [ -z $ARCH ]; then
>    ARCH=`uname -m 2>/dev/null | sed -e 's/aarch64.*/arm64/'`
>  fi
> -- 
> 2.17.1

^ permalink raw reply

* [PATCH 1/2] powerpc/pseries: group lmb operation and memblock's
From: Pingfan Liu @ 2020-02-11  2:59 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: kexec, Pingfan Liu, Paul Mackerras, Hari Bathini

This patch prepares for the incoming patch which swaps the order of KOBJ_
uevent and dt's updating.

It has no functional effect, just groups lmb operation and memblock's in
order to insert dt updating operation easily, and makes it easier to
review.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hari Bathini <hbathini@linux.ibm.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: kexec@lists.infradead.org
---
 arch/powerpc/platforms/pseries/hotplug-memory.c | 26 ++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index c126b94..a3a9353 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -375,7 +375,8 @@ static int dlpar_add_lmb(struct drmem_lmb *);
 static int dlpar_remove_lmb(struct drmem_lmb *lmb)
 {
 	unsigned long block_sz;
-	int rc;
+	phys_addr_t base_addr;
+	int rc, nid;
 
 	if (!lmb_is_removable(lmb))
 		return -EINVAL;
@@ -384,17 +385,19 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
 	if (rc)
 		return rc;
 
+	base_addr = lmb->base_addr;
+	nid = lmb->nid;
 	block_sz = pseries_memory_block_size();
 
-	__remove_memory(lmb->nid, lmb->base_addr, block_sz);
-
-	/* Update memory regions for memory remove */
-	memblock_remove(lmb->base_addr, block_sz);
-
 	invalidate_lmb_associativity_index(lmb);
 	lmb_clear_nid(lmb);
 	lmb->flags &= ~DRCONF_MEM_ASSIGNED;
 
+	__remove_memory(nid, base_addr, block_sz);
+
+	/* Update memory regions for memory remove */
+	memblock_remove(base_addr, block_sz);
+
 	return 0;
 }
 
@@ -661,6 +664,8 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
 	}
 
 	lmb_set_nid(lmb);
+	lmb->flags |= DRCONF_MEM_ASSIGNED;
+
 	block_sz = memory_block_size_bytes();
 
 	/* Add the memory */
@@ -672,11 +677,14 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
 
 	rc = dlpar_online_lmb(lmb);
 	if (rc) {
-		__remove_memory(lmb->nid, lmb->base_addr, block_sz);
+		int nid = lmb->nid;
+		phys_addr_t base_addr = lmb->base_addr;
+
 		invalidate_lmb_associativity_index(lmb);
 		lmb_clear_nid(lmb);
-	} else {
-		lmb->flags |= DRCONF_MEM_ASSIGNED;
+		lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+
+		__remove_memory(nid, base_addr, block_sz);
 	}
 
 	return rc;
-- 
2.7.5


^ permalink raw reply related

* [PATCH 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
From: Pingfan Liu @ 2020-02-11  2:59 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: kexec, Pingfan Liu, Paul Mackerras, Hari Bathini
In-Reply-To: <1581389982-5701-1-git-send-email-kernelfans@gmail.com>

A bug is observed on pseries by taking the following steps on rhel:
-1. drmgr -c mem -r -q 5
-2. echo c > /proc/sysrq-trigger

And then, the failure looks like:
kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/
kdump: saving vmcore-dmesg.txt
kdump: saving vmcore-dmesg.txt complete
kdump: saving vmcore
 Checking for memory holes                         : [  0.0 %] /                   Checking for memory holes                         : [100.0 %] |                   Excluding unnecessary pages                       : [100.0 %] \                   Copying data                                      : [  0.3 %] -          eta: 38s[   44.337636] hash-mmu: mm: Hashing failure ! EA=0x7fffba400000 access=0x8000000000000004 current=makedumpfile
[   44.337663] hash-mmu:     trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc000000050000504
[   44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba400000 access=0x8000000000000004 current=makedumpfile
[   44.337692] hash-mmu:     trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc000000050000504
[   44.337708] makedumpfile[469]: unhandled signal 7 at 00007fffba400000 nip 00007fffbbc4d7fc lr 000000011356ca3c code 2
[   44.338548] Core dump to |/bin/false pipe failed
/lib/kdump-lib-initramfs.sh: line 98:   469 Bus error               $CORE_COLLECTOR /proc/vmcore $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete
kdump: saving vmcore failed

* Root cause *
  After analyzing, it turns out that in the current implementation,
when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt updating as
the code __remove_memory() comes before drmem_update_dt().

From a viewpoint of listener and publisher, the publisher notifies the
listener before data is ready.  This introduces a problem where udev
launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before
updating. And in capture kernel, makedumpfile will access the memory based
on the stale dt info, and hit a SIGBUS error due to an un-existed lmb.

* Fix *
  In order to fix this issue, update dt before __remove_memory(), and
accordingly the same rule in hot-add path.

This will introduce extra dt updating payload for each involved lmb when hotplug.
But it should be fine since drmem_update_dt() is memory based operation and
hotplug is not a hot path.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hari Bathini <hbathini@linux.ibm.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: kexec@lists.infradead.org
---
 arch/powerpc/platforms/pseries/hotplug-memory.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index a3a9353..1f623c3 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -392,6 +392,9 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
 	invalidate_lmb_associativity_index(lmb);
 	lmb_clear_nid(lmb);
 	lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+	rtas_hp_event = true;
+	drmem_update_dt();
+	rtas_hp_event = false;
 
 	__remove_memory(nid, base_addr, block_sz);
 
@@ -665,6 +668,9 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
 
 	lmb_set_nid(lmb);
 	lmb->flags |= DRCONF_MEM_ASSIGNED;
+	rtas_hp_event = true;
+	drmem_update_dt();
+	rtas_hp_event = false;
 
 	block_sz = memory_block_size_bytes();
 
@@ -683,6 +689,9 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
 		invalidate_lmb_associativity_index(lmb);
 		lmb_clear_nid(lmb);
 		lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+		rtas_hp_event = true;
+		drmem_update_dt();
+		rtas_hp_event = false;
 
 		__remove_memory(nid, base_addr, block_sz);
 	}
@@ -939,12 +948,6 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
 		break;
 	}
 
-	if (!rc) {
-		rtas_hp_event = true;
-		rc = drmem_update_dt();
-		rtas_hp_event = false;
-	}
-
 	unlock_device_hotplug();
 	return rc;
 }
-- 
2.7.5


^ permalink raw reply related

* [PATCH v3 1/3] powerpc/tm: Fix clearing MSR[TS] in current when reclaiming on signal delivery
From: Gustavo Luiz Duarte @ 2020-02-11  3:38 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mikey, Gustavo Luiz Duarte, stable, gromero

After a treclaim, we expect to be in non-transactional state. If we don't clear
the current thread's MSR[TS] before we get preempted, then
tm_recheckpoint_new_task() will recheckpoint and we get rescheduled in
suspended transaction state.

When handling a signal caught in transactional state, handle_rt_signal64()
calls get_tm_stackpointer() that treclaims the transaction using
tm_reclaim_current() but without clearing the thread's MSR[TS]. This can cause
the TM Bad Thing exception below if later we pagefault and get preempted trying
to access the user's sigframe, using __put_user(). Afterwards, when we are
rescheduled back into do_page_fault() (but now in suspended state since the
thread's MSR[TS] was not cleared), upon executing 'rfid' after completion of
the page fault handling, the exception is raised because a transition from
suspended to non-transactional state is invalid.

	Unexpected TM Bad Thing exception at c00000000000de44 (msr 0x8000000302a03031) tm_scratch=800000010280b033
	Oops: Unrecoverable exception, sig: 6 [#1]
	LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
	Modules linked in: nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip6_tables ip_tables nft_compat ip_set nf_tables nfnetlink xts vmx_crypto sg virtio_balloon
	r_mod cdrom virtio_net net_failover virtio_blk virtio_scsi failover dm_mirror dm_region_hash dm_log dm_mod
	CPU: 25 PID: 15547 Comm: a.out Not tainted 5.4.0-rc2 #32
	NIP:  c00000000000de44 LR: c000000000034728 CTR: 0000000000000000
	REGS: c00000003fe7bd70 TRAP: 0700   Not tainted  (5.4.0-rc2)
	MSR:  8000000302a03031 <SF,VEC,VSX,FP,ME,IR,DR,LE,TM[SE]>  CR: 44000884  XER: 00000000
	CFAR: c00000000000dda4 IRQMASK: 0
	PACATMSCRATCH: 800000010280b033
	GPR00: c000000000034728 c000000f65a17c80 c000000001662800 00007fffacf3fd78
	GPR04: 0000000000001000 0000000000001000 0000000000000000 c000000f611f8af0
	GPR08: 0000000000000000 0000000078006001 0000000000000000 000c000000000000
	GPR12: c000000f611f84b0 c00000003ffcb200 0000000000000000 0000000000000000
	GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
	GPR20: 0000000000000000 0000000000000000 0000000000000000 c000000f611f8140
	GPR24: 0000000000000000 00007fffacf3fd68 c000000f65a17d90 c000000f611f7800
	GPR28: c000000f65a17e90 c000000f65a17e90 c000000001685e18 00007fffacf3f000
	NIP [c00000000000de44] fast_exception_return+0xf4/0x1b0
	LR [c000000000034728] handle_rt_signal64+0x78/0xc50
	Call Trace:
	[c000000f65a17c80] [c000000000034710] handle_rt_signal64+0x60/0xc50 (unreliable)
	[c000000f65a17d30] [c000000000023640] do_notify_resume+0x330/0x460
	[c000000f65a17e20] [c00000000000dcc4] ret_from_except_lite+0x70/0x74
	Instruction dump:
	7c4ff120 e8410170 7c5a03a6 38400000 f8410060 e8010070 e8410080 e8610088
	60000000 60000000 e8810090 e8210078 <4c000024> 48000000 e8610178 88ed0989
	---[ end trace 93094aa44b442f87 ]---

The simplified sequence of events that triggers the above exception is:

  ...				# userspace in NON-TRANSACTIONAL state
  tbegin			# userspace in TRANSACTIONAL state
  signal delivery		# kernelspace in SUSPENDED state
  handle_rt_signal64()
    get_tm_stackpointer()
      treclaim			# kernelspace in NON-TRANSACTIONAL state
    __put_user()
      page fault happens. We will never get back here because of the TM Bad Thing exception.

  page fault handling kicks in and we voluntarily preempt ourselves
  do_page_fault()
    __schedule()
      __switch_to(other_task)

  our task is rescheduled and we recheckpoint because the thread's MSR[TS] was not cleared
  __switch_to(our_task)
    switch_to_tm()
      tm_recheckpoint_new_task()
        trechkpt			# kernelspace in SUSPENDED state

  The page fault handling resumes, but now we are in suspended transaction state
  do_page_fault()    completes
  rfid     <----- trying to get back where the page fault happened (we were non-transactional back then)
  TM Bad Thing			# illegal transition from suspended to non-transactional

This patch fixes that issue by clearing the current thread's MSR[TS] just after
treclaim in get_tm_stackpointer() so that we stay in non-transactional state in
case we are preempted. In order to make treclaim and clearing the thread's
MSR[TS] atomic from a preemption perspective when CONFIG_PREEMPT is set,
preempt_disable/enable() is used. It's also necessary to save the previous
value of the thread's MSR before get_tm_stackpointer() is called so that it can
be exposed to the signal handler later in setup_tm_sigcontexts() to inform the
userspace MSR at the moment of the signal delivery.

Found with tm-signal-context-force-tm kernel selftest.

v3: Subject and comment improvements.
v2: Fix build failure when tm is disabled.

Fixes: 2b0a576d15e0 ("powerpc: Add new transactional memory state to the signal context")
Cc: stable@vger.kernel.org # v3.9
Signed-off-by: Gustavo Luiz Duarte <gustavold@linux.ibm.com>
---
 arch/powerpc/kernel/signal.c    | 16 ++++++++++++++--
 arch/powerpc/kernel/signal_32.c | 28 ++++++++++++++--------------
 arch/powerpc/kernel/signal_64.c | 22 ++++++++++------------
 3 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index e6c30cee6abf..76d99bc111dc 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -200,14 +200,26 @@ unsigned long get_tm_stackpointer(struct task_struct *tsk)
 	 * normal/non-checkpointed stack pointer.
 	 */
 
+	unsigned long ret = tsk->thread.regs->gpr[1];
+
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	BUG_ON(tsk != current);
 
 	if (MSR_TM_ACTIVE(tsk->thread.regs->msr)) {
+		preempt_disable();
 		tm_reclaim_current(TM_CAUSE_SIGNAL);
 		if (MSR_TM_TRANSACTIONAL(tsk->thread.regs->msr))
-			return tsk->thread.ckpt_regs.gpr[1];
+			ret = tsk->thread.ckpt_regs.gpr[1];
+
+		/* If we treclaim, we must clear the current thread's TM bits
+		 * before re-enabling preemption. Otherwise we might be
+		 * preempted and have the live MSR[TS] changed behind our back
+		 * (tm_recheckpoint_new_task() would recheckpoint). Besides, we
+		 * enter the signal handler in non-transactional state.
+		 */
+		tsk->thread.regs->msr &= ~MSR_TS_MASK;
+		preempt_enable();
 	}
 #endif
-	return tsk->thread.regs->gpr[1];
+	return ret;
 }
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 98600b276f76..1b090a76b444 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -489,19 +489,11 @@ static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
  */
 static int save_tm_user_regs(struct pt_regs *regs,
 			     struct mcontext __user *frame,
-			     struct mcontext __user *tm_frame, int sigret)
+			     struct mcontext __user *tm_frame, int sigret,
+			     unsigned long msr)
 {
-	unsigned long msr = regs->msr;
-
 	WARN_ON(tm_suspend_disabled);
 
-	/* Remove TM bits from thread's MSR.  The MSR in the sigcontext
-	 * just indicates to userland that we were doing a transaction, but we
-	 * don't want to return in transactional state.  This also ensures
-	 * that flush_fp_to_thread won't set TIF_RESTORE_TM again.
-	 */
-	regs->msr &= ~MSR_TS_MASK;
-
 	/* Save both sets of general registers */
 	if (save_general_regs(&current->thread.ckpt_regs, frame)
 	    || save_general_regs(regs, tm_frame))
@@ -912,6 +904,10 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
 	int sigret;
 	unsigned long tramp;
 	struct pt_regs *regs = tsk->thread.regs;
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	/* Save the thread's msr before get_tm_stackpointer() changes it */
+	unsigned long msr = regs->msr;
+#endif
 
 	BUG_ON(tsk != current);
 
@@ -944,13 +940,13 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	tm_frame = &rt_sf->uc_transact.uc_mcontext;
-	if (MSR_TM_ACTIVE(regs->msr)) {
+	if (MSR_TM_ACTIVE(msr)) {
 		if (__put_user((unsigned long)&rt_sf->uc_transact,
 			       &rt_sf->uc.uc_link) ||
 		    __put_user((unsigned long)tm_frame,
 			       &rt_sf->uc_transact.uc_regs))
 			goto badframe;
-		if (save_tm_user_regs(regs, frame, tm_frame, sigret))
+		if (save_tm_user_regs(regs, frame, tm_frame, sigret, msr))
 			goto badframe;
 	}
 	else
@@ -1369,6 +1365,10 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
 	int sigret;
 	unsigned long tramp;
 	struct pt_regs *regs = tsk->thread.regs;
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	/* Save the thread's msr before get_tm_stackpointer() changes it */
+	unsigned long msr = regs->msr;
+#endif
 
 	BUG_ON(tsk != current);
 
@@ -1402,9 +1402,9 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	tm_mctx = &frame->mctx_transact;
-	if (MSR_TM_ACTIVE(regs->msr)) {
+	if (MSR_TM_ACTIVE(msr)) {
 		if (save_tm_user_regs(regs, &frame->mctx, &frame->mctx_transact,
-				      sigret))
+				      sigret, msr))
 			goto badframe;
 	}
 	else
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 117515564ec7..84ed2e77ef9c 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -192,7 +192,8 @@ static long setup_sigcontext(struct sigcontext __user *sc,
 static long setup_tm_sigcontexts(struct sigcontext __user *sc,
 				 struct sigcontext __user *tm_sc,
 				 struct task_struct *tsk,
-				 int signr, sigset_t *set, unsigned long handler)
+				 int signr, sigset_t *set, unsigned long handler,
+				 unsigned long msr)
 {
 	/* When CONFIG_ALTIVEC is set, we _always_ setup v_regs even if the
 	 * process never used altivec yet (MSR_VEC is zero in pt_regs of
@@ -207,12 +208,11 @@ static long setup_tm_sigcontexts(struct sigcontext __user *sc,
 	elf_vrreg_t __user *tm_v_regs = sigcontext_vmx_regs(tm_sc);
 #endif
 	struct pt_regs *regs = tsk->thread.regs;
-	unsigned long msr = tsk->thread.regs->msr;
 	long err = 0;
 
 	BUG_ON(tsk != current);
 
-	BUG_ON(!MSR_TM_ACTIVE(regs->msr));
+	BUG_ON(!MSR_TM_ACTIVE(msr));
 
 	WARN_ON(tm_suspend_disabled);
 
@@ -222,13 +222,6 @@ static long setup_tm_sigcontexts(struct sigcontext __user *sc,
 	 */
 	msr |= tsk->thread.ckpt_regs.msr & (MSR_FP | MSR_VEC | MSR_VSX);
 
-	/* Remove TM bits from thread's MSR.  The MSR in the sigcontext
-	 * just indicates to userland that we were doing a transaction, but we
-	 * don't want to return in transactional state.  This also ensures
-	 * that flush_fp_to_thread won't set TIF_RESTORE_TM again.
-	 */
-	regs->msr &= ~MSR_TS_MASK;
-
 #ifdef CONFIG_ALTIVEC
 	err |= __put_user(v_regs, &sc->v_regs);
 	err |= __put_user(tm_v_regs, &tm_sc->v_regs);
@@ -824,6 +817,10 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 	unsigned long newsp = 0;
 	long err = 0;
 	struct pt_regs *regs = tsk->thread.regs;
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	/* Save the thread's msr before get_tm_stackpointer() changes it */
+	unsigned long msr = regs->msr;
+#endif
 
 	BUG_ON(tsk != current);
 
@@ -841,7 +838,7 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 	err |= __put_user(0, &frame->uc.uc_flags);
 	err |= __save_altstack(&frame->uc.uc_stack, regs->gpr[1]);
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-	if (MSR_TM_ACTIVE(regs->msr)) {
+	if (MSR_TM_ACTIVE(msr)) {
 		/* The ucontext_t passed to userland points to the second
 		 * ucontext_t (for transactional state) with its uc_link ptr.
 		 */
@@ -849,7 +846,8 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 		err |= setup_tm_sigcontexts(&frame->uc.uc_mcontext,
 					    &frame->uc_transact.uc_mcontext,
 					    tsk, ksig->sig, NULL,
-					    (unsigned long)ksig->ka.sa.sa_handler);
+					    (unsigned long)ksig->ka.sa.sa_handler,
+					    msr);
 	} else
 #endif
 	{
-- 
2.21.1


^ permalink raw reply related

* [PATCH v3 2/3] selftests/powerpc: Add tm-signal-pagefault test
From: Gustavo Luiz Duarte @ 2020-02-11  3:38 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mikey, Gustavo Luiz Duarte, gromero
In-Reply-To: <20200211033831.11165-1-gustavold@linux.ibm.com>

This test triggers a TM Bad Thing by raising a signal in transactional state
and forcing a pagefault to happen in kernelspace when the kernel signal
handling code first touches the user signal stack.

This is inspired by the test tm-signal-context-force-tm but uses userfaultfd to
make the test deterministic. While this test always triggers the bug in one
run, I had to execute tm-signal-context-force-tm several times (the test runs
5000 times each execution) to trigger the same bug.

tm-signal-context-force-tm is kept instead of replaced because, while this test
is more reliable and triggers the same bug, tm-signal-context-force-tm has a
better coverage, in the sense that by running the test several times it might
trigger the pagefault and/or be preempted at different places.

v3: skip test if userfaultfd is unavailable.

Signed-off-by: Gustavo Luiz Duarte <gustavold@linux.ibm.com>
---
 tools/testing/selftests/powerpc/tm/.gitignore |   1 +
 tools/testing/selftests/powerpc/tm/Makefile   |   3 +-
 .../powerpc/tm/tm-signal-pagefault.c          | 284 ++++++++++++++++++
 3 files changed, 287 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/powerpc/tm/tm-signal-pagefault.c

diff --git a/tools/testing/selftests/powerpc/tm/.gitignore b/tools/testing/selftests/powerpc/tm/.gitignore
index 98f2708d86cc..e1c72a4a3e91 100644
--- a/tools/testing/selftests/powerpc/tm/.gitignore
+++ b/tools/testing/selftests/powerpc/tm/.gitignore
@@ -13,6 +13,7 @@ tm-signal-context-chk-vmx
 tm-signal-context-chk-vsx
 tm-signal-context-force-tm
 tm-signal-sigreturn-nt
+tm-signal-pagefault
 tm-vmx-unavail
 tm-unavailable
 tm-trap
diff --git a/tools/testing/selftests/powerpc/tm/Makefile b/tools/testing/selftests/powerpc/tm/Makefile
index b15a1a325bd0..b1d99736f8b8 100644
--- a/tools/testing/selftests/powerpc/tm/Makefile
+++ b/tools/testing/selftests/powerpc/tm/Makefile
@@ -5,7 +5,7 @@ SIGNAL_CONTEXT_CHK_TESTS := tm-signal-context-chk-gpr tm-signal-context-chk-fpu
 TEST_GEN_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm-signal-stack \
 	tm-vmxcopy tm-fork tm-tar tm-tmspr tm-vmx-unavail tm-unavailable tm-trap \
 	$(SIGNAL_CONTEXT_CHK_TESTS) tm-sigreturn tm-signal-sigreturn-nt \
-	tm-signal-context-force-tm tm-poison
+	tm-signal-context-force-tm tm-poison tm-signal-pagefault
 
 top_srcdir = ../../../../..
 include ../../lib.mk
@@ -22,6 +22,7 @@ $(OUTPUT)/tm-resched-dscr: ../pmu/lib.c
 $(OUTPUT)/tm-unavailable: CFLAGS += -O0 -pthread -m64 -Wno-error=uninitialized -mvsx
 $(OUTPUT)/tm-trap: CFLAGS += -O0 -pthread -m64
 $(OUTPUT)/tm-signal-context-force-tm: CFLAGS += -pthread -m64
+$(OUTPUT)/tm-signal-pagefault: CFLAGS += -pthread -m64
 
 SIGNAL_CONTEXT_CHK_TESTS := $(patsubst %,$(OUTPUT)/%,$(SIGNAL_CONTEXT_CHK_TESTS))
 $(SIGNAL_CONTEXT_CHK_TESTS): tm-signal.S
diff --git a/tools/testing/selftests/powerpc/tm/tm-signal-pagefault.c b/tools/testing/selftests/powerpc/tm/tm-signal-pagefault.c
new file mode 100644
index 000000000000..5908bc6abe60
--- /dev/null
+++ b/tools/testing/selftests/powerpc/tm/tm-signal-pagefault.c
@@ -0,0 +1,284 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2020, Gustavo Luiz Duarte, IBM Corp.
+ *
+ * This test starts a transaction and triggers a signal, forcing a pagefault to
+ * happen when the kernel signal handling code touches the user signal stack.
+ *
+ * In order to avoid pre-faulting the signal stack memory and to force the
+ * pagefault to happen precisely in the kernel signal handling code, the
+ * pagefault handling is done in userspace using the userfaultfd facility.
+ *
+ * Further pagefaults are triggered by crafting the signal handler's ucontext
+ * to point to additional memory regions managed by the userfaultfd, so using
+ * the same mechanism used to avoid pre-faulting the signal stack memory.
+ *
+ * On failure (bug is present) kernel crashes or never returns control back to
+ * userspace. If bug is not present, tests completes almost immediately.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <linux/userfaultfd.h>
+#include <poll.h>
+#include <unistd.h>
+#include <sys/ioctl.h>
+#include <sys/syscall.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <pthread.h>
+#include <signal.h>
+#include <errno.h>
+
+#include "tm.h"
+
+
+#define UF_MEM_SIZE 655360	/* 10 x 64k pages */
+
+/* Memory handled by userfaultfd */
+static char *uf_mem;
+static size_t uf_mem_offset = 0;
+
+/*
+ * Data that will be copied into the faulting pages (instead of zero-filled
+ * pages). This is used to make the test more reliable and avoid segfaulting
+ * when we return from the signal handler. Since we are making the signal
+ * handler's ucontext point to newly allocated memory, when that memory is
+ * paged-in it will contain the expected content.
+ */
+static char backing_mem[UF_MEM_SIZE];
+
+static size_t pagesize;
+
+/*
+ * Return a chunk of at least 'size' bytes of memory that will be handled by
+ * userfaultfd. If 'backing_data' is not NULL, its content will be save to
+ * 'backing_mem' and then copied into the faulting pages when the page fault
+ * is handled.
+ */
+void *get_uf_mem(size_t size, void *backing_data)
+{
+	void *ret;
+
+	if (uf_mem_offset + size > UF_MEM_SIZE) {
+		fprintf(stderr, "Requesting more uf_mem than expected!\n");
+		exit(EXIT_FAILURE);
+	}
+
+	ret = &uf_mem[uf_mem_offset];
+
+	/* Save the data that will be copied into the faulting page */
+	if (backing_data != NULL)
+		memcpy(&backing_mem[uf_mem_offset], backing_data, size);
+
+	/* Reserve the requested amount of uf_mem */
+	uf_mem_offset += size;
+	/* Keep uf_mem_offset aligned to the page size (round up) */
+	uf_mem_offset = (uf_mem_offset + pagesize - 1) & ~(pagesize - 1);
+
+	return ret;
+}
+
+void *fault_handler_thread(void *arg)
+{
+	struct uffd_msg msg;	/* Data read from userfaultfd */
+	long uffd;		/* userfaultfd file descriptor */
+	struct uffdio_copy uffdio_copy;
+	struct pollfd pollfd;
+	ssize_t nread, offset;
+
+	uffd = (long) arg;
+
+	for (;;) {
+		pollfd.fd = uffd;
+		pollfd.events = POLLIN;
+		if (poll(&pollfd, 1, -1) == -1) {
+			perror("poll() failed");
+			exit(EXIT_FAILURE);
+		}
+
+		nread = read(uffd, &msg, sizeof(msg));
+		if (nread == 0) {
+			fprintf(stderr, "read(): EOF on userfaultfd\n");
+			exit(EXIT_FAILURE);
+		}
+
+		if (nread == -1) {
+			perror("read() failed");
+			exit(EXIT_FAILURE);
+		}
+
+		/* We expect only one kind of event */
+		if (msg.event != UFFD_EVENT_PAGEFAULT) {
+			fprintf(stderr, "Unexpected event on userfaultfd\n");
+			exit(EXIT_FAILURE);
+		}
+
+		/*
+		 * We need to handle page faults in units of pages(!).
+		 * So, round faulting address down to page boundary.
+		 */
+		uffdio_copy.dst = msg.arg.pagefault.address & ~(pagesize-1);
+
+		offset = (char *) uffdio_copy.dst - uf_mem;
+		uffdio_copy.src = (unsigned long) &backing_mem[offset];
+
+		uffdio_copy.len = pagesize;
+		uffdio_copy.mode = 0;
+		uffdio_copy.copy = 0;
+		if (ioctl(uffd, UFFDIO_COPY, &uffdio_copy) == -1) {
+			perror("ioctl-UFFDIO_COPY failed");
+			exit(EXIT_FAILURE);
+		}
+	}
+}
+
+void setup_uf_mem(void)
+{
+	long uffd;		/* userfaultfd file descriptor */
+	pthread_t thr;
+	struct uffdio_api uffdio_api;
+	struct uffdio_register uffdio_register;
+	int ret;
+
+	pagesize = sysconf(_SC_PAGE_SIZE);
+
+	/* Create and enable userfaultfd object */
+	uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
+	if (uffd == -1) {
+		perror("userfaultfd() failed");
+		exit(EXIT_FAILURE);
+	}
+	uffdio_api.api = UFFD_API;
+	uffdio_api.features = 0;
+	if (ioctl(uffd, UFFDIO_API, &uffdio_api) == -1) {
+		perror("ioctl-UFFDIO_API failed");
+		exit(EXIT_FAILURE);
+	}
+
+	/*
+	 * Create a private anonymous mapping. The memory will be demand-zero
+	 * paged, that is, not yet allocated. When we actually touch the memory
+	 * the related page will be allocated via the userfaultfd mechanism.
+	 */
+	uf_mem = mmap(NULL, UF_MEM_SIZE, PROT_READ | PROT_WRITE,
+		      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	if (uf_mem == MAP_FAILED) {
+		perror("mmap() failed");
+		exit(EXIT_FAILURE);
+	}
+
+	/*
+	 * Register the memory range of the mapping we've just mapped to be
+	 * handled by the userfaultfd object. In 'mode' we request to track
+	 * missing pages (i.e. pages that have not yet been faulted-in).
+	 */
+	uffdio_register.range.start = (unsigned long) uf_mem;
+	uffdio_register.range.len = UF_MEM_SIZE;
+	uffdio_register.mode = UFFDIO_REGISTER_MODE_MISSING;
+	if (ioctl(uffd, UFFDIO_REGISTER, &uffdio_register) == -1) {
+		perror("ioctl-UFFDIO_REGISTER");
+		exit(EXIT_FAILURE);
+	}
+
+	/* Create a thread that will process the userfaultfd events */
+	ret = pthread_create(&thr, NULL, fault_handler_thread, (void *) uffd);
+	if (ret != 0) {
+		fprintf(stderr, "pthread_create(): Error. Returned %d\n", ret);
+		exit(EXIT_FAILURE);
+	}
+}
+
+/*
+ * Assumption: the signal was delivered while userspace was in transactional or
+ * suspended state, i.e. uc->uc_link != NULL.
+ */
+void signal_handler(int signo, siginfo_t *si, void *uc)
+{
+	ucontext_t *ucp = uc;
+
+	/* Skip 'trap' after returning, otherwise we get a SIGTRAP again */
+	ucp->uc_link->uc_mcontext.regs->nip += 4;
+
+	ucp->uc_mcontext.v_regs =
+		get_uf_mem(sizeof(elf_vrreg_t), ucp->uc_mcontext.v_regs);
+
+	ucp->uc_link->uc_mcontext.v_regs =
+		get_uf_mem(sizeof(elf_vrreg_t), ucp->uc_link->uc_mcontext.v_regs);
+
+	ucp->uc_link = get_uf_mem(sizeof(ucontext_t), ucp->uc_link);
+}
+
+bool have_userfaultfd(void)
+{
+	long rc;
+
+	errno = 0;
+	rc = syscall(__NR_userfaultfd, -1);
+
+	return rc == 0 || errno != ENOSYS;
+}
+
+int tm_signal_pagefault(void)
+{
+	struct sigaction sa;
+	stack_t ss;
+
+	SKIP_IF(!have_htm());
+	SKIP_IF(!have_userfaultfd());
+
+	setup_uf_mem();
+
+	/*
+	 * Set an alternative stack that will generate a page fault when the
+	 * signal is raised. The page fault will be treated via userfaultfd,
+	 * i.e. via fault_handler_thread.
+	 */
+	ss.ss_sp = get_uf_mem(SIGSTKSZ, NULL);
+	ss.ss_size = SIGSTKSZ;
+	ss.ss_flags = 0;
+	if (sigaltstack(&ss, NULL) == -1) {
+		perror("sigaltstack() failed");
+		exit(EXIT_FAILURE);
+	}
+
+	sa.sa_flags = SA_SIGINFO | SA_ONSTACK;
+	sa.sa_sigaction = signal_handler;
+	if (sigaction(SIGTRAP, &sa, NULL) == -1) {
+		perror("sigaction() failed");
+		exit(EXIT_FAILURE);
+	}
+
+	/* Trigger a SIGTRAP in transactional state */
+	asm __volatile__(
+			"tbegin.;"
+			"beq    1f;"
+			"trap;"
+			"1: ;"
+			: : : "memory");
+
+	/* Trigger a SIGTRAP in suspended state */
+	asm __volatile__(
+			"tbegin.;"
+			"beq    1f;"
+			"tsuspend.;"
+			"trap;"
+			"tresume.;"
+			"1: ;"
+			: : : "memory");
+
+	return EXIT_SUCCESS;
+}
+
+int main(int argc, char **argv)
+{
+	/*
+	 * Depending on kernel config, the TM Bad Thing might not result in a
+	 * crash, instead the kernel never returns control back to userspace, so
+	 * set a tight timeout. If the test passes it completes almost
+	 * immediately.
+	 */
+	test_harness_set_timeout(2);
+	return test_harness(tm_signal_pagefault, "tm_signal_pagefault");
+}
-- 
2.21.1


^ permalink raw reply related

* [PATCH v3 3/3] selftests/powerpc: Don't rely on segfault to rerun the test
From: Gustavo Luiz Duarte @ 2020-02-11  3:38 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mikey, Gustavo Luiz Duarte, gromero
In-Reply-To: <20200211033831.11165-1-gustavold@linux.ibm.com>

The test case tm-signal-context-force-tm expects a segfault to happen on
returning from signal handler, and then does a setcontext() to run the test
again. However, the test doesn't always segfault, causing the test to run a
single time.

This patch fixes the test by putting it within a loop and jumping, via
setcontext, just prior to the loop in case it segfaults. This way we get the
desired behavior (run the test COUNT_MAX times) regardless if it segfaults or
not. This also reduces the use of setcontext for control flow logic, keeping it
only in the segfault handler.

Also, since 'count' is changed within the signal handler, it is declared as
volatile to prevent any compiler optimization getting confused with
asynchronous changes.

Signed-off-by: Gustavo Luiz Duarte <gustavold@linux.ibm.com>
---
 .../powerpc/tm/tm-signal-context-force-tm.c   | 79 +++++++++----------
 1 file changed, 37 insertions(+), 42 deletions(-)

diff --git a/tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c b/tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c
index 31717625f318..9ff7bdb6d47a 100644
--- a/tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c
+++ b/tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c
@@ -42,9 +42,10 @@
 #endif
 
 /* Setting contexts because the test will crash and we want to recover */
-ucontext_t init_context, main_context;
+ucontext_t init_context;
 
-static int count, first_time;
+/* count is changed in the signal handler, so it must be volatile */
+static volatile int count;
 
 void usr_signal_handler(int signo, siginfo_t *si, void *uc)
 {
@@ -98,11 +99,6 @@ void usr_signal_handler(int signo, siginfo_t *si, void *uc)
 
 void seg_signal_handler(int signo, siginfo_t *si, void *uc)
 {
-	if (count == COUNT_MAX) {
-		/* Return to tm_signal_force_msr() and exit */
-		setcontext(&main_context);
-	}
-
 	count++;
 
 	/* Reexecute the test */
@@ -126,37 +122,40 @@ void tm_trap_test(void)
 	 */
 	getcontext(&init_context);
 
-	/* Allocated an alternative signal stack area */
-	ss.ss_sp = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,
-			MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
-	ss.ss_size = SIGSTKSZ;
-	ss.ss_flags = 0;
-
-	if (ss.ss_sp == (void *)-1) {
-		perror("mmap error\n");
-		exit(-1);
-	}
-
-	/* Force the allocation through a page fault */
-	if (madvise(ss.ss_sp, SIGSTKSZ, MADV_DONTNEED)) {
-		perror("madvise\n");
-		exit(-1);
-	}
-
-	/* Setting an alternative stack to generate a page fault when
-	 * the signal is raised.
-	 */
-	if (sigaltstack(&ss, NULL)) {
-		perror("sigaltstack\n");
-		exit(-1);
+	while (count < COUNT_MAX) {
+		/* Allocated an alternative signal stack area */
+		ss.ss_sp = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,
+				MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
+		ss.ss_size = SIGSTKSZ;
+		ss.ss_flags = 0;
+
+		if (ss.ss_sp == (void *)-1) {
+			perror("mmap error\n");
+			exit(-1);
+		}
+
+		/* Force the allocation through a page fault */
+		if (madvise(ss.ss_sp, SIGSTKSZ, MADV_DONTNEED)) {
+			perror("madvise\n");
+			exit(-1);
+		}
+
+		/* Setting an alternative stack to generate a page fault when
+		 * the signal is raised.
+		 */
+		if (sigaltstack(&ss, NULL)) {
+			perror("sigaltstack\n");
+			exit(-1);
+		}
+
+		/* The signal handler will enable MSR_TS */
+		sigaction(SIGUSR1, &usr_sa, NULL);
+		/* If it does not crash, it might segfault, avoid it to retest */
+		sigaction(SIGSEGV, &seg_sa, NULL);
+
+		raise(SIGUSR1);
+		count++;
 	}
-
-	/* The signal handler will enable MSR_TS */
-	sigaction(SIGUSR1, &usr_sa, NULL);
-	/* If it does not crash, it will segfault, avoid it to retest */
-	sigaction(SIGSEGV, &seg_sa, NULL);
-
-	raise(SIGUSR1);
 }
 
 int tm_signal_context_force_tm(void)
@@ -169,11 +168,7 @@ int tm_signal_context_force_tm(void)
 	 */
 	SKIP_IF(!is_ppc64le());
 
-	/* Will get back here after COUNT_MAX interactions */
-	getcontext(&main_context);
-
-	if (!first_time++)
-		tm_trap_test();
+	tm_trap_test();
 
 	return EXIT_SUCCESS;
 }
-- 
2.21.1


^ permalink raw reply related

* Re: [PATCH V13] mm/debug: Add tests validating architecture page table helpers
From: Anshuman Khandual @ 2020-02-11  2:25 UTC (permalink / raw)
  To: Russell King - ARM Linux admin, Christophe Leroy
  Cc: Mark Rutland, uclinux-h8-devel, linux-m68k, linux-ia64, linux-sh,
	Peter Zijlstra, James Hogan, Tetsuo Handa, Heiko Carstens,
	Michal Hocko, linux-mm, Dave Hansen, Paul Mackerras, sparclinux,
	linux-hexagon, linux-riscv, Thomas Gleixner, linux-s390,
	kbuild test robot, Yoshinori Sato, Max Filippov, x86,
	Matthew Wilcox, Steven Price, Jason Gunthorpe, Geert Uytterhoeven,
	Gerald Schaefer, Mark Salter, Matt Turner, linux-snps-arc,
	Ingo Molnar, linux-xtensa, Kees Cook, Masahiro Yamada,
	linux-alpha, Aurelien Jacquiot, linux-c6x-dev, Mark Brown,
	Ivan Kokshaysky, Paul Walmsley, Kirill A . Shutemov, Dan Williams,
	Guan Xuetao, Vlastimil Babka, Richard Henderson, linux-arm-kernel,
	Chris Zankel, Michal Simek, kbuild-all, Brian Cain,
	Ard Biesheuvel, Greg Kroah-Hartman, linux-mips, Ralf Baechle,
	linux-kernel, Paul Burton, Mike Rapoport, Palmer Dabbelt,
	Vineet Gupta, Martin Schwidefsky, Qian Cai, Andrew Morton,
	linuxppc-dev, David S. Miller
In-Reply-To: <20200210110639.GC25745@shell.armlinux.org.uk>



On 02/10/2020 04:36 PM, Russell King - ARM Linux admin wrote:
> On Mon, Feb 10, 2020 at 11:46:23AM +0100, Christophe Leroy wrote:
>>
>>
>> Le 10/02/2020 à 11:02, Russell King - ARM Linux admin a écrit :
>>> On Mon, Feb 10, 2020 at 07:38:38AM +0100, Christophe Leroy wrote:
>>>>
>>>>
>>>> Le 10/02/2020 à 06:35, Anshuman Khandual a écrit :
>>>>>
>>>>>
>>>>> On 02/10/2020 10:22 AM, Andrew Morton wrote:
>>>>>> On Thu, 6 Feb 2020 13:49:35 +0530 Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>>>>>>
>>>>>>>
>>>>>>> On 02/06/2020 04:40 AM, kbuild test robot wrote:
>>>>>>>> Hi Anshuman,
>>>>>>>>
>>>>>>>> Thank you for the patch! Yet something to improve:
>>>>>>>>
>>>>>>>> [auto build test ERROR on powerpc/next]
>>>>>>>> [also build test ERROR on s390/features linus/master arc/for-next v5.5]
>>>>>>>> [cannot apply to mmotm/master tip/x86/core arm64/for-next/core next-20200205]
>>>>>>>> [if your patch is applied to the wrong git tree, please drop us a note to help
>>>>>>>> improve the system. BTW, we also suggest to use '--base' option to specify the
>>>>>>>> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>>>>>>>>
>>>>>>>> url:    https://github.com/0day-ci/linux/commits/Anshuman-Khandual/mm-debug-Add-tests-validating-architecture-page-table-helpers/20200205-215507
>>>>>>>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
>>>>>>>> config: ia64-allmodconfig (attached as .config)
>>>>>>>> compiler: ia64-linux-gcc (GCC) 7.5.0
>>>>>>>> reproduce:
>>>>>>>>           wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>>>>>>>           chmod +x ~/bin/make.cross
>>>>>>>>           # save the attached .config to linux build tree
>>>>>>>>           GCC_VERSION=7.5.0 make.cross ARCH=ia64
>>>>>>>>
>>>>>>>> If you fix the issue, kindly add following tag
>>>>>>>> Reported-by: kbuild test robot <lkp@intel.com>
>>>>>>>>
>>>>>>>> All error/warnings (new ones prefixed by >>):
>>>>>>>>
>>>>>>>>      In file included from include/asm-generic/pgtable-nopud.h:8:0,
>>>>>>>>                       from arch/ia64/include/asm/pgtable.h:586,
>>>>>>>>                       from include/linux/mm.h:99,
>>>>>>>>                       from include/linux/highmem.h:8,
>>>>>>>>                       from mm/debug_vm_pgtable.c:14:
>>>>>>>>      mm/debug_vm_pgtable.c: In function 'pud_clear_tests':
>>>>>>>>>> include/asm-generic/pgtable-nop4d-hack.h:47:32: error: implicit declaration of function '__pgd'; did you mean '__p4d'? [-Werror=implicit-function-declaration]
>>>>>>>>       #define __pud(x)    ((pud_t) { __pgd(x) })
>>>>>>>>                                      ^
>>>>>>>>>> mm/debug_vm_pgtable.c:141:8: note: in expansion of macro '__pud'
>>>>>>>>        pud = __pud(pud_val(pud) | RANDOM_ORVALUE);
>>>>>>>>              ^~~~~
>>>>>>>>>> include/asm-generic/pgtable-nop4d-hack.h:47:22: warning: missing braces around initializer [-Wmissing-braces]
>>>>>>>>       #define __pud(x)    ((pud_t) { __pgd(x) })
>>>>>>>>                            ^
>>>>>>>>>> mm/debug_vm_pgtable.c:141:8: note: in expansion of macro '__pud'
>>>>>>>>        pud = __pud(pud_val(pud) | RANDOM_ORVALUE);
>>>>>>>>              ^~~~~
>>>>>>>>      cc1: some warnings being treated as errors
>>>>>>>
>>>>>>> This build failure is expected now given that we have allowed DEBUG_VM_PGTABLE
>>>>>>> with EXPERT without platform requiring ARCH_HAS_DEBUG_VM_PGTABLE. This problem
>>>>>>> i.e build failure caused without a platform __pgd(), is known to exist both on
>>>>>>> ia64 and arm (32bit) platforms. Please refer https://lkml.org/lkml/2019/9/24/314
>>>>>>> for details where this was discussed earlier.
>>>>>>>
>>>>>>
>>>>>> I'd prefer not to merge a patch which is known to cause build
>>>>>> regressions.  Is there some temporary thing we can do to prevent these
>>>>>> errors until arch maintainers(?) get around to implementing the
>>>>>> long-term fixes?
>>>>>
>>>>> We could explicitly disable CONFIG_DEBUG_VM_PGTABLE on ia64 and arm platforms
>>>>> which will ensure that others can still use the EXPERT path.
>>>>>
>>>>> config DEBUG_VM_PGTABLE
>>>>> 	bool "Debug arch page table for semantics compliance"
>>>>> 	depends on MMU
>>>>> 	depends on !(IA64 || ARM)
>>>>> 	depends on ARCH_HAS_DEBUG_VM_PGTABLE || EXPERT
>>>>> 	default n if !ARCH_HAS_DEBUG_VM_PGTABLE
>>>>> 	default y if DEBUG_VM
>>>>>
>>>>
>>>> On both ia32 and arm, the fix is trivial.
>>>>
>>>> Can we include the fix within this patch, just the same way as the
>>>> mm_p4d_folded() fix for x86 ?
>>>
>>> Why should arm include a macro for something that nothing (apart from
>>> this checker) requires?  If the checker requires it but the rest of
>>> the kernel does not, it suggests that the checker isn't actually
>>> correct, and the results can't be relied upon.
>>>
>>
>> As far as I can see, the problem is that arm opencodes part of the API
>> instead of including asm-generic/pgtable-nopmd.h
>>
>> Here, the ARM has 2 levels, ie only PGD and PTE. But instead of defining
>> __pgd and __pte and getting everything else from asm-generic, it defines a
>> __pmd then redefines the folded levels like the pud, etc ...
>>
>> That's exactly what the checker aims at detecting: architectures than do not
>> properly use the standard linux page table structures.
> 
> There are good reasons for the way ARM does stuff.  The generic crap was
> written without regard for the circumstances that ARM has, and thus is
> entirely unsuitable for 32-bit ARM.

Since we dont have an agreement here, lets just settle with disabling the
test for now on platforms where the build fails. CONFIG_EXPERT is enabling
this test for better adaptability and coverage, hence how about re framing
the config like this ? This at the least conveys the fact that EXPERT only
works when platform is neither IA64 or ARM.

config DEBUG_VM_PGTABLE
	bool "Debug arch page table for semantics compliance"
	depends on MMU
	depends on ARCH_HAS_DEBUG_VM_PGTABLE || (EXPERT &&  !(IA64 || ARM))
	default n if !ARCH_HAS_DEBUG_VM_PGTABLE
	default y if DEBUG_VM

^ permalink raw reply

* Re: [PATCH V5 06/14] powerpc/vas: Setup thread IRQ handler per VAS instance
From: Michael Neuling @ 2020-02-11  4:08 UTC (permalink / raw)
  To: Haren Myneni; +Cc: herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1581311856.18705.23.camel@hbabu-laptop>

On Sun, 2020-02-09 at 21:17 -0800, Haren Myneni wrote:
> On Fri, 2020-02-07 at 16:57 +1100, Michael Neuling wrote:
> > >  /*
> > > + * Process CRBs that we receive on the fault window.
> > > + */
> > > +irqreturn_t vas_fault_handler(int irq, void *data)
> > > +{
> > > +	struct vas_instance *vinst = data;
> > > +	struct coprocessor_request_block buf, *crb;
> > > +	struct vas_window *window;
> > > +	void *fifo;
> > > +
> > > +	/*
> > > +	 * VAS can interrupt with multiple page faults. So process all
> > > +	 * valid CRBs within fault FIFO until reaches invalid CRB.
> > > +	 * NX updates nx_fault_stamp in CRB and pastes in fault FIFO.
> > > +	 * kernel retrives send window from parition send window ID
> > > +	 * (pswid) in nx_fault_stamp. So pswid should be non-zero and
> > > +	 * use this to check whether CRB is valid.
> > > +	 * After reading CRB entry, it is reset with 0's in fault FIFO.
> > > +	 *
> > > +	 * In case kernel receives another interrupt with different page
> > > +	 * fault and CRBs are processed by the previous handling, will be
> > > +	 * returned from this function when it sees invalid CRB (means 0's).
> > > +	 */
> > > +	do {
> > > +		mutex_lock(&vinst->mutex);
> > 
> > This isn't going to work.
> > 
> > From Documentation/locking/mutex-design.rst
> > 
> >     - Mutexes may not be used in hardware or software interrupt
> >       contexts such as tasklets and timers.
> 
> Initially used kernel thread per VAS instance and later using IRQ
> thread. 
> 
> vas_fault_handler() is IRQ thread function, not IRQ handler. I thought
> we can use mutex_lock() in thread function.

Sorry, I missed it was a threaded IRQ handler, so I think is ok to use a
mutex_lock() in there.

You should run with CONFIG DEBUG_MUTEXES and CONFIG_LOCKDEP enabled to give you
some more confidence.

It would be good to document how this mutex is used and document the start of
the function so it doesn't get changed later to a non-threaded handler. 

Mikey

^ permalink raw reply

* QEMU/KVM snapshot restore bug
From: dftxbs3e @ 2020-02-11  3:57 UTC (permalink / raw)
  To: linuxppc-dev


[-- Attachment #1.1.1: Type: text/plain, Size: 4205 bytes --]

Hello,

I took a snapshot of a ppc64 (big endian) VM from a ppc64 (little endian) host using `virsh snapshot-create-as --domain <name> --name <name>`

Then I restarted my system and tried restoring the snapshot:

# virsh snapshot-revert --domain <name> --snapshotname <name>
error: internal error: process exited while connecting to monitor: 2020-02-11T03:18:08.110582Z qemu-system-ppc64: KVM_SET_DEVICE_ATTR failed: Group 3 attr 0x0000000000001309: Device or resource busy
2020-02-11T03:18:08.110605Z qemu-system-ppc64: error while loading state for instance 0x0 of device 'spapr'
2020-02-11T03:18:08.112843Z qemu-system-ppc64: Error -1 while loading VM state

And dmesg shows each time the restore command is executed:

[  180.176606] WARNING: CPU: 16 PID: 5528 at arch/powerpc/kvm/book3s_xive.c:345 xive_try_pick_queue+0x40/0xb8 [kvm]
[  180.176608] Modules linked in: vhost_net vhost tap kvm_hv kvm xt_CHECKSUM xt_MASQUERADE nf_nat_tftp nf_conntrack_tftp tun bridge 8021q garp mrp stp llc rfkill nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT ip6t_REJECT nf_reject_ipv6 ip6t_rpfilter ipt_REJECT nf_reject_ipv4 xt_conntrack ebtable_nat ebtable_broute ip6table_nat ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_nat iptable_mangle iptable_raw iptable_security nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set nfnetlink ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter sunrpc raid1 at24 regmap_i2c snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg joydev snd_hda_codec snd_hda_core ofpart snd_hwdep crct10dif_vpmsum snd_seq ipmi_powernv powernv_flash ipmi_devintf snd_seq_device mtd ipmi_msghandler rtc_opal snd_pcm opal_prd i2c_opal snd_timer snd soundcore lz4 lz4_compress zram ip_tables xfs libcrc32c dm_crypt amdgpu ast drm_vram_helper mfd_core i2c_algo_bit gpu_sched drm_kms_helper mpt3sas
[  180.176652]  syscopyarea sysfillrect sysimgblt fb_sys_fops ttm drm vmx_crypto tg3 crc32c_vpmsum nvme raid_class scsi_transport_sas nvme_core drm_panel_orientation_quirks i2c_core fuse
[  180.176663] CPU: 16 PID: 5528 Comm: qemu-system-ppc Not tainted 5.4.17-200.fc31.ppc64le #1
[  180.176665] NIP:  c00800000a883c80 LR: c00800000a886db8 CTR: c00800000a88a9e0
[  180.176667] REGS: c000000767a17890 TRAP: 0700   Not tainted  (5.4.17-200.fc31.ppc64le)
[  180.176668] MSR:  9000000000029033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 48224248  XER: 20040000
[  180.176673] CFAR: c00800000a886db4 IRQMASK: 0 
               GPR00: c00800000a886db8 c000000767a17b20 c00800000a8aed00 c0002005468a4480 
               GPR04: 0000000000000000 0000000000000000 0000000000000000 0000000000000001 
               GPR08: c0002007142b2400 c0002007142b2400 0000000000000000 c00800000a8910f0 
               GPR12: c00800000a88a488 c0000007fffed000 0000000000000000 0000000000000000 
               GPR16: 0000000149524180 00007ffff39bda78 00007ffff39bda30 000000000000025c 
               GPR20: 0000000000000000 0000000000000003 c0002006f13a0000 0000000000000000 
               GPR24: 0000000000001359 0000000000000000 c0000002f8c96c38 c0000002f8c80000 
               GPR28: 0000000000000000 c0002006f13a0000 c0002006f13a4038 c000000767a17be4 
[  180.176688] NIP [c00800000a883c80] xive_try_pick_queue+0x40/0xb8 [kvm]
[  180.176693] LR [c00800000a886db8] kvmppc_xive_select_target+0x100/0x210 [kvm]
[  180.176694] Call Trace:
[  180.176696] [c000000767a17b20] [c000000767a17b70] 0xc000000767a17b70 (unreliable)
[  180.176701] [c000000767a17b70] [c00800000a88b420] kvmppc_xive_native_set_attr+0xf98/0x1760 [kvm]
[  180.176705] [c000000767a17cc0] [c00800000a86392c] kvm_device_ioctl+0xf4/0x180 [kvm]
[  180.176710] [c000000767a17d10] [c0000000005380b0] do_vfs_ioctl+0xaa0/0xd90
[  180.176712] [c000000767a17dd0] [c000000000538464] sys_ioctl+0xc4/0x110
[  180.176716] [c000000767a17e20] [c00000000000b9d0] system_call+0x5c/0x68
[  180.176717] Instruction dump:
[  180.176719] 794ad182 0b0a0000 2c290000 41820080 89490010 2c0a0000 41820074 78883664 
[  180.176723] 7d094214 e9480070 7d470074 78e7d182 <0b070000> 2c2a0000 41820054 81480078 
[  180.176727] ---[ end trace 056a6dd275e20684 ]---

Let me know if I can provide more information

Thanks


[-- Attachment #1.1.2: Type: text/html, Size: 4457 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH] powerpc/8xx: Fix clearing of bits 20-23 in ITLB miss
From: Leonardo Bras @ 2020-02-11  4:28 UTC (permalink / raw)
  To: christophe.leroy, benh, paulus, mpe; +Cc: linuxppc-dev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

Christophe Leroy <christophe.leroy@c-s.fr> writes:

> In ITLB miss handled the line supposed to clear bits 20-23 on the
> L2 ITLB entry is buggy and does indeed nothing, leading to undefined
> value which could allow execution when it shouldn't.
>
> Properly do the clearing with the relevant instruction.
>
> Fixes: 74fabcadfd43 ("powerpc/8xx: don't use r12/SPRN_SPRG_SCRATCH2 in TLB Miss handlers")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  arch/powerpc/kernel/head_8xx.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
> index 9922306ae512..073a651787df 100644
> --- a/arch/powerpc/kernel/head_8xx.S
> +++ b/arch/powerpc/kernel/head_8xx.S
> @@ -256,7 +256,7 @@ InstructionTLBMiss:
>  	 * set.  All other Linux PTE bits control the behavior
>  	 * of the MMU.
>  	 */
> -	rlwimi	r10, r10, 0, 0x0f00	/* Clear bits 20-23 */
> +	rlwinm	r10, r10, 0, ~0x0f00	/* Clear bits 20-23 */
>  	rlwimi	r10, r10, 4, 0x0400	/* Copy _PAGE_EXEC into bit 21 */
>  	ori	r10, r10, RPN_PATTERN | 0x200 /* Set 22 and 24-27 */
>  	mtspr	SPRN_MI_RPN, r10	/* Update TLB entry */
> -- 
> 2.25.0

Looks a valid change.
rlwimi  r10, r10, 0, 0x0f00 means: 
r10 = ((r10 << 0) & 0x0f00) | (r10 & ~0x0f00) which ends up being
r10 = r10 

On ISA, rlwinm is recommended for clearing high order bits.
rlwinm  r10, r10, 0, ~0x0f00 means:
r10 = (r10 << 0) & ~0x0f00

Which does exactly what the comments suggests.

FWIW:
Reviwed-by: Leonardo Bras <leonardo@linux.ibm.com>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH v2 00/13] Initial Prefixed Instruction support
From: Jordan Niethe @ 2020-02-11  5:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, mpe, Jordan Niethe, dja, bala24

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.

This v2 incorporates feedback from Daniel Axtens and and Balamuruhan
S. The major changes are:
    - Squashing together all commits about SRR1 bits
    - Squashing all commits for supporting prefixed load stores
    - Changing abbreviated references to sufx/prfx -> suffix/prefix
    - Introducing macros for returning the length of an instruction
    - Removing sign extension flag from pstd/pld in sstep.c
    - Dropping patch  "powerpc/fault: Use analyse_instr() to check for
      store with updates to sp" from the series, it did not really fit
      with prefixed enablement in the first place and as reported by Greg
      Kurz did not work correctly.
	

Alistair Popple (1):
  powerpc: Enable Prefixed Instructions

Jordan Niethe (12):
  powerpc: Define new SRR1 bits for a future ISA version
  powerpc sstep: Prepare to support prefixed instructions
  powerpc sstep: Add support for prefixed load/stores
  powerpc sstep: Add support for prefixed fixed-point arithmetic
  powerpc: Support prefixed instructions in alignment handler
  powerpc/traps: Check for prefixed instructions in
    facility_unavailable_exception()
  powerpc/xmon: Add initial support for prefixed instructions
  powerpc/xmon: Dump prefixed instructions
  powerpc/kprobes: Support kprobes on prefixed instructions
  powerpc/uprobes: Add support for prefixed instructions
  powerpc/hw_breakpoints: Initial support for prefixed instructions
  powerpc: Add prefix support to mce_find_instr_ea_and_pfn()

 arch/powerpc/include/asm/kprobes.h    |   5 +-
 arch/powerpc/include/asm/ppc-opcode.h |   5 +
 arch/powerpc/include/asm/reg.h        |   7 +-
 arch/powerpc/include/asm/sstep.h      |   9 +-
 arch/powerpc/include/asm/uaccess.h    |  30 +++++
 arch/powerpc/include/asm/uprobes.h    |  16 ++-
 arch/powerpc/kernel/align.c           |   8 +-
 arch/powerpc/kernel/dt_cpu_ftrs.c     |  23 ++++
 arch/powerpc/kernel/hw_breakpoint.c   |   8 +-
 arch/powerpc/kernel/kprobes.c         |  47 +++++--
 arch/powerpc/kernel/mce_power.c       |   6 +-
 arch/powerpc/kernel/optprobes.c       |  31 +++--
 arch/powerpc/kernel/optprobes_head.S  |   6 +
 arch/powerpc/kernel/traps.c           |  22 +++-
 arch/powerpc/kernel/uprobes.c         |   4 +-
 arch/powerpc/kvm/book3s_hv_nested.c   |   2 +-
 arch/powerpc/kvm/book3s_hv_rm_mmu.c   |   2 +-
 arch/powerpc/kvm/emulate_loadstore.c  |   2 +-
 arch/powerpc/lib/sstep.c              | 181 +++++++++++++++++++++++++-
 arch/powerpc/lib/test_emulate_step.c  |  30 ++---
 arch/powerpc/xmon/xmon.c              | 133 +++++++++++++++----
 21 files changed, 487 insertions(+), 90 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH v2 01/13] powerpc: Enable Prefixed Instructions
From: Jordan Niethe @ 2020-02-11  5:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, mpe, dja, bala24
In-Reply-To: <20200211053355.21574-1-jniethe5@gmail.com>

From: Alistair Popple <alistair@popple.id.au>

Prefix instructions have their own FSCR bit which needs to enabled via
a CPU feature. The kernel will save the FSCR for problem state but it
needs to be enabled initially.

Signed-off-by: Alistair Popple <alistair@popple.id.au>
---
 arch/powerpc/include/asm/reg.h    |  3 +++
 arch/powerpc/kernel/dt_cpu_ftrs.c | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 1aa46dff0957..c7758c2ccc5f 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -397,6 +397,7 @@
 #define SPRN_RWMR	0x375	/* Region-Weighting Mode Register */
 
 /* HFSCR and FSCR bit numbers are the same */
+#define FSCR_PREFIX_LG	13	/* Enable Prefix Instructions */
 #define FSCR_SCV_LG	12	/* Enable System Call Vectored */
 #define FSCR_MSGP_LG	10	/* Enable MSGP */
 #define FSCR_TAR_LG	8	/* Enable Target Address Register */
@@ -408,11 +409,13 @@
 #define FSCR_VECVSX_LG	1	/* Enable VMX/VSX  */
 #define FSCR_FP_LG	0	/* Enable Floating Point */
 #define SPRN_FSCR	0x099	/* Facility Status & Control Register */
+#define   FSCR_PREFIX	__MASK(FSCR_PREFIX_LG)
 #define   FSCR_SCV	__MASK(FSCR_SCV_LG)
 #define   FSCR_TAR	__MASK(FSCR_TAR_LG)
 #define   FSCR_EBB	__MASK(FSCR_EBB_LG)
 #define   FSCR_DSCR	__MASK(FSCR_DSCR_LG)
 #define SPRN_HFSCR	0xbe	/* HV=1 Facility Status & Control Register */
+#define   HFSCR_PREFIX	__MASK(FSCR_PREFIX_LG)
 #define   HFSCR_MSGP	__MASK(FSCR_MSGP_LG)
 #define   HFSCR_TAR	__MASK(FSCR_TAR_LG)
 #define   HFSCR_EBB	__MASK(FSCR_EBB_LG)
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 182b4047c1ef..396f2c6c588e 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -553,6 +553,28 @@ static int __init feat_enable_large_ci(struct dt_cpu_feature *f)
 	return 1;
 }
 
+static int __init feat_enable_prefix(struct dt_cpu_feature *f)
+{
+	u64 fscr, hfscr;
+
+	if (f->usable_privilege & USABLE_HV) {
+		hfscr = mfspr(SPRN_HFSCR);
+		hfscr |= HFSCR_PREFIX;
+		mtspr(SPRN_HFSCR, hfscr);
+	}
+
+	if (f->usable_privilege & USABLE_OS) {
+		fscr = mfspr(SPRN_FSCR);
+		fscr |= FSCR_PREFIX;
+		mtspr(SPRN_FSCR, fscr);
+
+		if (f->usable_privilege & USABLE_PR)
+			current->thread.fscr |= FSCR_PREFIX;
+	}
+
+	return 1;
+}
+
 struct dt_cpu_feature_match {
 	const char *name;
 	int (*enable)(struct dt_cpu_feature *f);
@@ -626,6 +648,7 @@ static struct dt_cpu_feature_match __initdata
 	{"vector-binary128", feat_enable, 0},
 	{"vector-binary16", feat_enable, 0},
 	{"wait-v3", feat_enable, 0},
+	{"prefix-instructions", feat_enable_prefix, 0},
 };
 
 static bool __initdata using_dt_cpu_ftrs;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 02/13] powerpc: Define new SRR1 bits for a future ISA version
From: Jordan Niethe @ 2020-02-11  5:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, mpe, Jordan Niethe, dja, bala24
In-Reply-To: <20200211053355.21574-1-jniethe5@gmail.com>

Add the BOUNDARY SRR1 bit definition for when the cause of an alignment
exception is a prefixed instruction that crosses a 64-byte boundary.
Add the PREFIXED SRR1 bit definition for exceptions caused by prefixed
instructions.

Bit 35 of SRR1 is called SRR1_ISI_N_OR_G. This name comes from it being
used to indicate that an ISI was due to the access being no-exec or
guarded. A future ISA version adds another purpose. It is also set if
there is an access in a cache-inhibited location for prefixed
instruction.  Rename from SRR1_ISI_N_OR_G to SRR1_ISI_N_G_OR_CIP.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: Combined all the commits concerning SRR1 bits.
---
 arch/powerpc/include/asm/reg.h      | 4 +++-
 arch/powerpc/kvm/book3s_hv_nested.c | 2 +-
 arch/powerpc/kvm/book3s_hv_rm_mmu.c | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index c7758c2ccc5f..173f33df4fab 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -762,7 +762,7 @@
 #endif
 
 #define   SRR1_ISI_NOPT		0x40000000 /* ISI: Not found in hash */
-#define   SRR1_ISI_N_OR_G	0x10000000 /* ISI: Access is no-exec or G */
+#define   SRR1_ISI_N_G_OR_CIP	0x10000000 /* ISI: Access is no-exec or G or CI for a prefixed instruction */
 #define   SRR1_ISI_PROT		0x08000000 /* ISI: Other protection fault */
 #define   SRR1_WAKEMASK		0x00380000 /* reason for wakeup */
 #define   SRR1_WAKEMASK_P8	0x003c0000 /* reason for wakeup on POWER8 and 9 */
@@ -789,6 +789,8 @@
 #define   SRR1_PROGADDR		0x00010000 /* SRR0 contains subsequent addr */
 
 #define   SRR1_MCE_MCP		0x00080000 /* Machine check signal caused interrupt */
+#define   SRR1_BOUNDARY		0x10000000 /* Prefixed instruction crosses 64-byte boundary */
+#define   SRR1_PREFIXED		0x20000000 /* Exception caused by prefixed instruction */
 
 #define SPRN_HSRR0	0x13A	/* Save/Restore Register 0 */
 #define SPRN_HSRR1	0x13B	/* Save/Restore Register 1 */
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index dc97e5be76f6..6ab685227574 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -1169,7 +1169,7 @@ static int kvmhv_translate_addr_nested(struct kvm_vcpu *vcpu,
 		} else if (vcpu->arch.trap == BOOK3S_INTERRUPT_H_INST_STORAGE) {
 			/* Can we execute? */
 			if (!gpte_p->may_execute) {
-				flags |= SRR1_ISI_N_OR_G;
+				flags |= SRR1_ISI_N_G_OR_CIP;
 				goto forward_to_l1;
 			}
 		} else {
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
index 220305454c23..b53a9f1c1a46 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
@@ -1260,7 +1260,7 @@ long kvmppc_hpte_hv_fault(struct kvm_vcpu *vcpu, unsigned long addr,
 	status &= ~DSISR_NOHPTE;	/* DSISR_NOHPTE == SRR1_ISI_NOPT */
 	if (!data) {
 		if (gr & (HPTE_R_N | HPTE_R_G))
-			return status | SRR1_ISI_N_OR_G;
+			return status | SRR1_ISI_N_G_OR_CIP;
 		if (!hpte_read_permission(pp, slb_v & key))
 			return status | SRR1_ISI_PROT;
 	} else if (status & DSISR_ISSTORE) {
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 03/13] powerpc sstep: Prepare to support prefixed instructions
From: Jordan Niethe @ 2020-02-11  5:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, mpe, Jordan Niethe, dja, bala24
In-Reply-To: <20200211053355.21574-1-jniethe5@gmail.com>

Currently all instructions are a single word long. A future ISA version
will include prefixed instructions which have a double word length. The
functions used for analysing and emulating instructions need to be
modified so that they can handle these new instruction types.

A prefixed instruction is a word prefix followed by a word suffix. All
prefixes uniquely have the primary op-code 1. Suffixes may be valid word
instructions or instructions that only exist as suffixes.

In handling prefixed instructions it will be convenient to treat the
suffix and prefix as separate words. To facilitate this modify
analyse_instr() and emulate_step() to take a suffix as a
parameter. For word instructions it does not matter what is passed in
here - it will be ignored.

We also define a new flag, PREFIXED, to be used in instruction_op:type.
This flag will indicate when emulating an analysed instruction if the
NIP should be advanced by word length or double word length.

The callers of analyse_instr() and emulate_step() will need their own
changes to be able to support prefixed instructions. For now modify them
to pass in 0 as a suffix.

Note that at this point no prefixed instructions are emulated or
analysed - this is just making it possible to do so.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: - Move definition of __get_user_instr() and
__get_user_instr_inatomic() to "powerpc: Support prefixed instructions
in alignment handler."
    - Use a macro for returning the length of an op
    - Rename sufx -> suffix
    - Define and use PPC_NO_SUFFIX instead of 0
---
 arch/powerpc/include/asm/ppc-opcode.h |  5 +++++
 arch/powerpc/include/asm/sstep.h      |  9 ++++++--
 arch/powerpc/kernel/align.c           |  2 +-
 arch/powerpc/kernel/hw_breakpoint.c   |  4 ++--
 arch/powerpc/kernel/kprobes.c         |  2 +-
 arch/powerpc/kernel/mce_power.c       |  2 +-
 arch/powerpc/kernel/optprobes.c       |  3 ++-
 arch/powerpc/kernel/uprobes.c         |  2 +-
 arch/powerpc/kvm/emulate_loadstore.c  |  2 +-
 arch/powerpc/lib/sstep.c              | 12 ++++++-----
 arch/powerpc/lib/test_emulate_step.c  | 30 +++++++++++++--------------
 arch/powerpc/xmon/xmon.c              |  5 +++--
 12 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index c1df75edde44..72783bc92e50 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -377,6 +377,11 @@
 #define PPC_INST_VCMPEQUD		0x100000c7
 #define PPC_INST_VCMPEQUB		0x10000006
 
+/* macro to check if a word is a prefix */
+#define IS_PREFIX(x)	(((x) >> 26) == 1)
+#define	PPC_NO_SUFFIX	0
+#define	PPC_INST_LENGTH(x)	(IS_PREFIX(x) ? 8 : 4)
+
 /* macros to insert fields into opcodes */
 #define ___PPC_RA(a)	(((a) & 0x1f) << 16)
 #define ___PPC_RB(b)	(((b) & 0x1f) << 11)
diff --git a/arch/powerpc/include/asm/sstep.h b/arch/powerpc/include/asm/sstep.h
index 769f055509c9..9ea8904a1549 100644
--- a/arch/powerpc/include/asm/sstep.h
+++ b/arch/powerpc/include/asm/sstep.h
@@ -89,11 +89,15 @@ enum instruction_type {
 #define VSX_LDLEFT	4	/* load VSX register from left */
 #define VSX_CHECK_VEC	8	/* check MSR_VEC not MSR_VSX for reg >= 32 */
 
+/* Prefixed flag, ORed in with type */
+#define PREFIXED	0x800
+
 /* Size field in type word */
 #define SIZE(n)		((n) << 12)
 #define GETSIZE(w)	((w) >> 12)
 
 #define GETTYPE(t)	((t) & INSTR_TYPE_MASK)
+#define OP_LENGTH(t)	(((t) & PREFIXED) ? 8 : 4)
 
 #define MKOP(t, f, s)	((t) | (f) | SIZE(s))
 
@@ -132,7 +136,7 @@ union vsx_reg {
  * otherwise.
  */
 extern int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
-			 unsigned int instr);
+			 unsigned int instr, unsigned int suffix);
 
 /*
  * Emulate an instruction that can be executed just by updating
@@ -149,7 +153,8 @@ void emulate_update_regs(struct pt_regs *reg, struct instruction_op *op);
  * 0 if it could not be emulated, or -1 for an instruction that
  * should not be emulated (rfid, mtmsrd clearing MSR_RI, etc.).
  */
-extern int emulate_step(struct pt_regs *regs, unsigned int instr);
+extern int emulate_step(struct pt_regs *regs, unsigned int instr,
+			unsigned int suffix);
 
 /*
  * Emulate a load or store instruction by reading/writing the
diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index 92045ed64976..ba3bf5c3ab62 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -334,7 +334,7 @@ int fix_alignment(struct pt_regs *regs)
 	if ((instr & 0xfc0006fe) == (PPC_INST_COPY & 0xfc0006fe))
 		return -EIO;
 
-	r = analyse_instr(&op, regs, instr);
+	r = analyse_instr(&op, regs, instr, PPC_NO_SUFFIX);
 	if (r < 0)
 		return -EINVAL;
 
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 2462cd7c565c..3a7ec6760dab 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -251,7 +251,7 @@ static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp,
 	if (__get_user_inatomic(instr, (unsigned int *)regs->nip))
 		goto fail;
 
-	ret = analyse_instr(&op, regs, instr);
+	ret = analyse_instr(&op, regs, instr, PPC_NO_SUFFIX);
 	type = GETTYPE(op.type);
 	size = GETSIZE(op.type);
 
@@ -275,7 +275,7 @@ static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp,
 		return false;
 	}
 
-	if (!emulate_step(regs, instr))
+	if (!emulate_step(regs, instr, PPC_NO_SUFFIX))
 		goto fail;
 
 	return true;
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 2d27ec4feee4..24a56f062d9e 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -219,7 +219,7 @@ static int try_to_emulate(struct kprobe *p, struct pt_regs *regs)
 	unsigned int insn = *p->ainsn.insn;
 
 	/* regs->nip is also adjusted if emulate_step returns 1 */
-	ret = emulate_step(regs, insn);
+	ret = emulate_step(regs, insn, PPC_NO_SUFFIX);
 	if (ret > 0) {
 		/*
 		 * Once this instruction has been boosted
diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index 1cbf7f1a4e3d..824eda536f5d 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -374,7 +374,7 @@ static int mce_find_instr_ea_and_phys(struct pt_regs *regs, uint64_t *addr,
 	if (pfn != ULONG_MAX) {
 		instr_addr = (pfn << PAGE_SHIFT) + (regs->nip & ~PAGE_MASK);
 		instr = *(unsigned int *)(instr_addr);
-		if (!analyse_instr(&op, &tmp, instr)) {
+		if (!analyse_instr(&op, &tmp, instr, PPC_NO_SUFFIX)) {
 			pfn = addr_to_pfn(regs, op.ea);
 			*addr = op.ea;
 			*phys_addr = (pfn << PAGE_SHIFT);
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index 024f7aad1952..f908d9422557 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -100,7 +100,8 @@ static unsigned long can_optimize(struct kprobe *p)
 	 * and that can be emulated.
 	 */
 	if (!is_conditional_branch(*p->ainsn.insn) &&
-			analyse_instr(&op, &regs, *p->ainsn.insn) == 1) {
+			analyse_instr(&op, &regs, *p->ainsn.insn,
+				      PPC_NO_SUFFIX) == 1) {
 		emulate_update_regs(&regs, &op);
 		nip = regs.nip;
 	}
diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index 1cfef0e5fec5..4ab40c4b576f 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -173,7 +173,7 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs)
 	 * emulate_step() returns 1 if the insn was successfully emulated.
 	 * For all other cases, we need to single-step in hardware.
 	 */
-	ret = emulate_step(regs, auprobe->insn);
+	ret = emulate_step(regs, auprobe->insn, PPC_NO_SUFFIX);
 	if (ret > 0)
 		return true;
 
diff --git a/arch/powerpc/kvm/emulate_loadstore.c b/arch/powerpc/kvm/emulate_loadstore.c
index 1139bc56e004..2fc1951cdae5 100644
--- a/arch/powerpc/kvm/emulate_loadstore.c
+++ b/arch/powerpc/kvm/emulate_loadstore.c
@@ -95,7 +95,7 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
 
 	emulated = EMULATE_FAIL;
 	vcpu->arch.regs.msr = vcpu->arch.shared->msr;
-	if (analyse_instr(&op, &vcpu->arch.regs, inst) == 0) {
+	if (analyse_instr(&op, &vcpu->arch.regs, inst, PPC_NO_SUFFIX) == 0) {
 		int type = op.type & INSTR_TYPE_MASK;
 		int size = GETSIZE(op.type);
 
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index c077acb983a1..65143ab1bf64 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1163,7 +1163,7 @@ static nokprobe_inline int trap_compare(long v1, long v2)
  * otherwise.
  */
 int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
-		  unsigned int instr)
+		  unsigned int instr, unsigned int suffix)
 {
 	unsigned int opcode, ra, rb, rc, rd, spr, u;
 	unsigned long int imm;
@@ -2756,7 +2756,8 @@ void emulate_update_regs(struct pt_regs *regs, struct instruction_op *op)
 {
 	unsigned long next_pc;
 
-	next_pc = truncate_if_32bit(regs->msr, regs->nip + 4);
+	next_pc = truncate_if_32bit(regs->msr,
+				    regs->nip + OP_LENGTH(op->type));
 	switch (GETTYPE(op->type)) {
 	case COMPUTE:
 		if (op->type & SETREG)
@@ -3101,14 +3102,14 @@ NOKPROBE_SYMBOL(emulate_loadstore);
  * or -1 if the instruction is one that should not be stepped,
  * such as an rfid, or a mtmsrd that would clear MSR_RI.
  */
-int emulate_step(struct pt_regs *regs, unsigned int instr)
+int emulate_step(struct pt_regs *regs, unsigned int instr, unsigned int suffix)
 {
 	struct instruction_op op;
 	int r, err, type;
 	unsigned long val;
 	unsigned long ea;
 
-	r = analyse_instr(&op, regs, instr);
+	r = analyse_instr(&op, regs, instr, suffix);
 	if (r < 0)
 		return r;
 	if (r > 0) {
@@ -3200,7 +3201,8 @@ int emulate_step(struct pt_regs *regs, unsigned int instr)
 	return 0;
 
  instr_done:
-	regs->nip = truncate_if_32bit(regs->msr, regs->nip + 4);
+	regs->nip = truncate_if_32bit(regs->msr,
+				      regs->nip + OP_LENGTH(op.type));
 	return 1;
 }
 NOKPROBE_SYMBOL(emulate_step);
diff --git a/arch/powerpc/lib/test_emulate_step.c b/arch/powerpc/lib/test_emulate_step.c
index 42347067739c..3bc042e15d00 100644
--- a/arch/powerpc/lib/test_emulate_step.c
+++ b/arch/powerpc/lib/test_emulate_step.c
@@ -103,7 +103,7 @@ static void __init test_ld(void)
 	regs.gpr[3] = (unsigned long) &a;
 
 	/* ld r5, 0(r3) */
-	stepped = emulate_step(&regs, TEST_LD(5, 3, 0));
+	stepped = emulate_step(&regs, TEST_LD(5, 3, 0), PPC_NO_SUFFIX);
 
 	if (stepped == 1 && regs.gpr[5] == a)
 		show_result("ld", "PASS");
@@ -121,7 +121,7 @@ static void __init test_lwz(void)
 	regs.gpr[3] = (unsigned long) &a;
 
 	/* lwz r5, 0(r3) */
-	stepped = emulate_step(&regs, TEST_LWZ(5, 3, 0));
+	stepped = emulate_step(&regs, TEST_LWZ(5, 3, 0), PPC_NO_SUFFIX);
 
 	if (stepped == 1 && regs.gpr[5] == a)
 		show_result("lwz", "PASS");
@@ -141,7 +141,7 @@ static void __init test_lwzx(void)
 	regs.gpr[5] = 0x8765;
 
 	/* lwzx r5, r3, r4 */
-	stepped = emulate_step(&regs, TEST_LWZX(5, 3, 4));
+	stepped = emulate_step(&regs, TEST_LWZX(5, 3, 4), PPC_NO_SUFFIX);
 	if (stepped == 1 && regs.gpr[5] == a[2])
 		show_result("lwzx", "PASS");
 	else
@@ -159,7 +159,7 @@ static void __init test_std(void)
 	regs.gpr[5] = 0x5678;
 
 	/* std r5, 0(r3) */
-	stepped = emulate_step(&regs, TEST_STD(5, 3, 0));
+	stepped = emulate_step(&regs, TEST_STD(5, 3, 0), PPC_NO_SUFFIX);
 	if (stepped == 1 || regs.gpr[5] == a)
 		show_result("std", "PASS");
 	else
@@ -184,7 +184,7 @@ static void __init test_ldarx_stdcx(void)
 	regs.gpr[5] = 0x5678;
 
 	/* ldarx r5, r3, r4, 0 */
-	stepped = emulate_step(&regs, TEST_LDARX(5, 3, 4, 0));
+	stepped = emulate_step(&regs, TEST_LDARX(5, 3, 4, 0), PPC_NO_SUFFIX);
 
 	/*
 	 * Don't touch 'a' here. Touching 'a' can do Load/store
@@ -202,7 +202,7 @@ static void __init test_ldarx_stdcx(void)
 	regs.gpr[5] = 0x9ABC;
 
 	/* stdcx. r5, r3, r4 */
-	stepped = emulate_step(&regs, TEST_STDCX(5, 3, 4));
+	stepped = emulate_step(&regs, TEST_STDCX(5, 3, 4), PPC_NO_SUFFIX);
 
 	/*
 	 * Two possible scenarios that indicates successful emulation
@@ -242,7 +242,7 @@ static void __init test_lfsx_stfsx(void)
 	regs.gpr[4] = 0;
 
 	/* lfsx frt10, r3, r4 */
-	stepped = emulate_step(&regs, TEST_LFSX(10, 3, 4));
+	stepped = emulate_step(&regs, TEST_LFSX(10, 3, 4), PPC_NO_SUFFIX);
 
 	if (stepped == 1)
 		show_result("lfsx", "PASS");
@@ -255,7 +255,7 @@ static void __init test_lfsx_stfsx(void)
 	c.a = 678.91;
 
 	/* stfsx frs10, r3, r4 */
-	stepped = emulate_step(&regs, TEST_STFSX(10, 3, 4));
+	stepped = emulate_step(&regs, TEST_STFSX(10, 3, 4), PPC_NO_SUFFIX);
 
 	if (stepped == 1 && c.b == cached_b)
 		show_result("stfsx", "PASS");
@@ -285,7 +285,7 @@ static void __init test_lfdx_stfdx(void)
 	regs.gpr[4] = 0;
 
 	/* lfdx frt10, r3, r4 */
-	stepped = emulate_step(&regs, TEST_LFDX(10, 3, 4));
+	stepped = emulate_step(&regs, TEST_LFDX(10, 3, 4), PPC_NO_SUFFIX);
 
 	if (stepped == 1)
 		show_result("lfdx", "PASS");
@@ -298,7 +298,7 @@ static void __init test_lfdx_stfdx(void)
 	c.a = 987654.32;
 
 	/* stfdx frs10, r3, r4 */
-	stepped = emulate_step(&regs, TEST_STFDX(10, 3, 4));
+	stepped = emulate_step(&regs, TEST_STFDX(10, 3, 4), PPC_NO_SUFFIX);
 
 	if (stepped == 1 && c.b == cached_b)
 		show_result("stfdx", "PASS");
@@ -344,7 +344,7 @@ static void __init test_lvx_stvx(void)
 	regs.gpr[4] = 0;
 
 	/* lvx vrt10, r3, r4 */
-	stepped = emulate_step(&regs, TEST_LVX(10, 3, 4));
+	stepped = emulate_step(&regs, TEST_LVX(10, 3, 4), PPC_NO_SUFFIX);
 
 	if (stepped == 1)
 		show_result("lvx", "PASS");
@@ -360,7 +360,7 @@ static void __init test_lvx_stvx(void)
 	c.b[3] = 498532;
 
 	/* stvx vrs10, r3, r4 */
-	stepped = emulate_step(&regs, TEST_STVX(10, 3, 4));
+	stepped = emulate_step(&regs, TEST_STVX(10, 3, 4), PPC_NO_SUFFIX);
 
 	if (stepped == 1 && cached_b[0] == c.b[0] && cached_b[1] == c.b[1] &&
 	    cached_b[2] == c.b[2] && cached_b[3] == c.b[3])
@@ -401,7 +401,7 @@ static void __init test_lxvd2x_stxvd2x(void)
 	regs.gpr[4] = 0;
 
 	/* lxvd2x vsr39, r3, r4 */
-	stepped = emulate_step(&regs, TEST_LXVD2X(39, 3, 4));
+	stepped = emulate_step(&regs, TEST_LXVD2X(39, 3, 4), PPC_NO_SUFFIX);
 
 	if (stepped == 1 && cpu_has_feature(CPU_FTR_VSX)) {
 		show_result("lxvd2x", "PASS");
@@ -421,7 +421,7 @@ static void __init test_lxvd2x_stxvd2x(void)
 	c.b[3] = 4;
 
 	/* stxvd2x vsr39, r3, r4 */
-	stepped = emulate_step(&regs, TEST_STXVD2X(39, 3, 4));
+	stepped = emulate_step(&regs, TEST_STXVD2X(39, 3, 4), PPC_NO_SUFFIX);
 
 	if (stepped == 1 && cached_b[0] == c.b[0] && cached_b[1] == c.b[1] &&
 	    cached_b[2] == c.b[2] && cached_b[3] == c.b[3] &&
@@ -848,7 +848,7 @@ static int __init emulate_compute_instr(struct pt_regs *regs,
 	if (!regs || !instr)
 		return -EINVAL;
 
-	if (analyse_instr(&op, regs, instr) != 1 ||
+	if (analyse_instr(&op, regs, instr, PPC_NO_SUFFIX) != 1 ||
 	    GETTYPE(op.type) != COMPUTE) {
 		pr_info("emulation failed, instruction = 0x%08x\n", instr);
 		return -EFAULT;
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index e8c84d265602..897e512c6379 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -705,7 +705,8 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
 	if ((regs->msr & (MSR_IR|MSR_PR|MSR_64BIT)) == (MSR_IR|MSR_64BIT)) {
 		bp = at_breakpoint(regs->nip);
 		if (bp != NULL) {
-			int stepped = emulate_step(regs, bp->instr[0]);
+			int stepped = emulate_step(regs, bp->instr[0],
+						   PPC_NO_SUFFIX);
 			if (stepped == 0) {
 				regs->nip = (unsigned long) &bp->instr[0];
 				atomic_inc(&bp->ref_count);
@@ -1170,7 +1171,7 @@ static int do_step(struct pt_regs *regs)
 	/* check we are in 64-bit kernel mode, translation enabled */
 	if ((regs->msr & (MSR_64BIT|MSR_PR|MSR_IR)) == (MSR_64BIT|MSR_IR)) {
 		if (mread(regs->nip, &instr, 4) == 4) {
-			stepped = emulate_step(regs, instr);
+			stepped = emulate_step(regs, instr, PPC_NO_SUFFIX);
 			if (stepped < 0) {
 				printf("Couldn't single-step %s instruction\n",
 				       (IS_RFID(instr)? "rfid": "mtmsrd"));
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 04/13] powerpc sstep: Add support for prefixed load/stores
From: Jordan Niethe @ 2020-02-11  5:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, mpe, Jordan Niethe, dja, bala24
In-Reply-To: <20200211053355.21574-1-jniethe5@gmail.com>

This adds emulation support for the following prefixed integer
load/stores:
  * Prefixed Load Byte and Zero (plbz)
  * Prefixed Load Halfword and Zero (plhz)
  * Prefixed Load Halfword Algebraic (plha)
  * Prefixed Load Word and Zero (plwz)
  * Prefixed Load Word Algebraic (plwa)
  * Prefixed Load Doubleword (pld)
  * Prefixed Store Byte (pstb)
  * Prefixed Store Halfword (psth)
  * Prefixed Store Word (pstw)
  * Prefixed Store Doubleword (pstd)
  * Prefixed Load Quadword (plq)
  * Prefixed Store Quadword (pstq)

the follow prefixed floating-point load/stores:
  * Prefixed Load Floating-Point Single (plfs)
  * Prefixed Load Floating-Point Double (plfd)
  * Prefixed Store Floating-Point Single (pstfs)
  * Prefixed Store Floating-Point Double (pstfd)

and for the following prefixed VSX load/stores:
  * Prefixed Load VSX Scalar Doubleword (plxsd)
  * Prefixed Load VSX Scalar Single-Precision (plxssp)
  * Prefixed Load VSX Vector [0|1]  (plxv, plxv0, plxv1)
  * Prefixed Store VSX Scalar Doubleword (pstxsd)
  * Prefixed Store VSX Scalar Single-Precision (pstxssp)
  * Prefixed Store VSX Vector [0|1] (pstxv, pstxv0, pstxv1)

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: - Combine all load/store patches
    - Fix the name of Type 01 instructions
    - Remove sign extension flag from pstd/pld
    - Rename sufx -> suffix
---
 arch/powerpc/lib/sstep.c | 165 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 165 insertions(+)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 65143ab1bf64..0e21c21ff2be 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -187,6 +187,44 @@ static nokprobe_inline unsigned long xform_ea(unsigned int instr,
 	return ea;
 }
 
+/*
+ * Calculate effective address for a MLS:D-form / 8LS:D-form
+ * prefixed instruction
+ */
+static nokprobe_inline unsigned long mlsd_8lsd_ea(unsigned int instr,
+						  unsigned int suffix,
+						  const struct pt_regs *regs)
+{
+	int ra, prefix_r;
+	unsigned int  dd;
+	unsigned long ea, d0, d1, d;
+
+	prefix_r = instr & (1ul << 20);
+	ra = (suffix >> 16) & 0x1f;
+
+	d0 = instr & 0x3ffff;
+	d1 = suffix & 0xffff;
+	d = (d0 << 16) | d1;
+
+	/*
+	 * sign extend a 34 bit number
+	 */
+	dd = (unsigned int) (d >> 2);
+	ea = (signed int) dd;
+	ea = (ea << 2) | (d & 0x3);
+
+	if (!prefix_r && ra)
+		ea += regs->gpr[ra];
+	else if (!prefix_r && !ra)
+		; /* Leave ea as is */
+	else if (prefix_r && !ra)
+		ea += regs->nip;
+	else if (prefix_r && ra)
+		; /* Invalid form. Should already be checked for by caller! */
+
+	return ea;
+}
+
 /*
  * Return the largest power of 2, not greater than sizeof(unsigned long),
  * such that x is a multiple of it.
@@ -1166,6 +1204,7 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 		  unsigned int instr, unsigned int suffix)
 {
 	unsigned int opcode, ra, rb, rc, rd, spr, u;
+	unsigned int suffixopcode, prefixtype, prefix_r;
 	unsigned long int imm;
 	unsigned long int val, val2;
 	unsigned int mb, me, sh;
@@ -2652,6 +2691,132 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 
 	}
 
+/*
+ * Prefixed instructions
+ */
+	switch (opcode) {
+	case 1:
+		prefix_r = instr & (1ul << 20);
+		ra = (suffix >> 16) & 0x1f;
+		op->update_reg = ra;
+		rd = (suffix >> 21) & 0x1f;
+		op->reg = rd;
+		op->val = regs->gpr[rd];
+
+		suffixopcode = suffix >> 26;
+		prefixtype = (instr >> 24) & 0x3;
+		switch (prefixtype) {
+		case 0: /* Type 00  Eight-Byte Load/Store */
+			if (prefix_r && ra)
+				break;
+			op->ea = mlsd_8lsd_ea(instr, suffix, regs);
+			switch (suffixopcode) {
+			case 41:	/* plwa */
+				op->type = MKOP(LOAD, PREFIXED | SIGNEXT, 4);
+				break;
+			case 42:        /* plxsd */
+				op->reg = rd + 32;
+				op->type = MKOP(LOAD_VSX, PREFIXED, 8);
+				op->element_size = 8;
+				op->vsx_flags = VSX_CHECK_VEC;
+				break;
+			case 43:	/* plxssp */
+				op->reg = rd + 32;
+				op->type = MKOP(LOAD_VSX, PREFIXED, 4);
+				op->element_size = 8;
+				op->vsx_flags = VSX_FPCONV | VSX_CHECK_VEC;
+				break;
+			case 46:	/* pstxsd */
+				op->reg = rd + 32;
+				op->type = MKOP(STORE_VSX, PREFIXED, 8);
+				op->element_size = 8;
+				op->vsx_flags = VSX_CHECK_VEC;
+				break;
+			case 47:	/* pstxssp */
+				op->reg = rd + 32;
+				op->type = MKOP(STORE_VSX, PREFIXED, 4);
+				op->element_size = 8;
+				op->vsx_flags = VSX_FPCONV | VSX_CHECK_VEC;
+				break;
+			case 51:	/* plxv1 */
+				op->reg += 32;
+
+				/* fallthru */
+			case 50:	/* plxv0 */
+				op->type = MKOP(LOAD_VSX, PREFIXED, 16);
+				op->element_size = 16;
+				op->vsx_flags = VSX_CHECK_VEC;
+				break;
+			case 55:	/* pstxv1 */
+				op->reg = rd + 32;
+
+				/* fallthru */
+			case 54:	/* pstxv0 */
+				op->type = MKOP(STORE_VSX, PREFIXED, 16);
+				op->element_size = 16;
+				op->vsx_flags = VSX_CHECK_VEC;
+				break;
+			case 56:        /* plq */
+				op->type = MKOP(LOAD, PREFIXED, 16);
+				break;
+			case 57:	/* pld */
+				op->type = MKOP(LOAD, PREFIXED, 8);
+				break;
+			case 60:        /* stq */
+				op->type = MKOP(STORE, PREFIXED, 16);
+				break;
+			case 61:	/* pstd */
+				op->type = MKOP(STORE, PREFIXED, 8);
+				break;
+			}
+			break;
+		case 1: /* Type 01 Eight-Byte Register-to-Register */
+			break;
+		case 2: /* Type 10 Modified Load/Store */
+			if (prefix_r && ra)
+				break;
+			op->ea = mlsd_8lsd_ea(instr, suffix, regs);
+			switch (suffixopcode) {
+			case 32:	/* plwz */
+				op->type = MKOP(LOAD, PREFIXED, 4);
+				break;
+			case 34:	/* plbz */
+				op->type = MKOP(LOAD, PREFIXED, 1);
+				break;
+			case 36:	/* pstw */
+				op->type = MKOP(STORE, PREFIXED, 4);
+				break;
+			case 38:	/* pstb */
+				op->type = MKOP(STORE, PREFIXED, 1);
+				break;
+			case 40:	/* plhz */
+				op->type = MKOP(LOAD, PREFIXED, 2);
+				break;
+			case 42:	/* plha */
+				op->type = MKOP(LOAD, PREFIXED | SIGNEXT, 2);
+				break;
+			case 44:	/* psth */
+				op->type = MKOP(STORE, PREFIXED, 2);
+				break;
+			case 48:        /* plfs */
+				op->type = MKOP(LOAD_FP, PREFIXED | FPCONV, 4);
+				break;
+			case 50:        /* plfd */
+				op->type = MKOP(LOAD_FP, PREFIXED, 8);
+				break;
+			case 52:        /* pstfs */
+				op->type = MKOP(STORE_FP, PREFIXED | FPCONV, 4);
+				break;
+			case 54:        /* pstfd */
+				op->type = MKOP(STORE_FP, PREFIXED, 8);
+				break;
+			}
+			break;
+		case 3: /* Type 11 Modified Register-to-Register */
+			break;
+		}
+	}
+
 #ifdef CONFIG_VSX
 	if ((GETTYPE(op->type) == LOAD_VSX ||
 	     GETTYPE(op->type) == STORE_VSX) &&
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 05/13] powerpc sstep: Add support for prefixed fixed-point arithmetic
From: Jordan Niethe @ 2020-02-11  5:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, mpe, Jordan Niethe, dja, bala24
In-Reply-To: <20200211053355.21574-1-jniethe5@gmail.com>

This adds emulation support for the following prefixed Fixed-Point
Arithmetic instructions:
  * Prefixed Add Immediate (paddi)

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
 arch/powerpc/lib/sstep.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 0e21c21ff2be..8ba74c10bc03 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -2777,6 +2777,10 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 				break;
 			op->ea = mlsd_8lsd_ea(instr, suffix, regs);
 			switch (suffixopcode) {
+			case 14:	/* paddi */
+				op->type = COMPUTE | PREFIXED;
+				op->val = op->ea;
+				goto compute_done;
 			case 32:	/* plwz */
 				op->type = MKOP(LOAD, PREFIXED, 4);
 				break;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 06/13] powerpc: Support prefixed instructions in alignment handler
From: Jordan Niethe @ 2020-02-11  5:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, mpe, Jordan Niethe, dja, bala24
In-Reply-To: <20200211053355.21574-1-jniethe5@gmail.com>

Alignment interrupts can be caused by prefixed instructions accessing
memory. In the alignment handler the instruction that caused the
exception is loaded and attempted emulate. If the instruction is a
prefixed instruction load the prefix and suffix to emulate. After
emulating increment the NIP by 8.

Prefixed instructions are not permitted to cross 64-byte boundaries. If
they do the alignment interrupt is invoked with SRR1 BOUNDARY bit set.
If this occurs send a SIGBUS to the offending process if in user mode.
If in kernel mode call bad_page_fault().

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: - Move __get_user_instr() and __get_user_instr_inatomic() to this
commit (previously in "powerpc sstep: Prepare to support prefixed
instructions").
    - Rename sufx to suffix
    - Use a macro for calculating instruction length
---
 arch/powerpc/include/asm/uaccess.h | 30 ++++++++++++++++++++++++++++++
 arch/powerpc/kernel/align.c        |  8 +++++---
 arch/powerpc/kernel/traps.c        | 21 ++++++++++++++++++++-
 3 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 2f500debae21..30f63a81c8d8 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -474,4 +474,34 @@ static __must_check inline bool user_access_begin(const void __user *ptr, size_t
 #define unsafe_copy_to_user(d, s, l, e) \
 	unsafe_op_wrap(raw_copy_to_user_allowed(d, s, l), e)
 
+/*
+ * When reading an instruction iff it is a prefix, the suffix needs to be also
+ * loaded.
+ */
+#define __get_user_instr(x, y, ptr)			\
+({							\
+	long __gui_ret = 0;				\
+	y = 0;						\
+	__gui_ret = __get_user(x, ptr);			\
+	if (!__gui_ret) {				\
+		if (IS_PREFIX(x))			\
+			__gui_ret = __get_user(y, ptr + 1);	\
+	}						\
+							\
+	__gui_ret;					\
+})
+
+#define __get_user_instr_inatomic(x, y, ptr)		\
+({							\
+	long __gui_ret = 0;				\
+	y = 0;						\
+	__gui_ret = __get_user_inatomic(x, ptr);	\
+	if (!__gui_ret) {				\
+		if (IS_PREFIX(x))			\
+			__gui_ret = __get_user_inatomic(y, ptr + 1);	\
+	}						\
+							\
+	__gui_ret;					\
+})
+
 #endif	/* _ARCH_POWERPC_UACCESS_H */
diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index ba3bf5c3ab62..e42cfaa616d3 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -293,7 +293,7 @@ static int emulate_spe(struct pt_regs *regs, unsigned int reg,
 
 int fix_alignment(struct pt_regs *regs)
 {
-	unsigned int instr;
+	unsigned int instr, suffix;
 	struct instruction_op op;
 	int r, type;
 
@@ -303,13 +303,15 @@ int fix_alignment(struct pt_regs *regs)
 	 */
 	CHECK_FULL_REGS(regs);
 
-	if (unlikely(__get_user(instr, (unsigned int __user *)regs->nip)))
+	if (unlikely(__get_user_instr(instr, suffix,
+				 (unsigned int __user *)regs->nip)))
 		return -EFAULT;
 	if ((regs->msr & MSR_LE) != (MSR_KERNEL & MSR_LE)) {
 		/* We don't handle PPC little-endian any more... */
 		if (cpu_has_feature(CPU_FTR_PPC_LE))
 			return -EIO;
 		instr = swab32(instr);
+		suffix = swab32(suffix);
 	}
 
 #ifdef CONFIG_SPE
@@ -334,7 +336,7 @@ int fix_alignment(struct pt_regs *regs)
 	if ((instr & 0xfc0006fe) == (PPC_INST_COPY & 0xfc0006fe))
 		return -EIO;
 
-	r = analyse_instr(&op, regs, instr, PPC_NO_SUFFIX);
+	r = analyse_instr(&op, regs, instr, suffix);
 	if (r < 0)
 		return -EINVAL;
 
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 82a3438300fd..d80b82fc1ae3 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -583,6 +583,10 @@ static inline int check_io_access(struct pt_regs *regs)
 #define REASON_ILLEGAL		(ESR_PIL | ESR_PUO)
 #define REASON_PRIVILEGED	ESR_PPR
 #define REASON_TRAP		ESR_PTR
+#define REASON_PREFIXED		0
+#define REASON_BOUNDARY		0
+
+#define inst_length(reason)	4
 
 /* single-step stuff */
 #define single_stepping(regs)	(current->thread.debug.dbcr0 & DBCR0_IC)
@@ -597,6 +601,10 @@ static inline int check_io_access(struct pt_regs *regs)
 #define REASON_ILLEGAL		SRR1_PROGILL
 #define REASON_PRIVILEGED	SRR1_PROGPRIV
 #define REASON_TRAP		SRR1_PROGTRAP
+#define REASON_PREFIXED		SRR1_PREFIXED
+#define REASON_BOUNDARY		SRR1_BOUNDARY
+
+#define inst_length(reason)	(((reason) & REASON_PREFIXED) ? 8 : 4)
 
 #define single_stepping(regs)	((regs)->msr & MSR_SE)
 #define clear_single_step(regs)	((regs)->msr &= ~MSR_SE)
@@ -1593,11 +1601,20 @@ void alignment_exception(struct pt_regs *regs)
 {
 	enum ctx_state prev_state = exception_enter();
 	int sig, code, fixed = 0;
+	unsigned long  reason;
 
 	/* We restore the interrupt state now */
 	if (!arch_irq_disabled_regs(regs))
 		local_irq_enable();
 
+	reason = get_reason(regs);
+
+	if (reason & REASON_BOUNDARY) {
+		sig = SIGBUS;
+		code = BUS_ADRALN;
+		goto bad;
+	}
+
 	if (tm_abort_check(regs, TM_CAUSE_ALIGNMENT | TM_CAUSE_PERSISTENT))
 		goto bail;
 
@@ -1606,7 +1623,8 @@ void alignment_exception(struct pt_regs *regs)
 		fixed = fix_alignment(regs);
 
 	if (fixed == 1) {
-		regs->nip += 4;	/* skip over emulated instruction */
+		/* skip over emulated instruction */
+		regs->nip += inst_length(reason);
 		emulate_single_step(regs);
 		goto bail;
 	}
@@ -1619,6 +1637,7 @@ void alignment_exception(struct pt_regs *regs)
 		sig = SIGBUS;
 		code = BUS_ADRALN;
 	}
+bad:
 	if (user_mode(regs))
 		_exception(sig, regs, code, regs->dar);
 	else
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 07/13] powerpc/traps: Check for prefixed instructions in facility_unavailable_exception()
From: Jordan Niethe @ 2020-02-11  5:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, mpe, Jordan Niethe, dja, bala24
In-Reply-To: <20200211053355.21574-1-jniethe5@gmail.com>

If prefixed instructions are made unavailable by the [H]FSCR, attempting
to use them will cause a facility unavailable exception. Add "PREFIX" to
the facility_strings[].

Currently there are no prefixed instructions that are actually emulated
by emulate_instruction() within facility_unavailable_exception().
However, when caused by a prefixed instructions the SRR1 PREFIXED bit is
set. Prepare for dealing with emulated prefixed instructions by checking
for this bit.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
 arch/powerpc/kernel/traps.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index d80b82fc1ae3..cd8b3043c268 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1739,6 +1739,7 @@ void facility_unavailable_exception(struct pt_regs *regs)
 		[FSCR_TAR_LG] = "TAR",
 		[FSCR_MSGP_LG] = "MSGP",
 		[FSCR_SCV_LG] = "SCV",
+		[FSCR_PREFIX_LG] = "PREFIX",
 	};
 	char *facility = "unknown";
 	u64 value;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 08/13] powerpc/xmon: Add initial support for prefixed instructions
From: Jordan Niethe @ 2020-02-11  5:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, mpe, Jordan Niethe, dja, bala24
In-Reply-To: <20200211053355.21574-1-jniethe5@gmail.com>

A prefixed instruction is composed of a word prefix and a word suffix.
It does not make sense to be able to have a breakpoint on the suffix of
a prefixed instruction, so make this impossible.

When leaving xmon_core() we check to see if we are currently at a
breakpoint. If this is the case, the breakpoint needs to be proceeded
from. Initially emulate_step() is tried, but if this fails then we need
to execute the saved instruction out of line. The NIP is set to the
address of bpt::instr[] for the current breakpoint.  bpt::instr[]
contains the instruction replaced by the breakpoint, followed by a trap
instruction.  After bpt::instr[0] is executed and we hit the trap we
enter back into xmon_bpt(). We know that if we got here and the offset
indicates we are at bpt::instr[1] then we have just executed out of line
so we can put the NIP back to the instruction after the breakpoint
location and continue on.

Adding prefixed instructions complicates this as the bpt::instr[1] needs
to be used to hold the suffix. To deal with this make bpt::instr[] big
enough for three word instructions.  bpt::instr[2] contains the trap,
and in the case of word instructions pad bpt::instr[1] with a noop.

No support for disassembling prefixed instructions.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: Rename sufx to suffix
---
 arch/powerpc/xmon/xmon.c | 82 ++++++++++++++++++++++++++++++++++------
 1 file changed, 71 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 897e512c6379..0b085642bbe7 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -97,7 +97,8 @@ static long *xmon_fault_jmp[NR_CPUS];
 /* Breakpoint stuff */
 struct bpt {
 	unsigned long	address;
-	unsigned int	instr[2];
+	/* Prefixed instructions can not cross 64-byte boundaries */
+	unsigned int	instr[3] __aligned(64);
 	atomic_t	ref_count;
 	int		enabled;
 	unsigned long	pad;
@@ -113,6 +114,7 @@ static struct bpt bpts[NBPTS];
 static struct bpt dabr;
 static struct bpt *iabr;
 static unsigned bpinstr = 0x7fe00008;	/* trap */
+static unsigned nopinstr = 0x60000000;	/* nop */
 
 #define BP_NUM(bp)	((bp) - bpts + 1)
 
@@ -120,6 +122,7 @@ static unsigned bpinstr = 0x7fe00008;	/* trap */
 static int cmds(struct pt_regs *);
 static int mread(unsigned long, void *, int);
 static int mwrite(unsigned long, void *, int);
+static int read_instr(unsigned long, unsigned int *, unsigned int *);
 static int handle_fault(struct pt_regs *);
 static void byterev(unsigned char *, int);
 static void memex(void);
@@ -706,7 +709,7 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
 		bp = at_breakpoint(regs->nip);
 		if (bp != NULL) {
 			int stepped = emulate_step(regs, bp->instr[0],
-						   PPC_NO_SUFFIX);
+						   bp->instr[1]);
 			if (stepped == 0) {
 				regs->nip = (unsigned long) &bp->instr[0];
 				atomic_inc(&bp->ref_count);
@@ -761,8 +764,8 @@ static int xmon_bpt(struct pt_regs *regs)
 
 	/* Are we at the trap at bp->instr[1] for some bp? */
 	bp = in_breakpoint_table(regs->nip, &offset);
-	if (bp != NULL && offset == 4) {
-		regs->nip = bp->address + 4;
+	if (bp != NULL && (offset == 4 || offset == 8)) {
+		regs->nip = bp->address + offset;
 		atomic_dec(&bp->ref_count);
 		return 1;
 	}
@@ -864,7 +867,8 @@ static struct bpt *in_breakpoint_table(unsigned long nip, unsigned long *offp)
 		return NULL;
 	off %= sizeof(struct bpt);
 	if (off != offsetof(struct bpt, instr[0])
-	    && off != offsetof(struct bpt, instr[1]))
+	    && off != offsetof(struct bpt, instr[1])
+	    && off != offsetof(struct bpt, instr[2]))
 		return NULL;
 	*offp = off - offsetof(struct bpt, instr[0]);
 	return (struct bpt *) (nip - off);
@@ -881,9 +885,18 @@ static struct bpt *new_breakpoint(unsigned long a)
 
 	for (bp = bpts; bp < &bpts[NBPTS]; ++bp) {
 		if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
+			/*
+			 * Prefixed instructions are two words, but regular
+			 * instructions are only one. Use a nop to pad out the
+			 * regular instructions so that we can place the trap
+			 * at the same plac. For prefixed instructions the nop
+			 * will get overwritten during insert_bpts().
+			 */
 			bp->address = a;
-			bp->instr[1] = bpinstr;
+			bp->instr[1] = nopinstr;
 			store_inst(&bp->instr[1]);
+			bp->instr[2] = bpinstr;
+			store_inst(&bp->instr[2]);
 			return bp;
 		}
 	}
@@ -895,13 +908,15 @@ static struct bpt *new_breakpoint(unsigned long a)
 static void insert_bpts(void)
 {
 	int i;
-	struct bpt *bp;
+	unsigned int prefix;
+	struct bpt *bp, *bp2;
 
 	bp = bpts;
 	for (i = 0; i < NBPTS; ++i, ++bp) {
 		if ((bp->enabled & (BP_TRAP|BP_CIABR)) == 0)
 			continue;
-		if (mread(bp->address, &bp->instr[0], 4) != 4) {
+		if (!read_instr(bp->address, &bp->instr[0],
+			       &bp->instr[1])) {
 			printf("Couldn't read instruction at %lx, "
 			       "disabling breakpoint there\n", bp->address);
 			bp->enabled = 0;
@@ -913,7 +928,34 @@ static void insert_bpts(void)
 			bp->enabled = 0;
 			continue;
 		}
+		/*
+		 * Check the address is not a suffix by looking for a prefix in
+		 * front of it.
+		 */
+		if ((mread(bp->address - 4, &prefix, 4) == 4) &&
+		    IS_PREFIX(prefix)) {
+			printf("Breakpoint at %lx is on the second word of a "
+			       "prefixed instruction, disabling it\n",
+			       bp->address);
+			bp->enabled = 0;
+			continue;
+		}
+		/*
+		 * We might still be a suffix - if the prefix has already been
+		 * replaced by a breakpoint we won't catch it with the above
+		 * test.
+		 */
+		bp2 = at_breakpoint(bp->address - 4);
+		if (bp2 && IS_PREFIX(bp2->instr[0])) {
+			printf("Breakpoint at %lx is on the second word of a "
+			       "prefixed instruction, disabling it\n",
+			       bp->address);
+			bp->enabled = 0;
+			continue;
+		}
 		store_inst(&bp->instr[0]);
+		if (IS_PREFIX(bp->instr[0]))
+			store_inst(&bp->instr[1]);
 		if (bp->enabled & BP_CIABR)
 			continue;
 		if (patch_instruction((unsigned int *)bp->address,
@@ -1164,14 +1206,14 @@ static int do_step(struct pt_regs *regs)
  */
 static int do_step(struct pt_regs *regs)
 {
-	unsigned int instr;
+	unsigned int instr, suffix;
 	int stepped;
 
 	force_enable_xmon();
 	/* check we are in 64-bit kernel mode, translation enabled */
 	if ((regs->msr & (MSR_64BIT|MSR_PR|MSR_IR)) == (MSR_64BIT|MSR_IR)) {
-		if (mread(regs->nip, &instr, 4) == 4) {
-			stepped = emulate_step(regs, instr, PPC_NO_SUFFIX);
+		if (read_instr(regs->nip, &instr, &suffix)) {
+			stepped = emulate_step(regs, instr, suffix);
 			if (stepped < 0) {
 				printf("Couldn't single-step %s instruction\n",
 				       (IS_RFID(instr)? "rfid": "mtmsrd"));
@@ -2130,6 +2172,24 @@ mwrite(unsigned long adrs, void *buf, int size)
 	return n;
 }
 
+static int read_instr(unsigned long addr, unsigned int *instr,
+		      unsigned int *suffix)
+{
+	int r;
+
+	r = mread(addr, instr, 4);
+	if (r != 4)
+		return 0;
+	if (!IS_PREFIX(*instr))
+		return 4;
+	r = mread(addr + 4, suffix, 4);
+	if (r != 4)
+		return 0;
+
+	return 8;
+}
+
+
 static int fault_type;
 static int fault_except;
 static char *fault_chars[] = { "--", "**", "##" };
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 09/13] powerpc/xmon: Dump prefixed instructions
From: Jordan Niethe @ 2020-02-11  5:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, mpe, Jordan Niethe, dja, bala24
In-Reply-To: <20200211053355.21574-1-jniethe5@gmail.com>

Currently when xmon is dumping instructions it reads a word at a time
and then prints that instruction (either as a hex number or by
disassembling it). For prefixed instructions it would be nice to show
its prefix and suffix as together. Use read_instr() so that if a prefix
is encountered its suffix is loaded too. Then print these in the form:
    prefix:suffix
Xmon uses the disassembly routines from GNU binutils. These currently do
not support prefixed instructions so we will not disassemble the
prefixed instructions yet.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: Rename sufx to suffix
---
 arch/powerpc/xmon/xmon.c | 50 +++++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 0b085642bbe7..513901ee18b0 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -2903,6 +2903,21 @@ prdump(unsigned long adrs, long ndump)
 	}
 }
 
+static bool instrs_are_equal(unsigned long insta, unsigned long suffixa,
+			     unsigned long instb, unsigned long suffixb)
+{
+	if (insta != instb)
+		return false;
+
+	if (!IS_PREFIX(insta) && !IS_PREFIX(instb))
+		return true;
+
+	if (IS_PREFIX(insta) && IS_PREFIX(instb))
+		return suffixa == suffixb;
+
+	return false;
+}
+
 typedef int (*instruction_dump_func)(unsigned long inst, unsigned long addr);
 
 static int
@@ -2911,12 +2926,11 @@ generic_inst_dump(unsigned long adr, long count, int praddr,
 {
 	int nr, dotted;
 	unsigned long first_adr;
-	unsigned int inst, last_inst = 0;
-	unsigned char val[4];
+	unsigned int inst, suffix, last_inst = 0, last_suffix = 0;
 
 	dotted = 0;
-	for (first_adr = adr; count > 0; --count, adr += 4) {
-		nr = mread(adr, val, 4);
+	for (first_adr = adr; count > 0; --count, adr += nr) {
+		nr = read_instr(adr, &inst, &suffix);
 		if (nr == 0) {
 			if (praddr) {
 				const char *x = fault_chars[fault_type];
@@ -2924,8 +2938,9 @@ generic_inst_dump(unsigned long adr, long count, int praddr,
 			}
 			break;
 		}
-		inst = GETWORD(val);
-		if (adr > first_adr && inst == last_inst) {
+		if (adr > first_adr && instrs_are_equal(inst, suffix,
+							last_inst,
+							last_suffix)) {
 			if (!dotted) {
 				printf(" ...\n");
 				dotted = 1;
@@ -2934,11 +2949,24 @@ generic_inst_dump(unsigned long adr, long count, int praddr,
 		}
 		dotted = 0;
 		last_inst = inst;
-		if (praddr)
-			printf(REG"  %.8x", adr, inst);
-		printf("\t");
-		dump_func(inst, adr);
-		printf("\n");
+		last_suffix = suffix;
+		if (IS_PREFIX(inst)) {
+			if (praddr)
+				printf(REG"  %.8x:%.8x", adr, inst, suffix);
+			printf("\t");
+			/*
+			 * Just use this until binutils ppc disassembly
+			 * prints prefixed instructions.
+			 */
+			printf("%.8x:%.8x", inst, suffix);
+			printf("\n");
+		} else {
+			if (praddr)
+				printf(REG"  %.8x", adr, inst);
+			printf("\t");
+			dump_func(inst, adr);
+			printf("\n");
+		}
 	}
 	return adr - first_adr;
 }
-- 
2.17.1


^ permalink raw reply related


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