Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg

2024-05-08 Thread Nicholas Piggin
On Wed May 8, 2024 at 11:36 PM AEST, Thomas Huth wrote:
> On 08/05/2024 14.58, Thomas Huth wrote:
> > On 08/05/2024 14.55, Thomas Huth wrote:
> >> On 08/05/2024 14.27, Nicholas Piggin wrote:
> >>> On Wed May 8, 2024 at 1:08 AM AEST, Thomas Huth wrote:
>  On 04/05/2024 14.28, Nicholas Piggin wrote:
> > This allows different machines with different requirements to be
> > supported by run_tests.sh, similarly to how different accelerators
> > are handled.
> >
> > Acked-by: Thomas Huth 
> > Acked-by: Andrew Jones 
> > Signed-off-by: Nicholas Piggin 
> > ---
> >    docs/unittests.txt   |  7 +++
> >    scripts/common.bash  |  8 ++--
> >    scripts/runtime.bash | 16 
> >    3 files changed, 25 insertions(+), 6 deletions(-)
> >
> > diff --git a/docs/unittests.txt b/docs/unittests.txt
> > index 7cf2c55ad..6449efd78 100644
> > --- a/docs/unittests.txt
> > +++ b/docs/unittests.txt
> > @@ -42,6 +42,13 @@ For / directories that support multiple 
> > architectures, this restricts
> >    the test to the specified arch. By default, the test will run on any
> >    architecture.
> > +machine
> > +---
> > +For those architectures that support multiple machine types, this 
> > restricts
> > +the test to the specified machine. By default, the test will run on
> > +any machine type. (Note, the machine can be specified with the MACHINE=
> > +environment variable, and defaults to the architecture's default.)
> > +
> >    smp
> >    ---
> >    smp = 
> > diff --git a/scripts/common.bash b/scripts/common.bash
> > index 5e9ad53e2..3aa557c8c 100644
> > --- a/scripts/common.bash
> > +++ b/scripts/common.bash
> > @@ -10,6 +10,7 @@ function for_each_unittest()
> >    local opts
> >    local groups
> >    local arch
> > +    local machine
> >    local check
> >    local accel
> >    local timeout
> > @@ -21,7 +22,7 @@ function for_each_unittest()
> >    if [[ "$line" =~ ^\[(.*)\]$ ]]; then
> >    rematch=${BASH_REMATCH[1]}
> >    if [ -n "${testname}" ]; then
> > -    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
> > "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> > +    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
> > "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> >    fi
> >    testname=$rematch
> >    smp=1
> > @@ -29,6 +30,7 @@ function for_each_unittest()
> >    opts=""
> >    groups=""
> >    arch=""
> > +    machine=""
> >    check=""
> >    accel=""
> >    timeout=""
> > @@ -58,6 +60,8 @@ function for_each_unittest()
> >    groups=${BASH_REMATCH[1]}
> >    elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
> >    arch=${BASH_REMATCH[1]}
> > +    elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
> > +    machine=${BASH_REMATCH[1]}
> >    elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
> >    check=${BASH_REMATCH[1]}
> >    elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
> > @@ -67,7 +71,7 @@ function for_each_unittest()
> >    fi
> >    done
> >    if [ -n "${testname}" ]; then
> > -    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
> > "$opts" "$arch" "$check" "$accel" "$timeout"
> > +    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
> > "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> >    fi
> >    exec {fd}<&-
> >    }
> > diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> > index 177b62166..0c96d6ea2 100644
> > --- a/scripts/runtime.bash
> > +++ b/scripts/runtime.bash
> > @@ -32,7 +32,7 @@ premature_failure()
> >    get_cmdline()
> >    {
> >    local kernel=$1
> > -    echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel 
> > $RUNTIME_arch_run $kernel -smp $smp $opts"
> > +    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine 
> > ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
> >    }
> >    skip_nodefault()
> > @@ -80,9 +80,10 @@ function run()
> >    local kernel="$4"
> >    local opts="$5"
> >    local arch="$6"
> > -    local check="${CHECK:-$7}"
> > -    local accel="$8"
> > -    local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the 
> > default
> > +    local machine="$7"
> > +    local check="${CHECK:-$8}"
> > +    local accel="$9"
> > +    local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the 
> > default
> >    if [ "${CONFIG_EFI}" == "y" 

Re: [PATCH v6] arch/powerpc/kvm: Add support for reading VPA counters for pseries guests

2024-05-08 Thread Nicholas Piggin
On Wed May 8, 2024 at 10:36 PM AEST, Michael Ellerman wrote:
> Gautam Menghani  writes:
> > PAPR hypervisor has introduced three new counters in the VPA area of
> > LPAR CPUs for KVM L2 guest (see [1] for terminology) observability - 2
> > for context switches from host to guest and vice versa, and 1 counter
> > for getting the total time spent inside the KVM guest. Add a tracepoint
> > that enables reading the counters for use by ftrace/perf. Note that this
> > tracepoint is only available for nestedv2 API (i.e, KVM on PowerVM).
> ...
> > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> > index 8e86eb577eb8..ed69ad58bd02 100644
> > --- a/arch/powerpc/kvm/book3s_hv.c
> > +++ b/arch/powerpc/kvm/book3s_hv.c
> > @@ -4108,6 +4108,54 @@ static void vcpu_vpa_increment_dispatch(struct 
> > kvm_vcpu *vcpu)
> > }
> >  }
> >  
> > +static inline int kvmhv_get_l2_counters_status(void)
> > +{
> > +   return get_lppaca()->l2_counters_enable;
> > +}
>
> This is breaking the powernv build:

[...]

All the nested KVM code should really go under CONFIG_PSERIES.
Possibly even moved out to its own file.

For now maybe you could just ifdef these few functions and
replace with noop variants for !PSERIES.

Thanks,
Nick


Re: [FSL P50x0] Kernel 6.9-rc1 compiling issue

2024-05-08 Thread Christian Zigotzky
Christophe Leroy  writes:
Hi Christian, hi Hari,

Le 04/04/2024 à 19:44, Christian Zigotzky a écrit :
Shall we use CONFIG_CRASH_DUMP to get int crashing_cpu = -1;?

Further information:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2024-March/269985.html


Looking at problematic commit 5c4233cc0920 ("powerpc/kdump: Split
KEXEC_CORE and CRASH_DUMP dependency"), my feeling is that the change
should be as follows.

Hari, can you confirm ?

diff --git a/arch/powerpc/platforms/85xx/smp.c
b/arch/powerpc/platforms/85xx/smp.c
index 40aa58206888..3209fc92ac19 100644
--- a/arch/powerpc/platforms/85xx/smp.c
+++ b/arch/powerpc/platforms/85xx/smp.c
@@ -362,7 +362,7 @@ struct smp_ops_t smp_85xx_ops = {
 #endif
 };

-#ifdef CONFIG_KEXEC_CORE
+#ifdef CONFIG_CRASH_DUMP
 #ifdef CONFIG_PPC32
 atomic_t kexec_down_cpus = ATOMIC_INIT(0);

@@ -465,7 +465,7 @@ static void mpc85xx_smp_machine_kexec(struct kimage
*image)

 default_machine_kexec(image);
 }
-#endif /* CONFIG_KEXEC_CORE */
+#endif /* CONFIG_CRASH_DUMP */

- - - -

On 8. Apr 2024, at 14:20, Michael Ellerman  wrote:

That doesn't look right to me.

I think it needs something like below.

cheers

diff --git a/arch/powerpc/platforms/85xx/smp.c 
b/arch/powerpc/platforms/85xx/smp.c
index 40aa58206888..276060c993a0 100644
--- a/arch/powerpc/platforms/85xx/smp.c
+++ b/arch/powerpc/platforms/85xx/smp.c
@@ -398,6 +398,7 @@ static void mpc85xx_smp_kexec_cpu_down(int crash_shutdown, 
int secondary)
   hard_irq_disable();
   mpic_teardown_this_cpu(secondary);

+#ifdef CONFIG_CRASH_DUMP
   if (cpu == crashing_cpu && cpu_thread_in_core(cpu) != 0) {
   /*
* We enter the crash kernel on whatever cpu crashed,
@@ -406,9 +407,11 @@ static void mpc85xx_smp_kexec_cpu_down(int crash_shutdown, 
int secondary)
*/
   disable_threadbit = 1;
   disable_cpu = cpu_first_thread_sibling(cpu);
-} else if (sibling != crashing_cpu &&
-   cpu_thread_in_core(cpu) == 0 &&
-   cpu_thread_in_core(sibling) != 0) {
+} else if (sibling == crashing_cpu)
+return;
+#endif
+if (cpu_thread_in_core(cpu) == 0 &&
+cpu_thread_in_core(sibling) != 0) {
   disable_threadbit = 2;
   disable_cpu = sibling;
   }

- - - -

Any news? I still need a patch for compiling the kernel.

- - Christian


[PATCH 4/4] ASoC: fsl_xcvr: Add support for i.MX95 platform

2024-05-08 Thread Shengjiu Wang
On i.MX95, the XCVR uses a new PLL in the PHY, which is
General Purpose (GP) PLL. Add GP PLL configuration support
in the driver and add the 'pll_ver' flag to distinguish
different PLL on different platforms.

The XCVR also use PHY but limited for SPDIF only case
Add 'use_phy' flag to distinguish these platforms.

Signed-off-by: Shengjiu Wang 
Reviewed-by: Chancel Liu 
---
 sound/soc/fsl/fsl_xcvr.c | 120 +--
 sound/soc/fsl/fsl_xcvr.h |  91 +
 2 files changed, 168 insertions(+), 43 deletions(-)

diff --git a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c
index 0ffa10e924ef..6b1715ac67c5 100644
--- a/sound/soc/fsl/fsl_xcvr.c
+++ b/sound/soc/fsl/fsl_xcvr.c
@@ -20,10 +20,17 @@
 
 #define FSL_XCVR_CAPDS_SIZE256
 
+enum fsl_xcvr_pll_verison {
+   PLL_MX8MP,
+   PLL_MX95,
+};
+
 struct fsl_xcvr_soc_data {
const char *fw_name;
bool spdif_only;
bool use_edma;
+   bool use_phy;
+   enum fsl_xcvr_pll_verison pll_ver;
 };
 
 struct fsl_xcvr {
@@ -265,10 +272,10 @@ static int fsl_xcvr_ai_write(struct fsl_xcvr *xcvr, u8 
reg, u32 data, bool phy)
 static int fsl_xcvr_en_phy_pll(struct fsl_xcvr *xcvr, u32 freq, bool tx)
 {
struct device *dev = >pdev->dev;
-   u32 i, div = 0, log2;
+   u32 i, div = 0, log2, val;
int ret;
 
-   if (xcvr->soc_data->spdif_only)
+   if (!xcvr->soc_data->use_phy)
return 0;
 
for (i = 0; i < ARRAY_SIZE(fsl_xcvr_pll_cfg); i++) {
@@ -291,45 +298,62 @@ static int fsl_xcvr_en_phy_pll(struct fsl_xcvr *xcvr, u32 
freq, bool tx)
return ret;
}
 
-   /* PLL: BANDGAP_SET: EN_VBG (enable bandgap) */
-   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_BANDGAP_SET,
- FSL_XCVR_PLL_BANDGAP_EN_VBG, 0);
-
-   /* PLL: CTRL0: DIV_INTEGER */
-   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_CTRL0, fsl_xcvr_pll_cfg[i].mfi, 0);
-   /* PLL: NUMERATOR: MFN */
-   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_NUM, fsl_xcvr_pll_cfg[i].mfn, 0);
-   /* PLL: DENOMINATOR: MFD */
-   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_DEN, fsl_xcvr_pll_cfg[i].mfd, 0);
-   /* PLL: CTRL0_SET: HOLD_RING_OFF, POWER_UP */
-   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_CTRL0_SET,
- FSL_XCVR_PLL_CTRL0_HROFF | FSL_XCVR_PLL_CTRL0_PWP, 0);
-   udelay(25);
-   /* PLL: CTRL0: Clear Hold Ring Off */
-   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_CTRL0_CLR,
- FSL_XCVR_PLL_CTRL0_HROFF, 0);
-   udelay(100);
-   if (tx) { /* TX is enabled for SPDIF only */
-   /* PLL: POSTDIV: PDIV0 */
-   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_PDIV,
- FSL_XCVR_PLL_PDIVx(log2, 0), 0);
-   /* PLL: CTRL_SET: CLKMUX0_EN */
-   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_CTRL0_SET,
- FSL_XCVR_PLL_CTRL0_CM0_EN, 0);
-   } else if (xcvr->mode == FSL_XCVR_MODE_EARC) { /* eARC RX */
-   /* PLL: POSTDIV: PDIV1 */
-   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_PDIV,
- FSL_XCVR_PLL_PDIVx(log2, 1), 0);
-   /* PLL: CTRL_SET: CLKMUX1_EN */
-   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_CTRL0_SET,
- FSL_XCVR_PLL_CTRL0_CM1_EN, 0);
-   } else { /* SPDIF / ARC RX */
-   /* PLL: POSTDIV: PDIV2 */
-   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_PDIV,
- FSL_XCVR_PLL_PDIVx(log2, 2), 0);
-   /* PLL: CTRL_SET: CLKMUX2_EN */
+   switch (xcvr->soc_data->pll_ver) {
+   case PLL_MX8MP:
+   /* PLL: BANDGAP_SET: EN_VBG (enable bandgap) */
+   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_BANDGAP_SET,
+ FSL_XCVR_PLL_BANDGAP_EN_VBG, 0);
+
+   /* PLL: CTRL0: DIV_INTEGER */
+   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_CTRL0, 
fsl_xcvr_pll_cfg[i].mfi, 0);
+   /* PLL: NUMERATOR: MFN */
+   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_NUM, 
fsl_xcvr_pll_cfg[i].mfn, 0);
+   /* PLL: DENOMINATOR: MFD */
+   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_DEN, 
fsl_xcvr_pll_cfg[i].mfd, 0);
+   /* PLL: CTRL0_SET: HOLD_RING_OFF, POWER_UP */
fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_CTRL0_SET,
- FSL_XCVR_PLL_CTRL0_CM2_EN, 0);
+ FSL_XCVR_PLL_CTRL0_HROFF | 
FSL_XCVR_PLL_CTRL0_PWP, 0);
+   udelay(25);
+   /* PLL: CTRL0: Clear Hold Ring Off */
+   fsl_xcvr_ai_write(xcvr, FSL_XCVR_PLL_CTRL0_CLR,
+ FSL_XCVR_PLL_CTRL0_HROFF, 0);
+   udelay(100);
+   if (tx) { /* TX is enabled for SPDIF only */
+   /* PLL: POSTDIV: PDIV0 */
+   fsl_xcvr_ai_write(xcvr, 

[PATCH 3/4] ASoC: fsl_xcvr: Support reparent pll clocks for phy_clk

2024-05-08 Thread Shengjiu Wang
When there are 'pll8k' and 'pll11k' clock existing, the clock
source of 'phy_clk' can be changed for different sample rate
requirement.

Signed-off-by: Shengjiu Wang 
---
 sound/soc/fsl/fsl_xcvr.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c
index c46f64557a7f..0ffa10e924ef 100644
--- a/sound/soc/fsl/fsl_xcvr.c
+++ b/sound/soc/fsl/fsl_xcvr.c
@@ -15,6 +15,7 @@
 #include 
 
 #include "fsl_xcvr.h"
+#include "fsl_utils.h"
 #include "imx-pcm.h"
 
 #define FSL_XCVR_CAPDS_SIZE256
@@ -33,6 +34,8 @@ struct fsl_xcvr {
struct clk *pll_ipg_clk;
struct clk *phy_clk;
struct clk *spba_clk;
+   struct clk *pll8k_clk;
+   struct clk *pll11k_clk;
struct reset_control *reset;
u8 streams;
u32 mode;
@@ -362,6 +365,8 @@ static int fsl_xcvr_en_aud_pll(struct fsl_xcvr *xcvr, u32 
freq)
 
freq = xcvr->soc_data->spdif_only ? freq / 5 : freq;
clk_disable_unprepare(xcvr->phy_clk);
+   fsl_asoc_reparent_pll_clocks(dev, xcvr->phy_clk,
+xcvr->pll8k_clk, xcvr->pll11k_clk, freq);
ret = clk_set_rate(xcvr->phy_clk, freq);
if (ret < 0) {
dev_err(dev, "Error while setting AUD PLL rate: %d\n", ret);
@@ -1287,6 +1292,9 @@ static int fsl_xcvr_probe(struct platform_device *pdev)
return PTR_ERR(xcvr->pll_ipg_clk);
}
 
+   fsl_asoc_get_pll_clocks(dev, >pll8k_clk,
+   >pll11k_clk);
+
xcvr->ram_addr = devm_platform_ioremap_resource_byname(pdev, "ram");
if (IS_ERR(xcvr->ram_addr))
return PTR_ERR(xcvr->ram_addr);
-- 
2.34.1



[PATCH 0/4] ASoC: fsl_xcvr: Support i.MX95 platform

2024-05-08 Thread Shengjiu Wang
On i.MX95 wakeup domain, there is one instance of Audio XCVR
supporting SPDIF mode with a connection to the Audio XCVR physical
interface.

Shengjiu Wang (4):
  ASoC: dt-bindings: fsl,xcvr: Add compatible string for i.MX95
  ASoC: dt-bindings: fsl,xcvr: Add two PLL clock sources
  ASoC: fsl_xcvr: Support reparent pll clocks for phy_clk
  ASoC: fsl_xcvr: Add support for i.MX95 platform

 .../devicetree/bindings/sound/fsl,xcvr.yaml   |   7 +
 sound/soc/fsl/fsl_xcvr.c  | 128 --
 sound/soc/fsl/fsl_xcvr.h  |  91 +
 3 files changed, 183 insertions(+), 43 deletions(-)

-- 
2.34.1



[PATCH 2/4] ASoC: dt-bindings: fsl,xcvr: Add two PLL clock sources

2024-05-08 Thread Shengjiu Wang
Add two PLL clock sources, they are the parent clocks of the root clock
one is for 8kHz series rates, named as 'pll8k', another one is for
11kHz series rates, named as 'pll11k'. They are optional clocks,
if there are such clocks, then the driver can switch between them to
support more accurate sample rates.

As 'pll8k' and 'pll11k' are optional, then add 'minItems: 4' for
clocks and clock-names properties.

Signed-off-by: Shengjiu Wang 
---
 Documentation/devicetree/bindings/sound/fsl,xcvr.yaml | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/fsl,xcvr.yaml 
b/Documentation/devicetree/bindings/sound/fsl,xcvr.yaml
index 1c74a32def09..c4660faed404 100644
--- a/Documentation/devicetree/bindings/sound/fsl,xcvr.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,xcvr.yaml
@@ -50,6 +50,9 @@ properties:
   - description: PHY clock
   - description: SPBA clock
   - description: PLL clock
+  - description: PLL clock source for 8kHz series
+  - description: PLL clock source for 11kHz series
+minItems: 4
 
   clock-names:
 items:
@@ -57,6 +60,9 @@ properties:
   - const: phy
   - const: spba
   - const: pll_ipg
+  - const: pll8k
+  - const: pll11k
+minItems: 4
 
   dmas:
 items:
-- 
2.34.1



[PATCH 1/4] ASoC: dt-bindings: fsl,xcvr: Add compatible string for i.MX95

2024-05-08 Thread Shengjiu Wang
Add compatible string "fsl,imx95-xcvr" for i.MX95 platform.

Signed-off-by: Shengjiu Wang 
---
 Documentation/devicetree/bindings/sound/fsl,xcvr.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/sound/fsl,xcvr.yaml 
b/Documentation/devicetree/bindings/sound/fsl,xcvr.yaml
index 0eb0c1ba8710..1c74a32def09 100644
--- a/Documentation/devicetree/bindings/sound/fsl,xcvr.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,xcvr.yaml
@@ -22,6 +22,7 @@ properties:
 enum:
   - fsl,imx8mp-xcvr
   - fsl,imx93-xcvr
+  - fsl,imx95-xcvr
 
   reg:
 items:
-- 
2.34.1



Re: [PATCH v3 00/11] sysctl: treewide: constify ctl_table argument of sysctl handlers

2024-05-08 Thread Jakub Kicinski
On Wed, 8 May 2024 10:11:35 -0700 Kees Cook wrote:
> > Split this per subsystem, please.  
> 
> I've done a few painful API transitions before, and I don't think the
> complexity of these changes needs a per-subsystem constification pass. I
> think this series is the right approach, but that patch 11 will need
> coordination with Linus. We regularly do system-wide prototype changes
> like this right at the end of the merge window before -rc1 comes out.

Right. I didn't read the code closely enough before responding.
Chalk my response up to being annoyed by the constant stream of
cross-tree changes in procfs without proper cover letter explaining 
how they will be merged :|


Re: [PATCH 1/1] [RFC] ethernet: Convert from tasklet to BH workqueue

2024-05-08 Thread Simon Horman
On Tue, May 07, 2024 at 12:27:10PM -0700, Allen wrote:
> On Tue, May 7, 2024 at 12:23 PM Russell King (Oracle)
>  wrote:
> >
> > On Tue, May 07, 2024 at 07:01:11PM +, Allen Pais wrote:
> > > The only generic interface to execute asynchronously in the BH context is
> > > tasklet; however, it's marked deprecated and has some design flaws. To
> > > replace tasklets, BH workqueue support was recently added. A BH workqueue
> > > behaves similarly to regular workqueues except that the queued work items
> > > are executed in the BH context.
> > >
> > > This patch converts drivers/ethernet/* from tasklet to BH workqueue.
> >
> > I doubt you're going to get many comments on this patch, being so large
> > and spread across all drivers. I'm not going to bother trying to edit
> > this down to something more sensible, I'll just plonk my comment here.
> >
> > For the mvpp2 driver, you're only updating a comment - and looking at
> > it, the comment no longer reflects the code. It doesn't make use of
> > tasklets at all. That makes the comment wrong whether or not it's
> > updated. So I suggest rather than doing a search and replace for
> > "tasklet" to "BH blahblah" (sorry, I don't remember what you replaced
> > it with) just get rid of that bit of the comment.
> >
> 
>  Thank you Russell.
> 
>  I will get rid of the comment. If it helps, I can create a patch for each
> driver. We did that in the past, with this series, I thought it would be
> easier to apply one patch.

Hi Allen and Russell,

My 2c worth:

* In general non bug-fix patches for networking code should be targeted at
  net-next. This means that they should include net-next in the subject,
  and be based on that tree.

  Subject: [PATCH net-next] ...

* This series does not appear to apply to net-next

* This series appears to depend on code which is not present in net-next.
  f.e. disable_work_sync

* The Infiniband patches should probably be submitted separately
  to the relevant maintainers

* As this patch seems to involve many non-trivial changes
  it seems to me that it would be best to break it up somehow.
  To allow proper review.

* Patch-sets for net-next should be limited to 15 patches,
  so perhaps multiple sequential batches would be a way forwards.

Link: https://docs.kernel.org/process/maintainer-netdev.html


Re: [PATCH v3 00/11] sysctl: treewide: constify ctl_table argument of sysctl handlers

2024-05-08 Thread Kees Cook
On Wed, Apr 24, 2024 at 08:12:34PM -0700, Jakub Kicinski wrote:
> On Tue, 23 Apr 2024 09:54:35 +0200 Thomas Weißschuh wrote:
> > The series was split from my larger series sysctl-const series [0].
> > It only focusses on the proc_handlers but is an important step to be
> > able to move all static definitions of ctl_table into .rodata.
> 
> Split this per subsystem, please.

I've done a few painful API transitions before, and I don't think the
complexity of these changes needs a per-subsystem constification pass. I
think this series is the right approach, but that patch 11 will need
coordination with Linus. We regularly do system-wide prototype changes
like this right at the end of the merge window before -rc1 comes out.

The requirements are pretty simple: it needs to be a obvious changes
(this certainly is) and as close to 100% mechanical as possible. I think
patch 11 easily qualifies. Linus should be able to run the same Coccinelle
script and get nearly the same results, etc. And all the other changes
need to have landed. This change also has no "silent failure" conditions:
anything mismatched will immediately stand out.

So, have patches 1-10 go via their respective subsystems, and once all
of those are in Linus's tree, send patch 11 as a stand-alone PR.

(From patch 11, it looks like the seccomp read/write function changes
could be split out? I'll do that now...)

-Kees

-- 
Kees Cook


Re: [PATCH bpf v2] powerpc/bpf: enforce full ordering for ATOMIC operations with BPF_FETCH

2024-05-08 Thread Naveen N Rao
On Wed, May 08, 2024 at 11:54:04AM GMT, Puranjay Mohan wrote:
> The Linux Kernel Memory Model [1][2] requires RMW operations that have a
> return value to be fully ordered.
> 
> BPF atomic operations with BPF_FETCH (including BPF_XCHG and
> BPF_CMPXCHG) return a value back so they need to be JITed to fully
> ordered operations. POWERPC currently emits relaxed operations for
> these.
> 
> We can show this by running the following litmus-test:
> 
> PPC SB+atomic_add+fetch
> 
> {
> 0:r0=x;  (* dst reg assuming offset is 0 *)
> 0:r1=2;  (* src reg *)
> 0:r2=1;
> 0:r4=y;  (* P0 writes to this, P1 reads this *)
> 0:r5=z;  (* P1 writes to this, P0 reads this *)
> 0:r6=0;
> 
> 1:r2=1;
> 1:r4=y;
> 1:r5=z;
> }
> 
> P0  | P1;
> stw r2, 0(r4)   | stw  r2,0(r5) ;
> |   ;
> loop:lwarx  r3, r6, r0  |   ;
> mr  r8, r3  |   ;
> add r3, r3, r1  | sync  ;
> stwcx.  r3, r6, r0  |   ;
> bne loop|   ;
> mr  r1, r8  |   ;
> |   ;
> lwa r7, 0(r5)   | lwa  r7,0(r4) ;
> 
> ~exists(0:r7=0 /\ 1:r7=0)
> 
> Witnesses
> Positive: 9 Negative: 3
> Condition ~exists (0:r7=0 /\ 1:r7=0)
> Observation SB+atomic_add+fetch Sometimes 3 9
> 
> This test shows that the older store in P0 is reordered with a newer
> load to a different address. Although there is a RMW operation with
> fetch between them. Adding a sync before and after RMW fixes the issue:
> 
> Witnesses
> Positive: 9 Negative: 0
> Condition ~exists (0:r7=0 /\ 1:r7=0)
> Observation SB+atomic_add+fetch Never 0 9
> 
> [1] https://www.kernel.org/doc/Documentation/memory-barriers.txt
> [2] https://www.kernel.org/doc/Documentation/atomic_t.txt
> 
> Fixes: 65112709115f ("powerpc/bpf/64: add support for BPF_ATOMIC bitwise 
> operations")
> Signed-off-by: Puranjay Mohan 

Thanks for reporting and fixing this.

There are actually four commits that this fixes across ppc32/ppc64:
Fixes: aea7ef8a82c0 ("powerpc/bpf/32: add support for BPF_ATOMIC bitwise 
operations")
Fixes: 2d9206b22743 ("powerpc/bpf/32: Add instructions for atomic_[cmp]xchg")
Fixes: dbe6e2456fb0 ("powerpc/bpf/64: add support for atomic fetch operations")
Fixes: 1e82dfaa7819 ("powerpc/bpf/64: Add instructions for atomic_[cmp]xchg")

> ---
> Changes in v1 -> v2:
> v1: https://lore.kernel.org/all/20240507175439.119467-1-puran...@kernel.org/
> - Don't emit `sync` for non-SMP kernels as that adds unessential overhead.
> ---
>  arch/powerpc/net/bpf_jit_comp32.c | 12 
>  arch/powerpc/net/bpf_jit_comp64.c | 12 
>  2 files changed, 24 insertions(+)
> 
> diff --git a/arch/powerpc/net/bpf_jit_comp32.c 
> b/arch/powerpc/net/bpf_jit_comp32.c
> index 2f39c50ca729..0318b83f2e6a 100644
> --- a/arch/powerpc/net/bpf_jit_comp32.c
> +++ b/arch/powerpc/net/bpf_jit_comp32.c
> @@ -853,6 +853,15 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, 
> u32 *fimage, struct code
>   /* Get offset into TMP_REG */
>   EMIT(PPC_RAW_LI(tmp_reg, off));
>   tmp_idx = ctx->idx * 4;
> + /*
> +  * Enforce full ordering for operations with BPF_FETCH 
> by emitting a 'sync'
> +  * before and after the operation.
> +  *
> +  * This is a requirement in the Linux Kernel Memory 
> Model.
> +  * See __cmpxchg_u64() in asm/cmpxchg.h as an example.
 ^^^
Nit...   u32

> +  */
> + if (imm & BPF_FETCH && IS_ENABLED(CONFIG_SMP))
> + EMIT(PPC_RAW_SYNC());

I think this block should go before the previous two instructions. We 
use tmp_idx as a label to retry the ll/sc sequence, so we will end up 
executing the 'sync' operation on a retry here.

>   /* load value from memory into r0 */
>   EMIT(PPC_RAW_LWARX(_R0, tmp_reg, dst_reg, 0));
>  
> @@ -905,6 +914,9 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, 
> u32 *fimage, struct code
>  
>   /* For the BPF_FETCH variant, get old data into src_reg 
> */
>   if (imm & BPF_FETCH) {
> + /* Emit 'sync' to enforce full ordering */
> + if (IS_ENABLED(CONFIG_SMP))
> + EMIT(PPC_RAW_SYNC());
>   EMIT(PPC_RAW_MR(ret_reg, ax_reg));
>   if (!fp->aux->verifier_zext)
>   EMIT(PPC_RAW_LI(ret_reg - 1, 0)); /* 
> higher 32-bit */
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c 
> b/arch/powerpc/net/bpf_jit_comp64.c
> index 79f23974a320..9a077f8acf7b 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> 

[powerpc:topic/ppc-kvm] BUILD SUCCESS b52e8cd3f835869370f8540f1bc804a47a47f02b

2024-05-08 Thread kernel test robot
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git 
topic/ppc-kvm
branch HEAD: b52e8cd3f835869370f8540f1bc804a47a47f02b  KVM: PPC: Book3S HV 
nestedv2: Fix an error handling path in 
gs_msg_ops_kvmhv_nestedv2_config_fill_info()

elapsed time: 1474m

configs tested: 157
configs skipped: 9

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha allnoconfig   gcc  
alphaallyesconfig   gcc  
alpha   defconfig   gcc  
arc  allmodconfig   gcc  
arc   allnoconfig   gcc  
arc  allyesconfig   gcc  
arc defconfig   gcc  
arc   randconfig-001-20240508   gcc  
arc   randconfig-002-20240508   gcc  
arm  allmodconfig   gcc  
arm   allnoconfig   clang
arm  allyesconfig   gcc  
arm at91_dt_defconfig   clang
arm defconfig   clang
arm   milbeaut_m10v_defconfig   clang
arm  pxa168_defconfig   clang
arm   randconfig-001-20240508   gcc  
arm s3c6400_defconfig   gcc  
arm64allmodconfig   clang
arm64 allnoconfig   gcc  
arm64   defconfig   gcc  
arm64 randconfig-001-20240508   gcc  
arm64 randconfig-003-20240508   gcc  
csky allmodconfig   gcc  
csky  allnoconfig   gcc  
csky allyesconfig   gcc  
cskydefconfig   gcc  
csky  randconfig-001-20240508   gcc  
csky  randconfig-002-20240508   gcc  
hexagon  allmodconfig   clang
hexagon   allnoconfig   clang
hexagon  allyesconfig   clang
hexagon defconfig   clang
i386 allmodconfig   gcc  
i386  allnoconfig   gcc  
i386 allyesconfig   gcc  
i386 buildonly-randconfig-001-20240508   clang
i386 buildonly-randconfig-002-20240508   clang
i386 buildonly-randconfig-005-20240508   clang
i386defconfig   clang
i386  randconfig-002-20240508   clang
i386  randconfig-003-20240508   clang
i386  randconfig-005-20240508   clang
i386  randconfig-013-20240508   clang
i386  randconfig-014-20240508   clang
i386  randconfig-015-20240508   clang
i386  randconfig-016-20240508   clang
loongarchallmodconfig   gcc  
loongarch allnoconfig   gcc  
loongarchallyesconfig   gcc  
loongarch   defconfig   gcc  
loongarch randconfig-001-20240508   gcc  
loongarch randconfig-002-20240508   gcc  
m68k allmodconfig   gcc  
m68k  allnoconfig   gcc  
m68k allyesconfig   gcc  
m68kdefconfig   gcc  
microblaze   allmodconfig   gcc  
microblazeallnoconfig   gcc  
microblaze   allyesconfig   gcc  
microblaze  defconfig   gcc  
mips allmodconfig   gcc  
mips  allnoconfig   gcc  
mips allyesconfig   gcc  
mips decstation_defconfig   gcc  
mips   ip27_defconfig   gcc  
mips  loongson3_defconfig   gcc  
mips   mtx1_defconfig   clang
mips   sb1250_swarm_defconfig   gcc  
nios2allmodconfig   gcc  
nios2 allnoconfig   gcc  
nios2allyesconfig   gcc  
nios2   defconfig   gcc  
nios2 randconfig-001-20240508   gcc  
nios2 randconfig-002-20240508   gcc  
openrisc allmodconfig   gcc  
openrisc  allnoconfig   gcc  
openrisc allyesconfig   gcc  
openriscdefconfig   gcc  
parisc   allmodconfig   gcc  
pariscallnoconfig   gcc  
parisc   allyesconfig   gcc  
parisc  defconfig   gcc  
pariscrandconfig-001-20240508   gcc  
parisc

Re: [PATCH v2 1/2] powerpc/io: Avoid clang null pointer arithmetic warnings

2024-05-08 Thread Naresh Kamboju
On Fri, 3 May 2024 at 13:26, Michael Ellerman  wrote:
>
> With -Wextra clang warns about pointer arithmetic using a null pointer.
> When building with CONFIG_PCI=n, that triggers a warning in the IO
> accessors, eg:
>
>   In file included from linux/arch/powerpc/include/asm/io.h:672:
>   linux/arch/powerpc/include/asm/io-defs.h:23:1: warning: performing pointer 
> arithmetic on a null pointer has undefined behavior 
> [-Wnull-pointer-arithmetic]
>  23 | DEF_PCI_AC_RET(inb, u8, (unsigned long port), (port), pio, port)
> | ^~~~
>   ...
>   linux/arch/powerpc/include/asm/io.h:591:53: note: expanded from macro 
> '__do_inb'
> 591 | #define __do_inb(port)  readb((PCI_IO_ADDR)_IO_BASE + port);
> |   ~ ^
>
> That is because when CONFIG_PCI=n, _IO_BASE is defined as 0.
>
> Although _IO_BASE is defined as plain 0, the cast (PCI_IO_ADDR) converts
> it to void * before the addition with port happens.
>
> Instead the addition can be done first, and then the cast. The resulting
> value will be the same, but avoids the warning, and also avoids void
> pointer arithmetic which is apparently non-standard.
>
> Reported-by: Naresh Kamboju 
> Closes: 
> https://lore.kernel.org/all/CA+G9fYtEh8zmq8k8wE-8RZwW-Qr927RLTn+KqGnq1F=ptaa...@mail.gmail.com
> Signed-off-by: Michael Ellerman 

Tested-by: Linux Kernel Functional Testing 

--
Linaro LKFT
https://lkft.linaro.org


Re: [PATCH bpf v2] powerpc/bpf: enforce full ordering for ATOMIC operations with BPF_FETCH

2024-05-08 Thread Paul E. McKenney
On Wed, May 08, 2024 at 11:54:04AM +, Puranjay Mohan wrote:
> The Linux Kernel Memory Model [1][2] requires RMW operations that have a
> return value to be fully ordered.
> 
> BPF atomic operations with BPF_FETCH (including BPF_XCHG and
> BPF_CMPXCHG) return a value back so they need to be JITed to fully
> ordered operations. POWERPC currently emits relaxed operations for
> these.
> 
> We can show this by running the following litmus-test:
> 
> PPC SB+atomic_add+fetch
> 
> {
> 0:r0=x;  (* dst reg assuming offset is 0 *)
> 0:r1=2;  (* src reg *)
> 0:r2=1;
> 0:r4=y;  (* P0 writes to this, P1 reads this *)
> 0:r5=z;  (* P1 writes to this, P0 reads this *)
> 0:r6=0;
> 
> 1:r2=1;
> 1:r4=y;
> 1:r5=z;
> }
> 
> P0  | P1;
> stw r2, 0(r4)   | stw  r2,0(r5) ;
> |   ;
> loop:lwarx  r3, r6, r0  |   ;
> mr  r8, r3  |   ;
> add r3, r3, r1  | sync  ;
> stwcx.  r3, r6, r0  |   ;
> bne loop|   ;
> mr  r1, r8  |   ;
> |   ;
> lwa r7, 0(r5)   | lwa  r7,0(r4) ;
> 
> ~exists(0:r7=0 /\ 1:r7=0)
> 
> Witnesses
> Positive: 9 Negative: 3
> Condition ~exists (0:r7=0 /\ 1:r7=0)
> Observation SB+atomic_add+fetch Sometimes 3 9
> 
> This test shows that the older store in P0 is reordered with a newer
> load to a different address. Although there is a RMW operation with
> fetch between them. Adding a sync before and after RMW fixes the issue:
> 
> Witnesses
> Positive: 9 Negative: 0
> Condition ~exists (0:r7=0 /\ 1:r7=0)
> Observation SB+atomic_add+fetch Never 0 9
> 
> [1] https://www.kernel.org/doc/Documentation/memory-barriers.txt
> [2] https://www.kernel.org/doc/Documentation/atomic_t.txt
> 
> Fixes: 65112709115f ("powerpc/bpf/64: add support for BPF_ATOMIC bitwise 
> operations")
> Signed-off-by: Puranjay Mohan 

Acked-by: Paul E. McKenney 

> ---
> Changes in v1 -> v2:
> v1: https://lore.kernel.org/all/20240507175439.119467-1-puran...@kernel.org/
> - Don't emit `sync` for non-SMP kernels as that adds unessential overhead.
> ---
>  arch/powerpc/net/bpf_jit_comp32.c | 12 
>  arch/powerpc/net/bpf_jit_comp64.c | 12 
>  2 files changed, 24 insertions(+)
> 
> diff --git a/arch/powerpc/net/bpf_jit_comp32.c 
> b/arch/powerpc/net/bpf_jit_comp32.c
> index 2f39c50ca729..0318b83f2e6a 100644
> --- a/arch/powerpc/net/bpf_jit_comp32.c
> +++ b/arch/powerpc/net/bpf_jit_comp32.c
> @@ -853,6 +853,15 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, 
> u32 *fimage, struct code
>   /* Get offset into TMP_REG */
>   EMIT(PPC_RAW_LI(tmp_reg, off));
>   tmp_idx = ctx->idx * 4;
> + /*
> +  * Enforce full ordering for operations with BPF_FETCH 
> by emitting a 'sync'
> +  * before and after the operation.
> +  *
> +  * This is a requirement in the Linux Kernel Memory 
> Model.
> +  * See __cmpxchg_u64() in asm/cmpxchg.h as an example.
> +  */
> + if (imm & BPF_FETCH && IS_ENABLED(CONFIG_SMP))
> + EMIT(PPC_RAW_SYNC());
>   /* load value from memory into r0 */
>   EMIT(PPC_RAW_LWARX(_R0, tmp_reg, dst_reg, 0));
>  
> @@ -905,6 +914,9 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, 
> u32 *fimage, struct code
>  
>   /* For the BPF_FETCH variant, get old data into src_reg 
> */
>   if (imm & BPF_FETCH) {
> + /* Emit 'sync' to enforce full ordering */
> + if (IS_ENABLED(CONFIG_SMP))
> + EMIT(PPC_RAW_SYNC());
>   EMIT(PPC_RAW_MR(ret_reg, ax_reg));
>   if (!fp->aux->verifier_zext)
>   EMIT(PPC_RAW_LI(ret_reg - 1, 0)); /* 
> higher 32-bit */
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c 
> b/arch/powerpc/net/bpf_jit_comp64.c
> index 79f23974a320..9a077f8acf7b 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -804,6 +804,15 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, 
> u32 *fimage, struct code
>   /* Get offset into TMP_REG_1 */
>   EMIT(PPC_RAW_LI(tmp1_reg, off));
>   tmp_idx = ctx->idx * 4;
> + /*
> +  * Enforce full ordering for operations with BPF_FETCH 
> by emitting a 'sync'
> +  * before and after the operation.
> +  *
> +  * This is a requirement in the Linux Kernel Memory 
> Model.
> +  * See __cmpxchg_u64() in asm/cmpxchg.h as an 

Re: [PATCH 7/8] powerpc: Fix typos

2024-05-08 Thread Michael Ellerman
Bjorn Helgaas  writes:
> From: Bjorn Helgaas 
>
> Fix typos, most reported by "codespell arch/powerpc".  Only touches
> comments, no code changes.
>
> Signed-off-by: Bjorn Helgaas 
> Cc: Nicholas Piggin 
> Cc: Christophe Leroy 
> Cc: linuxppc-dev@lists.ozlabs.org

Applied to powerpc/next.

[1/1] powerpc: Fix typos
  https://git.kernel.org/powerpc/c/0ddbbb8960eaf91c7b432ec80566dfa60a8d79e4

cheers


Re: WARNING: CPU: 1 PID: 1 at net/core/netpoll.c:370 netpoll_send_skb+0x1fc/0x20c at boot when netconsole is enabled (kernel v6.9-rc5, v6.8.7, sungem, PowerMac G4 DP)

2024-05-08 Thread Jakub Kicinski
On Wed, 8 May 2024 10:55:05 +0200 Erhard Furtner wrote:
> I could do that with the explanation you stated. But should any
> further questions arise in this process I would also lack the
> technical background to deal with them. ;)

Alright, submitted :)

> I also noticed a similar #ifdef CONFIG_NET_POLL_CONTROLLER logic shows up in
> many network drivers, e.g. net/ethernet/realtek/8139too.c:
> 
> #ifdef CONFIG_NET_POLL_CONTROLLER
> static void rtl8139_poll_controller(struct net_device *dev);
> #endif
> [...]
> #ifdef CONFIG_NET_POLL_CONTROLLER
> /*
>  * Polling receive - used by netconsole and other diagnostic tools
>  * to allow network i/o with interrupts disabled.
>  */
> static void rtl8139_poll_controller(struct net_device *dev)
> {
> struct rtl8139_private *tp = netdev_priv(dev);
>   const int irq = tp->pci_dev->irq;
> 
>   disable_irq_nosync(irq);
>   rtl8139_interrupt(irq, dev);
>   enable_irq(irq);
> }
> #endif
> [...]
> #ifdef CONFIG_NET_POLL_CONTROLLER
>   .ndo_poll_controller= rtl8139_poll_controller,
> #endif
> 
> 
> Should it be removed here too? This would be more cards I can test.
> So far I only see this on my G4 and I think something similar on an
> old Pentium4 box I no longer have. 

That one looks legit. Note the _nosync() which solves the immediate
IRQ masking / deadlock issue. And the rtl8139_interrupt() function
actually does the packet cleanup in case of 8139too.


Re: [PATCH v2] KVM: PPC: Book3S HV nestedv2: Cancel pending DEC exception

2024-05-08 Thread Michael Ellerman
On Mon, 15 Apr 2024 09:27:29 +0530, Vaibhav Jain wrote:
> This reverts commit 180c6b072bf3 ("KVM: PPC: Book3S HV nestedv2: Do not
> cancel pending decrementer exception") [1] which prevented canceling a
> pending HDEC exception for nestedv2 KVM guests. It was done to avoid
> overhead of a H_GUEST_GET_STATE hcall to read the 'DEC expiry TB' register
> which was higher compared to handling extra decrementer exceptions.
> 
> However recent benchmarks indicate that overhead of not handling 'DECR'
> expiry for Nested KVM Guest(L2) is higher and results in much larger exits
> to Pseries Host(L1) as indicated by the Unixbench-arithoh bench[2]
> 
> [...]

Applied to powerpc/topic/ppc-kvm.

[1/1] KVM: PPC: Book3S HV nestedv2: Cancel pending DEC exception
  https://git.kernel.org/powerpc/c/7be6ce7043b4cf293c8826a48fd9f56931cef2cf

cheers


Re: [PATCH] KVM: PPC: Fix documentation for ppc mmu caps

2024-05-08 Thread Michael Ellerman
On Tue, 11 Apr 2023 15:44:46 +0930, Joel Stanley wrote:
> The documentation mentions KVM_CAP_PPC_RADIX_MMU, but the defines in the
> kvm headers spell it KVM_CAP_PPC_MMU_RADIX. Similarly with
> KVM_CAP_PPC_MMU_HASH_V3.
> 
> 

Applied to powerpc/topic/ppc-kvm.

[1/1] KVM: PPC: Fix documentation for ppc mmu caps
  https://git.kernel.org/powerpc/c/651d61bc8b7d8bb622cfc24be2ee92eebb4ed3cc

cheers


Re: [PATCH] KVM: PPC: Book3S HV nestedv2: Fix an error handling path in gs_msg_ops_kvmhv_nestedv2_config_fill_info()

2024-05-08 Thread Michael Ellerman
On Sun, 28 Jan 2024 12:34:25 +0100, Christophe JAILLET wrote:
> The return value of kvmppc_gse_put_buff_info() is not assigned to 'rc' and
> 'rc' is uninitialized at this point.
> So the error handling can not work.
> 
> Assign the expected value to 'rc' to fix the issue.
> 
> 
> [...]

Applied to powerpc/topic/ppc-kvm.

[1/1] KVM: PPC: Book3S HV nestedv2: Fix an error handling path in 
gs_msg_ops_kvmhv_nestedv2_config_fill_info()
  https://git.kernel.org/powerpc/c/b52e8cd3f835869370f8540f1bc804a47a47f02b

cheers


Re: [PATCH v2] KVM: PPC: code cleanup for kvmppc_book3s_irqprio_deliver

2024-05-08 Thread Michael Ellerman
On Thu, 25 Jan 2024 16:33:48 +0800, Kunwu Chan wrote:
> This part was commented from commit 2f4cf5e42d13 ("Add book3s.c")
> in about 14 years before.
> If there are no plans to enable this part code in the future,
> we can remove this dead code.
> 
> 

Applied to powerpc/topic/ppc-kvm.

[1/1] KVM: PPC: code cleanup for kvmppc_book3s_irqprio_deliver
  https://git.kernel.org/powerpc/c/a9c08bcd3179a59998d6339505d0010b82cbcb93

cheers


Re: [PATCH v2 1/2] powerpc/io: Avoid clang null pointer arithmetic warnings

2024-05-08 Thread Michael Ellerman
On Fri, 03 May 2024 17:56:18 +1000, Michael Ellerman wrote:
> With -Wextra clang warns about pointer arithmetic using a null pointer.
> When building with CONFIG_PCI=n, that triggers a warning in the IO
> accessors, eg:
> 
>   In file included from linux/arch/powerpc/include/asm/io.h:672:
>   linux/arch/powerpc/include/asm/io-defs.h:23:1: warning: performing pointer 
> arithmetic on a null pointer has undefined behavior 
> [-Wnull-pointer-arithmetic]
>  23 | DEF_PCI_AC_RET(inb, u8, (unsigned long port), (port), pio, port)
> | ^~~~
>   ...
>   linux/arch/powerpc/include/asm/io.h:591:53: note: expanded from macro 
> '__do_inb'
> 591 | #define __do_inb(port)  readb((PCI_IO_ADDR)_IO_BASE + port);
> |   ~ ^
> 
> [...]

Applied to powerpc/next.

[1/2] powerpc/io: Avoid clang null pointer arithmetic warnings
  https://git.kernel.org/powerpc/c/03c0f2c2b2220fc9cf8785cd7b61d3e71e24a366
[2/2] powerpc/64: Set _IO_BASE to POISON_POINTER_DELTA not 0 for CONFIG_PCI=n
  https://git.kernel.org/powerpc/c/be140f1732b523947425aaafbe2e37b41b622d96

cheers


Re: [PATCH] Fix the address of the linuxppc-dev mailing list

2024-05-08 Thread Michael Ellerman
On Fri, 03 May 2024 12:10:12 +1000, Stephen Rothwell wrote:
> This list was moved many years ago.
> 
> 

Applied to powerpc/next.

[1/1] Fix the address of the linuxppc-dev mailing list
  https://git.kernel.org/powerpc/c/fae573060c8da4d84a2551c6753d272abfda8ddc

cheers


Re: [PATCH -next 2/3 v2] powerpc: Fix kernel-doc comments in fsl_gtm.c

2024-05-08 Thread Michael Ellerman
On Mon, 08 Apr 2024 13:31:08 +0800, Yang Li wrote:
> Fix some function names in kernel-doc comments.
> 
> 

Applied to powerpc/next.

[2/3] powerpc: Fix kernel-doc comments in fsl_gtm.c
  https://git.kernel.org/powerpc/c/97bd2693b399cfd436acaa230d8f09e4c39e8e5c
[3/3] powerpc/rtas: Add kernel-doc comments to smp_startup_cpu()
  https://git.kernel.org/powerpc/c/554da5e0f71238384787954242d881cfeeff844d

cheers


Re: [PATCH -next 1/3 v2] powerpc: boot: Fix kernel-doc param for partial_decompress

2024-05-08 Thread Michael Ellerman
On Mon, 08 Apr 2024 16:39:16 +0800, Yang Li wrote:
> Fix the kernel-doc annotation for the 'skip' parameter in the
> partial_decompress() function by adding a missing underscore and colon.
> 
> 

Applied to powerpc/next.

[1/3] powerpc: boot: Fix kernel-doc param for partial_decompress
  https://git.kernel.org/powerpc/c/6efc2f1a64ef62f1e3893da90d6ac618988992c2

cheers


Re: [PATCH v2] powerpc/Makefile: Remove bits related to the previous use of -mcmodel=large

2024-05-08 Thread Michael Ellerman
On Wed, 10 Jan 2024 19:42:37 +0530, Naveen N Rao wrote:
> All supported compilers today (gcc v5.1+ and clang v11+) have support for
> -mcmodel=medium. As such, NO_MINIMAL_TOC is no longer being set. Remove
> NO_MINIMAL_TOC as well as the fallback to -mminimal-toc.
> 
> 

Applied to powerpc/next.

[1/1] powerpc/Makefile: Remove bits related to the previous use of 
-mcmodel=large
  https://git.kernel.org/powerpc/c/c330b50d8cae1a7b1fed7622eedacaf652396bb7

cheers


Re: [PATCH] macintosh/ams: Fix unused variable warning

2024-05-08 Thread Michael Ellerman
On Wed, 08 May 2024 00:01:50 +1000, Michael Ellerman wrote:
> If both CONFIG_SENSORS_AMS_PMU and CONFIG_SENSORS_AMS_I2C are unset,
> there is an unused variable warning in the ams driver:
> 
>   drivers/macintosh/ams/ams-core.c: In function 'ams_init':
>   drivers/macintosh/ams/ams-core.c:181:29: warning: unused variable 'np'
> 181 | struct device_node *np;
> 
> [...]

Applied to powerpc/next.

[1/1] macintosh/ams: Fix unused variable warning
  https://git.kernel.org/powerpc/c/bc8744c6bf0d487dcb7911d093fce60a62cc2654

cheers


Re: [PATCH] powerpc: rename SPRN_HID2 define to SPRN_HID2_750FX

2024-05-08 Thread Michael Ellerman
On Wed, 24 Jan 2024 11:50:31 +0100, Matthias Schiffer wrote:
> This register number is hardware-specific, rename it for clarity.
> 
> FIXME comments are added in a few places where it seems like the wrong
> register is used. As I can't test this, only the rename is done with no
> functional change.
> 
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc: rename SPRN_HID2 define to SPRN_HID2_750FX
  https://git.kernel.org/powerpc/c/ad679719d7020a200c4a10248ebb3bbb374d423d

cheers


Re: [PATCH] powerpc: remove unused *_syscall_64.o variables in Makefile

2024-05-08 Thread Michael Ellerman
On Fri, 16 Feb 2024 22:55:17 +0900, Masahiro Yamada wrote:
> Commit ab1a517d55b0 ("powerpc/syscall: Rename syscall_64.c into
> interrupt.c") missed to update these three lines:
> 
>   GCOV_PROFILE_syscall_64.o := n
>   KCOV_INSTRUMENT_syscall_64.o := n
>   UBSAN_SANITIZE_syscall_64.o := n
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc: remove unused *_syscall_64.o variables in Makefile
  https://git.kernel.org/powerpc/c/4f1dad618587fa2fa90323530c8c382b6f3e

cheers


Re: [PATCH] power: Remove arch specific module bug stuff

2024-05-08 Thread Michael Ellerman
On Fri, 03 May 2024 01:23:17 +0100, li...@treblig.org wrote:
> The last function to reference module_bug_list went in 2008's
>   commit b9754568ef17 ("powerpc: Remove dead module_find_bug code")
> but I don't think that was called since 2006's
>   commit 73c9ceab40b1 ("[POWERPC] Generic BUG for powerpc")
> 
> Now that the list has gone, I think we can also clean up the bug
> entries in mod_arch_specific.
> 
> [...]

Applied to powerpc/next.

[1/1] power: Remove arch specific module bug stuff
  https://git.kernel.org/powerpc/c/4071739249fd2e647e7058dbab0db4ddc0a0c427

cheers


Re: [PATCH v4 1/2] powerpc64/bpf: fix tail calls for PCREL addressing

2024-05-08 Thread Michael Ellerman
On Thu, 02 May 2024 23:02:04 +0530, Hari Bathini wrote:
> With PCREL addressing, there is no kernel TOC. So, it is not setup in
> prologue when PCREL addressing is used. But the number of instructions
> to skip on a tail call was not adjusted accordingly. That resulted in
> not so obvious failures while using tailcalls. 'tailcalls' selftest
> crashed the system with the below call trace:
> 
>   bpf_test_run+0xe8/0x3cc (unreliable)
>   bpf_prog_test_run_skb+0x348/0x778
>   __sys_bpf+0xb04/0x2b00
>   sys_bpf+0x28/0x38
>   system_call_exception+0x168/0x340
>   system_call_vectored_common+0x15c/0x2ec
> 
> [...]

Applied to powerpc/next.

[1/2] powerpc64/bpf: fix tail calls for PCREL addressing
  https://git.kernel.org/powerpc/c/2ecfe59cd7de1f202e9af2516a61fbbf93d0bd4d
[2/2] powerpc/bpf: enable kfunc call
  https://git.kernel.org/powerpc/c/61688a82e047a4166436bf2665716cc070572ffa

cheers


Re: [PATCH v2] powerpc: Fix preserved memory size for int-vectors

2024-05-08 Thread Michael Ellerman
On Sat, 13 Jan 2024 08:05:09 +, GUO Zihua wrote:
> The first 32k of memory is reserved for interrupt vectors, however for
> powerpc64 this might not be enough. Fix this by reserving the maximum
> size between 32k and the real size of interrupt vectors.
> 
> 

Applied to powerpc/next.

[1/1] powerpc: Fix preserved memory size for int-vectors
  https://git.kernel.org/powerpc/c/473e2311f31fdcae8e3f4410d119dbfece656edc

cheers


Re: [PATCH] powerpc/xmon: Check cpu id in commands "c#", "dp#" and "dx#"

2024-05-08 Thread Michael Ellerman
On Tue, 09 Mar 2021 19:11:10 +0100, Greg Kurz wrote:
> All these commands end up peeking into the PACA using the user originated
> cpu id as an index. Check the cpu id is valid in order to prevent xmon to
> crash. Instead of printing an error, this follows the same behavior as the
> "lp s #" command : ignore the buggy cpu id parameter and fall back to the
> #-less version of the command.
> 
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/xmon: Check cpu id in commands "c#", "dp#" and "dx#"
  https://git.kernel.org/powerpc/c/8873aab8646194a4446117bb617cc71bddda2dee

cheers


Re: [PATCH v2 0/3] Fix typos, grammatical errors and add units of function param

2024-05-08 Thread Michael Ellerman
On Thu, 28 Dec 2023 15:45:17 +0530, Ghanshyam Agrawal wrote:
> This patch series fixes spelling mistake in the word "auxillary",
> fixes a grammatical error related to full stop and adds the units
> of the size param in the description of eeh_set_pe_aux_size function.
> 
> Ghanshyam Agrawal (3):
>   powerpc/eeh: Fix spelling of the word "auxillary"
>   powerpc/eeh: Add full stop to fix a grammatical error
>   powerpc/eeh: Add the units of size param in the description
> 
> [...]

Applied to powerpc/next.

[1/3] powerpc/eeh: Fix spelling of the word "auxillary"
  https://git.kernel.org/powerpc/c/39434af10f1045b50826b8b506415f36681d4b40
[2/3] powerpc/eeh: Add full stop to fix a grammatical error
  (squashed)
[3/3] powerpc/eeh: Add the units of size param in the description
  (squashed)

cheers


Re: [PATCH 1/4] powerpc: dts: add power management nodes to FSL chips

2024-05-08 Thread Michael Ellerman
On Fri, 19 Jan 2024 15:38:54 -0500, Frank Li wrote:
> Enable Power Management feature on device tree, including MPC8536,
> MPC8544, MPC8548, MPC8572, P1010, P1020, P1021, P1022, P2020, P2041,
> P3041, T104X, T1024.
> 
> 

Applied to powerpc/next.

[1/4] powerpc: dts: add power management nodes to FSL chips
  https://git.kernel.org/powerpc/c/b12ba096b89084d1e2d6ebdb71b852eeebef95d3
[2/4] powerpc: dts: p1010rdb: fix INTx interrupt issue on P1010RDB-PB
  https://git.kernel.org/powerpc/c/9c8dc6f34351cd0c6a2ef83be2266f7dd67c152c
[3/4] powerpc: dts: mpc85xx: remove "simple-bus" compatible from ifc node
  https://git.kernel.org/powerpc/c/0bf51cc9e9e57a751b4c5dacbfa499ba5cd8bd72
[4/4] powerpc: dts: fsl: rename ifc node name to be memory-controller
  https://git.kernel.org/powerpc/c/acb354fe97e5aa6d9534b601ce18ef7866f25c4d

cheers


Re: [PATCH][next] selftests/powerpc/dexcr: Fix spelling mistake "predicition" -> "prediction"

2024-05-08 Thread Michael Ellerman
On Wed, 08 May 2024 09:41:17 +0100, Colin Ian King wrote:
> There is a spelling mistake in the help message. Fix it.
> 
> 

Applied to powerpc/next.

[1/1] selftests/powerpc/dexcr: Fix spelling mistake "predicition" -> 
"prediction"
  https://git.kernel.org/powerpc/c/98ec6d38ee57a734123c6f5d42640804034024ef

cheers


Re: [PATCH] powerpc/bpf/32: Fix failing test_bpf tests

2024-05-08 Thread Michael Ellerman
On Tue, 05 Mar 2024 16:36:23 +0100, Christophe Leroy wrote:
> Recent additions in BPF like cpu v4 instructions, test_bpf module
> exhibits the following failures:
> 
>   test_bpf: #82 ALU_MOVSX | BPF_B jited:1 ret 2 != 1 (0x2 != 0x1)FAIL (1 
> times)
>   test_bpf: #83 ALU_MOVSX | BPF_H jited:1 ret 2 != 1 (0x2 != 0x1)FAIL (1 
> times)
>   test_bpf: #84 ALU64_MOVSX | BPF_B jited:1 ret 2 != 1 (0x2 != 0x1)FAIL 
> (1 times)
>   test_bpf: #85 ALU64_MOVSX | BPF_H jited:1 ret 2 != 1 (0x2 != 0x1)FAIL 
> (1 times)
>   test_bpf: #86 ALU64_MOVSX | BPF_W jited:1 ret 2 != 1 (0x2 != 0x1)FAIL 
> (1 times)
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/bpf/32: Fix failing test_bpf tests
  https://git.kernel.org/powerpc/c/8ecf3c1dab1c675721d3d026abe2306fa340

cheers


Re: [PATCH v2] powerpc/iommu: Code cleanup for cell/iommu.c

2024-05-08 Thread Michael Ellerman
On Thu, 25 Jan 2024 16:26:37 +0800, Kunwu Chan wrote:
> This part was commented from commit 165785e5c0be ("[POWERPC] Cell
> iommu support") in about 17 years before.
> 
> If there are no plans to enable this part code in the future,
> we can remove this dead code.
> 
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/iommu: Code cleanup for cell/iommu.c
  https://git.kernel.org/powerpc/c/f3560a2ba5cbbb6c62c14dbdc1e33cb3565199d0

cheers


Re: [PATCH v2] powerpc/cell: Code cleanup for spufs_mfc_flush

2024-05-08 Thread Michael Ellerman
On Fri, 26 Jan 2024 10:12:58 +0800, Kunwu Chan wrote:
> This part was commented from commit a33a7d7309d7
> ("[PATCH] spufs: implement mfc access for PPE-side DMA")
> in about 18 years before.
> 
> If there are no plans to enable this part code in the future,
> we can remove this dead code.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/cell: Code cleanup for spufs_mfc_flush
  https://git.kernel.org/powerpc/c/66d8e646e8e78ea6088d9f6b9465e211566b5133

cheers


Re: [PATCH] powerpc/pseries/pci: Code cleanup

2024-05-08 Thread Michael Ellerman
On Fri, 26 Jan 2024 10:50:30 +0800, Kunwu Chan wrote:
> This part was commented in about 19 years before.
> If there are no plans to enable this part code in the future,
> we can remove this dead code.
> 
> 

Applied to powerpc/next.

[1/1] powerpc/pseries/pci: Code cleanup
  https://git.kernel.org/powerpc/c/2d8ebee0aac3a45d81de4f44255c8021d5a3401e

cheers


Re: [PATCH v1 0/9] Add dynamic DEXCR support

2024-05-08 Thread Michael Ellerman
On Wed, 17 Apr 2024 21:23:16 +1000, Benjamin Gray wrote:
> Adds support for a process to change its DEXCR value. The implementation is
> somewhat conservative; SBHE (speculative branch hint enable) is not exposed
> as an editable aspect because its effects can bleed over to other threads.
> 
> As explained in the third patch, this series changes the reset/inherit
> behaviour on exec. Previously there was a bitmask that tracked which aspects
> to copy from the current state vs resetting to a fixed default. This
> allows unprivileged processes to disable ROP protection for setuid binaries
> though, and is generally a weird interface to work with. The actual intent
> (and new implementation) tracks the exec value as an independent value that
> doesn't use the parent's DEXCR at all. The parent can control this reset value
> separately to its own DEXCR value.
> 
> [...]

Applied to powerpc/next.

[1/9] selftests/powerpc/dexcr: Add -no-pie to hashchk tests
  https://git.kernel.org/powerpc/c/d7228a58d9438d6f219dc7f33eab0d1980b3bd2f
[2/9] powerpc/dexcr: Track the DEXCR per-process
  https://git.kernel.org/powerpc/c/75171f06c4507c3b6b5a69d793879fb20d108bb1
[3/9] powerpc/dexcr: Reset DEXCR value across exec
  https://git.kernel.org/powerpc/c/bbd99922d0f4518518282217159666c679c6a0d1
[4/9] powerpc/dexcr: Add DEXCR prctl interface
  https://git.kernel.org/powerpc/c/628d701f2de5b9a16d1dd82bea68fd895f56f1a1
[5/9] selftests/powerpc/dexcr: Add DEXCR prctl interface test
  https://git.kernel.org/powerpc/c/5bfa66bf86d792bbcc76bc09cf99a2ae9d6e0eec
[6/9] selftests/powerpc/dexcr: Attempt to enable NPHIE in hashchk selftest
  https://git.kernel.org/powerpc/c/9930fba02a1c587849aea1e6c5688168013c065f
[7/9] selftests/powerpc/dexcr: Add DEXCR config details to lsdexcr
  https://git.kernel.org/powerpc/c/9c4866b209ad31cae7c832d45c6137ce6a993ca0
[8/9] selftests/powerpc/dexcr: Add chdexcr utility
  https://git.kernel.org/powerpc/c/f88723a609787254f7645eb6ac261b8363e8a5bc
[9/9] Documentation: Document PowerPC kernel dynamic DEXCR interface
  https://git.kernel.org/powerpc/c/9248edf31ab28723fb00900ecb8bacdb05eeefff

cheers


Re: [PATCH v2 1/2] powerpc/code-patching: Test patch_instructions() during boot

2024-05-08 Thread Michael Ellerman
On Mon, 25 Mar 2024 16:28:14 +1100, Benjamin Gray wrote:
> patch_instructions() introduces new behaviour with a couple of
> variations. Test each case of
> 
>   * a repeated 32-bit instruction,
>   * a repeated 64-bit instruction (ppc64), and
>   * a copied sequence of instructions
> 
> [...]

Applied to powerpc/next.

[1/2] powerpc/code-patching: Test patch_instructions() during boot
  https://git.kernel.org/powerpc/c/c5ef5e35844ad30503c49802b9d6a6c818fca886
[2/2] powerpc/code-patching: Use dedicated memory routines for patching
  https://git.kernel.org/powerpc/c/c3710ee7cd695dc1b0b4b8cfbf464e313467f970

cheers


Re: [PATCH] powerpc64/kasan: Pass virtual addresses to kasan_init_phys_region()

2024-05-08 Thread Michael Ellerman
On Mon, 12 Feb 2024 15:50:20 +1100, Benjamin Gray wrote:
> The kasan_init_phys_region() function maps shadow pages necessary for
> the ranges of the linear map backed by physical pages. Currently
> kasan_init_phys_region() is being passed physical addresses, but
> kasan_mem_to_shadow() expects virtual addresses.
> 
> It works right now because the lower bits (12:64) of the
> kasan_mem_to_shadow() calculation are the same for the real and virtual
> addresses, so the actual PTE value is the same in the end. But virtual
> addresses are the intended input, so fix it.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc64/kasan: Pass virtual addresses to kasan_init_phys_region()
  https://git.kernel.org/powerpc/c/295454eda97b9c5f7a64ac5c2bb827fd15efb623

cheers


Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg

2024-05-08 Thread Thomas Huth

On 08/05/2024 14.58, Thomas Huth wrote:

On 08/05/2024 14.55, Thomas Huth wrote:

On 08/05/2024 14.27, Nicholas Piggin wrote:

On Wed May 8, 2024 at 1:08 AM AEST, Thomas Huth wrote:

On 04/05/2024 14.28, Nicholas Piggin wrote:

This allows different machines with different requirements to be
supported by run_tests.sh, similarly to how different accelerators
are handled.

Acked-by: Thomas Huth 
Acked-by: Andrew Jones 
Signed-off-by: Nicholas Piggin 
---
   docs/unittests.txt   |  7 +++
   scripts/common.bash  |  8 ++--
   scripts/runtime.bash | 16 
   3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index 7cf2c55ad..6449efd78 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -42,6 +42,13 @@ For / directories that support multiple 
architectures, this restricts

   the test to the specified arch. By default, the test will run on any
   architecture.
+machine
+---
+For those architectures that support multiple machine types, this 
restricts

+the test to the specified machine. By default, the test will run on
+any machine type. (Note, the machine can be specified with the MACHINE=
+environment variable, and defaults to the architecture's default.)
+
   smp
   ---
   smp = 
diff --git a/scripts/common.bash b/scripts/common.bash
index 5e9ad53e2..3aa557c8c 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -10,6 +10,7 @@ function for_each_unittest()
   local opts
   local groups
   local arch
+    local machine
   local check
   local accel
   local timeout
@@ -21,7 +22,7 @@ function for_each_unittest()
   if [[ "$line" =~ ^\[(.*)\]$ ]]; then
   rematch=${BASH_REMATCH[1]}
   if [ -n "${testname}" ]; then
-    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
"$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
"$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"

   fi
   testname=$rematch
   smp=1
@@ -29,6 +30,7 @@ function for_each_unittest()
   opts=""
   groups=""
   arch=""
+    machine=""
   check=""
   accel=""
   timeout=""
@@ -58,6 +60,8 @@ function for_each_unittest()
   groups=${BASH_REMATCH[1]}
   elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
   arch=${BASH_REMATCH[1]}
+    elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
+    machine=${BASH_REMATCH[1]}
   elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
   check=${BASH_REMATCH[1]}
   elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
@@ -67,7 +71,7 @@ function for_each_unittest()
   fi
   done
   if [ -n "${testname}" ]; then
-    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
"$opts" "$arch" "$check" "$accel" "$timeout"
+    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
"$opts" "$arch" "$machine" "$check" "$accel" "$timeout"

   fi
   exec {fd}<&-
   }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 177b62166..0c96d6ea2 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -32,7 +32,7 @@ premature_failure()
   get_cmdline()
   {
   local kernel=$1
-    echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel 
$RUNTIME_arch_run $kernel -smp $smp $opts"
+    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine 
ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"

   }
   skip_nodefault()
@@ -80,9 +80,10 @@ function run()
   local kernel="$4"
   local opts="$5"
   local arch="$6"
-    local check="${CHECK:-$7}"
-    local accel="$8"
-    local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
+    local machine="$7"
+    local check="${CHECK:-$8}"
+    local accel="$9"
+    local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
   if [ "${CONFIG_EFI}" == "y" ]; then
   kernel=${kernel/%.flat/.efi}
@@ -116,6 +117,13 @@ function run()
   return 2
   fi
+    if [ -n "$machine" ] && [ -n "$MACHINE" ] && [ "$machine" != 
"$MACHINE" ]; then

+    print_result "SKIP" $testname "" "$machine only"
+    return 2
+    elif [ -n "$MACHINE" ]; then
+    machine="$MACHINE"
+    fi
+
   if [ -n "$accel" ] && [ -n "$ACCEL" ] && [ "$accel" != "$ACCEL" 
]; then
   print_result "SKIP" $testname "" "$accel only, but 
ACCEL=$ACCEL"

   return 2


For some reasons that I don't quite understand yet, this patch causes the
"sieve" test to always timeout on the s390x runner, see e.g.:

   https://gitlab.com/thuth/kvm-unit-tests/-/jobs/6798954987


How do you use the s390x runner?



Everything is fine in the previous patches (I pushed now the previous 5
patches to the repo):

   https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/pipelines/1281919104

Could it be that he 

[PATCH 2/2] powerpc/kexec_file: fix cpus node update to FDT

2024-05-08 Thread Sourabh Jain
While updating the cpus node, commit 40c753993e3a ("powerpc/kexec_file:
 Use current CPU info while setting up FDT") first deletes all subnodes
under the /cpus node. However, while adding sub-nodes back, it missed
adding cpus subnodes whose device_type != "cpu", such as l2-cache*,
l3-cache*, ibm,powerpc-cpu-features.

Fix this by only deleting cpus sub-nodes of device_type == "cpus" and
then adding all available nodes with device_type == "cpu".

Fixes: 40c753993e3a ("powerpc/kexec_file: Use current CPU info while setting up 
FDT")
Cc: Aditya Gupta 
Cc: Hari Bathini 
Cc: Mahesh Salgaonkar 
Cc: Michael Ellerman 
Cc: "Naveen N. Rao" 
Signed-off-by: Sourabh Jain 
---
 arch/powerpc/kexec/core_64.c | 53 +---
 1 file changed, 37 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 85050be08a23..2e625c2cb6b9 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -456,9 +456,15 @@ static int add_node_props(void *fdt, int node_offset, 
const struct device_node *
  * @fdt:  Flattened device tree of the kernel.
  *
  * Returns 0 on success, negative errno on error.
+ *
+ * Note: expecting no subnodes under /cpus/ with device_type == "cpu".
+ * If this changes, update this function to include them.
  */
 int update_cpus_node(void *fdt)
 {
+   int prev_node_offset;
+   const char *device_type;
+   const struct fdt_property *prop;
struct device_node *cpus_node, *dn;
int cpus_offset, cpus_subnode_offset, ret = 0;
 
@@ -469,30 +475,44 @@ int update_cpus_node(void *fdt)
return cpus_offset;
}
 
-   if (cpus_offset > 0) {
-   ret = fdt_del_node(fdt, cpus_offset);
+   prev_node_offset = cpus_offset;
+   /* Delete sub-nodes of /cpus node with device_type == "cpu" */
+   for (cpus_subnode_offset = fdt_first_subnode(fdt, cpus_offset); 
cpus_subnode_offset >= 0;) {
+   /* Ignore nodes that do not have a device_type property or 
device_type != "cpu" */
+   prop = fdt_get_property(fdt, cpus_subnode_offset, 
"device_type", NULL);
+   if (!prop || strcmp(prop->data, "cpu")) {
+   prev_node_offset = cpus_subnode_offset;
+   goto next_node;
+   }
+
+   ret = fdt_del_node(fdt, cpus_subnode_offset);
if (ret < 0) {
-   pr_err("Error deleting /cpus node: %s\n", 
fdt_strerror(ret));
-   return -EINVAL;
+   pr_err("Failed to delete a cpus sub-node: %s\n", 
fdt_strerror(ret));
+   return ret;
}
+next_node:
+   if (prev_node_offset == cpus_offset)
+   cpus_subnode_offset = fdt_first_subnode(fdt, 
cpus_offset);
+   else
+   cpus_subnode_offset = fdt_next_subnode(fdt, 
prev_node_offset);
}
 
-   /* Add cpus node to fdt */
-   cpus_offset = fdt_add_subnode(fdt, fdt_path_offset(fdt, "/"), "cpus");
-   if (cpus_offset < 0) {
-   pr_err("Error creating /cpus node: %s\n", 
fdt_strerror(cpus_offset));
+   cpus_node = of_find_node_by_path("/cpus");
+   /* Fail here to avoid kexec/kdump kernel boot hung */
+   if (!cpus_node) {
+   pr_err("No /cpus node found\n");
return -EINVAL;
}
 
-   /* Add cpus node properties */
-   cpus_node = of_find_node_by_path("/cpus");
-   ret = add_node_props(fdt, cpus_offset, cpus_node);
-   of_node_put(cpus_node);
-   if (ret < 0)
-   return ret;
+   /* Add all /cpus sub-nodes of device_type == "cpu" to FDT */
+   for_each_child_of_node(cpus_node, dn) {
+   /* Ignore device nodes that do not have a device_type property
+* or device_type != "cpu".
+*/
+   device_type = of_get_property(dn, "device_type", NULL);
+   if (!device_type || strcmp(device_type, "cpu"))
+   continue;
 
-   /* Loop through all subnodes of cpus and add them to fdt */
-   for_each_node_by_type(dn, "cpu") {
cpus_subnode_offset = fdt_add_subnode(fdt, cpus_offset, 
dn->full_name);
if (cpus_subnode_offset < 0) {
pr_err("Unable to add %s subnode: %s\n", dn->full_name,
@@ -506,6 +526,7 @@ int update_cpus_node(void *fdt)
goto out;
}
 out:
+   of_node_put(cpus_node);
of_node_put(dn);
return ret;
 }
-- 
2.44.0



[PATCH 0/2] powerpc: kexec fixes

2024-05-08 Thread Sourabh Jain
Patch series fixes two kexec issues.

01/02: Update extra size calculation for kexec FDT to avoid kexec load
failure due to FDT_ERR_NOSPACE while including CPU nodes added post
boot and reserved memory ranges.

02/02: Fix update_cpus_node/core_64.c function to include missing device
nodes under /cpus node with device_type != "cpu".

Note: this patch series is rebased on top of the linux-next/master
to avoid the conflict with the below patch series:
https://lore.kernel.org/all/171509287314.62008.11812494124513471250.b4...@ellerman.id.au/

Cc: Aditya Gupta 
Cc: Hari Bathini 
Cc: Mahesh Salgaonkar 
Cc: Michael Ellerman 
Cc: "Naveen N. Rao" 

Sourabh Jain (2):
  powerpc/kexec_file: fix extra size calculation for kexec FDT
  powerpc/kexec_file: fix cpus node update to FDT

 arch/powerpc/include/asm/kexec.h  |  6 ++--
 arch/powerpc/kexec/core_64.c  | 53 +--
 arch/powerpc/kexec/elf_64.c   | 12 +--
 arch/powerpc/kexec/file_load_64.c | 53 +--
 4 files changed, 70 insertions(+), 54 deletions(-)

-- 
2.44.0



[PATCH 1/2] powerpc/kexec_file: fix extra size calculation for kexec FDT

2024-05-08 Thread Sourabh Jain
While setting up the FDT for kexec, CPU nodes that are added after the
system boots and reserved memory ranges are incorporated into the
initial_boot_params (base FDT).

However, they are not taken into account when determining the additional
size needed for the kexec FDT. As a result, kexec fails to load,
generating the following error:

[1116.774451] Error updating memory reserve map: FDT_ERR_NOSPACE
kexec_file_load failed: No such process

Therefore, consider the extra size for CPU nodes added post-system boot
and reserved memory ranges while preparing the kexec FDT.

While adding a new parameter to the setup_new_fdt_ppc64 function, it was
noticed that there were a couple of unused parameters, so they were
removed.

Cc: Aditya Gupta 
Cc: Hari Bathini 
Cc: Mahesh Salgaonkar 
Cc: Michael Ellerman 
Cc: "Naveen N. Rao" 
Signed-off-by: Sourabh Jain 
---
 arch/powerpc/include/asm/kexec.h  |  6 ++--
 arch/powerpc/kexec/elf_64.c   | 12 +--
 arch/powerpc/kexec/file_load_64.c | 53 +--
 3 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 95a98b390d62..270ee93a0f7d 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -103,10 +103,8 @@ int load_crashdump_segments_ppc64(struct kimage *image,
 int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
  const void *fdt, unsigned long kernel_load_addr,
  unsigned long fdt_load_addr);
-unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image);
-int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
-   unsigned long initrd_load_addr,
-   unsigned long initrd_len, const char *cmdline);
+unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image, struct crash_mem 
*rmem);
+int setup_new_fdt_ppc64(const struct kimage *image, void *fdt, struct 
crash_mem *rmem);
 #endif /* CONFIG_PPC64 */
 
 #endif /* CONFIG_KEXEC_FILE */
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 214c071c58ed..5d6d616404cf 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 static void *elf64_load(struct kimage *image, char *kernel_buf,
unsigned long kernel_len, char *initrd,
@@ -36,6 +37,7 @@ static void *elf64_load(struct kimage *image, char 
*kernel_buf,
const void *slave_code;
struct elfhdr ehdr;
char *modified_cmdline = NULL;
+   struct crash_mem *rmem = NULL;
struct kexec_elf_info elf_info;
struct kexec_buf kbuf = { .image = image, .buf_min = 0,
  .buf_max = ppc64_rma_size };
@@ -102,17 +104,20 @@ static void *elf64_load(struct kimage *image, char 
*kernel_buf,
kexec_dprintk("Loaded initrd at 0x%lx\n", initrd_load_addr);
}
 
+   ret = get_reserved_memory_ranges();
+   if (ret)
+   goto out;
+
fdt = of_kexec_alloc_and_setup_fdt(image, initrd_load_addr,
   initrd_len, cmdline,
-  kexec_extra_fdt_size_ppc64(image));
+  kexec_extra_fdt_size_ppc64(image, 
rmem));
if (!fdt) {
pr_err("Error setting up the new device tree.\n");
ret = -EINVAL;
goto out;
}
 
-   ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
- initrd_len, cmdline);
+   ret = setup_new_fdt_ppc64(image, fdt, rmem);
if (ret)
goto out_free_fdt;
 
@@ -146,6 +151,7 @@ static void *elf64_load(struct kimage *image, char 
*kernel_buf,
 out_free_fdt:
kvfree(fdt);
 out:
+   kfree(rmem);
kfree(modified_cmdline);
kexec_free_elf_info(_info);
 
diff --git a/arch/powerpc/kexec/file_load_64.c 
b/arch/powerpc/kexec/file_load_64.c
index 925a69ad2468..41be4546a34e 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -803,10 +803,9 @@ static unsigned int cpu_node_size(void)
return size;
 }
 
-static unsigned int kdump_extra_fdt_size_ppc64(struct kimage *image)
+static unsigned int kdump_extra_fdt_size_ppc64(struct kimage *image, unsigned 
int cpu_nodes)
 {
-   unsigned int cpu_nodes, extra_size = 0;
-   struct device_node *dn;
+   unsigned int extra_size = 0;
u64 usm_entries;
 #ifdef CONFIG_CRASH_HOTPLUG
unsigned int possible_cpu_nodes;
@@ -826,18 +825,6 @@ static unsigned int kdump_extra_fdt_size_ppc64(struct 
kimage *image)
extra_size += (unsigned int)(usm_entries * sizeof(u64));
}
 
-   /*
-* Get the number of CPU nodes in the current DT. This allows to
-* reserve places for CPU nodes added since the boot time.
-*/
-  

Re: [PATCH v2 1/1] x86/elf: Add a new .note section containing Xfeatures information to x86 core files

2024-05-08 Thread Thomas Gleixner
On Tue, May 07 2024 at 15:23, Vignesh Balasubramanian wrote:
> +struct xfeat_component {
> + u32 xfeat_type;
> + u32 xfeat_sz;
> + u32 xfeat_off;
> + u32 xfeat_flags;
> +} __packed;

Why repeating xfeat_ for all member names?

u32   type;
u32   size;
u32   offset;
u32   flags;

is sufficient and obvious, no?

> +enum custom_feature {
> + FEATURE_XSAVE_FP = 0,
> + FEATURE_XSAVE_SSE = 1,
> + FEATURE_XSAVE_YMM = 2,
> + FEATURE_XSAVE_BNDREGS = 3,
> + FEATURE_XSAVE_BNDCSR = 4,
> + FEATURE_XSAVE_OPMASK = 5,
> + FEATURE_XSAVE_ZMM_Hi256 = 6,
> + FEATURE_XSAVE_Hi16_ZMM = 7,
> + FEATURE_XSAVE_PT = 8,
> + FEATURE_XSAVE_PKRU = 9,
> + FEATURE_XSAVE_PASID = 10,
> + FEATURE_XSAVE_CET_USER = 11,
> + FEATURE_XSAVE_CET_SHADOW_STACK = 12,
> + FEATURE_XSAVE_HDC = 13,
> + FEATURE_XSAVE_UINTR = 14,
> + FEATURE_XSAVE_LBR = 15,
> + FEATURE_XSAVE_HWP = 16,
> + FEATURE_XSAVE_XTILE_CFG = 17,
> + FEATURE_XSAVE_XTILE_DATA = 18,
> + FEATURE_MAX,
> + FEATURE_XSAVE_EXTENDED_START = FEATURE_XSAVE_YMM,
> + FEATURE_XSAVE_EXTENDED_END = FEATURE_XSAVE_XTILE_DATA,
> +};

Why can't this use the existing 'enum xfeature' which is providing
exactly the same information already?

> +#ifdef CONFIG_COREDUMP
> +static int get_sub_leaf(int custom_xfeat)
> +{
> + switch (custom_xfeat) {
> + case FEATURE_XSAVE_YMM: return XFEATURE_YMM;
> + case FEATURE_XSAVE_BNDREGS: return XFEATURE_BNDREGS;
> + case FEATURE_XSAVE_BNDCSR:  return XFEATURE_BNDCSR;
> + case FEATURE_XSAVE_OPMASK:  return XFEATURE_OPMASK;
> + case FEATURE_XSAVE_ZMM_Hi256:   return XFEATURE_ZMM_Hi256;
> + case FEATURE_XSAVE_Hi16_ZMM:return XFEATURE_Hi16_ZMM;
> + case FEATURE_XSAVE_PT:  return 
> XFEATURE_PT_UNIMPLEMENTED_SO_FAR;
> + case FEATURE_XSAVE_PKRU:return XFEATURE_PKRU;
> + case FEATURE_XSAVE_PASID:   return XFEATURE_PASID;
> + case FEATURE_XSAVE_CET_USER:return XFEATURE_CET_USER;
> + case FEATURE_XSAVE_CET_SHADOW_STACK:return 
> XFEATURE_CET_KERNEL_UNUSED;
> + case FEATURE_XSAVE_HDC: return XFEATURE_RSRVD_COMP_13;
> + case FEATURE_XSAVE_UINTR:   return XFEATURE_RSRVD_COMP_14;
> + case FEATURE_XSAVE_LBR: return XFEATURE_LBR;
> + case FEATURE_XSAVE_HWP: return XFEATURE_RSRVD_COMP_16;
> + case FEATURE_XSAVE_XTILE_CFG:   return XFEATURE_XTILE_CFG;
> + case FEATURE_XSAVE_XTILE_DATA:  return XFEATURE_XTILE_DATA;
> + default:
> + pr_warn_ratelimited("Not a valid XSAVE Feature.");
> + return 0;
> + }
> +}

This function then maps the identical enums one to one. The only actual
"functionality" is the default case and that's completely pointless.

> +/*
> + * Dump type, size, offset and flag values for every xfeature that is 
> present.
> + */
> +static int dump_xsave_layout_desc(struct coredump_params *cprm)
> +{
> + u32 supported_features = 0;
> + struct xfeat_component xc;
> + u32 eax, ebx, ecx, edx;
> + int num_records = 0;
> + int sub_leaf = 0;
> + int i;
> +
> + /* Find supported extended features */
> + cpuid_count(XSTATE_CPUID, 0, , , , );
> + supported_features = eax;

Why does this need to re-evaluate CPUID instead of just using the
existing fpu_user_cfg.max_features?

> + for (i = FEATURE_XSAVE_EXTENDED_START;
> + i <= FEATURE_XSAVE_EXTENDED_END; i++) {

Please use the full 100 character line width.

> + sub_leaf = get_sub_leaf(i);
> + if (!sub_leaf)
> + continue;
> + if (supported_features & (1U << sub_leaf)) {
> + cpuid_count(XSTATE_CPUID, sub_leaf, , , , 
> );
> + xc.xfeat_type = i;
> + xc.xfeat_sz = eax;
> + xc.xfeat_off = ebx;
> + /* Reserved for future use */
> + xc.xfeat_flags = 0;
> +
> + if (!dump_emit(cprm, ,
> +sizeof(struct xfeat_component)))

sizeof(xc), no?

> + return 0;
> + num_records++;
> + }
> + }

This whole thing can be written as:

for_each_extended_xfeature(i, fpu_user_cfg.max_features) {
struct xfeat_component xc = {
.type   = i,
.size   = xstate_sizes[i],
.offset = xstate_offsets[i],
};

if (!dump_emit(cprm, , sizeof(xc)))
return 0;
num_records++;
}

It omits the features which are supported by the CPU, but not enabled by
the kernel. That's perfectly fine because:

  1) the corresponding xfeature 

Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg

2024-05-08 Thread Thomas Huth

On 08/05/2024 14.55, Thomas Huth wrote:

On 08/05/2024 14.27, Nicholas Piggin wrote:

On Wed May 8, 2024 at 1:08 AM AEST, Thomas Huth wrote:

On 04/05/2024 14.28, Nicholas Piggin wrote:

This allows different machines with different requirements to be
supported by run_tests.sh, similarly to how different accelerators
are handled.

Acked-by: Thomas Huth 
Acked-by: Andrew Jones 
Signed-off-by: Nicholas Piggin 
---
   docs/unittests.txt   |  7 +++
   scripts/common.bash  |  8 ++--
   scripts/runtime.bash | 16 
   3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index 7cf2c55ad..6449efd78 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -42,6 +42,13 @@ For / directories that support multiple 
architectures, this restricts

   the test to the specified arch. By default, the test will run on any
   architecture.
+machine
+---
+For those architectures that support multiple machine types, this 
restricts

+the test to the specified machine. By default, the test will run on
+any machine type. (Note, the machine can be specified with the MACHINE=
+environment variable, and defaults to the architecture's default.)
+
   smp
   ---
   smp = 
diff --git a/scripts/common.bash b/scripts/common.bash
index 5e9ad53e2..3aa557c8c 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -10,6 +10,7 @@ function for_each_unittest()
   local opts
   local groups
   local arch
+    local machine
   local check
   local accel
   local timeout
@@ -21,7 +22,7 @@ function for_each_unittest()
   if [[ "$line" =~ ^\[(.*)\]$ ]]; then
   rematch=${BASH_REMATCH[1]}
   if [ -n "${testname}" ]; then
-    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
"$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
"$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"

   fi
   testname=$rematch
   smp=1
@@ -29,6 +30,7 @@ function for_each_unittest()
   opts=""
   groups=""
   arch=""
+    machine=""
   check=""
   accel=""
   timeout=""
@@ -58,6 +60,8 @@ function for_each_unittest()
   groups=${BASH_REMATCH[1]}
   elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
   arch=${BASH_REMATCH[1]}
+    elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
+    machine=${BASH_REMATCH[1]}
   elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
   check=${BASH_REMATCH[1]}
   elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
@@ -67,7 +71,7 @@ function for_each_unittest()
   fi
   done
   if [ -n "${testname}" ]; then
-    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
"$opts" "$arch" "$check" "$accel" "$timeout"
+    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
"$opts" "$arch" "$machine" "$check" "$accel" "$timeout"

   fi
   exec {fd}<&-
   }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 177b62166..0c96d6ea2 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -32,7 +32,7 @@ premature_failure()
   get_cmdline()
   {
   local kernel=$1
-    echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel 
$RUNTIME_arch_run $kernel -smp $smp $opts"
+    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine 
ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"

   }
   skip_nodefault()
@@ -80,9 +80,10 @@ function run()
   local kernel="$4"
   local opts="$5"
   local arch="$6"
-    local check="${CHECK:-$7}"
-    local accel="$8"
-    local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
+    local machine="$7"
+    local check="${CHECK:-$8}"
+    local accel="$9"
+    local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
   if [ "${CONFIG_EFI}" == "y" ]; then
   kernel=${kernel/%.flat/.efi}
@@ -116,6 +117,13 @@ function run()
   return 2
   fi
+    if [ -n "$machine" ] && [ -n "$MACHINE" ] && [ "$machine" != 
"$MACHINE" ]; then

+    print_result "SKIP" $testname "" "$machine only"
+    return 2
+    elif [ -n "$MACHINE" ]; then
+    machine="$MACHINE"
+    fi
+
   if [ -n "$accel" ] && [ -n "$ACCEL" ] && [ "$accel" != "$ACCEL" 
]; then

   print_result "SKIP" $testname "" "$accel only, but ACCEL=$ACCEL"
   return 2


For some reasons that I don't quite understand yet, this patch causes the
"sieve" test to always timeout on the s390x runner, see e.g.:

   https://gitlab.com/thuth/kvm-unit-tests/-/jobs/6798954987


How do you use the s390x runner?



Everything is fine in the previous patches (I pushed now the previous 5
patches to the repo):

   https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/pipelines/1281919104

Could it be that he TIMEOUT gets messed up in certain cases?



Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg

2024-05-08 Thread Thomas Huth

On 08/05/2024 14.27, Nicholas Piggin wrote:

On Wed May 8, 2024 at 1:08 AM AEST, Thomas Huth wrote:

On 04/05/2024 14.28, Nicholas Piggin wrote:

This allows different machines with different requirements to be
supported by run_tests.sh, similarly to how different accelerators
are handled.

Acked-by: Thomas Huth 
Acked-by: Andrew Jones 
Signed-off-by: Nicholas Piggin 
---
   docs/unittests.txt   |  7 +++
   scripts/common.bash  |  8 ++--
   scripts/runtime.bash | 16 
   3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index 7cf2c55ad..6449efd78 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -42,6 +42,13 @@ For / directories that support multiple architectures, 
this restricts
   the test to the specified arch. By default, the test will run on any
   architecture.
   
+machine

+---
+For those architectures that support multiple machine types, this restricts
+the test to the specified machine. By default, the test will run on
+any machine type. (Note, the machine can be specified with the MACHINE=
+environment variable, and defaults to the architecture's default.)
+
   smp
   ---
   smp = 
diff --git a/scripts/common.bash b/scripts/common.bash
index 5e9ad53e2..3aa557c8c 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -10,6 +10,7 @@ function for_each_unittest()
local opts
local groups
local arch
+   local machine
local check
local accel
local timeout
@@ -21,7 +22,7 @@ function for_each_unittest()
if [[ "$line" =~ ^\[(.*)\]$ ]]; then
rematch=${BASH_REMATCH[1]}
if [ -n "${testname}" ]; then
-   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" 
"$check" "$accel" "$timeout"
+   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" 
"$machine" "$check" "$accel" "$timeout"
fi
testname=$rematch
smp=1
@@ -29,6 +30,7 @@ function for_each_unittest()
opts=""
groups=""
arch=""
+   machine=""
check=""
accel=""
timeout=""
@@ -58,6 +60,8 @@ function for_each_unittest()
groups=${BASH_REMATCH[1]}
elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
arch=${BASH_REMATCH[1]}
+   elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
+   machine=${BASH_REMATCH[1]}
elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
check=${BASH_REMATCH[1]}
elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
@@ -67,7 +71,7 @@ function for_each_unittest()
fi
done
if [ -n "${testname}" ]; then
-   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" 
"$accel" "$timeout"
+   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" 
"$check" "$accel" "$timeout"
fi
exec {fd}<&-
   }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 177b62166..0c96d6ea2 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -32,7 +32,7 @@ premature_failure()
   get_cmdline()
   {
   local kernel=$1
-echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run 
$kernel -smp $smp $opts"
+echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel 
$RUNTIME_arch_run $kernel -smp $smp $opts"
   }
   
   skip_nodefault()

@@ -80,9 +80,10 @@ function run()
   local kernel="$4"
   local opts="$5"
   local arch="$6"
-local check="${CHECK:-$7}"
-local accel="$8"
-local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
+local machine="$7"
+local check="${CHECK:-$8}"
+local accel="$9"
+local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
   
   if [ "${CONFIG_EFI}" == "y" ]; then

   kernel=${kernel/%.flat/.efi}
@@ -116,6 +117,13 @@ function run()
   return 2
   fi
   
+if [ -n "$machine" ] && [ -n "$MACHINE" ] && [ "$machine" != "$MACHINE" ]; then

+print_result "SKIP" $testname "" "$machine only"
+return 2
+elif [ -n "$MACHINE" ]; then
+machine="$MACHINE"
+fi
+
   if [ -n "$accel" ] && [ -n "$ACCEL" ] && [ "$accel" != "$ACCEL" ]; then
   print_result "SKIP" $testname "" "$accel only, but ACCEL=$ACCEL"
   return 2


For some reasons that I don't quite understand yet, this patch causes the
"sieve" test to always timeout on the s390x runner, see e.g.:

   https://gitlab.com/thuth/kvm-unit-tests/-/jobs/6798954987


How do you use the s390x runner?



Everything is fine in the 

Re: [PATCH v6] arch/powerpc/kvm: Add support for reading VPA counters for pseries guests

2024-05-08 Thread Michael Ellerman
Gautam Menghani  writes:
> PAPR hypervisor has introduced three new counters in the VPA area of
> LPAR CPUs for KVM L2 guest (see [1] for terminology) observability - 2
> for context switches from host to guest and vice versa, and 1 counter
> for getting the total time spent inside the KVM guest. Add a tracepoint
> that enables reading the counters for use by ftrace/perf. Note that this
> tracepoint is only available for nestedv2 API (i.e, KVM on PowerVM).
...
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 8e86eb577eb8..ed69ad58bd02 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -4108,6 +4108,54 @@ static void vcpu_vpa_increment_dispatch(struct 
> kvm_vcpu *vcpu)
>   }
>  }
>  
> +static inline int kvmhv_get_l2_counters_status(void)
> +{
> + return get_lppaca()->l2_counters_enable;
> +}

This is breaking the powernv build:

$ make powernv_defconfig ; make -s -j (nproc)
make[1]: Entering directory '/home/michael/linux/.build'
  GEN Makefile
#
# configuration written to .config
#
make[1]: Leaving directory '/home/michael/linux/.build'
../arch/powerpc/kvm/book3s_hv.c: In function ‘kvmhv_get_l2_counters_status’:
../arch/powerpc/kvm/book3s_hv.c:4113:16: error: implicit declaration of 
function ‘get_lppaca’; did you mean ‘get_paca’? 
[-Werror=implicit-function-declaration]
 4113 | return get_lppaca()->l2_counters_enable;
  |^~
  |get_paca
../arch/powerpc/kvm/book3s_hv.c:4113:28: error: invalid type argument of ‘->’ 
(have ‘int’)
 4113 | return get_lppaca()->l2_counters_enable;
  |^~
In file included from ../arch/powerpc/include/asm/paravirt.h:9,
 from ../arch/powerpc/include/asm/qspinlock.h:7,
 from ../arch/powerpc/include/asm/spinlock.h:7,
 from ../include/linux/spinlock.h:95,
 from ../include/linux/sched.h:2138,
 from ../include/linux/hardirq.h:9,
 from ../include/linux/kvm_host.h:7,
 from ../arch/powerpc/kvm/book3s_hv.c:18:
../arch/powerpc/kvm/book3s_hv.c: In function ‘kvmhv_set_l2_counters_status’:
../arch/powerpc/include/asm/lppaca.h:105:41: error: ‘struct paca_struct’ has no 
member named ‘lppaca_ptr’
  105 | #define lppaca_of(cpu)  (*paca_ptrs[cpu]->lppaca_ptr)
  | ^~
../arch/powerpc/kvm/book3s_hv.c:4119:17: note: in expansion of macro ‘lppaca_of’
 4119 | lppaca_of(cpu).l2_counters_enable = 1;
  | ^
../arch/powerpc/include/asm/lppaca.h:105:41: error: ‘struct paca_struct’ has no 
member named ‘lppaca_ptr’
  105 | #define lppaca_of(cpu)  (*paca_ptrs[cpu]->lppaca_ptr)
  | ^~
../arch/powerpc/kvm/book3s_hv.c:4121:17: note: in expansion of macro ‘lppaca_of’
 4121 | lppaca_of(cpu).l2_counters_enable = 0;
  | ^
../arch/powerpc/kvm/book3s_hv.c: In function ‘do_trace_nested_cs_time’:
../arch/powerpc/kvm/book3s_hv.c:4145:29: error: initialization of ‘struct 
lppaca *’ from ‘int’ makes pointer from integer without a cast 
[-Werror=int-conversion]
 4145 | struct lppaca *lp = get_lppaca();
  | ^~
../arch/powerpc/kvm/book3s_hv.c: In function ‘kvmhv_get_l2_counters_status’:
../arch/powerpc/kvm/book3s_hv.c:4114:1: error: control reaches end of non-void 
function [-Werror=return-type]
 4114 | }
  | ^
cc1: all warnings being treated as errors
make[5]: *** [../scripts/Makefile.build:244: arch/powerpc/kvm/book3s_hv.o] 
Error 1
make[5]: *** Waiting for unfinished jobs
make[4]: *** [../scripts/Makefile.build:485: arch/powerpc/kvm] Error 2
make[4]: *** Waiting for unfinished jobs
make[3]: *** [../scripts/Makefile.build:485: arch/powerpc] Error 2
make[3]: *** Waiting for unfinished jobs
make[2]: *** [/home/michael/linux/Makefile:1919: .] Error 2
make[1]: *** [/home/michael/linux/Makefile:240: __sub-make] Error 2
make: *** [Makefile:240: __sub-make] Error 2


cheers


Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg

2024-05-08 Thread Nicholas Piggin
On Wed May 8, 2024 at 1:08 AM AEST, Thomas Huth wrote:
> On 04/05/2024 14.28, Nicholas Piggin wrote:
> > This allows different machines with different requirements to be
> > supported by run_tests.sh, similarly to how different accelerators
> > are handled.
> > 
> > Acked-by: Thomas Huth 
> > Acked-by: Andrew Jones 
> > Signed-off-by: Nicholas Piggin 
> > ---
> >   docs/unittests.txt   |  7 +++
> >   scripts/common.bash  |  8 ++--
> >   scripts/runtime.bash | 16 
> >   3 files changed, 25 insertions(+), 6 deletions(-)
> > 
> > diff --git a/docs/unittests.txt b/docs/unittests.txt
> > index 7cf2c55ad..6449efd78 100644
> > --- a/docs/unittests.txt
> > +++ b/docs/unittests.txt
> > @@ -42,6 +42,13 @@ For / directories that support multiple 
> > architectures, this restricts
> >   the test to the specified arch. By default, the test will run on any
> >   architecture.
> >   
> > +machine
> > +---
> > +For those architectures that support multiple machine types, this restricts
> > +the test to the specified machine. By default, the test will run on
> > +any machine type. (Note, the machine can be specified with the MACHINE=
> > +environment variable, and defaults to the architecture's default.)
> > +
> >   smp
> >   ---
> >   smp = 
> > diff --git a/scripts/common.bash b/scripts/common.bash
> > index 5e9ad53e2..3aa557c8c 100644
> > --- a/scripts/common.bash
> > +++ b/scripts/common.bash
> > @@ -10,6 +10,7 @@ function for_each_unittest()
> > local opts
> > local groups
> > local arch
> > +   local machine
> > local check
> > local accel
> > local timeout
> > @@ -21,7 +22,7 @@ function for_each_unittest()
> > if [[ "$line" =~ ^\[(.*)\]$ ]]; then
> > rematch=${BASH_REMATCH[1]}
> > if [ -n "${testname}" ]; then
> > -   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
> > "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> > +   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
> > "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> > fi
> > testname=$rematch
> > smp=1
> > @@ -29,6 +30,7 @@ function for_each_unittest()
> > opts=""
> > groups=""
> > arch=""
> > +   machine=""
> > check=""
> > accel=""
> > timeout=""
> > @@ -58,6 +60,8 @@ function for_each_unittest()
> > groups=${BASH_REMATCH[1]}
> > elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
> > arch=${BASH_REMATCH[1]}
> > +   elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
> > +   machine=${BASH_REMATCH[1]}
> > elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
> > check=${BASH_REMATCH[1]}
> > elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
> > @@ -67,7 +71,7 @@ function for_each_unittest()
> > fi
> > done
> > if [ -n "${testname}" ]; then
> > -   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
> > "$opts" "$arch" "$check" "$accel" "$timeout"
> > +   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
> > "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> > fi
> > exec {fd}<&-
> >   }
> > diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> > index 177b62166..0c96d6ea2 100644
> > --- a/scripts/runtime.bash
> > +++ b/scripts/runtime.bash
> > @@ -32,7 +32,7 @@ premature_failure()
> >   get_cmdline()
> >   {
> >   local kernel=$1
> > -echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel 
> > $RUNTIME_arch_run $kernel -smp $smp $opts"
> > +echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine 
> > ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
> >   }
> >   
> >   skip_nodefault()
> > @@ -80,9 +80,10 @@ function run()
> >   local kernel="$4"
> >   local opts="$5"
> >   local arch="$6"
> > -local check="${CHECK:-$7}"
> > -local accel="$8"
> > -local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
> > +local machine="$7"
> > +local check="${CHECK:-$8}"
> > +local accel="$9"
> > +local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
> >   
> >   if [ "${CONFIG_EFI}" == "y" ]; then
> >   kernel=${kernel/%.flat/.efi}
> > @@ -116,6 +117,13 @@ function run()
> >   return 2
> >   fi
> >   
> > +if [ -n "$machine" ] && [ -n "$MACHINE" ] && [ "$machine" != 
> > "$MACHINE" ]; then
> > +print_result "SKIP" $testname "" "$machine only"
> > +return 2
> > +elif [ -n "$MACHINE" ]; then
> > +machine="$MACHINE"
> > +fi
> > +
> >   if [ -n "$accel" ] && [ -n "$ACCEL" ] && [ "$accel" != "$ACCEL" ]; 
> > then
> >   print_result "SKIP" $testname "" "$accel only, but ACCEL=$ACCEL"
> 

Re: [PATCH v6] arch/powerpc/kvm: Add support for reading VPA counters for pseries guests

2024-05-08 Thread Nicholas Piggin
On Tue May 7, 2024 at 12:56 AM AEST, Gautam Menghani wrote:
> PAPR hypervisor has introduced three new counters in the VPA area of
> LPAR CPUs for KVM L2 guest (see [1] for terminology) observability - 2
> for context switches from host to guest and vice versa, and 1 counter
> for getting the total time spent inside the KVM guest. Add a tracepoint
> that enables reading the counters for use by ftrace/perf. Note that this
> tracepoint is only available for nestedv2 API (i.e, KVM on PowerVM).
>
> [1] Terminology:
> a. L1 refers to the VM (LPAR) booted on top of PAPR hypervisor
> b. L2 refers to the KVM guest booted on top of L1.
>
> Signed-off-by: Vaibhav Jain 
> Signed-off-by: Gautam Menghani 
> ---
> v5 -> v6:
> 1. Use TRACE_EVENT_FN to enable/disable counters only once.
> 2. Remove the agg. counters from vcpu->arch.
> 3. Use PACA to maintain old counter values instead of zeroing on every
> entry.
> 4. Simplify variable names
>
> v4 -> v5:
> 1. Define helper functions for getting/setting the accumulation counter
> in L2's VPA
>
> v3 -> v4:
> 1. After vcpu_run, check the VPA flag instead of checking for tracepoint
> being enabled for disabling the cs time accumulation.
>
> v2 -> v3:
> 1. Move the counter disabling and zeroing code to a different function.
> 2. Move the get_lppaca() inside the tracepoint_enabled() branch.
> 3. Add the aggregation logic to maintain total context switch time.
>
> v1 -> v2:
> 1. Fix the build error due to invalid struct member reference.
>
>  arch/powerpc/include/asm/lppaca.h | 11 +--
>  arch/powerpc/include/asm/paca.h   |  5 +++
>  arch/powerpc/kvm/book3s_hv.c  | 52 +++
>  arch/powerpc/kvm/trace_hv.h   | 27 
>  4 files changed, 92 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/lppaca.h 
> b/arch/powerpc/include/asm/lppaca.h
> index 61ec2447dabf..f40a646bee3c 100644
> --- a/arch/powerpc/include/asm/lppaca.h
> +++ b/arch/powerpc/include/asm/lppaca.h
> @@ -62,7 +62,8 @@ struct lppaca {
>   u8  donate_dedicated_cpu;   /* Donate dedicated CPU cycles */
>   u8  fpregs_in_use;
>   u8  pmcregs_in_use;
> - u8  reserved8[28];
> + u8  l2_counters_enable;  /* Enable usage of counters for KVM guest 
> */
> + u8  reserved8[27];
>   __be64  wait_state_cycles;  /* Wait cycles for this proc */
>   u8  reserved9[28];
>   __be16  slb_count;  /* # of SLBs to maintain */
> @@ -92,9 +93,13 @@ struct lppaca {
>   /* cacheline 4-5 */
>  
>   __be32  page_ins;   /* CMO Hint - # page ins by OS */
> - u8  reserved12[148];
> + u8  reserved12[28];
> + volatile __be64 l1_to_l2_cs_tb;
> + volatile __be64 l2_to_l1_cs_tb;
> + volatile __be64 l2_runtime_tb;

I wonder if we shouldn't be moving over to use READ_ONCE for these
instead of volatile.

Probably doesn't really matter here. Maybe general audit of volatiles
in arch/powerpc could be something to put in linuxppc issues.

> + u8 reserved13[96];
>   volatile __be64 dtl_idx;/* Dispatch Trace Log head index */
> - u8  reserved13[96];
> + u8  reserved14[96];
>  } cacheline_aligned;
>  
>  #define lppaca_of(cpu)   (*paca_ptrs[cpu]->lppaca_ptr)
> diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
> index 1d58da946739..f20ac7a6efa4 100644
> --- a/arch/powerpc/include/asm/paca.h
> +++ b/arch/powerpc/include/asm/paca.h
> @@ -278,6 +278,11 @@ struct paca_struct {
>   struct mce_info *mce_info;
>   u8 mce_pending_irq_work;
>  #endif /* CONFIG_PPC_BOOK3S_64 */
> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> + u64 l1_to_l2_cs;
> + u64 l2_to_l1_cs;
> + u64 l2_runtime_agg;
> +#endif
>  } cacheline_aligned;

I don't think these really need to be in the paca.

paca is for per-cpu stuff that is accessed in real mode, early interrupt
entry, etc.

>  
>  extern void copy_mm_to_paca(struct mm_struct *mm);
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 8e86eb577eb8..ed69ad58bd02 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -4108,6 +4108,54 @@ static void vcpu_vpa_increment_dispatch(struct 
> kvm_vcpu *vcpu)
>   }
>  }
>  
> +static inline int kvmhv_get_l2_counters_status(void)
> +{
> + return get_lppaca()->l2_counters_enable;
> +}
> +
> +static inline void kvmhv_set_l2_counters_status(int cpu, bool status)
> +{
> + if (status)
> + lppaca_of(cpu).l2_counters_enable = 1;
> + else
> + lppaca_of(cpu).l2_counters_enable = 0;
> +}
> +
> +int kmvhv_counters_tracepoint_regfunc(void)
> +{
> + int cpu;
> +
> + for_each_possible_cpu(cpu) {
> + kvmhv_set_l2_counters_status(cpu, true);
> + }
> + return 0;
> +}
> +
> +void kmvhv_counters_tracepoint_unregfunc(void)
> +{
> + int cpu;
> +
> + for_each_possible_cpu(cpu) {
> + 

Re: [PATCH v15 00/16] Add audio support in v4l2 framework

2024-05-08 Thread Amadeusz Sławiński

On 5/8/2024 10:00 AM, Hans Verkuil wrote:

On 06/05/2024 10:49, Shengjiu Wang wrote:

On Fri, May 3, 2024 at 4:42 PM Mauro Carvalho Chehab  wrote:


Em Fri, 3 May 2024 10:47:19 +0900
Mark Brown  escreveu:


On Thu, May 02, 2024 at 10:26:43AM +0100, Mauro Carvalho Chehab wrote:

Mauro Carvalho Chehab  escreveu:



There are still time control associated with it, as audio and video
needs to be in sync. This is done by controlling the buffers size
and could be fine-tuned by checking when the buffer transfer is done.


...


Just complementing: on media, we do this per video buffer (or
per half video buffer). A typical use case on cameras is to have
buffers transferred 30 times per second, if the video was streamed
at 30 frames per second.


IIRC some big use case for this hardware was transcoding so there was a
desire to just go at whatever rate the hardware could support as there
is no interactive user consuming the output as it is generated.


Indeed, codecs could be used to just do transcoding, but I would
expect it to be a border use case. See, as the chipsets implementing
codecs are typically the ones used on mobiles, I would expect that
the major use cases to be to watch audio and video and to participate
on audio/video conferences.

Going further, the codec API may end supporting not only transcoding
(which is something that CPU can usually handle without too much
processing) but also audio processing that may require more
complex algorithms - even deep learning ones - like background noise
removal, echo detection/removal, volume auto-gain, audio enhancement
and such.

On other words, the typical use cases will either have input
or output being a physical hardware (microphone or speaker).



All, thanks for spending time to discuss, it seems we go back to
the start point of this topic again.

Our main request is that there is a hardware sample rate converter
on the chip, so users can use it in user space as a component like
software sample rate converter. It mostly may run as a gstreamer plugin.
so it is a memory to memory component.

I didn't find such API in ALSA for such purpose, the best option for this
in the kernel is the V4L2 memory to memory framework I found.
As Hans said it is well designed for memory to memory.

And I think audio is one of 'media'.  As I can see that part of Radio
function is in ALSA, part of Radio function is in V4L2. part of HDMI
function is in DRM, part of HDMI function is in ALSA...
So using V4L2 for audio is not new from this point of view.

Even now I still think V4L2 is the best option, but it looks like there
are a lot of rejects.  If develop a new ALSA-mem2mem, it is also
a duplication of code (bigger duplication that just add audio support
in V4L2 I think).


After reading this thread I still believe that the mem2mem framework is
a reasonable option, unless someone can come up with a method that is
easy to implement in the alsa subsystem. From what I can tell from this
discussion no such method exists.



Hi,

my main question would be how is mem2mem use case different from 
loopback exposing playback and capture frontends in user space with DSP 
(or other piece of HW) in the middle?


Amadeusz



[PATCH bpf v2] powerpc/bpf: enforce full ordering for ATOMIC operations with BPF_FETCH

2024-05-08 Thread Puranjay Mohan
The Linux Kernel Memory Model [1][2] requires RMW operations that have a
return value to be fully ordered.

BPF atomic operations with BPF_FETCH (including BPF_XCHG and
BPF_CMPXCHG) return a value back so they need to be JITed to fully
ordered operations. POWERPC currently emits relaxed operations for
these.

We can show this by running the following litmus-test:

PPC SB+atomic_add+fetch

{
0:r0=x;  (* dst reg assuming offset is 0 *)
0:r1=2;  (* src reg *)
0:r2=1;
0:r4=y;  (* P0 writes to this, P1 reads this *)
0:r5=z;  (* P1 writes to this, P0 reads this *)
0:r6=0;

1:r2=1;
1:r4=y;
1:r5=z;
}

P0  | P1;
stw r2, 0(r4)   | stw  r2,0(r5) ;
|   ;
loop:lwarx  r3, r6, r0  |   ;
mr  r8, r3  |   ;
add r3, r3, r1  | sync  ;
stwcx.  r3, r6, r0  |   ;
bne loop|   ;
mr  r1, r8  |   ;
|   ;
lwa r7, 0(r5)   | lwa  r7,0(r4) ;

~exists(0:r7=0 /\ 1:r7=0)

Witnesses
Positive: 9 Negative: 3
Condition ~exists (0:r7=0 /\ 1:r7=0)
Observation SB+atomic_add+fetch Sometimes 3 9

This test shows that the older store in P0 is reordered with a newer
load to a different address. Although there is a RMW operation with
fetch between them. Adding a sync before and after RMW fixes the issue:

Witnesses
Positive: 9 Negative: 0
Condition ~exists (0:r7=0 /\ 1:r7=0)
Observation SB+atomic_add+fetch Never 0 9

[1] https://www.kernel.org/doc/Documentation/memory-barriers.txt
[2] https://www.kernel.org/doc/Documentation/atomic_t.txt

Fixes: 65112709115f ("powerpc/bpf/64: add support for BPF_ATOMIC bitwise 
operations")
Signed-off-by: Puranjay Mohan 
---
Changes in v1 -> v2:
v1: https://lore.kernel.org/all/20240507175439.119467-1-puran...@kernel.org/
- Don't emit `sync` for non-SMP kernels as that adds unessential overhead.
---
 arch/powerpc/net/bpf_jit_comp32.c | 12 
 arch/powerpc/net/bpf_jit_comp64.c | 12 
 2 files changed, 24 insertions(+)

diff --git a/arch/powerpc/net/bpf_jit_comp32.c 
b/arch/powerpc/net/bpf_jit_comp32.c
index 2f39c50ca729..0318b83f2e6a 100644
--- a/arch/powerpc/net/bpf_jit_comp32.c
+++ b/arch/powerpc/net/bpf_jit_comp32.c
@@ -853,6 +853,15 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, 
u32 *fimage, struct code
/* Get offset into TMP_REG */
EMIT(PPC_RAW_LI(tmp_reg, off));
tmp_idx = ctx->idx * 4;
+   /*
+* Enforce full ordering for operations with BPF_FETCH 
by emitting a 'sync'
+* before and after the operation.
+*
+* This is a requirement in the Linux Kernel Memory 
Model.
+* See __cmpxchg_u64() in asm/cmpxchg.h as an example.
+*/
+   if (imm & BPF_FETCH && IS_ENABLED(CONFIG_SMP))
+   EMIT(PPC_RAW_SYNC());
/* load value from memory into r0 */
EMIT(PPC_RAW_LWARX(_R0, tmp_reg, dst_reg, 0));
 
@@ -905,6 +914,9 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, u32 
*fimage, struct code
 
/* For the BPF_FETCH variant, get old data into src_reg 
*/
if (imm & BPF_FETCH) {
+   /* Emit 'sync' to enforce full ordering */
+   if (IS_ENABLED(CONFIG_SMP))
+   EMIT(PPC_RAW_SYNC());
EMIT(PPC_RAW_MR(ret_reg, ax_reg));
if (!fp->aux->verifier_zext)
EMIT(PPC_RAW_LI(ret_reg - 1, 0)); /* 
higher 32-bit */
diff --git a/arch/powerpc/net/bpf_jit_comp64.c 
b/arch/powerpc/net/bpf_jit_comp64.c
index 79f23974a320..9a077f8acf7b 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -804,6 +804,15 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, 
u32 *fimage, struct code
/* Get offset into TMP_REG_1 */
EMIT(PPC_RAW_LI(tmp1_reg, off));
tmp_idx = ctx->idx * 4;
+   /*
+* Enforce full ordering for operations with BPF_FETCH 
by emitting a 'sync'
+* before and after the operation.
+*
+* This is a requirement in the Linux Kernel Memory 
Model.
+* See __cmpxchg_u64() in asm/cmpxchg.h as an example.
+*/
+   if (imm & BPF_FETCH && IS_ENABLED(CONFIG_SMP))
+   EMIT(PPC_RAW_SYNC());
/* load value from memory into TMP_REG_2 */
if (size == BPF_DW)
   

Re: [PATCH v3 00/11] sysctl: treewide: constify ctl_table argument of sysctl handlers

2024-05-08 Thread Joel Granados
On Fri, May 03, 2024 at 04:09:40PM +0200, Thomas Weißschuh wrote:
> Hey Joel,
> 
...
> > # Motivation
> > As I read it, the motivation for these constification efforts are:
> > 1. It provides increased safety: Having things in .rodata section reduces 
> > the
> >attack surface. This is especially relevant for structures that have 
> > function
> >pointers (like ctl_table); having these in .rodata means that these 
> > pointers
> >always point to the "intended" function and cannot be changed.
> > 2. Compiler optimizations: This was just a comment in the patchsets that I 
> > have
> >mentioned ([3,4,5]). Do you know what optimizations specifically? Does it
> >have to do with enhancing locality for the data in .rodata? Do you have 
> > other
> >specific optimizations in mind?
> 
> I don't know about anything that would make it faster.
> It's more about safety and transmission of intent to API users,
> especially callback implementers.
Noted.

...
> > # Show the move
> > I created [8] because there is no easy way to validate which objects made it
> > into .rodata. I ran [8] for your Dec 2nd patcheset [7] and there are less in
> > .rodata than I expected (the results are in [9]) Why is that? Is it 
> > something
> > that has not been posted to the lists yet? 
> 
> Constifying the APIs only *allows* the actual table to be constified
> themselves.
> Then each table definition will have to be touched and "const" added.
That is what I thought. thx for clarifying.

> 
> See patches 17 and 18 in [7] for two examples.
> 
> Some tables in net/ are already "const" as the static definitions are
> never registered themselves but only their copies are.
> 
...

best

-- 

Joel Granados


signature.asc
Description: PGP signature


Re: [PATCH bpf] powerpc/bpf: enforce full ordering for ATOMIC operations with BPF_FETCH

2024-05-08 Thread Puranjay Mohan
Michael Ellerman  writes:

> Puranjay Mohan  writes:
>> The Linux Kernel Memory Model [1][2] requires RMW operations that have a
>> return value to be fully ordered.
>>
>> BPF atomic operations with BPF_FETCH (including BPF_XCHG and
>> BPF_CMPXCHG) return a value back so they need to be JITed to fully
>> ordered operations. POWERPC currently emits relaxed operations for
>> these.
>
> Thanks for catching this.
>
>> diff --git a/arch/powerpc/net/bpf_jit_comp32.c 
>> b/arch/powerpc/net/bpf_jit_comp32.c
>> index 2f39c50ca729..b635e5344e8a 100644
>> --- a/arch/powerpc/net/bpf_jit_comp32.c
>> +++ b/arch/powerpc/net/bpf_jit_comp32.c
>> @@ -853,6 +853,15 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, 
>> u32 *fimage, struct code
>>  /* Get offset into TMP_REG */
>>  EMIT(PPC_RAW_LI(tmp_reg, off));
>>  tmp_idx = ctx->idx * 4;
>> +/*
>> + * Enforce full ordering for operations with BPF_FETCH 
>> by emitting a 'sync'
>> + * before and after the operation.
>> + *
>> + * This is a requirement in the Linux Kernel Memory 
>> Model.
>> + * See __cmpxchg_u64() in asm/cmpxchg.h as an example.
>> + */
>> +if (imm & BPF_FETCH)
>> +EMIT(PPC_RAW_SYNC());
>>  /* load value from memory into r0 */
>>  EMIT(PPC_RAW_LWARX(_R0, tmp_reg, dst_reg, 0));
>>  
>> @@ -905,6 +914,8 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, 
>> u32 *fimage, struct code
>>  
>>  /* For the BPF_FETCH variant, get old data into src_reg 
>> */
>>  if (imm & BPF_FETCH) {
>> +/* Emit 'sync' to enforce full ordering */
>> +EMIT(PPC_RAW_SYNC());
>>  EMIT(PPC_RAW_MR(ret_reg, ax_reg));
>>  if (!fp->aux->verifier_zext)
>>  EMIT(PPC_RAW_LI(ret_reg - 1, 0)); /* 
>> higher 32-bit */
>
> On 32-bit there are non-SMP systems where those syncs will probably be 
> expensive.
>
> I think just adding an IS_ENABLED(CONFIG_SMP) around the syncs is
> probably sufficient. Christophe?

Yes, I should do it for both 32-bit and 64-bit because the kernel does
that as well:

In POWERPC __atomic_pre/post_full_fence resolves to 'sync' in case of
CONFIG_SMP and barrier() in case of !CONFIG_SMP.

barrier() is not relevant for JITs as it is used at compile time.

So, I will use

if (IS_ENABLED(CONFIG_SMP))
EMIT(PPC_RAW_SYNC());

in the next version.


Thanks,
Puranjay


Re: [PATCH v3 00/11] sysctl: treewide: constify ctl_table argument of sysctl handlers

2024-05-08 Thread Joel Granados
Kees

Could you comment on the feasibility of this alternative from the
Control Flow Integrity perspective. My proposal is to change the
proc_handler to void* and back in the same release. So there would not
be a kernel released with a void* proc_handler.

> > However, there is an alternative way to do this that allows chunking. We
> > first define the proc_handler as a void pointer (casting it where it is
> > being used) [1]. Then we could do the constification by subsystem (like
> > Jakub proposes). Finally we can "revert the void pointer change so we
> > don't have one size fit all pointer as our proc_handler [2].
> > 
> > Here are some comments about the alternative:
> > 1. We would need to make the first argument const in all the derived
> >proc_handlers [3] 
> > 2. There would be no undefined behavior for two reasons:
> >2.1. There is no case where we change the first argument. We know
> > this because there are no compile errors after we make it const.
> >2.2. We would always go from non-const to const. This is the case
> > because all the stuff that is unchanged in non-const.
> > 3. If the idea sticks, it should go into mainline as one patchset. I
> >would not like to have a void* proc_handler in a kernel release.
> > 4. I think this is a "win/win" solution were the constification goes
> >through and it is divided in such a way that it is reviewable.
> > 
> > I would really like to hear what ppl think about this "heretic"
> > alternative. @Thomas, @Luis, @Kees @Jakub?
> 
> Thanks for that alternative, I'm not a big fan though.
> 
> Besides the wonky syntax, Control Flow Integrity should trap on
> this construct. Functions are called through different pointers than
> their actual types which is exactly what CFI is meant to prevent.
> 
> Maybe people find it easier to review when using
> "--word-diff" and/or "-U0" with git diff/show.
> There is really nothing going an besides adding a few "const"s.
> 
> But if the consensus prefers this solution, I'll be happy to adopt it.
> 
> > [1] 
> > https://git.kernel.org/pub/scm/linux/kernel/git/joel.granados/linux.git/commit/?h=jag/constfy_treewide_alternative=4a383503b1ea650d4e12c1f5838974e879f5aa6f
> > [2] 
> > https://git.kernel.org/pub/scm/linux/kernel/git/joel.granados/linux.git/commit/?h=jag/constfy_treewide_alternative=a3be65973d27ec2933b9e81e1bec60be3a9b460d
> > [3] proc_dostring, proc_dobool, proc_dointvec
> 
> 
> Thomas

Best
-- 

Joel Granados


signature.asc
Description: PGP signature


Re: WARNING: CPU: 1 PID: 1 at net/core/netpoll.c:370 netpoll_send_skb+0x1fc/0x20c at boot when netconsole is enabled (kernel v6.9-rc5, v6.8.7, sungem, PowerMac G4 DP)

2024-05-08 Thread Erhard Furtner
On Mon, 6 May 2024 18:10:20 -0700
Jakub Kicinski  wrote:

> Excellent! Do you want to submit that as an official patch?
> The explanation is that we can't call disable_irq() from atomic
> context (which which netpoll runs). But the callback is no longer
> necessary as we can depend on NAPI to do the polling these days.

I could do that with the explanation you stated. But should any further 
questions arise in this process I would also lack the technical background to 
deal with them. ;)

I also noticed a similar #ifdef CONFIG_NET_POLL_CONTROLLER logic shows up in
many network drivers, e.g. net/ethernet/realtek/8139too.c:

#ifdef CONFIG_NET_POLL_CONTROLLER
static void rtl8139_poll_controller(struct net_device *dev);
#endif
[...]
#ifdef CONFIG_NET_POLL_CONTROLLER
/*
 * Polling receive - used by netconsole and other diagnostic tools
 * to allow network i/o with interrupts disabled.
 */
static void rtl8139_poll_controller(struct net_device *dev)
{
struct rtl8139_private *tp = netdev_priv(dev);
const int irq = tp->pci_dev->irq;

disable_irq_nosync(irq);
rtl8139_interrupt(irq, dev);
enable_irq(irq);
}
#endif
[...]
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller= rtl8139_poll_controller,
#endif


Should it be removed here too? This would be more cards I can test. So far I 
only see this on my G4 and I think something similar on an old Pentium4 box I 
no longer have. 
 
> > What I still get with 'modprobe -v dev_addr_lists_test', even with 
> > gem_poll_controller() removed is:
> > 
> > [...]
> > KTAP version 1
> > 1..1
> > KTAP version 1
> > # Subtest: dev-addr-list-test
> > # module: dev_addr_lists_test
> > 1..6
> > 
> > 
> > WARNING: kunit_try_catch/1770 still has locks held!
> > 6.9.0-rc6-PMacG4-dirty #5 Tainted: GWN
> > 
> > 1 lock held by kunit_try_catch/1770:
> >  #0: c0dbfce4 (rtnl_mutex){}-{3:3}, at: dev_addr_test_init+0xbc/0xc8 
> > [dev_addr_lists_test]  
> 
> I think that's fixed in net-next.

Ah, good to hear!

Regards,
Erhard F.


[PATCH][next] selftests/powerpc/dexcr: Fix spelling mistake "predicition" -> "prediction"

2024-05-08 Thread Colin Ian King
There is a spelling mistake in the help message. Fix it.

Signed-off-by: Colin Ian King 
---
 tools/testing/selftests/powerpc/dexcr/chdexcr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/powerpc/dexcr/chdexcr.c 
b/tools/testing/selftests/powerpc/dexcr/chdexcr.c
index bda44630cada..c548d7a5bb9b 100644
--- a/tools/testing/selftests/powerpc/dexcr/chdexcr.c
+++ b/tools/testing/selftests/powerpc/dexcr/chdexcr.c
@@ -26,7 +26,7 @@ static void help(void)
   "\n"
   "The normal option sets the aspect in the DEXCR. The --no- 
variant\n"
   "clears that aspect. For example, --ibrtpd sets the IBRTPD 
aspect bit,\n"
-  "so indirect branch predicition will be disabled in the provided 
program.\n"
+  "so indirect branch prediction will be disabled in the provided 
program.\n"
   "Conversely, --no-ibrtpd clears the aspect bit, so indirect 
branch\n"
   "prediction may occur.\n"
   "\n"
-- 
2.39.2



Re: [PATCH v2 1/1] x86/elf: Add a new .note section containing Xfeatures information to x86 core files

2024-05-08 Thread Kees Cook
On Tue, May 07, 2024 at 03:23:31PM +0530, Vignesh Balasubramanian wrote:
> Add a new .note section containing type, size, offset and flags of
> every xfeature that is present.
> 
> This information will be used by the debuggers to understand the XSAVE
> layout of the machine where the core file is dumped, and to read XSAVE
> registers, especially during cross-platform debugging.
> 
> Some background:
> 
> The XSAVE layouts of modern AMD and Intel CPUs differ, especially since
> Memory Protection Keys and the AVX-512 features have been inculcated into
> the AMD CPUs.
> This is since AMD never adopted (and hence never left room in the XSAVE
> layout for) the Intel MPX feature. Tools like GDB had assumed a fixed XSAVE
> layout matching that of Intel (based on the XCR0 mask).
> Hence, the core dumps from AMD CPUs didn't match the known size for the
> XCR0 mask. This resulted in GDB and other tools not being able to access
> the values of the AVX-512 and PKRU registers on AMD CPUs.
> To solve this, an interim solution has been accepted into GDB, and is
> already a part of GDB 14, thanks to these series of patches
> [ https://sourceware.org/pipermail/gdb-patches/2023-March/198081.html ].
> But this patch series depends on heuristics based on the total XSAVE
> register set size and the XCR0 mask to infer the layouts of the various
> register blocks for core dumps, and hence, is not a foolproof mechanism to
> determine the layout of the XSAVE area.
> 
> Hence this new core dump note has been proposed as a more sturdy mechanism
> to allow GDB/LLDB and other relevant tools to determine the layout of the
> XSAVE area of the machine where the corefile was dumped.
> The new core dump note (which is being proposed as a per-process .note
> section), NT_X86_XSAVE_LAYOUT (0x205) contains an array of structures.
> Each structure describes an individual extended feature containing offset,
> size and flags (that is obtained through CPUID instruction) in a format
> roughly matching the follow C structure:
> 
> struct xfeat_component {
>u32 xfeat_type;
>u32 xfeat_sz;
>u32 xfeat_off;
>u32 xfeat_flags;
> };
> 
> Co-developed-by: Jini Susan George 
> Signed-off-by: Jini Susan George 
> Signed-off-by: Vignesh Balasubramanian 
> ---
> v1->v2: Removed kernel internal defn dependency, code improvements
> 
>  arch/x86/Kconfig |   1 +
>  arch/x86/include/asm/elf.h   |  34 +
>  arch/x86/kernel/fpu/xstate.c | 141 +++
>  fs/binfmt_elf.c  |   4 +-
>  include/uapi/linux/elf.h |   1 +
>  5 files changed, 179 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 928820e61cb5..cc67daab3396 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -105,6 +105,7 @@ config X86
>   select ARCH_HAS_DEBUG_WX
>   select ARCH_HAS_ZONE_DMA_SET if EXPERT
>   select ARCH_HAVE_NMI_SAFE_CMPXCHG
> + select ARCH_HAVE_EXTRA_ELF_NOTES
>   select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE
>   select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
>   select ARCH_MIGHT_HAVE_PC_PARPORT
> diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
> index 1fb83d47711f..5952574db64b 100644
> --- a/arch/x86/include/asm/elf.h
> +++ b/arch/x86/include/asm/elf.h
> @@ -13,6 +13,40 @@
>  #include 
>  #include 
>  
> +struct xfeat_component {
> + u32 xfeat_type;
> + u32 xfeat_sz;
> + u32 xfeat_off;
> + u32 xfeat_flags;
> +} __packed;
> +
> +_Static_assert(sizeof(struct xfeat_component)%4 == 0, "xfeat_component is 
> not aligned");
> +
> +enum custom_feature {
> + FEATURE_XSAVE_FP = 0,
> + FEATURE_XSAVE_SSE = 1,
> + FEATURE_XSAVE_YMM = 2,
> + FEATURE_XSAVE_BNDREGS = 3,
> + FEATURE_XSAVE_BNDCSR = 4,
> + FEATURE_XSAVE_OPMASK = 5,
> + FEATURE_XSAVE_ZMM_Hi256 = 6,
> + FEATURE_XSAVE_Hi16_ZMM = 7,
> + FEATURE_XSAVE_PT = 8,
> + FEATURE_XSAVE_PKRU = 9,
> + FEATURE_XSAVE_PASID = 10,
> + FEATURE_XSAVE_CET_USER = 11,
> + FEATURE_XSAVE_CET_SHADOW_STACK = 12,
> + FEATURE_XSAVE_HDC = 13,
> + FEATURE_XSAVE_UINTR = 14,
> + FEATURE_XSAVE_LBR = 15,
> + FEATURE_XSAVE_HWP = 16,
> + FEATURE_XSAVE_XTILE_CFG = 17,
> + FEATURE_XSAVE_XTILE_DATA = 18,
> + FEATURE_MAX,
> + FEATURE_XSAVE_EXTENDED_START = FEATURE_XSAVE_YMM,
> + FEATURE_XSAVE_EXTENDED_END = FEATURE_XSAVE_XTILE_DATA,
> +};
> +
>  typedef unsigned long elf_greg_t;
>  
>  #define ELF_NGREG (sizeof(struct user_regs_struct) / sizeof(elf_greg_t))
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index 33a214b1a4ce..3d1c3c96e34d 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -87,6 +88,8 @@ static unsigned int xstate_flags[XFEATURE_MAX] 
> __ro_after_init;
>  #define XSTATE_FLAG_SUPERVISOR   BIT(0)
>  #define 

Re: [PATCH v15 00/16] Add audio support in v4l2 framework

2024-05-08 Thread Hans Verkuil
On 06/05/2024 10:49, Shengjiu Wang wrote:
> On Fri, May 3, 2024 at 4:42 PM Mauro Carvalho Chehab  
> wrote:
>>
>> Em Fri, 3 May 2024 10:47:19 +0900
>> Mark Brown  escreveu:
>>
>>> On Thu, May 02, 2024 at 10:26:43AM +0100, Mauro Carvalho Chehab wrote:
 Mauro Carvalho Chehab  escreveu:
>>>
> There are still time control associated with it, as audio and video
> needs to be in sync. This is done by controlling the buffers size
> and could be fine-tuned by checking when the buffer transfer is done.
>>>
>>> ...
>>>
 Just complementing: on media, we do this per video buffer (or
 per half video buffer). A typical use case on cameras is to have
 buffers transferred 30 times per second, if the video was streamed
 at 30 frames per second.
>>>
>>> IIRC some big use case for this hardware was transcoding so there was a
>>> desire to just go at whatever rate the hardware could support as there
>>> is no interactive user consuming the output as it is generated.
>>
>> Indeed, codecs could be used to just do transcoding, but I would
>> expect it to be a border use case. See, as the chipsets implementing
>> codecs are typically the ones used on mobiles, I would expect that
>> the major use cases to be to watch audio and video and to participate
>> on audio/video conferences.
>>
>> Going further, the codec API may end supporting not only transcoding
>> (which is something that CPU can usually handle without too much
>> processing) but also audio processing that may require more
>> complex algorithms - even deep learning ones - like background noise
>> removal, echo detection/removal, volume auto-gain, audio enhancement
>> and such.
>>
>> On other words, the typical use cases will either have input
>> or output being a physical hardware (microphone or speaker).
>>
> 
> All, thanks for spending time to discuss, it seems we go back to
> the start point of this topic again.
> 
> Our main request is that there is a hardware sample rate converter
> on the chip, so users can use it in user space as a component like
> software sample rate converter. It mostly may run as a gstreamer plugin.
> so it is a memory to memory component.
> 
> I didn't find such API in ALSA for such purpose, the best option for this
> in the kernel is the V4L2 memory to memory framework I found.
> As Hans said it is well designed for memory to memory.
> 
> And I think audio is one of 'media'.  As I can see that part of Radio
> function is in ALSA, part of Radio function is in V4L2. part of HDMI
> function is in DRM, part of HDMI function is in ALSA...
> So using V4L2 for audio is not new from this point of view.
> 
> Even now I still think V4L2 is the best option, but it looks like there
> are a lot of rejects.  If develop a new ALSA-mem2mem, it is also
> a duplication of code (bigger duplication that just add audio support
> in V4L2 I think).

After reading this thread I still believe that the mem2mem framework is
a reasonable option, unless someone can come up with a method that is
easy to implement in the alsa subsystem. From what I can tell from this
discussion no such method exists.

>From the media side there are arguments that it adds extra maintenance
load, which is true, but I believe that it is quite limited in practice.

That said, perhaps we should make a statement that while we support the
use of audio m2m drivers, this is only for simple m2m audio processing like
this driver, specifically where there is a 1-to-1 mapping between input and
output buffers. At this point we do not want to add audio codec support or
similar complex audio processing.

Part of the reason is that codecs are hard, and we already have our hands
full with all the video codecs. Part of the reason is that the v4l2-mem2mem
framework probably needs to be forked to make a more advanced version geared
towards codecs since the current framework is too limiting for some of the
things we want to do. It was really designed for scalers, deinterlacers, etc.
and the codec support was added later.

If we ever allow such complex audio processing devices, then we would have
to have another discussion, and I believe that will only be possible if
most of the maintenance load would be on the alsa subsystem where the audio
experts are.

So my proposal is to:

1) add a clear statement to dev-audio-mem2mem.rst (patch 08/16) that only
   simple audio devices with a 1-to-1 mapping of input to output buffer are
   supported. Perhaps also in videodev2.h before struct v4l2_audio_format.

2) I will experiment a bit trying to solve the main complaint about creating
   new audio fourcc values and thus duplicating existing SNDRV_PCM_FORMAT_
   values. I have some ideas for that.

But I do not want to spend time on 2 until we agree that this is the way
forward.

Regards,

Hans