diff mbox series

[v2,2/4] powerpc/rtas: make all exports GPL

Message ID 20230124140448.45938-3-nathanl@linux.ibm.com (mailing list archive)
State Accepted
Commit 9bce6243848dfd0ff7c2be6e8d82ab9b1e6c7858
Headers show
Series powerpc/rtas: exports and locking | expand

Commit Message

Nathan Lynch Jan. 24, 2023, 2:04 p.m. UTC
The first symbol exports of RTAS functions and data came with the (now
removed) scanlog driver in 2003:

https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=f92e361842d5251e50562b09664082dcbd0548bb

At the time this was applied, EXPORT_SYMBOL_GPL() was very new, and
the exports of rtas_call() etc have remained non-GPL. As new APIs have
been added to the RTAS subsystem, their symbol exports have followed
the convention set by existing code.

However, the historical evidence is that RTAS function exports have
been added over time only to satisfy the needs of in-kernel users, and
these clients must have fairly intimate knowledge of how the APIs work
to use them safely. No out of tree users are known, and future ones
seem unlikely.

Arguably the default for RTAS symbols should have become
EXPORT_SYMBOL_GPL once it was available. Let's make it so now, and
exceptions can be evaluated as needed.

Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
 arch/powerpc/kernel/rtas.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Comments

Laurent Dufour Jan. 24, 2023, 4:18 p.m. UTC | #1
On 24/01/2023 15:04:46, Nathan Lynch wrote:
> The first symbol exports of RTAS functions and data came with the (now
> removed) scanlog driver in 2003:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=f92e361842d5251e50562b09664082dcbd0548bb
> 
> At the time this was applied, EXPORT_SYMBOL_GPL() was very new, and
> the exports of rtas_call() etc have remained non-GPL. As new APIs have
> been added to the RTAS subsystem, their symbol exports have followed
> the convention set by existing code.
> 
> However, the historical evidence is that RTAS function exports have
> been added over time only to satisfy the needs of in-kernel users, and
> these clients must have fairly intimate knowledge of how the APIs work
> to use them safely. No out of tree users are known, and future ones
> seem unlikely.
> 
> Arguably the default for RTAS symbols should have become
> EXPORT_SYMBOL_GPL once it was available. Let's make it so now, and
> exceptions can be evaluated as needed.

I also think this is unlikely to happen. But in the case a non GPL driver
needs one of this symbol, I guess it will be hard to move backward once it
is upstream. Crossing fingers!

Reviewed-by: Laurent Dufour <laurent.dufour@fr.ibm.com>

> 
> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
> ---
>  arch/powerpc/kernel/rtas.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 6c5716b19d69..e60e2f5af7b9 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -65,10 +65,10 @@ struct rtas_t rtas = {
>  };
>  
>  DEFINE_SPINLOCK(rtas_data_buf_lock);
> -EXPORT_SYMBOL(rtas_data_buf_lock);
> +EXPORT_SYMBOL_GPL(rtas_data_buf_lock);
>  
>  char rtas_data_buf[RTAS_DATA_BUF_SIZE] __cacheline_aligned;
> -EXPORT_SYMBOL(rtas_data_buf);
> +EXPORT_SYMBOL_GPL(rtas_data_buf);
>  
>  unsigned long rtas_rmo_buf;
>  
> @@ -77,7 +77,7 @@ unsigned long rtas_rmo_buf;
>   * This is done like this so rtas_flash can be a module.
>   */
>  void (*rtas_flash_term_hook)(int);
> -EXPORT_SYMBOL(rtas_flash_term_hook);
> +EXPORT_SYMBOL_GPL(rtas_flash_term_hook);
>  
>  /* RTAS use home made raw locking instead of spin_lock_irqsave
>   * because those can be called from within really nasty contexts
> @@ -325,7 +325,7 @@ void rtas_progress(char *s, unsigned short hex)
>   
>  	spin_unlock(&progress_lock);
>  }
> -EXPORT_SYMBOL(rtas_progress);		/* needed by rtas_flash module */
> +EXPORT_SYMBOL_GPL(rtas_progress);		/* needed by rtas_flash module */
>  
>  int rtas_token(const char *service)
>  {
> @@ -335,13 +335,13 @@ int rtas_token(const char *service)
>  	tokp = of_get_property(rtas.dev, service, NULL);
>  	return tokp ? be32_to_cpu(*tokp) : RTAS_UNKNOWN_SERVICE;
>  }
> -EXPORT_SYMBOL(rtas_token);
> +EXPORT_SYMBOL_GPL(rtas_token);
>  
>  int rtas_service_present(const char *service)
>  {
>  	return rtas_token(service) != RTAS_UNKNOWN_SERVICE;
>  }
> -EXPORT_SYMBOL(rtas_service_present);
> +EXPORT_SYMBOL_GPL(rtas_service_present);
>  
>  #ifdef CONFIG_RTAS_ERROR_LOGGING
>  
> @@ -356,7 +356,7 @@ int rtas_get_error_log_max(void)
>  {
>  	return rtas_error_log_max;
>  }
> -EXPORT_SYMBOL(rtas_get_error_log_max);
> +EXPORT_SYMBOL_GPL(rtas_get_error_log_max);
>  
>  static void __init init_error_log_max(void)
>  {
> @@ -584,7 +584,7 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...)
>  	}
>  	return ret;
>  }
> -EXPORT_SYMBOL(rtas_call);
> +EXPORT_SYMBOL_GPL(rtas_call);
>  
>  /**
>   * rtas_busy_delay_time() - From an RTAS status value, calculate the
> @@ -622,7 +622,7 @@ unsigned int rtas_busy_delay_time(int status)
>  
>  	return ms;
>  }
> -EXPORT_SYMBOL(rtas_busy_delay_time);
> +EXPORT_SYMBOL_GPL(rtas_busy_delay_time);
>  
>  /**
>   * rtas_busy_delay() - helper for RTAS busy and extended delay statuses
> @@ -696,7 +696,7 @@ bool rtas_busy_delay(int status)
>  
>  	return ret;
>  }
> -EXPORT_SYMBOL(rtas_busy_delay);
> +EXPORT_SYMBOL_GPL(rtas_busy_delay);
>  
>  static int rtas_error_rc(int rtas_rc)
>  {
> @@ -741,7 +741,7 @@ int rtas_get_power_level(int powerdomain, int *level)
>  		return rtas_error_rc(rc);
>  	return rc;
>  }
> -EXPORT_SYMBOL(rtas_get_power_level);
> +EXPORT_SYMBOL_GPL(rtas_get_power_level);
>  
>  int rtas_set_power_level(int powerdomain, int level, int *setlevel)
>  {
> @@ -759,7 +759,7 @@ int rtas_set_power_level(int powerdomain, int level, int *setlevel)
>  		return rtas_error_rc(rc);
>  	return rc;
>  }
> -EXPORT_SYMBOL(rtas_set_power_level);
> +EXPORT_SYMBOL_GPL(rtas_set_power_level);
>  
>  int rtas_get_sensor(int sensor, int index, int *state)
>  {
> @@ -777,7 +777,7 @@ int rtas_get_sensor(int sensor, int index, int *state)
>  		return rtas_error_rc(rc);
>  	return rc;
>  }
> -EXPORT_SYMBOL(rtas_get_sensor);
> +EXPORT_SYMBOL_GPL(rtas_get_sensor);
>  
>  int rtas_get_sensor_fast(int sensor, int index, int *state)
>  {
> @@ -820,7 +820,7 @@ bool rtas_indicator_present(int token, int *maxindex)
>  
>  	return false;
>  }
> -EXPORT_SYMBOL(rtas_indicator_present);
> +EXPORT_SYMBOL_GPL(rtas_indicator_present);
>  
>  int rtas_set_indicator(int indicator, int index, int new_value)
>  {
> @@ -838,7 +838,7 @@ int rtas_set_indicator(int indicator, int index, int new_value)
>  		return rtas_error_rc(rc);
>  	return rc;
>  }
> -EXPORT_SYMBOL(rtas_set_indicator);
> +EXPORT_SYMBOL_GPL(rtas_set_indicator);
>  
>  /*
>   * Ignoring RTAS extended delay
Andrew Donnellan Feb. 2, 2023, 4 a.m. UTC | #2
On Tue, 2023-01-24 at 08:04 -0600, Nathan Lynch wrote:
> The first symbol exports of RTAS functions and data came with the
> (now
> removed) scanlog driver in 2003:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=f92e361842d5251e50562b09664082dcbd0548bb
> 
> At the time this was applied, EXPORT_SYMBOL_GPL() was very new, and
> the exports of rtas_call() etc have remained non-GPL. As new APIs
> have
> been added to the RTAS subsystem, their symbol exports have followed
> the convention set by existing code.
> 
> However, the historical evidence is that RTAS function exports have
> been added over time only to satisfy the needs of in-kernel users,
> and
> these clients must have fairly intimate knowledge of how the APIs
> work
> to use them safely. No out of tree users are known, and future ones
> seem unlikely.
> 
> Arguably the default for RTAS symbols should have become
> EXPORT_SYMBOL_GPL once it was available. Let's make it so now, and
> exceptions can be evaluated as needed.
> 
> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>

Agreed.

Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
diff mbox series

Patch

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 6c5716b19d69..e60e2f5af7b9 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -65,10 +65,10 @@  struct rtas_t rtas = {
 };
 
 DEFINE_SPINLOCK(rtas_data_buf_lock);
-EXPORT_SYMBOL(rtas_data_buf_lock);
+EXPORT_SYMBOL_GPL(rtas_data_buf_lock);
 
 char rtas_data_buf[RTAS_DATA_BUF_SIZE] __cacheline_aligned;
-EXPORT_SYMBOL(rtas_data_buf);
+EXPORT_SYMBOL_GPL(rtas_data_buf);
 
 unsigned long rtas_rmo_buf;
 
@@ -77,7 +77,7 @@  unsigned long rtas_rmo_buf;
  * This is done like this so rtas_flash can be a module.
  */
 void (*rtas_flash_term_hook)(int);
-EXPORT_SYMBOL(rtas_flash_term_hook);
+EXPORT_SYMBOL_GPL(rtas_flash_term_hook);
 
 /* RTAS use home made raw locking instead of spin_lock_irqsave
  * because those can be called from within really nasty contexts
@@ -325,7 +325,7 @@  void rtas_progress(char *s, unsigned short hex)
  
 	spin_unlock(&progress_lock);
 }
-EXPORT_SYMBOL(rtas_progress);		/* needed by rtas_flash module */
+EXPORT_SYMBOL_GPL(rtas_progress);		/* needed by rtas_flash module */
 
 int rtas_token(const char *service)
 {
@@ -335,13 +335,13 @@  int rtas_token(const char *service)
 	tokp = of_get_property(rtas.dev, service, NULL);
 	return tokp ? be32_to_cpu(*tokp) : RTAS_UNKNOWN_SERVICE;
 }
-EXPORT_SYMBOL(rtas_token);
+EXPORT_SYMBOL_GPL(rtas_token);
 
 int rtas_service_present(const char *service)
 {
 	return rtas_token(service) != RTAS_UNKNOWN_SERVICE;
 }
-EXPORT_SYMBOL(rtas_service_present);
+EXPORT_SYMBOL_GPL(rtas_service_present);
 
 #ifdef CONFIG_RTAS_ERROR_LOGGING
 
@@ -356,7 +356,7 @@  int rtas_get_error_log_max(void)
 {
 	return rtas_error_log_max;
 }
-EXPORT_SYMBOL(rtas_get_error_log_max);
+EXPORT_SYMBOL_GPL(rtas_get_error_log_max);
 
 static void __init init_error_log_max(void)
 {
@@ -584,7 +584,7 @@  int rtas_call(int token, int nargs, int nret, int *outputs, ...)
 	}
 	return ret;
 }
-EXPORT_SYMBOL(rtas_call);
+EXPORT_SYMBOL_GPL(rtas_call);
 
 /**
  * rtas_busy_delay_time() - From an RTAS status value, calculate the
@@ -622,7 +622,7 @@  unsigned int rtas_busy_delay_time(int status)
 
 	return ms;
 }
-EXPORT_SYMBOL(rtas_busy_delay_time);
+EXPORT_SYMBOL_GPL(rtas_busy_delay_time);
 
 /**
  * rtas_busy_delay() - helper for RTAS busy and extended delay statuses
@@ -696,7 +696,7 @@  bool rtas_busy_delay(int status)
 
 	return ret;
 }
-EXPORT_SYMBOL(rtas_busy_delay);
+EXPORT_SYMBOL_GPL(rtas_busy_delay);
 
 static int rtas_error_rc(int rtas_rc)
 {
@@ -741,7 +741,7 @@  int rtas_get_power_level(int powerdomain, int *level)
 		return rtas_error_rc(rc);
 	return rc;
 }
-EXPORT_SYMBOL(rtas_get_power_level);
+EXPORT_SYMBOL_GPL(rtas_get_power_level);
 
 int rtas_set_power_level(int powerdomain, int level, int *setlevel)
 {
@@ -759,7 +759,7 @@  int rtas_set_power_level(int powerdomain, int level, int *setlevel)
 		return rtas_error_rc(rc);
 	return rc;
 }
-EXPORT_SYMBOL(rtas_set_power_level);
+EXPORT_SYMBOL_GPL(rtas_set_power_level);
 
 int rtas_get_sensor(int sensor, int index, int *state)
 {
@@ -777,7 +777,7 @@  int rtas_get_sensor(int sensor, int index, int *state)
 		return rtas_error_rc(rc);
 	return rc;
 }
-EXPORT_SYMBOL(rtas_get_sensor);
+EXPORT_SYMBOL_GPL(rtas_get_sensor);
 
 int rtas_get_sensor_fast(int sensor, int index, int *state)
 {
@@ -820,7 +820,7 @@  bool rtas_indicator_present(int token, int *maxindex)
 
 	return false;
 }
-EXPORT_SYMBOL(rtas_indicator_present);
+EXPORT_SYMBOL_GPL(rtas_indicator_present);
 
 int rtas_set_indicator(int indicator, int index, int new_value)
 {
@@ -838,7 +838,7 @@  int rtas_set_indicator(int indicator, int index, int new_value)
 		return rtas_error_rc(rc);
 	return rc;
 }
-EXPORT_SYMBOL(rtas_set_indicator);
+EXPORT_SYMBOL_GPL(rtas_set_indicator);
 
 /*
  * Ignoring RTAS extended delay