Re: [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass
Hello Prakhar, Prakhar Srivastava writes: > On 5/12/20 4:05 PM, Rob Herring wrote: >> On Wed, May 06, 2020 at 10:50:04PM -0700, Prakhar Srivastava wrote: >>> Hi Mark, >> >> Please don't top post. >> >>> This patch set currently only address the Pure DT implementation. >>> EFI and ACPI implementations will be posted in subsequent patchsets. >>> >>> The logs are intended to be carried over the kexec and once read the >>> logs are no longer needed and in prior conversation with James( >>> https://lore.kernel.org/linux-arm-kernel/0053eb68-0905-4679-c97a-00c5cb6f1...@arm.com/) >>> the apporach of using a chosen node doesn't >>> support the case. >>> >>> The DT entries make the reservation permanent and thus doesnt need kernel >>> segments to be used for this, however using a chosen-node with >>> reserved memory only changes the node information but memory still is >>> reserved via reserved-memory section. >> >> I think Mark's point was whether it needs to be permanent. We don't >> hardcode the initrd address for example. >> > Thankyou for clarifying my misunderstanding, i am modelling this keeping to > the > TPM log implementation that uses a reserved memory. I will rev up the version > with chosen-node support. > That will make the memory reservation free after use. Nice. Do you intend to use the same property that powerpc uses (linux,ima-kexec-buffer)? >>> On 5/5/20 2:59 AM, Mark Rutland wrote: Hi Prakhar, On Mon, May 04, 2020 at 01:38:27PM -0700, Prakhar Srivastava wrote: > IMA during kexec(kexec file load) verifies the kernel signature and > measures >> >> What's IMAIMA is a LSM attempting to detect if files have been accidentally >> or > maliciously altered, both remotely and locally, it can also be used > to appraise a file's measurement against a "good" value stored as an extended > attribute, and enforce local file integrity. > > IMA also validates and measures the signers of the kernel and initrd > during kexec. The measurements are extended to PCR 10(configurable) and the > logs > stored in memory, however once kexec'd the logs are lost. Kexec is used as > secondary boot loader in may use cases and loosing the signer > creates a security hole. > > This patch is an implementation to carry over the logs and making it > possible to remotely validate the signers of the kernel and initrd. Such a > support exits only in powerpc. > This patch makes the carry over of logs architecture independent and puts the > complexity in a driver. If I'm not mistaken, the code at arch/powerpc/kexec/ima.c isn't actually powerpc-specific. It could be moved to an arch-independent directory and used by any other architecture which supports device trees. I think that's the simplest way forward. And to be honest I'm still trying to understand why you didn't take that approach. Did you try it and hit some obstacle or noticed a disadvantage for your use case? -- Thiago Jung Bauermann IBM Linux Technology Center
Re: [PATCH -next] soc: fsl: dpio: Remove unused inline function qbman_write_eqcr_am_rt_register
On Fri, May 8, 2020 at 9:13 AM YueHaibing wrote:
>
> There's no callers in-tree anymore since commit
> 3b2abda7d28c ("soc: fsl: dpio: Replace QMAN array mode with ring mode
> enqueue")
>
> Signed-off-by: YueHaibing
Applied to next. Thanks.
Regards,
Leo
> ---
> drivers/soc/fsl/dpio/qbman-portal.c | 12
> 1 file changed, 12 deletions(-)
>
> diff --git a/drivers/soc/fsl/dpio/qbman-portal.c
> b/drivers/soc/fsl/dpio/qbman-portal.c
> index 804b8ba9bf5c..e2e9fbb58a72 100644
> --- a/drivers/soc/fsl/dpio/qbman-portal.c
> +++ b/drivers/soc/fsl/dpio/qbman-portal.c
> @@ -572,18 +572,6 @@ void qbman_eq_desc_set_qd(struct qbman_eq_desc *d, u32
> qdid,
> #define EQAR_VB(eqar) ((eqar) & 0x80)
> #define EQAR_SUCCESS(eqar) ((eqar) & 0x100)
>
> -static inline void qbman_write_eqcr_am_rt_register(struct qbman_swp *p,
> - u8 idx)
> -{
> - if (idx < 16)
> - qbman_write_register(p, QBMAN_CINH_SWP_EQCR_AM_RT + idx * 4,
> -QMAN_RT_MODE);
> - else
> - qbman_write_register(p, QBMAN_CINH_SWP_EQCR_AM_RT2 +
> -(idx - 16) * 4,
> -QMAN_RT_MODE);
> -}
> -
> #define QB_RT_BIT ((u32)0x100)
> /**
> * qbman_swp_enqueue_direct() - Issue an enqueue command
> --
> 2.17.1
>
>
Re: [PATCH] soc: fsl: qe: clean up an indentation issue
On Fri, Mar 27, 2020 at 11:15 AM Colin King wrote: > > From: Colin Ian King > > There is a statement that not indented correctly, remove the > extraneous space. > > Signed-off-by: Colin Ian King Applied for next. Thanks. > --- > drivers/soc/fsl/qe/ucc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c > index d6c93970df4d..cac0fb7693a0 100644 > --- a/drivers/soc/fsl/qe/ucc.c > +++ b/drivers/soc/fsl/qe/ucc.c > @@ -519,7 +519,7 @@ int ucc_set_tdm_rxtx_clk(u32 tdm_num, enum qe_clock clock, > int clock_bits; > u32 shift; > struct qe_mux __iomem *qe_mux_reg; > -__be32 __iomem *cmxs1cr; > + __be32 __iomem *cmxs1cr; > > qe_mux_reg = _immr->qmx; > > -- > 2.25.1 >
Re: [PATCH] net/ethernet/freescale: rework quiesce/activate for ucc_geth
From: Valentin Longchamp Date: Wed, 20 May 2020 17:53:50 +0200 > ugeth_quiesce/activate are used to halt the controller when there is a > link change that requires to reconfigure the mac. > > The previous implementation called netif_device_detach(). This however > causes the initial activation of the netdevice to fail precisely because > it's detached. For details, see [1]. > > A possible workaround was the revert of commit > net: linkwatch: add check for netdevice being present to linkwatch_do_dev > However, the check introduced in the above commit is correct and shall be > kept. > > The netif_device_detach() is thus replaced with > netif_tx_stop_all_queues() that prevents any tranmission. This allows to > perform mac config change required by the link change, without detaching > the corresponding netdevice and thus not preventing its initial > activation. > > [1] https://lists.openwall.net/netdev/2020/01/08/201 > > Signed-off-by: Valentin Longchamp > Acked-by: Matteo Ghidoni Applied, thanks.
Re: [PATCH] usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()
On Tue, Apr 14, 2020 at 4:13 AM Tang Bin wrote: > > Hi > > On 2020/4/14 16:30, Dan Carpenter wrote: > > On Fri, Apr 10, 2020 at 04:05:06PM +0800, Tang Bin wrote: > >>> > Thus it must be fixed. > >>> Wording alternative: > >>> Thus adjust the error detection and corresponding exception handling. > >> Got it. > > Wow... > > > > No, don't listen to Markus when it comes to writing commit messages. > > You couldn't find worse advice anywhere. :P > > I'm actively waiting for a reply from the maintainer. Thank you very much. Sorry for the late response. Acked-by: Li Yang Regards, Leo
Re: [PATCH -next] soc: fsl: qbman: Remove unused inline function qm_eqcr_get_ci_stashing
On Fri, May 8, 2020 at 9:11 AM YueHaibing wrote:
>
> There's no callers in-tree anymore.
>
> Signed-off-by: YueHaibing
Applied for next. Thanks.
Regards,
Leo
> ---
> drivers/soc/fsl/qbman/qman.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
> index 1e164e03410a..9888a7061873 100644
> --- a/drivers/soc/fsl/qbman/qman.c
> +++ b/drivers/soc/fsl/qbman/qman.c
> @@ -449,11 +449,6 @@ static inline int qm_eqcr_init(struct qm_portal *portal,
> return 0;
> }
>
> -static inline unsigned int qm_eqcr_get_ci_stashing(struct qm_portal *portal)
> -{
> - return (qm_in(portal, QM_REG_CFG) >> 28) & 0x7;
> -}
> -
> static inline void qm_eqcr_finish(struct qm_portal *portal)
> {
> struct qm_eqcr *eqcr = >eqcr;
> --
> 2.17.1
>
>
Re: [PATCH] treewide: Replace zero-length array with flexible-array
On Thu, May 7, 2020 at 1:49 PM Gustavo A. R. Silva
wrote:
>
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
>
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
>
> "Flexible array members have incomplete type, and so the sizeof operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
>
> sizeof(flexible-array-member) triggers a warning because flexible array
> members have incomplete type[1]. There are some instances of code in
> which the sizeof operator is being incorrectly/erroneously applied to
> zero-length arrays and the result is zero. Such instances may be hiding
> some bugs. So, this work (flexible-array member conversions) will also
> help to get completely rid of those sorts of issues.
>
> This issue was found with the help of Coccinelle.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva
> ---
> include/linux/fsl/bestcomm/bestcomm.h |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied for next. Thanks.
Regards,
Leo
>
> diff --git a/include/linux/fsl/bestcomm/bestcomm.h
> b/include/linux/fsl/bestcomm/bestcomm.h
> index a0e2e6b19b57..154e541ce57e 100644
> --- a/include/linux/fsl/bestcomm/bestcomm.h
> +++ b/include/linux/fsl/bestcomm/bestcomm.h
> @@ -27,7 +27,7 @@
> */
> struct bcom_bd {
> u32 status;
> - u32 data[0];/* variable payload size */
> + u32 data[]; /* variable payload size */
> };
>
> /*
> */
>
Re: [PATCH] soc: fsl: qe: Replace one-element array and use struct_size() helper
On Mon, May 18, 2020 at 5:16 PM Gustavo A. R. Silva
wrote:
>
> The current codebase makes use of one-element arrays in the following
> form:
>
> struct something {
> int length;
> u8 data[1];
> };
>
> struct something *instance;
>
> instance = kmalloc(sizeof(*instance) + size, GFP_KERNEL);
> instance->length = size;
> memcpy(instance->data, source, size);
>
> but the preferred mechanism to declare variable-length types such as
> these ones is a flexible array member[1][2], introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on. So, replace
> the one-element array with a flexible-array member.
>
> Also, make use of the new struct_size() helper to properly calculate the
> size of struct qe_firmware.
>
> This issue was found with the help of Coccinelle and, audited and fixed
> _manually_.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva
> ---
> drivers/soc/fsl/qe/qe.c | 4 ++--
> include/soc/fsl/qe/qe.h | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
Applied for next. Thanks.
Regards,
Leo
Re: [PATCH] soc: fsl: qe: Replace one-element array and use struct_size() helper
On Wed, May 20, 2020 at 10:24 PM Kees Cook wrote:
>
> On Wed, May 20, 2020 at 06:52:21PM -0500, Li Yang wrote:
> > On Mon, May 18, 2020 at 5:57 PM Kees Cook wrote:
> > > Hm, looking at this code, I see a few other things that need to be
> > > fixed:
> > >
> > > 1) drivers/tty/serial/ucc_uart.c does not do a be32_to_cpu() conversion
> > >on the length test (understandably, a little-endian system has never
> > > run
> > >this code since it's ppc specific), but it's still wrong:
> > >
> > > if (firmware->header.length != fw->size) {
> > >
> > >compare to the firmware loader:
> > >
> > > length = be32_to_cpu(hdr->length);
> > >
> > > 2) drivers/soc/fsl/qe/qe.c does not perform bounds checking on the
> > >per-microcode offsets, so the uploader might send data outside the
> > >firmware buffer. Perhaps:
> >
> > We do validate the CRC for each microcode, it is unlikely the CRC
> > check can pass if the offset or length is not correct. But you are
> > probably right that it will be safer to check the boundary and fail
>
> Right, but a malicious firmware file could still match CRC but trick the
> kernel code.
>
> > quicker before we actually start the CRC check. Will you come up with
> > a formal patch or you want us to deal with it?
>
> It sounds like Gustavo will be sending one, though I don't think either
> of us have the hardware to test it with, so if you could do that part,
> that would be great! :)
That will be great. I think Zhao Qiang can help with the testing part.
Regards,
Leo
Re: [PATCH v3 0/2] PCI/ERR: Allow Native AER/DPC using _OSC
Hi Bjorn, Derrick,
On 5/22/20 12:46 PM, Bjorn Helgaas wrote:
On Fri, May 22, 2020 at 05:23:31PM +, Derrick, Jonathan wrote:
On Fri, 2020-05-01 at 11:35 -0600, Jonathan Derrick wrote:
On Fri, 2020-05-01 at 12:16 -0500, Bjorn Helgaas wrote:
On Thu, Apr 30, 2020 at 12:46:07PM -0600, Jon Derrick wrote:
Hi Bjorn & Kuppuswamy,
I see a problem in the DPC ECN [1] to _OSC in that it doesn't
give us a way to determine if firmware supports _OSC DPC
negotation, and therefore how to handle DPC.
Here is the wording of the ECN that implies that Firmware
without _OSC DPC negotiation support should have the OSPM rely
on _OSC AER negotiation when determining DPC control:
PCIe Base Specification suggests that Downstream Port
Containment may be controlled either by the Firmware or the
Operating System. It also suggests that the Firmware retain
ownership of Downstream Port Containment if it also owns
AER. When the Firmware owns Downstream Port Containment, it
is expected to use the new "Error Disconnect Recover"
notification to alert OSPM of a Downstream Port Containment
event.
In legacy platforms, as bits in _OSC are reserved prior to
implementation, ACPI Root Bus enumeration will mark these Host
Bridges as without Native DPC support, even though the
specification implies it's expected that AER _OSC negotiation
determines DPC control for these platforms. There seems to be
a need for a way to determine if the DPC control bit in _OSC
is supported and fallback on AER otherwise.
Currently portdrv assumes DPC control if the port has Native
AER services:
static int get_port_device_capability(struct pci_dev *dev)
...
if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
pci_aer_available() &&
(pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))
services |= PCIE_PORT_SERVICE_DPC;
Newer firmware may not grant OSPM DPC control, if for
instance, it expects to use Error Disconnect Recovery. However
it looks like ACPI will use DPC services via the EDR driver,
without binding the full DPC port service driver.
If we change portdrv to probe based on host->native_dpc and
not AER, then we break instances with legacy firmware where
OSPM will clear host->native_dpc solely due to _OSC bits being
reserved:
struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
...
if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL))
host_bridge->native_dpc = 0;
So my assumption instead is that host->native_dpc can be 0 and
expect Native DPC services if AER is used. In other words, if
and only if DPC probe is invoked from portdrv, then it needs
to rely on the AER dependency. Otherwise it should be assumed
that ACPI set up DPC via EDR. This covers legacy firmware.
However it seems like that could be trouble with newer
firmware that might give OSPM control of AER but not DPC, and
would result in both Native DPC and EDR being in effect.
Anyways here are two patches that give control of AER and DPC
on the results of _OSC. They don't mess with the HEST parser
as I expect those to be removed at some point. I need these
for VMD support which doesn't even rely on _OSC, but I suspect
this won't be the last effort as we detangle Firmware First.
[1] https://members.pcisig.com/wg/PCI-SIG/document/12888
Hi Jon, I think we need to sort out the _OSC/FIRMWARE_FIRST patches
from Alex and Sathy first, then see what needs to be done on top of
those, so I'm going to push these off for a few days and they'll
probably need a refresh.
Bjorn
Agreed, no need to merge now. Just wanted to bring up the DPC
ambiguity, which I think was first addressed by dpc-native:
commit 35a0b2378c199d4f26e458b2ca38ea56aaf2d9b8
Author: Olof Johansson
Date: Wed Oct 23 12:22:05 2019 -0700
PCI/DPC: Add "pcie_ports=dpc-native" to allow DPC without AER control
Prior to eed85ff4c0da7 ("PCI/DPC: Enable DPC only if AER is available"),
Linux handled DPC events regardless of whether firmware had granted it
ownership of AER or DPC, e.g., via _OSC.
PCIe r5.0, sec 6.2.10, recommends that the OS link control of DPC to
control of AER, so after eed85ff4c0da7, Linux handles DPC events only if it
has control of AER.
On platforms that do not grant OS control of AER via _OSC, Linux DPC
handling worked before eed85ff4c0da7 but not after.
To make Linux DPC handling work on those platforms the same way they did
before, add a "pcie_ports=dpc-native" kernel parameter that makes Linux
handle DPC events regardless of whether it has control of AER.
[bhelgaas: commit log, move pcie_ports_dpc_native to drivers/pci/]
Link: https://lore.kernel.org/r/20191023192205.97024-1-o...@lixom.net
Signed-off-by: Olof Johansson
Signed-off-by: Bjorn Helgaas
Are you still thinking about removing the HEST parser?
For VMD we still need the ability to bind DPC if
Re: [PATCH v3 0/2] PCI/ERR: Allow Native AER/DPC using _OSC
On Fri, May 22, 2020 at 05:23:31PM +, Derrick, Jonathan wrote:
> On Fri, 2020-05-01 at 11:35 -0600, Jonathan Derrick wrote:
> > On Fri, 2020-05-01 at 12:16 -0500, Bjorn Helgaas wrote:
> > > On Thu, Apr 30, 2020 at 12:46:07PM -0600, Jon Derrick wrote:
> > > > Hi Bjorn & Kuppuswamy,
> > > >
> > > > I see a problem in the DPC ECN [1] to _OSC in that it doesn't
> > > > give us a way to determine if firmware supports _OSC DPC
> > > > negotation, and therefore how to handle DPC.
> > > >
> > > > Here is the wording of the ECN that implies that Firmware
> > > > without _OSC DPC negotiation support should have the OSPM rely
> > > > on _OSC AER negotiation when determining DPC control:
> > > >
> > > > PCIe Base Specification suggests that Downstream Port
> > > > Containment may be controlled either by the Firmware or the
> > > > Operating System. It also suggests that the Firmware retain
> > > > ownership of Downstream Port Containment if it also owns
> > > > AER. When the Firmware owns Downstream Port Containment, it
> > > > is expected to use the new "Error Disconnect Recover"
> > > > notification to alert OSPM of a Downstream Port Containment
> > > > event.
> > > >
> > > > In legacy platforms, as bits in _OSC are reserved prior to
> > > > implementation, ACPI Root Bus enumeration will mark these Host
> > > > Bridges as without Native DPC support, even though the
> > > > specification implies it's expected that AER _OSC negotiation
> > > > determines DPC control for these platforms. There seems to be
> > > > a need for a way to determine if the DPC control bit in _OSC
> > > > is supported and fallback on AER otherwise.
> > > >
> > > >
> > > > Currently portdrv assumes DPC control if the port has Native
> > > > AER services:
> > > >
> > > > static int get_port_device_capability(struct pci_dev *dev)
> > > > ...
> > > > if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
> > > > pci_aer_available() &&
> > > > (pcie_ports_dpc_native || (services &
> > > > PCIE_PORT_SERVICE_AER)))
> > > > services |= PCIE_PORT_SERVICE_DPC;
> > > >
> > > > Newer firmware may not grant OSPM DPC control, if for
> > > > instance, it expects to use Error Disconnect Recovery. However
> > > > it looks like ACPI will use DPC services via the EDR driver,
> > > > without binding the full DPC port service driver.
> > > >
> > > >
> > > > If we change portdrv to probe based on host->native_dpc and
> > > > not AER, then we break instances with legacy firmware where
> > > > OSPM will clear host->native_dpc solely due to _OSC bits being
> > > > reserved:
> > > >
> > > > struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
> > > > ...
> > > > if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL))
> > > > host_bridge->native_dpc = 0;
> > > >
> > > >
> > > >
> > > > So my assumption instead is that host->native_dpc can be 0 and
> > > > expect Native DPC services if AER is used. In other words, if
> > > > and only if DPC probe is invoked from portdrv, then it needs
> > > > to rely on the AER dependency. Otherwise it should be assumed
> > > > that ACPI set up DPC via EDR. This covers legacy firmware.
> > > >
> > > > However it seems like that could be trouble with newer
> > > > firmware that might give OSPM control of AER but not DPC, and
> > > > would result in both Native DPC and EDR being in effect.
> > > >
> > > >
> > > > Anyways here are two patches that give control of AER and DPC
> > > > on the results of _OSC. They don't mess with the HEST parser
> > > > as I expect those to be removed at some point. I need these
> > > > for VMD support which doesn't even rely on _OSC, but I suspect
> > > > this won't be the last effort as we detangle Firmware First.
> > > >
> > > > [1] https://members.pcisig.com/wg/PCI-SIG/document/12888
> > >
> > > Hi Jon, I think we need to sort out the _OSC/FIRMWARE_FIRST patches
> > > from Alex and Sathy first, then see what needs to be done on top of
> > > those, so I'm going to push these off for a few days and they'll
> > > probably need a refresh.
> > >
> > > Bjorn
> >
> > Agreed, no need to merge now. Just wanted to bring up the DPC
> > ambiguity, which I think was first addressed by dpc-native:
> >
> > commit 35a0b2378c199d4f26e458b2ca38ea56aaf2d9b8
> > Author: Olof Johansson
> > Date: Wed Oct 23 12:22:05 2019 -0700
> >
> > PCI/DPC: Add "pcie_ports=dpc-native" to allow DPC without AER control
> >
> > Prior to eed85ff4c0da7 ("PCI/DPC: Enable DPC only if AER is available"),
> > Linux handled DPC events regardless of whether firmware had granted it
> > ownership of AER or DPC, e.g., via _OSC.
> >
> > PCIe r5.0, sec 6.2.10, recommends that the OS link control of DPC to
> > control of AER, so after eed85ff4c0da7, Linux handles DPC events only
> > if it
> > has control of AER.
> >
> > On platforms that do not grant OS control of AER via
Re: [v2 2/2] dts: ppc: t1024rdb: remove interrupts property
On Wed, May 20, 2020 at 4:21 AM Biwen Li wrote:
>
> From: Biwen Li
>
> This removes interrupts property to drop warning as follows:
> - $ hwclock.util-linux
> hwclock.util-linux: select() to /dev/rtc0
> to wait for clock tick timed out
>
> My case:
> - RTC ds1339s INT pin isn't connected to cpus INT pin on T1024RDB,
> then the RTC cannot inform cpu about alarm interrupt
>
> How to fix it?
> - remove IRQ line
This style is not the recommended style for commit message. Please
see my comment for the other patch.
>
> Signed-off-by: Biwen Li
> ---
> arch/powerpc/boot/dts/fsl/t1024rdb.dts | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> b/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> index 645caff98ed1..605ceec66af3 100644
> --- a/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> +++ b/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> @@ -161,7 +161,6 @@
> rtc@68 {
> compatible = "dallas,ds1339";
> reg = <0x68>;
> - interrupts = <0x1 0x1 0 0>;
> };
> };
>
> --
> 2.17.1
>
Re: [v2 1/2] dts: ppc: t4240rdb: remove interrupts property
On Wed, May 20, 2020 at 4:21 AM Biwen Li wrote:
>
> From: Biwen Li
>
> This removes interrupts property to drop warning as follows:
> - $ hwclock.util-linux
> hwclock.util-linux: select() to /dev/rtc0
> to wait for clock tick timed out
>
> My case:
> - RTC ds1374's INT pin is connected to VCC on T4240RDB,
> then the RTC cannot inform cpu about the alarm interrupt
The commit message need a little bit improvement. Something like:
Since the interrupt pin for RTC DS1374 is not connected to the CPU on
T4240RDB, remove
the interrupt property from the device tree.
This also fix the following warning for hwclock.util-linux:
$ hwclock.util-linux
hwclock.util-linux: select() to /dev/rtc0
to wait for clock tick timed out
>
> Signed-off-by: Biwen Li
> ---
> arch/powerpc/boot/dts/fsl/t4240rdb.dts | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> b/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> index a56a705d41f7..145896f2eef6 100644
> --- a/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> +++ b/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> @@ -144,7 +144,6 @@
> rtc@68 {
> compatible = "dallas,ds1374";
> reg = <0x68>;
> - interrupts = <0x1 0x1 0 0>;
> };
> };
>
> --
> 2.17.1
>
Re: [PATCH] kbuild: reuse vmlinux.o in vmlinux_link
+ Michael, and PPC ML.
They may know something about the reason of failure.
On Sat, May 23, 2020 at 2:41 AM Masahiro Yamada wrote:
>
> On Fri, May 22, 2020 at 5:27 AM Sami Tolvanen wrote:
> >
> > Instead of linking all compilation units again each time vmlinux_link is
> > called, reuse vmlinux.o from modpost_link.
> >
> > With x86_64 allyesconfig, vmlinux_link is called three times and reusing
> > vmlinux.o reduces the build time ~38 seconds on my system (59% reduction
> > in the time spent in vmlinux_link).
> >
> > Signed-off-by: Sami Tolvanen
> > ---
> > scripts/link-vmlinux.sh | 5 +
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > index d09ab4afbda4..c6cc4305950c 100755
> > --- a/scripts/link-vmlinux.sh
> > +++ b/scripts/link-vmlinux.sh
> > @@ -77,11 +77,8 @@ vmlinux_link()
> >
> > if [ "${SRCARCH}" != "um" ]; then
> > objects="--whole-archive\
> > - ${KBUILD_VMLINUX_OBJS} \
> > + vmlinux.o \
> > --no-whole-archive \
> > - --start-group \
> > - ${KBUILD_VMLINUX_LIBS} \
> > - --end-group \
> > ${@}"
> >
> > ${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux} \
> >
> > base-commit: b85051e755b0e9d6dd8f17ef1da083851b83287d
> > --
> > 2.27.0.rc0.183.gde8f92d652-goog
> >
>
>
> I like this patch irrespective of CLANG_LTO, but
> unfortunately, my build test failed.
>
>
> ARCH=powerpc failed to build as follows:
>
>
>
> MODPOST vmlinux.o
> MODINFO modules.builtin.modinfo
> GEN modules.builtin
> LD .tmp_vmlinux.kallsyms1
> vmlinux.o:(__ftr_alt_97+0x20): relocation truncated to fit:
> R_PPC64_REL14 against `.text'+4b1c
> vmlinux.o:(__ftr_alt_97+0x164): relocation truncated to fit:
> R_PPC64_REL14 against `.text'+1cf78
> vmlinux.o:(__ftr_alt_97+0x288): relocation truncated to fit:
> R_PPC64_REL14 against `.text'+1dac4
> vmlinux.o:(__ftr_alt_97+0x2f0): relocation truncated to fit:
> R_PPC64_REL14 against `.text'+1e254
> make: *** [Makefile:1125: vmlinux] Error 1
>
>
>
> I used powerpc-linux-gcc
> available at
> https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/9.2.0/
>
>
> Build command:
>
> make -j24 ARCH=powerpc CROSS_COMPILE=powerpc-linux- defconfig all
>
>
> Could you check it please?
>
>
>
> I will apply it to my test branch.
> Perhaps, 0-day bot may find more failure cases.
>
>
> --
> Best Regards
> Masahiro Yamada
--
Best Regards
Masahiro Yamada
Re: [PATCH v4 2/6] printk: honor the max_reason field in kmsg_dumper
On Fri, May 22, 2020 at 06:51:20PM +0200, Petr Mladek wrote: > On Fri 2020-05-15 11:44:30, Kees Cook wrote: > > From: Pavel Tatashin > > > > kmsg_dump() allows to dump kmesg buffer for various system events: oops, > > panic, reboot, etc. It provides an interface to register a callback call > > for clients, and in that callback interface there is a field "max_reason" > > which gets ignored unless always_kmsg_dump is passed as kernel parameter. > > Strictly speaking, this is not fully true. "max_reason" field is not > ignored when set to KMSG_DUMP_PANIC even when always_kmsg_dump was not set. > > It should be something like: > > "which gets ignored for reason higher than KMSG_DUMP_OOPS unless > always_kmsg_dump is passed as kernel parameter". > > Heh, I wonder if anyone will be able to parse this ;-) Ah yeah, good point. I've reworded things like this: kmsg_dump() allows to dump kmesg buffer for various system events: oops, panic, reboot, etc. It provides an interface to register a callback call for clients, and in that callback interface there is a field "max_reason", but it was getting ignored when set to any "reason" higher than KMSG_DUMP_OOPS unless "always_kmsg_dump" was passed as kernel parameter. Allow clients to actually control their "max_reason", and keep the current behavior when "max_reason" is not set. > Otherwise, it looks good to me. With the updated commit message: > > Reviewed-by: Petr Mladek Thanks! -- Kees Cook
Re: [PATCH v3 0/2] PCI/ERR: Allow Native AER/DPC using _OSC
On Fri, 2020-05-01 at 11:35 -0600, Jonathan Derrick wrote:
> On Fri, 2020-05-01 at 12:16 -0500, Bjorn Helgaas wrote:
> > On Thu, Apr 30, 2020 at 12:46:07PM -0600, Jon Derrick wrote:
> > > Hi Bjorn & Kuppuswamy,
> > >
> > > I see a problem in the DPC ECN [1] to _OSC in that it doesn't give us a
> > > way to
> > > determine if firmware supports _OSC DPC negotation, and therefore how to
> > > handle
> > > DPC.
> > >
> > > Here is the wording of the ECN that implies that Firmware without _OSC DPC
> > > negotiation support should have the OSPM rely on _OSC AER negotiation when
> > > determining DPC control:
> > >
> > > PCIe Base Specification suggests that Downstream Port Containment may be
> > > controlled either by the Firmware or the Operating System. It also
> > > suggests
> > > that the Firmware retain ownership of Downstream Port Containment if it
> > > also
> > > owns AER. When the Firmware owns Downstream Port Containment, it is
> > > expected
> > > to use the new "Error Disconnect Recover" notification to alert OSPM of
> > > a
> > > Downstream Port Containment event.
> > >
> > > In legacy platforms, as bits in _OSC are reserved prior to
> > > implementation, ACPI
> > > Root Bus enumeration will mark these Host Bridges as without Native DPC
> > > support, even though the specification implies it's expected that AER _OSC
> > > negotiation determines DPC control for these platforms. There seems to be
> > > a
> > > need for a way to determine if the DPC control bit in _OSC is supported
> > > and
> > > fallback on AER otherwise.
> > >
> > >
> > > Currently portdrv assumes DPC control if the port has Native AER services:
> > >
> > > static int get_port_device_capability(struct pci_dev *dev)
> > > ...
> > > if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
> > > pci_aer_available() &&
> > > (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))
> > > services |= PCIE_PORT_SERVICE_DPC;
> > >
> > > Newer firmware may not grant OSPM DPC control, if for instance, it
> > > expects to
> > > use Error Disconnect Recovery. However it looks like ACPI will use DPC
> > > services
> > > via the EDR driver, without binding the full DPC port service driver.
> > >
> > >
> > > If we change portdrv to probe based on host->native_dpc and not AER, then
> > > we
> > > break instances with legacy firmware where OSPM will clear
> > > host->native_dpc
> > > solely due to _OSC bits being reserved:
> > >
> > > struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
> > > ...
> > > if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL))
> > > host_bridge->native_dpc = 0;
> > >
> > >
> > >
> > > So my assumption instead is that host->native_dpc can be 0 and expect
> > > Native
> > > DPC services if AER is used. In other words, if and only if DPC probe is
> > > invoked from portdrv, then it needs to rely on the AER dependency.
> > > Otherwise it
> > > should be assumed that ACPI set up DPC via EDR. This covers legacy
> > > firmware.
> > >
> > > However it seems like that could be trouble with newer firmware that
> > > might give
> > > OSPM control of AER but not DPC, and would result in both Native DPC and
> > > EDR
> > > being in effect.
> > >
> > >
> > > Anyways here are two patches that give control of AER and DPC on the
> > > results of
> > > _OSC. They don't mess with the HEST parser as I expect those to be
> > > removed at
> > > some point. I need these for VMD support which doesn't even rely on _OSC,
> > > but I
> > > suspect this won't be the last effort as we detangle Firmware First.
> > >
> > > [1] https://members.pcisig.com/wg/PCI-SIG/document/12888
> >
> > Hi Jon, I think we need to sort out the _OSC/FIRMWARE_FIRST patches
> > from Alex and Sathy first, then see what needs to be done on top of
> > those, so I'm going to push these off for a few days and they'll
> > probably need a refresh.
> >
> > Bjorn
>
> Agreed, no need to merge now. Just wanted to bring up the DPC
> ambiguity, which I think was first addressed by dpc-native:
>
> commit 35a0b2378c199d4f26e458b2ca38ea56aaf2d9b8
> Author: Olof Johansson
> Date: Wed Oct 23 12:22:05 2019 -0700
>
> PCI/DPC: Add "pcie_ports=dpc-native" to allow DPC without AER control
>
> Prior to eed85ff4c0da7 ("PCI/DPC: Enable DPC only if AER is available"),
> Linux handled DPC events regardless of whether firmware had granted it
> ownership of AER or DPC, e.g., via _OSC.
>
> PCIe r5.0, sec 6.2.10, recommends that the OS link control of DPC to
> control of AER, so after eed85ff4c0da7, Linux handles DPC events only if
> it
> has control of AER.
>
> On platforms that do not grant OS control of AER via _OSC, Linux DPC
> handling worked before eed85ff4c0da7 but not after.
>
> To make Linux DPC handling work on those platforms the same way they did
> before, add a "pcie_ports=dpc-native" kernel parameter that
Re: [PATCH v4 2/6] printk: honor the max_reason field in kmsg_dumper
On Fri 2020-05-15 11:44:30, Kees Cook wrote: > From: Pavel Tatashin > > kmsg_dump() allows to dump kmesg buffer for various system events: oops, > panic, reboot, etc. It provides an interface to register a callback call > for clients, and in that callback interface there is a field "max_reason" > which gets ignored unless always_kmsg_dump is passed as kernel parameter. Strictly speaking, this is not fully true. "max_reason" field is not ignored when set to KMSG_DUMP_PANIC even when always_kmsg_dump was not set. It should be something like: "which gets ignored for reason higher than KMSG_DUMP_OOPS unless always_kmsg_dump is passed as kernel parameter". Heh, I wonder if anyone will be able to parse this ;-) Otherwise, it looks good to me. With the updated commit message: Reviewed-by: Petr Mladek Best Regards, Petr
Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.7-5 tag
The pull request you sent on Sat, 23 May 2020 00:06:55 +1000: > https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git > tags/powerpc-5.7-5 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/c8347bbf19f265c1bd254ca148f27caa71e77d61 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason
On Fri 2020-05-15 11:44:29, Kees Cook wrote: > To turn the KMSG_DUMP_* reasons into a more ordered list, collapse > the redundant KMSG_DUMP_(RESTART|HALT|POWEROFF) reasons into > KMSG_DUMP_SHUTDOWN. The current users already don't meaningfully > distinguish between them, so there's no need to, as discussed here: > https://lore.kernel.org/lkml/ca+ck2bapv5u1ih5y9t5funtyximtfctdyxjcpuyjoyhnokr...@mail.gmail.com/ > > Signed-off-by: Kees Cook Looks good to me: Reviewed-by: Petr Mladek Best Regards, Petr
Re: [PATCH 09/29] arm64: use asm-generic/cacheflush.h
On Fri, May 15, 2020 at 04:36:26PM +0200, Christoph Hellwig wrote: > ARM64 needs almost no cache flushing routines of its own. Rely on > asm-generic/cacheflush.h for the defaults. > > Signed-off-by: Christoph Hellwig Acked-by: Catalin Marinas
[GIT PULL] Please pull powerpc/linux.git powerpc-5.7-5 tag
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Hi Linus, Please pull some more powerpc fixes for 5.7: The following changes since commit 249c9b0cd193d983c3a0b00f3fd3b92333bfeebe: powerpc/40x: Make more space for system call exception (2020-05-12 21:22:11 +1000) are available in the git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-5.7-5 for you to fetch changes up to 8659a0e0efdd975c73355dbc033f79ba3b31e82c: powerpc/64s: Disable STRICT_KERNEL_RWX (2020-05-22 00:04:51 +1000) - -- powerpc fixes for 5.7 #5 A revert of a recent change to the PTE bits for 32-bit BookS, which broke swap. And a "fix" to disable STRICT_KERNEL_RWX for 64-bit in Kconfig, as it's causing crashes for some people. Thanks to: Christophe Leroy, Rui Salvaterra. - -- Christophe Leroy (1): Revert "powerpc/32s: reorder Linux PTE bits to better match Hash PTE bits." Michael Ellerman (1): powerpc/64s: Disable STRICT_KERNEL_RWX arch/powerpc/Kconfig | 2 +- arch/powerpc/include/asm/book3s/32/hash.h | 8 arch/powerpc/kernel/head_32.S | 9 ++--- arch/powerpc/mm/book3s32/hash_low.S | 14 -- 4 files changed, 19 insertions(+), 14 deletions(-) -BEGIN PGP SIGNATURE- iQIzBAEBCAAdFiEEJFGtCPCthwEv2Y/bUevqPMjhpYAFAl7H3DQACgkQUevqPMjh pYCc6RAAtxBm0aHlZsitFfLGRVWjXAanWlstF0Gnd/uOT+OLERE7MCD4PQzISKU8 XeHJORzhnfBgcN596vE5EHvTPNkZQdd+VW1Fxp9Pw1wcfZi0XACOdA5YrSN6nDlH vWUFE0WdcX7MksaviX8AnTmjK9MmHiKySZnTZ5cKlFcJWuawy4mjmK/L1y2DbAId YdGPJ/Yptda/aZbgi4hV8T1fhgl7odqKhrFf+I5SRojL6M2hnt0mSpUq5shk4Zsw YZtz86o10CLHpyXdi7zcJb0h7n8JLX3mB+veNVHrQ2o+NeSkwEhQJFYq2CzmTZSK n79vjG9iCg5KQIQ513cwPjJuo9tIoFULuBPomWiKQ8z/wAyJwzApAJ9y3YWTpQ3e MBo/rvhMCJvlDt2uAKp8DVMCaXdG8zeE/2iGcnvbQ5l9RwJDEUO8IcUHCdUyZchF erIjeGLodqB4Vm9+v8gOSjAL4CRrgVRHX6ZDZdOMoHkQBc36mMIu3/X4LfknZ1PT 6TR6AK+DQXY5pguiUeutHPEjzB+f5dgjueTnU4yjOST07xq28dRfeBvRAWdnH5ev bzQ/VAShdQBLTuwOcYajvYh9HCzcTvuRDOj4LrPRud/qIBECKjB9QlqbsWIPM1Vv RBX79Fs2ML1Lz3GtVXKUy+9n+KncCEC7H0YkbYiVRZdbYQBbw/U= =KT4L -END PGP SIGNATURE-
[PATCH v2] powerpc: Add ppc_inst_next()
In a few places we want to calculate the address of the next
instruction. Previously that was simple, we just added 4 bytes, or if
using a u32 * we incremented that pointer by 1.
But prefixed instructions make it more complicated, we need to advance
by either 4 or 8 bytes depending on the actual instruction. We also
can't do pointer arithmetic using struct ppc_inst, because it is
always 8 bytes in size on 64-bit, even though we might only need to
advance by 4 bytes.
So add a ppc_inst_next() helper which calculates the location of the
next instruction, if the given instruction was located at the given
address. Note the instruction doesn't need to actually be at the
address in memory.
Although it would seem natural for the value to be passed by value,
that makes it too easy to write a loop that will read off the end of a
page, eg:
for (; src < end; src = ppc_inst_next(src, *src),
dest = ppc_inst_next(dest, *dest))
As noticed by Christophe and Jordan, if end is the exact end of a
page, and the next page is not mapped, this will fault, because *dest
will read 8 bytes, 4 bytes into the next page.
So value is passed by reference, so the helper can be careful to use
ppc_inst_read() on it.
Signed-off-by: Michael Ellerman
---
arch/powerpc/include/asm/inst.h | 13 +
arch/powerpc/kernel/uprobes.c | 2 +-
arch/powerpc/lib/feature-fixups.c | 15 ---
arch/powerpc/xmon/xmon.c | 2 +-
4 files changed, 23 insertions(+), 9 deletions(-)
v2: Pass the value as a pointer and use ppc_inst_read() on it.
diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index d82e0c99cfa1..5b756ba77ed2 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -100,6 +100,19 @@ static inline int ppc_inst_len(struct ppc_inst x)
return ppc_inst_prefixed(x) ? 8 : 4;
}
+/*
+ * Return the address of the next instruction, if the instruction @value was
+ * located at @location.
+ */
+static inline struct ppc_inst *ppc_inst_next(void *location, struct ppc_inst
*value)
+{
+ struct ppc_inst tmp;
+
+ tmp = ppc_inst_read(value);
+
+ return location + ppc_inst_len(tmp);
+}
+
int probe_user_read_inst(struct ppc_inst *inst,
struct ppc_inst __user *nip);
diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index 83e883e1a42d..d200e7df7167 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -112,7 +112,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe,
struct pt_regs *regs)
* support doesn't exist and have to fix-up the next instruction
* to be executed.
*/
- regs->nip = utask->vaddr + ppc_inst_len(ppc_inst_read(>insn));
+ regs->nip = (unsigned long)ppc_inst_next((void *)utask->vaddr,
>insn);
user_disable_single_step(current);
return 0;
diff --git a/arch/powerpc/lib/feature-fixups.c
b/arch/powerpc/lib/feature-fixups.c
index 80f320c2e189..4c0a7ee9fa00 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -68,7 +68,7 @@ static int patch_alt_instruction(struct ppc_inst *src, struct
ppc_inst *dest,
static int patch_feature_section(unsigned long value, struct fixup_entry *fcur)
{
- struct ppc_inst *start, *end, *alt_start, *alt_end, *src, *dest;
+ struct ppc_inst *start, *end, *alt_start, *alt_end, *src, *dest, nop;
start = calc_addr(fcur, fcur->start_off);
end = calc_addr(fcur, fcur->end_off);
@@ -84,14 +84,15 @@ static int patch_feature_section(unsigned long value,
struct fixup_entry *fcur)
src = alt_start;
dest = start;
- for (; src < alt_end; src = (void *)src +
ppc_inst_len(ppc_inst_read(src)),
-(dest = (void *)dest + ppc_inst_len(ppc_inst_read(dest {
+ for (; src < alt_end; src = ppc_inst_next(src, src),
+ dest = ppc_inst_next(dest, dest)) {
if (patch_alt_instruction(src, dest, alt_start, alt_end))
return 1;
}
- for (; dest < end; dest = (void *)dest +
ppc_inst_len(ppc_inst(PPC_INST_NOP)))
- raw_patch_instruction(dest, ppc_inst(PPC_INST_NOP));
+ nop = ppc_inst(PPC_INST_NOP);
+ for (; dest < end; dest = ppc_inst_next(dest, ))
+ raw_patch_instruction(dest, nop);
return 0;
}
@@ -405,8 +406,8 @@ static void do_final_fixups(void)
while (src < end) {
inst = ppc_inst_read(src);
raw_patch_instruction(dest, inst);
- src = (void *)src + ppc_inst_len(inst);
- dest = (void *)dest + ppc_inst_len(inst);
+ src = ppc_inst_next(src, src);
+ dest = ppc_inst_next(dest, dest);
}
#endif
}
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index fb135f2cd6b0..65cf853a4d26 100644
---
Re: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe
On Thu, May 21, 2020 at 07:30:04PM +0800, Shengjiu Wang wrote: > On Wed, May 20, 2020 at 8:38 PM Mark Brown wrote: > > Other drivers having problems means those drivers should be fixed, not > > that we should copy the problems. In the case of the PXA driver that's > > very old code which predates deferred probe by I'd guess a decade. > Thanks. > For the FE-BE case, do you have any suggestion for how fix it? > With DMA1->ASRC->DMA2->ESAI case, the DMA1->ASRC->DMA2 > is in FE, ESAI is in BE. When ESAI drvier probe, DMA3 channel is > created with ESAI's "dma:tx" (DMA3 channel > is not used in this FE-BE case).When FE-BE startup, DMA2 > channel is created, it needs the ESAI's "dma:tx", so the warning > comes out. Not really TBH, this seems like another one of those csaes where DPCM is creaking at the seams :/ signature.asc Description: PGP signature
Re: [PATCH v2 3/5] libnvdimm/nvdimm/flush: Allow architecture to override the flush barrier
On Fri, 22 May 2020, Aneesh Kumar K.V wrote:
> On 5/22/20 3:01 PM, Michal Suchánek wrote:
> > On Thu, May 21, 2020 at 02:52:30PM -0400, Mikulas Patocka wrote:
> > >
> > >
> > > On Thu, 21 May 2020, Dan Williams wrote:
> > >
> > > > On Thu, May 21, 2020 at 10:03 AM Aneesh Kumar K.V
> > > > wrote:
> > > > >
> > > > > > Moving on to the patch itself--Aneesh, have you audited other
> > > > > > persistent
> > > > > > memory users in the kernel? For example, drivers/md/dm-writecache.c
> > > > > > does
> > > > > > this:
> > > > > >
> > > > > > static void writecache_commit_flushed(struct dm_writecache *wc, bool
> > > > > > wait_for_ios)
> > > > > > {
> > > > > >if (WC_MODE_PMEM(wc))
> > > > > >wmb(); <==
> > > > > > else
> > > > > > ssd_commit_flushed(wc, wait_for_ios);
> > > > > > }
> > > > > >
> > > > > > I believe you'll need to make modifications there.
> > > > > >
> > > > >
> > > > > Correct. Thanks for catching that.
> > > > >
> > > > >
> > > > > I don't understand dm much, wondering how this will work with
> > > > > non-synchronous DAX device?
> > > >
> > > > That's a good point. DM-writecache needs to be cognizant of things
> > > > like virtio-pmem that violate the rule that persisent memory writes
> > > > can be flushed by CPU functions rather than calling back into the
> > > > driver. It seems we need to always make the flush case a dax_operation
> > > > callback to account for this.
> > >
> > > dm-writecache is normally sitting on the top of dm-linear, so it would
> > > need to pass the wmb() call through the dm core and dm-linear target ...
> > > that would slow it down ... I remember that you already did it this way
> > > some times ago and then removed it.
> > >
> > > What's the exact problem with POWER? Could the POWER system have two types
> > > of persistent memory that need two different ways of flushing?
> >
> > As far as I understand the discussion so far
> >
> > - on POWER $oldhardware uses $oldinstruction to ensure pmem consistency
> > - on POWER $newhardware uses $newinstruction to ensure pmem consistency
> > (compatible with $oldinstruction on $oldhardware)
>
> Correct.
>
> > - on some platforms instead of barrier instruction a callback into the
> > driver is issued to ensure consistency
>
> This is virtio-pmem only at this point IIUC.
>
> -aneesh
And does the virtio-pmem driver track which pages are dirty? Or does it
need to specify the range of pages to flush in the flush function?
> > None of this is reflected by the dm driver.
We could make a new dax method:
void *(dax_get_flush_function)(void);
This would return a pointer to "wmb()" on x86 and something else on Power.
The method "dax_get_flush_function" would be called only once when
initializing the writecache driver (because the call would be slow because
it would have to go through the DM stack) and then, the returned function
would be called each time we need write ordering. The returned function
would do just "sfence; ret".
Mikulas
[powerpc:fixes-test] BUILD SUCCESS 8659a0e0efdd975c73355dbc033f79ba3b31e82c
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test branch HEAD: 8659a0e0efdd975c73355dbc033f79ba3b31e82c powerpc/64s: Disable STRICT_KERNEL_RWX elapsed time: 1260m configs tested: 104 configs skipped: 1 The following configs have been built successfully. More configs may be tested in the coming days. arm64allyesconfig arm64 defconfig arm64allmodconfig arm64 allnoconfig arm defconfig arm allyesconfig arm allmodconfig arm allnoconfig sparcallyesconfig mips allyesconfig m68k allyesconfig i386 allnoconfig i386 allyesconfig i386defconfig i386 debian-10.3 ia64 allmodconfig ia64defconfig ia64 allnoconfig ia64 allyesconfig m68k allmodconfig m68k allnoconfig m68k sun3_defconfig m68kdefconfig nios2 defconfig nios2allyesconfig openriscdefconfig c6x allyesconfig c6x allnoconfig openrisc allyesconfig nds32 defconfig nds32 allnoconfig csky allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa defconfig xtensa allyesconfig h8300allyesconfig h8300allmodconfig arc defconfig arc allyesconfig sh allmodconfig shallnoconfig microblazeallnoconfig mips allnoconfig mips allmodconfig pariscallnoconfig parisc defconfig parisc allyesconfig parisc allmodconfig powerpc defconfig powerpc allyesconfig powerpc rhel-kconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a001-20200521 i386 randconfig-a004-20200521 i386 randconfig-a006-20200521 i386 randconfig-a003-20200521 i386 randconfig-a002-20200521 i386 randconfig-a005-20200521 x86_64 randconfig-a013-20200520 x86_64 randconfig-a015-20200520 x86_64 randconfig-a016-20200520 x86_64 randconfig-a012-20200520 x86_64 randconfig-a014-20200520 x86_64 randconfig-a011-20200520 i386 randconfig-a013-20200522 i386 randconfig-a012-20200522 i386 randconfig-a015-20200522 i386 randconfig-a011-20200522 i386 randconfig-a016-20200522 i386 randconfig-a014-20200522 x86_64 randconfig-a002-20200521 x86_64 randconfig-a006-20200521 x86_64 randconfig-a005-20200521 x86_64 randconfig-a004-20200521 x86_64 randconfig-a003-20200521 x86_64 randconfig-a001-20200521 riscvallyesconfig riscv allnoconfig riscv defconfig riscvallmodconfig s390 allyesconfig s390 allnoconfig s390 allmodconfig s390defconfig x86_64 defconfig sparc defconfig sparc64 defconfig sparc64 allnoconfig sparc64 allyesconfig sparc64 allmodconfig umallnoconfig um defconfig um allyesconfig um allmodconfig x86_64 rhel x86_64 rhel-7.6 x86_64rhel-7.6-kselftests x86_64 rhel-7.2-clear
Re: [PATCH v2 3/5] libnvdimm/nvdimm/flush: Allow architecture to override the flush barrier
On 5/22/20 3:01 PM, Michal Suchánek wrote:
On Thu, May 21, 2020 at 02:52:30PM -0400, Mikulas Patocka wrote:
On Thu, 21 May 2020, Dan Williams wrote:
On Thu, May 21, 2020 at 10:03 AM Aneesh Kumar K.V
wrote:
Moving on to the patch itself--Aneesh, have you audited other persistent
memory users in the kernel? For example, drivers/md/dm-writecache.c does
this:
static void writecache_commit_flushed(struct dm_writecache *wc, bool
wait_for_ios)
{
if (WC_MODE_PMEM(wc))
wmb(); <==
else
ssd_commit_flushed(wc, wait_for_ios);
}
I believe you'll need to make modifications there.
Correct. Thanks for catching that.
I don't understand dm much, wondering how this will work with
non-synchronous DAX device?
That's a good point. DM-writecache needs to be cognizant of things
like virtio-pmem that violate the rule that persisent memory writes
can be flushed by CPU functions rather than calling back into the
driver. It seems we need to always make the flush case a dax_operation
callback to account for this.
dm-writecache is normally sitting on the top of dm-linear, so it would
need to pass the wmb() call through the dm core and dm-linear target ...
that would slow it down ... I remember that you already did it this way
some times ago and then removed it.
What's the exact problem with POWER? Could the POWER system have two types
of persistent memory that need two different ways of flushing?
As far as I understand the discussion so far
- on POWER $oldhardware uses $oldinstruction to ensure pmem consistency
- on POWER $newhardware uses $newinstruction to ensure pmem consistency
(compatible with $oldinstruction on $oldhardware)
Correct.
- on some platforms instead of barrier instruction a callback into the
driver is issued to ensure consistency
This is virtio-pmem only at this point IIUC.
None of this is reflected by the dm driver.
-aneesh
[PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume
With dedicated power domain for asrc, power can be disabled after
probe and pm runtime suspend, then the value of all registers need to
be restored in pm runtime resume. So we can merge suspend/resume function
to runtime_suspend/resume function and enable regcache only in end of
probe.
Signed-off-by: Shengjiu Wang
---
sound/soc/fsl/fsl_asrc.c | 70
1 file changed, 27 insertions(+), 43 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 432936039de4..3ebbe15ac378 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -1100,6 +1100,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, asrc);
pm_runtime_enable(>dev);
spin_lock_init(>lock);
+ regcache_cache_only(asrc->regmap, true);
ret = devm_snd_soc_register_component(>dev, _asrc_component,
_asrc_dai, 1);
@@ -1117,6 +1118,7 @@ static int fsl_asrc_runtime_resume(struct device *dev)
struct fsl_asrc *asrc = dev_get_drvdata(dev);
struct fsl_asrc_priv *asrc_priv = asrc->private;
int i, ret;
+ u32 asrctr;
ret = clk_prepare_enable(asrc->mem_clk);
if (ret)
@@ -1135,6 +1137,24 @@ static int fsl_asrc_runtime_resume(struct device *dev)
goto disable_asrck_clk;
}
+ /* Stop all pairs provisionally */
+ regmap_read(asrc->regmap, REG_ASRCTR, );
+ regmap_update_bits(asrc->regmap, REG_ASRCTR,
+ ASRCTR_ASRCEi_ALL_MASK, 0);
+
+ /* Restore all registers */
+ regcache_cache_only(asrc->regmap, false);
+ regcache_mark_dirty(asrc->regmap);
+ regcache_sync(asrc->regmap);
+
+ regmap_update_bits(asrc->regmap, REG_ASRCFG,
+ ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
+ ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
+
+ /* Restart enabled pairs */
+ regmap_update_bits(asrc->regmap, REG_ASRCTR,
+ ASRCTR_ASRCEi_ALL_MASK, asrctr);
+
return 0;
disable_asrck_clk:
@@ -1155,6 +1175,11 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
struct fsl_asrc_priv *asrc_priv = asrc->private;
int i;
+ regmap_read(asrc->regmap, REG_ASRCFG,
+ _priv->regcache_cfg);
+
+ regcache_cache_only(asrc->regmap, true);
+
for (i = 0; i < ASRC_CLK_MAX_NUM; i++)
clk_disable_unprepare(asrc_priv->asrck_clk[i]);
if (!IS_ERR(asrc->spba_clk))
@@ -1166,51 +1191,10 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
}
#endif /* CONFIG_PM */
-#ifdef CONFIG_PM_SLEEP
-static int fsl_asrc_suspend(struct device *dev)
-{
- struct fsl_asrc *asrc = dev_get_drvdata(dev);
- struct fsl_asrc_priv *asrc_priv = asrc->private;
-
- regmap_read(asrc->regmap, REG_ASRCFG,
- _priv->regcache_cfg);
-
- regcache_cache_only(asrc->regmap, true);
- regcache_mark_dirty(asrc->regmap);
-
- return 0;
-}
-
-static int fsl_asrc_resume(struct device *dev)
-{
- struct fsl_asrc *asrc = dev_get_drvdata(dev);
- struct fsl_asrc_priv *asrc_priv = asrc->private;
- u32 asrctr;
-
- /* Stop all pairs provisionally */
- regmap_read(asrc->regmap, REG_ASRCTR, );
- regmap_update_bits(asrc->regmap, REG_ASRCTR,
- ASRCTR_ASRCEi_ALL_MASK, 0);
-
- /* Restore all registers */
- regcache_cache_only(asrc->regmap, false);
- regcache_sync(asrc->regmap);
-
- regmap_update_bits(asrc->regmap, REG_ASRCFG,
- ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
- ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
-
- /* Restart enabled pairs */
- regmap_update_bits(asrc->regmap, REG_ASRCTR,
- ASRCTR_ASRCEi_ALL_MASK, asrctr);
-
- return 0;
-}
-#endif /* CONFIG_PM_SLEEP */
-
static const struct dev_pm_ops fsl_asrc_pm = {
SET_RUNTIME_PM_OPS(fsl_asrc_runtime_suspend, fsl_asrc_runtime_resume,
NULL)
- SET_SYSTEM_SLEEP_PM_OPS(fsl_asrc_suspend, fsl_asrc_resume)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
};
static const struct fsl_asrc_soc_data fsl_asrc_imx35_data = {
--
2.21.0
Re: [PATCH v2 3/5] libnvdimm/nvdimm/flush: Allow architecture to override the flush barrier
On Thu, May 21, 2020 at 02:52:30PM -0400, Mikulas Patocka wrote:
>
>
> On Thu, 21 May 2020, Dan Williams wrote:
>
> > On Thu, May 21, 2020 at 10:03 AM Aneesh Kumar K.V
> > wrote:
> > >
> > > > Moving on to the patch itself--Aneesh, have you audited other persistent
> > > > memory users in the kernel? For example, drivers/md/dm-writecache.c
> > > > does
> > > > this:
> > > >
> > > > static void writecache_commit_flushed(struct dm_writecache *wc, bool
> > > > wait_for_ios)
> > > > {
> > > > if (WC_MODE_PMEM(wc))
> > > > wmb(); <==
> > > > else
> > > > ssd_commit_flushed(wc, wait_for_ios);
> > > > }
> > > >
> > > > I believe you'll need to make modifications there.
> > > >
> > >
> > > Correct. Thanks for catching that.
> > >
> > >
> > > I don't understand dm much, wondering how this will work with
> > > non-synchronous DAX device?
> >
> > That's a good point. DM-writecache needs to be cognizant of things
> > like virtio-pmem that violate the rule that persisent memory writes
> > can be flushed by CPU functions rather than calling back into the
> > driver. It seems we need to always make the flush case a dax_operation
> > callback to account for this.
>
> dm-writecache is normally sitting on the top of dm-linear, so it would
> need to pass the wmb() call through the dm core and dm-linear target ...
> that would slow it down ... I remember that you already did it this way
> some times ago and then removed it.
>
> What's the exact problem with POWER? Could the POWER system have two types
> of persistent memory that need two different ways of flushing?
As far as I understand the discussion so far
- on POWER $oldhardware uses $oldinstruction to ensure pmem consistency
- on POWER $newhardware uses $newinstruction to ensure pmem consistency
(compatible with $oldinstruction on $oldhardware)
- on some platforms instead of barrier instruction a callback into the
driver is issued to ensure consistency
None of this is reflected by the dm driver.
Thanks
Michal
Re: [RESEND PATCH v7 4/5] ndctl/papr_scm, uapi: Add support for PAPR nvdimm specific methods
Michael Ellerman writes:
> Ira Weiny writes:
>> On Wed, May 20, 2020 at 12:30:57AM +0530, Vaibhav Jain wrote:
>>> Introduce support for Papr nvDimm Specific Methods (PDSM) in papr_scm
>>> modules and add the command family to the white list of NVDIMM command
>>> sets. Also advertise support for ND_CMD_CALL for the dimm
>>> command mask and implement necessary scaffolding in the module to
>>> handle ND_CMD_CALL ioctl and PDSM requests that we receive.
> ...
>>> + *
>>> + * Payload Version:
>>> + *
>>> + * A 'payload_version' field is present in PDSM header that indicates a
>>> specific
>>> + * version of the structure present in PDSM Payload for a given PDSM
>>> command.
>>> + * This provides backward compatibility in case the PDSM Payload structure
>>> + * evolves and different structures are supported by 'papr_scm' and
>>> 'libndctl'.
>>> + *
>>> + * When sending a PDSM Payload to 'papr_scm', 'libndctl' should send the
>>> version
>>> + * of the payload struct it supports via 'payload_version' field. The
>>> 'papr_scm'
>>> + * module when servicing the PDSM envelope checks the 'payload_version'
>>> and then
>>> + * uses 'payload struct version' == MIN('payload_version field',
>>> + * 'max payload-struct-version supported by papr_scm') to service the PDSM.
>>> + * After servicing the PDSM, 'papr_scm' put the negotiated version of
>>> payload
>>> + * struct in returned 'payload_version' field.
>>
>> FWIW many people believe using a size rather than version is more
>> sustainable.
>> It is expected that new payload structures are larger (more features) than
>> the
>> previous payload structure.
>>
>> I can't find references at the moment through.
>
> I think clone_args is a good modern example:
>
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/sched.h#n88
>
> cheers
Thank Ira and Mpe for pointing this out. I looked into how clone3 sycall
handles clone_args and few differences came out:
* Unlike clone_args that are always transferred in one direction from
user-space to kernel, payload contents of pdsms are transferred in both
directions. Having a single version number makes it easier for
user-space and kernel to determine what data will be exchanged.
* For PDSMs, the version number is negotiated between libndctl and
kernel. For example in case kernel only supports an older version of
a structure, its free to send a lower version number back to
libndctl. Such negotiations doesnt happen with clone3 syscall.
--
Cheers
~ Vaibhav
Re: [PATCH V3 0/3] arm64: Enable vmemmap mapping from device memory
Hi On 2020/3/31 13:09, Anshuman Khandual wrote: This series enables vmemmap backing memory allocation from device memory ranges on arm64. But before that, it enables vmemmap_populate_basepages() and vmemmap_alloc_block_buf() to accommodate struct vmem_altmap based alocation requests. I verified no obvious regression after this patch series. Host: ThunderX2(armv8a server), kernel v5.4 qemu:v3.1, -M virt \ -object memory-backend-file,id=mem1,share=on,mem-path=/tmp2/nvdimm.img,size=4G,align=2M \ -device nvdimm,id=nvdimm1,memdev=mem1,label-size=2M Guest: kernel v5.7.0-rc5 with this patch series. Tested case: - 4K PAGESIZE, boot, mount w/ -o dax, mount w/o -o dax, basic io - 64K PAGESIZE,boot, mount w/ -o dax, mount w/o -o dax, basic io Not tested: - 16K pagesize due to my hardware limiation(can't run 16K pgsz kernel) - hot-add/remove nvdimm device from qemu due to no fully support on arm64 qemu yet - Host nvdimm device hotplug Hence from above result, Tested-by: Jia He This series applies after latest (v14) arm64 memory hot remove series (https://lkml.org/lkml/2020/3/3/1746) on Linux 5.6. Pending Question: altmap_alloc_block_buf() does not have any other remaining users in the tree after this change. Should it be converted into a static function and it's declaration be dropped from the header (include/linux/mm.h). Avoided doing so because I was not sure if there are any off-tree users or not. Changes in V3: - Dropped comment from free_hotplug_page_range() per Robin - Modified comment in unmap_hotplug_range() per Robin - Enabled altmap support in vmemmap_alloc_block_buf() per Robin Changes in V2: (https://lkml.org/lkml/2020/3/4/475) - Rebased on latest hot-remove series (v14) adding P4D page table support Changes in V1: (https://lkml.org/lkml/2020/1/23/12) - Added an WARN_ON() in unmap_hotplug_range() when altmap is provided without the page table backing memory being freed Changes in RFC V2: (https://lkml.org/lkml/2019/10/21/11) - Changed the commit message on 1/2 patch per Will - Changed the commit message on 2/2 patch as well - Rebased on arm64 memory hot remove series (v10) RFC V1: (https://lkml.org/lkml/2019/6/28/32) Cc: Catalin Marinas Cc: Will Deacon Cc: Mark Rutland Cc: Paul Walmsley Cc: Palmer Dabbelt Cc: Tony Luck Cc: Fenghua Yu Cc: Dave Hansen Cc: Andy Lutomirski Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Ingo Molnar Cc: David Hildenbrand Cc: Mike Rapoport Cc: Michal Hocko Cc: "Matthew Wilcox (Oracle)" Cc: "Kirill A. Shutemov" Cc: Andrew Morton Cc: Dan Williams Cc: Pavel Tatashin Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc:linux-arm-ker...@lists.infradead.org Cc:linux-i...@vger.kernel.org Cc:linux-ri...@lists.infradead.org Cc:x...@kernel.org Cc:linuxppc-dev@lists.ozlabs.org Cc:linux...@kvack.org Cc:linux-ker...@vger.kernel.org Anshuman Khandual (3): mm/sparsemem: Enable vmem_altmap support in vmemmap_populate_basepages() mm/sparsemem: Enable vmem_altmap support in vmemmap_alloc_block_buf() arm64/mm: Enable vmem_altmap support for vmemmap mappings arch/arm64/mm/mmu.c | 59 ++- arch/ia64/mm/discontig.c | 2 +- arch/powerpc/mm/init_64.c | 10 +++ arch/riscv/mm/init.c | 2 +- arch/x86/mm/init_64.c | 12 include/linux/mm.h| 8 -- mm/sparse-vmemmap.c | 38 - 7 files changed, 87 insertions(+), 44 deletions(-) -- --- Cheers, Justin (Jia He)
