Re: next/master bisection: baseline.login on ox820-cloudengines-pogoplug-series-3

Re: next/master bisection: baseline.login on ox820-cloudengines-pogoplug-series-3

[Guillaume Tucker]

Please see the bisection report below about a kernel panic.

Reports aren't automatically sent to the public while we're
trialing new bisection features on kernelci.org but this one
looks valid.

See the kernel Oops due to a NULL pointer followed by a panic:

  https://storage.kernelci.org/next/master/next-20200618/arm/oxnas_v6_defconfig/gcc-8/lab-baylibre/baseline-ox820-cloudengines-pogoplug-series-3.html#L504

Thanks,
Guillaume


On 18/06/2020 13:20, kernelci.org bot wrote:
> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> * This automated bisection report was sent to you on the basis  *
> * that you may be involved with the breaking commit it has      *
> * found.  No manual investigation has been done to verify it,   *
> * and the root cause of the problem may be somewhere else.      *
> *                                                               *
> * If you do send a fix, please include this trailer:            *
> *   Reported-by: "kernelci.org bot" <bot@kernelci.org>          *
> *                                                               *
> * Hope this helps!                                              *
> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> 
> next/master bisection: baseline.login on ox820-cloudengines-pogoplug-series-3
> 
> Summary:
>   Start:      ce2cc8efd7a4 Add linux-next specific files for 20200618
>   Plain log:  https://storage.kernelci.org/next/master/next-20200618/arm/oxnas_v6_defconfig/gcc-8/lab-baylibre/baseline-ox820-cloudengines-pogoplug-series-3.txt
>   HTML log:   https://storage.kernelci.org/next/master/next-20200618/arm/oxnas_v6_defconfig/gcc-8/lab-baylibre/baseline-ox820-cloudengines-pogoplug-series-3.html
>   Result:     7b929258ff0e mtd: rawnand: Allocate the interface configurations dynamically
> 
> Checks:
>   revert:     PASS
>   verify:     PASS
> 
> Parameters:
>   Tree:       next
>   URL:        https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>   Branch:     master
>   Target:     ox820-cloudengines-pogoplug-series-3
>   CPU arch:   arm
>   Lab:        lab-baylibre
>   Compiler:   gcc-8
>   Config:     oxnas_v6_defconfig
>   Test case:  baseline.login
> 
> Breaking commit found:
> 
> -------------------------------------------------------------------------------
> commit 7b929258ff0e913616e21661a757f5ecb776d337
> Author: Miquel Raynal <miquel.raynal@bootlin.com>
> Date:   Fri May 29 13:13:22 2020 +0200
> 
>     mtd: rawnand: Allocate the interface configurations dynamically
>     
>     Instead of manipulating the statically allocated structure and copy
>     timings around, allocate one at identification time and save it in the
>     nand_chip structure once it has been initialized.
>     
>     All NAND chips using the same interface configuration during reset and
>     startup, we define a helper to retrieve a single reset interface
>     configuration object, shared across all NAND chips.
>     
>     We use a second pointer to always have a reference on the currently
>     applied interface configuration, which may either point to the "best
>     interface configuration" or to the "default reset interface
>     configuration".
>     
>     Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>     Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
>     Link: https://lore.kernel.org/linux-mtd/20200529111322.7184-29-miquel.raynal@bootlin.com
> 
> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> index 5ebfbb89e572..012876e14317 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -93,6 +93,7 @@ onfi_find_closest_sdr_mode(const struct nand_sdr_timings *spec_timings);
>  int nand_choose_best_sdr_timings(struct nand_chip *chip,
>  				 struct nand_interface_config *iface,
>  				 struct nand_sdr_timings *spec_timings);
> +const struct nand_interface_config *nand_get_reset_interface_config(void);
>  int nand_get_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
>  int nand_set_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
>  int nand_read_page_raw_notsupp(struct nand_chip *chip, u8 *buf,
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 753328f106c1..4a0d486210e9 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -928,9 +928,9 @@ static int nand_reset_interface(struct nand_chip *chip, int chipnr)
>  	 * timings to timing mode 0.
>  	 */
>  
> -	onfi_fill_interface_config(chip, &chip->interface_config,
> -				   NAND_SDR_IFACE, 0);
> -	ret = ops->setup_interface(chip, chipnr, &chip->interface_config);
> +	chip->current_interface_config = nand_get_reset_interface_config();
> +	ret = ops->setup_interface(chip, chipnr,
> +				   chip->current_interface_config);
>  	if (ret)
>  		pr_err("Failed to configure data interface to SDR timing mode 0\n");
>  
> @@ -949,7 +949,8 @@ static int nand_reset_interface(struct nand_chip *chip, int chipnr)
>   */
>  static int nand_setup_interface(struct nand_chip *chip, int chipnr)
>  {
> -	u8 mode = chip->interface_config.timings.mode;
> +	const struct nand_controller_ops *ops = chip->controller->ops;
> +	u8 mode = chip->best_interface_config->timings.mode;
>  	u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = { mode, };
>  	int ret;
>  
> @@ -967,14 +968,13 @@ static int nand_setup_interface(struct nand_chip *chip, int chipnr)
>  	}
>  
>  	/* Change the mode on the controller side */
> -	ret = chip->controller->ops->setup_interface(chip, chipnr,
> -						     &chip->interface_config);
> +	ret = ops->setup_interface(chip, chipnr, chip->best_interface_config);
>  	if (ret)
>  		return ret;
>  
>  	/* Check the mode has been accepted by the chip, if supported */
>  	if (!nand_supports_get_features(chip, ONFI_FEATURE_ADDR_TIMING_MODE))
> -		return 0;
> +		goto update_interface_config;
>  
>  	memset(tmode_param, 0, ONFI_SUBFEATURE_PARAM_LEN);
>  	nand_select_target(chip, chipnr);
> @@ -990,6 +990,9 @@ static int nand_setup_interface(struct nand_chip *chip, int chipnr)
>  		goto err_reset_chip;
>  	}
>  
> +update_interface_config:
> +	chip->current_interface_config = chip->best_interface_config;
> +
>  	return 0;
>  
>  err_reset_chip:
> @@ -1031,8 +1034,10 @@ int nand_choose_best_sdr_timings(struct nand_chip *chip,
>  		/* Verify the controller supports the requested interface */
>  		ret = ops->setup_interface(chip, NAND_DATA_IFACE_CHECK_ONLY,
>  					   iface);
> -		if (!ret)
> +		if (!ret) {
> +			chip->best_interface_config = iface;
>  			return ret;
> +		}
>  
>  		/* Fallback to slower modes */
>  		best_mode = iface->timings.mode;
> @@ -1046,9 +1051,11 @@ int nand_choose_best_sdr_timings(struct nand_chip *chip,
>  		ret = ops->setup_interface(chip, NAND_DATA_IFACE_CHECK_ONLY,
>  					   iface);
>  		if (!ret)
> -			return 0;
> +			break;
>  	}
>  
> +	chip->best_interface_config = iface;
> +
>  	return 0;
>  }
>  
> @@ -1067,15 +1074,25 @@ int nand_choose_best_sdr_timings(struct nand_chip *chip,
>   */
>  static int nand_choose_interface_config(struct nand_chip *chip)
>  {
> +	struct nand_interface_config *iface;
> +	int ret;
> +
>  	if (!nand_controller_can_setup_interface(chip))
>  		return 0;
>  
> +	iface = kzalloc(sizeof(*iface), GFP_KERNEL);
> +	if (!iface)
> +		return -ENOMEM;
> +
>  	if (chip->ops.choose_interface_config)
> -		return chip->ops.choose_interface_config(chip,
> -							 &chip->interface_config);
> +		ret = chip->ops.choose_interface_config(chip, iface);
> +	else
> +		ret = nand_choose_best_sdr_timings(chip, iface, NULL);
>  
> -	return nand_choose_best_sdr_timings(chip, &chip->interface_config,
> -					    NULL);
> +	if (ret)
> +		kfree(iface);
> +
> +	return ret;
>  }
>  
>  /**
> @@ -2501,7 +2518,6 @@ EXPORT_SYMBOL_GPL(nand_subop_get_data_len);
>   */
>  int nand_reset(struct nand_chip *chip, int chipnr)
>  {
> -	struct nand_interface_config saved_intf_config = chip->interface_config;
>  	int ret;
>  
>  	ret = nand_reset_interface(chip, chipnr);
> @@ -2526,11 +2542,9 @@ int nand_reset(struct nand_chip *chip, int chipnr)
>  	 * nand_setup_interface() uses ->set/get_features() which would
>  	 * fail anyway as the parameter page is not available yet.
>  	 */
> -	if (!memcmp(&chip->interface_config, &saved_intf_config,
> -		    sizeof(saved_intf_config)))
> +	if (!chip->best_interface_config)
>  		return 0;
>  
> -	chip->interface_config = saved_intf_config;
>  	ret = nand_setup_interface(chip, chipnr);
>  	if (ret)
>  		return ret;
> @@ -5198,7 +5212,7 @@ static int nand_scan_ident(struct nand_chip *chip, unsigned int maxchips,
>  	mutex_init(&chip->lock);
>  
>  	/* Enforce the right timings for reset/detection */
> -	onfi_fill_interface_config(chip, &chip->interface_config, NAND_SDR_IFACE, 0);
> +	chip->current_interface_config = nand_get_reset_interface_config();
>  
>  	ret = nand_dt_init(chip);
>  	if (ret)
> @@ -5994,7 +6008,7 @@ static int nand_scan_tail(struct nand_chip *chip)
>  	for (i = 0; i < nanddev_ntargets(&chip->base); i++) {
>  		ret = nand_setup_interface(chip, i);
>  		if (ret)
> -			goto err_nanddev_cleanup;
> +			goto err_free_interface_config;
>  	}
>  
>  	/* Check, if we should skip the bad block table scan */
> @@ -6004,10 +6018,12 @@ static int nand_scan_tail(struct nand_chip *chip)
>  	/* Build bad block table */
>  	ret = nand_create_bbt(chip);
>  	if (ret)
> -		goto err_nanddev_cleanup;
> +		goto err_free_interface_config;
>  
>  	return 0;
>  
> +err_free_interface_config:
> +	kfree(chip->best_interface_config);
>  
>  err_nanddev_cleanup:
>  	nanddev_cleanup(&chip->base);
> @@ -6101,6 +6117,9 @@ void nand_cleanup(struct nand_chip *chip)
>  			& NAND_BBT_DYNAMICSTRUCT)
>  		kfree(chip->badblock_pattern);
>  
> +	/* Free the data interface */
> +	kfree(chip->best_interface_config);
> +
>  	/* Free manufacturer priv data. */
>  	nand_manufacturer_cleanup(chip);
>  
> diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
> index 1e22006c79ba..94d832646487 100644
> --- a/drivers/mtd/nand/raw/nand_timings.c
> +++ b/drivers/mtd/nand/raw/nand_timings.c
> @@ -292,6 +292,12 @@ static const struct nand_interface_config onfi_sdr_timings[] = {
>  	},
>  };
>  
> +/* All NAND chips share the same reset data interface: SDR mode 0 */
> +const struct nand_interface_config *nand_get_reset_interface_config(void)
> +{
> +	return &onfi_sdr_timings[0];
> +}
> +
>  /**
>   * onfi_find_closest_sdr_mode - Derive the closest ONFI SDR timing mode given a
>   *                              set of timings
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index a2427c67d38b..a725b620aca2 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1069,7 +1069,11 @@ struct nand_manufacturer {
>   * @options: Various chip options. They can partly be set to inform nand_scan
>   *           about special functionality. See the defines for further
>   *           explanation.
> - * @interface_config: NAND interface timing information
> + * @current_interface_config: The currently used NAND interface configuration
> + * @best_interface_config: The best NAND interface configuration which fits both
> + *                         the NAND chip and NAND controller constraints. If
> + *                         unset, the default reset interface configuration must
> + *                         be used.
>   * @bbt_erase_shift: Number of address bits in a bbt entry
>   * @bbt_options: Bad block table specific options. All options used here must
>   *               come from bbm.h. By default, these options will be copied to
> @@ -1116,7 +1120,8 @@ struct nand_chip {
>  	unsigned int options;
>  
>  	/* Data interface */
> -	struct nand_interface_config interface_config;
> +	const struct nand_interface_config *current_interface_config;
> +	struct nand_interface_config *best_interface_config;
>  
>  	/* Bad block information */
>  	unsigned int bbt_erase_shift;
> @@ -1209,7 +1214,7 @@ static inline struct device_node *nand_get_flash_node(struct nand_chip *chip)
>  static inline const struct nand_interface_config *
>  nand_get_interface_config(struct nand_chip *chip)
>  {
> -	return &chip->interface_config;
> +	return chip->current_interface_config;
>  }
>  
>  /*
> -------------------------------------------------------------------------------
> 
> 
> Git bisection log:
> 
> -------------------------------------------------------------------------------
> git bisect start
> # good: [1b5044021070efa3259f3e9548dc35d1eb6aa844] Merge tag 'dma-mapping-5.8-3' of git://git.infradead.org/users/hch/dma-mapping
> git bisect good 1b5044021070efa3259f3e9548dc35d1eb6aa844
> # bad: [ce2cc8efd7a40cbd17841add878cb691d0ce0bba] Add linux-next specific files for 20200618
> git bisect bad ce2cc8efd7a40cbd17841add878cb691d0ce0bba
> # bad: [b91d047b9cb0441237354c3b4fd40fb5a6e649ef] next-20200616/amdgpu
> git bisect bad b91d047b9cb0441237354c3b4fd40fb5a6e649ef
> # good: [ff028acf8c34f3707946fc806b8873ddb95e0f9a] Merge remote-tracking branch 'hid/for-next'
> git bisect good ff028acf8c34f3707946fc806b8873ddb95e0f9a
> # good: [5073201f3b80c6d798519589a81263563b4f8520] drm/amdgpu: add internal reg offset translation for VCN inst 1
> git bisect good 5073201f3b80c6d798519589a81263563b4f8520
> # good: [f3cdec463796c25ab829e5959488f269812fa1bb] Merge remote-tracking branch 'gfs2/for-next'
> git bisect good f3cdec463796c25ab829e5959488f269812fa1bb
> # good: [1134d52d6f95ae61fbd2da980a29f05c656c5934] Revert "drm/[radeon|amdgpu]: Replace one-element array and use struct_size() helper"
> git bisect good 1134d52d6f95ae61fbd2da980a29f05c656c5934
> # good: [3b6588fa6f92297dc26681686fd3c719647576c4] mtd: rawnand: toshiba: Choose the interface configuration for TH58NVG2S3HBAI4
> git bisect good 3b6588fa6f92297dc26681686fd3c719647576c4
> # bad: [97bd0ed43524a3530d5651627779c0b40385187e] mtd: rawnand: qcom: set BAM mode only if not set already
> git bisect bad 97bd0ed43524a3530d5651627779c0b40385187e
> # bad: [ebbd1aae9deeccf8c2adcd84dd536b451110bf07] mtd: rawnand: fsl_upm: Use gpio descriptors
> git bisect bad ebbd1aae9deeccf8c2adcd84dd536b451110bf07
> # bad: [d8a826cfb059b2d5d4e97ced9469d6bcf7cf40f7] mtd: rawnand: fsl_upm: Get rid of the unused fsl_upm_nand.parts field
> git bisect bad d8a826cfb059b2d5d4e97ced9469d6bcf7cf40f7
> # bad: [7b929258ff0e913616e21661a757f5ecb776d337] mtd: rawnand: Allocate the interface configurations dynamically
> git bisect bad 7b929258ff0e913616e21661a757f5ecb776d337
> # good: [a82f691ad48c63af22844cee4eb3107cf5d9ff00] mtd: rawnand: Get rid of the default ONFI timing mode
> git bisect good a82f691ad48c63af22844cee4eb3107cf5d9ff00
> # first bad commit: [7b929258ff0e913616e21661a757f5ecb776d337] mtd: rawnand: Allocate the interface configurations dynamically
> -------------------------------------------------------------------------------
> 

Re: next/master bisection: baseline.login on ox820-cloudengines-pogoplug-series-3

[Miquel Raynal]

Hi Guillaume,

Guillaume Tucker <guillaume.tucker@collabora.com> wrote on Thu, 18 Jun
2020 13:28:05 +0100:

> Please see the bisection report below about a kernel panic.
> 
> Reports aren't automatically sent to the public while we're
> trialing new bisection features on kernelci.org but this one
> looks valid.
> 
> See the kernel Oops due to a NULL pointer followed by a panic:
> 
>   https://storage.kernelci.org/next/master/next-20200618/arm/oxnas_v6_defconfig/gcc-8/lab-baylibre/baseline-ox820-cloudengines-pogoplug-series-3.html#L504

Thanks for the report, I will not be able to manage it before Monday,
but I'll try to take care of it early next week.

Thanks for your patience,
Miquèl

Re: next/master bisection: baseline.login on ox820-cloudengines-pogoplug-series-3

[Miquel Raynal]

Hi Guillaume,

Miquel Raynal <miquel.raynal@bootlin.com> wrote on Thu, 18 Jun 2020
15:23:24 +0200:

> Hi Guillaume,
> 
> Guillaume Tucker <guillaume.tucker@collabora.com> wrote on Thu, 18 Jun
> 2020 13:28:05 +0100:
> 
> > Please see the bisection report below about a kernel panic.
> > 
> > Reports aren't automatically sent to the public while we're
> > trialing new bisection features on kernelci.org but this one
> > looks valid.
> > 
> > See the kernel Oops due to a NULL pointer followed by a panic:
> > 
> >   https://storage.kernelci.org/next/master/next-20200618/arm/oxnas_v6_defconfig/gcc-8/lab-baylibre/baseline-ox820-cloudengines-pogoplug-series-3.html#L504
> 
> Thanks for the report, I will not be able to manage it before Monday,
> but I'll try to take care of it early next week.

Actually Boris saw the issue, I just updated nand/next, it should be
part of tomorrow's linux-next. Could you please report if it fixes your
boot?

Thanks,
Miquèl

Re: next/master bisection: baseline.login on ox820-cloudengines-pogoplug-series-3

[Guillaume Tucker]

On 18/06/2020 15:09, Miquel Raynal wrote:
> Hi Guillaume,
> 
> Miquel Raynal <miquel.raynal@bootlin.com> wrote on Thu, 18 Jun 2020
> 15:23:24 +0200:
> 
>> Hi Guillaume,
>>
>> Guillaume Tucker <guillaume.tucker@collabora.com> wrote on Thu, 18 Jun
>> 2020 13:28:05 +0100:
>>
>>> Please see the bisection report below about a kernel panic.
>>>
>>> Reports aren't automatically sent to the public while we're
>>> trialing new bisection features on kernelci.org but this one
>>> looks valid.
>>>
>>> See the kernel Oops due to a NULL pointer followed by a panic:
>>>
>>>   https://storage.kernelci.org/next/master/next-20200618/arm/oxnas_v6_defconfig/gcc-8/lab-baylibre/baseline-ox820-cloudengines-pogoplug-series-3.html#L504
>>
>> Thanks for the report, I will not be able to manage it before Monday,
>> but I'll try to take care of it early next week.
> 
> Actually Boris saw the issue, I just updated nand/next, it should be
> part of tomorrow's linux-next. Could you please report if it fixes your
> boot?

Sure, will check tomorrow.  Thanks for the update.

We may also consider adding the nand/next branch to kernelci.org
and catch issues earlier.  We can discuss that separately.

Guillaume

Re: next/master bisection: baseline.login on ox820-cloudengines-pogoplug-series-3

[Boris Brezillon]

On Thu, 18 Jun 2020 15:23:45 +0100
Guillaume Tucker <guillaume.tucker@collabora.com> wrote:

> On 18/06/2020 15:09, Miquel Raynal wrote:
> > Hi Guillaume,
> > 
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Thu, 18 Jun 2020
> > 15:23:24 +0200:
> >   
> >> Hi Guillaume,
> >>
> >> Guillaume Tucker <guillaume.tucker@collabora.com> wrote on Thu, 18 Jun
> >> 2020 13:28:05 +0100:
> >>  
> >>> Please see the bisection report below about a kernel panic.
> >>>
> >>> Reports aren't automatically sent to the public while we're
> >>> trialing new bisection features on kernelci.org but this one
> >>> looks valid.
> >>>
> >>> See the kernel Oops due to a NULL pointer followed by a panic:
> >>>
> >>>   https://storage.kernelci.org/next/master/next-20200618/arm/oxnas_v6_defconfig/gcc-8/lab-baylibre/baseline-ox820-cloudengines-pogoplug-series-3.html#L504  
> >>
> >> Thanks for the report, I will not be able to manage it before Monday,
> >> but I'll try to take care of it early next week.  
> > 
> > Actually Boris saw the issue, I just updated nand/next, it should be
> > part of tomorrow's linux-next. Could you please report if it fixes your
> > boot?  
> 
> Sure, will check tomorrow.  Thanks for the update.
> 
> We may also consider adding the nand/next branch to kernelci.org
> and catch issues earlier.  We can discuss that separately.

If you're testing linux-next that shouldn't help us much (nand/next is
pulled in linux-next already), but maybe it's just about making the
bisection easier for kernelci (less commits to inspect), in which case
that's probably a good idea. You might also want to add mtd/next,
spi-nor/next and cfi/next so the entire MTD subsystem gets tested.

Re: next/master bisection: baseline.login on ox820-cloudengines-pogoplug-series-3

[Guillaume Tucker]

Hi Ard,

Please see the bisection report below about a boot failure on
ox820-cloudengines-pogoplug-series-3.  There was also a bisection
yesterday with next-20201216 which landed on the same commit, on
the same platform and also with oxnas_v6_defconfig.  I'm not
aware of any other platform on kernelci.org showing the same
regression.

Hope this helps!

Best wishes,
Guillaume

On 18/12/2020 10:51, KernelCI bot wrote:
> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> * This automated bisection report was sent to you on the basis  *
> * that you may be involved with the breaking commit it has      *
> * found.  No manual investigation has been done to verify it,   *
> * and the root cause of the problem may be somewhere else.      *
> *                                                               *
> * If you do send a fix, please include this trailer:            *
> *   Reported-by: "kernelci.org bot" <bot@kernelci.org>          *
> *                                                               *
> * Hope this helps!                                              *
> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> 
> next/master bisection: baseline.login on ox820-cloudengines-pogoplug-series-3
> 
> Summary:
>   Start:      90cc8cf2d1ab Add linux-next specific files for 20201217
>   Plain log:  https://storage.kernelci.org/next/master/next-20201217/arm/oxnas_v6_defconfig/gcc-8/lab-baylibre/baseline-ox820-cloudengines-pogoplug-series-3.txt
>   HTML log:   https://storage.kernelci.org/next/master/next-20201217/arm/oxnas_v6_defconfig/gcc-8/lab-baylibre/baseline-ox820-cloudengines-pogoplug-series-3.html
>   Result:     f77ac2e378be ARM: 9030/1: entry: omit FP emulation for UND exceptions taken in kernel mode
> 
> Checks:
>   revert:     PASS
>   verify:     PASS
> 
> Parameters:
>   Tree:       next
>   URL:        https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>   Branch:     master
>   Target:     ox820-cloudengines-pogoplug-series-3
>   CPU arch:   arm
>   Lab:        lab-baylibre
>   Compiler:   gcc-8
>   Config:     oxnas_v6_defconfig
>   Test case:  baseline.login
> 
> Breaking commit found:
> 
> -------------------------------------------------------------------------------
> commit f77ac2e378be9dd61eb88728f0840642f045d9d1
> Author: Ard Biesheuvel <ardb@kernel.org>
> Date:   Thu Nov 19 18:09:16 2020 +0100
> 
>     ARM: 9030/1: entry: omit FP emulation for UND exceptions taken in kernel mode
>     
>     There are a couple of problems with the exception entry code that deals
>     with FP exceptions (which are reported as UND exceptions) when building
>     the kernel in Thumb2 mode:
>     - the conditional branch to vfp_kmode_exception in vfp_support_entry()
>       may be out of range for its target, depending on how the linker decides
>       to arrange the sections;
>     - when the UND exception is taken in kernel mode, the emulation handling
>       logic is entered via the 'call_fpe' label, which means we end up using
>       the wrong value/mask pairs to match and detect the NEON opcodes.
>     
>     Since UND exceptions in kernel mode are unlikely to occur on a hot path
>     (as opposed to the user mode version which is invoked for VFP support
>     code and lazy restore), we can use the existing undef hook machinery for
>     any kernel mode instruction emulation that is needed, including calling
>     the existing vfp_kmode_exception() routine for unexpected cases. So drop
>     the call to call_fpe, and instead, install an undef hook that will get
>     called for NEON and VFP instructions that trigger an UND exception in
>     kernel mode.
>     
>     While at it, make sure that the PC correction is accurate for the
>     execution mode where the exception was taken, by checking the PSR
>     Thumb bit.
>     
>     Cc: Dmitry Osipenko <digetx@gmail.com>
>     Cc: Kees Cook <keescook@chromium.org>
>     Fixes: eff8728fe698 ("vmlinux.lds.h: Add PGO and AutoFDO input sections")
>     Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>     Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>     Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>     Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> 
> diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
> index c4220f51fcf3..0ea8529a4872 100644
> --- a/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -252,31 +252,10 @@ __und_svc:
>  #else
>  	svc_entry
>  #endif
> -	@
> -	@ call emulation code, which returns using r9 if it has emulated
> -	@ the instruction, or the more conventional lr if we are to treat
> -	@ this as a real undefined instruction
> -	@
> -	@  r0 - instruction
> -	@
> -#ifndef CONFIG_THUMB2_KERNEL
> -	ldr	r0, [r4, #-4]
> -#else
> -	mov	r1, #2
> -	ldrh	r0, [r4, #-2]			@ Thumb instruction at LR - 2
> -	cmp	r0, #0xe800			@ 32-bit instruction if xx >= 0
> -	blo	__und_svc_fault
> -	ldrh	r9, [r4]			@ bottom 16 bits
> -	add	r4, r4, #2
> -	str	r4, [sp, #S_PC]
> -	orr	r0, r9, r0, lsl #16
> -#endif
> -	badr	r9, __und_svc_finish
> -	mov	r2, r4
> -	bl	call_fpe
>  
>  	mov	r1, #4				@ PC correction to apply
> -__und_svc_fault:
> + THUMB(	tst	r5, #PSR_T_BIT		)	@ exception taken in Thumb mode?
> + THUMB(	movne	r1, #2			)	@ if so, fix up PC correction
>  	mov	r0, sp				@ struct pt_regs *regs
>  	bl	__und_fault
>  
> diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S
> index 4fcff9f59947..d5837bf05a9a 100644
> --- a/arch/arm/vfp/vfphw.S
> +++ b/arch/arm/vfp/vfphw.S
> @@ -79,11 +79,6 @@ ENTRY(vfp_support_entry)
>  	DBGSTR3	"instr %08x pc %08x state %p", r0, r2, r10
>  
>  	.fpu	vfpv2
> -	ldr	r3, [sp, #S_PSR]	@ Neither lazy restore nor FP exceptions
> -	and	r3, r3, #MODE_MASK	@ are supported in kernel mode
> -	teq	r3, #USR_MODE
> -	bne	vfp_kmode_exception	@ Returns through lr
> -
>  	VFPFMRX	r1, FPEXC		@ Is the VFP enabled?
>  	DBGSTR1	"fpexc %08x", r1
>  	tst	r1, #FPEXC_EN
> diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
> index 8c9e7f9f0277..c3b6451c18bd 100644
> --- a/arch/arm/vfp/vfpmodule.c
> +++ b/arch/arm/vfp/vfpmodule.c
> @@ -23,6 +23,7 @@
>  #include <asm/cputype.h>
>  #include <asm/system_info.h>
>  #include <asm/thread_notify.h>
> +#include <asm/traps.h>
>  #include <asm/vfp.h>
>  
>  #include "vfpinstr.h"
> @@ -642,7 +643,9 @@ static int vfp_starting_cpu(unsigned int unused)
>  	return 0;
>  }
>  
> -void vfp_kmode_exception(void)
> +#ifdef CONFIG_KERNEL_MODE_NEON
> +
> +static int vfp_kmode_exception(struct pt_regs *regs, unsigned int instr)
>  {
>  	/*
>  	 * If we reach this point, a floating point exception has been raised
> @@ -660,9 +663,51 @@ void vfp_kmode_exception(void)
>  		pr_crit("BUG: unsupported FP instruction in kernel mode\n");
>  	else
>  		pr_crit("BUG: FP instruction issued in kernel mode with FP unit disabled\n");
> +	pr_crit("FPEXC == 0x%08x\n", fmrx(FPEXC));
> +	return 1;
>  }
>  
> -#ifdef CONFIG_KERNEL_MODE_NEON
> +static struct undef_hook vfp_kmode_exception_hook[] = {{
> +	.instr_mask	= 0xfe000000,
> +	.instr_val	= 0xf2000000,
> +	.cpsr_mask	= MODE_MASK | PSR_T_BIT,
> +	.cpsr_val	= SVC_MODE,
> +	.fn		= vfp_kmode_exception,
> +}, {
> +	.instr_mask	= 0xff100000,
> +	.instr_val	= 0xf4000000,
> +	.cpsr_mask	= MODE_MASK | PSR_T_BIT,
> +	.cpsr_val	= SVC_MODE,
> +	.fn		= vfp_kmode_exception,
> +}, {
> +	.instr_mask	= 0xef000000,
> +	.instr_val	= 0xef000000,
> +	.cpsr_mask	= MODE_MASK | PSR_T_BIT,
> +	.cpsr_val	= SVC_MODE | PSR_T_BIT,
> +	.fn		= vfp_kmode_exception,
> +}, {
> +	.instr_mask	= 0xff100000,
> +	.instr_val	= 0xf9000000,
> +	.cpsr_mask	= MODE_MASK | PSR_T_BIT,
> +	.cpsr_val	= SVC_MODE | PSR_T_BIT,
> +	.fn		= vfp_kmode_exception,
> +}, {
> +	.instr_mask	= 0x0c000e00,
> +	.instr_val	= 0x0c000a00,
> +	.cpsr_mask	= MODE_MASK,
> +	.cpsr_val	= SVC_MODE,
> +	.fn		= vfp_kmode_exception,
> +}};
> +
> +static int __init vfp_kmode_exception_hook_init(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(vfp_kmode_exception_hook); i++)
> +		register_undef_hook(&vfp_kmode_exception_hook[i]);
> +	return 0;
> +}
> +core_initcall(vfp_kmode_exception_hook_init);
>  
>  /*
>   * Kernel-side NEON support functions
> -------------------------------------------------------------------------------
> 
> 
> Git bisection log:
> 
> -------------------------------------------------------------------------------
> git bisect start
> # good: [31f6551ad75608d9c71fd4d3548c33f1abc52093] cifs: handle "guest" mount parameter
> git bisect good 31f6551ad75608d9c71fd4d3548c33f1abc52093
> # bad: [90cc8cf2d1ab87d708ebc311ac104ccbbefad9fc] Add linux-next specific files for 20201217
> git bisect bad 90cc8cf2d1ab87d708ebc311ac104ccbbefad9fc
> # good: [3db1a3fa98808aa90f95ec3e0fa2fc7abf28f5c9] Merge tag 'staging-5.11-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging
> git bisect good 3db1a3fa98808aa90f95ec3e0fa2fc7abf28f5c9
> # bad: [4538b80cd2885c885e3bf06e5103bc0f214c11ee] Merge remote-tracking branch 'arm-soc/for-next'
> git bisect bad 4538b80cd2885c885e3bf06e5103bc0f214c11ee
> # good: [5ee863bec794f30bdf7fdf57ce0d9f579b0d1aa3] Merge branch 'parisc-5.11-1' of git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux
> git bisect good 5ee863bec794f30bdf7fdf57ce0d9f579b0d1aa3
> # good: [fb10b701887b9fce88aacc06ca4b6c9160ca7a66] Merge branch 'arm/dt' into for-next
> git bisect good fb10b701887b9fce88aacc06ca4b6c9160ca7a66
> # good: [821a8a63a6b55ee5325e07bf5ab7d7a1752009d1] Merge remote-tracking branch 'asm-generic/master'
> git bisect good 821a8a63a6b55ee5325e07bf5ab7d7a1752009d1
> # good: [7debceff46eef5350bf2b62494c7772f78b22257] Merge branch 'arm/dt' into for-next
> git bisect good 7debceff46eef5350bf2b62494c7772f78b22257
> # bad: [b383a1412e0df59c556abb93c1fa2399e91b6102] Merge remote-tracking branch 'arm64/for-next/core'
> git bisect bad b383a1412e0df59c556abb93c1fa2399e91b6102
> # good: [113eb4ce4fc33ef3deda1431497811d43342c0cc] Merge branch 'for-next/iommu/vt-d' into for-next/iommu/core
> git bisect good 113eb4ce4fc33ef3deda1431497811d43342c0cc
> # skip: [eb86b15a2c53e2263b7cbad00a0100c8451989dc] Merge branches 'fixes' and 'misc' into for-next
> git bisect skip eb86b15a2c53e2263b7cbad00a0100c8451989dc
> # skip: [2a50d6b9cfe96ef5d39b9c3497dcc83a907d00cd] ARM: 9033/1: arm/smp: Drop the macro S(x,s)
> git bisect skip 2a50d6b9cfe96ef5d39b9c3497dcc83a907d00cd
> # good: [421015713b306e47af95d4d61cdfbd96d462e4cb] ARM: 9017/2: Enable KASan for ARM
> git bisect good 421015713b306e47af95d4d61cdfbd96d462e4cb
> # good: [2c736bb4087f2cb949cbbaf4148733131b8466dc] Merge tag 'arm-adrl-replacement-for-v5.11' of git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux into devel-stable
> git bisect good 2c736bb4087f2cb949cbbaf4148733131b8466dc
> # skip: [3597a10e4c444f3d4d6a64bdc98e4585a99301b9] ARM: 9036/1: uncompress: Fix dbgadtb size parameter name
> git bisect skip 3597a10e4c444f3d4d6a64bdc98e4585a99301b9
> # skip: [1205285c7a71c3b7a879e31ec477639f2dac22b1] ARM: 9027/1: head.S: explicitly map DT even if it lives in the first physical section
> git bisect skip 1205285c7a71c3b7a879e31ec477639f2dac22b1
> # good: [71fe89ceb55bc0a85121b757cc8d3fb6ff457263] dma-iommu: remove __iommu_dma_mmap
> git bisect good 71fe89ceb55bc0a85121b757cc8d3fb6ff457263
> # skip: [17fb1a208129e3472ec45be08cbe39601a407e2c] ARM: 9031/1: hyp-stub: remove unused .L__boot_cpu_mode_offset symbol
> git bisect skip 17fb1a208129e3472ec45be08cbe39601a407e2c
> # good: [95d1718c961e49cfa02995ce1587a22cd7e2fbf5] Merge remote-tracking branch 'arm64/for-next/iommu/core' into for-next/core
> git bisect good 95d1718c961e49cfa02995ce1587a22cd7e2fbf5
> # skip: [0fe88ade3f937d6efc276901a56be6575a5a2171] ARM: 9032/1: arm/mm: Convert PUD level pgtable helper macros into functions
> git bisect skip 0fe88ade3f937d6efc276901a56be6575a5a2171
> # good: [730b5764ea8526e48bdb85a24ed96d62de435940] ARM: 9024/1: Drop useless cast of "u64" to "long long"
> git bisect good 730b5764ea8526e48bdb85a24ed96d62de435940
> # good: [4d576cab16f57e1f87978f6997a725179398341e] ARM: 9028/1: disable KASAN in call stack capturing routines
> git bisect good 4d576cab16f57e1f87978f6997a725179398341e
> # skip: [3bdf1a7503d61d2241f93f36e9c0d8a0d444bc5c] ARM: 9037/1: uncompress: Add OF_DT_MAGIC macro
> git bisect skip 3bdf1a7503d61d2241f93f36e9c0d8a0d444bc5c
> # bad: [f77ac2e378be9dd61eb88728f0840642f045d9d1] ARM: 9030/1: entry: omit FP emulation for UND exceptions taken in kernel mode
> git bisect bad f77ac2e378be9dd61eb88728f0840642f045d9d1
> # good: [3c9f5708b7aed6a963e2aefccbd1854802de163e] ARM: 9029/1: Make iwmmxt.S support Clang's integrated assembler
> git bisect good 3c9f5708b7aed6a963e2aefccbd1854802de163e
> # first bad commit: [f77ac2e378be9dd61eb88728f0840642f045d9d1] ARM: 9030/1: entry: omit FP emulation for UND exceptions taken in kernel mode
> -------------------------------------------------------------------------------
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#4773): https://groups.io/g/kernelci-results/message/4773
> Mute This Topic: https://groups.io/mt/79040317/924702
> Group Owner: kernelci-results+owner@groups.io
> Unsubscribe: https://groups.io/g/kernelci-results/unsub [guillaume.tucker@collabora.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 
> 

Re: next/master bisection: baseline.login on ox820-cloudengines-pogoplug-series-3

[Russell King - ARM Linux admin]

On Fri, Dec 18, 2020 at 01:48:09PM +0000, Guillaume Tucker wrote:
> Please see the bisection report below about a boot failure on
> ox820-cloudengines-pogoplug-series-3.  There was also a bisection
> yesterday with next-20201216 which landed on the same commit, on
> the same platform and also with oxnas_v6_defconfig.  I'm not
> aware of any other platform on kernelci.org showing the same
> regression.

Ah, I bet I know what's happening.

We test for the presence of VFP by issuing an instruction to read
FPSID. If VFP is not present, this will raise an undefined instruction
exception, and we expect to head into the vfp_testing_entry code.

I bet Pogoplug, being an ARM11 MPCore platform, either raises an
exception here.

We probably need to also rework the code in vfp_init() as well to
register a temporary hook when reading the FPSID.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: next/master bisection: baseline.login on ox820-cloudengines-pogoplug-series-3

[Ard Biesheuvel]

On Fri, 18 Dec 2020 at 15:00, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Fri, Dec 18, 2020 at 01:48:09PM +0000, Guillaume Tucker wrote:
> > Please see the bisection report below about a boot failure on
> > ox820-cloudengines-pogoplug-series-3.  There was also a bisection
> > yesterday with next-20201216 which landed on the same commit, on
> > the same platform and also with oxnas_v6_defconfig.  I'm not
> > aware of any other platform on kernelci.org showing the same
> > regression.
>
> Ah, I bet I know what's happening.
>
> We test for the presence of VFP by issuing an instruction to read
> FPSID. If VFP is not present, this will raise an undefined instruction
> exception, and we expect to head into the vfp_testing_entry code.
>
> I bet Pogoplug, being an ARM11 MPCore platform, either raises an
> exception here.
>
> We probably need to also rework the code in vfp_init() as well to
> register a temporary hook when reading the FPSID.
>

Thanks for diagnosing that - I wasn't quite sure what was going on.

I will look into this later today.

Re: next/master bisection: baseline.login on ox820-cloudengines-pogoplug-series-3

[Ard Biesheuvel]

On Fri, 18 Dec 2020 at 15:01, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 18 Dec 2020 at 15:00, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Fri, Dec 18, 2020 at 01:48:09PM +0000, Guillaume Tucker wrote:
> > > Please see the bisection report below about a boot failure on
> > > ox820-cloudengines-pogoplug-series-3.  There was also a bisection
> > > yesterday with next-20201216 which landed on the same commit, on
> > > the same platform and also with oxnas_v6_defconfig.  I'm not
> > > aware of any other platform on kernelci.org showing the same
> > > regression.
> >
> > Ah, I bet I know what's happening.
> >
> > We test for the presence of VFP by issuing an instruction to read
> > FPSID. If VFP is not present, this will raise an undefined instruction
> > exception, and we expect to head into the vfp_testing_entry code.
> >
> > I bet Pogoplug, being an ARM11 MPCore platform, either raises an
> > exception here.
> >
> > We probably need to also rework the code in vfp_init() as well to
> > register a temporary hook when reading the FPSID.
> >
>
> Thanks for diagnosing that - I wasn't quite sure what was going on.
>
> I will look into this later today.

Working again with my fix applied:

Re: next/master bisection: baseline.login on ox820-cloudengines-pogoplug-series-3

[Ard Biesheuvel]

On Fri, 18 Dec 2020 at 20:12, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 18 Dec 2020 at 15:01, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 18 Dec 2020 at 15:00, Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Fri, Dec 18, 2020 at 01:48:09PM +0000, Guillaume Tucker wrote:
> > > > Please see the bisection report below about a boot failure on
> > > > ox820-cloudengines-pogoplug-series-3.  There was also a bisection
> > > > yesterday with next-20201216 which landed on the same commit, on
> > > > the same platform and also with oxnas_v6_defconfig.  I'm not
> > > > aware of any other platform on kernelci.org showing the same
> > > > regression.
> > >
> > > Ah, I bet I know what's happening.
> > >
> > > We test for the presence of VFP by issuing an instruction to read
> > > FPSID. If VFP is not present, this will raise an undefined instruction
> > > exception, and we expect to head into the vfp_testing_entry code.
> > >
> > > I bet Pogoplug, being an ARM11 MPCore platform, either raises an
> > > exception here.
> > >
> > > We probably need to also rework the code in vfp_init() as well to
> > > register a temporary hook when reading the FPSID.
> > >
> >
> > Thanks for diagnosing that - I wasn't quite sure what was going on.
> >
> > I will look into this later today.
>
> Working again with my fix applied:

https://kernelci.org/test/plan/id/5fdcebf1a6350e2bd1c94cd3/

I'll drop it into the patch system tomorrow (unless there is a
pressing need to do it earlier)