* Re: [PATCH v2 06/27] ocxl: Tally up the LPC memory on a link & allow it to be mapped
From: Alastair D'Silva @ 2020-02-19 0:01 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Paul Mackerras, Mauro Carvalho Chehab, Ira Weiny, Thomas Gleixner,
Rob Herring, Dave Jiang, linux-nvdimm, Vishal Verma,
Krzysztof Kozlowski, Anju T Sudhakar, Mahesh Salgaonkar,
Andrew Donnellan, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Frederic Barrat, Andrew Morton,
linuxppc-dev, David S. Miller
In-Reply-To: <20200203123712.0000461a@Huawei.com>
On Mon, 2020-02-03 at 12:37 +0000, Jonathan Cameron wrote:
> On Tue, 3 Dec 2019 14:46:34 +1100
> Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > Tally up the LPC memory on an OpenCAPI link & allow it to be mapped
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Hi Alastair,
>
> A few trivial comments inline.
>
> Jonathan
>
> > ---
> > drivers/misc/ocxl/core.c | 10 ++++++
> > drivers/misc/ocxl/link.c | 60
> > +++++++++++++++++++++++++++++++
> > drivers/misc/ocxl/ocxl_internal.h | 33 +++++++++++++++++
> > 3 files changed, 103 insertions(+)
> >
> > diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
> > index b7a09b21ab36..2531c6cf19a0 100644
> > --- a/drivers/misc/ocxl/core.c
> > +++ b/drivers/misc/ocxl/core.c
> > @@ -230,8 +230,18 @@ static int configure_afu(struct ocxl_afu *afu,
> > u8 afu_idx, struct pci_dev *dev)
> > if (rc)
> > goto err_free_pasid;
> >
> > + if (afu->config.lpc_mem_size || afu-
> > >config.special_purpose_mem_size) {
> > + rc = ocxl_link_add_lpc_mem(afu->fn->link, afu-
> > >config.lpc_mem_offset,
> > + afu->config.lpc_mem_size +
> > + afu-
> > >config.special_purpose_mem_size);
> > + if (rc)
> > + goto err_free_mmio;
> > + }
> > +
> > return 0;
> >
> > +err_free_mmio:
> > + unmap_mmio_areas(afu);
> > err_free_pasid:
> > reclaim_afu_pasid(afu);
> > err_free_actag:
> > diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> > index 58d111afd9f6..d8503f0dc6ec 100644
> > --- a/drivers/misc/ocxl/link.c
> > +++ b/drivers/misc/ocxl/link.c
> > @@ -84,6 +84,11 @@ struct ocxl_link {
> > int dev;
> > atomic_t irq_available;
> > struct spa *spa;
> > + struct mutex lpc_mem_lock;
>
> Always a good idea to explicitly document what a lock is intended to
> protect.
>
Ok
> > + u64 lpc_mem_sz; /* Total amount of LPC memory presented on the
> > link */
> > + u64 lpc_mem;
> > + int lpc_consumers;
> > +
> > void *platform_data;
> > };
> > static struct list_head links_list = LIST_HEAD_INIT(links_list);
> > @@ -396,6 +401,8 @@ static int alloc_link(struct pci_dev *dev, int
> > PE_mask, struct ocxl_link **out_l
> > if (rc)
> > goto err_spa;
> >
> > + mutex_init(&link->lpc_mem_lock);
> > +
> > /* platform specific hook */
> > rc = pnv_ocxl_spa_setup(dev, link->spa->spa_mem, PE_mask,
> > &link->platform_data);
> > @@ -711,3 +718,56 @@ void ocxl_link_free_irq(void *link_handle, int
> > hw_irq)
> > atomic_inc(&link->irq_available);
> > }
> > EXPORT_SYMBOL_GPL(ocxl_link_free_irq);
> > +
> > +int ocxl_link_add_lpc_mem(void *link_handle, u64 offset, u64 size)
> > +{
> > + struct ocxl_link *link = (struct ocxl_link *) link_handle;
> > +
> > + // Check for overflow
>
> Stray c++ style comment.
>
This is permitted in powerpc.
> > + if (offset > (offset + size))
> > + return -EINVAL;
> > +
> > + mutex_lock(&link->lpc_mem_lock);
> > + link->lpc_mem_sz = max(link->lpc_mem_sz, offset + size);
> > +
> > + mutex_unlock(&link->lpc_mem_lock);
> > +
> > + return 0;
> > +}
> > +
> > +u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
> > +{
> > + struct ocxl_link *link = (struct ocxl_link *) link_handle;
> > + u64 lpc_mem;
> > +
> > + mutex_lock(&link->lpc_mem_lock);
> > + if (link->lpc_mem) {
>
> If you don't modify this later in the series (I haven't read it all
> yet :),
> it rather feels like it would be more compact and just as readable as
> something like...
>
> if (!link->lpc_mem)
> link->lpc_mem = pnv_ocxl...
>
> if (link->lpc_mem)
> link->lpc_consumers++;
> mutex_unlock(&link->lpc_mem_lock);
>
> return link->lpc_mem;
>
Agreed, thanks.
> > + lpc_mem = link->lpc_mem;
> > +
> > + link->lpc_consumers++;
> > + mutex_unlock(&link->lpc_mem_lock);
> > + return lpc_mem;
> > + }
> > +
> > + link->lpc_mem = pnv_ocxl_platform_lpc_setup(pdev, link-
> > >lpc_mem_sz);
> > + if (link->lpc_mem)
> > + link->lpc_consumers++;
> > + lpc_mem = link->lpc_mem;
> > + mutex_unlock(&link->lpc_mem_lock);
> > +
> > + return lpc_mem;
> > +}
> > +
> > +void ocxl_link_lpc_release(void *link_handle, struct pci_dev
> > *pdev)
> > +{
> > + struct ocxl_link *link = (struct ocxl_link *) link_handle;
> > +
> > + mutex_lock(&link->lpc_mem_lock);
> > + WARN_ON(--link->lpc_consumers < 0);
> > + if (link->lpc_consumers == 0) {
> > + pnv_ocxl_platform_lpc_release(pdev);
> > + link->lpc_mem = 0;
> > + }
> > +
> > + mutex_unlock(&link->lpc_mem_lock);
> > +}
> > diff --git a/drivers/misc/ocxl/ocxl_internal.h
> > b/drivers/misc/ocxl/ocxl_internal.h
> > index 97415afd79f3..20b417e00949 100644
> > --- a/drivers/misc/ocxl/ocxl_internal.h
> > +++ b/drivers/misc/ocxl/ocxl_internal.h
> > @@ -141,4 +141,37 @@ int ocxl_irq_offset_to_id(struct ocxl_context
> > *ctx, u64 offset);
> > u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id);
> > void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
> >
> > +/**
> > + * ocxl_link_add_lpc_mem() - Increment the amount of memory
> > required by an OpenCAPI link
> > + *
> > + * @link_handle: The OpenCAPI link handle
> > + * @offset: The offset of the memory to add
> > + * @size: The amount of memory to increment by
> > + *
> > + * Return 0 on success, negative on overflow
> > + */
> > +int ocxl_link_add_lpc_mem(void *link_handle, u64 offset, u64
> > size);
> > +
> > +/**
> > + * ocxl_link_lpc_map() - Map the LPC memory for an OpenCAPI device
> > + *
> > + * Since LPC memory belongs to a link, the whole LPC memory
> > available
> > + * on the link bust be mapped in order to make it accessible to a
> > device.
>
> must
Whoops :)
>
> > + *
> > + * @link_handle: The OpenCAPI link handle
> > + * @pdev: A device that is on the link
> > + */
> > +u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
> > +
> > +/**
> > + * ocxl_link_lpc_release() - Release the LPC memory device for an
> > OpenCAPI device
> > + *
> > + * Offlines LPC memory on an OpenCAPI link for a device. If this
> > is the
> > + * last device on the link to release the memory, unmap it from
> > the link.
> > + *
> > + * @link_handle: The OpenCAPI link handle
> > + * @pdev: A device that is on the link
> > + */
> > +void ocxl_link_lpc_release(void *link_handle, struct pci_dev
> > *pdev);
> > +
> > #endif /* _OCXL_INTERNAL_H_ */
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH v2 05/27] powerpc: Map & release OpenCAPI LPC memory
From: Alastair D'Silva @ 2020-02-18 23:44 UTC (permalink / raw)
To: Frederic Barrat
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Keith Busch,
Masahiro Yamada, Paul Mackerras, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Vishal Verma, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <85e5a3d4-bac2-a8fc-8fc7-865be539dc3c@linux.ibm.com>
On Fri, 2020-02-14 at 12:09 +0100, Frederic Barrat wrote:
>
> Le 03/12/2019 à 04:46, Alastair D'Silva a écrit :
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > This patch adds platform support to map & release LPC memory.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> > arch/powerpc/include/asm/pnv-ocxl.h | 2 ++
> > arch/powerpc/platforms/powernv/ocxl.c | 42
> > +++++++++++++++++++++++++++
> > 2 files changed, 44 insertions(+)
> >
> > diff --git a/arch/powerpc/include/asm/pnv-ocxl.h
> > b/arch/powerpc/include/asm/pnv-ocxl.h
> > index 7de82647e761..f8f8ffb48aa8 100644
> > --- a/arch/powerpc/include/asm/pnv-ocxl.h
> > +++ b/arch/powerpc/include/asm/pnv-ocxl.h
> > @@ -32,5 +32,7 @@ extern int pnv_ocxl_spa_remove_pe_from_cache(void
> > *platform_data, int pe_handle)
> >
> > extern int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr);
> > extern void pnv_ocxl_free_xive_irq(u32 irq);
> > +extern u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64
> > size);
> > +extern void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev);
> >
> > #endif /* _ASM_PNV_OCXL_H */
> > diff --git a/arch/powerpc/platforms/powernv/ocxl.c
> > b/arch/powerpc/platforms/powernv/ocxl.c
> > index 8c65aacda9c8..b56a48daf48c 100644
> > --- a/arch/powerpc/platforms/powernv/ocxl.c
> > +++ b/arch/powerpc/platforms/powernv/ocxl.c
> > @@ -475,6 +475,48 @@ void pnv_ocxl_spa_release(void *platform_data)
> > }
> > EXPORT_SYMBOL_GPL(pnv_ocxl_spa_release);
> >
> > +u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size)
> > +{
> > + struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> > + struct pnv_phb *phb = hose->private_data;
> > + u32 bdfn = pci_dev_id(pdev);
> > + __be64 base_addr_be64;
> > + u64 base_addr;
> > + int rc;
> > +
> > + rc = opal_npu_mem_alloc(phb->opal_id, bdfn, size,
> > &base_addr_be64);
> > + if (rc) {
> > + dev_warn(&pdev->dev,
> > + "OPAL could not allocate LPC memory, rc=%d\n",
> > rc);
> > + return 0;
> > + }
> > +
> > + base_addr = be64_to_cpu(base_addr_be64);
> > +
> > + rc = check_hotplug_memory_addressable(base_addr >> PAGE_SHIFT,
> > + size >> PAGE_SHIFT);
>
> check_hotplug_memory_addressable() is only declared if
> CONFIG_MEMORY_HOTPLUG_SPARSE is selected.
> I think we also need a #ifdef here.
>
Agreed. I think that since any actual use of the memory is going to be
dependant on both hotplug & sparse, moving the ifdef to wrap the
functions & declarations makes sense.
> Fred
>
>
> > + if (rc)
> > + return 0;
> > +
> > + return base_addr;
> > +}
> > +EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_setup);
> > +
> > +void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev)
> > +{
> > + struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> > + struct pnv_phb *phb = hose->private_data;
> > + u32 bdfn = pci_dev_id(pdev);
> > + int rc;
> > +
> > + rc = opal_npu_mem_release(phb->opal_id, bdfn);
> > + if (rc)
> > + dev_warn(&pdev->dev,
> > + "OPAL reported rc=%d when releasing LPC
> > memory\n", rc);
> > +}
> > +EXPORT_SYMBOL_GPL(pnv_ocxl_platform_lpc_release);
> > +
> > +
> > int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int
> > pe_handle)
> > {
> > struct spa_data *data = (struct spa_data *) platform_data;
> >
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH] powerpc/Makefile: Mark phony targets as PHONY
From: Masahiro Yamada @ 2020-02-18 12:40 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Linux Kbuild mailing list
In-Reply-To: <20200218111842.1641-1-mpe@ellerman.id.au>
On Tue, Feb 18, 2020 at 8:19 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Some of our phony targets are not marked as such. This can lead to
> confusing errors, eg:
>
> $ make clean
> $ touch install
> $ make install
> make: 'install' is up to date.
> $
>
> Fix it by adding them to the PHONY variable which is marked phony in
> the top-level Makefile. In arch/powerpc/boot/Makefile we do it
> manually.
You can do likewise in arch/powerpc/boot/Makefile
because it is marked phony in scripts/Makefile.build
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v2 2/3] ASoC: dt-bindings: fsl_easrc: Add document for EASRC
From: Nicolin Chen @ 2020-02-18 22:54 UTC (permalink / raw)
To: Shengjiu Wang
Cc: mark.rutland, devicetree, alsa-devel, timur, Xiubo.Lee,
linuxppc-dev, tiwai, lgirdwood, robh+dt, perex, broonie, festevam,
linux-kernel
In-Reply-To: <a02af544c73914fe3a5ab2f35eb237ef68ee29e7.1582007379.git.shengjiu.wang@nxp.com>
On Tue, Feb 18, 2020 at 02:39:36PM +0800, Shengjiu Wang wrote:
> EASRC (Enhanced Asynchronous Sample Rate Converter) is a new
> IP module found on i.MX8MN.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
> .../devicetree/bindings/sound/fsl,easrc.txt | 57 +++++++++++++++++++
> 1 file changed, 57 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/fsl,easrc.txt
>
> diff --git a/Documentation/devicetree/bindings/sound/fsl,easrc.txt b/Documentation/devicetree/bindings/sound/fsl,easrc.txt
> new file mode 100644
> index 000000000000..0e8153165e3b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/fsl,easrc.txt
> @@ -0,0 +1,57 @@
> +NXP Asynchronous Sample Rate Converter (ASRC) Controller
Missing "Enhanced", I guess.
And "ASRC" => "EASRC"
> +The Asynchronous Sample Rate Converter (ASRC) converts the sampling rate of a
Ditto
> +signal associated with an input clock into a signal associated with a different
> +output clock. The driver currently works as a Front End of DPCM with other Back
> +Ends Audio controller such as ESAI, SSI and SAI. It has four context to support
"context" => "contexts"
Btw, what's the definition of this "context"?
And, is SSI still available on imx8mn?
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Book3S HV: Treat unrecognized TM instructions as illegal
From: Gustavo Romero @ 2020-02-18 21:23 UTC (permalink / raw)
To: Segher Boessenkool, Michael Neuling; +Cc: linuxppc-dev, kvm-ppc, Gustavo Romero
In-Reply-To: <20200217073743.GT22482@gate.crashing.org>
Hi,
On 02/17/2020 04:37 AM, Segher Boessenkool wrote:
> On Mon, Feb 17, 2020 at 05:23:07PM +1100, Michael Neuling wrote:
>>>> Hence, we should NOP this, not generate an illegal.
>>>
>>> It is not a reserved bit.
>>>
>>> The IMC entry for it matches op1=011111 op2=1////01110 presumably, which
>>> catches all TM instructions and nothing else (bits 0..5 and bits 21..30).
>>> That does not look at bit 31, the softpatch handler has to deal with this.
>>>
>>> Some TM insns have bit 31 as 1 and some have it as /. All instructions
>>> with a "." in the mnemonic have bit 31 is 1, all other have it reserved.
>>> The tables in appendices D, E, F show tend. and tsr. as having it
>>> reserved, which contradicts the individual instruction description (and
>>> does not make much sense). (Only tcheck has /, everything else has 1;
>>> everything else has a mnemonic with a dot, and does write CR0 always).
>>
>> Wow, interesting.
>>
>> P8 seems to be treating 31 as a reserved bit (with the table definition rather
>> than the individual instruction description). I'm inclined to match P8 even
>> though it's inconsistent with the dot mnemonic as you say.
>
> "The POWER8 core ignores the state of reserved bits in the instructions
> (denoted by “///” in the instruction definition) and executes the
> instruction normally. Software should set these bits to ‘0’ per the
> Power ISA." (p8 UM, 3.1.1.3; same in the p9 UM).
For the records, I've sent a v2 addressing Mikey's comments:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-February/204502.html
or
https://marc.info/?l=kvm-ppc&m=158206045520038&w=2
Thanks for the review.
Best regards,
Gustavo
^ permalink raw reply
* [PATCH] KVM: PPC: Book3S HV: Treat TM-related invalid form instructions on P9 like the valid ones
From: Gustavo Romero @ 2020-02-18 21:13 UTC (permalink / raw)
To: kvm-ppc, paulus; +Cc: mikey, linuxppc-dev, gromero
On P9 DD2.2 due to a CPU defect some TM instructions need to be emulated by
KVM. This is handled at first by the hardware raising a softpatch interrupt
when certain TM instructions that need KVM assistance are executed in the
guest. Althought some TM instructions per Power ISA are invalid forms they
can raise a softpatch interrupt too. For instance, 'tresume.' instruction
as defined in the ISA must have bit 31 set (1), but an instruction that
matches 'tresume.' PO and XO opcode fields but has bit 31 not set (0), like
0x7cfe9ddc, also raises a softpatch interrupt. Similarly for 'treclaim.'
and 'trechkpt.' instructions with bit 31 = 0, i.e. 0x7c00075c and
0x7c0007dc, respectively. Hence, if a code like the following is executed
in the guest it will raise a softpatch interrupt just like a 'tresume.'
when the TM facility is enabled ('tabort. 0' in the example is used only
to enable the TM facility):
int main() { asm("tabort. 0; .long 0x7cfe9ddc;"); }
Currently in such a case KVM throws a complete trace like:
[345523.705984] WARNING: CPU: 24 PID: 64413 at arch/powerpc/kvm/book3s_hv_tm.c:211 kvmhv_p9_tm_emulation+0x68/0x620 [kvm_hv]
[345523.705985] Modules linked in: kvm_hv(E) xt_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp ip6table_mangle ip6table_nat
iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ebtable_filter ebtables ip6table_filter
ip6_tables iptable_filter bridge stp llc sch_fq_codel ipmi_powernv at24 vmx_crypto ipmi_devintf ipmi_msghandler
ibmpowernv uio_pdrv_genirq kvm opal_prd uio leds_powernv ib_iser rdma_cm iw_cm ib_cm ib_core iscsi_tcp libiscsi_tcp
libiscsi scsi_transport_iscsi ip_tables x_tables autofs4 btrfs blake2b_generic zstd_compress raid10 raid456
async_raid6_recov async_memcpy async_pq async_xor async_tx libcrc32c xor raid6_pq raid1 raid0 multipath linear tg3
crct10dif_vpmsum crc32c_vpmsum ipr [last unloaded: kvm_hv]
[345523.706030] CPU: 24 PID: 64413 Comm: CPU 0/KVM Tainted: G W E 5.5.0+ #1
[345523.706031] NIP: c0080000072cb9c0 LR: c0080000072b5e80 CTR: c0080000085c7850
[345523.706034] REGS: c000000399467680 TRAP: 0700 Tainted: G W E (5.5.0+)
[345523.706034] MSR: 900000010282b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE,TM[E]> CR: 24022428 XER: 00000000
[345523.706042] CFAR: c0080000072b5e7c IRQMASK: 0
GPR00: c0080000072b5e80 c000000399467910 c0080000072db500 c000000375ccc720
GPR04: c000000375ccc720 00000003fbec0000 0000a10395dda5a6 0000000000000000
GPR08: 000000007cfe9ddc 7cfe9ddc000005dc 7cfe9ddc7c0005dc c0080000072cd530
GPR12: c0080000085c7850 c0000003fffeb800 0000000000000001 00007dfb737f0000
GPR16: c0002001edcca558 0000000000000000 0000000000000000 0000000000000001
GPR20: c000000001b21258 c0002001edcca558 0000000000000018 0000000000000000
GPR24: 0000000001000000 ffffffffffffffff 0000000000000001 0000000000001500
GPR28: c0002001edcc4278 c00000037dd80000 800000050280f033 c000000375ccc720
[345523.706062] NIP [c0080000072cb9c0] kvmhv_p9_tm_emulation+0x68/0x620 [kvm_hv]
[345523.706065] LR [c0080000072b5e80] kvmppc_handle_exit_hv.isra.53+0x3e8/0x798 [kvm_hv]
[345523.706066] Call Trace:
[345523.706069] [c000000399467910] [c000000399467940] 0xc000000399467940 (unreliable)
[345523.706071] [c000000399467950] [c000000399467980] 0xc000000399467980
[345523.706075] [c0000003994679f0] [c0080000072bd1c4] kvmhv_run_single_vcpu+0xa1c/0xb80 [kvm_hv]
[345523.706079] [c000000399467ac0] [c0080000072bd8e0] kvmppc_vcpu_run_hv+0x5b8/0xb00 [kvm_hv]
[345523.706087] [c000000399467b90] [c0080000085c93cc] kvmppc_vcpu_run+0x34/0x48 [kvm]
[345523.706095] [c000000399467bb0] [c0080000085c582c] kvm_arch_vcpu_ioctl_run+0x244/0x420 [kvm]
[345523.706101] [c000000399467c40] [c0080000085b7498] kvm_vcpu_ioctl+0x3d0/0x7b0 [kvm]
[345523.706105] [c000000399467db0] [c0000000004adf9c] ksys_ioctl+0x13c/0x170
[345523.706107] [c000000399467e00] [c0000000004adff8] sys_ioctl+0x28/0x80
[345523.706111] [c000000399467e20] [c00000000000b278] system_call+0x5c/0x68
[345523.706112] Instruction dump:
[345523.706114] 419e0390 7f8a4840 409d0048 6d497c00 2f89075d 419e021c 6d497c00 2f8907dd
[345523.706119] 419e01c0 6d497c00 2f8905dd 419e00a4 <0fe00000> 38210040 38600000 ebc1fff0
and then treats the executed instruction as a 'nop'.
However the POWER9 User's Manual, in section "4.6.10 Book II Invalid
Forms", informs that for TM instructions bit 31 is in fact ignored, thus
for the TM-related invalid forms ignoring bit 31 and handling them like the
valid forms is an acceptable way to handle them. POWER8 behaves the same
way too.
This commit changes the handling of the cases here described by treating
the TM-related invalid forms that can generate a softpatch interrupt
just like their valid forms (w/ bit 31 = 1) instead of as a 'nop' and by
gently reporting any other unrecognized case to the host and treating it as
illegal instruction instead of throwing a trace and treating it as a 'nop'.
Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
---
arch/powerpc/include/asm/kvm_asm.h | 3 +++
arch/powerpc/kvm/book3s_hv_tm.c | 28 +++++++++++++++++++++++-----
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_asm.h b/arch/powerpc/include/asm/kvm_asm.h
index 635fb154b33f..a3633560493b 100644
--- a/arch/powerpc/include/asm/kvm_asm.h
+++ b/arch/powerpc/include/asm/kvm_asm.h
@@ -150,4 +150,7 @@
#define KVM_INST_FETCH_FAILED -1
+/* Extract PO and XOP opcode fields */
+#define PO_XOP_OPCODE_MASK 0xfc0007fe
+
#endif /* __POWERPC_KVM_ASM_H__ */
diff --git a/arch/powerpc/kvm/book3s_hv_tm.c b/arch/powerpc/kvm/book3s_hv_tm.c
index 0db937497169..cc90b8b82329 100644
--- a/arch/powerpc/kvm/book3s_hv_tm.c
+++ b/arch/powerpc/kvm/book3s_hv_tm.c
@@ -3,6 +3,8 @@
* Copyright 2017 Paul Mackerras, IBM Corp. <paulus@au1.ibm.com>
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/kvm_host.h>
#include <asm/kvm_ppc.h>
@@ -44,7 +46,18 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
u64 newmsr, bescr;
int ra, rs;
- switch (instr & 0xfc0007ff) {
+ /*
+ * rfid, rfebb, and mtmsrd encode bit 31 = 0 since it's a reserved bit
+ * in these instructions, so masking bit 31 out doesn't change these
+ * instructions. For treclaim., tsr., and trechkpt. instructions if bit
+ * 31 = 0 then they are per ISA invalid forms, however P9 UM, in section
+ * 4.6.10 Book II Invalid Forms, informs specifically that ignoring bit
+ * 31 is an acceptable way to handle these invalid forms that have
+ * bit 31 = 0. Moreover, for emulation purposes both forms (w/ and wo/
+ * bit 31 set) can generate a softpatch interrupt. Hence both forms
+ * are handled below for these instructions so they behave the same way.
+ */
+ switch (instr & PO_XOP_OPCODE_MASK) {
case PPC_INST_RFID:
/* XXX do we need to check for PR=0 here? */
newmsr = vcpu->arch.shregs.srr1;
@@ -105,7 +118,8 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
vcpu->arch.shregs.msr = newmsr;
return RESUME_GUEST;
- case PPC_INST_TSR:
+ /* ignore bit 31, see comment above */
+ case (PPC_INST_TSR & PO_XOP_OPCODE_MASK):
/* check for PR=1 and arch 2.06 bit set in PCR */
if ((msr & MSR_PR) && (vcpu->arch.vcore->pcr & PCR_ARCH_206)) {
/* generate an illegal instruction interrupt */
@@ -140,7 +154,8 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
vcpu->arch.shregs.msr = msr;
return RESUME_GUEST;
- case PPC_INST_TRECLAIM:
+ /* ignore bit 31, see comment above */
+ case (PPC_INST_TRECLAIM & PO_XOP_OPCODE_MASK):
/* check for TM disabled in the HFSCR or MSR */
if (!(vcpu->arch.hfscr & HFSCR_TM)) {
/* generate an illegal instruction interrupt */
@@ -176,7 +191,8 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
vcpu->arch.shregs.msr &= ~MSR_TS_MASK;
return RESUME_GUEST;
- case PPC_INST_TRECHKPT:
+ /* ignore bit 31, see comment above */
+ case (PPC_INST_TRECHKPT & PO_XOP_OPCODE_MASK):
/* XXX do we need to check for PR=0 here? */
/* check for TM disabled in the HFSCR or MSR */
if (!(vcpu->arch.hfscr & HFSCR_TM)) {
@@ -208,6 +224,8 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
}
/* What should we do here? We didn't recognize the instruction */
- WARN_ON_ONCE(1);
+ kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+ pr_warn_ratelimited("Unrecognized TM-related instruction %#x for emulation", instr);
+
return RESUME_GUEST;
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] libnvdimm/bus: return the outvar 'cmd_rc' error code in __nd_ioctl()
From: Dan Williams @ 2020-02-18 21:03 UTC (permalink / raw)
To: Jeff Moyer; +Cc: Vaibhav Jain, linuxppc-dev, Aneesh Kumar K.V, linux-nvdimm
In-Reply-To: <x49r1yrboh2.fsf@segfault.boston.devel.redhat.com>
On Tue, Feb 18, 2020 at 1:00 PM Jeff Moyer <jmoyer@redhat.com> wrote:
>
> Vaibhav Jain <vaibhav@linux.ibm.com> writes:
>
> > Presently the error code returned via out variable 'cmd_rc' from the
> > nvdimm-bus controller function is ignored when called from
> > __nd_ioctl() and never communicated back to user-space code that called
> > an ioctl on dimm/bus.
> >
> > This minor patch updates __nd_ioctl() to propagate the value of out
> > variable 'cmd_rc' back to user-space in case it reports an error.
> >
> > Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> > ---
> > drivers/nvdimm/bus.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
> > index a8b515968569..5b687a27fdf2 100644
> > --- a/drivers/nvdimm/bus.c
> > +++ b/drivers/nvdimm/bus.c
> > @@ -1153,6 +1153,11 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
> > if (rc < 0)
> > goto out_unlock;
> >
> > + if (cmd_rc < 0) {
> > + rc = cmd_rc;
> > + goto out_unlock;
> > + }
> > +
> > if (!nvdimm && cmd == ND_CMD_CLEAR_ERROR && cmd_rc >= 0) {
> > struct nd_cmd_clear_error *clear_err = buf;
>
> Looks good to me.
>
> Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH] libnvdimm/of_pmem: Fix leaking bus_desc.provider_name in some paths
From: Jeff Moyer @ 2020-02-18 21:03 UTC (permalink / raw)
To: Vaibhav Jain; +Cc: Aneesh Kumar K . V, linuxppc-dev, stable, linux-nvdimm
In-Reply-To: <20200123031847.149325-1-vaibhav@linux.ibm.com>
Vaibhav Jain <vaibhav@linux.ibm.com> writes:
> String 'bus_desc.provider_name' allocated inside
> of_pmem_region_probe() will leak in case call to nvdimm_bus_register()
> fails or when of_pmem_region_remove() is called.
>
> This minor patch ensures that 'bus_desc.provider_name' is freed in
> error path for of_pmem_region_probe() as well as in
> of_pmem_region_remove().
>
> Cc: stable@vger.kernel.org
> Fixes:49bddc73d15c2("libnvdimm/of_pmem: Provide a unique name for bus provider")
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> drivers/nvdimm/of_pmem.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/nvdimm/of_pmem.c b/drivers/nvdimm/of_pmem.c
> index 8224d1431ea9..9cb76f9837ad 100644
> --- a/drivers/nvdimm/of_pmem.c
> +++ b/drivers/nvdimm/of_pmem.c
> @@ -36,6 +36,7 @@ static int of_pmem_region_probe(struct platform_device *pdev)
>
> priv->bus = bus = nvdimm_bus_register(&pdev->dev, &priv->bus_desc);
> if (!bus) {
> + kfree(priv->bus_desc.provider_name);
> kfree(priv);
> return -ENODEV;
> }
> @@ -81,6 +82,7 @@ static int of_pmem_region_remove(struct platform_device *pdev)
> struct of_pmem_private *priv = platform_get_drvdata(pdev);
>
> nvdimm_bus_unregister(priv->bus);
> + kfree(priv->bus_desc.provider_name);
> kfree(priv);
>
> return 0;
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
^ permalink raw reply
* Re: [PATCH] libnvdimm/bus: return the outvar 'cmd_rc' error code in __nd_ioctl()
From: Jeff Moyer @ 2020-02-18 21:00 UTC (permalink / raw)
To: Vaibhav Jain; +Cc: Aneesh Kumar K.V, linuxppc-dev, linux-nvdimm
In-Reply-To: <20200122155304.120733-1-vaibhav@linux.ibm.com>
Vaibhav Jain <vaibhav@linux.ibm.com> writes:
> Presently the error code returned via out variable 'cmd_rc' from the
> nvdimm-bus controller function is ignored when called from
> __nd_ioctl() and never communicated back to user-space code that called
> an ioctl on dimm/bus.
>
> This minor patch updates __nd_ioctl() to propagate the value of out
> variable 'cmd_rc' back to user-space in case it reports an error.
>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> drivers/nvdimm/bus.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
> index a8b515968569..5b687a27fdf2 100644
> --- a/drivers/nvdimm/bus.c
> +++ b/drivers/nvdimm/bus.c
> @@ -1153,6 +1153,11 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
> if (rc < 0)
> goto out_unlock;
>
> + if (cmd_rc < 0) {
> + rc = cmd_rc;
> + goto out_unlock;
> + }
> +
> if (!nvdimm && cmd == ND_CMD_CLEAR_ERROR && cmd_rc >= 0) {
> struct nd_cmd_clear_error *clear_err = buf;
Looks good to me.
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
^ permalink raw reply
* Re: [PATCH v7 10/12] drivers/oprofile: open access for CAP_PERFMON privileged process
From: James Morris @ 2020-02-18 19:44 UTC (permalink / raw)
To: Alexey Budankov
Cc: linux-man, linux-doc@vger.kernel.org, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Alexei Starovoitov,
Stephane Eranian, Paul Mackerras, Jiri Olsa, Ingo Molnar,
Andi Kleen, Will Deacon, Helge Deller, Igor Lubashev,
oprofile-list, Stephen Smalley, Serge Hallyn,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Thomas Gleixner, linux-arm-kernel,
linux-parisc@vger.kernel.org, linux-kernel,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <046beedf-e074-58e2-579d-df535799169c@linux.intel.com>
On Mon, 17 Feb 2020, Alexey Budankov wrote:
> For backward compatibility reasons access to the monitoring remains
> open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage
> for secure monitoring is discouraged with respect to CAP_PERFMON
> capability.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v7 09/12] drivers/perf: open access for CAP_PERFMON privileged process
From: James Morris @ 2020-02-18 19:43 UTC (permalink / raw)
To: Alexey Budankov
Cc: linux-man, linux-doc@vger.kernel.org, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Alexei Starovoitov,
Stephane Eranian, Paul Mackerras, Jiri Olsa, Ingo Molnar,
Andi Kleen, Will Deacon, Helge Deller, Igor Lubashev,
oprofile-list, Stephen Smalley, Serge Hallyn,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Thomas Gleixner, linux-arm-kernel,
linux-parisc@vger.kernel.org, linux-kernel,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <48baecd5-a015-3dbf-f774-9f2caee13893@linux.intel.com>
On Mon, 17 Feb 2020, Alexey Budankov wrote:
> For backward compatibility reasons access to the monitoring remains
> open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage
> for secure monitoring is discouraged with respect to CAP_PERFMON
> capability.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* [PATCH v3] powerpc/kprobes: Ignore traps that happened in real mode
From: Christophe Leroy @ 2020-02-18 19:38 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Larry Finger, Naveen N. Rao, Masami Hiramatsu
Cc: stable, linuxppc-dev, linux-kernel, Anil S Keshavamurthy,
David S. Miller
When a program check exception happens while MMU translation is
disabled, following Oops happens in kprobe_handler() in the following
code:
} else if (*addr != BREAKPOINT_INSTRUCTION) {
[ 33.098554] BUG: Unable to handle kernel data access on read at 0x0000e268
[ 33.105091] Faulting instruction address: 0xc000ec34
[ 33.110010] Oops: Kernel access of bad area, sig: 11 [#1]
[ 33.115348] BE PAGE_SIZE=16K PREEMPT CMPC885
[ 33.119540] Modules linked in:
[ 33.122591] CPU: 0 PID: 429 Comm: cat Not tainted 5.6.0-rc1-s3k-dev-00824-g84195dc6c58a #3267
[ 33.131005] NIP: c000ec34 LR: c000ecd8 CTR: c019cab8
[ 33.136002] REGS: ca4d3b58 TRAP: 0300 Not tainted (5.6.0-rc1-s3k-dev-00824-g84195dc6c58a)
[ 33.144324] MSR: 00001032 <ME,IR,DR,RI> CR: 2a4d3c52 XER: 00000000
[ 33.150699] DAR: 0000e268 DSISR: c0000000
[ 33.150699] GPR00: c000b09c ca4d3c10 c66d0620 00000000 ca4d3c60 00000000 00009032 00000000
[ 33.150699] GPR08: 00020000 00000000 c087de44 c000afe0 c66d0ad0 100d3dd6 fffffff3 00000000
[ 33.150699] GPR16: 00000000 00000041 00000000 ca4d3d70 00000000 00000000 0000416d 00000000
[ 33.150699] GPR24: 00000004 c53b6128 00000000 0000e268 00000000 c07c0000 c07bb6fc ca4d3c60
[ 33.188015] NIP [c000ec34] kprobe_handler+0x128/0x290
[ 33.192989] LR [c000ecd8] kprobe_handler+0x1cc/0x290
[ 33.197854] Call Trace:
[ 33.200340] [ca4d3c30] [c000b09c] program_check_exception+0xbc/0x6fc
[ 33.206590] [ca4d3c50] [c000e43c] ret_from_except_full+0x0/0x4
[ 33.212392] --- interrupt: 700 at 0xe268
[ 33.270401] Instruction dump:
[ 33.273335] 913e0008 81220000 38600001 3929ffff 91220000 80010024 bb410008 7c0803a6
[ 33.280992] 38210020 4e800020 38600000 4e800020 <813b0000> 6d2a7fe0 2f8a0008 419e0154
[ 33.288841] ---[ end trace 5b9152d4cdadd06d ]---
kprobe is not prepared to handle events in real mode and functions
running in real mode should have been blacklisted, so kprobe_handler()
can safely bail out telling 'this trap is not mine' for any trap that
happened while in real-mode.
If the trap happened with MSR_IR or MSR_DR cleared, return 0 immediately.
Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
Fixes: 6cc89bad60a6 ("powerpc/kprobes: Invoke handlers directly")
Cc: stable@vger.kernel.org
Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
v3: Also bail out if MSR_DR is cleared.
Resending v2 with a more appropriate name
v2: bailing out instead of converting real-time address to virtual and continuing.
The bug might have existed even before that commit from Naveen.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/kprobes.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 2d27ec4feee4..9b340af02c38 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -264,6 +264,9 @@ int kprobe_handler(struct pt_regs *regs)
if (user_mode(regs))
return 0;
+ if (!(regs->msr & MSR_IR) || !(regs->msr & MSR_DR))
+ return 0;
+
/*
* We don't want to be preempted for the entire
* duration of kprobe processing
--
2.25.0
^ permalink raw reply related
* Re: [PATCH v2] powerpc/kprobes: Fix trap address when trap happened in real mode
From: Christophe Leroy @ 2020-02-18 19:37 UTC (permalink / raw)
To: Naveen N. Rao, Benjamin Herrenschmidt, Larry Finger,
Masami Hiramatsu, Michael Ellerman, Paul Mackerras
Cc: stable, linuxppc-dev, David S. Miller, Anil S Keshavamurthy,
linux-kernel
In-Reply-To: <1582037375.4mkd6m1m5m.naveen@linux.ibm.com>
Le 18/02/2020 à 15:55, Naveen N. Rao a écrit :
> Christophe Leroy wrote:
>> When a program check exception happens while MMU translation is
>> disabled, following Oops happens in kprobe_handler() in the following
>> code:
>>
>> } else if (*addr != BREAKPOINT_INSTRUCTION) {
>>
>> [ 33.098554] BUG: Unable to handle kernel data access on read at
>> 0x0000e268
>> [ 33.105091] Faulting instruction address: 0xc000ec34
>> [ 33.110010] Oops: Kernel access of bad area, sig: 11 [#1]
>> [ 33.115348] BE PAGE_SIZE=16K PREEMPT CMPC885
>> [ 33.119540] Modules linked in:
>> [ 33.122591] CPU: 0 PID: 429 Comm: cat Not tainted
>> 5.6.0-rc1-s3k-dev-00824-g84195dc6c58a #3267
>> [ 33.131005] NIP: c000ec34 LR: c000ecd8 CTR: c019cab8
>> [ 33.136002] REGS: ca4d3b58 TRAP: 0300 Not tainted
>> (5.6.0-rc1-s3k-dev-00824-g84195dc6c58a)
>> [ 33.144324] MSR: 00001032 <ME,IR,DR,RI> CR: 2a4d3c52 XER: 00000000
>> [ 33.150699] DAR: 0000e268 DSISR: c0000000
>> [ 33.150699] GPR00: c000b09c ca4d3c10 c66d0620 00000000 ca4d3c60
>> 00000000 00009032 00000000
>> [ 33.150699] GPR08: 00020000 00000000 c087de44 c000afe0 c66d0ad0
>> 100d3dd6 fffffff3 00000000
>> [ 33.150699] GPR16: 00000000 00000041 00000000 ca4d3d70 00000000
>> 00000000 0000416d 00000000
>> [ 33.150699] GPR24: 00000004 c53b6128 00000000 0000e268 00000000
>> c07c0000 c07bb6fc ca4d3c60
>> [ 33.188015] NIP [c000ec34] kprobe_handler+0x128/0x290
>> [ 33.192989] LR [c000ecd8] kprobe_handler+0x1cc/0x290
>> [ 33.197854] Call Trace:
>> [ 33.200340] [ca4d3c30] [c000b09c] program_check_exception+0xbc/0x6fc
>> [ 33.206590] [ca4d3c50] [c000e43c] ret_from_except_full+0x0/0x4
>> [ 33.212392] --- interrupt: 700 at 0xe268
>> [ 33.270401] Instruction dump:
>> [ 33.273335] 913e0008 81220000 38600001 3929ffff 91220000 80010024
>> bb410008 7c0803a6
>> [ 33.280992] 38210020 4e800020 38600000 4e800020 <813b0000> 6d2a7fe0
>> 2f8a0008 419e0154
>> [ 33.288841] ---[ end trace 5b9152d4cdadd06d ]---
>>
>> kprobe is not prepared to handle events in real mode and functions
>> running in real mode should have been blacklisted, so kprobe_handler()
>> can safely bail out telling 'this trap is not mine' for any trap that
>> happened while in real-mode.
>>
>> If the trap happened with MSR_IR cleared, return 0 immediately.
>>
>> Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
>> Fixes: 6cc89bad60a6 ("powerpc/kprobes: Invoke handlers directly")
>> Cc: stable@vger.kernel.org
>> Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>> Cc: Masami Hiramatsu <mhiramat@kernel.org>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>
>> ---
>> v2: bailing out instead of converting real-time address to virtual and
>> continuing.
>>
>> The bug might have existed even before that commit from Naveen.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> arch/powerpc/kernel/kprobes.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/kprobes.c
>> b/arch/powerpc/kernel/kprobes.c
>> index 2d27ec4feee4..673f349662e8 100644
>> --- a/arch/powerpc/kernel/kprobes.c
>> +++ b/arch/powerpc/kernel/kprobes.c
>> @@ -264,6 +264,9 @@ int kprobe_handler(struct pt_regs *regs)
>> if (user_mode(regs))
>> return 0;
>>
>> + if (!(regs->msr & MSR_IR))
>> + return 0;
>> +
>
> Should we also check for MSR_DR? Are there scenarios with ppc32 where
> MSR_IR is on, but MSR_DR is off?
Yes indeed.
Christophe
^ permalink raw reply
* Re: [PATCH v7 08/12] parisc/perf: open access for CAP_PERFMON privileged process
From: James Morris @ 2020-02-18 19:29 UTC (permalink / raw)
To: Alexey Budankov
Cc: linux-man, linux-doc@vger.kernel.org, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Alexei Starovoitov,
Stephane Eranian, Paul Mackerras, Jiri Olsa, Ingo Molnar,
Andi Kleen, Will Deacon, Helge Deller, Igor Lubashev,
oprofile-list, Stephen Smalley, Serge Hallyn,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Thomas Gleixner, linux-arm-kernel,
linux-parisc@vger.kernel.org, linux-kernel,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <29e45605-7a3c-944b-7bea-5959f8ff0793@linux.intel.com>
On Mon, 17 Feb 2020, Alexey Budankov wrote:
> For backward compatibility reasons access to the monitoring remains
> open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage
> for secure monitoring is discouraged with respect to CAP_PERFMON
> capability.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v7 07/12] powerpc/perf: open access for CAP_PERFMON privileged process
From: James Morris @ 2020-02-18 19:28 UTC (permalink / raw)
To: Alexey Budankov
Cc: linux-man, linux-doc@vger.kernel.org, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Alexei Starovoitov,
Stephane Eranian, Paul Mackerras, Jiri Olsa, Ingo Molnar,
Andi Kleen, Will Deacon, Helge Deller, Igor Lubashev,
oprofile-list, Stephen Smalley, Serge Hallyn,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Thomas Gleixner, linux-arm-kernel,
linux-parisc@vger.kernel.org, linux-kernel,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <b144d52b-6040-4660-46d1-2c8c58e98e7e@linux.intel.com>
On Mon, 17 Feb 2020, Alexey Budankov wrote:
> For backward compatibility reasons access to the monitoring remains
> open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage
> for secure monitoring is discouraged with respect to CAP_PERFMON
> capability.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v7 06/12] trace/bpf_trace: open access for CAP_PERFMON privileged process
From: James Morris @ 2020-02-18 19:25 UTC (permalink / raw)
To: Alexey Budankov
Cc: linux-man, linux-doc@vger.kernel.org, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Alexei Starovoitov,
Stephane Eranian, Paul Mackerras, Jiri Olsa, Ingo Molnar,
Andi Kleen, Will Deacon, Helge Deller, Igor Lubashev,
oprofile-list, Stephen Smalley, Serge Hallyn,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Thomas Gleixner, linux-arm-kernel,
linux-parisc@vger.kernel.org, linux-kernel,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <fc689865-f8ff-1e85-ac0c-b2f1c28b7eb6@linux.intel.com>
On Mon, 17 Feb 2020, Alexey Budankov wrote:
>
> Open access to bpf_trace monitoring for CAP_PERFMON privileged process.
> Providing the access under CAP_PERFMON capability singly, without the
> rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
> credentials and makes operation more secure.
>
> CAP_PERFMON implements the principal of least privilege for performance
> monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39
> principle of least privilege: A security design principle that states
> that a process or program be granted only those privileges (e.g.,
> capabilities) necessary to accomplish its legitimate function, and only
> for the time that such privileges are actually required)
>
> For backward compatibility reasons access to bpf_trace monitoring
> remains open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN
> usage for secure bpf_trace monitoring is discouraged with respect to
> CAP_PERFMON capability.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v7 05/12] drm/i915/perf: open access for CAP_PERFMON privileged process
From: James Morris @ 2020-02-18 19:25 UTC (permalink / raw)
To: Alexey Budankov
Cc: linux-man, linux-doc@vger.kernel.org, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Alexei Starovoitov,
Stephane Eranian, Paul Mackerras, Jiri Olsa, Ingo Molnar,
Andi Kleen, Will Deacon, Helge Deller, Igor Lubashev,
oprofile-list, Stephen Smalley, Serge Hallyn,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Thomas Gleixner, linux-arm-kernel,
linux-parisc@vger.kernel.org, linux-kernel,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <8b408c10-9bb0-4b08-8681-93c0f4a1132e@linux.intel.com>
On Mon, 17 Feb 2020, Alexey Budankov wrote:
>
> Open access to i915_perf monitoring for CAP_PERFMON privileged process.
> Providing the access under CAP_PERFMON capability singly, without the
> rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
> credentials and makes operation more secure.
>
> CAP_PERFMON implements the principal of least privilege for performance
> monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39
> principle of least privilege: A security design principle that states
> that a process or program be granted only those privileges (e.g.,
> capabilities) necessary to accomplish its legitimate function, and only
> for the time that such privileges are actually required)
>
> For backward compatibility reasons access to i915_events subsystem
> remains open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN
> usage for secure i915_events monitoring is discouraged with respect to
> CAP_PERFMON capability.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v7 04/12] perf tool: extend Perf tool with CAP_PERFMON capability support
From: James Morris @ 2020-02-18 19:24 UTC (permalink / raw)
To: Alexey Budankov
Cc: linux-man, linux-doc@vger.kernel.org, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Alexei Starovoitov,
Stephane Eranian, Paul Mackerras, Jiri Olsa, Ingo Molnar,
Andi Kleen, Will Deacon, Helge Deller, Igor Lubashev,
oprofile-list, Stephen Smalley, Serge Hallyn,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Thomas Gleixner, linux-arm-kernel,
linux-parisc@vger.kernel.org, linux-kernel,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <5f961a07-36d0-d8f4-1895-6cfc38bcb81e@linux.intel.com>
On Mon, 17 Feb 2020, Alexey Budankov wrote:
>
> Extend error messages to mention CAP_PERFMON capability as an option
> to substitute CAP_SYS_ADMIN capability for secure system performance
> monitoring and observability. Make perf_event_paranoid_check() and
> __cmd_ftrace() to be aware of CAP_PERFMON capability.
>
> CAP_PERFMON implements the principal of least privilege for performance
> monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39
> principle of least privilege: A security design principle that states
> that a process or program be granted only those privileges (e.g.,
> capabilities) necessary to accomplish its legitimate function, and only
> for the time that such privileges are actually required)
>
> For backward compatibility reasons access to perf_events subsystem
> remains open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN
> usage for secure perf_events monitoring is discouraged with respect to
> CAP_PERFMON capability.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v7 03/12] perf/core: open access to probes for CAP_PERFMON privileged process
From: James Morris @ 2020-02-18 19:22 UTC (permalink / raw)
To: Alexey Budankov
Cc: linux-man, linux-doc@vger.kernel.org, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Alexei Starovoitov,
Stephane Eranian, Paul Mackerras, Jiri Olsa, Ingo Molnar,
Andi Kleen, Will Deacon, Helge Deller, Igor Lubashev,
oprofile-list, Stephen Smalley, Serge Hallyn,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Thomas Gleixner, linux-arm-kernel,
linux-parisc@vger.kernel.org, linux-kernel,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <3364fa26-b5d1-1808-aaee-c057f26e0eb4@linux.intel.com>
On Mon, 17 Feb 2020, Alexey Budankov wrote:
>
> Open access to monitoring via kprobes and uprobes and eBPF tracing for
> CAP_PERFMON privileged process. Providing the access under CAP_PERFMON
> capability singly, without the rest of CAP_SYS_ADMIN credentials,
> excludes chances to misuse the credentials and makes operation more
> secure.
>
> perf kprobes and uprobes are used by ftrace and eBPF. perf probe uses
> ftrace to define new kprobe events, and those events are treated as
> tracepoint events. eBPF defines new probes via perf_event_open interface
> and then the probes are used in eBPF tracing.
>
> CAP_PERFMON implements the principal of least privilege for performance
> monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39
> principle of least privilege: A security design principle that states
> that a process or program be granted only those privileges (e.g.,
> capabilities) necessary to accomplish its legitimate function, and only
> for the time that such privileges are actually required)
>
> For backward compatibility reasons access to perf_events subsystem
> remains open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN
> usage for secure perf_events monitoring is discouraged with respect to
> CAP_PERFMON capability.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v7 02/12] perf/core: open access to the core for CAP_PERFMON privileged process
From: James Morris @ 2020-02-18 19:22 UTC (permalink / raw)
To: Alexey Budankov
Cc: linux-man, linux-doc@vger.kernel.org, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Alexei Starovoitov,
Stephane Eranian, Paul Mackerras, Jiri Olsa, Ingo Molnar,
Andi Kleen, Will Deacon, Helge Deller, Igor Lubashev,
oprofile-list, Stephen Smalley, Serge Hallyn,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Thomas Gleixner, linux-arm-kernel,
linux-parisc@vger.kernel.org, linux-kernel,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <e68be109-7174-2c9e-11a9-9770a6316834@linux.intel.com>
On Mon, 17 Feb 2020, Alexey Budankov wrote:
>
> Open access to monitoring of kernel code, cpus, tracepoints and
> namespaces data for a CAP_PERFMON privileged process. Providing the
> access under CAP_PERFMON capability singly, without the rest of
> CAP_SYS_ADMIN credentials, excludes chances to misuse the credentials
> and makes operation more secure.
>
> CAP_PERFMON implements the principal of least privilege for performance
> monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39
> principle of least privilege: A security design principle that states
> that a process or program be granted only those privileges (e.g.,
> capabilities) necessary to accomplish its legitimate function, and only
> for the time that such privileges are actually required)
>
> For backward compatibility reasons access to perf_events subsystem
> remains open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN
> usage for secure perf_events monitoring is discouraged with respect to
> CAP_PERFMON capability.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v7 01/12] capabilities: introduce CAP_PERFMON to kernel and user space
From: James Morris @ 2020-02-18 19:21 UTC (permalink / raw)
To: Alexey Budankov
Cc: linux-man, linux-doc@vger.kernel.org, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Alexei Starovoitov,
Stephane Eranian, Paul Mackerras, Jiri Olsa, Ingo Molnar,
Andi Kleen, Will Deacon, Helge Deller, Igor Lubashev,
oprofile-list, Stephen Smalley, Serge Hallyn,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Thomas Gleixner, linux-arm-kernel,
linux-parisc@vger.kernel.org, linux-kernel,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <f56fbb5c-1477-44d5-7346-85a1ca0869dc@linux.intel.com>
On Mon, 17 Feb 2020, Alexey Budankov wrote:
>
> Introduce CAP_PERFMON capability designed to secure system performance
> monitoring and observability operations so that CAP_PERFMON would assist
> CAP_SYS_ADMIN capability in its governing role for performance
> monitoring and observability subsystems.
Acked-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v7 01/12] capabilities: introduce CAP_PERFMON to kernel and user space
From: Stephen Smalley @ 2020-02-18 18:22 UTC (permalink / raw)
To: Alexey Budankov, James Morris, Serge Hallyn, Peter Zijlstra,
Arnaldo Carvalho de Melo, Ingo Molnar,
joonas.lahtinen@linux.intel.com, Alexei Starovoitov, Will Deacon,
Paul Mackerras, Helge Deller, Thomas Gleixner
Cc: linux-man, Andi Kleen, linux-parisc@vger.kernel.org,
linux-doc@vger.kernel.org, selinux@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, intel-gfx@lists.freedesktop.org,
Igor Lubashev, linux-kernel, Stephane Eranian,
linux-security-module@vger.kernel.org, oprofile-list, Jiri Olsa,
linux-arm-kernel
In-Reply-To: <f56fbb5c-1477-44d5-7346-85a1ca0869dc@linux.intel.com>
On 2/17/20 3:06 AM, Alexey Budankov wrote:
>
> Introduce CAP_PERFMON capability designed to secure system performance
> monitoring and observability operations so that CAP_PERFMON would assist
> CAP_SYS_ADMIN capability in its governing role for performance
> monitoring and observability subsystems.
>
> CAP_PERFMON hardens system security and integrity during performance
> monitoring and observability operations by decreasing attack surface
> that is available to a CAP_SYS_ADMIN privileged process [2]. Providing
> the access to system performance monitoring and observability operations
> under CAP_PERFMON capability singly, without the rest of CAP_SYS_ADMIN
> credentials, excludes chances to misuse the credentials and makes the
> operation more secure. Thus, CAP_PERFMON implements the principal of
> least privilege for performance monitoring and observability operations
> (POSIX IEEE 1003.1e: 2.2.2.39 principle of least privilege: A security
> design principle that states that a process or program be granted only
> those privileges (e.g., capabilities) necessary to accomplish its
> legitimate function, and only for the time that such privileges are
> actually required)
>
> CAP_PERFMON meets the demand to secure system performance monitoring and
> observability operations for adoption in security sensitive, restricted,
> multiuser production environments (e.g. HPC clusters, cloud and virtual
> compute environments), where root or CAP_SYS_ADMIN credentials are not
> available to mass users of a system, and securely unblocks accessibility
> of system performance monitoring and observability operations beyond
> the root and CAP_SYS_ADMIN use cases.
>
> CAP_PERFMON takes over CAP_SYS_ADMIN credentials related to system
> performance monitoring and observability operations and balances amount
> of CAP_SYS_ADMIN credentials following the recommendations in the
> capabilities man page [1] for CAP_SYS_ADMIN: "Note: this capability is
> overloaded; see Notes to kernel developers, below." For backward
> compatibility reasons access to system performance monitoring and
> observability subsystems of the kernel remains open for CAP_SYS_ADMIN
> privileged processes but CAP_SYS_ADMIN usage for secure system
> performance monitoring and observability operations is discouraged with
> respect to the designed CAP_PERFMON capability.
>
> Although the software running under CAP_PERFMON can not ensure avoidance
> of related hardware issues, the software can still mitigate these issues
> following the official hardware issues mitigation procedure [2].
> The bugs in the software itself can be fixed following the standard
> kernel development process [3] to maintain and harden security of system
> performance monitoring and observability operations.
>
> [1] http://man7.org/linux/man-pages/man7/capabilities.7.html
> [2] https://www.kernel.org/doc/html/latest/process/embargoed-hardware-issues.html
> [3] https://www.kernel.org/doc/html/latest/admin-guide/security-bugs.html
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
[...]
^ permalink raw reply
* Re: MCE handler gets NIP wrong on MPC8378
From: Christophe Leroy @ 2020-02-18 18:08 UTC (permalink / raw)
To: Radu Rendec, linuxppc-dev
In-Reply-To: <CAD5jUk_8DAvneGjkQ7JOOuNeXaKU1g9E09+H8M5Eo=ttgthdgg@mail.gmail.com>
Le 18/02/2020 à 18:07, Radu Rendec a écrit :
> Hi Everyone,
>
> The saved NIP seems to be broken inside machine_check_exception() on
> MPC8378, running Linux 4.9.191. The value is 0x900 most of the times,
> but I have seen other weird values.
>
> I've been able to track down the entry code to head_32.S (vector 0x200),
> but I'm not sure where/how the NIP value (where the exception occurred)
> is captured.
NIP value is supposed to come from SRR0, loaded in r12 in PROLOG_2 and
saved into _NIP(r11) in transfer_to_handler in entry_32.S
Can something clobber r12 at some point ?
Maybe add the following at some place to trap when it happens ?
tweqi r12, 0x900
If you put it just after reading SRR0, and just before writing into
NIP(r11), you'll see if its wrong from the begining or if it is
overwriten later.
Christophe
>
> I also noticed most of the code has moved to head_32.h in newer kernel
> versions, but EXCEPTION_PROLOG_1 and EXCEPTION_PROLOG_2 look pretty much
> the same. I guess the same thing happens on a recent kernel, even though
> I don't have an easy way to test it.
>
> The original MCE that I see is triggered by a failed PCIe transaction,
> but I was able to reproduce it by just reading from a (physically)
> unmapped memory area. Sample code and kernel crash dump are included
> below.
>
> Can anyone please provide any suggestion as to what to look at next?
>
> Thanks,
> Radu
>
> 8<--------------------------------------------------------------------
>
> #include <linux/module.h>
> #include <linux/delay.h>
> #include <asm/io.h>
>
> static void __iomem *bad_addr_base;
>
> static int __init test_mce_init(void)
> {
> unsigned int x;
>
> bad_addr_base = ioremap(0xf0000000, 0x100);
>
> if (bad_addr_base) {
> __asm__ __volatile__ ("isync");
> x = ioread32(bad_addr_base);
> pr_info("Test: %#0x\n", x);
> } else
> pr_err("Cannot map\n");
>
> return 0;
> }
>
> static void __exit test_mce_exit(void)
> {
> iounmap(bad_addr_base);
> }
>
> module_init(test_mce_init);
> module_exit(test_mce_exit);
>
> MODULE_LICENSE("GPL");
>
> 8<--------------------------------------------------------------------
>
> [ 14.977053] mce: loading out-of-tree module taints kernel.
> [ 15.004285] Disabling lock debugging due to kernel taint
> [ 15.026151] Machine check in kernel mode.
> [ 15.030153] Caused by (from SRR1=41000): [ 15.033982] Transfer
> error ack signal
> [ 15.037652] Oops: Machine check 1, sig: 7 [#1]
> [ 15.042088] PREEMPT [ 15.044091] MPC8378_CUSTOM
> [ 15.046967] Modules linked in: mce(O+) iptable_filter ip_tables
> x_tables ipv6 mpc8xxx_wdt yaffs spidev spi_fsl_spi spi_fsl_lib
> spi_fsl_cpm fsl_mph_dr_of ehci_fsl ehci_hcd
> [ 15.067486] CPU: 0 PID: 1213 Comm: insmod Tainted: G M C O
> 4.9.191-default-mpc8378-p3c692a64ae1d #31
> [ 15.078175] task: 9e83e550 task.stack: 9ea2e000
> [ 15.082699] NIP: 00000900 LR: b147e030 CTR: 80015d50
> [ 15.087659] REGS: 9ea2fca0 TRAP: 0200 Tainted: G M C O
> (4.9.191-default-mpc8378-p3c692a64ae1d)
> [ 15.098084] MSR: 00041000 <ME>[ 15.100973] CR: 42002228 XER: 00000000
> [ 15.104976] DAR: 80017414 DSISR: 00000000
> GPR00: b147e030 9ea2fd50 9e83e550 00000000 b1480000 9c652200 9ea2fd18 00000000
> GPR08: 9c652200 00000000 b1480000 00001032 80015d50 100b93d0 b147e308 805eb3d8
> GPR16: 0000003a 00000550 b1473b5c b147c2a4 8048e444 80082b08 00000000 b147c0e8
> GPR24: 00000124 00000578 00000000 00000000 b147c0a0 b147e000 9eb7c280 b147c0a0
> NIP [00000900] 0x900
> [ 15.139310] LR [b147e030] test_mce_init+0x30/0xa8 [mce]
> [ 15.144528] Call Trace:
> [ 15.146973] [9ea2fd50] [b147e000] test_mce_init+0x0/0xa8 [mce] (unreliable)
> [ 15.153940] [9ea2fd60] [b147e030] test_mce_init+0x30/0xa8 [mce]
> [ 15.159864] [9ea2fd70] [80003ac4] do_one_initcall+0xbc/0x184
> [ 15.165527] [9ea2fde0] [804857e8] do_init_module+0x64/0x1e4
> [ 15.171107] [9ea2fe00] [80086014] load_module+0x1c78/0x2268
> [ 15.176680] [9ea2fec0] [80086780] SyS_init_module+0x17c/0x190
> [ 15.182433] [9ea2ff40] [80010acc] ret_from_syscall+0x0/0x38
> [ 15.188005] --- interrupt: c01 at 0xfdfdb64
> [ 15.188005] LR = 0x10013c64
> [ 15.195309] Instruction dump:
> [ 15.198274] 00000000 XXXXXXXX 00000000 XXXXXXXX 00000000 XXXXXXXX
> 00000000 XXXXXXXX
> [ 15.206047] 00000000 XXXXXXXX 00000000 XXXXXXXX 7d5043a6 XXXXXXXX
> 7d400026 XXXXXXXX
> [ 15.213824] ---[ end trace d38922938e009d45 ]---
> [ 15.218434]
> [ 16.219951] Kernel panic - not syncing: Fatal exception
> [ 16.225174] Rebooting in 1 seconds..
>
^ permalink raw reply
* MCE handler gets NIP wrong on MPC8378
From: Radu Rendec @ 2020-02-18 17:07 UTC (permalink / raw)
To: linuxppc-dev
Hi Everyone,
The saved NIP seems to be broken inside machine_check_exception() on
MPC8378, running Linux 4.9.191. The value is 0x900 most of the times,
but I have seen other weird values.
I've been able to track down the entry code to head_32.S (vector 0x200),
but I'm not sure where/how the NIP value (where the exception occurred)
is captured.
I also noticed most of the code has moved to head_32.h in newer kernel
versions, but EXCEPTION_PROLOG_1 and EXCEPTION_PROLOG_2 look pretty much
the same. I guess the same thing happens on a recent kernel, even though
I don't have an easy way to test it.
The original MCE that I see is triggered by a failed PCIe transaction,
but I was able to reproduce it by just reading from a (physically)
unmapped memory area. Sample code and kernel crash dump are included
below.
Can anyone please provide any suggestion as to what to look at next?
Thanks,
Radu
8<--------------------------------------------------------------------
#include <linux/module.h>
#include <linux/delay.h>
#include <asm/io.h>
static void __iomem *bad_addr_base;
static int __init test_mce_init(void)
{
unsigned int x;
bad_addr_base = ioremap(0xf0000000, 0x100);
if (bad_addr_base) {
__asm__ __volatile__ ("isync");
x = ioread32(bad_addr_base);
pr_info("Test: %#0x\n", x);
} else
pr_err("Cannot map\n");
return 0;
}
static void __exit test_mce_exit(void)
{
iounmap(bad_addr_base);
}
module_init(test_mce_init);
module_exit(test_mce_exit);
MODULE_LICENSE("GPL");
8<--------------------------------------------------------------------
[ 14.977053] mce: loading out-of-tree module taints kernel.
[ 15.004285] Disabling lock debugging due to kernel taint
[ 15.026151] Machine check in kernel mode.
[ 15.030153] Caused by (from SRR1=41000): [ 15.033982] Transfer
error ack signal
[ 15.037652] Oops: Machine check 1, sig: 7 [#1]
[ 15.042088] PREEMPT [ 15.044091] MPC8378_CUSTOM
[ 15.046967] Modules linked in: mce(O+) iptable_filter ip_tables
x_tables ipv6 mpc8xxx_wdt yaffs spidev spi_fsl_spi spi_fsl_lib
spi_fsl_cpm fsl_mph_dr_of ehci_fsl ehci_hcd
[ 15.067486] CPU: 0 PID: 1213 Comm: insmod Tainted: G M C O
4.9.191-default-mpc8378-p3c692a64ae1d #31
[ 15.078175] task: 9e83e550 task.stack: 9ea2e000
[ 15.082699] NIP: 00000900 LR: b147e030 CTR: 80015d50
[ 15.087659] REGS: 9ea2fca0 TRAP: 0200 Tainted: G M C O
(4.9.191-default-mpc8378-p3c692a64ae1d)
[ 15.098084] MSR: 00041000 <ME>[ 15.100973] CR: 42002228 XER: 00000000
[ 15.104976] DAR: 80017414 DSISR: 00000000
GPR00: b147e030 9ea2fd50 9e83e550 00000000 b1480000 9c652200 9ea2fd18 00000000
GPR08: 9c652200 00000000 b1480000 00001032 80015d50 100b93d0 b147e308 805eb3d8
GPR16: 0000003a 00000550 b1473b5c b147c2a4 8048e444 80082b08 00000000 b147c0e8
GPR24: 00000124 00000578 00000000 00000000 b147c0a0 b147e000 9eb7c280 b147c0a0
NIP [00000900] 0x900
[ 15.139310] LR [b147e030] test_mce_init+0x30/0xa8 [mce]
[ 15.144528] Call Trace:
[ 15.146973] [9ea2fd50] [b147e000] test_mce_init+0x0/0xa8 [mce] (unreliable)
[ 15.153940] [9ea2fd60] [b147e030] test_mce_init+0x30/0xa8 [mce]
[ 15.159864] [9ea2fd70] [80003ac4] do_one_initcall+0xbc/0x184
[ 15.165527] [9ea2fde0] [804857e8] do_init_module+0x64/0x1e4
[ 15.171107] [9ea2fe00] [80086014] load_module+0x1c78/0x2268
[ 15.176680] [9ea2fec0] [80086780] SyS_init_module+0x17c/0x190
[ 15.182433] [9ea2ff40] [80010acc] ret_from_syscall+0x0/0x38
[ 15.188005] --- interrupt: c01 at 0xfdfdb64
[ 15.188005] LR = 0x10013c64
[ 15.195309] Instruction dump:
[ 15.198274] 00000000 XXXXXXXX 00000000 XXXXXXXX 00000000 XXXXXXXX
00000000 XXXXXXXX
[ 15.206047] 00000000 XXXXXXXX 00000000 XXXXXXXX 7d5043a6 XXXXXXXX
7d400026 XXXXXXXX
[ 15.213824] ---[ end trace d38922938e009d45 ]---
[ 15.218434]
[ 16.219951] Kernel panic - not syncing: Fatal exception
[ 16.225174] Rebooting in 1 seconds..
^ permalink raw reply
* Re: [PATCH 2/2] powerpc: avoid adjusting memory_limit for capture kernel memory reservation
From: Michal Suchánek @ 2020-02-18 16:34 UTC (permalink / raw)
To: Hari Bathini
Cc: linuxppc-dev, Mahesh J Salgaonkar, Ananth N Mavinakayanahalli
In-Reply-To: <156166327993.13320.10788410344711883330.stgit@hbathini.in.ibm.com>
On Fri, Jun 28, 2019 at 12:51:19AM +0530, Hari Bathini wrote:
> Currently, if memory_limit is specified and it overlaps with memory to
> be reserved for capture kernel, memory_limit is adjusted to accommodate
> capture kernel. With memory reservation for capture kernel moved later
> (after enforcing memory limit), this adjustment no longer holds water.
> So, avoid adjusting memory_limit and error out instead.
The adjustment of memory limit does not look quite sound
- There is no code to undo the adjustment in case reservation fails
- I don't think reservation is still forced to the end of memory
causing the kernel to use memory it was supposed not to
- The CMA reservation again causes teh reserved memory to be used
- Finally the CMA reservation makes this obsolete because the reserved
memory is can be used by the system
>
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
Reviewed-by: Michal Suchanek <msuchanek@suse.de>
> ---
> arch/powerpc/kernel/fadump.c | 16 ----------------
> arch/powerpc/kernel/machine_kexec.c | 22 +++++++++++-----------
> 2 files changed, 11 insertions(+), 27 deletions(-)
>
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 4eab972..a784695 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -476,22 +476,6 @@ int __init fadump_reserve_mem(void)
> #endif
> }
>
> - /*
> - * Calculate the memory boundary.
> - * If memory_limit is less than actual memory boundary then reserve
> - * the memory for fadump beyond the memory_limit and adjust the
> - * memory_limit accordingly, so that the running kernel can run with
> - * specified memory_limit.
> - */
> - if (memory_limit && memory_limit < memblock_end_of_DRAM()) {
> - size = get_fadump_area_size();
> - if ((memory_limit + size) < memblock_end_of_DRAM())
> - memory_limit += size;
> - else
> - memory_limit = memblock_end_of_DRAM();
> - printk(KERN_INFO "Adjusted memory_limit for firmware-assisted"
> - " dump, now %#016llx\n", memory_limit);
> - }
> if (memory_limit)
> memory_boundary = memory_limit;
> else
> diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
> index c4ed328..fc5533b 100644
> --- a/arch/powerpc/kernel/machine_kexec.c
> +++ b/arch/powerpc/kernel/machine_kexec.c
> @@ -125,10 +125,8 @@ void __init reserve_crashkernel(void)
> crashk_res.end = crash_base + crash_size - 1;
> }
>
> - if (crashk_res.end == crashk_res.start) {
> - crashk_res.start = crashk_res.end = 0;
> - return;
> - }
> + if (crashk_res.end == crashk_res.start)
> + goto error_out;
>
> /* We might have got these values via the command line or the
> * device tree, either way sanitise them now. */
> @@ -170,15 +168,13 @@ void __init reserve_crashkernel(void)
> if (overlaps_crashkernel(__pa(_stext), _end - _stext)) {
> printk(KERN_WARNING
> "Crash kernel can not overlap current kernel\n");
> - crashk_res.start = crashk_res.end = 0;
> - return;
> + goto error_out;
> }
>
> /* Crash kernel trumps memory limit */
> if (memory_limit && memory_limit <= crashk_res.end) {
> - memory_limit = crashk_res.end + 1;
> - printk("Adjusted memory limit for crashkernel, now 0x%llx\n",
> - memory_limit);
> + pr_err("Crash kernel size can't exceed memory_limit\n");
> + goto error_out;
> }
>
> printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
> @@ -190,9 +186,13 @@ void __init reserve_crashkernel(void)
> if (!memblock_is_region_memory(crashk_res.start, crash_size) ||
> memblock_reserve(crashk_res.start, crash_size)) {
> pr_err("Failed to reserve memory for crashkernel!\n");
> - crashk_res.start = crashk_res.end = 0;
> - return;
> + goto error_out;
> }
> +
> + return;
> +error_out:
> + crashk_res.start = crashk_res.end = 0;
> + return;
> }
>
> int overlaps_crashkernel(unsigned long start, unsigned long size)
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox