diff mbox series

[v7,3/8] firmware: Rename FW_OPT_NOFALLBACK to FW_OPT_NOFALLBACK_SYSFS

Message ID 20191004145056.43267-4-hdegoede@redhat.com (mailing list archive)
State Superseded
Headers show
Series efi/firmware/platform-x86: Add EFI embedded fw support | expand

Commit Message

Hans de Goede Oct. 4, 2019, 2:50 p.m. UTC
This is a preparation patch for adding a new platform fallback mechanism,
which will have its own enable/disable FW_OPT_xxx option.

Note this also fixes a typo in one of the re-wordwrapped comments:
enfoce -> enforce.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/base/firmware_loader/fallback.c | 11 ++++++-----
 drivers/base/firmware_loader/firmware.h | 16 ++++++++--------
 drivers/base/firmware_loader/main.c     |  2 +-
 3 files changed, 15 insertions(+), 14 deletions(-)

Comments

Luis Chamberlain Oct. 11, 2019, 3:02 p.m. UTC | #1
On Fri, Oct 04, 2019 at 04:50:51PM +0200, Hans de Goede wrote:
> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
> index 62ee90b4db56..665b350419cb 100644
> --- a/drivers/base/firmware_loader/fallback.c
> +++ b/drivers/base/firmware_loader/fallback.c
> @@ -606,7 +606,7 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
>  		return false;
>  	}
>  
> -	if ((opt_flags & FW_OPT_NOFALLBACK))
> +	if ((opt_flags & FW_OPT_NOFALLBACK_SYSFS))
>  		return false;
>  
>  	/* Also permit LSMs and IMA to fail firmware sysfs fallback */
> @@ -630,10 +630,11 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
>   * interface. Userspace is in charge of loading the firmware through the sysfs
>   * loading interface. This sysfs 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.
> + * If this false we check if the internal API caller set the
         ignore_sysfs_fallback set to true or force_sysfs_fallback is
	 set to false

Otherwise looks good. You can add:

Acked-by: Luis Chamberlain <mcgrof@kernel.org>

  Luis
Hans de Goede Nov. 14, 2019, 8:22 p.m. UTC | #2
Hi,

On 11-10-2019 17:02, Luis Chamberlain wrote:
> On Fri, Oct 04, 2019 at 04:50:51PM +0200, Hans de Goede wrote:
>> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
>> index 62ee90b4db56..665b350419cb 100644
>> --- a/drivers/base/firmware_loader/fallback.c
>> +++ b/drivers/base/firmware_loader/fallback.c
>> @@ -606,7 +606,7 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
>>   		return false;
>>   	}
>>   
>> -	if ((opt_flags & FW_OPT_NOFALLBACK))
>> +	if ((opt_flags & FW_OPT_NOFALLBACK_SYSFS))
>>   		return false;
>>   
>>   	/* Also permit LSMs and IMA to fail firmware sysfs fallback */
>> @@ -630,10 +630,11 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
>>    * interface. Userspace is in charge of loading the firmware through the sysfs
>>    * loading interface. This sysfs 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.
>> + * If this false we check if the internal API caller set the
>           ignore_sysfs_fallback set to true or force_sysfs_fallback is
> 	 set to false

I do not think that that is correct, looking at the code the order of
checks is:

	if (fw_fallback_config.ignore_sysfs_fallback)
		BAIL

	if (opt_flags & FW_OPT_NOFALLBACK_SYSFS)
		BAIL

	if (fw_fallback_config.force_sysfs_fallback)
		DO_FALLBACK (and return)

	if (!(opt_flags & FW_OPT_USERHELPER))
		BAIL

	DO_FALLBACK

So the original comment seems correct as FW_OPT_NOFALLBACK trumps / has
higher prio then force_sysfs_fallback.

Anyways I do not believe that fixing up/rewording this comment (if it needs
fixing) belongs in the commit/patch. This patch is purely about renaming
FW_OPT_NOFALLBACK to FW_OPT_NOFALLBACK_SYSFS, the lines changed in this
chunk are not changed, they are merely re-word/line-wrapped with the
exception of fixing the enfoce typo to enforce, as mentioned in the
commit message.

Regards,

Hans
diff mbox series

Patch

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 62ee90b4db56..665b350419cb 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -606,7 +606,7 @@  static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
 		return false;
 	}
 
-	if ((opt_flags & FW_OPT_NOFALLBACK))
+	if ((opt_flags & FW_OPT_NOFALLBACK_SYSFS))
 		return false;
 
 	/* Also permit LSMs and IMA to fail firmware sysfs fallback */
@@ -630,10 +630,11 @@  static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
  * interface. Userspace is in charge of loading the firmware through the sysfs
  * loading interface. This sysfs 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.
+ * If this false we check if the internal API caller set the
+ * @FW_OPT_NOFALLBACK_SYSFS flag, if so it would also disable the fallback
+ * mechanism. A system may want to enforce 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.
  **/
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index 7ecd590e67fe..8656e5239a80 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -27,16 +27,16 @@ 
  *	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.
+ * @FW_OPT_NOFALLBACK_SYSFS: Disable the sysfs 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),
+	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_SYSFS		= BIT(5),
 };
 
 enum fw_status {
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index bf44c79beae9..08f8995a530a 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -876,7 +876,7 @@  int request_firmware_direct(const struct firmware **firmware_p,
 	__module_get(THIS_MODULE);
 	ret = _request_firmware(firmware_p, name, device, NULL, 0,
 				FW_OPT_UEVENT | FW_OPT_NO_WARN |
-				FW_OPT_NOFALLBACK);
+				FW_OPT_NOFALLBACK_SYSFS);
 	module_put(THIS_MODULE);
 	return ret;
 }