* Re: [PATCH 1/2] dma-mapping: introduce dma_get_seg_boundary_nr_pages()
From: Andy Shevchenko @ 2020-09-03 10:57 UTC (permalink / raw)
To: Nicolin Chen
Cc: linux-ia64, James Bottomley, Paul Mackerras, H. Peter Anvin,
sparclinux, Christoph Hellwig, Stephen Rothwell, Helge Deller,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
Christian Borntraeger, Ingo Molnar, Matt Turner, Fenghua Yu,
Vasily Gorbik, schnelle, hca, ink, Thomas Gleixner,
gerald.schaefer, rth, Tony Luck, linux-parisc, linux-s390,
Linux Kernel Mailing List, linux-alpha, Borislav Petkov,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, David S. Miller
In-Reply-To: <20200901221646.26491-2-nicoleotsuka@gmail.com>
On Wed, Sep 2, 2020 at 1:20 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> We found that callers of dma_get_seg_boundary mostly do an ALIGN
> with page mask and then do a page shift to get number of pages:
> ALIGN(boundary + 1, 1 << shift) >> shift
>
> However, the boundary might be as large as ULONG_MAX, which means
> that a device has no specific boundary limit. So either "+ 1" or
> passing it to ALIGN() would potentially overflow.
>
> According to kernel defines:
> #define ALIGN_MASK(x, mask) (((x) + (mask)) & ~(mask))
> #define ALIGN(x, a) ALIGN_MASK(x, (typeof(x))(a) - 1)
>
> We can simplify the logic here into a helper function doing:
> ALIGN(boundary + 1, 1 << shift) >> shift
> = ALIGN_MASK(b + 1, (1 << s) - 1) >> s
> = {[b + 1 + (1 << s) - 1] & ~[(1 << s) - 1]} >> s
> = [b + 1 + (1 << s) - 1] >> s
> = [b + (1 << s)] >> s
> = (b >> s) + 1
>
> This patch introduces and applies dma_get_seg_boundary_nr_pages()
> as an overflow-free helper for the dma_get_seg_boundary() callers
> to get numbers of pages. It also takes care of the NULL dev case
> for non-DMA API callers.
...
> +static inline unsigned long dma_get_seg_boundary_nr_pages(struct device *dev,
> + unsigned int page_shift)
> +{
> + if (!dev)
> + return (U32_MAX >> page_shift) + 1;
> + return (dma_get_seg_boundary(dev) >> page_shift) + 1;
Can it be better to do something like
unsigned long boundary = dev ? dma_get_seg_boundary(dev) : U32_MAX;
return (boundary >> page_shift) + 1;
?
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v2] cpuidle-pseries: Fix CEDE latency conversion from tb to us
From: Gautham R. Shenoy @ 2020-09-03 9:27 UTC (permalink / raw)
To: Michael Ellerman, Rafael J. Wysocki, Vaidyanathan Srinivasan,
Joel Stanley
Cc: Gautham R. Shenoy, linuxppc-dev, linux-kernel, linux-pm
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
CEDE(0)") sets the exit latency of CEDE(0) based on the latency values
of the Extended CEDE states advertised by the platform. The values
advertised by the platform are in timebase ticks. However the cpuidle
framework requires the latency values in microseconds.
If the tb-ticks value advertised by the platform correspond to a value
smaller than 1us, during the conversion from tb-ticks to microseconds,
in the current code, the result becomes zero. This is incorrect as it
puts a CEDE state on par with the snooze state.
This patch fixes this by rounding up the result obtained while
converting the latency value from tb-ticks to microseconds. It also
prints a warning in case we discover an extended-cede state with
wakeup latency to be 0. In such a case, ensure that CEDE(0) has a
non-zero wakeup latency.
Fixes: commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
CEDE(0)")
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
v1-->v2: Added a warning if a CEDE state has 0 wakeup latency (Suggested by Joel Stanley)
Also added code to ensure that CEDE(0) has a non-zero wakeup latency.
drivers/cpuidle/cpuidle-pseries.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index ff6d99e..a2b5c6f 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -361,7 +361,10 @@ static void __init fixup_cede0_latency(void)
for (i = 0; i < nr_xcede_records; i++) {
struct xcede_latency_record *record = &payload->records[i];
u64 latency_tb = be64_to_cpu(record->latency_ticks);
- u64 latency_us = tb_to_ns(latency_tb) / NSEC_PER_USEC;
+ u64 latency_us = DIV_ROUND_UP_ULL(tb_to_ns(latency_tb), NSEC_PER_USEC);
+
+ if (latency_us == 0)
+ pr_warn("cpuidle: xcede record %d has an unrealistic latency of 0us.\n", i);
if (latency_us < min_latency_us)
min_latency_us = latency_us;
@@ -378,10 +381,14 @@ static void __init fixup_cede0_latency(void)
* Perform the fix-up.
*/
if (min_latency_us < dedicated_states[1].exit_latency) {
- u64 cede0_latency = min_latency_us - 1;
+ /*
+ * We set a minimum of 1us wakeup latency for cede0 to
+ * distinguish it from snooze
+ */
+ u64 cede0_latency = 1;
- if (cede0_latency <= 0)
- cede0_latency = min_latency_us;
+ if (min_latency_us > cede0_latency)
+ cede0_latency = min_latency_us - 1;
dedicated_states[1].exit_latency = cede0_latency;
dedicated_states[1].target_residency = 10 * (cede0_latency);
--
1.9.4
^ permalink raw reply related
* Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: Christophe Leroy @ 2020-09-03 8:55 UTC (permalink / raw)
To: Christoph Hellwig, Linus Torvalds
Cc: linux-arch, Kees Cook, the arch/x86 maintainers,
Linux Kernel Mailing List, Al Viro, linux-fsdevel, linuxppc-dev
In-Reply-To: <20200903071144.GA19247@lst.de>
Le 03/09/2020 à 09:11, Christoph Hellwig a écrit :
>
> Except that we do not actually have such a patch. For normal user
> writes we only use ->write_iter if ->write is not present. But what
> shows up in the profile is that /dev/zero only has a read_iter op and
> not a normal read. I've added a patch below that implements a normal
> read which might help a tad with this workload, but should not be part
> of a regression.
>
With that patch below, throughput is 113.5MB/s (instead of 99.9MB/s).
So a 14% improvement. That's not bad.
Christophe
>
> ---
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index abd4ffdc8cdebc..1dc99ab158457a 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -726,6 +726,27 @@ static ssize_t read_iter_zero(struct kiocb *iocb, struct iov_iter *iter)
> return written;
> }
>
> +static ssize_t read_zero(struct file *file, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + size_t cleared = 0;
> +
> + while (count) {
> + size_t chunk = min_t(size_t, count, PAGE_SIZE);
> +
> + if (clear_user(buf + cleared, chunk))
> + return cleared ? cleared : -EFAULT;
> + cleared += chunk;
> + count -= chunk;
> +
> + if (signal_pending(current))
> + return cleared ? cleared : -ERESTARTSYS;
> + cond_resched();
> + }
> +
> + return cleared;
> +}
> +
> static int mmap_zero(struct file *file, struct vm_area_struct *vma)
> {
> #ifndef CONFIG_MMU
> @@ -921,6 +942,7 @@ static const struct file_operations zero_fops = {
> .llseek = zero_lseek,
> .write = write_zero,
> .read_iter = read_iter_zero,
> + .read = read_zero,
> .write_iter = write_iter_zero,
> .mmap = mmap_zero,
> .get_unmapped_area = get_unmapped_area_zero,
>
^ permalink raw reply
* Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: Christophe Leroy @ 2020-09-03 7:27 UTC (permalink / raw)
To: Christoph Hellwig, Linus Torvalds
Cc: linux-arch, Kees Cook, the arch/x86 maintainers,
Linux Kernel Mailing List, Al Viro, linux-fsdevel, linuxppc-dev
In-Reply-To: <20200903071144.GA19247@lst.de>
Le 03/09/2020 à 09:11, Christoph Hellwig a écrit :
> On Wed, Sep 02, 2020 at 11:02:22AM -0700, Linus Torvalds wrote:
>> I don't see why this change would make any difference.
>
> Me neither, but while looking at a different project I did spot places
> that actually do an access_ok with len 0, that's why I wanted him to
> try.
>
> That being said: Christophe are these number stables? Do you get
> similar numbers with multiple runs?
Yes the numbers are similar with multiple runs and multiple reboots.
>
>> And btw, why do the 32-bit and 64-bit checks even differ? It's not
>> like the extra (single) instruction should even matter. I think the
>> main reason is that the simpler 64-bit case could stay as a macro
>> (because it only uses "addr" and "size" once), but honestly, that
>> "simplification" doesn't help when you then need to have that #ifdef
>> for the 32-bit case and an inline function anyway.
>
> I'll have to leave that to the powerpc folks. The intent was to not
> change the behavior (and I even fucked that up for the the size == 0
> case).
>
>> However, I suspect a bigger reason for the actual performance
>> degradation would be the patch that makes things use "write_iter()"
>> for writing, even when a simpler "write()" exists.
>
> Except that we do not actually have such a patch. For normal user
> writes we only use ->write_iter if ->write is not present. But what
> shows up in the profile is that /dev/zero only has a read_iter op and
> not a normal read. I've added a patch below that implements a normal
> read which might help a tad with this workload, but should not be part
> of a regression.
>
> Also Christophe: can you bisect which patch starts this? Is it really
> this last patch in the series?
5.9-rc2: 91.5MB/s
Patch 1: 74.9MB/s
Patch 2: 97.9MB/s
Patch 3: 97.7MB/s
Patch 4 to 9: 97.9MB/s
Patch 10: 85.3MB/s
Patch 11: 75.4MB/s
See my other mail, when removing CONFIG_STACKPROTECTOR, I get a stable
99.8MB/s throughput.
Christophe
^ permalink raw reply
* Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: Christophe Leroy @ 2020-09-03 7:20 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-arch, Kees Cook, the arch/x86 maintainers,
Linux Kernel Mailing List, Al Viro, linux-fsdevel, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <CAHk-=wiDCcxuHgENo3UtdFi2QW9B7yXvNpG5CtF=A6bc6PTTgA@mail.gmail.com>
Le 02/09/2020 à 20:02, Linus Torvalds a écrit :
> On Wed, Sep 2, 2020 at 8:17 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>>
>> With this fix, I get
>>
>> root@vgoippro:~# time dd if=/dev/zero of=/dev/null count=1M
>> 536870912 bytes (512.0MB) copied, 6.776327 seconds, 75.6MB/s
>>
>> That's still far from the 91.7MB/s I get with 5.9-rc2, but better than
>> the 65.8MB/s I got yesterday with your series. Still some way to go thought.
>
> I don't see why this change would make any difference.
>
Neither do I.
Looks like nowadays, CONFIG_STACKPROTECTOR has become a default.
I rebuilt the kernel without it, I now get a throughput of 99.8MB/s both
without and with this series.
Looking at the generated code (GCC 10.1), a small change in a function
seems to make large changes in the generated code when
CONFIG_STACKPROTECTOR is set.
In addition to that, trivial functions which don't use the stack at all
get a stack frame anyway when CONFIG_STACKPROTECTOR is set, allthough
that's only -fstack-protector-strong. And there is no canary check.
Without CONFIG_STACKPROTECTOR:
c01572a0 <no_llseek>:
c01572a0: 38 60 ff ff li r3,-1
c01572a4: 38 80 ff e3 li r4,-29
c01572a8: 4e 80 00 20 blr
With CONFIG_STACKPROTECTOR (regardless of CONFIG_STACKPROTECTOR_STRONG
or not):
c0164e08 <no_llseek>:
c0164e08: 94 21 ff f0 stwu r1,-16(r1)
c0164e0c: 38 60 ff ff li r3,-1
c0164e10: 38 80 ff e3 li r4,-29
c0164e14: 38 21 00 10 addi r1,r1,16
c0164e18: 4e 80 00 20 blr
Wondering why CONFIG_STACKPROTECTOR has become the default. It seems to
imply a 10% performance loss even in the best case (91.7MB/s versus
99.8MB/s)
Note that without CONFIG_STACKPROTECTOR_STRONG, I'm at 99.3MB/s, so
that's really the _STRONG alternative that hurts.
Christophe
^ permalink raw reply
* Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: Christoph Hellwig @ 2020-09-03 7:11 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-arch, Kees Cook, the arch/x86 maintainers,
Linux Kernel Mailing List, Al Viro, linux-fsdevel, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <CAHk-=wiDCcxuHgENo3UtdFi2QW9B7yXvNpG5CtF=A6bc6PTTgA@mail.gmail.com>
On Wed, Sep 02, 2020 at 11:02:22AM -0700, Linus Torvalds wrote:
> I don't see why this change would make any difference.
Me neither, but while looking at a different project I did spot places
that actually do an access_ok with len 0, that's why I wanted him to
try.
That being said: Christophe are these number stables? Do you get
similar numbers with multiple runs?
> And btw, why do the 32-bit and 64-bit checks even differ? It's not
> like the extra (single) instruction should even matter. I think the
> main reason is that the simpler 64-bit case could stay as a macro
> (because it only uses "addr" and "size" once), but honestly, that
> "simplification" doesn't help when you then need to have that #ifdef
> for the 32-bit case and an inline function anyway.
I'll have to leave that to the powerpc folks. The intent was to not
change the behavior (and I even fucked that up for the the size == 0
case).
> However, I suspect a bigger reason for the actual performance
> degradation would be the patch that makes things use "write_iter()"
> for writing, even when a simpler "write()" exists.
Except that we do not actually have such a patch. For normal user
writes we only use ->write_iter if ->write is not present. But what
shows up in the profile is that /dev/zero only has a read_iter op and
not a normal read. I've added a patch below that implements a normal
read which might help a tad with this workload, but should not be part
of a regression.
Also Christophe: can you bisect which patch starts this? Is it really
this last patch in the series?
---
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index abd4ffdc8cdebc..1dc99ab158457a 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -726,6 +726,27 @@ static ssize_t read_iter_zero(struct kiocb *iocb, struct iov_iter *iter)
return written;
}
+static ssize_t read_zero(struct file *file, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ size_t cleared = 0;
+
+ while (count) {
+ size_t chunk = min_t(size_t, count, PAGE_SIZE);
+
+ if (clear_user(buf + cleared, chunk))
+ return cleared ? cleared : -EFAULT;
+ cleared += chunk;
+ count -= chunk;
+
+ if (signal_pending(current))
+ return cleared ? cleared : -ERESTARTSYS;
+ cond_resched();
+ }
+
+ return cleared;
+}
+
static int mmap_zero(struct file *file, struct vm_area_struct *vma)
{
#ifndef CONFIG_MMU
@@ -921,6 +942,7 @@ static const struct file_operations zero_fops = {
.llseek = zero_lseek,
.write = write_zero,
.read_iter = read_iter_zero,
+ .read = read_zero,
.write_iter = write_iter_zero,
.mmap = mmap_zero,
.get_unmapped_area = get_unmapped_area_zero,
^ permalink raw reply related
* [PATCH v5] soc: fsl: enable acpi support in RCPM driver
From: Ran Wang @ 2020-09-03 7:01 UTC (permalink / raw)
To: Li Yang, Christophe Leroy
Cc: Peng Ma, Ran Wang, linuxppc-dev, linux-kernel, linux-arm-kernel
From: Peng Ma <peng.ma@nxp.com>
This patch enables ACPI support in RCPM driver.
Signed-off-by: Peng Ma <peng.ma@nxp.com>
Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
---
Change in v5:
- Fix panic when dev->of_node is null
Change in v4:
- Make commit subject more accurate
- Remove unrelated new blank line
Change in v3:
- Add #ifdef CONFIG_ACPI for acpi_device_id
- Rename rcpm_acpi_imx_ids to rcpm_acpi_ids
Change in v2:
- Update acpi_device_id to fix conflict with other driver
drivers/soc/fsl/rcpm.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c
index a093dbe..58870f4 100644
--- a/drivers/soc/fsl/rcpm.c
+++ b/drivers/soc/fsl/rcpm.c
@@ -2,7 +2,7 @@
//
// rcpm.c - Freescale QorIQ RCPM driver
//
-// Copyright 2019 NXP
+// Copyright 2019-2020 NXP
//
// Author: Ran Wang <ran.wang_1@nxp.com>
@@ -13,6 +13,7 @@
#include <linux/slab.h>
#include <linux/suspend.h>
#include <linux/kernel.h>
+#include <linux/acpi.h>
#define RCPM_WAKEUP_CELL_MAX_SIZE 7
@@ -56,10 +57,14 @@ static int rcpm_pm_prepare(struct device *dev)
"fsl,rcpm-wakeup", value,
rcpm->wakeup_cells + 1);
- /* Wakeup source should refer to current rcpm device */
- if (ret || (np->phandle != value[0]))
+ if (ret)
continue;
+ if (is_of_node(dev->fwnode))
+ /* Should refer to current rcpm device */
+ if (np->phandle != value[0])
+ continue;
+
/* Property "#fsl,rcpm-wakeup-cells" of rcpm node defines the
* number of IPPDEXPCR register cells, and "fsl,rcpm-wakeup"
* of wakeup source IP contains an integer array: <phandle to
@@ -139,10 +144,19 @@ static const struct of_device_id rcpm_of_match[] = {
};
MODULE_DEVICE_TABLE(of, rcpm_of_match);
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id rcpm_acpi_ids[] = {
+ {"NXP0015",},
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, rcpm_acpi_ids);
+#endif
+
static struct platform_driver rcpm_driver = {
.driver = {
.name = "rcpm",
.of_match_table = rcpm_of_match,
+ .acpi_match_table = ACPI_PTR(rcpm_acpi_ids),
.pm = &rcpm_pm_ops,
},
.probe = rcpm_probe,
--
2.7.4
^ permalink raw reply related
* [PATCH v2] ASoC: fsl_sai: Set SAI Channel Mode to Output Mode
From: Shengjiu Wang @ 2020-09-03 5:53 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel, lgirdwood
Cc: linuxppc-dev, linux-kernel
Transmit data pins will output zero when slots are masked or channels
are disabled. In CHMOD TDM mode, transmit data pins are tri-stated when
slots are masked or channels are disabled. When data pins are tri-stated,
there is noise on some channels when FS clock value is high and data is
read while fsclk is transitioning from high to low.
Signed-off-by: Cosmin-Gabriel Samoila <cosmin.samoila@nxp.com>
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
changes in v2
- update note
- add acked-by Nicolin
sound/soc/fsl/fsl_sai.c | 10 ++++++++--
sound/soc/fsl/fsl_sai.h | 2 ++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 38c7bcbb361d..b2d65e53dbc4 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -489,6 +489,10 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
val_cr4 |= FSL_SAI_CR4_FRSZ(slots);
+ /* Set to output mode to avoid tri-stated data pins */
+ if (tx)
+ val_cr4 |= FSL_SAI_CR4_CHMOD;
+
/*
* For SAI master mode, when Tx(Rx) sync with Rx(Tx) clock, Rx(Tx) will
* generate bclk and frame clock for Tx(Rx), we should set RCR4(TCR4),
@@ -497,7 +501,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
if (!sai->is_slave_mode && fsl_sai_dir_is_synced(sai, adir)) {
regmap_update_bits(sai->regmap, FSL_SAI_xCR4(!tx, ofs),
- FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
+ FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK |
+ FSL_SAI_CR4_CHMOD_MASK,
val_cr4);
regmap_update_bits(sai->regmap, FSL_SAI_xCR5(!tx, ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
@@ -508,7 +513,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
FSL_SAI_CR3_TRCE_MASK,
FSL_SAI_CR3_TRCE((1 << pins) - 1));
regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
- FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
+ FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK |
+ FSL_SAI_CR4_CHMOD_MASK,
val_cr4);
regmap_update_bits(sai->regmap, FSL_SAI_xCR5(tx, ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 5f630be74853..736a437450c8 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -119,6 +119,8 @@
#define FSL_SAI_CR4_FRSZ_MASK (0x1f << 16)
#define FSL_SAI_CR4_SYWD(x) (((x) - 1) << 8)
#define FSL_SAI_CR4_SYWD_MASK (0x1f << 8)
+#define FSL_SAI_CR4_CHMOD BIT(5)
+#define FSL_SAI_CR4_CHMOD_MASK BIT(5)
#define FSL_SAI_CR4_MF BIT(4)
#define FSL_SAI_CR4_FSE BIT(3)
#define FSL_SAI_CR4_FSP BIT(1)
--
2.27.0
^ permalink raw reply related
* Re: [PATCH] ASoC: fsl_sai: Set SAI Channel Mode to Output Mode
From: Shengjiu Wang @ 2020-09-03 5:38 UTC (permalink / raw)
To: Nicolin Chen
Cc: Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Liam Girdwood, Takashi Iwai, Mark Brown, linuxppc-dev,
linux-kernel
In-Reply-To: <20200903034018.GC4517@Asurada-Nvidia>
On Thu, Sep 3, 2020 at 11:42 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Thu, Sep 03, 2020 at 11:09:15AM +0800, Shengjiu Wang wrote:
> > Transmit data pins will output zero when slots are masked or channels
> > are disabled. In CHMOD TDM mode, transmit data pins are tri-stated when
> > slots are masked or channels are disabled. When data pins are tri-stated,
> > there is noise on some channels when FS clock value is high and data is
> > read while fsclk is transitioning from high to low.
> >
> > Signed-off-by: Cosmin-Gabriel Samoila <cosmin.samoila@nxp.com>
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
>
> Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
>
> Though one nit inline:
>
> > ---
> > sound/soc/fsl/fsl_sai.c | 12 ++++++++++--
> > sound/soc/fsl/fsl_sai.h | 2 ++
> > 2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > index 62c5fdb678fc..33b194a5c1dc 100644
> > --- a/sound/soc/fsl/fsl_sai.c
> > +++ b/sound/soc/fsl/fsl_sai.c
> > @@ -486,6 +486,12 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
> >
> > val_cr4 |= FSL_SAI_CR4_FRSZ(slots);
> >
> > + /* Output Mode - data pins transmit 0 when slots are masked
> > + * or channels are disabled
> > + */
>
> Coding style for multi-line comments. Yet, probably can simplify?
>
> /* Set to output mode to avoid tri-stated data pins */
Ok, will update in v2.
best regards
wang shengjiu
^ permalink raw reply
* Re: [PATCH v2] scsi: ibmvfc: interface updates for future FPIN and MQ support
From: Tyrel Datwyler @ 2020-09-03 5:31 UTC (permalink / raw)
To: Martin K. Petersen
Cc: james.bottomley, brking, linuxppc-dev, linux-kernel, linux-scsi
In-Reply-To: <yq1y2lrd2ys.fsf@ca-mkp.ca.oracle.com>
On 9/2/20 7:11 PM, Martin K. Petersen wrote:
>
> Tyrel,
>
>> Fixup complier errors from neglected commit --amend
>
> Bunch of formatting-related checkpatch warnings. Please fix.
>
> Thanks!
>
So, I stuck to the existing style already in that header. If I'm going to fixup
to make checkpatch happy I imagine it makes sense to send a prerequisite patch
that fixes up the rest of the header.
-Tyrel
^ permalink raw reply
* Re: [PATCH v1 02/10] powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE on iommu_*_coherent()
From: Alexey Kardashevskiy @ 2020-09-03 4:41 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Christophe Leroy, Joel Stanley,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <39ad3a9c103faf9c5fc2fd5700d8606eb4a2b67e.camel@gmail.com>
On 02/09/2020 08:34, Leonardo Bras wrote:
> On Mon, 2020-08-31 at 10:47 +1000, Alexey Kardashevskiy wrote:
>>>
>>> Maybe testing with host 64k pagesize and IOMMU 16MB pagesize in qemu
>>> should be enough, is there any chance to get indirect mapping in qemu
>>> like this? (DDW but with smaller DMA window available)
>>
>> You will have to hack the guest kernel to always do indirect mapping or
>> hack QEMU's rtas_ibm_query_pe_dma_window() to return a small number of
>> available TCEs. But you will be testing QEMU/KVM which behave quite
>> differently to pHyp in this particular case.
>>
>
> As you suggested before, building for 4k cpu pagesize should be the
> best approach. It would allow testing for both pHyp and qemu scenarios.
>
>>>>>> Because if we want the former (==support), then we'll have to align the
>>>>>> size up to the bigger page size when allocating/zeroing system pages,
>>>>>> etc.
>>>>>
>>>>> This part I don't understand. Why do we need to align everything to the
>>>>> bigger pagesize?
>>>>>
>>>>> I mean, is not that enough that the range [ret, ret + size[ is both
>>>>> allocated by mm and mapped on a iommu range?
>>>>>
>>>>> Suppose a iommu_alloc_coherent() of 16kB on PAGESIZE = 4k and
>>>>> IOMMU_PAGE_SIZE() == 64k.
>>>>> Why 4 * cpu_pages mapped by a 64k IOMMU page is not enough?
>>>>> All the space the user asked for is allocated and mapped for DMA.
>>>>
>>>> The user asked to map 16K, the rest - 48K - is used for something else
>>>> (may be even mapped to another device) but you are making all 64K
>>>> accessible by the device which only should be able to access 16K.
>>>>
>>>> In practice, if this happens, H_PUT_TCE will simply fail.
>>>
>>> I have noticed mlx5 driver getting a few bytes in a buffer, and using
>>> iommu_map_page(). It does map a whole page for as few bytes as the user
>>
>> Whole 4K system page or whole 64K iommu page?
>
> I tested it in 64k system page + 64k iommu page.
>
> The 64K system page may be used for anything, and a small portion of it
> (say 128 bytes) needs to be used for DMA.
> The whole page is mapped by IOMMU, and the driver gets info of the
> memory range it should access / modify.
This works because the whole system page belongs to the same memory
context and IOMMU allows a device to access that page. You can still
have problems if there is a bug within the page but it will go mostly
unnoticed as it will be memory corruption.
If you system page is smaller (4K) than IOMMU page (64K), then the
device gets wider access than it should but it is still going to be
silent memory corruption.
>
>>
>>> wants mapped, and the other bytes get used for something else, or just
>>> mapped on another DMA page.
>>> It seems to work fine.
>>
>>
>> With 4K system page and 64K IOMMU page? In practice it would take an
>> effort or/and bad luck to see it crashing. Thanks,
>
> I haven't tested it yet. On a 64k system page and 4k/64k iommu page, it
> works as described above.
>
> I am new to this, so I am trying to understand how a memory page mapped
> as DMA, and used for something else could be a problem.
From the device prospective, there is PCI space and everything from 0
till 1<<64 is accessible and what is that mapped to - the device does
not know. PHB's IOMMU is the thing to notice invalid access and raise
EEH but PHB only knows about PCI->physical memory mapping (with IOMMU
pages) but nothing about the host kernel pages. Does this help? Thanks,
>
> Thanks!
>
>>
>>>>
>>>>>> Bigger pages are not the case here as I understand it.
>>>>>
>>>>> I did not get this part, what do you mean?
>>>>
>>>> Possible IOMMU page sizes are 4K, 64K, 2M, 16M, 256M, 1GB, and the
>>>> supported set of sizes is different for P8/P9 and type of IO (PHB,
>>>> NVLink/CAPI).
>>>>
>>>>
>>>>>>> Update those functions to guarantee alignment with requested size
>>>>>>> using IOMMU_PAGE_ALIGN() before doing iommu_alloc() / iommu_free().
>>>>>>>
>>>>>>> Also, on iommu_range_alloc(), replace ALIGN(n, 1 << tbl->it_page_shift)
>>>>>>> with IOMMU_PAGE_ALIGN(n, tbl), which seems easier to read.
>>>>>>>
>>>>>>> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
>>>>>>> ---
>>>>>>> arch/powerpc/kernel/iommu.c | 17 +++++++++--------
>>>>>>> 1 file changed, 9 insertions(+), 8 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
>>>>>>> index 9704f3f76e63..d7086087830f 100644
>>>>>>> --- a/arch/powerpc/kernel/iommu.c
>>>>>>> +++ b/arch/powerpc/kernel/iommu.c
>>>>>>> @@ -237,10 +237,9 @@ static unsigned long iommu_range_alloc(struct device *dev,
>>>>>>> }
>>>>>>>
>>>>>>> if (dev)
>>>>>>> - boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
>>>>>>> - 1 << tbl->it_page_shift);
>>>>>>> + boundary_size = IOMMU_PAGE_ALIGN(dma_get_seg_boundary(dev) + 1, tbl);
>>>>>>
>>>>>> Run checkpatch.pl, should complain about a long line.
>>>>>
>>>>> It's 86 columns long, which is less than the new limit of 100 columns
>>>>> Linus announced a few weeks ago. checkpatch.pl was updated too:
>>>>> https://www.phoronix.com/scan.php?page=news_item&px=Linux-Kernel-Deprecates-80-Col
>>>>
>>>> Yay finally :) Thanks,
>>>
>>> :)
>>>
>>>>
>>>>>>> else
>>>>>>> - boundary_size = ALIGN(1UL << 32, 1 << tbl->it_page_shift);
>>>>>>> + boundary_size = IOMMU_PAGE_ALIGN(1UL << 32, tbl);
>>>>>>> /* 4GB boundary for iseries_hv_alloc and iseries_hv_map */
>>>>>>>
>>>>>>> n = iommu_area_alloc(tbl->it_map, limit, start, npages, tbl->it_offset,
>>>>>>> @@ -858,6 +857,7 @@ void *iommu_alloc_coherent(struct device *dev, struct iommu_table *tbl,
>>>>>>> unsigned int order;
>>>>>>> unsigned int nio_pages, io_order;
>>>>>>> struct page *page;
>>>>>>> + size_t size_io = size;
>>>>>>>
>>>>>>> size = PAGE_ALIGN(size);
>>>>>>> order = get_order(size);
>>>>>>> @@ -884,8 +884,9 @@ void *iommu_alloc_coherent(struct device *dev, struct iommu_table *tbl,
>>>>>>> memset(ret, 0, size);
>>>>>>>
>>>>>>> /* Set up tces to cover the allocated range */
>>>>>>> - nio_pages = size >> tbl->it_page_shift;
>>>>>>> - io_order = get_iommu_order(size, tbl);
>>>>>>> + size_io = IOMMU_PAGE_ALIGN(size_io, tbl);
>>>>>>> + nio_pages = size_io >> tbl->it_page_shift;
>>>>>>> + io_order = get_iommu_order(size_io, tbl);
>>>>>>> mapping = iommu_alloc(dev, tbl, ret, nio_pages, DMA_BIDIRECTIONAL,
>>>>>>> mask >> tbl->it_page_shift, io_order, 0);
>>>>>>> if (mapping == DMA_MAPPING_ERROR) {
>>>>>>> @@ -900,11 +901,11 @@ void iommu_free_coherent(struct iommu_table *tbl, size_t size,
>>>>>>> void *vaddr, dma_addr_t dma_handle)
>>>>>>> {
>>>>>>> if (tbl) {
>>>>>>> - unsigned int nio_pages;
>>>>>>> + size_t size_io = IOMMU_PAGE_ALIGN(size, tbl);
>>>>>>> + unsigned int nio_pages = size_io >> tbl->it_page_shift;
>>>>>>>
>>>>>>> - size = PAGE_ALIGN(size);
>>>>>>> - nio_pages = size >> tbl->it_page_shift;
>>>>>>> iommu_free(tbl, dma_handle, nio_pages);
>>>>>>> +
>>>>>>
>>>>>> Unrelated new line.
>>>>>
>>>>> Will be removed. Thanks!
>>>>>
>>>>>>> size = PAGE_ALIGN(size);
>>>>>>> free_pages((unsigned long)vaddr, get_order(size));
>>>>>>> }
>>>>>>>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH v1 01/10] powerpc/pseries/iommu: Replace hard-coded page shift
From: Alexey Kardashevskiy @ 2020-09-03 4:26 UTC (permalink / raw)
To: Leonardo Bras, Oliver O'Halloran
Cc: Christophe Leroy, David Dai, Ram Pai, Linux Kernel Mailing List,
Murilo Fossa Vicentini, Paul Mackerras, Joel Stanley, Brian King,
linuxppc-dev, Thiago Jung Bauermann
In-Reply-To: <c381d7e60d0924e432b0f36dce9a44b89733a129.camel@gmail.com>
On 02/09/2020 07:38, Leonardo Bras wrote:
> On Mon, 2020-08-31 at 13:48 +1000, Alexey Kardashevskiy wrote:
>>>>> Well, I created this TCE_RPN_BITS = 52 because the previous mask was a
>>>>> hardcoded 40-bit mask (0xfffffffffful), for hard-coded 12-bit (4k)
>>>>> pagesize, and on PAPR+/LoPAR also defines TCE as having bits 0-51
>>>>> described as RPN, as described before.
>>>>>
>>>>> IODA3 Revision 3.0_prd1 (OpenPowerFoundation), Figure 3.4 and 3.5.
>>>>> shows system memory mapping into a TCE, and the TCE also has bits 0-51
>>>>> for the RPN (52 bits). "Table 3.6. TCE Definition" also shows it.
>>>>> In fact, by the looks of those figures, the RPN_MASK should always be a
>>>>> 52-bit mask, and RPN = (page >> tceshift) & RPN_MASK.
>>>>
>>>> I suspect the mask is there in the first place for extra protection
>>>> against too big addresses going to the TCE table (or/and for virtial vs
>>>> physical addresses). Using 52bit mask makes no sense for anything, you
>>>> could just drop the mask and let c compiler deal with 64bit "uint" as it
>>>> is basically a 4K page address anywhere in the 64bit space. Thanks,
>>>
>>> Assuming 4K pages you need 52 RPN bits to cover the whole 64bit
>>> physical address space. The IODA3 spec does explicitly say the upper
>>> bits are optional and the implementation only needs to support enough
>>> to cover up to the physical address limit, which is 56bits of P9 /
>>> PHB4. If you want to validate that the address will fit inside of
>>> MAX_PHYSMEM_BITS then fine, but I think that should be done as a
>>> WARN_ON or similar rather than just silently masking off the bits.
>>
>> We can do this and probably should anyway but I am also pretty sure we
>> can just ditch the mask and have the hypervisor return an error which
>> will show up in dmesg.
>
> Ok then, ditching the mask.
Well, you could run a little experiment and set some bits above that old
mask and see how phyp reacts :)
> Thanks!
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 1/2] dma-mapping: introduce dma_get_seg_boundary_nr_pages()
From: Michael Ellerman @ 2020-09-03 3:47 UTC (permalink / raw)
To: Nicolin Chen, hch
Cc: x86, linux-ia64, James.Bottomley, paulus, hpa, sparclinux, sfr,
deller, schnelle, borntraeger, mingo, mattst88, fenghua.yu, gor,
linux-s390, hca, ink, tglx, gerald.schaefer, rth, tony.luck,
linux-parisc, linux-kernel, linux-alpha, bp, linuxppc-dev, davem
In-Reply-To: <20200901221646.26491-2-nicoleotsuka@gmail.com>
Nicolin Chen <nicoleotsuka@gmail.com> writes:
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index 9704f3f76e63..cbc2e62db597 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -236,15 +236,10 @@ static unsigned long iommu_range_alloc(struct device *dev,
> }
> }
>
> - if (dev)
> - boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
> - 1 << tbl->it_page_shift);
> - else
> - boundary_size = ALIGN(1UL << 32, 1 << tbl->it_page_shift);
> - /* 4GB boundary for iseries_hv_alloc and iseries_hv_map */
> + boundary_size = dma_get_seg_boundary_nr_pages(dev, tbl->it_page_shift);
>
> n = iommu_area_alloc(tbl->it_map, limit, start, npages, tbl->it_offset,
> - boundary_size >> tbl->it_page_shift, align_mask);
> + boundary_size, align_mask);
This has changed the units of boundary_size, but it's unused elsewhere
in the function so that's OK.
If you need to do a v2 for any other reason, then I'd just drop
boundary_size and call dma_get_seg_boundary_nr_pages() directly in the
function call.
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_sai: Set SAI Channel Mode to Output Mode
From: Nicolin Chen @ 2020-09-03 3:40 UTC (permalink / raw)
To: Shengjiu Wang
Cc: alsa-devel, timur, Xiubo.Lee, lgirdwood, linuxppc-dev, tiwai,
perex, broonie, festevam, linux-kernel
In-Reply-To: <1599102555-17178-1-git-send-email-shengjiu.wang@nxp.com>
On Thu, Sep 03, 2020 at 11:09:15AM +0800, Shengjiu Wang wrote:
> Transmit data pins will output zero when slots are masked or channels
> are disabled. In CHMOD TDM mode, transmit data pins are tri-stated when
> slots are masked or channels are disabled. When data pins are tri-stated,
> there is noise on some channels when FS clock value is high and data is
> read while fsclk is transitioning from high to low.
>
> Signed-off-by: Cosmin-Gabriel Samoila <cosmin.samoila@nxp.com>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
Though one nit inline:
> ---
> sound/soc/fsl/fsl_sai.c | 12 ++++++++++--
> sound/soc/fsl/fsl_sai.h | 2 ++
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index 62c5fdb678fc..33b194a5c1dc 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -486,6 +486,12 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
>
> val_cr4 |= FSL_SAI_CR4_FRSZ(slots);
>
> + /* Output Mode - data pins transmit 0 when slots are masked
> + * or channels are disabled
> + */
Coding style for multi-line comments. Yet, probably can simplify?
/* Set to output mode to avoid tri-stated data pins */
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_sai: Support multiple data channel enable bits
From: Nicolin Chen @ 2020-09-03 3:30 UTC (permalink / raw)
To: Shengjiu Wang
Cc: alsa-devel, timur, Xiubo.Lee, lgirdwood, linuxppc-dev, tiwai,
perex, broonie, festevam, linux-kernel
In-Reply-To: <1598958068-10552-1-git-send-email-shengjiu.wang@nxp.com>
On Tue, Sep 01, 2020 at 07:01:08PM +0800, Shengjiu Wang wrote:
> One data channel is one data line. From imx7ulp, the SAI IP is
> enhanced to support multiple data channels.
>
> If there is only two channels input and slots is 2, then enable one
> data channel is enough for data transfer. So enable the TCE/RCE and
> transmit/receive mask register according to the input channels and
> slots configuration.
>
> Move the data channel enablement from startup() to hw_params().
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
^ permalink raw reply
* Re: [PATCH 0/2] dma-mapping: update default segment_boundary_mask
From: Nicolin Chen @ 2020-09-03 3:26 UTC (permalink / raw)
To: Niklas Schnelle
Cc: linux-ia64, James.Bottomley, paulus, hpa, sparclinux, hch, sfr,
deller, x86, borntraeger, mingo, mattst88, fenghua.yu, gor,
linux-s390, hca, ink, tglx, gerald.schaefer, rth, tony.luck,
linux-parisc, linux-kernel, linux-alpha, bp, linuxppc-dev, davem
In-Reply-To: <2c8db0aa-e8b5-e577-b971-1de10ecc6747@linux.ibm.com>
On Wed, Sep 02, 2020 at 10:13:12AM +0200, Niklas Schnelle wrote:
> On 9/2/20 12:16 AM, Nicolin Chen wrote:
> > These two patches are to update default segment_boundary_mask.
> >
> > PATCH-1 fixes overflow issues in callers of dma_get_seg_boundary.
> > Previous version was a series: https://lkml.org/lkml/2020/8/31/1026
> >
> > Then PATCH-2 sets default segment_boundary_mask to ULONG_MAX.
> >
> > Nicolin Chen (2):
> > dma-mapping: introduce dma_get_seg_boundary_nr_pages()
> > dma-mapping: set default segment_boundary_mask to ULONG_MAX
>
> I gave both of your patches a quick test ride on a couple of dev mainframes,
> both NVMe, ConnectX and virtio-pci devices all seems to work fine.
> I already commented on Christoph's mail that I like the helper approach,
> so as for s390 you can add my
>
> Acked-by: Niklas Schnelle <schnelle@linux.ibm.com>
Thanks for testing and the ack!
^ permalink raw reply
* [PATCH] ASoC: fsl_sai: Set SAI Channel Mode to Output Mode
From: Shengjiu Wang @ 2020-09-03 3:09 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel, lgirdwood
Cc: linuxppc-dev, linux-kernel
Transmit data pins will output zero when slots are masked or channels
are disabled. In CHMOD TDM mode, transmit data pins are tri-stated when
slots are masked or channels are disabled. When data pins are tri-stated,
there is noise on some channels when FS clock value is high and data is
read while fsclk is transitioning from high to low.
Signed-off-by: Cosmin-Gabriel Samoila <cosmin.samoila@nxp.com>
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_sai.c | 12 ++++++++++--
sound/soc/fsl/fsl_sai.h | 2 ++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 62c5fdb678fc..33b194a5c1dc 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -486,6 +486,12 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
val_cr4 |= FSL_SAI_CR4_FRSZ(slots);
+ /* Output Mode - data pins transmit 0 when slots are masked
+ * or channels are disabled
+ */
+ if (tx)
+ val_cr4 |= FSL_SAI_CR4_CHMOD;
+
/*
* For SAI master mode, when Tx(Rx) sync with Rx(Tx) clock, Rx(Tx) will
* generate bclk and frame clock for Tx(Rx), we should set RCR4(TCR4),
@@ -494,7 +500,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
if (!sai->is_slave_mode && fsl_sai_dir_is_synced(sai, adir)) {
regmap_update_bits(sai->regmap, FSL_SAI_xCR4(!tx, ofs),
- FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
+ FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK |
+ FSL_SAI_CR4_CHMOD_MASK,
val_cr4);
regmap_update_bits(sai->regmap, FSL_SAI_xCR5(!tx, ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
@@ -502,7 +509,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
}
regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
- FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
+ FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK |
+ FSL_SAI_CR4_CHMOD_MASK,
val_cr4);
regmap_update_bits(sai->regmap, FSL_SAI_xCR5(tx, ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 6aba7d28f5f3..19cd4e1bbff9 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -119,6 +119,8 @@
#define FSL_SAI_CR4_FRSZ_MASK (0x1f << 16)
#define FSL_SAI_CR4_SYWD(x) (((x) - 1) << 8)
#define FSL_SAI_CR4_SYWD_MASK (0x1f << 8)
+#define FSL_SAI_CR4_CHMOD BIT(5)
+#define FSL_SAI_CR4_CHMOD_MASK BIT(5)
#define FSL_SAI_CR4_MF BIT(4)
#define FSL_SAI_CR4_FSE BIT(3)
#define FSL_SAI_CR4_FSP BIT(1)
--
2.27.0
^ permalink raw reply related
* Re: [PATCH v2] scsi: ibmvfc: interface updates for future FPIN and MQ support
From: Martin K. Petersen @ 2020-09-03 2:11 UTC (permalink / raw)
To: Tyrel Datwyler
Cc: martin.petersen, linux-scsi, linux-kernel, james.bottomley,
brking, linuxppc-dev
In-Reply-To: <20200901002420.648532-1-tyreld@linux.ibm.com>
Tyrel,
> Fixup complier errors from neglected commit --amend
Bunch of formatting-related checkpatch warnings. Please fix.
Thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH] powerpc/64s: handle ISA v3.1 local copy-paste context switches
From: Paul Mackerras @ 2020-09-03 1:19 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <20200825075535.224536-1-npiggin@gmail.com>
On Tue, Aug 25, 2020 at 05:55:35PM +1000, Nicholas Piggin wrote:
> The ISA v3.1 the copy-paste facility has a new memory move functionality
> which allows the copy buffer to be pasted to domestic memory (RAM) as
> opposed to foreign memory (accelerator).
>
> This means the POWER9 trick of avoiding the cp_abort on context switch if
> the process had not mapped foreign memory does not work on POWER10. Do the
> cp_abort unconditionally there.
>
> KVM must also cp_abort on guest exit to prevent copy buffer state leaking
> between contexts.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
For the KVM part:
Acked-by: Paul Mackerras <paulus@ozlabs.org>
^ permalink raw reply
* Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: Linus Torvalds @ 2020-09-02 18:02 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, Kees Cook, the arch/x86 maintainers,
Linux Kernel Mailing List, Al Viro, linux-fsdevel, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <d78cb4be-48a9-a7c5-d9d1-d04d2a02b4c6@csgroup.eu>
On Wed, Sep 2, 2020 at 8:17 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
> With this fix, I get
>
> root@vgoippro:~# time dd if=/dev/zero of=/dev/null count=1M
> 536870912 bytes (512.0MB) copied, 6.776327 seconds, 75.6MB/s
>
> That's still far from the 91.7MB/s I get with 5.9-rc2, but better than
> the 65.8MB/s I got yesterday with your series. Still some way to go thought.
I don't see why this change would make any difference.
And btw, why do the 32-bit and 64-bit checks even differ? It's not
like the extra (single) instruction should even matter. I think the
main reason is that the simpler 64-bit case could stay as a macro
(because it only uses "addr" and "size" once), but honestly, that
"simplification" doesn't help when you then need to have that #ifdef
for the 32-bit case and an inline function anyway.
So why isn't it just
static inline int __access_ok(unsigned long addr, unsigned long size)
{ return addr <= TASK_SIZE_MAX && size <= TASK_SIZE_MAX-addr; }
for both and be done with it?
The "size=0" check is only relevant for the "addr == TASK_SIZE_MAX"
case, and existed in the old code because it had that "-1" thing
becasue "seg.seg" was actually TASK_SIZE-1.
Now that we don't have any TASK_SIZE-1, zero isn't special any more.
However, I suspect a bigger reason for the actual performance
degradation would be the patch that makes things use "write_iter()"
for writing, even when a simpler "write()" exists.
For writing to /dev/null, the cost of setting up iterators and all the
pointless indirection is all kinds of stupid.
So I think "write()" should just go back to default to using
"->write()" rather than "->write_iter()" if the simpler case exists.
Linus
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker
From: Christophe Leroy @ 2020-09-02 18:02 UTC (permalink / raw)
To: Nick Desaulniers, Michael Ellerman
Cc: Christophe Leroy, Joe Lawrence, Kees Cook, Fangrui Song, LKML,
Nicholas Piggin, clang-built-linux, Paul Mackerras, linuxppc-dev
In-Reply-To: <CAKwvOd=ZeJU+vLUk2P7FpX35haj7AC50B9Yps4pyoGCpd7ueTw@mail.gmail.com>
Le 02/09/2020 à 19:41, Nick Desaulniers a écrit :
> On Wed, Sep 2, 2020 at 5:14 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> Nick Desaulniers <ndesaulniers@google.com> writes:
>>> Fixes: commit f2af201002a8 ("powerpc/build: vdso linker warning for orphan sections")
>>
>> I think I'll just revert that for v5.9 ?
>
> SGTM; you'll probably still want these changes with some modifications
> at some point; vdso32 did have at least one orphaned section, and will
> be important for hermetic builds. Seeing crashes in supported
> versions of the tools ties our hands at the moment.
>
Keeping the tool problem aside with binutils 2.26, do you have a way to
really link an elf32ppc object when building vdso32 for PPC64 ?
Christophe
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker
From: Nick Desaulniers @ 2020-09-02 17:41 UTC (permalink / raw)
To: Michael Ellerman
Cc: Christophe Leroy, Joe Lawrence, Kees Cook, Fangrui Song, LKML,
Nicholas Piggin, clang-built-linux, Paul Mackerras, linuxppc-dev
In-Reply-To: <87blio1ilu.fsf@mpe.ellerman.id.au>
On Wed, Sep 2, 2020 at 5:14 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Nick Desaulniers <ndesaulniers@google.com> writes:
> > Fixes: commit f2af201002a8 ("powerpc/build: vdso linker warning for orphan sections")
>
> I think I'll just revert that for v5.9 ?
SGTM; you'll probably still want these changes with some modifications
at some point; vdso32 did have at least one orphaned section, and will
be important for hermetic builds. Seeing crashes in supported
versions of the tools ties our hands at the moment.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/vdso32: link vdso64 with linker
From: Segher Boessenkool @ 2020-09-02 16:57 UTC (permalink / raw)
To: Christophe Leroy
Cc: Christophe Leroy, Joe Lawrence, Kees Cook, Fangrui Song,
Nick Desaulniers, linux-kernel, Nicholas Piggin,
clang-built-linux, Paul Mackerras, linuxppc-dev
In-Reply-To: <0c895acf-b6d7-baaf-d613-236f8be8e1fe@csgroup.eu>
On Wed, Sep 02, 2020 at 05:43:03PM +0200, Christophe Leroy wrote:
> >Try with a newer ld? If it still happens with current versions, please
> >open a bug report? https://sourceware.org/bugzilla
>
> Yes it works with 2.30 and 2.34
Ah okay, I missed this part.
> But minimum for building kernel is supposed to be 2.23
Sure. Tthat could be upgraded to 2.24 -- you should use a binutils at
least as new as your GCC, and that requires 4.9 now -- but that
probably doesn't help you here).
Segher
^ permalink raw reply
* Re: ptrace_syscall_32 is failing
From: Andy Lutomirski @ 2020-09-02 16:49 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-s390, linuxppc-dev, Vasily Gorbik, Brian Gerst,
Heiko Carstens, X86 ML, LKML, Christian Borntraeger,
Paul Mackerras, Catalin Marinas, Andy Lutomirski, Will Deacon,
linux-arm-kernel
In-Reply-To: <87blioinub.fsf@nanos.tec.linutronix.de>
On Wed, Sep 2, 2020 at 1:29 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Tue, Sep 01 2020 at 17:09, Andy Lutomirski wrote:
> > On Tue, Sep 1, 2020 at 4:50 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> >> > I think that they almost work for x86, but not quite as
> >> > indicated by this bug. Even if we imagine we can somehow hack around
> >> > this bug, I imagine we're going to find other problems with this
> >> > model, e.g. the potential upcoming exit problem I noted in my review.
> >>
> >> What's the upcoming problem?
> >
> > If we ever want to get single-stepping fully correct across syscalls,
> > we might need to inject SIGTRAP on syscall return. This would be more
> > awkward if we can't run instrumentable code after the syscall part of
> > the syscall is done.
>
> We run a lot of instrumentable code after sys_foo() returns. Otherwise
> all the TIF work would not be possible at all.
>
> But you might tell me where exactly you want to inject the SIGTRAP in
> the syscall exit code flow.
It would be a bit complicated. Definitely after any signals from the
syscall are delivered. Right now, I think that we don't deliver a
SIGTRAP on the instruction boundary after SYSCALL while
single-stepping. (I think we used to, but only sometimes, and now we
are at least consistent.) This is because IRET will not trap if it
starts with TF clear and ends up setting it. (I asked Intel to
document this, and I think they finally did, although I haven't gotten
around to reading the new docs. Certainly the old docs as of a year
or two ago had no description whatsoever of how TF changes worked.)
Deciding exactly *when* a trap should occur would be nontrivial -- we
can't trap on sigreturn() from a SIGTRAP, for example.
So this isn't fully worked out.
>
> >> I don't think we want that in general. The current variant is perfectly
> >> fine for everything except the 32bit fast syscall nonsense. Also
> >> irqentry_entry/exit is not equivalent to the syscall_enter/exit
> >> counterparts.
> >
> > If there are any architectures in which actual work is needed to
> > figure out whether something is a syscall in the first place, they'll
> > want to do the usual kernel entry work before the syscall entry work.
>
> That's low level entry code which does not require RCU, lockdep, tracing
> or whatever muck we setup before actual work can be done.
>
> arch_asm_entry()
> ...
> arch_c_entry(cause) {
> switch(cause) {
> case EXCEPTION: arch_c_exception(...);
> case SYSCALL: arch_c_syscall(...);
> ...
> }
You're assuming that figuring out the cause doesn't need the kernel
entry code to run first. In the case of the 32-bit vDSO fast
syscalls, we arguably don't know whether an entry is a syscall until
we have done a user memory access. Logically, we're doing:
if (get_user() < 0) {
/* Not a syscall. This is actually a silly operation that sets AX =
-EFAULT and returns. Do not audit or invoke ptrace. */
} else {
/* This actually is a syscall. */
}
So we really do want to stick arch code between the
enter_from_user_mode() and the audit check. We *can't* audit because
we don't know the syscall args. Now maybe we could invent new
semantics for this in which a fault here is still somehow a syscall,
but I think that would be a real ABI change and would want very
careful thought. And it would be weird -- syscalls are supposed to
actually call the syscall handler, aren't they? (Arguably we should
go back in time and make this a SIGSEGV. We have the infrastructure
to do this cleanly, but when I wrote the code I just copied the ABI
from code that was before my time. Even so, it would be an exception,
not a syscall.)
>
> You really want to differentiate between exception and syscall
> entry/exit.
>
Why do we want to distinguish between exception and syscall
entry/exit? For the enter part, AFAICS the exception case boils down
to enter_from_user_mode() and the syscall case is:
enter_from_user_mode(regs);
instrumentation_begin();
local_irq_enable();
ti_work = READ_ONCE(current_thread_info()->flags);
if (ti_work & SYSCALL_ENTER_WORK)
syscall = syscall_trace_enter(regs, syscall, ti_work);
instrumentation_end();
Which would decompose quite nicely as a regular (non-syscall) entry
plus the syscall part later.
> The splitting of syscall_enter_from_user_mode() is only necessary for
> that 32bit fast syscall thing on x86 and there is no point to open code
> it with two calls for e.g. do_syscall_64().
>
> > Maybe your patch actually makes this possible -- I haven't digested
> > all the details yet.
> >
> > Who advised you to drop the arch parameter?
>
> Kees, IIRC, but I would have to search through the gazillions of mail
> threads to be sure.
>
> >> + syscall_enter_from_user_mode_prepare(regs);
> >
> > I'm getting lost in all these "enter" functions...
>
> It's not that hard.
>
> syscall_enter_from_user_mode_prepare()
> + syscall_enter_from_user_mode_work()
> = syscall_enter_from_user_mode()
>
> That's exactly what you suggested just with the difference that it is
> explicit for syscalls and not using irqentry_enter/exit().
>
> If we would do that then instead of having a single call for sane
> syscall pathes:
>
> arch_c_entry()
> nr = syscall_enter_from_user_mode();
>
> or for that 32bit fast syscall nonsense the split variant:
>
> arch_c_entry()
> syscall_enter_from_user_mode_prepare();
> do_fast_syscall_muck();
> nr = syscall_enter_from_user_mode_work();
>
> we'd have:
>
> arch_c_entry()
> irqentry_enter();
> local_irq_enble();
> nr = syscall_enter_from_user_mode_work();
> ...
>
> which enforces two calls for sane entries and more code in arch/....
This is why I still like my:
arch_c_entry()
irqentry_enter_from_user_mode();
generic_syscall();
exit...
}
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/vdso32: link vdso64 with linker
From: Christophe Leroy @ 2020-09-02 15:43 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Christophe Leroy, Joe Lawrence, Kees Cook, Fangrui Song,
Nick Desaulniers, linux-kernel, Nicholas Piggin,
clang-built-linux, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200902141431.GV28786@gate.crashing.org>
Hi,
Le 02/09/2020 à 16:14, Segher Boessenkool a écrit :
> Hi!
>
> On Wed, Sep 02, 2020 at 06:46:45AM +0000, Christophe Leroy wrote:
>> ld crashes:
>>
>> LD arch/powerpc/kernel/vdso32/vdso32.so.dbg
>> /bin/sh: line 1: 23780 Segmentation fault (core dumped)
>> ppc-linux-ld -EB -m elf32ppc -shared -soname linux-vdso32.so.1
>> --eh-frame-hdr --orphan-handling=warn -T
>> arch/powerpc/kernel/vdso32/vdso32.lds
>> arch/powerpc/kernel/vdso32/sigtramp.o
>> arch/powerpc/kernel/vdso32/gettimeofday.o
>> arch/powerpc/kernel/vdso32/datapage.o
>> arch/powerpc/kernel/vdso32/cacheflush.o
>> arch/powerpc/kernel/vdso32/note.o arch/powerpc/kernel/vdso32/getcpu.o -o
>> arch/powerpc/kernel/vdso32/vdso32.so.dbg
>> make[4]: *** [arch/powerpc/kernel/vdso32/vdso32.so.dbg] Error 139
>>
>>
>> [root@localhost linux-powerpc]# ppc-linux-ld --version
>> GNU ld (GNU Binutils) 2.26.20160125
>
> [ Don't build as root :-P ]
>
> Try with a newer ld? If it still happens with current versions, please
> open a bug report? https://sourceware.org/bugzilla
Yes it works with 2.30 and 2.34
But minimum for building kernel is supposed to be 2.23
Christophe
^ 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