* Re: [PATCH] macintosh: therm_windtunnel: fix regression when instantiating devices
From: Wolfram Sang @ 2020-02-28 17:00 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Mathieu Malaterre, debian-powerpc, linux-i2c, Erhard Furtner
In-Reply-To: <20200225141229.5424-1-wsa@the-dreams.de>
[-- Attachment #1: Type: text/plain, Size: 1422 bytes --]
On Tue, Feb 25, 2020 at 03:12:29PM +0100, Wolfram Sang wrote:
> Removing attach_adapter from this driver caused a regression for at
> least some machines. Those machines had the sensors described in their
> DT, too, so they didn't need manual creation of the sensor devices. The
> old code worked, though, because manual creation came first. Creation of
> DT devices then failed later and caused error logs, but the sensors
> worked nonetheless because of the manually created devices.
>
> When removing attach_adaper, manual creation now comes later and loses
> the race. The sensor devices were already registered via DT, yet with
> another binding, so the driver could not be bound to it.
>
> This fix refactors the code to remove the race and only manually creates
> devices if there are no DT nodes present. Also, the DT binding is updated
> to match both, the DT and manually created devices. Because we don't
> know which device creation will be used at runtime, the code to start
> the kthread is moved to do_probe() which will be called by both methods.
>
> Fixes: 3e7bed52719d ("macintosh: therm_windtunnel: drop using attach_adapter")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=201723
> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
> Tested-by: Erhard Furtner <erhard_f@mailbox.org>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Applied to for-current, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] macintosh: therm_windtunnel: fix regression when instantiating devices
From: Wolfram Sang @ 2020-02-28 17:00 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mathieu Malaterre, Erhard Furtner, debian-powerpc, linuxppc-dev,
linux-i2c
In-Reply-To: <87imjsu8k2.fsf@mpe.ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 169 bytes --]
> I think that would be best, it's more I2C related than powerpc arch
> stuff that I could review.
It is more DT handling than I2C, but I am happy to take this patch.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* RE: [PATCH v4 00/11] Add the multiple PF support for DWC and Layerscape
From: Xiaowei Bao @ 2020-02-28 14:52 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: mark.rutland@arm.com, Roy Zang, devicetree@vger.kernel.org,
jingoohan1@gmail.com, linuxppc-dev@lists.ozlabs.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
kishon@ti.com, M.h. Lian, robh+dt@kernel.org,
linux-arm-kernel@lists.infradead.org,
gustavo.pimentel@synopsys.com, Leo Li, shawnguo@kernel.org,
Mingkai Hu, Andrew Murray
In-Reply-To: <20200228113010.GB4064@e121166-lin.cambridge.arm.com>
> -----Original Message-----
> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Sent: 2020年2月28日 19:30
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: robh+dt@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org; Leo
> Li <leoyang.li@nxp.com>; kishon@ti.com; M.h. Lian
> <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Roy Zang
> <roy.zang@nxp.com>; jingoohan1@gmail.com;
> gustavo.pimentel@synopsys.com; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Andrew Murray
> <amurray@thegoodpenguin.co.uk>
> Subject: Re: [PATCH v4 00/11] Add the multiple PF support for DWC and
> Layerscape
>
> On Tue, Sep 24, 2019 at 10:18:38AM +0800, Xiaowei Bao wrote:
> > Add the PCIe EP multiple PF support for DWC and Layerscape, add the
> > doorbell MSIX function for DWC, use list to manage the PF of one PCIe
> > controller, and refactor the Layerscape EP driver due to some
> > platforms difference.
> >
> > Xiaowei Bao (11):
> > PCI: designware-ep: Add multiple PFs support for DWC
> > PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
> > PCI: designware-ep: Move the function of getting MSI capability
> > forward
> > PCI: designware-ep: Modify MSI and MSIX CAP way of finding
> > dt-bindings: pci: layerscape-pci: add compatible strings for ls1088a
> > and ls2088a
> > PCI: layerscape: Fix some format issue of the code
> > PCI: layerscape: Modify the way of getting capability with different
> > PEX
> > PCI: layerscape: Modify the MSIX to the doorbell mode
> > PCI: layerscape: Add EP mode support for ls1088a and ls2088a
> > arm64: dts: layerscape: Add PCIe EP node for ls1088a
> > misc: pci_endpoint_test: Add LS1088a in pci_device_id table
> >
> > .../devicetree/bindings/pci/layerscape-pci.txt | 2 +
> > arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 31 +++
> > drivers/misc/pci_endpoint_test.c | 2 +
> > drivers/pci/controller/dwc/pci-layerscape-ep.c | 100 ++++++--
> > drivers/pci/controller/dwc/pcie-designware-ep.c | 255
> +++++++++++++++++----
> > drivers/pci/controller/dwc/pcie-designware.c | 59 +++--
> > drivers/pci/controller/dwc/pcie-designware.h | 48 +++-
> > 7 files changed, 404 insertions(+), 93 deletions(-)
>
> Hi,
>
> are you resending this patchset ? I would also like Andrew and Kishon to have
> a look and ACK relevant code before merging it.
Thank you for following these patches, I have replay the email to Kishon, and I will
decide whether to resend the sets patches based on his comments.
Thanks
Xiaowei
> Thanks,
> Lorenzo
^ permalink raw reply
* RE: [PATCH v4 08/11] PCI: layerscape: Modify the MSIX to the doorbell mode
From: Xiaowei Bao @ 2020-02-28 14:43 UTC (permalink / raw)
To: Kishon Vijay Abraham I, robh+dt@kernel.org, mark.rutland@arm.com,
shawnguo@kernel.org, Leo Li, lorenzo.pieralisi@arm.com, M.h. Lian,
Mingkai Hu, Roy Zang, jingoohan1@gmail.com,
gustavo.pimentel@synopsys.com, andrew.murray@arm.com,
linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <55752e77-1094-4cbf-2822-e10b875b4dd9@ti.com>
> -----Original Message-----
> From: Kishon Vijay Abraham I <kishon@ti.com>
> Sent: 2020年2月28日 19:41
> To: Xiaowei Bao <xiaowei.bao@nxp.com>; robh+dt@kernel.org;
> mark.rutland@arm.com; shawnguo@kernel.org; Leo Li
> <leoyang.li@nxp.com>; lorenzo.pieralisi@arm.com; M.h. Lian
> <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Roy Zang
> <roy.zang@nxp.com>; jingoohan1@gmail.com;
> gustavo.pimentel@synopsys.com; andrew.murray@arm.com;
> linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH v4 08/11] PCI: layerscape: Modify the MSIX to the
> doorbell mode
>
> Hi Xiaowei,
>
> On 24/09/19 7:48 am, Xiaowei Bao wrote:
> > dw_pcie_ep_raise_msix_irq was never called in the exisitng driver
> > before, because the ls1046a platform don't support the MSIX feature
> > and msix_capable was always set to false.
> > Now that add the ls1088a platform with MSIX support, but the existing
> > dw_pcie_ep_raise_msix_irq doesn't work, so use the doorbell method to
> > support the MSIX feature.
>
>
> It does work after [1]. So the commit message might not be exactly true.
Got it, I will verify it with your patch, do you mean that I should correct the commit
message? I think we can reserve my MSI-X patch, this patch can provide another
MSI-X trigger way, and this way is clearly stated in the DWC manual, thanks, please
give your comments.
Thanks
Xiaowei
>
> [1] ->
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ke
> rnel.org%2Fr%2F20200225081703.8857-1-kishon%40ti.com&data=02%
> 7C01%7Cxiaowei.bao%40nxp.com%7C84191df0cd09451ef3e608d7bc427745
> %7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637184865969684
> 169&sdata=K9fvbpQ4xuuZKhWT6UV2M4SVyHxQ3LjpuJQztktLaRI%3D&
> amp;reserved=0
>
> Thanks
> Kishon
>
> >
> > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > Reviewed-by: Andrew Murray <andrew.murray@arm.com>
> > ---
> > v2:
> > - No change
> > v3:
> > - Modify the commit message make it clearly.
> > v4:
> > - No change
> >
> > drivers/pci/controller/dwc/pci-layerscape-ep.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > index 1e07287..5f0cb99 100644
> > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > @@ -79,7 +79,8 @@ static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep,
> u8 func_no,
> > case PCI_EPC_IRQ_MSI:
> > return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> > case PCI_EPC_IRQ_MSIX:
> > - return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
> > + return dw_pcie_ep_raise_msix_irq_doorbell(ep, func_no,
> > + interrupt_num);
> > default:
> > dev_err(pci->dev, "UNKNOWN IRQ type\n");
> > return -EINVAL;
> >
^ permalink raw reply
* Re: [PATCH v4 08/11] PCI: layerscape: Modify the MSIX to the doorbell mode
From: Kishon Vijay Abraham I @ 2020-02-28 11:40 UTC (permalink / raw)
To: Xiaowei Bao, robh+dt, mark.rutland, shawnguo, leoyang.li,
lorenzo.pieralisi, minghuan.Lian, mingkai.hu, roy.zang,
jingoohan1, gustavo.pimentel, andrew.murray, linux-pci,
linux-arm-kernel, devicetree, linux-kernel, linuxppc-dev
In-Reply-To: <20190924021849.3185-9-xiaowei.bao@nxp.com>
Hi Xiaowei,
On 24/09/19 7:48 am, Xiaowei Bao wrote:
> dw_pcie_ep_raise_msix_irq was never called in the exisitng driver
> before, because the ls1046a platform don't support the MSIX feature
> and msix_capable was always set to false.
> Now that add the ls1088a platform with MSIX support, but the existing
> dw_pcie_ep_raise_msix_irq doesn't work, so use the doorbell method to
> support the MSIX feature.
It does work after [1]. So the commit message might not be exactly true.
[1] -> https://lore.kernel.org/r/20200225081703.8857-1-kishon@ti.com
Thanks
Kishon
>
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
> ---
> v2:
> - No change
> v3:
> - Modify the commit message make it clearly.
> v4:
> - No change
>
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 1e07287..5f0cb99 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -79,7 +79,8 @@ static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> case PCI_EPC_IRQ_MSI:
> return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> case PCI_EPC_IRQ_MSIX:
> - return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
> + return dw_pcie_ep_raise_msix_irq_doorbell(ep, func_no,
> + interrupt_num);
> default:
> dev_err(pci->dev, "UNKNOWN IRQ type\n");
> return -EINVAL;
>
^ permalink raw reply
* Re: [PATCH] selftests: pidfd: Add pidfd_fdinfo_test in .gitignore
From: Christian Brauner @ 2020-02-28 13:41 UTC (permalink / raw)
To: Christophe Leroy, Christian Kellner, Shuah Khan
Cc: linux-mm, linuxppc-dev, linux-kernel, linux-kselftest
In-Reply-To: <DB631DFB-DF8B-4B95-AC50-74F1ED733CAE@ubuntu.com>
On Fri, Feb 28, 2020 at 01:18:44AM +0100, Christian Brauner wrote:
> On February 28, 2020 1:00:08 AM GMT+01:00, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> >The commit identified below added pidfd_fdinfo_test
> >but failed to add it to .gitignore
> >
> >Fixes: 2def297ec7fb ("pidfd: add tests for NSpid info in fdinfo")
> >Cc: stable@vger.kernel.org
> >Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> >---
> > tools/testing/selftests/pidfd/.gitignore | 1 +
> > 1 file changed, 1 insertion(+)
> >
> >diff --git a/tools/testing/selftests/pidfd/.gitignore
> >b/tools/testing/selftests/pidfd/.gitignore
> >index 3a779c084d96..39559d723c41 100644
> >--- a/tools/testing/selftests/pidfd/.gitignore
> >+++ b/tools/testing/selftests/pidfd/.gitignore
> >@@ -2,4 +2,5 @@ pidfd_open_test
> > pidfd_poll_test
> > pidfd_test
> > pidfd_wait
> >+pidfd_fdinfo_test
> > pidfd_getfd_test
>
> Thanks for spotting this.
> I'll pick this up along with other fixes I have waiting.
>
> Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Applied, thanks!
Christian
^ permalink raw reply
* Re: [PATCH v4 00/11] Add the multiple PF support for DWC and Layerscape
From: Lorenzo Pieralisi @ 2020-02-28 11:30 UTC (permalink / raw)
To: Xiaowei Bao
Cc: mark.rutland, roy.zang, devicetree, jingoohan1, linuxppc-dev,
linux-pci, linux-kernel, kishon, minghuan.Lian, robh+dt,
linux-arm-kernel, gustavo.pimentel, leoyang.li, shawnguo,
mingkai.hu, Andrew Murray
In-Reply-To: <20190924021849.3185-1-xiaowei.bao@nxp.com>
On Tue, Sep 24, 2019 at 10:18:38AM +0800, Xiaowei Bao wrote:
> Add the PCIe EP multiple PF support for DWC and Layerscape, add
> the doorbell MSIX function for DWC, use list to manage the PF of
> one PCIe controller, and refactor the Layerscape EP driver due to
> some platforms difference.
>
> Xiaowei Bao (11):
> PCI: designware-ep: Add multiple PFs support for DWC
> PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
> PCI: designware-ep: Move the function of getting MSI capability
> forward
> PCI: designware-ep: Modify MSI and MSIX CAP way of finding
> dt-bindings: pci: layerscape-pci: add compatible strings for ls1088a
> and ls2088a
> PCI: layerscape: Fix some format issue of the code
> PCI: layerscape: Modify the way of getting capability with different
> PEX
> PCI: layerscape: Modify the MSIX to the doorbell mode
> PCI: layerscape: Add EP mode support for ls1088a and ls2088a
> arm64: dts: layerscape: Add PCIe EP node for ls1088a
> misc: pci_endpoint_test: Add LS1088a in pci_device_id table
>
> .../devicetree/bindings/pci/layerscape-pci.txt | 2 +
> arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 31 +++
> drivers/misc/pci_endpoint_test.c | 2 +
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 100 ++++++--
> drivers/pci/controller/dwc/pcie-designware-ep.c | 255 +++++++++++++++++----
> drivers/pci/controller/dwc/pcie-designware.c | 59 +++--
> drivers/pci/controller/dwc/pcie-designware.h | 48 +++-
> 7 files changed, 404 insertions(+), 93 deletions(-)
Hi,
are you resending this patchset ? I would also like Andrew and Kishon to
have a look and ACK relevant code before merging it.
Thanks,
Lorenzo
^ permalink raw reply
* [PATCHv2 2/2] pSeries/papr_scm: buffer pmem's bound addr in dt for kexec kernel
From: Pingfan Liu @ 2020-02-28 9:41 UTC (permalink / raw)
To: linuxppc-dev
Cc: Andrew Donnellan, kexec, Pingfan Liu, Paul Mackerras,
Aneesh Kumar K . V, Oliver O'Halloran, Dan Williams,
Hari Bathini
In-Reply-To: <1582882895-3142-1-git-send-email-kernelfans@gmail.com>
At present, plpar_hcall(H_SCM_BIND_MEM, ...) takes a very long time, so
if dumping to fsdax, it will take a very long time.
Take a closer look, during the papr_scm initialization, the only
configuration is through drc_pmem_bind()-> plpar_hcall(H_SCM_BIND_MEM,
...), which helps to set up the bound address.
On pseries, for kexec -l/-p kernel, there is no reset of hardware, and this
step can be stepped around to save times. So the pmem bound address can be
passed to the 2nd kernel through a dynamic added property "bound-addr" in
dt node 'ibm,pmemory'.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Andrew Donnellan <ajd@linux.ibm.com>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: kexec@lists.infradead.org
---
note: This patch has not been tested since I can not get such a pseries with pmem.
Please kindly to give some suggestion, thanks.
arch/powerpc/platforms/pseries/papr_scm.c | 32 +++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 0b4467e..40cd214 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -14,6 +14,7 @@
#include <linux/delay.h>
#include <asm/plpar_wrappers.h>
+#include "of_helpers.h"
#define BIND_ANY_ADDR (~0ul)
@@ -383,7 +384,7 @@ static int papr_scm_probe(struct platform_device *pdev)
{
struct device_node *dn = pdev->dev.of_node;
u32 drc_index, metadata_size;
- u64 blocks, block_size;
+ u64 blocks, block_size, bound_addr = 0;
struct papr_scm_priv *p;
const char *uuid_str;
u64 uuid[2];
@@ -440,17 +441,28 @@ static int papr_scm_probe(struct platform_device *pdev)
p->metadata_size = metadata_size;
p->pdev = pdev;
- /* request the hypervisor to bind this region to somewhere in memory */
- rc = drc_pmem_bind(p);
+ of_property_read_u64(dn, "bound-addr", &bound_addr);
+ if (bound_addr) {
+ p->bound_addr = bound_addr;
+ } else {
+ struct property *property;
+ u64 big;
- /* If phyp says drc memory still bound then force unbound and retry */
- if (rc == H_OVERLAP)
- rc = drc_pmem_query_n_bind(p);
+ /* request the hypervisor to bind this region to somewhere in memory */
+ rc = drc_pmem_bind(p);
- if (rc != H_SUCCESS) {
- dev_err(&p->pdev->dev, "bind err: %d\n", rc);
- rc = -ENXIO;
- goto err;
+ /* If phyp says drc memory still bound then force unbound and retry */
+ if (rc == H_OVERLAP)
+ rc = drc_pmem_query_n_bind(p);
+
+ if (rc != H_SUCCESS) {
+ dev_err(&p->pdev->dev, "bind err: %d\n", rc);
+ rc = -ENXIO;
+ goto err;
+ }
+ big = cpu_to_be64(p->bound_addr);
+ property = new_property("bound-addr", sizeof(u64), &big, NULL);
+ of_add_property(dn, property);
}
/* setup the resource for the newly bound range */
--
2.7.5
^ permalink raw reply related
* [PATCHv2 1/2] powerpc/of: split out new_property() for reusing
From: Pingfan Liu @ 2020-02-28 9:41 UTC (permalink / raw)
To: linuxppc-dev
Cc: Andrew Donnellan, kexec, Pingfan Liu, Paul Mackerras,
Aneesh Kumar K . V, Oliver O'Halloran, Dan Williams,
Hari Bathini
Splitting out new_property() for coming reusing and moving it to
of_helpers.c.
Also do some coding style cleanup.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Andrew Donnellan <ajd@linux.ibm.com>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: kexec@lists.infradead.org
---
arch/powerpc/platforms/pseries/of_helpers.c | 28 ++++++++++++++++++++++++++++
arch/powerpc/platforms/pseries/of_helpers.h | 3 +++
arch/powerpc/platforms/pseries/reconfig.c | 26 --------------------------
3 files changed, 31 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
index 66dfd82..1022e0f 100644
--- a/arch/powerpc/platforms/pseries/of_helpers.c
+++ b/arch/powerpc/platforms/pseries/of_helpers.c
@@ -7,6 +7,34 @@
#include "of_helpers.h"
+struct property *new_property(const char *name, const int length,
+ const unsigned char *value, struct property *last)
+{
+ struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
+
+ if (!new)
+ return NULL;
+
+ new->name = kstrdup(name, GFP_KERNEL);
+ if (!new->name)
+ goto cleanup;
+ new->value = kmalloc(length + 1, GFP_KERNEL);
+ if (!new->value)
+ goto cleanup;
+
+ memcpy(new->value, value, length);
+ *(((char *)new->value) + length) = 0;
+ new->length = length;
+ new->next = last;
+ return new;
+
+cleanup:
+ kfree(new->name);
+ kfree(new->value);
+ kfree(new);
+ return NULL;
+}
+
/**
* pseries_of_derive_parent - basically like dirname(1)
* @path: the full_name of a node to be added to the tree
diff --git a/arch/powerpc/platforms/pseries/of_helpers.h b/arch/powerpc/platforms/pseries/of_helpers.h
index decad65..34add82 100644
--- a/arch/powerpc/platforms/pseries/of_helpers.h
+++ b/arch/powerpc/platforms/pseries/of_helpers.h
@@ -4,6 +4,9 @@
#include <linux/of.h>
+struct property *new_property(const char *name, const int length,
+ const unsigned char *value, struct property *last);
+
struct device_node *pseries_of_derive_parent(const char *path);
#endif /* _PSERIES_OF_HELPERS_H */
diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
index 7f7369f..8e5a2ba 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -165,32 +165,6 @@ static char * parse_next_property(char *buf, char *end, char **name, int *length
return tmp;
}
-static struct property *new_property(const char *name, const int length,
- const unsigned char *value, struct property *last)
-{
- struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
-
- if (!new)
- return NULL;
-
- if (!(new->name = kstrdup(name, GFP_KERNEL)))
- goto cleanup;
- if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
- goto cleanup;
-
- memcpy(new->value, value, length);
- *(((char *)new->value) + length) = 0;
- new->length = length;
- new->next = last;
- return new;
-
-cleanup:
- kfree(new->name);
- kfree(new->value);
- kfree(new);
- return NULL;
-}
-
static int do_add_node(char *buf, size_t bufsize)
{
char *path, *end, *name;
--
2.7.5
^ permalink raw reply related
* Re: [PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel
From: Pingfan Liu @ 2020-02-28 9:27 UTC (permalink / raw)
To: Christophe Leroy
Cc: Aneesh Kumar K . V, Kexec Mailing List, Oliver O'Halloran,
Paul Mackerras, Dan Williams, linuxppc-dev, Hari Bathini
In-Reply-To: <2a1906b0-6c53-4ff5-78ea-3254f8224fe4@c-s.fr>
On Fri, Feb 28, 2020 at 2:52 PM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
>
>
> Le 28/02/2020 à 06:53, Pingfan Liu a écrit :
> > At present, plpar_hcall(H_SCM_BIND_MEM, ...) takes a very long time, so
> > if dumping to fsdax, it will take a very long time.
> >
> > Take a closer look, during the papr_scm initialization, the only
> > configuration is through drc_pmem_bind()-> plpar_hcall(H_SCM_BIND_MEM,
> > ...), which helps to set up the bound address.
> >
> > On pseries, for kexec -l/-p kernel, there is no reset of hardware, and this
> > step can be stepped around to save times. So the pmem bound address can be
> > passed to the 2nd kernel through a dynamic added property "bound-addr" in
> > dt node 'ibm,pmemory'.
> >
> > Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> > To: linuxppc-dev@lists.ozlabs.org
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Hari Bathini <hbathini@linux.ibm.com>
> > Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > Cc: Oliver O'Halloran <oohall@gmail.com>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Cc: kexec@lists.infradead.org
> > ---
> > note: I can not find such a pseries machine, and not finish it yet.
> > ---
> > arch/powerpc/platforms/pseries/papr_scm.c | 32 +++++++++++++++++++++----------
> > 1 file changed, 22 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> > index c2ef320..555e746 100644
> > --- a/arch/powerpc/platforms/pseries/papr_scm.c
> > +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> > @@ -382,7 +382,7 @@ static int papr_scm_probe(struct platform_device *pdev)
> > {
> > struct device_node *dn = pdev->dev.of_node;
> > u32 drc_index, metadata_size;
> > - u64 blocks, block_size;
> > + u64 blocks, block_size, bound_addr = 0;
> > struct papr_scm_priv *p;
> > const char *uuid_str;
> > u64 uuid[2];
> > @@ -439,17 +439,29 @@ static int papr_scm_probe(struct platform_device *pdev)
> > p->metadata_size = metadata_size;
> > p->pdev = pdev;
> >
> > - /* request the hypervisor to bind this region to somewhere in memory */
> > - rc = drc_pmem_bind(p);
> > + of_property_read_u64(dn, "bound-addr", &bound_addr);
> > + if (bound_addr)
> > + p->bound_addr = bound_addr;
> > + else {
>
> All legs of an if/else must have { } when one leg need them, see codying
> style.
OK,
>
> > + struct property *property;
> > + u64 big;
> >
> > - /* If phyp says drc memory still bound then force unbound and retry */
> > - if (rc == H_OVERLAP)
> > - rc = drc_pmem_query_n_bind(p);
> > + /* request the hypervisor to bind this region to somewhere in memory */
> > + rc = drc_pmem_bind(p);
> >
> > - if (rc != H_SUCCESS) {
> > - dev_err(&p->pdev->dev, "bind err: %d\n", rc);
> > - rc = -ENXIO;
> > - goto err;
> > + /* If phyp says drc memory still bound then force unbound and retry */
> > + if (rc == H_OVERLAP)
> > + rc = drc_pmem_query_n_bind(p);
> > +
> > + if (rc != H_SUCCESS) {
> > + dev_err(&p->pdev->dev, "bind err: %d\n", rc);
> > + rc = -ENXIO;
> > + goto err;
> > + }
> > + big = cpu_to_be64(p->bound_addr);
> > + property = new_property("bound-addr", sizeof(u64), &big,
> > + NULL);
>
> Why plitting this line in two parts ? You have lines far longer above.
> In powerpc we allow lines 90 chars long.
OK, good to know it.
Thanks,
Pingfan
^ permalink raw reply
* Re: [PATCH] mm/debug: Add tests validating arch page table helpers for core features
From: Anshuman Khandual @ 2020-02-28 9:20 UTC (permalink / raw)
To: Christophe Leroy, linux-mm
Cc: Catalin Marinas, Heiko Carstens, Paul Mackerras, H. Peter Anvin,
linux-riscv, Will Deacon, linux-arch, linux-s390, x86,
Mike Rapoport, Christian Borntraeger, Ingo Molnar, linux-snps-arc,
Vasily Gorbik, Borislav Petkov, Paul Walmsley,
Kirill A . Shutemov, Thomas Gleixner, linux-arm-kernel,
Vineet Gupta, linux-kernel, Palmer Dabbelt, Andrew Morton,
linuxppc-dev
In-Reply-To: <51421bb3-9075-d7e9-1750-0553a1ebe64a@c-s.fr>
On 02/27/2020 04:12 PM, Christophe Leroy wrote:
>
>
> Le 27/02/2020 à 11:33, Anshuman Khandual a écrit :
>> This adds new tests validating arch page table helpers for these following
>> core memory features. These tests create and test specific mapping types at
>> various page table levels.
>>
>> * SPECIAL mapping
>> * PROTNONE mapping
>> * DEVMAP mapping
>> * SOFTDIRTY mapping
>> * SWAP mapping
>> * MIGRATION mapping
>> * HUGETLB mapping
>
> For testing HUGETLB mappings, you also have to include tests of hugepd functions/helpers. Not all archictures have hugepage size which matches with page tables levels (e.g. powerpc). Those architectures use hugepd_t.
Dont see much hugepd_t in generic HugeTLB. Just wondering which generic
hugepd helpers can be tested here. Could you please be bit more specific.
As we have not yet started looking for arch specific page table helpers
test requirements, all the test scenarios here need to be generic.
>
> Christophe
>
^ permalink raw reply
* [PATCH v3 2/2] powerpc/powernv: Re-enable imc trace-mode in kernel
From: Anju T Sudhakar @ 2020-02-28 7:50 UTC (permalink / raw)
To: mpe; +Cc: maddy, linuxppc-dev, anju
In-Reply-To: <20200228075043.28364-1-anju@linux.vnet.ibm.com>
commit <249fad734a25> ""powerpc/perf: Disable trace_imc pmu"
disables IMC(In-Memory Collection) trace-mode in kernel, since frequent
mode switching between accumulation mode and trace mode via the spr LDBAR
in the hardware can trigger a checkstop(system crash).
Patch to re-enable imc-trace mode in kernel.
The previous patch(1/2) in this series will address the mode switching issue
by implementing a global lock, and will restrict the usage of
accumulation and trace-mode at a time.
Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/opal-imc.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
index 000b350d4060..3b4518f4b643 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -278,14 +278,7 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
domain = IMC_DOMAIN_THREAD;
break;
case IMC_TYPE_TRACE:
- /*
- * FIXME. Using trace_imc events to monitor application
- * or KVM thread performance can cause a checkstop
- * (system crash).
- * Disable it for now.
- */
- pr_info_once("IMC: disabling trace_imc PMU\n");
- domain = -1;
+ domain = IMC_DOMAIN_TRACE;
break;
default:
pr_warn("IMC Unknown Device type \n");
--
2.20.1
^ permalink raw reply related
* [PATCH v3 1/2] powerpc/perf: Implement a global lock to avoid races between trace, core and thread imc events.
From: Anju T Sudhakar @ 2020-02-28 7:50 UTC (permalink / raw)
To: mpe; +Cc: maddy, linuxppc-dev, anju
IMC(In-memory Collection Counters) does performance monitoring in
two different modes, i.e accumulation mode(core-imc and thread-imc events),
and trace mode(trace-imc events). A cpu thread can either be in
accumulation-mode or trace-mode at a time and this is done via the LDBAR
register in POWER architecture. The current design does not address the
races between thread-imc and trace-imc events.
Patch implements a global id and lock to avoid the races between
core, trace and thread imc events. With this global id-lock
implementation, the system can either run core, thread or trace imc
events at a time. i.e. to run any core-imc events, thread/trace imc events
should not be enabled/monitored.
Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
---
Changes from v2->v3:
- Addressed the off-line comments from Michael Ellerman
- Optimized the *_event_init code path for trace, core and thread imc
- Handled the global refc in cpuhotplug scenario
- Re-order the patch series
- Removed the selftest patches and will send as a follow up patch
Changes from v1 -> v2:
- Added self test patches to the series.
---
arch/powerpc/perf/imc-pmu.c | 165 ++++++++++++++++++++++++++++++------
1 file changed, 141 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index cb50a9e1fd2d..a366e2ec0351 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -44,6 +44,16 @@ static DEFINE_PER_CPU(u64 *, trace_imc_mem);
static struct imc_pmu_ref *trace_imc_refc;
static int trace_imc_mem_size;
+/*
+ * Global data structure used to avoid races between thread,
+ * core and trace-imc
+ */
+static struct imc_pmu_ref imc_global_refc = {
+ .lock = __MUTEX_INITIALIZER(imc_global_refc.lock),
+ .id = 0,
+ .refc = 0,
+};
+
static struct imc_pmu *imc_event_to_pmu(struct perf_event *event)
{
return container_of(event->pmu, struct imc_pmu, pmu);
@@ -698,6 +708,13 @@ static int ppc_core_imc_cpu_offline(unsigned int cpu)
return -EINVAL;
ref->refc = 0;
+ /*
+ * Reduce the global reference count, if this is the
+ * last cpu in this core and core-imc event running
+ * in this cpu.
+ */
+ if (imc_global_refc.id == IMC_DOMAIN_CORE)
+ imc_global_refc.refc--;
}
return 0;
}
@@ -710,6 +727,23 @@ static int core_imc_pmu_cpumask_init(void)
ppc_core_imc_cpu_offline);
}
+static void reset_global_refc(struct perf_event *event)
+{
+ mutex_lock(&imc_global_refc.lock);
+ imc_global_refc.refc--;
+
+ /*
+ * If no other thread is running any
+ * event for this domain(thread/core/trace),
+ * set the global id to zero.
+ */
+ if (imc_global_refc.refc <= 0) {
+ imc_global_refc.refc = 0;
+ imc_global_refc.id = 0;
+ }
+ mutex_unlock(&imc_global_refc.lock);
+}
+
static void core_imc_counters_release(struct perf_event *event)
{
int rc, core_id;
@@ -759,6 +793,8 @@ static void core_imc_counters_release(struct perf_event *event)
ref->refc = 0;
}
mutex_unlock(&ref->lock);
+
+ reset_global_refc(event);
}
static int core_imc_event_init(struct perf_event *event)
@@ -819,6 +855,29 @@ static int core_imc_event_init(struct perf_event *event)
++ref->refc;
mutex_unlock(&ref->lock);
+ /*
+ * Since the system can run either in accumulation or trace-mode
+ * of IMC at a time, core-imc events are allowed only if no other
+ * trace/thread imc events are enabled/monitored.
+ *
+ * Take the global lock, and check the refc.id
+ * to know whether any other trace/thread imc
+ * events are running.
+ */
+ mutex_lock(&imc_global_refc.lock);
+ if (imc_global_refc.id == 0 || imc_global_refc.id == IMC_DOMAIN_CORE) {
+ /*
+ * No other trace/thread imc events are running in
+ * the system, so set the refc.id to core-imc.
+ */
+ imc_global_refc.id = IMC_DOMAIN_CORE;
+ imc_global_refc.refc++;
+ } else {
+ mutex_unlock(&imc_global_refc.lock);
+ return -EBUSY;
+ }
+ mutex_unlock(&imc_global_refc.lock);
+
event->hw.event_base = (u64)pcmi->vbase + (config & IMC_EVENT_OFFSET_MASK);
event->destroy = core_imc_counters_release;
return 0;
@@ -877,7 +936,20 @@ static int ppc_thread_imc_cpu_online(unsigned int cpu)
static int ppc_thread_imc_cpu_offline(unsigned int cpu)
{
- mtspr(SPRN_LDBAR, 0);
+ /*
+ * Set the bit 0 of LDBAR to zero.
+ *
+ * If bit 0 of LDBAR is unset, it will stop posting
+ * the counetr data to memory.
+ * For thread-imc, bit 0 of LDBAR will be set to 1 in the
+ * event_add function. So reset this bit here, to stop the updates
+ * to memory in the cpu_offline path.
+ */
+ mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) & (~(1UL << 63))));
+
+ /* Reduce the refc if thread-imc event running on this cpu */
+ if (imc_global_refc.id == IMC_DOMAIN_THREAD)
+ imc_global_refc.refc--;
return 0;
}
@@ -916,7 +988,22 @@ static int thread_imc_event_init(struct perf_event *event)
if (!target)
return -EINVAL;
+ mutex_lock(&imc_global_refc.lock);
+ /*
+ * Check if any other trace/core imc events are running in the
+ * system, if not set the global id to thread-imc.
+ */
+ if (imc_global_refc.id == 0 || imc_global_refc.id == IMC_DOMAIN_THREAD) {
+ imc_global_refc.id = IMC_DOMAIN_THREAD;
+ imc_global_refc.refc++;
+ } else {
+ mutex_unlock(&imc_global_refc.lock);
+ return -EBUSY;
+ }
+ mutex_unlock(&imc_global_refc.lock);
+
event->pmu->task_ctx_nr = perf_sw_context;
+ event->destroy = reset_global_refc;
return 0;
}
@@ -1063,10 +1150,12 @@ static void thread_imc_event_del(struct perf_event *event, int flags)
int core_id;
struct imc_pmu_ref *ref;
- mtspr(SPRN_LDBAR, 0);
-
core_id = smp_processor_id() / threads_per_core;
ref = &core_imc_refc[core_id];
+ if (!ref) {
+ pr_debug("imc: Failed to get event reference count\n");
+ return;
+ }
mutex_lock(&ref->lock);
ref->refc--;
@@ -1082,6 +1171,10 @@ static void thread_imc_event_del(struct perf_event *event, int flags)
ref->refc = 0;
}
mutex_unlock(&ref->lock);
+
+ /* Set bit 0 of LDBAR to zero, to stop posting updates to memory */
+ mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) & (~(1UL << 63))));
+
/*
* Take a snapshot and calculate the delta and update
* the event counter values.
@@ -1133,7 +1226,16 @@ static int ppc_trace_imc_cpu_online(unsigned int cpu)
static int ppc_trace_imc_cpu_offline(unsigned int cpu)
{
- mtspr(SPRN_LDBAR, 0);
+ /*
+ * No need to set bit 0 of LDBAR to zero, as
+ * it is set to zero for imc trace-mode
+ *
+ * Reduce the refc if any trace-imc event running
+ * on this cpu.
+ */
+ if (imc_global_refc.id == IMC_DOMAIN_TRACE)
+ imc_global_refc.refc--;
+
return 0;
}
@@ -1226,15 +1328,14 @@ static int trace_imc_event_add(struct perf_event *event, int flags)
local_mem = get_trace_imc_event_base_addr();
ldbar_value = ((u64)local_mem & THREAD_IMC_LDBAR_MASK) | TRACE_IMC_ENABLE;
- if (core_imc_refc)
- ref = &core_imc_refc[core_id];
+ /* trace-imc reference count */
+ if (trace_imc_refc)
+ ref = &trace_imc_refc[core_id];
if (!ref) {
- /* If core-imc is not enabled, use trace-imc reference count */
- if (trace_imc_refc)
- ref = &trace_imc_refc[core_id];
- if (!ref)
- return -EINVAL;
+ pr_debug("imc: Failed to get the event reference count\n");
+ return -EINVAL;
}
+
mtspr(SPRN_LDBAR, ldbar_value);
mutex_lock(&ref->lock);
if (ref->refc == 0) {
@@ -1242,13 +1343,11 @@ static int trace_imc_event_add(struct perf_event *event, int flags)
get_hard_smp_processor_id(smp_processor_id()))) {
mutex_unlock(&ref->lock);
pr_err("trace-imc: Unable to start the counters for core %d\n", core_id);
- mtspr(SPRN_LDBAR, 0);
return -EINVAL;
}
}
++ref->refc;
mutex_unlock(&ref->lock);
-
return 0;
}
@@ -1274,16 +1373,13 @@ static void trace_imc_event_del(struct perf_event *event, int flags)
int core_id = smp_processor_id() / threads_per_core;
struct imc_pmu_ref *ref = NULL;
- if (core_imc_refc)
- ref = &core_imc_refc[core_id];
+ if (trace_imc_refc)
+ ref = &trace_imc_refc[core_id];
if (!ref) {
- /* If core-imc is not enabled, use trace-imc reference count */
- if (trace_imc_refc)
- ref = &trace_imc_refc[core_id];
- if (!ref)
- return;
+ pr_debug("imc: Failed to get event reference count\n");
+ return;
}
- mtspr(SPRN_LDBAR, 0);
+
mutex_lock(&ref->lock);
ref->refc--;
if (ref->refc == 0) {
@@ -1297,6 +1393,7 @@ static void trace_imc_event_del(struct perf_event *event, int flags)
ref->refc = 0;
}
mutex_unlock(&ref->lock);
+
trace_imc_event_stop(event, flags);
}
@@ -1314,10 +1411,30 @@ static int trace_imc_event_init(struct perf_event *event)
if (event->attr.sample_period == 0)
return -ENOENT;
+ /*
+ * Take the global lock, and make sure
+ * no other thread is running any core/thread imc
+ * events
+ */
+ mutex_lock(&imc_global_refc.lock);
+ if (imc_global_refc.id == 0 || imc_global_refc.id == IMC_DOMAIN_TRACE) {
+ /*
+ * No core/thread imc events are running in the
+ * system, so set the refc.id to trace-imc.
+ */
+ imc_global_refc.id = IMC_DOMAIN_TRACE;
+ imc_global_refc.refc++;
+ } else {
+ mutex_unlock(&imc_global_refc.lock);
+ return -EBUSY;
+ }
+ mutex_unlock(&imc_global_refc.lock);
+
event->hw.idx = -1;
target = event->hw.target;
event->pmu->task_ctx_nr = perf_hw_context;
+ event->destroy = reset_global_refc;
return 0;
}
@@ -1429,10 +1546,10 @@ static void cleanup_all_core_imc_memory(void)
static void thread_imc_ldbar_disable(void *dummy)
{
/*
- * By Zeroing LDBAR, we disable thread-imc
- * updates.
+ * By setting 0th bit of LDBAR to zero, we disable thread-imc
+ * updates to memory.
*/
- mtspr(SPRN_LDBAR, 0);
+ mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) & (~(1UL << 63))));
}
void thread_imc_disable(void)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v3 25/27] powerpc/powernv/pmem: Expose the serial number in sysfs
From: Greg Kroah-Hartman @ 2020-02-28 7:15 UTC (permalink / raw)
To: Andrew Donnellan
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Mahesh Salgaonkar,
Masahiro Yamada, Oliver O'Halloran, Mauro Carvalho Chehab,
Ira Weiny, Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
alastair, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini,
Alastair D'Silva, linux-mm, linux-kernel, Vishal Verma,
Frederic Barrat, Paul Mackerras, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <96687fbf-38ab-13ff-ca19-ccb67bbc4405@linux.ibm.com>
On Fri, Feb 28, 2020 at 05:25:31PM +1100, Andrew Donnellan wrote:
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > +int ocxlpmem_sysfs_add(struct ocxlpmem *ocxlpmem)
> > +{
> > + int i, rc;
> > +
> > + for (i = 0; i < ARRAY_SIZE(attrs); i++) {
> > + rc = device_create_file(&ocxlpmem->dev, &attrs[i]);
> > + if (rc) {
> > + for (; --i >= 0;)
> > + device_remove_file(&ocxlpmem->dev, &attrs[i]);
>
> I'd rather avoid weird for loop constructs if possible.
>
> Is it actually dangerous to call device_remove_file() on an attr that hasn't
> been added? If not then I'd rather define an err: label and loop over the
> whole array there.
None of this should be used at all, just use attribute groups properly
and the driver core will handle this all for you.
device_create/remove_file should never be called by anyone anymore if at all
possible.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3 2/4] ASoC: fsl_asrc: Move common definition to fsl_asrc_common
From: Shengjiu Wang @ 2020-02-28 6:59 UTC (permalink / raw)
To: Nicolin Chen
Cc: Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, Rob Herring, Mark Brown,
linuxppc-dev, linux-kernel
In-Reply-To: <20200227183926.GA456@NICOLINC-LT.nvidia.com>
On Fri, Feb 28, 2020 at 2:41 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Thu, Feb 27, 2020 at 10:41:56AM +0800, Shengjiu Wang wrote:
> > There is a new ASRC included in i.MX serial platform, there
> > are some common definition can be shared with each other.
> > So move the common definition to a separate header file.
> >
> > And add fsl_asrc_pair_internal and fsl_asrc_internal for
> > the variable specific for the module, which can be used
> > internally.
>
> I think we can just call it "priv", instead of "internal", since
> it's passed by the "void *private" pointer.
>
> And it'd be nicer to have an extra preparational patch to rename
> existing "struct fsl_asrc *asrc_priv" to "struct fsl_asrc *asrc".
>
> Something like:
> struct fsl_asrc *asrc = yyyy;
> struct fsl_asrc_pair *pair = xxxx;
> struct fsl_asrc_priv *asrc_priv = asrc->private;
> struct fsl_asrc_pair_priv *pair_priv = pair->private;
>
> Thanks
> ------
>
ok, will change it.
best regards
wang shengjiu
^ permalink raw reply
* Re: [PATCH v3 1/4] ASoC: fsl_asrc: Change asrc_width to asrc_format
From: Shengjiu Wang @ 2020-02-28 6:56 UTC (permalink / raw)
To: Nicolin Chen
Cc: Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, Rob Herring, Mark Brown,
linuxppc-dev, linux-kernel
In-Reply-To: <20200228063958.GA473@NICOLINC-LT.nvidia.com>
On Fri, Feb 28, 2020 at 2:40 PM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Fri, Feb 28, 2020 at 10:54:02AM +0800, Shengjiu Wang wrote:
> > Hi
> >
> > On Fri, Feb 28, 2020 at 1:45 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> > >
> > > On Thu, Feb 27, 2020 at 01:10:19PM +0800, Shengjiu Wang wrote:
> > > > On Thu, Feb 27, 2020 at 11:43 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> > > > >
> > > > > On Thu, Feb 27, 2020 at 10:41:55AM +0800, Shengjiu Wang wrote:
> > > > > > asrc_format is more inteligent variable, which is align
> > > > > > with the alsa definition snd_pcm_format_t.
> > > > > >
> > > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > > > ---
> > > > > > sound/soc/fsl/fsl_asrc.c | 23 +++++++++++------------
> > > > > > sound/soc/fsl/fsl_asrc.h | 4 ++--
> > > > > > sound/soc/fsl/fsl_asrc_dma.c | 2 +-
> > > > > > 3 files changed, 14 insertions(+), 15 deletions(-)
> > > > > >
> > > > > > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> > > > > > index 0dcebc24c312..2b6a1643573c 100644
> > > > > > --- a/sound/soc/fsl/fsl_asrc.c
> > > > > > +++ b/sound/soc/fsl/fsl_asrc.c
> > > > >
> > > > > > @@ -600,11 +599,6 @@ static int fsl_asrc_dai_hw_params(struct snd_pcm_substream *substream,
> > > > > >
> > > > > > pair->config = &config;
> > > > > >
> > > > > > - if (asrc_priv->asrc_width == 16)
> > > > > > - format = SNDRV_PCM_FORMAT_S16_LE;
> > > > > > - else
> > > > > > - format = SNDRV_PCM_FORMAT_S24_LE;
> > > > >
> > > > > It feels better to me that we have format settings in hw_params().
> > > > >
> > > > > Why not let fsl_easrc align with this? Any reason that I'm missing?
> > > >
> > > > because the asrc_width is not formal, in the future we can direct
> > >
> > > Hmm..that's our DT binding. And I don't feel it is a problem
> > > to be ASoC irrelative.
> > >
> > > > input the format from the dts. format involve the info about width.
> > >
> > > Is there such any formal ASoC binding? I don't see those PCM
> > > formats under include/dt-bindings/ folder. How are we going
> > > to involve those formats in DT?
> >
> > There is no formal binding of this case.
> >
> > I think it is not good to convert width to format, because, for example
>
> The thing is that fsl_easrc does the conversion too... It just
> does in the probe instead of hw_params(), and then copies them
> in the hw_params(). So I don't see obvious benefit by doing so.
>
> > width = 24, there is two option, we can select format S24_LE, or
> > format S24_3LE, width is ambiguous for selecting.
> >
> > In EASRC, it support other two 24bit format U24_LE, U24_3LE .
>
> I understood the reason here, but am not seeing the necessity,
> at this point.
>
> > if we use the format in DT, then it is clear for usage in driver.
>
> I think this is the thing that we should address first. If we
> have such a binding being added with the new fsl_easrc driver,
> I'd love to see the old driver aligning with the new one.
>
> Otherwise, we can keep the old way, and change it when the new
> binding is ready.
ok, I will change the binding this time in next version.
best regards
wang shengjiu
^ permalink raw reply
* Re: [PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel
From: Christophe Leroy @ 2020-02-28 6:52 UTC (permalink / raw)
To: Pingfan Liu, linuxppc-dev
Cc: Aneesh Kumar K . V, kexec, Paul Mackerras, Oliver O'Halloran,
Dan Williams, Hari Bathini
In-Reply-To: <1582869192-9284-3-git-send-email-kernelfans@gmail.com>
Le 28/02/2020 à 06:53, Pingfan Liu a écrit :
> At present, plpar_hcall(H_SCM_BIND_MEM, ...) takes a very long time, so
> if dumping to fsdax, it will take a very long time.
>
> Take a closer look, during the papr_scm initialization, the only
> configuration is through drc_pmem_bind()-> plpar_hcall(H_SCM_BIND_MEM,
> ...), which helps to set up the bound address.
>
> On pseries, for kexec -l/-p kernel, there is no reset of hardware, and this
> step can be stepped around to save times. So the pmem bound address can be
> passed to the 2nd kernel through a dynamic added property "bound-addr" in
> dt node 'ibm,pmemory'.
>
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Cc: Oliver O'Halloran <oohall@gmail.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: kexec@lists.infradead.org
> ---
> note: I can not find such a pseries machine, and not finish it yet.
> ---
> arch/powerpc/platforms/pseries/papr_scm.c | 32 +++++++++++++++++++++----------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index c2ef320..555e746 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -382,7 +382,7 @@ static int papr_scm_probe(struct platform_device *pdev)
> {
> struct device_node *dn = pdev->dev.of_node;
> u32 drc_index, metadata_size;
> - u64 blocks, block_size;
> + u64 blocks, block_size, bound_addr = 0;
> struct papr_scm_priv *p;
> const char *uuid_str;
> u64 uuid[2];
> @@ -439,17 +439,29 @@ static int papr_scm_probe(struct platform_device *pdev)
> p->metadata_size = metadata_size;
> p->pdev = pdev;
>
> - /* request the hypervisor to bind this region to somewhere in memory */
> - rc = drc_pmem_bind(p);
> + of_property_read_u64(dn, "bound-addr", &bound_addr);
> + if (bound_addr)
> + p->bound_addr = bound_addr;
> + else {
All legs of an if/else must have { } when one leg need them, see codying
style.
> + struct property *property;
> + u64 big;
>
> - /* If phyp says drc memory still bound then force unbound and retry */
> - if (rc == H_OVERLAP)
> - rc = drc_pmem_query_n_bind(p);
> + /* request the hypervisor to bind this region to somewhere in memory */
> + rc = drc_pmem_bind(p);
>
> - if (rc != H_SUCCESS) {
> - dev_err(&p->pdev->dev, "bind err: %d\n", rc);
> - rc = -ENXIO;
> - goto err;
> + /* If phyp says drc memory still bound then force unbound and retry */
> + if (rc == H_OVERLAP)
> + rc = drc_pmem_query_n_bind(p);
> +
> + if (rc != H_SUCCESS) {
> + dev_err(&p->pdev->dev, "bind err: %d\n", rc);
> + rc = -ENXIO;
> + goto err;
> + }
> + big = cpu_to_be64(p->bound_addr);
> + property = new_property("bound-addr", sizeof(u64), &big,
> + NULL);
Why plitting this line in two parts ? You have lines far longer above.
In powerpc we allow lines 90 chars long.
> + of_add_property(dn, property);
> }
>
> /* setup the resource for the newly bound range */
>
Christophe
^ permalink raw reply
* [PATCH] powerpc/perf: Use SIER_USER_MASK while updating SPRN_SIER for EBB events
From: Athira Rajeev @ 2020-02-28 6:48 UTC (permalink / raw)
To: mpe; +Cc: maddy, linuxppc-dev
commit 330a1eb7775b ("powerpc/perf: Core EBB support for 64-bit book3s")
defines user mask for SIER. But this is not used in ebb_switch_out while
saving SPR's. Patch fixes this by updating SPRN_SIER with the user mask.
Fixes: 330a1eb7775b ("powerpc/perf: Core EBB support for 64-bit book3s")
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
arch/powerpc/perf/core-book3s.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 3086055..48b61cc 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -579,7 +579,7 @@ static void ebb_switch_out(unsigned long mmcr0)
return;
current->thread.siar = mfspr(SPRN_SIAR);
- current->thread.sier = mfspr(SPRN_SIER);
+ current->thread.sier = mfspr(SPRN_SIER) & SIER_USER_MASK;
current->thread.sdar = mfspr(SPRN_SDAR);
current->thread.mmcr0 = mmcr0 & MMCR0_USER_MASK;
current->thread.mmcr2 = mfspr(SPRN_MMCR2) & MMCR2_USER_MASK;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 1/3] powerpc/of: split out new_property() for reusing
From: Christophe Leroy @ 2020-02-28 6:47 UTC (permalink / raw)
To: Pingfan Liu, linuxppc-dev
Cc: Aneesh Kumar K . V, kexec, Paul Mackerras, Oliver O'Halloran,
Dan Williams, Hari Bathini
In-Reply-To: <1582869192-9284-1-git-send-email-kernelfans@gmail.com>
Le 28/02/2020 à 06:53, Pingfan Liu a écrit :
> Since new_property() is used in several calling sites, splitting it out for
> reusing.
>
> To ease the review, although the split out part has coding style issue,
> keeping it untouched and fixed in next patch.
The moved function fits in one screen. I don't think it is worth
splitting out for coding style that applies on the new lines only. You
can squash the two patches, it will still be easy to review.
>
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Cc: Oliver O'Halloran <oohall@gmail.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: kexec@lists.infradead.org
> ---
> arch/powerpc/include/asm/prom.h | 2 ++
> arch/powerpc/kernel/Makefile | 2 +-
> arch/powerpc/kernel/of_property.c | 32 +++++++++++++++++++++++++++++++
> arch/powerpc/mm/drmem.c | 26 -------------------------
> arch/powerpc/platforms/pseries/reconfig.c | 26 -------------------------
> 5 files changed, 35 insertions(+), 53 deletions(-)
> create mode 100644 arch/powerpc/kernel/of_property.c
>
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index 94e3fd5..02f7b1b 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -90,6 +90,8 @@ struct of_drc_info {
> extern int of_read_drc_info_cell(struct property **prop,
> const __be32 **curval, struct of_drc_info *data);
>
> +extern struct property *new_property(const char *name, const int length,
> + const unsigned char *value, struct property *last);
Don't add useless 'extern' keyword.
>
> /*
> * There are two methods for telling firmware what our capabilities are.
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 157b014..23375fd 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -47,7 +47,7 @@ obj-y := cputable.o ptrace.o syscalls.o \
> signal.o sysfs.o cacheinfo.o time.o \
> prom.o traps.o setup-common.o \
> udbg.o misc.o io.o misc_$(BITS).o \
> - of_platform.o prom_parse.o
> + of_platform.o prom_parse.o of_property.o
> obj-$(CONFIG_PPC64) += setup_64.o sys_ppc32.o \
> signal_64.o ptrace32.o \
> paca.o nvram_64.o firmware.o note.o
> diff --git a/arch/powerpc/kernel/of_property.c b/arch/powerpc/kernel/of_property.c
> new file mode 100644
> index 0000000..e56c832
> --- /dev/null
> +++ b/arch/powerpc/kernel/of_property.c
> @@ -0,0 +1,32 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/of.h>
> +#include <linux/string.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +
> +struct property *new_property(const char *name, const int length,
> + const unsigned char *value, struct property *last)
> +{
> + struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
> +
> + if (!new)
> + return NULL;
> +
> + if (!(new->name = kstrdup(name, GFP_KERNEL)))
> + goto cleanup;
> + if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
> + goto cleanup;
> +
> + memcpy(new->value, value, length);
> + *(((char *)new->value) + length) = 0;
> + new->length = length;
> + new->next = last;
> + return new;
> +
> +cleanup:
> + kfree(new->name);
> + kfree(new->value);
> + kfree(new);
> + return NULL;
> +}
> +
> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
> index 85b088a..888227e 100644
> --- a/arch/powerpc/mm/drmem.c
> +++ b/arch/powerpc/mm/drmem.c
> @@ -99,32 +99,6 @@ static void init_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>
> extern int test_hotplug;
>
> -static struct property *new_property(const char *name, const int length,
> - const unsigned char *value, struct property *last)
> -{
> - struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
> -
> - if (!new)
> - return NULL;
> -
> - if (!(new->name = kstrdup(name, GFP_KERNEL)))
> - goto cleanup;
> - if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
> - goto cleanup;
> -
> - memcpy(new->value, value, length);
> - *(((char *)new->value) + length) = 0;
> - new->length = length;
> - new->next = last;
> - return new;
> -
> -cleanup:
> - kfree(new->name);
> - kfree(new->value);
> - kfree(new);
> - return NULL;
> -}
> -
> static int drmem_update_dt_v2(struct device_node *memory,
> struct property *prop)
> {
> diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
> index 7f7369f..8e5a2ba 100644
> --- a/arch/powerpc/platforms/pseries/reconfig.c
> +++ b/arch/powerpc/platforms/pseries/reconfig.c
> @@ -165,32 +165,6 @@ static char * parse_next_property(char *buf, char *end, char **name, int *length
> return tmp;
> }
>
> -static struct property *new_property(const char *name, const int length,
> - const unsigned char *value, struct property *last)
> -{
> - struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
> -
> - if (!new)
> - return NULL;
> -
> - if (!(new->name = kstrdup(name, GFP_KERNEL)))
> - goto cleanup;
> - if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
> - goto cleanup;
> -
> - memcpy(new->value, value, length);
> - *(((char *)new->value) + length) = 0;
> - new->length = length;
> - new->next = last;
> - return new;
> -
> -cleanup:
> - kfree(new->name);
> - kfree(new->value);
> - kfree(new);
> - return NULL;
> -}
> -
> static int do_add_node(char *buf, size_t bufsize)
> {
> char *path, *end, *name;
>
Christophe
^ permalink raw reply
* Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
From: Jason Yan @ 2020-02-28 6:47 UTC (permalink / raw)
To: Scott Wood, Daniel Axtens, mpe, linuxppc-dev, diana.craciun,
christophe.leroy, benh, paulus, npiggin, keescook,
kernel-hardening
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <e8cd8f287934954cfa07dcf76ac73492e2d49a5b.camel@buserror.net>
在 2020/2/28 13:53, Scott Wood 写道:
> On Wed, 2020-02-26 at 16:18 +0800, Jason Yan wrote:
>> Hi Daniel,
>>
>> 在 2020/2/26 15:16, Daniel Axtens 写道:
>>> Hi Jason,
>>>
>>>> This is a try to implement KASLR for Freescale BookE64 which is based on
>>>> my earlier implementation for Freescale BookE32:
>>>> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=131718
>>>>
>>>> The implementation for Freescale BookE64 is similar as BookE32. One
>>>> difference is that Freescale BookE64 set up a TLB mapping of 1G during
>>>> booting. Another difference is that ppc64 needs the kernel to be
>>>> 64K-aligned. So we can randomize the kernel in this 1G mapping and make
>>>> it 64K-aligned. This can save some code to creat another TLB map at
>>>> early boot. The disadvantage is that we only have about 1G/64K = 16384
>>>> slots to put the kernel in.
>>>>
>>>> KERNELBASE
>>>>
>>>> 64K |--> kernel <--|
>>>> | | |
>>>> +--+--+--+ +--+--+--+--+--+--+--+--+--+ +--+--+
>>>> | | | |....| | | | | | | | | |....| | |
>>>> +--+--+--+ +--+--+--+--+--+--+--+--+--+ +--+--+
>>>> | | 1G
>>>> |-----> offset <-----|
>>>>
>>>> kernstart_virt_addr
>>>>
>>>> I'm not sure if the slot numbers is enough or the design has any
>>>> defects. If you have some better ideas, I would be happy to hear that.
>>>>
>>>> Thank you all.
>>>>
>>>
>>> Are you making any attempt to hide kernel address leaks in this series?
>>
>> Yes.
>>
>>> I've just been looking at the stackdump code just now, and it directly
>>> prints link registers and stack pointers, which is probably enough to
>>> determine the kernel base address:
>>>
>>> SPs: LRs: %pS pointer
>>> [ 0.424506] [c0000000de403970] [c000000001fc0458] dump_stack+0xfc/0x154
>>> (unreliable)
>>> [ 0.424593] [c0000000de4039c0] [c000000000267eec] panic+0x258/0x5ac
>>> [ 0.424659] [c0000000de403a60] [c0000000024d7a00]
>>> mount_block_root+0x634/0x7c0
>>> [ 0.424734] [c0000000de403be0] [c0000000024d8100]
>>> prepare_namespace+0x1ec/0x23c
>>> [ 0.424811] [c0000000de403c60] [c0000000024d7010]
>>> kernel_init_freeable+0x804/0x880
>>>
>>> git grep \\\"REG\\\" arch/powerpc shows a few other uses like this, all
>>> in process.c or in xmon.
>>>
>>
>> Thanks for reminding this.
>>
>>> Maybe replacing the REG format string in KASLR mode would be sufficient?
>>>
>>
>> Most archs have removed the address printing when dumping stack. Do we
>> really have to print this?
>>
>> If we have to do this, maybe we can use "%pK" so that they will be
>> hidden from unprivileged users.
>
> I've found the addresses to be useful, especially if I had a way to dump the
> stack data itself. Wouldn't the register dump also be likely to give away the
> addresses?
If we have to print the address, then kptr_restrict and dmesg_restrict
must be set properly so that unprivileged users cannot see them.
>
> I don't see any debug setting for %pK (or %p) to always print the actual
> address (closest is kptr_restrict=1 but that only works in certain
> contexts)... from looking at the code it seems it hashes even if kaslr is
> entirely disabled? Or am I missing something?
>
Yes, %pK (or %p) always hashes whether kaslr is disabled or not. So if
we want the real value of the address, we cannot use it. But if you only
want to distinguish if two pointers are the same, it's ok.
> -Scott
>
>
>
> .
>
^ permalink raw reply
* Re: [PATCH v3 1/4] ASoC: fsl_asrc: Change asrc_width to asrc_format
From: Nicolin Chen @ 2020-02-28 6:40 UTC (permalink / raw)
To: Shengjiu Wang
Cc: Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, Rob Herring, Mark Brown,
linuxppc-dev, linux-kernel
In-Reply-To: <CAA+D8AM6t79cPoNmt-8HbGwTSM9bfXSW8g76HtkCF7eauL_Xmw@mail.gmail.com>
On Fri, Feb 28, 2020 at 10:54:02AM +0800, Shengjiu Wang wrote:
> Hi
>
> On Fri, Feb 28, 2020 at 1:45 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> >
> > On Thu, Feb 27, 2020 at 01:10:19PM +0800, Shengjiu Wang wrote:
> > > On Thu, Feb 27, 2020 at 11:43 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> > > >
> > > > On Thu, Feb 27, 2020 at 10:41:55AM +0800, Shengjiu Wang wrote:
> > > > > asrc_format is more inteligent variable, which is align
> > > > > with the alsa definition snd_pcm_format_t.
> > > > >
> > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > > ---
> > > > > sound/soc/fsl/fsl_asrc.c | 23 +++++++++++------------
> > > > > sound/soc/fsl/fsl_asrc.h | 4 ++--
> > > > > sound/soc/fsl/fsl_asrc_dma.c | 2 +-
> > > > > 3 files changed, 14 insertions(+), 15 deletions(-)
> > > > >
> > > > > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> > > > > index 0dcebc24c312..2b6a1643573c 100644
> > > > > --- a/sound/soc/fsl/fsl_asrc.c
> > > > > +++ b/sound/soc/fsl/fsl_asrc.c
> > > >
> > > > > @@ -600,11 +599,6 @@ static int fsl_asrc_dai_hw_params(struct snd_pcm_substream *substream,
> > > > >
> > > > > pair->config = &config;
> > > > >
> > > > > - if (asrc_priv->asrc_width == 16)
> > > > > - format = SNDRV_PCM_FORMAT_S16_LE;
> > > > > - else
> > > > > - format = SNDRV_PCM_FORMAT_S24_LE;
> > > >
> > > > It feels better to me that we have format settings in hw_params().
> > > >
> > > > Why not let fsl_easrc align with this? Any reason that I'm missing?
> > >
> > > because the asrc_width is not formal, in the future we can direct
> >
> > Hmm..that's our DT binding. And I don't feel it is a problem
> > to be ASoC irrelative.
> >
> > > input the format from the dts. format involve the info about width.
> >
> > Is there such any formal ASoC binding? I don't see those PCM
> > formats under include/dt-bindings/ folder. How are we going
> > to involve those formats in DT?
>
> There is no formal binding of this case.
>
> I think it is not good to convert width to format, because, for example
The thing is that fsl_easrc does the conversion too... It just
does in the probe instead of hw_params(), and then copies them
in the hw_params(). So I don't see obvious benefit by doing so.
> width = 24, there is two option, we can select format S24_LE, or
> format S24_3LE, width is ambiguous for selecting.
>
> In EASRC, it support other two 24bit format U24_LE, U24_3LE .
I understood the reason here, but am not seeing the necessity,
at this point.
> if we use the format in DT, then it is clear for usage in driver.
I think this is the thing that we should address first. If we
have such a binding being added with the new fsl_easrc driver,
I'd love to see the old driver aligning with the new one.
Otherwise, we can keep the old way, and change it when the new
binding is ready.
^ permalink raw reply
* Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
From: Scott Wood @ 2020-02-28 5:53 UTC (permalink / raw)
To: Jason Yan, Daniel Axtens, mpe, linuxppc-dev, diana.craciun,
christophe.leroy, benh, paulus, npiggin, keescook,
kernel-hardening
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <8171d326-5138-4f5c-cff6-ad3ee606f0c2@huawei.com>
On Wed, 2020-02-26 at 16:18 +0800, Jason Yan wrote:
> Hi Daniel,
>
> 在 2020/2/26 15:16, Daniel Axtens 写道:
> > Hi Jason,
> >
> > > This is a try to implement KASLR for Freescale BookE64 which is based on
> > > my earlier implementation for Freescale BookE32:
> > > https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=131718
> > >
> > > The implementation for Freescale BookE64 is similar as BookE32. One
> > > difference is that Freescale BookE64 set up a TLB mapping of 1G during
> > > booting. Another difference is that ppc64 needs the kernel to be
> > > 64K-aligned. So we can randomize the kernel in this 1G mapping and make
> > > it 64K-aligned. This can save some code to creat another TLB map at
> > > early boot. The disadvantage is that we only have about 1G/64K = 16384
> > > slots to put the kernel in.
> > >
> > > KERNELBASE
> > >
> > > 64K |--> kernel <--|
> > > | | |
> > > +--+--+--+ +--+--+--+--+--+--+--+--+--+ +--+--+
> > > | | | |....| | | | | | | | | |....| | |
> > > +--+--+--+ +--+--+--+--+--+--+--+--+--+ +--+--+
> > > | | 1G
> > > |-----> offset <-----|
> > >
> > > kernstart_virt_addr
> > >
> > > I'm not sure if the slot numbers is enough or the design has any
> > > defects. If you have some better ideas, I would be happy to hear that.
> > >
> > > Thank you all.
> > >
> >
> > Are you making any attempt to hide kernel address leaks in this series?
>
> Yes.
>
> > I've just been looking at the stackdump code just now, and it directly
> > prints link registers and stack pointers, which is probably enough to
> > determine the kernel base address:
> >
> > SPs: LRs: %pS pointer
> > [ 0.424506] [c0000000de403970] [c000000001fc0458] dump_stack+0xfc/0x154
> > (unreliable)
> > [ 0.424593] [c0000000de4039c0] [c000000000267eec] panic+0x258/0x5ac
> > [ 0.424659] [c0000000de403a60] [c0000000024d7a00]
> > mount_block_root+0x634/0x7c0
> > [ 0.424734] [c0000000de403be0] [c0000000024d8100]
> > prepare_namespace+0x1ec/0x23c
> > [ 0.424811] [c0000000de403c60] [c0000000024d7010]
> > kernel_init_freeable+0x804/0x880
> >
> > git grep \\\"REG\\\" arch/powerpc shows a few other uses like this, all
> > in process.c or in xmon.
> >
>
> Thanks for reminding this.
>
> > Maybe replacing the REG format string in KASLR mode would be sufficient?
> >
>
> Most archs have removed the address printing when dumping stack. Do we
> really have to print this?
>
> If we have to do this, maybe we can use "%pK" so that they will be
> hidden from unprivileged users.
I've found the addresses to be useful, especially if I had a way to dump the
stack data itself. Wouldn't the register dump also be likely to give away the
addresses?
I don't see any debug setting for %pK (or %p) to always print the actual
address (closest is kptr_restrict=1 but that only works in certain
contexts)... from looking at the code it seems it hashes even if kaslr is
entirely disabled? Or am I missing something?
-Scott
^ permalink raw reply
* Re: [PATCH v3 25/27] powerpc/powernv/pmem: Expose the serial number in sysfs
From: Andrew Donnellan @ 2020-02-28 6:25 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200221032720.33893-26-alastair@au1.ibm.com>
On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> +int ocxlpmem_sysfs_add(struct ocxlpmem *ocxlpmem)
> +{
> + int i, rc;
> +
> + for (i = 0; i < ARRAY_SIZE(attrs); i++) {
> + rc = device_create_file(&ocxlpmem->dev, &attrs[i]);
> + if (rc) {
> + for (; --i >= 0;)
> + device_remove_file(&ocxlpmem->dev, &attrs[i]);
I'd rather avoid weird for loop constructs if possible.
Is it actually dangerous to call device_remove_file() on an attr that
hasn't been added? If not then I'd rather define an err: label and loop
over the whole array there.
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/of: split out new_property() for reusing
From: Pingfan Liu @ 2020-02-28 6:22 UTC (permalink / raw)
To: Andrew Donnellan
Cc: Aneesh Kumar K . V, Kexec Mailing List, Oliver O'Halloran,
Paul Mackerras, Dan Williams, linuxppc-dev, Hari Bathini
In-Reply-To: <44745496-9d30-45cb-b67c-0ca283c4c90d@linux.ibm.com>
On Fri, Feb 28, 2020 at 2:03 PM Andrew Donnellan <ajd@linux.ibm.com> wrote:
>
> On 28/2/20 4:53 pm, Pingfan Liu wrote:
> > Since new_property() is used in several calling sites, splitting it out for
> > reusing.
> >
> > To ease the review, although the split out part has coding style issue,
> > keeping it untouched and fixed in next patch.
> >
> > Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> > To: linuxppc-dev@lists.ozlabs.org
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Hari Bathini <hbathini@linux.ibm.com>
> > Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > Cc: Oliver O'Halloran <oohall@gmail.com>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Cc: kexec@lists.infradead.org
>
> Which tree does this apply to? I don't see a new_property() in mm/drmem.c...
Sorry, there is mud in my git tree, I check, either linux git or
powerpc git tree does not have this function.
Nack this series, and I will send out V2 for patch 3/3.
Thanks,
Pingfan
>
> --
> Andrew Donnellan OzLabs, ADL Canberra
> ajd@linux.ibm.com IBM Australia Limited
>
^ permalink raw reply
* Re: [PATCH v3 22/27] powerpc/powernv/pmem: Implement the heartbeat command
From: Andrew Donnellan @ 2020-02-28 6:20 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200221032720.33893-23-alastair@au1.ibm.com>
On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> The heartbeat admin command is a simple admin command that exercises
> the communication mechanisms within the controller.
>
> This patch issues a heartbeat command to the card during init to ensure
> we can communicate with the card's controller.
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Looks okay.
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ 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