Netdev List
 help / color / mirror / Atom feed
* [PATCH net 1/2] qed: Fix l2 initializations over iWARP personality
From: Michal Kalderon @ 2018-05-08 18:29 UTC (permalink / raw)
  To: michal.kalderon, davem
  Cc: netdev, linux-rdma, chad.dupuis, Michal Kalderon,
	Sudarsana Kalluru
In-Reply-To: <20180508182919.23590-1-Michal.Kalderon@cavium.com>

If qede driver was loaded on a device configured for iWARP
the l2 mutex wouldn't be allocated, and some l2 related
resources wouldn't be freed.

fixes: c851a9dc4359 ("qed: Introduce iWARP personality")
Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com>
Signed-off-by: Sudarsana Kalluru <Sudarsana.Kalluru@cavium.com>
---
 drivers/net/ethernet/qlogic/qed/qed_l2.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_l2.c b/drivers/net/ethernet/qlogic/qed/qed_l2.c
index e874504..8667799 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_l2.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_l2.c
@@ -115,8 +115,7 @@ int qed_l2_alloc(struct qed_hwfn *p_hwfn)
 
 void qed_l2_setup(struct qed_hwfn *p_hwfn)
 {
-	if (p_hwfn->hw_info.personality != QED_PCI_ETH &&
-	    p_hwfn->hw_info.personality != QED_PCI_ETH_ROCE)
+	if (!QED_IS_L2_PERSONALITY(p_hwfn))
 		return;
 
 	mutex_init(&p_hwfn->p_l2_info->lock);
@@ -126,8 +125,7 @@ void qed_l2_free(struct qed_hwfn *p_hwfn)
 {
 	u32 i;
 
-	if (p_hwfn->hw_info.personality != QED_PCI_ETH &&
-	    p_hwfn->hw_info.personality != QED_PCI_ETH_ROCE)
+	if (!QED_IS_L2_PERSONALITY(p_hwfn))
 		return;
 
 	if (!p_hwfn->p_l2_info)
-- 
2.9.5

^ permalink raw reply related

* [PATCH net 0/2] qed*: Rdma fixes
From: Michal Kalderon @ 2018-05-08 18:29 UTC (permalink / raw)
  To: michal.kalderon, davem
  Cc: netdev, linux-rdma, chad.dupuis, Michal Kalderon,
	Sudarsana Kalluru

This patch series include two fixes for bugs related to rdma.
The first has to do with loading the driver over an iWARP
device. 
The second fixes a previous commit that added proper link
indication for iWARP / RoCE.

Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com>
Signed-off-by: Sudarsana Kalluru <Sudarsana.Kalluru@cavium.com>

Michal Kalderon (2):
  qed: Fix l2 initializations over iWARP personality
  qede: Fix gfp flags sent to rdma event node allocation

 drivers/net/ethernet/qlogic/qed/qed_l2.c     | 6 ++----
 drivers/net/ethernet/qlogic/qede/qede_rdma.c | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

-- 
2.9.5

^ permalink raw reply

* Re: [PATCH] hv_netvsc: Fix net device attach on older Windows hosts
From: Mohammed Gamal @ 2018-05-08 18:17 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: netdev, sthemmin, haiyangz, linux-kernel, devel, vkuznets
In-Reply-To: <20180508111323.1767fc0c@xeon-e3>

On Tue, 2018-05-08 at 11:13 -0700, Stephen Hemminger wrote:
> On Tue,  8 May 2018 19:40:47 +0200
> Mohammed Gamal <mgamal@redhat.com> wrote:
> 
> > On older windows hosts the net_device instance is returned to
> > the caller of rndis_filter_device_add() without having the presence
> > bit set first. This would cause any subsequent calls to network
> > device
> > operations (e.g. MTU change, channel change) to fail after the
> > device
> > is detached once, returning -ENODEV.
> > 
> > Make sure we explicitly call netif_device_attach() before returning
> > the net_device instance to make sure the presence bit is set
> > 
> > Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
> > 
> > Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
> > ---
> >  drivers/net/hyperv/rndis_filter.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/hyperv/rndis_filter.c
> > b/drivers/net/hyperv/rndis_filter.c
> > index 6b127be..09a3c1d 100644
> > --- a/drivers/net/hyperv/rndis_filter.c
> > +++ b/drivers/net/hyperv/rndis_filter.c
> > @@ -1287,8 +1287,10 @@ struct netvsc_device
> > *rndis_filter_device_add(struct hv_device *dev,
> >  		   rndis_device->hw_mac_adr,
> >  		   rndis_device->link_state ? "down" : "up");
> >  
> > -	if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5)
> > +	if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5) {
> > +		netif_device_attach(net);
> >  		return net_device;
> > +	}
> 
> Yes, this looks right, but it might be easier to use goto existing
> exit
> path.
> 

I was just not sure if we should set max_chn and num_chn here. I will
modify the patch and resend.

> diff --git a/drivers/net/hyperv/rndis_filter.c
> b/drivers/net/hyperv/rndis_filter.c
> index 3b6dbacaf77d..ed941c5a0be9 100644
> --- a/drivers/net/hyperv/rndis_filter.c
> +++ b/drivers/net/hyperv/rndis_filter.c
> @@ -1316,7 +1316,7 @@ struct netvsc_device
> *rndis_filter_device_add(struct hv_device *dev,
>                    rndis_device->link_state ? "down" : "up");
>  
>         if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5)
> -               return net_device;
> +               goto out;
>  
>         rndis_filter_query_link_speed(rndis_device, net_device);
> 

^ permalink raw reply

* Re: [PATCH] hv_netvsc: Fix net device attach on older Windows hosts
From: Stephen Hemminger @ 2018-05-08 18:13 UTC (permalink / raw)
  To: Mohammed Gamal; +Cc: netdev, sthemmin, haiyangz, linux-kernel, devel, vkuznets
In-Reply-To: <1525801247-27765-1-git-send-email-mgamal@redhat.com>

On Tue,  8 May 2018 19:40:47 +0200
Mohammed Gamal <mgamal@redhat.com> wrote:

> On older windows hosts the net_device instance is returned to
> the caller of rndis_filter_device_add() without having the presence
> bit set first. This would cause any subsequent calls to network device
> operations (e.g. MTU change, channel change) to fail after the device
> is detached once, returning -ENODEV.
> 
> Make sure we explicitly call netif_device_attach() before returning
> the net_device instance to make sure the presence bit is set
> 
> Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
> 
> Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
> ---
>  drivers/net/hyperv/rndis_filter.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
> index 6b127be..09a3c1d 100644
> --- a/drivers/net/hyperv/rndis_filter.c
> +++ b/drivers/net/hyperv/rndis_filter.c
> @@ -1287,8 +1287,10 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
>  		   rndis_device->hw_mac_adr,
>  		   rndis_device->link_state ? "down" : "up");
>  
> -	if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5)
> +	if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5) {
> +		netif_device_attach(net);
>  		return net_device;
> +	}

Yes, this looks right, but it might be easier to use goto existing exit
path.

diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 3b6dbacaf77d..ed941c5a0be9 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -1316,7 +1316,7 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
                   rndis_device->link_state ? "down" : "up");
 
        if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5)
-               return net_device;
+               goto out;
 
        rndis_filter_query_link_speed(rndis_device, net_device);

^ permalink raw reply related

* [PATCH v6 13/13] Documentation: clarify firmware_class provenance and why we can't rename the module
From: Luis R. Rodriguez @ 2018-05-08 18:12 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc, davem
In-Reply-To: <20180508181247.19431-1-mcgrof@kernel.org>

Clarify the provenance of the firmware loader firmware_class module name
and why we cannot rename the module in the future.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 .../driver-api/firmware/fallback-mechanisms.rst          | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst b/Documentation/driver-api/firmware/fallback-mechanisms.rst
index a39323ef7d29..a8047be4a96e 100644
--- a/Documentation/driver-api/firmware/fallback-mechanisms.rst
+++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst
@@ -72,9 +72,12 @@ the firmware requested, and establishes it in the device hierarchy by
 associating the device used to make the request as the device's parent.
 The sysfs directory's file attributes are defined and controlled through
 the new device's class (firmware_class) and group (fw_dev_attr_groups).
-This is actually where the original firmware_class.c file name comes from,
-as originally the only firmware loading mechanism available was the
-mechanism we now use as a fallback mechanism.
+This is actually where the original firmware_class module name came from,
+given that originally the only firmware loading mechanism available was the
+mechanism we now use as a fallback mechanism, which which registers a
+struct class firmware_class. Because the attributes exposed are part of the
+module name, the module name firmware_class cannot be renamed in the future, to
+ensure backward compatibilty with old userspace.
 
 To load firmware using the sysfs interface we expose a loading indicator,
 and a file upload firmware into:
-- 
2.17.0

^ permalink raw reply related

* [PATCH v6 12/13] Documentation: remove stale firmware API reference
From: Luis R. Rodriguez @ 2018-05-08 18:12 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc, davem
In-Reply-To: <20180508181247.19431-1-mcgrof@kernel.org>

It refers to a pending patch, but this was merged eons ago.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 Documentation/dell_rbu.txt | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/Documentation/dell_rbu.txt b/Documentation/dell_rbu.txt
index 0fdb6aa2704c..077fdc29a0d0 100644
--- a/Documentation/dell_rbu.txt
+++ b/Documentation/dell_rbu.txt
@@ -121,9 +121,6 @@ read back the image downloaded.
 
 .. note::
 
-   This driver requires a patch for firmware_class.c which has the modified
-   request_firmware_nowait function.
-
    Also after updating the BIOS image a user mode application needs to execute
    code which sends the BIOS update request to the BIOS. So on the next reboot
    the BIOS knows about the new image downloaded and it updates itself.
-- 
2.17.0

^ permalink raw reply related

* [PATCH v6 11/13] Documentation: fix few typos and clarifications for the firmware loader
From: Luis R. Rodriguez @ 2018-05-08 18:12 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc, davem
In-Reply-To: <20180508181247.19431-1-mcgrof@kernel.org>

Fix a few typos, and clarify a few sentences.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 Documentation/driver-api/firmware/fallback-mechanisms.rst | 5 +++--
 Documentation/driver-api/firmware/firmware_cache.rst      | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst b/Documentation/driver-api/firmware/fallback-mechanisms.rst
index f353783ae0be..a39323ef7d29 100644
--- a/Documentation/driver-api/firmware/fallback-mechanisms.rst
+++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst
@@ -83,7 +83,7 @@ and a file upload firmware into:
   * /sys/$DEVPATH/data
 
 To upload firmware you will echo 1 onto the loading file to indicate
-you are loading firmware. You then cat the firmware into the data file,
+you are loading firmware. You then write the firmware into the data file,
 and you notify the kernel the firmware is ready by echo'ing 0 onto
 the loading file.
 
@@ -136,7 +136,8 @@ by kobject uevents. This is specially exacerbated due to the fact that most
 distributions today disable CONFIG_FW_LOADER_USER_HELPER_FALLBACK.
 
 Refer to do_firmware_uevent() for details of the kobject event variables
-setup. Variables passwdd with a kobject add event:
+setup. The variables currently passed to userspace with a "kobject add"
+event are:
 
 * FIRMWARE=firmware name
 * TIMEOUT=timeout value
diff --git a/Documentation/driver-api/firmware/firmware_cache.rst b/Documentation/driver-api/firmware/firmware_cache.rst
index 2210e5bfb332..c2e69d9c6bf1 100644
--- a/Documentation/driver-api/firmware/firmware_cache.rst
+++ b/Documentation/driver-api/firmware/firmware_cache.rst
@@ -29,8 +29,8 @@ Some implementation details about the firmware cache setup:
 * If an asynchronous call is used the firmware cache is only set up for a
   device if if the second argument (uevent) to request_firmware_nowait() is
   true. When uevent is true it requests that a kobject uevent be sent to
-  userspace for the firmware request. For details refer to the Fackback
-  mechanism documented below.
+  userspace for the firmware request through the sysfs fallback mechanism
+  if the firmware file is not found.
 
 * If the firmware cache is determined to be needed as per the above two
   criteria the firmware cache is setup by adding a devres entry for the
-- 
2.17.0

^ permalink raw reply related

* [PATCH v6 10/13] ath10k: re-enable the firmware fallback mechanism for testmode
From: Luis R. Rodriguez @ 2018-05-08 18:12 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc, davem
In-Reply-To: <20180508181247.19431-1-mcgrof@kernel.org>

From: Andres Rodriguez <andresx7@gmail.com>

The ath10k testmode uses request_firmware_direct() in order to avoid
producing firmware load warnings. Disabling the fallback mechanism was a
side effect of disabling warnings.

We now have a new API that allows us to avoid warnings while keeping the
fallback mechanism enabled. So use that instead.

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
Acked-by: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/net/wireless/ath/ath10k/testmode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/testmode.c b/drivers/net/wireless/ath/ath10k/testmode.c
index 568810b41657..c24ee616833c 100644
--- a/drivers/net/wireless/ath/ath10k/testmode.c
+++ b/drivers/net/wireless/ath/ath10k/testmode.c
@@ -157,7 +157,7 @@ static int ath10k_tm_fetch_utf_firmware_api_1(struct ath10k *ar,
 		 ar->hw_params.fw.dir, ATH10K_FW_UTF_FILE);
 
 	/* load utf firmware image */
-	ret = request_firmware_direct(&fw_file->firmware, filename, ar->dev);
+	ret = firmware_request_nowarn(&fw_file->firmware, filename, ar->dev);
 	ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode fw request '%s': %d\n",
 		   filename, ret);
 
-- 
2.17.0

^ permalink raw reply related

* [PATCH v6 09/13] ath10k: use firmware_request_nowarn() to load firmware
From: Luis R. Rodriguez @ 2018-05-08 18:12 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc, davem
In-Reply-To: <20180508181247.19431-1-mcgrof@kernel.org>

From: Andres Rodriguez <andresx7@gmail.com>

This reduces the unnecessary spew when trying to load optional firmware:
"Direct firmware load for ... failed with error -2"

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
Acked-by: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/net/wireless/ath/ath10k/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 4cf54a7ef09a..ad4f6e3c0737 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -694,7 +694,7 @@ static const struct firmware *ath10k_fetch_fw_file(struct ath10k *ar,
 		dir = ".";
 
 	snprintf(filename, sizeof(filename), "%s/%s", dir, file);
-	ret = request_firmware(&fw, filename, ar->dev);
+	ret = firmware_request_nowarn(&fw, filename, ar->dev);
 	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot fw request '%s': %d\n",
 		   filename, ret);
 
-- 
2.17.0

^ permalink raw reply related

* [PATCH v6 08/13] firmware: add firmware_request_nowarn() - load firmware without warnings
From: Luis R. Rodriguez @ 2018-05-08 18:12 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc, davem
In-Reply-To: <20180508181247.19431-1-mcgrof@kernel.org>

From: Andres Rodriguez <andresx7@gmail.com>

Currently the firmware loader only exposes one silent path for querying
optional firmware, and that is firmware_request_direct(). This function
also disables the sysfs fallback mechanism, which might not always be the
desired behaviour [0].

This patch introduces a variations of request_firmware() that enable the
caller to disable the undesired warning messages but enables the sysfs
fallback mechanism. This is equivalent to adding FW_OPT_NO_WARN to the
old behaviour.

[0]: https://git.kernel.org/linus/c0cc00f250e1

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
[mcgrof: used the old API calls as the full rename is not done yet, and
 add the caller for when FW_LOADER is disabled, enhance documentation ]
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 .../driver-api/firmware/request_firmware.rst  |  5 ++++
 drivers/base/firmware_loader/main.c           | 27 +++++++++++++++++++
 include/linux/firmware.h                      | 10 +++++++
 3 files changed, 42 insertions(+)

diff --git a/Documentation/driver-api/firmware/request_firmware.rst b/Documentation/driver-api/firmware/request_firmware.rst
index d5ec95a7195b..f62bdcbfed5b 100644
--- a/Documentation/driver-api/firmware/request_firmware.rst
+++ b/Documentation/driver-api/firmware/request_firmware.rst
@@ -20,6 +20,11 @@ request_firmware
 .. kernel-doc:: drivers/base/firmware_loader/main.c
    :functions: request_firmware
 
+firmware_request_nowarn
+-----------------------
+.. kernel-doc:: drivers/base/firmware_loader/main.c
+   :functions: firmware_request_nowarn
+
 request_firmware_direct
 -----------------------
 .. kernel-doc:: drivers/base/firmware_loader/main.c
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index abdc4e4d55f1..2be79ee773c0 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -631,6 +631,33 @@ request_firmware(const struct firmware **firmware_p, const char *name,
 }
 EXPORT_SYMBOL(request_firmware);
 
+/**
+ * firmware_request_nowarn() - request for an optional fw module
+ * @firmware: pointer to firmware image
+ * @name: name of firmware file
+ * @device: device for which firmware is being loaded
+ *
+ * This function is similar in behaviour to request_firmware(), except
+ * it doesn't produce warning messages when the file is not found.
+ * The sysfs fallback mechanism is enabled if direct filesystem lookup fails,
+ * however, however failures to find the firmware file with it are still
+ * supressed. It is therefore up to the driver to check for the return value
+ * of this call and to decide when to inform the users of errors.
+ **/
+int firmware_request_nowarn(const struct firmware **firmware, const char *name,
+			    struct device *device)
+{
+	int ret;
+
+	/* Need to pin this module until return */
+	__module_get(THIS_MODULE);
+	ret = _request_firmware(firmware, name, device, NULL, 0,
+				FW_OPT_UEVENT | FW_OPT_NO_WARN);
+	module_put(THIS_MODULE);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(firmware_request_nowarn);
+
 /**
  * request_firmware_direct() - load firmware directly without usermode helper
  * @firmware_p: pointer to firmware image
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index 41050417cafb..2dd566c91d44 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -42,6 +42,8 @@ struct builtin_fw {
 #if defined(CONFIG_FW_LOADER) || (defined(CONFIG_FW_LOADER_MODULE) && defined(MODULE))
 int request_firmware(const struct firmware **fw, const char *name,
 		     struct device *device);
+int firmware_request_nowarn(const struct firmware **fw, const char *name,
+			    struct device *device);
 int request_firmware_nowait(
 	struct module *module, bool uevent,
 	const char *name, struct device *device, gfp_t gfp, void *context,
@@ -59,6 +61,14 @@ static inline int request_firmware(const struct firmware **fw,
 {
 	return -EINVAL;
 }
+
+static inline int firmware_request_nowarn(const struct firmware **fw,
+					  const char *name,
+					  struct device *device)
+{
+	return -EINVAL;
+}
+
 static inline int request_firmware_nowait(
 	struct module *module, bool uevent,
 	const char *name, struct device *device, gfp_t gfp, void *context,
-- 
2.17.0

^ permalink raw reply related

* [PATCH v6 07/13] firmware_loader: make firmware_fallback_sysfs() print more useful
From: Luis R. Rodriguez @ 2018-05-08 18:12 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc, davem
In-Reply-To: <20180508181247.19431-1-mcgrof@kernel.org>

If we resort to using the sysfs fallback mechanism we don't print
the filename. This can be deceiving given we could have a series of
callers intertwined and it'd be unclear exactly for what firmware
this was meant for.

Additionally, although we don't currently use FW_OPT_NO_WARN when
dealing with the fallback mechanism, we will soon, so just respect
its use consistently.

And even if you *don't* want to print always on failure, you may
want to print when debugging so enable dynamic debug print when
FW_OPT_NO_WARN is used.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/base/firmware_loader/fallback.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 9169e7b9800c..b676a99c469c 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -690,6 +690,11 @@ int firmware_fallback_sysfs(struct firmware *fw, const char *name,
 	if (!fw_run_sysfs_fallback(opt_flags))
 		return ret;
 
-	dev_warn(device, "Falling back to user helper\n");
+	if (!(opt_flags & FW_OPT_NO_WARN))
+		dev_warn(device, "Falling back to syfs fallback for: %s\n",
+				 name);
+	else
+		dev_dbg(device, "Falling back to sysfs fallback for: %s\n",
+				name);
 	return fw_load_from_user_helper(fw, name, device, opt_flags);
 }
-- 
2.17.0

^ permalink raw reply related

* [PATCH v6 06/13] firmware_loader: move kconfig FW_LOADER entries to its own file
From: Luis R. Rodriguez @ 2018-05-08 18:12 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc, davem
In-Reply-To: <20180508181247.19431-1-mcgrof@kernel.org>

This will make it easier to track and easier to understand
what components and features are part of the FW_LOADER. There
are some components related to firmware which have *nothing* to
do with the FW_LOADER, souch as PREVENT_FIRMWARE_BUILD.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/base/Kconfig                 | 155 +--------------------------
 drivers/base/firmware_loader/Kconfig | 154 ++++++++++++++++++++++++++
 2 files changed, 155 insertions(+), 154 deletions(-)
 create mode 100644 drivers/base/firmware_loader/Kconfig

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index a4fe86caecca..06d6e27784fa 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -88,160 +88,7 @@ config PREVENT_FIRMWARE_BUILD
 	    o CONFIG_WANXL through CONFIG_WANXL_BUILD_FIRMWARE
 	    o CONFIG_SCSI_AIC79XX through CONFIG_AIC79XX_BUILD_FIRMWARE
 
-menu "Firmware loader"
-
-config FW_LOADER
-	tristate "Firmware loading facility" if EXPERT
-	default y
-	---help---
-	  This enables the firmware loading facility in the kernel. The kernel
-	  will first look for built-in firmware, if it has any. Next, it will
-	  look for the requested firmware in a series of filesystem paths:
-
-		o firmware_class path module parameter or kernel boot param
-		o /lib/firmware/updates/UTS_RELEASE
-		o /lib/firmware/updates
-		o /lib/firmware/UTS_RELEASE
-		o /lib/firmware
-
-	  Enabling this feature only increases your kernel image by about
-	  828 bytes, enable this option unless you are certain you don't
-	  need firmware.
-
-	  You typically want this built-in (=y) but you can also enable this
-	  as a module, in which case the firmware_class module will be built.
-	  You also want to be sure to enable this built-in if you are going to
-	  enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
-
-if FW_LOADER
-
-config EXTRA_FIRMWARE
-	string "Build these firmware blobs into the kernel binary"
-	help
-	  Device drivers which require firmware can typically deal with
-	  having the kernel load firmware from the various supported
-	  /lib/firmware/ paths. This option enables you to build into the
-	  kernel firmware files. Built-in firmware searches are preceeded
-	  over firmware lookups using your filesystem over the supported
-	  /lib/firmware paths documented on CONFIG_FW_LOADER.
-
-	  This may be useful for testing or if the firmware is required early on
-	  in boot and cannot rely on the firmware being placed in an initrd or
-	  initramfs.
-
-	  This option is a string and takes the (space-separated) names of the
-	  firmware files -- the same names that appear in MODULE_FIRMWARE()
-	  and request_firmware() in the source. These files should exist under
-	  the directory specified by the EXTRA_FIRMWARE_DIR option, which is
-	  /lib/firmware by default.
-
-	  For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
-	  the usb8388.bin file into /lib/firmware, and build the kernel. Then
-	  any request_firmware("usb8388.bin") will be satisfied internally
-	  inside the kernel without ever looking at your filesystem at runtime.
-
-	  WARNING: If you include additional firmware files into your binary
-	  kernel image that are not available under the terms of the GPL,
-	  then it may be a violation of the GPL to distribute the resulting
-	  image since it combines both GPL and non-GPL work. You should
-	  consult a lawyer of your own before distributing such an image.
-
-config EXTRA_FIRMWARE_DIR
-	string "Firmware blobs root directory"
-	depends on EXTRA_FIRMWARE != ""
-	default "/lib/firmware"
-	help
-	  This option controls the directory in which the kernel build system
-	  looks for the firmware files listed in the EXTRA_FIRMWARE option.
-
-config FW_LOADER_USER_HELPER
-	bool "Enable the firmware sysfs fallback mechanism"
-	help
-	  This option enables a sysfs loading facility to enable firmware
-	  loading to the kernel through userspace as a fallback mechanism
-	  if and only if the kernel's direct filesystem lookup for the
-	  firmware failed using the different /lib/firmware/ paths, or the
-	  path specified in the firmware_class path module parameter, or the
-	  firmware_class path kernel boot parameter if the firmware_class is
-	  built-in. For details on how to work with the sysfs fallback mechanism
-	  refer to Documentation/driver-api/firmware/fallback-mechanisms.rst.
-
-	  The direct filesystem lookup for firwmare is always used first now.
-
-	  If the kernel's direct filesystem lookup for firware fails to find
-	  the requested firmware a sysfs fallback loading facility is made
-	  available and userspace is informed about this through uevents.
-	  The uevent can be supressed if the driver explicitly requested it,
-	  this is known as the driver using the custom fallback mechanism.
-	  If the custom fallback mechanism is used userspace must always
-	  acknowledge failure to find firmware as the timeout for the fallback
-	  mechanism is disabled, and failed requests will linger forever.
-
-	  This used to be the default firmware loading facility, and udev used
-	  to listen for uvents to load firmware for the kernel. The firmware
-	  loading facility functionality in udev has been removed, as such it
-	  can no longer be relied upon as a fallback mechanism. Linux no longer
-	  relies on or uses a fallback mechanism in userspace. If you need to
-	  rely on one refer to the permissively licensed firmwared:
-
-	  https://github.com/teg/firmwared
-
-	  Since this was the default firmware loading facility at one point,
-	  old userspace may exist which relies upon it, and as such this
-	  mechanism can never be removed from the kernel.
-
-	  You should only enable this functionality if you are certain you
-	  require a fallback mechanism and have a userspace mechanism ready to
-	  load firmware in case it is not found. One main reason for this may
-	  be if you have drivers which require firmware built-in and for
-	  whatever reason cannot place the required firmware in initramfs.
-	  Another reason kernels may have this feature enabled is to support a
-	  driver which explicitly relies on this fallback mechanism. Only two
-	  drivers need this today:
-
-	    o CONFIG_LEDS_LP55XX_COMMON
-	    o CONFIG_DELL_RBU
-
-	  Outside of supporting the above drivers, another reason for needing
-	  this may be that your firmware resides outside of the paths the kernel
-	  looks for and cannot possibily be specified using the firmware_class
-	  path module parameter or kernel firmware_class path boot parameter
-	  if firmware_class is built-in.
-
-	  A modern use case may be to temporarily mount a custom partition
-	  during provisioning which is only accessible to userspace, and then
-	  to use it to look for and fetch the required firmware. Such type of
-	  driver functionality may not even ever be desirable upstream by
-	  vendors, and as such is only required to be supported as an interface
-	  for provisioning. Since udev's firmware loading facility has been
-	  removed you can use firmwared or a fork of it to customize how you
-	  want to load firmware based on uevents issued.
-
-	  Enabling this option will increase your kernel image size by about
-	  13436 bytes.
-
-	  If you are unsure about this, say N here, unless you are Linux
-	  distribution and need to support the above two drivers, or you are
-	  certain you need to support some really custom firmware loading
-	  facility in userspace.
-
-config FW_LOADER_USER_HELPER_FALLBACK
-	bool "Force the firmware sysfs fallback mechanism when possible"
-	depends on FW_LOADER_USER_HELPER
-	help
-	  Enabling this option forces a sysfs userspace fallback mechanism
-	  to be used for all firmware requests which explicitly do not disable a
-	  a fallback mechanism. Firmware calls which do prohibit a fallback
-	  mechanism is request_firmware_direct(). This option is kept for
-          backward compatibility purposes given this precise mechanism can also
-	  be enabled by setting the proc sysctl value to true:
-
-	       /proc/sys/kernel/firmware_config/force_sysfs_fallback
-
-	  If you are unsure about this, say N here.
-
-endif # FW_LOADER
-endmenu
+source "drivers/base/firmware_loader/Kconfig"
 
 config WANT_DEV_COREDUMP
 	bool
diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
new file mode 100644
index 000000000000..bff3606bc7fa
--- /dev/null
+++ b/drivers/base/firmware_loader/Kconfig
@@ -0,0 +1,154 @@
+menu "Firmware loader"
+
+config FW_LOADER
+	tristate "Firmware loading facility" if EXPERT
+	default y
+	---help---
+	  This enables the firmware loading facility in the kernel. The kernel
+	  will first look for built-in firmware, if it has any. Next, it will
+	  look for the requested firmware in a series of filesystem paths:
+
+		o firmware_class path module parameter or kernel boot param
+		o /lib/firmware/updates/UTS_RELEASE
+		o /lib/firmware/updates
+		o /lib/firmware/UTS_RELEASE
+		o /lib/firmware
+
+	  Enabling this feature only increases your kernel image by about
+	  828 bytes, enable this option unless you are certain you don't
+	  need firmware.
+
+	  You typically want this built-in (=y) but you can also enable this
+	  as a module, in which case the firmware_class module will be built.
+	  You also want to be sure to enable this built-in if you are going to
+	  enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
+
+if FW_LOADER
+
+config EXTRA_FIRMWARE
+	string "Build these firmware blobs into the kernel binary"
+	help
+	  Device drivers which require firmware can typically deal with
+	  having the kernel load firmware from the various supported
+	  /lib/firmware/ paths. This option enables you to build into the
+	  kernel firmware files. Built-in firmware searches are preceeded
+	  over firmware lookups using your filesystem over the supported
+	  /lib/firmware paths documented on CONFIG_FW_LOADER.
+
+	  This may be useful for testing or if the firmware is required early on
+	  in boot and cannot rely on the firmware being placed in an initrd or
+	  initramfs.
+
+	  This option is a string and takes the (space-separated) names of the
+	  firmware files -- the same names that appear in MODULE_FIRMWARE()
+	  and request_firmware() in the source. These files should exist under
+	  the directory specified by the EXTRA_FIRMWARE_DIR option, which is
+	  /lib/firmware by default.
+
+	  For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
+	  the usb8388.bin file into /lib/firmware, and build the kernel. Then
+	  any request_firmware("usb8388.bin") will be satisfied internally
+	  inside the kernel without ever looking at your filesystem at runtime.
+
+	  WARNING: If you include additional firmware files into your binary
+	  kernel image that are not available under the terms of the GPL,
+	  then it may be a violation of the GPL to distribute the resulting
+	  image since it combines both GPL and non-GPL work. You should
+	  consult a lawyer of your own before distributing such an image.
+
+config EXTRA_FIRMWARE_DIR
+	string "Firmware blobs root directory"
+	depends on EXTRA_FIRMWARE != ""
+	default "/lib/firmware"
+	help
+	  This option controls the directory in which the kernel build system
+	  looks for the firmware files listed in the EXTRA_FIRMWARE option.
+
+config FW_LOADER_USER_HELPER
+	bool "Enable the firmware sysfs fallback mechanism"
+	help
+	  This option enables a sysfs loading facility to enable firmware
+	  loading to the kernel through userspace as a fallback mechanism
+	  if and only if the kernel's direct filesystem lookup for the
+	  firmware failed using the different /lib/firmware/ paths, or the
+	  path specified in the firmware_class path module parameter, or the
+	  firmware_class path kernel boot parameter if the firmware_class is
+	  built-in. For details on how to work with the sysfs fallback mechanism
+	  refer to Documentation/driver-api/firmware/fallback-mechanisms.rst.
+
+	  The direct filesystem lookup for firwmare is always used first now.
+
+	  If the kernel's direct filesystem lookup for firware fails to find
+	  the requested firmware a sysfs fallback loading facility is made
+	  available and userspace is informed about this through uevents.
+	  The uevent can be supressed if the driver explicitly requested it,
+	  this is known as the driver using the custom fallback mechanism.
+	  If the custom fallback mechanism is used userspace must always
+	  acknowledge failure to find firmware as the timeout for the fallback
+	  mechanism is disabled, and failed requests will linger forever.
+
+	  This used to be the default firmware loading facility, and udev used
+	  to listen for uvents to load firmware for the kernel. The firmware
+	  loading facility functionality in udev has been removed, as such it
+	  can no longer be relied upon as a fallback mechanism. Linux no longer
+	  relies on or uses a fallback mechanism in userspace. If you need to
+	  rely on one refer to the permissively licensed firmwared:
+
+	  https://github.com/teg/firmwared
+
+	  Since this was the default firmware loading facility at one point,
+	  old userspace may exist which relies upon it, and as such this
+	  mechanism can never be removed from the kernel.
+
+	  You should only enable this functionality if you are certain you
+	  require a fallback mechanism and have a userspace mechanism ready to
+	  load firmware in case it is not found. One main reason for this may
+	  be if you have drivers which require firmware built-in and for
+	  whatever reason cannot place the required firmware in initramfs.
+	  Another reason kernels may have this feature enabled is to support a
+	  driver which explicitly relies on this fallback mechanism. Only two
+	  drivers need this today:
+
+	    o CONFIG_LEDS_LP55XX_COMMON
+	    o CONFIG_DELL_RBU
+
+	  Outside of supporting the above drivers, another reason for needing
+	  this may be that your firmware resides outside of the paths the kernel
+	  looks for and cannot possibily be specified using the firmware_class
+	  path module parameter or kernel firmware_class path boot parameter
+	  if firmware_class is built-in.
+
+	  A modern use case may be to temporarily mount a custom partition
+	  during provisioning which is only accessible to userspace, and then
+	  to use it to look for and fetch the required firmware. Such type of
+	  driver functionality may not even ever be desirable upstream by
+	  vendors, and as such is only required to be supported as an interface
+	  for provisioning. Since udev's firmware loading facility has been
+	  removed you can use firmwared or a fork of it to customize how you
+	  want to load firmware based on uevents issued.
+
+	  Enabling this option will increase your kernel image size by about
+	  13436 bytes.
+
+	  If you are unsure about this, say N here, unless you are Linux
+	  distribution and need to support the above two drivers, or you are
+	  certain you need to support some really custom firmware loading
+	  facility in userspace.
+
+config FW_LOADER_USER_HELPER_FALLBACK
+	bool "Force the firmware sysfs fallback mechanism when possible"
+	depends on FW_LOADER_USER_HELPER
+	help
+	  Enabling this option forces a sysfs userspace fallback mechanism
+	  to be used for all firmware requests which explicitly do not disable a
+	  a fallback mechanism. Firmware calls which do prohibit a fallback
+	  mechanism is request_firmware_direct(). This option is kept for
+          backward compatibility purposes given this precise mechanism can also
+	  be enabled by setting the proc sysctl value to true:
+
+	       /proc/sys/kernel/firmware_config/force_sysfs_fallback
+
+	  If you are unsure about this, say N here.
+
+endif # FW_LOADER
+endmenu
-- 
2.17.0

^ permalink raw reply related

* [PATCH v6 05/13] firmware_loader: enhance Kconfig documentation over FW_LOADER
From: Luis R. Rodriguez @ 2018-05-08 18:12 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc, davem
In-Reply-To: <20180508181247.19431-1-mcgrof@kernel.org>

If you try to read FW_LOADER today it speaks of old riddles and
unless you have been following development closely you will loose
track of what is what. Even the documentation for PREVENT_FIRMWARE_BUILD
is a bit fuzzy and how it fits into this big picture.

Give the FW_LOADER kconfig documentation some love with more up to
date developments and recommendations. While at it, wrap the FW_LOADER
code into its own menu to compartamentalize and make it clearer which
components really are part of the FW_LOADER. This should also make
it easier to later move these kconfig entries into the firmware_loader/
directory later.

This also now recommends using firmwared [0] for folks left needing a uevent
handler in userspace for the sysfs firmware fallback mechanis given udev's
uevent firmware mechanism was ripped out a while ago.

[0] https://github.com/teg/firmwared

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/base/Kconfig | 165 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 131 insertions(+), 34 deletions(-)

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 29b0eb452b3a..a4fe86caecca 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -70,39 +70,64 @@ config STANDALONE
 	  If unsure, say Y.
 
 config PREVENT_FIRMWARE_BUILD
-	bool "Prevent firmware from being built"
+	bool "Disable drivers features which enable custom firmware building"
 	default y
 	help
-	  Say yes to avoid building firmware. Firmware is usually shipped
-	  with the driver and only when updating the firmware should a
-	  rebuild be made.
-	  If unsure, say Y here.
+	  Say yes to disable driver features which enable building a custom
+	  driver firmwar at kernel build time. These drivers do not use the
+	  kernel firmware API to load firmware (CONFIG_FW_LOADER), instead they
+	  use their own custom loading mechanism. The required firmware is
+	  usually shipped with the driver, building the driver firmware
+	  should only be needed if you have an updated firmware source.
+
+	  Firmware should not be being built as part of kernel, these days
+	  you should always prevent this and say Y here. There are only two
+	  old drivers which enable building of its firmware at kernel build
+	  time:
+
+	    o CONFIG_WANXL through CONFIG_WANXL_BUILD_FIRMWARE
+	    o CONFIG_SCSI_AIC79XX through CONFIG_AIC79XX_BUILD_FIRMWARE
+
+menu "Firmware loader"
 
 config FW_LOADER
-	tristate "Userspace firmware loading support" if EXPERT
+	tristate "Firmware loading facility" if EXPERT
 	default y
 	---help---
-	  This option is provided for the case where none of the in-tree modules
-	  require userspace firmware loading support, but a module built
-	  out-of-tree does.
+	  This enables the firmware loading facility in the kernel. The kernel
+	  will first look for built-in firmware, if it has any. Next, it will
+	  look for the requested firmware in a series of filesystem paths:
+
+		o firmware_class path module parameter or kernel boot param
+		o /lib/firmware/updates/UTS_RELEASE
+		o /lib/firmware/updates
+		o /lib/firmware/UTS_RELEASE
+		o /lib/firmware
+
+	  Enabling this feature only increases your kernel image by about
+	  828 bytes, enable this option unless you are certain you don't
+	  need firmware.
+
+	  You typically want this built-in (=y) but you can also enable this
+	  as a module, in which case the firmware_class module will be built.
+	  You also want to be sure to enable this built-in if you are going to
+	  enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
+
+if FW_LOADER
 
 config EXTRA_FIRMWARE
-	string "External firmware blobs to build into the kernel binary"
-	depends on FW_LOADER
+	string "Build these firmware blobs into the kernel binary"
 	help
-	  Various drivers in the kernel source tree may require firmware,
-	  which is generally available in your distribution's linux-firmware
-	  package.
+	  Device drivers which require firmware can typically deal with
+	  having the kernel load firmware from the various supported
+	  /lib/firmware/ paths. This option enables you to build into the
+	  kernel firmware files. Built-in firmware searches are preceeded
+	  over firmware lookups using your filesystem over the supported
+	  /lib/firmware paths documented on CONFIG_FW_LOADER.
 
-	  The linux-firmware package should install firmware into
-	  /lib/firmware/ on your system, so they can be loaded by userspace
-	  helpers on request.
-
-	  This option allows firmware to be built into the kernel for the case
-	  where the user either cannot or doesn't want to provide it from
-	  userspace at runtime (for example, when the firmware in question is
-	  required for accessing the boot device, and the user doesn't want to
-	  use an initrd).
+	  This may be useful for testing or if the firmware is required early on
+	  in boot and cannot rely on the firmware being placed in an initrd or
+	  initramfs.
 
 	  This option is a string and takes the (space-separated) names of the
 	  firmware files -- the same names that appear in MODULE_FIRMWARE()
@@ -113,7 +138,7 @@ config EXTRA_FIRMWARE
 	  For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
 	  the usb8388.bin file into /lib/firmware, and build the kernel. Then
 	  any request_firmware("usb8388.bin") will be satisfied internally
-	  without needing to call out to userspace.
+	  inside the kernel without ever looking at your filesystem at runtime.
 
 	  WARNING: If you include additional firmware files into your binary
 	  kernel image that are not available under the terms of the GPL,
@@ -130,22 +155,94 @@ config EXTRA_FIRMWARE_DIR
 	  looks for the firmware files listed in the EXTRA_FIRMWARE option.
 
 config FW_LOADER_USER_HELPER
-	bool
+	bool "Enable the firmware sysfs fallback mechanism"
+	help
+	  This option enables a sysfs loading facility to enable firmware
+	  loading to the kernel through userspace as a fallback mechanism
+	  if and only if the kernel's direct filesystem lookup for the
+	  firmware failed using the different /lib/firmware/ paths, or the
+	  path specified in the firmware_class path module parameter, or the
+	  firmware_class path kernel boot parameter if the firmware_class is
+	  built-in. For details on how to work with the sysfs fallback mechanism
+	  refer to Documentation/driver-api/firmware/fallback-mechanisms.rst.
+
+	  The direct filesystem lookup for firwmare is always used first now.
+
+	  If the kernel's direct filesystem lookup for firware fails to find
+	  the requested firmware a sysfs fallback loading facility is made
+	  available and userspace is informed about this through uevents.
+	  The uevent can be supressed if the driver explicitly requested it,
+	  this is known as the driver using the custom fallback mechanism.
+	  If the custom fallback mechanism is used userspace must always
+	  acknowledge failure to find firmware as the timeout for the fallback
+	  mechanism is disabled, and failed requests will linger forever.
+
+	  This used to be the default firmware loading facility, and udev used
+	  to listen for uvents to load firmware for the kernel. The firmware
+	  loading facility functionality in udev has been removed, as such it
+	  can no longer be relied upon as a fallback mechanism. Linux no longer
+	  relies on or uses a fallback mechanism in userspace. If you need to
+	  rely on one refer to the permissively licensed firmwared:
+
+	  https://github.com/teg/firmwared
+
+	  Since this was the default firmware loading facility at one point,
+	  old userspace may exist which relies upon it, and as such this
+	  mechanism can never be removed from the kernel.
+
+	  You should only enable this functionality if you are certain you
+	  require a fallback mechanism and have a userspace mechanism ready to
+	  load firmware in case it is not found. One main reason for this may
+	  be if you have drivers which require firmware built-in and for
+	  whatever reason cannot place the required firmware in initramfs.
+	  Another reason kernels may have this feature enabled is to support a
+	  driver which explicitly relies on this fallback mechanism. Only two
+	  drivers need this today:
+
+	    o CONFIG_LEDS_LP55XX_COMMON
+	    o CONFIG_DELL_RBU
+
+	  Outside of supporting the above drivers, another reason for needing
+	  this may be that your firmware resides outside of the paths the kernel
+	  looks for and cannot possibily be specified using the firmware_class
+	  path module parameter or kernel firmware_class path boot parameter
+	  if firmware_class is built-in.
+
+	  A modern use case may be to temporarily mount a custom partition
+	  during provisioning which is only accessible to userspace, and then
+	  to use it to look for and fetch the required firmware. Such type of
+	  driver functionality may not even ever be desirable upstream by
+	  vendors, and as such is only required to be supported as an interface
+	  for provisioning. Since udev's firmware loading facility has been
+	  removed you can use firmwared or a fork of it to customize how you
+	  want to load firmware based on uevents issued.
+
+	  Enabling this option will increase your kernel image size by about
+	  13436 bytes.
+
+	  If you are unsure about this, say N here, unless you are Linux
+	  distribution and need to support the above two drivers, or you are
+	  certain you need to support some really custom firmware loading
+	  facility in userspace.
 
 config FW_LOADER_USER_HELPER_FALLBACK
-	bool "Fallback user-helper invocation for firmware loading"
-	depends on FW_LOADER
-	select FW_LOADER_USER_HELPER
+	bool "Force the firmware sysfs fallback mechanism when possible"
+	depends on FW_LOADER_USER_HELPER
 	help
-	  This option enables / disables the invocation of user-helper
-	  (e.g. udev) for loading firmware files as a fallback after the
-	  direct file loading in kernel fails.  The user-mode helper is
-	  no longer required unless you have a special firmware file that
-	  resides in a non-standard path. Moreover, the udev support has
-	  been deprecated upstream.
+	  Enabling this option forces a sysfs userspace fallback mechanism
+	  to be used for all firmware requests which explicitly do not disable a
+	  a fallback mechanism. Firmware calls which do prohibit a fallback
+	  mechanism is request_firmware_direct(). This option is kept for
+          backward compatibility purposes given this precise mechanism can also
+	  be enabled by setting the proc sysctl value to true:
+
+	       /proc/sys/kernel/firmware_config/force_sysfs_fallback
 
 	  If you are unsure about this, say N here.
 
+endif # FW_LOADER
+endmenu
+
 config WANT_DEV_COREDUMP
 	bool
 	help
-- 
2.17.0

^ permalink raw reply related

* [PATCH v6 04/13] firmware_loader: document firmware_sysfs_fallback()
From: Luis R. Rodriguez @ 2018-05-08 18:12 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc, davem
In-Reply-To: <20180508181247.19431-1-mcgrof@kernel.org>

This also sets the expecations for future fallback interfaces, even
if they are not exported.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/base/firmware_loader/fallback.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 3db9e0f225ac..9169e7b9800c 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -662,6 +662,26 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
 	return fw_force_sysfs_fallback(opt_flags);
 }
 
+/**
+ * firmware_fallback_sysfs() - use the fallback mechanism to find firmware
+ * @fw: pointer to firmware image
+ * @name: name of firmware file to look for
+ * @device: device for which firmware is being loaded
+ * @opt_flags: options to control firmware loading behaviour
+ * @ret: return value from direct lookup which triggered the fallback mechanism
+ *
+ * This function is called if direct lookup for the firmware failed, it enables
+ * a fallback mechanism through userspace by exposing a sysfs loading
+ * interface. Userspace is in charge of loading the firmware through the syfs
+ * loading interface. This syfs fallback mechanism may be disabled completely
+ * on a system by setting the proc sysctl value ignore_sysfs_fallback to true.
+ * If this false we check if the internal API caller set the @FW_OPT_NOFALLBACK
+ * flag, if so it would also disable the fallback mechanism. A system may want
+ * to enfoce the sysfs fallback mechanism at all times, it can do this by
+ * setting ignore_sysfs_fallback to false and force_sysfs_fallback to true.
+ * Enabling force_sysfs_fallback is functionally equivalent to build a kernel
+ * with CONFIG_FW_LOADER_USER_HELPER_FALLBACK.
+ **/
 int firmware_fallback_sysfs(struct firmware *fw, const char *name,
 			    struct device *device,
 			    enum fw_opt opt_flags,
-- 
2.17.0

^ permalink raw reply related

* [PATCH v6 03/13] firmware: rename fw_sysfs_fallback to firmware_fallback_sysfs()
From: Luis R. Rodriguez @ 2018-05-08 18:12 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc, davem
In-Reply-To: <20180508181247.19431-1-mcgrof@kernel.org>

From: Andres Rodriguez <andresx7@gmail.com>

This is done since this call is now exposed through kernel-doc,
and since this also paves the way for different future types of
fallback mechanims.

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
[mcgrof: small coding style changes]
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/base/firmware_loader/fallback.c |  8 ++++----
 drivers/base/firmware_loader/fallback.h | 16 ++++++++--------
 drivers/base/firmware_loader/firmware.h |  2 +-
 drivers/base/firmware_loader/main.c     |  2 +-
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 529f7013616f..3db9e0f225ac 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -662,10 +662,10 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
 	return fw_force_sysfs_fallback(opt_flags);
 }
 
-int fw_sysfs_fallback(struct firmware *fw, const char *name,
-		      struct device *device,
-		      enum fw_opt opt_flags,
-		      int ret)
+int firmware_fallback_sysfs(struct firmware *fw, const char *name,
+			    struct device *device,
+			    enum fw_opt opt_flags,
+			    int ret)
 {
 	if (!fw_run_sysfs_fallback(opt_flags))
 		return ret;
diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h
index a3b73a09db6c..21063503e4ea 100644
--- a/drivers/base/firmware_loader/fallback.h
+++ b/drivers/base/firmware_loader/fallback.h
@@ -31,10 +31,10 @@ struct firmware_fallback_config {
 };
 
 #ifdef CONFIG_FW_LOADER_USER_HELPER
-int fw_sysfs_fallback(struct firmware *fw, const char *name,
-		      struct device *device,
-		      enum fw_opt opt_flags,
-		      int ret);
+int firmware_fallback_sysfs(struct firmware *fw, const char *name,
+			    struct device *device,
+			    enum fw_opt opt_flags,
+			    int ret);
 void kill_pending_fw_fallback_reqs(bool only_kill_custom);
 
 void fw_fallback_set_cache_timeout(void);
@@ -43,10 +43,10 @@ void fw_fallback_set_default_timeout(void);
 int register_sysfs_loader(void);
 void unregister_sysfs_loader(void);
 #else /* CONFIG_FW_LOADER_USER_HELPER */
-static inline int fw_sysfs_fallback(struct firmware *fw, const char *name,
-				    struct device *device,
-				    enum fw_opt opt_flags,
-				    int ret)
+static inline int firmware_fallback_sysfs(struct firmware *fw, const char *name,
+					  struct device *device,
+					  enum fw_opt opt_flags,
+					  int ret)
 {
 	/* Keep carrying over the same error */
 	return ret;
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index 4f433b447367..4c1395f8e7ed 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -20,7 +20,7 @@
  * @FW_OPT_NOWAIT: Used to describe the firmware request is asynchronous.
  * @FW_OPT_USERHELPER: Enable the fallback mechanism, in case the direct
  *	filesystem lookup fails at finding the firmware.  For details refer to
- *	fw_sysfs_fallback().
+ *	firmware_fallback_sysfs().
  * @FW_OPT_NO_WARN: Quiet, avoid printing warning messages.
  * @FW_OPT_NOCACHE: Disables firmware caching. Firmware caching is used to
  *	cache the firmware upon suspend, so that upon resume races against the
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 4d11efadb3a4..abdc4e4d55f1 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -581,7 +581,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 			dev_warn(device,
 				 "Direct firmware load for %s failed with error %d\n",
 				 name, ret);
-		ret = fw_sysfs_fallback(fw, name, device, opt_flags, ret);
+		ret = firmware_fallback_sysfs(fw, name, device, opt_flags, ret);
 	} else
 		ret = assign_fw(fw, device, opt_flags);
 
-- 
2.17.0

^ permalink raw reply related

* [PATCH v6 02/13] firmware: use () to terminate kernel-doc function names
From: Luis R. Rodriguez @ 2018-05-08 18:12 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc, davem
In-Reply-To: <20180508181247.19431-1-mcgrof@kernel.org>

From: Andres Rodriguez <andresx7@gmail.com>

The kernel-doc spec dictates a function name ends in ().

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
[mcgrof: adjust since the wide API rename is not yet merged]
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/base/firmware_loader/fallback.c |  8 ++++----
 drivers/base/firmware_loader/main.c     | 22 +++++++++++-----------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index b57a7b3b4122..529f7013616f 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -125,7 +125,7 @@ static ssize_t timeout_show(struct class *class, struct class_attribute *attr,
 }
 
 /**
- * firmware_timeout_store - set number of seconds to wait for firmware
+ * firmware_timeout_store() - set number of seconds to wait for firmware
  * @class: device class pointer
  * @attr: device attribute pointer
  * @buf: buffer to scan for timeout value
@@ -239,7 +239,7 @@ static int map_fw_priv_pages(struct fw_priv *fw_priv)
 }
 
 /**
- * firmware_loading_store - set value in the 'loading' control file
+ * firmware_loading_store() - set value in the 'loading' control file
  * @dev: device pointer
  * @attr: device attribute pointer
  * @buf: buffer to scan for loading control value
@@ -431,7 +431,7 @@ static int fw_realloc_pages(struct fw_sysfs *fw_sysfs, int min_size)
 }
 
 /**
- * firmware_data_write - write method for firmware
+ * firmware_data_write() - write method for firmware
  * @filp: open sysfs file
  * @kobj: kobject for the device
  * @bin_attr: bin_attr structure
@@ -537,7 +537,7 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
 }
 
 /**
- * fw_load_sysfs_fallback - load a firmware via the sysfs fallback mechanism
+ * fw_load_sysfs_fallback() - load a firmware via the sysfs fallback mechanism
  * @fw_sysfs: firmware sysfs information for the firmware to load
  * @opt_flags: flags of options, FW_OPT_*
  * @timeout: timeout to wait for the load
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 9919f0e6a7cc..4d11efadb3a4 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -597,7 +597,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 }
 
 /**
- * request_firmware: - send firmware request and wait for it
+ * request_firmware() - send firmware request and wait for it
  * @firmware_p: pointer to firmware image
  * @name: name of firmware file
  * @device: device for which firmware is being loaded
@@ -632,7 +632,7 @@ request_firmware(const struct firmware **firmware_p, const char *name,
 EXPORT_SYMBOL(request_firmware);
 
 /**
- * request_firmware_direct: - load firmware directly without usermode helper
+ * request_firmware_direct() - load firmware directly without usermode helper
  * @firmware_p: pointer to firmware image
  * @name: name of firmware file
  * @device: device for which firmware is being loaded
@@ -657,7 +657,7 @@ int request_firmware_direct(const struct firmware **firmware_p,
 EXPORT_SYMBOL_GPL(request_firmware_direct);
 
 /**
- * firmware_request_cache: - cache firmware for suspend so resume can use it
+ * firmware_request_cache() - cache firmware for suspend so resume can use it
  * @name: name of firmware file
  * @device: device for which firmware should be cached for
  *
@@ -681,7 +681,7 @@ int firmware_request_cache(struct device *device, const char *name)
 EXPORT_SYMBOL_GPL(firmware_request_cache);
 
 /**
- * request_firmware_into_buf - load firmware into a previously allocated buffer
+ * request_firmware_into_buf() - load firmware into a previously allocated buffer
  * @firmware_p: pointer to firmware image
  * @name: name of firmware file
  * @device: device for which firmware is being loaded and DMA region allocated
@@ -713,7 +713,7 @@ request_firmware_into_buf(const struct firmware **firmware_p, const char *name,
 EXPORT_SYMBOL(request_firmware_into_buf);
 
 /**
- * release_firmware: - release the resource associated with a firmware image
+ * release_firmware() - release the resource associated with a firmware image
  * @fw: firmware resource to release
  **/
 void release_firmware(const struct firmware *fw)
@@ -755,7 +755,7 @@ static void request_firmware_work_func(struct work_struct *work)
 }
 
 /**
- * request_firmware_nowait - asynchronous version of request_firmware
+ * request_firmware_nowait() - asynchronous version of request_firmware
  * @module: module requesting the firmware
  * @uevent: sends uevent to copy the firmware image if this flag
  *	is non-zero else the firmware copy must be done manually.
@@ -824,7 +824,7 @@ EXPORT_SYMBOL(request_firmware_nowait);
 static ASYNC_DOMAIN_EXCLUSIVE(fw_cache_domain);
 
 /**
- * cache_firmware - cache one firmware image in kernel memory space
+ * cache_firmware() - cache one firmware image in kernel memory space
  * @fw_name: the firmware image name
  *
  * Cache firmware in kernel memory so that drivers can use it when
@@ -866,7 +866,7 @@ static struct fw_priv *lookup_fw_priv(const char *fw_name)
 }
 
 /**
- * uncache_firmware - remove one cached firmware image
+ * uncache_firmware() - remove one cached firmware image
  * @fw_name: the firmware image name
  *
  * Uncache one firmware image which has been cached successfully
@@ -1042,7 +1042,7 @@ static void __device_uncache_fw_images(void)
 }
 
 /**
- * device_cache_fw_images - cache devices' firmware
+ * device_cache_fw_images() - cache devices' firmware
  *
  * If one device called request_firmware or its nowait version
  * successfully before, the firmware names are recored into the
@@ -1075,7 +1075,7 @@ static void device_cache_fw_images(void)
 }
 
 /**
- * device_uncache_fw_images - uncache devices' firmware
+ * device_uncache_fw_images() - uncache devices' firmware
  *
  * uncache all firmwares which have been cached successfully
  * by device_uncache_fw_images earlier
@@ -1092,7 +1092,7 @@ static void device_uncache_fw_images_work(struct work_struct *work)
 }
 
 /**
- * device_uncache_fw_images_delay - uncache devices firmwares
+ * device_uncache_fw_images_delay() - uncache devices firmwares
  * @delay: number of milliseconds to delay uncache device firmwares
  *
  * uncache all devices's firmwares which has been cached successfully
-- 
2.17.0

^ permalink raw reply related

* [PATCH v6 01/13] firmware: wrap FW_OPT_* into an enum
From: Luis R. Rodriguez @ 2018-05-08 18:12 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc, davem
In-Reply-To: <20180508181247.19431-1-mcgrof@kernel.org>

From: Andres Rodriguez <andresx7@gmail.com>

This should let us associate enum kdoc to these values.
While at it, kdocify the fw_opt.

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
[mcgrof: coding style fixes, merge kdoc with enum move]
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/base/firmware_loader/fallback.c | 12 ++++----
 drivers/base/firmware_loader/fallback.h |  6 ++--
 drivers/base/firmware_loader/firmware.h | 37 +++++++++++++++++++------
 drivers/base/firmware_loader/main.c     |  6 ++--
 4 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 358354148dec..b57a7b3b4122 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -512,7 +512,7 @@ static const struct attribute_group *fw_dev_attr_groups[] = {
 
 static struct fw_sysfs *
 fw_create_instance(struct firmware *firmware, const char *fw_name,
-		   struct device *device, unsigned int opt_flags)
+		   struct device *device, enum fw_opt opt_flags)
 {
 	struct fw_sysfs *fw_sysfs;
 	struct device *f_dev;
@@ -545,7 +545,7 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
  * In charge of constructing a sysfs fallback interface for firmware loading.
  **/
 static int fw_load_sysfs_fallback(struct fw_sysfs *fw_sysfs,
-				  unsigned int opt_flags, long timeout)
+				  enum fw_opt opt_flags, long timeout)
 {
 	int retval = 0;
 	struct device *f_dev = &fw_sysfs->dev;
@@ -599,7 +599,7 @@ static int fw_load_sysfs_fallback(struct fw_sysfs *fw_sysfs,
 
 static int fw_load_from_user_helper(struct firmware *firmware,
 				    const char *name, struct device *device,
-				    unsigned int opt_flags)
+				    enum fw_opt opt_flags)
 {
 	struct fw_sysfs *fw_sysfs;
 	long timeout;
@@ -640,7 +640,7 @@ static int fw_load_from_user_helper(struct firmware *firmware,
 	return ret;
 }
 
-static bool fw_force_sysfs_fallback(unsigned int opt_flags)
+static bool fw_force_sysfs_fallback(enum fw_opt opt_flags)
 {
 	if (fw_fallback_config.force_sysfs_fallback)
 		return true;
@@ -649,7 +649,7 @@ static bool fw_force_sysfs_fallback(unsigned int opt_flags)
 	return true;
 }
 
-static bool fw_run_sysfs_fallback(unsigned int opt_flags)
+static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
 {
 	if (fw_fallback_config.ignore_sysfs_fallback) {
 		pr_info_once("Ignoring firmware sysfs fallback due to sysctl knob\n");
@@ -664,7 +664,7 @@ static bool fw_run_sysfs_fallback(unsigned int opt_flags)
 
 int fw_sysfs_fallback(struct firmware *fw, const char *name,
 		      struct device *device,
-		      unsigned int opt_flags,
+		      enum fw_opt opt_flags,
 		      int ret)
 {
 	if (!fw_run_sysfs_fallback(opt_flags))
diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h
index f8255670a663..a3b73a09db6c 100644
--- a/drivers/base/firmware_loader/fallback.h
+++ b/drivers/base/firmware_loader/fallback.h
@@ -5,6 +5,8 @@
 #include <linux/firmware.h>
 #include <linux/device.h>
 
+#include "firmware.h"
+
 /**
  * struct firmware_fallback_config - firmware fallback configuration settings
  *
@@ -31,7 +33,7 @@ struct firmware_fallback_config {
 #ifdef CONFIG_FW_LOADER_USER_HELPER
 int fw_sysfs_fallback(struct firmware *fw, const char *name,
 		      struct device *device,
-		      unsigned int opt_flags,
+		      enum fw_opt opt_flags,
 		      int ret);
 void kill_pending_fw_fallback_reqs(bool only_kill_custom);
 
@@ -43,7 +45,7 @@ void unregister_sysfs_loader(void);
 #else /* CONFIG_FW_LOADER_USER_HELPER */
 static inline int fw_sysfs_fallback(struct firmware *fw, const char *name,
 				    struct device *device,
-				    unsigned int opt_flags,
+				    enum fw_opt opt_flags,
 				    int ret)
 {
 	/* Keep carrying over the same error */
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index 64acbb1a392c..4f433b447367 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -2,6 +2,7 @@
 #ifndef __FIRMWARE_LOADER_H
 #define __FIRMWARE_LOADER_H
 
+#include <linux/bitops.h>
 #include <linux/firmware.h>
 #include <linux/types.h>
 #include <linux/kref.h>
@@ -10,13 +11,33 @@
 
 #include <generated/utsrelease.h>
 
-/* firmware behavior options */
-#define FW_OPT_UEVENT			(1U << 0)
-#define FW_OPT_NOWAIT			(1U << 1)
-#define FW_OPT_USERHELPER		(1U << 2)
-#define FW_OPT_NO_WARN			(1U << 3)
-#define FW_OPT_NOCACHE			(1U << 4)
-#define FW_OPT_NOFALLBACK		(1U << 5)
+/**
+ * enum fw_opt - options to control firmware loading behaviour
+ *
+ * @FW_OPT_UEVENT: Enables the fallback mechanism to send a kobject uevent
+ *	when the firmware is not found. Userspace is in charge to load the
+ *	firmware using the sysfs loading facility.
+ * @FW_OPT_NOWAIT: Used to describe the firmware request is asynchronous.
+ * @FW_OPT_USERHELPER: Enable the fallback mechanism, in case the direct
+ *	filesystem lookup fails at finding the firmware.  For details refer to
+ *	fw_sysfs_fallback().
+ * @FW_OPT_NO_WARN: Quiet, avoid printing warning messages.
+ * @FW_OPT_NOCACHE: Disables firmware caching. Firmware caching is used to
+ *	cache the firmware upon suspend, so that upon resume races against the
+ *	firmware file lookup on storage is avoided. Used for calls where the
+ *	file may be too big, or where the driver takes charge of its own
+ *	firmware caching mechanism.
+ * @FW_OPT_NOFALLBACK: Disable the fallback mechanism. Takes precedence over
+ *	&FW_OPT_UEVENT and &FW_OPT_USERHELPER.
+ */
+enum fw_opt {
+	FW_OPT_UEVENT =         BIT(0),
+	FW_OPT_NOWAIT =         BIT(1),
+	FW_OPT_USERHELPER =     BIT(2),
+	FW_OPT_NO_WARN =        BIT(3),
+	FW_OPT_NOCACHE =        BIT(4),
+	FW_OPT_NOFALLBACK =     BIT(5),
+};
 
 enum fw_status {
 	FW_STATUS_UNKNOWN,
@@ -110,6 +131,6 @@ static inline void fw_state_done(struct fw_priv *fw_priv)
 }
 
 int assign_fw(struct firmware *fw, struct device *device,
-	      unsigned int opt_flags);
+	      enum fw_opt opt_flags);
 
 #endif /* __FIRMWARE_LOADER_H */
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index eb34089e4299..9919f0e6a7cc 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -443,7 +443,7 @@ static int fw_add_devm_name(struct device *dev, const char *name)
 #endif
 
 int assign_fw(struct firmware *fw, struct device *device,
-	      unsigned int opt_flags)
+	      enum fw_opt opt_flags)
 {
 	struct fw_priv *fw_priv = fw->priv;
 	int ret;
@@ -558,7 +558,7 @@ static void fw_abort_batch_reqs(struct firmware *fw)
 static int
 _request_firmware(const struct firmware **firmware_p, const char *name,
 		  struct device *device, void *buf, size_t size,
-		  unsigned int opt_flags)
+		  enum fw_opt opt_flags)
 {
 	struct firmware *fw = NULL;
 	int ret;
@@ -734,7 +734,7 @@ struct firmware_work {
 	struct device *device;
 	void *context;
 	void (*cont)(const struct firmware *fw, void *context);
-	unsigned int opt_flags;
+	enum fw_opt opt_flags;
 };
 
 static void request_firmware_work_func(struct work_struct *work)
-- 
2.17.0

^ permalink raw reply related

* [PATCH v6 00/13] firmware_loader changes for v4.18
From: Luis R. Rodriguez @ 2018-05-08 18:12 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc, davem

Greg,

Here is what I have queued up for the firmware_loader for v4.18. It
includes a slew of cleanup work, and the new firmware_request_nowarn()
which is quiet but enables the sysfs fallback mechanism. I've gone ahead
and also queued up a few minor fixes for the firmware loader documentation
which have come up recently. These changes are available on my git tree
both based on linux-next [0] and Linus' latest tree [1]. Folks working
on new developments for the firmware loader can use my linux-next
branch 20180508-firmware_loader_for-v4.18-try2 for now.

0-day sends its blessings.

The patches from Mimi's series still require a bit more discussion and
review. The discussion over the EFI firmware fallback mechanism is still
ongoing.

As for the rename that you wanted, perhaps we can do this late in the
merge window considering we're at rc4 now. I can prep something up for
that later.

Question, and specially rants are warmly welcomed.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20180508-firmware_loader_for-v4.18-try2
[1] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=20180508-firmware_loader_for-v4.18-try2

Andres Rodriguez (6):
  firmware: wrap FW_OPT_* into an enum
  firmware: use () to terminate kernel-doc function names
  firmware: rename fw_sysfs_fallback to firmware_fallback_sysfs()
  firmware: add firmware_request_nowarn() - load firmware without
    warnings
  ath10k: use firmware_request_nowarn() to load firmware
  ath10k: re-enable the firmware fallback mechanism for testmode

Luis R. Rodriguez (7):
  firmware_loader: document firmware_sysfs_fallback()
  firmware_loader: enhance Kconfig documentation over FW_LOADER
  firmware_loader: move kconfig FW_LOADER entries to its own file
  firmware_loader: make firmware_fallback_sysfs() print more useful
  Documentation: fix few typos and clarifications for the firmware
    loader
  Documentation: remove stale firmware API reference
  Documentation: clarify firmware_class provenance and why we can't
    rename the module

 Documentation/dell_rbu.txt                    |   3 -
 .../firmware/fallback-mechanisms.rst          |  14 +-
 .../driver-api/firmware/firmware_cache.rst    |   4 +-
 .../driver-api/firmware/request_firmware.rst  |   5 +
 drivers/base/Kconfig                          |  90 ++--------
 drivers/base/firmware_loader/Kconfig          | 154 ++++++++++++++++++
 drivers/base/firmware_loader/fallback.c       |  53 ++++--
 drivers/base/firmware_loader/fallback.h       |  18 +-
 drivers/base/firmware_loader/firmware.h       |  37 ++++-
 drivers/base/firmware_loader/main.c           |  57 +++++--
 drivers/net/wireless/ath/ath10k/core.c        |   2 +-
 drivers/net/wireless/ath/ath10k/testmode.c    |   2 +-
 include/linux/firmware.h                      |  10 ++
 13 files changed, 319 insertions(+), 130 deletions(-)
 create mode 100644 drivers/base/firmware_loader/Kconfig

-- 
2.17.0

^ permalink raw reply

* Re: Failed to clone net-next.git
From: Konstantin Ryabitsev @ 2018-05-08 18:06 UTC (permalink / raw)
  To: Song Liu, Networking; +Cc: Alexei Starovoitov
In-Reply-To: <FB252D9C-2FB2-437C-8646-4F61406AC96D@fb.com>


[-- Attachment #1.1: Type: text/plain, Size: 878 bytes --]

On 05/08/18 13:46, Song Liu wrote:
> We are seeing the following error on multiple different systems while 
> cloning net-next tree. 
> 
>   $ git clone https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
>   Cloning into 'net-next'...
>   remote: Counting objects: 6017287, done.
>   remote: Compressing objects: 100% (2458/2458), done.
>   fatal: The remote end hung up unexpectedly 1.00 GiB | 8.13 MiB/s
>   fatal: early EOF
>   fatal: index-pack failed
> 
> It looks like the size of the data being fetched reached server side
> limit of 1.00 GiB. So we probably need change server side configuration.
> Could someone please look into it?

It was probably due to a timeout value. Can you try it now, I've bumped
it to a much larger number.

Regards,
-- 
Konstantin Ryabitsev
Director, IT Infrastructure Security
The Linux Foundation


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [net-next PATCH 0/3] Symmetric queue selection using XPS for Rx queues
From: Nambiar, Amritha @ 2018-05-08 17:56 UTC (permalink / raw)
  To: Alexander Duyck, Eric Dumazet
  Cc: Tom Herbert, Eric Dumazet, netdev, David Miller, Alexander Duyck,
	Samudrala, Sridhar, Hannes Frederic Sowa
In-Reply-To: <CAKgT0Ud=msT46kmEASzxD-0gL7EEKSB6hkSJ6BDx0gE3wpMxow@mail.gmail.com>

On 5/8/2018 10:19 AM, Alexander Duyck wrote:
> On Tue, May 8, 2018 at 10:07 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>>
>> On 05/08/2018 09:02 AM, Alexander Duyck wrote:
>>> On Tue, May 8, 2018 at 8:15 AM, Tom Herbert <tom@herbertland.com> wrote:
>>>> On Thu, Apr 19, 2018 at 7:41 PM, Eric Dumazet <edumazet@google.com> wrote:
>>>>> On Thu, Apr 19, 2018 at 6:07 PM Amritha Nambiar <amritha.nambiar@intel.com>
>>>>> wrote:
>>>>>
>>>>>> This patch series implements support for Tx queue selection based on
>>>>>> Rx queue map. This is done by configuring Rx queue map per Tx-queue
>>>>>> using sysfs attribute. If the user configuration for Rx queues does
>>>>>> not apply, then the Tx queue selection falls back to XPS using CPUs and
>>>>>> finally to hashing.
>>>>>
>>>>>> XPS is refactored to support Tx queue selection based on either the
>>>>>> CPU map or the Rx-queue map. The config option CONFIG_XPS needs to be
>>>>>> enabled. By default no receive queues are configured for the Tx queue.
>>>>>
>>>>>> - /sys/class/net/eth0/queues/tx-*/xps_rxqs
>>>>>
>>>>>> This is to enable sending packets on the same Tx-Rx queue pair as this
>>>>>> is useful for busy polling multi-threaded workloads where it is not
>>>>>> possible to pin the threads to a CPU. This is a rework of Sridhar's
>>>>>> patch for symmetric queueing via socket option:
>>>>>> https://www.spinics.net/lists/netdev/msg453106.html
>>>>>
>>>> I suspect this is an artifact of flow director which I believe
>>>> required queue pairs to be able to work (i.e. receive queue chose
>>>> hardware is determined send queue). But that was only required because
>>>> of hardware design, I don't see the rationale for introducing queue
>>>> pairs in the software stack. There's no need to correlate the transmit
>>>> path with receive path, no need to enforce a 1-1 mapping between RX
>>>> and TX queues, and the OOO mitigations should be sufficient when TX
>>>> queue changes for a flow.
>>>>
>>>> Tom
>>>
>>> If I am not mistaken I think there are benefits to doing this sort of
>>> thing with polling as it keeps the Tx work locked into the same queue
>>> pair that a given application is polling on. So as a result you can
>>> keep the interrupts contained to the queue pair that is being busy
>>> polled on and if the application cleans up the packets during the busy
>>> poll it ends up being a net savings in terms of both latency and power
>>> since the Tx clean-up happens sooner, and it happens on the queue that
>>> is already busy polling instead of possibly triggering an interrupt on
>>> another CPU.
>>>
>>> So for example in the case of routing and bridging workloads we
>>> already had code that would take the Rx queue and associate it to a Tx
>>> queue. One of the ideas behind doing this is to try and keep the CPU
>>> overhead low by having a 1:1 mapping. In the case of this code we
>>> allow for a little more flexibility in that you could have
>>> many-to-many mappings but the general idea and common use case is the
>>> same which is a 1:1 mapping.
>>
>>
>> I thought we had everything in place to be able to have this already.
>>
>> Setting IRQ affinities and XPS is certainly something doable.
>>
>> This is why I wanted a proper documentation of yet another way to reach the
>> same behavior.
> 
> IRQ affinities and XPS work for pure NAPI setups, but the problem is
> you have to also do application affinity in the case of busy polling
> which can provide some additional challenges since then you have to
> add code in your application to associate a given queue/CPU to a given
> application thread. I believe this is a way of simplifying this.
> 
> I agree on the documentation aspect. The usage of this should be well
> documented as well as the why of using it.
> 
> - Alex
> 
I'll submit another version of the series with the documentation added.

- Amritha

^ permalink raw reply

* Failed to clone net-next.git
From: Song Liu @ 2018-05-08 17:46 UTC (permalink / raw)
  To: Networking; +Cc: Alexei Starovoitov, Konstantin Ryabitsev

We are seeing the following error on multiple different systems while 
cloning net-next tree. 

  $ git clone https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
  Cloning into 'net-next'...
  remote: Counting objects: 6017287, done.
  remote: Compressing objects: 100% (2458/2458), done.
  fatal: The remote end hung up unexpectedly 1.00 GiB | 8.13 MiB/s
  fatal: early EOF
  fatal: index-pack failed

It looks like the size of the data being fetched reached server side
limit of 1.00 GiB. So we probably need change server side configuration.
Could someone please look into it?

If you see same problem. Here is a work-around for it: 
  1. clone a smaller tree, for example net.git;
  2. add net-next as a new remote, and fetch it. 

Thanks,
Song

^ permalink raw reply

* Re: general protection fault in encode_rpcb_string
From: Bruce Fields @ 2018-05-08 17:44 UTC (permalink / raw)
  To: Chuck Lever
  Cc: Trond Myklebust,
	syzbot+4b98281f2401ab849f4b@syzkaller.appspotmail.com,
	syzkaller-bugs@googlegroups.com, Anna Schumaker,
	davem@davemloft.net, linux-kernel@vger.kernel.org,
	Linux NFS Mailing List, jlayton@kernel.org,
	netdev@vger.kernel.org
In-Reply-To: <4B24FAE4-C7E8-4D01-9808-B8F4E9E59D64@gmail.com>

On Tue, May 08, 2018 at 12:34:48PM -0400, Chuck Lever wrote:
> 
> 
> > On May 8, 2018, at 12:15 PM, bfields@fieldses.org wrote:
> > 
> > On Tue, Apr 17, 2018 at 09:54:36PM +0000, Trond Myklebust wrote:
> >> Yes, and we can probably convert it, and the other GFP_ATOMIC
> >> allocations in the rpcbind client to use GFP_NOFS in order to improve
> >> reliability.
> > 
> > Chuck, I think the GFP_ATOMIC is unnecessary here as well?
> > 
> > --b.
> > 
> > diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
> > index e8adad33d0bb..de90c6c90cde 100644
> > --- a/net/sunrpc/xprtrdma/rpc_rdma.c
> > +++ b/net/sunrpc/xprtrdma/rpc_rdma.c
> > @@ -228,7 +228,7 @@ rpcrdma_convert_iovs(struct rpcrdma_xprt *r_xprt, struct xdr_buf *xdrbuf,
> > 			/* XXX: Certain upper layer operations do
> > 			 *	not provide receive buffer pages.
> > 			 */
> > -			*ppages = alloc_page(GFP_ATOMIC);
> > +			*ppages = alloc_page(GFP_NOFS);
> > 			if (!*ppages)
> > 				return -EAGAIN;
> > 		}
> 
> This code can't sleep, as I understand it. Caller is holding
> the transport write lock. This logic was copied from
> xdr_partial_copy_from_skb, which uses GFP_ATOMIC.

OK.

> Recall that this is here because of GETACL. As I've stated in
> the past, the correct solution is to ensure that these pages
> are provided in every case by the upper layer, making this
> alloc_page call site unnecessary.

Got it.

--b.

^ permalink raw reply

* Re: KASAN: use-after-free Read in sctp_do_sm
From: Xin Long @ 2018-05-08 17:41 UTC (permalink / raw)
  To: syzbot
  Cc: davem, LKML, linux-sctp, Marcelo Ricardo Leitner, network dev,
	Neil Horman, syzkaller-bugs, Vlad Yasevich
In-Reply-To: <000000000000c10690056bb22ccd@google.com>

On Tue, May 8, 2018 at 9:58 PM, syzbot
<syzbot+141d898c5f24489db4aa@syzkaller.appspotmail.com> wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    f142f08bf7ec Fix typo in comment.
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=1159ade7800000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=31f4b3733894ef79
> dashboard link: https://syzkaller.appspot.com/bug?extid=141d898c5f24489db4aa
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+141d898c5f24489db4aa@syzkaller.appspotmail.com
>
> RDX: 0000000000000008 RSI: 0000000020000000 RDI: 0000000000000014
> RBP: 000000000072bea0 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000015
> R13: 000000000000071e R14: 00000000006feb70 R15: 0000000000000007
> ==================================================================
> BUG: KASAN: use-after-free in sctp_cmd_interpreter
> net/sctp/sm_sideeffect.c:1817 [inline]
> BUG: KASAN: use-after-free in sctp_side_effects
> net/sctp/sm_sideeffect.c:1220 [inline]
> BUG: KASAN: use-after-free in sctp_do_sm+0x6015/0x7160
> net/sctp/sm_sideeffect.c:1191
> Read of size 1 at addr ffff8801c7883cb8 by task syz-executor6/18616
>
> CPU: 1 PID: 18616 Comm: syz-executor6 Not tainted 4.17.0-rc4+ #38
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
>  print_address_description+0x6c/0x20b mm/kasan/report.c:256
>  kasan_report_error mm/kasan/report.c:354 [inline]
>  kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
>  __asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:430
>  sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1817 [inline]
>  sctp_side_effects net/sctp/sm_sideeffect.c:1220 [inline]
>  sctp_do_sm+0x6015/0x7160 net/sctp/sm_sideeffect.c:1191
>  sctp_assoc_bh_rcv+0x30f/0x520 net/sctp/associola.c:1065
>  sctp_inq_push+0x263/0x320 net/sctp/inqueue.c:95
>  sctp_backlog_rcv+0x192/0xc00 net/sctp/input.c:350
>  sk_backlog_rcv include/net/sock.h:909 [inline]
>  __release_sock+0x12f/0x3a0 net/core/sock.c:2335
>  release_sock+0xa4/0x2b0 net/core/sock.c:2850
>  sctp_sendmsg+0x13cc/0x1d70 net/sctp/socket.c:2128
>  inet_sendmsg+0x19f/0x690 net/ipv4/af_inet.c:798
>  sock_sendmsg_nosec net/socket.c:629 [inline]
>  sock_sendmsg+0xd5/0x120 net/socket.c:639
>  sock_write_iter+0x35a/0x5a0 net/socket.c:908
>  call_write_iter include/linux/fs.h:1784 [inline]
>  new_sync_write fs/read_write.c:474 [inline]
>  __vfs_write+0x64d/0x960 fs/read_write.c:487
>  vfs_write+0x1f8/0x560 fs/read_write.c:549
>  ksys_write+0xf9/0x250 fs/read_write.c:598
>  __do_sys_write fs/read_write.c:610 [inline]
>  __se_sys_write fs/read_write.c:607 [inline]
>  __x64_sys_write+0x73/0xb0 fs/read_write.c:607
>  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x455979
> RSP: 002b:00007f6fad842c68 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 00007f6fad8436d4 RCX: 0000000000455979
> RDX: 0000000000000008 RSI: 0000000020000000 RDI: 0000000000000014
> RBP: 000000000072bea0 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000015
> R13: 000000000000071e R14: 00000000006feb70 R15: 0000000000000007
>
> Allocated by task 18616:
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
>  set_track mm/kasan/kasan.c:460 [inline]
>  kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
>  kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:490
>  kmem_cache_alloc+0x12e/0x760 mm/slab.c:3554
>  kmem_cache_zalloc include/linux/slab.h:691 [inline]
>  sctp_chunkify+0xce/0x400 net/sctp/sm_make_chunk.c:1355
>  sctp_rcv+0xc65/0x3a60 net/sctp/input.c:221
>  sctp6_rcv+0x15/0x30 net/sctp/ipv6.c:1045
>  ip6_input_finish+0x3ff/0x1a30 net/ipv6/ip6_input.c:284
>  NF_HOOK include/linux/netfilter.h:288 [inline]
>  ip6_input+0xe1/0x5e0 net/ipv6/ip6_input.c:327
>  dst_input include/net/dst.h:450 [inline]
>  ip6_rcv_finish+0x29c/0xa10 net/ipv6/ip6_input.c:71
>  NF_HOOK include/linux/netfilter.h:288 [inline]
>  ipv6_rcv+0xed6/0x22a0 net/ipv6/ip6_input.c:208
>  __netif_receive_skb_core+0x26f5/0x3630 net/core/dev.c:4592
>  __netif_receive_skb+0x2c/0x1e0 net/core/dev.c:4657
>  process_backlog+0x219/0x760 net/core/dev.c:5337
>  napi_poll net/core/dev.c:5735 [inline]
>  net_rx_action+0x7b7/0x1930 net/core/dev.c:5801
>  __do_softirq+0x2e0/0xaf5 kernel/softirq.c:285
>
> Freed by task 18616:
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
>  set_track mm/kasan/kasan.c:460 [inline]
>  __kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
>  kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
>  __cache_free mm/slab.c:3498 [inline]
>  kmem_cache_free+0x86/0x2d0 mm/slab.c:3756
>  sctp_chunk_destroy net/sctp/sm_make_chunk.c:1481 [inline]
>  sctp_chunk_put+0x321/0x440 net/sctp/sm_make_chunk.c:1504
>  sctp_ulpevent_make_rcvmsg+0x955/0xd40 net/sctp/ulpevent.c:718
There's no reason to put the chunk in sctp_ulpevent_make_rcvmsg's
fail_mark err path before holding this chunk later there.

We should just remove it.

@@ -715,7 +715,6 @@ struct sctp_ulpevent
*sctp_ulpevent_make_rcvmsg(struct sctp_association *asoc,
        return event;

 fail_mark:
-       sctp_chunk_put(chunk);
        kfree_skb(skb);
 fail:
        return NULL;

>  sctp_ulpq_tail_data+0xa8/0x12b0 net/sctp/ulpqueue.c:108
>  sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1478 [inline]
>  sctp_side_effects net/sctp/sm_sideeffect.c:1220 [inline]
>  sctp_do_sm+0x1658/0x7160 net/sctp/sm_sideeffect.c:1191
>  sctp_assoc_bh_rcv+0x30f/0x520 net/sctp/associola.c:1065
>  sctp_inq_push+0x263/0x320 net/sctp/inqueue.c:95
>  sctp_backlog_rcv+0x192/0xc00 net/sctp/input.c:350
>  sk_backlog_rcv include/net/sock.h:909 [inline]
>  __release_sock+0x12f/0x3a0 net/core/sock.c:2335
>  release_sock+0xa4/0x2b0 net/core/sock.c:2850
>  sctp_sendmsg+0x13cc/0x1d70 net/sctp/socket.c:2128
>  inet_sendmsg+0x19f/0x690 net/ipv4/af_inet.c:798
>  sock_sendmsg_nosec net/socket.c:629 [inline]
>  sock_sendmsg+0xd5/0x120 net/socket.c:639
>  sock_write_iter+0x35a/0x5a0 net/socket.c:908
>  call_write_iter include/linux/fs.h:1784 [inline]
>  new_sync_write fs/read_write.c:474 [inline]
>  __vfs_write+0x64d/0x960 fs/read_write.c:487
>  vfs_write+0x1f8/0x560 fs/read_write.c:549
>  ksys_write+0xf9/0x250 fs/read_write.c:598
>  __do_sys_write fs/read_write.c:610 [inline]
>  __se_sys_write fs/read_write.c:607 [inline]
>  __x64_sys_write+0x73/0xb0 fs/read_write.c:607
>  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> The buggy address belongs to the object at ffff8801c7883bc0
>  which belongs to the cache sctp_chunk of size 256
> The buggy address is located 248 bytes inside of
>  256-byte region [ffff8801c7883bc0, ffff8801c7883cc0)
> The buggy address belongs to the page:
> page:ffffea00071e20c0 count:1 mapcount:0 mapping:ffff8801c7883080 index:0x0
> flags: 0x2fffc0000000100(slab)
> raw: 02fffc0000000100 ffff8801c7883080 0000000000000000 000000010000000c
> raw: ffffea000723a220 ffff8801cdb66f48 ffff8801cdb65600 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
>  ffff8801c7883b80: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
>  ffff8801c7883c00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>>
>> ffff8801c7883c80: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
>
>                                         ^
>  ffff8801c7883d00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>  ffff8801c7883d80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ==================================================================
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report.
> If you forgot to add the Reported-by tag, once the fix for this bug is
> merged
> into any tree, please reply to this email with:
> #syz fix: exact-commit-title
> To mark this as a duplicate of another syzbot report, please reply with:
> #syz dup: exact-subject-of-another-report
> If it's a one-off invalid bug report, please reply with:
> #syz invalid
> Note: if the crash happens again, it will cause creation of a new bug
> report.
> Note: all commands must start from beginning of the line in the email body.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [net-next 2/6] fm10k: reduce duplicate fm10k_stat macro code
From: Keller, Jacob E @ 2018-05-08 17:40 UTC (permalink / raw)
  To: Joe Perches, Kirsher, Jeffrey T, davem@davemloft.net
  Cc: netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com,
	jogreene@redhat.com
In-Reply-To: <63a9f68a10b1b4754918df527b0697ee9d435753.camel@perches.com>

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Joe Perches
> Sent: Tuesday, May 08, 2018 10:00 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; davem@davemloft.net
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; netdev@vger.kernel.org;
> nhorman@redhat.com; sassmann@redhat.com; jogreene@redhat.com
> Subject: Re: [net-next 2/6] fm10k: reduce duplicate fm10k_stat macro code
> 
> On Mon, 2018-05-07 at 07:45 -0700, Jeff Kirsher wrote:
> > Share some of the code for setting up fm10k_stat macros by implementing
> > an FM10K_STAT_FIELDS macro which we can use when setting up the type
> > specific macros.
> []
> > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
> b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
> []
> > @@ -11,12 +11,16 @@ struct fm10k_stats {
> >  	int stat_offset;
> >  };o
> >
> > -#define FM10K_NETDEV_STAT(_net_stat) { \
> > -	.stat_string = #_net_stat, \
> > -	.sizeof_stat = FIELD_SIZEOF(struct net_device_stats, _net_stat), \
> > -	.stat_offset = offsetof(struct net_device_stats, _net_stat) \
> > +#define FM10K_STAT_FIELDS(_type, _name, _stat) { \
> > +	.stat_string = _name, \
> > +	.sizeof_stat = FIELD_SIZEOF(_type, _stat), \
> > +	.stat_offset = offsetof(_type, _stat) \
> >  }
> >
> > +/* netdevice statistics */
> > +#define FM10K_NETDEV_STAT(_net_stat) \
> > +	FM10K_STAT_FIELDS(struct net_device_stats, #_net_stat, _net_stat)
> 
> trivia:
> 
> It's somewhat unusual to use # in a macro argument.
> Perhaps this would be slightly easier to understand using __stringify
> 
> #define FM10K_NETDEV_STAT(_net_stat) \
> 	FM10K_STAT_FIELDS(struct net_device_stats, __stringify(_net_stat),
> _net_stat)

Makes sense. Will change.

Thanks,
Jake

^ permalink raw reply

* [PATCH] hv_netvsc: Fix net device attach on older Windows hosts
From: Mohammed Gamal @ 2018-05-08 17:40 UTC (permalink / raw)
  To: netdev, sthemmin
  Cc: kys, haiyangz, devel, vkuznets, linux-kernel, Mohammed Gamal

On older windows hosts the net_device instance is returned to
the caller of rndis_filter_device_add() without having the presence
bit set first. This would cause any subsequent calls to network device
operations (e.g. MTU change, channel change) to fail after the device
is detached once, returning -ENODEV.

Make sure we explicitly call netif_device_attach() before returning
the net_device instance to make sure the presence bit is set

Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")

Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
---
 drivers/net/hyperv/rndis_filter.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 6b127be..09a3c1d 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -1287,8 +1287,10 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
 		   rndis_device->hw_mac_adr,
 		   rndis_device->link_state ? "down" : "up");
 
-	if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5)
+	if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5) {
+		netif_device_attach(net);
 		return net_device;
+	}
 
 	rndis_filter_query_link_speed(rndis_device, net_device);
 
-- 
1.8.3.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox