[PATCH v3 1/2] powerpc/powernv: Enhance opal message read interface

[PATCH v3 1/2] powerpc/powernv: Enhance opal message read interface

[Vasant Hegde]

Use "opal-msg-size" device tree property to allocate memory for "opal_msg".

Cc: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Cc: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
---
Changes in v3:
  - Call BUG_ON, if we fail to allocate memory during init.

-Vasant

 arch/powerpc/platforms/powernv/opal.c | 29 ++++++++++++++++++---------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index aba443be7daa..4f1f68f568bf 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -58,6 +58,8 @@ static DEFINE_SPINLOCK(opal_write_lock);
 static struct atomic_notifier_head opal_msg_notifier_head[OPAL_MSG_TYPE_MAX];
 static uint32_t opal_heartbeat;
 static struct task_struct *kopald_tsk;
+static struct opal_msg *opal_msg;
+static uint64_t opal_msg_size;
 
 void opal_configure_cores(void)
 {
@@ -271,14 +273,9 @@ static void opal_message_do_notify(uint32_t msg_type, void *msg)
 static void opal_handle_message(void)
 {
 	s64 ret;
-	/*
-	 * TODO: pre-allocate a message buffer depending on opal-msg-size
-	 * value in /proc/device-tree.
-	 */
-	static struct opal_msg msg;
 	u32 type;
 
-	ret = opal_get_msg(__pa(&msg), sizeof(msg));
+	ret = opal_get_msg(__pa(opal_msg), opal_msg_size);
 	/* No opal message pending. */
 	if (ret == OPAL_RESOURCE)
 		return;
@@ -290,14 +287,14 @@ static void opal_handle_message(void)
 		return;
 	}
 
-	type = be32_to_cpu(msg.msg_type);
+	type = be32_to_cpu(opal_msg->msg_type);
 
 	/* Sanity check */
 	if (type >= OPAL_MSG_TYPE_MAX) {
 		pr_warn_once("%s: Unknown message type: %u\n", __func__, type);
 		return;
 	}
-	opal_message_do_notify(type, (void *)&msg);
+	opal_message_do_notify(type, (void *)opal_msg);
 }
 
 static irqreturn_t opal_message_notify(int irq, void *data)
@@ -306,9 +303,21 @@ static irqreturn_t opal_message_notify(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static int __init opal_message_init(void)
+static int __init opal_message_init(struct device_node *opal_node)
 {
 	int ret, i, irq;
+	const __be32 *val;
+
+	val = of_get_property(opal_node, "opal-msg-size", NULL);
+	if (val)
+		opal_msg_size = be32_to_cpup(val);
+
+	/* If opal-msg-size property is not available then use default size */
+	if (!opal_msg_size)
+		opal_msg_size = sizeof(struct opal_msg);
+
+	opal_msg = kmalloc(opal_msg_size, GFP_KERNEL);
+	BUG_ON(opal_msg == NULL);
 
 	for (i = 0; i < OPAL_MSG_TYPE_MAX; i++)
 		ATOMIC_INIT_NOTIFIER_HEAD(&opal_msg_notifier_head[i]);
@@ -910,7 +919,7 @@ static int __init opal_init(void)
 	}
 
 	/* Initialise OPAL messaging system */
-	opal_message_init();
+	opal_message_init(opal_node);
 
 	/* Initialise OPAL asynchronous completion interface */
 	opal_async_comp_init();
-- 
2.21.0

[PATCH v3 2/2] powerpc/powernv: Add new opal message type

[Vasant Hegde]

We have OPAL_MSG_PRD message type to pass prd related messages from OPAL
to `opal-prd`. It can handle messages upto 64 bytes. We have a requirement
to send bigger than 64 bytes of data from OPAL to `opal-prd`. Lets add new
message type (OPAL_MSG_PRD2) to pass bigger data.

Cc: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/opal-api.h       | 1 +
 arch/powerpc/platforms/powernv/opal-prd.c | 9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index 383242eb0dea..1cad413e1e0e 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -453,6 +453,7 @@ enum opal_msg_type {
 	OPAL_MSG_DPO		= 5,
 	OPAL_MSG_PRD		= 6,
 	OPAL_MSG_OCC		= 7,
+	OPAL_MSG_PRD2		= 8,
 	OPAL_MSG_TYPE_MAX,
 };
 
diff --git a/arch/powerpc/platforms/powernv/opal-prd.c b/arch/powerpc/platforms/powernv/opal-prd.c
index e072bf157d62..50a735d77192 100644
--- a/arch/powerpc/platforms/powernv/opal-prd.c
+++ b/arch/powerpc/platforms/powernv/opal-prd.c
@@ -342,7 +342,7 @@ static int opal_prd_msg_notifier(struct notifier_block *nb,
 	int msg_size, item_size;
 	unsigned long flags;
 
-	if (msg_type != OPAL_MSG_PRD)
+	if (msg_type != OPAL_MSG_PRD && msg_type != OPAL_MSG_PRD2)
 		return 0;
 
 	/* Calculate total size of the message and item we need to store. The
@@ -393,6 +393,13 @@ static int opal_prd_probe(struct platform_device *pdev)
 		return rc;
 	}
 
+	rc = opal_message_notifier_register(OPAL_MSG_PRD2, &opal_prd_event_nb);
+	if (rc) {
+		pr_err("%s: Couldn't register event notifier (%d)\n",
+		       __func__, OPAL_MSG_PRD2);
+		return rc;
+	}
+
 	rc = misc_register(&opal_prd_dev);
 	if (rc) {
 		pr_err("failed to register miscdev\n");
-- 
2.21.0

Re: [PATCH v3 1/2] powerpc/powernv: Enhance opal message read interface

[Oliver O'Halloran]

On Wed, 2019-08-21 at 13:43 +0530, Vasant Hegde wrote:
> Use "opal-msg-size" device tree property to allocate memory for "opal_msg".
> 
> Cc: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> Cc: Jeremy Kerr <jk@ozlabs.org>
> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> ---
> Changes in v3:
>   - Call BUG_ON, if we fail to allocate memory during init.
> 
> -Vasant
> 
>  arch/powerpc/platforms/powernv/opal.c | 29 ++++++++++++++++++---------
>  1 file changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index aba443be7daa..4f1f68f568bf 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -58,6 +58,8 @@ static DEFINE_SPINLOCK(opal_write_lock);
>  static struct atomic_notifier_head opal_msg_notifier_head[OPAL_MSG_TYPE_MAX];
>  static uint32_t opal_heartbeat;
>  static struct task_struct *kopald_tsk;
> +static struct opal_msg *opal_msg;
> +static uint64_t opal_msg_size;
>  
>  void opal_configure_cores(void)
>  {
> @@ -271,14 +273,9 @@ static void opal_message_do_notify(uint32_t msg_type, void *msg)
>  static void opal_handle_message(void)
>  {
>  	s64 ret;
> -	/*
> -	 * TODO: pre-allocate a message buffer depending on opal-msg-size
> -	 * value in /proc/device-tree.
> -	 */
> -	static struct opal_msg msg;
>  	u32 type;
>  
> -	ret = opal_get_msg(__pa(&msg), sizeof(msg));
> +	ret = opal_get_msg(__pa(opal_msg), opal_msg_size);
>  	/* No opal message pending. */
>  	if (ret == OPAL_RESOURCE)
>  		return;
> @@ -290,14 +287,14 @@ static void opal_handle_message(void)
>  		return;
>  	}
>  
> -	type = be32_to_cpu(msg.msg_type);
> +	type = be32_to_cpu(opal_msg->msg_type);
>  
>  	/* Sanity check */
>  	if (type >= OPAL_MSG_TYPE_MAX) {
>  		pr_warn_once("%s: Unknown message type: %u\n", __func__, type);
>  		return;
>  	}
> -	opal_message_do_notify(type, (void *)&msg);
> +	opal_message_do_notify(type, (void *)opal_msg);
>  }
>  
>  static irqreturn_t opal_message_notify(int irq, void *data)
> @@ -306,9 +303,21 @@ static irqreturn_t opal_message_notify(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> -static int __init opal_message_init(void)
> +static int __init opal_message_init(struct device_node *opal_node)
>  {
>  	int ret, i, irq;

> +	const __be32 *val;
> +
> +	val = of_get_property(opal_node, "opal-msg-size", NULL);
> +	if (val)
> +		opal_msg_size = be32_to_cpup(val);

Use of_property_read_u32()

> +
> +	/* If opal-msg-size property is not available then use default size */
> +	if (!opal_msg_size)
> +		opal_msg_size = sizeof(struct opal_msg);
> +
> +	opal_msg = kmalloc(opal_msg_size, GFP_KERNEL);

> +	BUG_ON(opal_msg == NULL);

Seems questionable. Why not fall back to using a staticly allocated
struct opal_msg? Or re-try the allocation with the size limited to
sizeof(struct opal_msg)?

>  	for (i = 0; i < OPAL_MSG_TYPE_MAX; i++)
>  		ATOMIC_INIT_NOTIFIER_HEAD(&opal_msg_notifier_head[i]);
> @@ -910,7 +919,7 @@ static int __init opal_init(void)
>  	}
>  
>  	/* Initialise OPAL messaging system */
> -	opal_message_init();
> +	opal_message_init(opal_node);
>  
>  	/* Initialise OPAL asynchronous completion interface */
>  	opal_async_comp_init();

Re: [PATCH v3 1/2] powerpc/powernv: Enhance opal message read interface

[Vasant Hegde]

On 8/22/19 11:21 AM, Oliver O'Halloran wrote:
> On Wed, 2019-08-21 at 13:43 +0530, Vasant Hegde wrote:
>> Use "opal-msg-size" device tree property to allocate memory for "opal_msg".
>>
>> Cc: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>> Cc: Jeremy Kerr <jk@ozlabs.org>
>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>> ---
>> Changes in v3:
>>    - Call BUG_ON, if we fail to allocate memory during init.
>>
>> -Vasant
>>
>>   arch/powerpc/platforms/powernv/opal.c | 29 ++++++++++++++++++---------
>>   1 file changed, 19 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
>> index aba443be7daa..4f1f68f568bf 100644
>> --- a/arch/powerpc/platforms/powernv/opal.c
>> +++ b/arch/powerpc/platforms/powernv/opal.c
>> @@ -58,6 +58,8 @@ static DEFINE_SPINLOCK(opal_write_lock);
>>   static struct atomic_notifier_head opal_msg_notifier_head[OPAL_MSG_TYPE_MAX];
>>   static uint32_t opal_heartbeat;
>>   static struct task_struct *kopald_tsk;
>> +static struct opal_msg *opal_msg;
>> +static uint64_t opal_msg_size;
>>   
>>   void opal_configure_cores(void)
>>   {
>> @@ -271,14 +273,9 @@ static void opal_message_do_notify(uint32_t msg_type, void *msg)
>>   static void opal_handle_message(void)
>>   {
>>   	s64 ret;
>> -	/*
>> -	 * TODO: pre-allocate a message buffer depending on opal-msg-size
>> -	 * value in /proc/device-tree.
>> -	 */
>> -	static struct opal_msg msg;
>>   	u32 type;
>>   
>> -	ret = opal_get_msg(__pa(&msg), sizeof(msg));
>> +	ret = opal_get_msg(__pa(opal_msg), opal_msg_size);
>>   	/* No opal message pending. */
>>   	if (ret == OPAL_RESOURCE)
>>   		return;
>> @@ -290,14 +287,14 @@ static void opal_handle_message(void)
>>   		return;
>>   	}
>>   
>> -	type = be32_to_cpu(msg.msg_type);
>> +	type = be32_to_cpu(opal_msg->msg_type);
>>   
>>   	/* Sanity check */
>>   	if (type >= OPAL_MSG_TYPE_MAX) {
>>   		pr_warn_once("%s: Unknown message type: %u\n", __func__, type);
>>   		return;
>>   	}
>> -	opal_message_do_notify(type, (void *)&msg);
>> +	opal_message_do_notify(type, (void *)opal_msg);
>>   }
>>   
>>   static irqreturn_t opal_message_notify(int irq, void *data)
>> @@ -306,9 +303,21 @@ static irqreturn_t opal_message_notify(int irq, void *data)
>>   	return IRQ_HANDLED;
>>   }
>>   
>> -static int __init opal_message_init(void)
>> +static int __init opal_message_init(struct device_node *opal_node)
>>   {
>>   	int ret, i, irq;
> 
>> +	const __be32 *val;
>> +
>> +	val = of_get_property(opal_node, "opal-msg-size", NULL);
>> +	if (val)
>> +		opal_msg_size = be32_to_cpup(val);
> 
> Use of_property_read_u32()

Yes. Will fix it.

> 
>> +
>> +	/* If opal-msg-size property is not available then use default size */
>> +	if (!opal_msg_size)
>> +		opal_msg_size = sizeof(struct opal_msg);
>> +
>> +	opal_msg = kmalloc(opal_msg_size, GFP_KERNEL);
> 
>> +	BUG_ON(opal_msg == NULL);
> 
> Seems questionable. Why not fall back to using a staticly allocated
> struct opal_msg? Or re-try the allocation with the size limited to
> sizeof(struct opal_msg)?

If we are not able to allocate memory during init then we have bigger problem.
No point in continuing. Hence added BUG_ON().
May be I can retry allocation with fixed size before calling BUG_ON().
How about something like below :

+       /* If opal-msg-size property is not available then use default size */
+       if (!opal_msg_size)
+               opal_msg_size = sizeof(struct opal_msg);
+
+       opal_msg = kmalloc(opal_msg_size, GFP_KERNEL);
+       if (!opal_msg) {
+               opal_msg = kmalloc(sizeof(struct opal_msg), GFP_KERNEL);
+               BUG_ON(opal_msg == NULL);
+       }


-Vasant

Re: [PATCH v3 1/2] powerpc/powernv: Enhance opal message read interface

[Vasant Hegde]

On 8/22/19 9:06 PM, Vasant Hegde wrote:
> On 8/22/19 11:21 AM, Oliver O'Halloran wrote:
>> On Wed, 2019-08-21 at 13:43 +0530, Vasant Hegde wrote:
>>> Use "opal-msg-size" device tree property to allocate memory for "opal_msg".
>>>
>>> Cc: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>> Cc: Jeremy Kerr <jk@ozlabs.org>
>>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>>> ---
>>> Changes in v3:
>>>    - Call BUG_ON, if we fail to allocate memory during init.
>>>
>>> -Vasant
>>>
>>>   arch/powerpc/platforms/powernv/opal.c | 29 ++++++++++++++++++---------
>>>   1 file changed, 19 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/powernv/opal.c 
>>> b/arch/powerpc/platforms/powernv/opal.c
>>> index aba443be7daa..4f1f68f568bf 100644
>>> --- a/arch/powerpc/platforms/powernv/opal.c
>>> +++ b/arch/powerpc/platforms/powernv/opal.c
>>> @@ -58,6 +58,8 @@ static DEFINE_SPINLOCK(opal_write_lock);
>>>   static struct atomic_notifier_head opal_msg_notifier_head[OPAL_MSG_TYPE_MAX];
>>>   static uint32_t opal_heartbeat;
>>>   static struct task_struct *kopald_tsk;
>>> +static struct opal_msg *opal_msg;
>>> +static uint64_t opal_msg_size;
>>>   void opal_configure_cores(void)
>>>   {
>>> @@ -271,14 +273,9 @@ static void opal_message_do_notify(uint32_t msg_type, 
>>> void *msg)
>>>   static void opal_handle_message(void)
>>>   {
>>>       s64 ret;
>>> -    /*
>>> -     * TODO: pre-allocate a message buffer depending on opal-msg-size
>>> -     * value in /proc/device-tree.
>>> -     */
>>> -    static struct opal_msg msg;
>>>       u32 type;
>>> -    ret = opal_get_msg(__pa(&msg), sizeof(msg));
>>> +    ret = opal_get_msg(__pa(opal_msg), opal_msg_size);
>>>       /* No opal message pending. */
>>>       if (ret == OPAL_RESOURCE)
>>>           return;
>>> @@ -290,14 +287,14 @@ static void opal_handle_message(void)
>>>           return;
>>>       }
>>> -    type = be32_to_cpu(msg.msg_type);
>>> +    type = be32_to_cpu(opal_msg->msg_type);
>>>       /* Sanity check */
>>>       if (type >= OPAL_MSG_TYPE_MAX) {
>>>           pr_warn_once("%s: Unknown message type: %u\n", __func__, type);
>>>           return;
>>>       }
>>> -    opal_message_do_notify(type, (void *)&msg);
>>> +    opal_message_do_notify(type, (void *)opal_msg);
>>>   }
>>>   static irqreturn_t opal_message_notify(int irq, void *data)
>>> @@ -306,9 +303,21 @@ static irqreturn_t opal_message_notify(int irq, void *data)
>>>       return IRQ_HANDLED;
>>>   }
>>> -static int __init opal_message_init(void)
>>> +static int __init opal_message_init(struct device_node *opal_node)
>>>   {
>>>       int ret, i, irq;
>>
>>> +    const __be32 *val;
>>> +
>>> +    val = of_get_property(opal_node, "opal-msg-size", NULL);
>>> +    if (val)
>>> +        opal_msg_size = be32_to_cpup(val);
>>
>> Use of_property_read_u32()
> 
> Yes. Will fix it.
> 
>>
>>> +
>>> +    /* If opal-msg-size property is not available then use default size */
>>> +    if (!opal_msg_size)
>>> +        opal_msg_size = sizeof(struct opal_msg);
>>> +
>>> +    opal_msg = kmalloc(opal_msg_size, GFP_KERNEL);
>>
>>> +    BUG_ON(opal_msg == NULL);
>>
>> Seems questionable. Why not fall back to using a staticly allocated
>> struct opal_msg? Or re-try the allocation with the size limited to
>> sizeof(struct opal_msg)?
> 
> If we are not able to allocate memory during init then we have bigger problem.
> No point in continuing. Hence added BUG_ON().
> May be I can retry allocation with fixed size before calling BUG_ON().
> How about something like below :
> 
> +       /* If opal-msg-size property is not available then use default size */
> +       if (!opal_msg_size)
> +               opal_msg_size = sizeof(struct opal_msg);
> +
> +       opal_msg = kmalloc(opal_msg_size, GFP_KERNEL);
> +       if (!opal_msg) {

Just to be clear I will adjust `opal_msg_size` size here.

-Vasant

> +               opal_msg = kmalloc(sizeof(struct opal_msg), GFP_KERNEL);
> +               BUG_ON(opal_msg == NULL);
> +       }
> 
> 
> -Vasant
>