[PATCH v2 0/3] firmware: arm_ffa: Add support to run inside a guest/VM under a hypervisor

[PATCH v2 0/3] firmware: arm_ffa: Add support to run inside a guest/VM under a hypervisor

[Sudeep Holla]

Add support for running the driver inside a guest/VM under a hypervisor.

The main difference include:
1. supporting introducing notification pending interrupt and
2. Avoiding creation of all the notification bitmaps as they must be
   created by the hypervisor before the VM is initialised

The guest may need to use a notification pending interrupt instead of
or in addition to the schedule receiver interrupt.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
Hi Jens,

This is almost identical to you single patch now. Just split into
3 patches with refactoring kept separate. Can you please check if
this continues to work on your setup(unlikely to break anything).
Once you confirm, I am happy to queue it.

Regards,
Sudeep

Changes in v2:
- Dropped the idea of SRI or NPI support
- Added back both SRI and NPI to co-exist as in the original patch
  from Jens
  https://lore.kernel.org/all/20240325081335.2326979-1-jens.wiklander@linaro.org/
- Link to v1: https://lore.kernel.org/r/20240410-ffa_npi_support-v1-0-1a5223391bd1@arm.com

---
Jens Wiklander (2):
      firmware: arm_ffa: Skip creation of the notification bitmaps
      firmware: arm_ffa: Add support for handling notification pending interrupt(NPI)

Sudeep Holla (1):
      firmware: arm_ffa: Refactor SRI handling in prepartion to add NPI support

 drivers/firmware/arm_ffa/driver.c | 128 ++++++++++++++++++++++++++------------
 1 file changed, 87 insertions(+), 41 deletions(-)
---
base-commit: 2c71fdf02a95b3dd425b42f28fd47fb2b1d22702
change-id: 20240410-ffa_npi_support-98edfdcc4882

Best regards,
-- 
Regards,
Sudeep


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/3] firmware: arm_ffa: Skip creation of the notification bitmaps

[Sudeep Holla]

From: Jens Wiklander <jens.wiklander@linaro.org>

When the FF-A driver is running inside a guest VM under an hypervisor,
the driver/guest VM doesn't have the permission/capability to request
the creation of notification bitmaps. For a VM, the hypervisor reserves
memory for its VM and hypervisor framework notification bitmaps and the
SPMC reserves memory for its SP and SPMC framework notification bitmaps
before the hypervisor initializes it.

The hypervisor does not initialize a VM if memory cannot be reserved
for all its notification bitmaps. So the creation of all the necessary
bitmaps are already done when the driver initialises and hence it can be
skipped. We rely on FFA_FEATURES(FFA_NOTIFICATION_BITMAP_CREATE) to fail
when running in the guest to handle this in the driver.

Signed-off-by:Jens Wiklander <jens.wiklander@linaro.org>
[sudeep.holla: Updated the commit message]
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_ffa/driver.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index f2556a8e9401..4a576af7b8fd 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -1442,17 +1442,15 @@ static void ffa_notifications_setup(void)
 	int ret, irq;
 
 	ret = ffa_features(FFA_NOTIFICATION_BITMAP_CREATE, 0, NULL, NULL);
-	if (ret) {
-		pr_info("Notifications not supported, continuing with it ..\n");
-		return;
-	}
+	if (!ret) {
+		ret = ffa_notification_bitmap_create();
+		if (ret) {
+			pr_info("Notification bitmap create error %d\n", ret);
+			return;
+		}
 
-	ret = ffa_notification_bitmap_create();
-	if (ret) {
-		pr_info("Notification bitmap create error %d\n", ret);
-		return;
+		drv_info->bitmap_created = true;
 	}
-	drv_info->bitmap_created = true;
 
 	irq = ffa_sched_recv_irq_map();
 	if (irq <= 0) {

-- 
2.43.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/3] firmware: arm_ffa: Refactor SRI handling in prepartion to add NPI support

[Sudeep Holla]

In preparation to support handling of Notification Pending Interrupt(NPI)
in addition to the existing support for Schedule Reciever Interrupt(SRI),
refactor the code around SRI handling so that NPI support can reuse some
of it. This change shouldn't have any functionality impact. It neither
adds the support for NPIs nor changes any SRI support.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_ffa/driver.c | 47 +++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 4a576af7b8fd..1ef5d6752c35 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -105,7 +105,7 @@ struct ffa_drv_info {
 	struct ffa_pcpu_irq __percpu *irq_pcpu;
 	struct workqueue_struct *notif_pcpu_wq;
 	struct work_struct notif_pcpu_work;
-	struct work_struct irq_work;
+	struct work_struct sched_recv_irq_work;
 	struct xarray partition_info;
 	DECLARE_HASHTABLE(notifier_hash, ilog2(FFA_MAX_NOTIFICATIONS));
 	struct mutex notify_lock; /* lock to protect notifier hashtable  */
@@ -1291,12 +1291,12 @@ static void ffa_partitions_cleanup(void)
 #define FFA_FEAT_SCHEDULE_RECEIVER_INT		(2)
 #define FFA_FEAT_MANAGED_EXIT_INT		(3)
 
-static irqreturn_t irq_handler(int irq, void *irq_data)
+static irqreturn_t ffa_sched_recv_irq_handler(int irq, void *irq_data)
 {
 	struct ffa_pcpu_irq *pcpu = irq_data;
 	struct ffa_drv_info *info = pcpu->info;
 
-	queue_work(info->notif_pcpu_wq, &info->irq_work);
+	queue_work(info->notif_pcpu_wq, &info->sched_recv_irq_work);
 
 	return IRQ_HANDLED;
 }
@@ -1306,15 +1306,23 @@ static void ffa_sched_recv_irq_work_fn(struct work_struct *work)
 	ffa_notification_info_get();
 }
 
-static int ffa_sched_recv_irq_map(void)
+static int ffa_irq_map(u32 id)
 {
-	int ret, irq, sr_intid;
+	char *err_str;
+	int ret, irq, intid;
 
-	/* The returned sr_intid is assumed to be SGI donated to NS world */
-	ret = ffa_features(FFA_FEAT_SCHEDULE_RECEIVER_INT, 0, &sr_intid, NULL);
+	if (id == FFA_FEAT_NOTIFICATION_PENDING_INT)
+		err_str = "Notification Pending Interrupt";
+	else if (id == FFA_FEAT_SCHEDULE_RECEIVER_INT)
+		err_str = "Schedule Receiver Interrupt";
+	else
+		err_str = "Unknown ID";
+
+	/* The returned intid is assumed to be SGI donated to NS world */
+	ret = ffa_features(id, 0, &intid, NULL);
 	if (ret < 0) {
 		if (ret != -EOPNOTSUPP)
-			pr_err("Failed to retrieve scheduler Rx interrupt\n");
+			pr_err("Failed to retrieve FF-A %s %u\n", err_str ,id);
 		return ret;
 	}
 
@@ -1329,12 +1337,12 @@ static int ffa_sched_recv_irq_map(void)
 
 		oirq.np = gic;
 		oirq.args_count = 1;
-		oirq.args[0] = sr_intid;
+		oirq.args[0] = intid;
 		irq = irq_create_of_mapping(&oirq);
 		of_node_put(gic);
 #ifdef CONFIG_ACPI
 	} else {
-		irq = acpi_register_gsi(NULL, sr_intid, ACPI_EDGE_SENSITIVE,
+		irq = acpi_register_gsi(NULL, intid, ACPI_EDGE_SENSITIVE,
 					ACPI_ACTIVE_HIGH);
 #endif
 	}
@@ -1347,12 +1355,11 @@ static int ffa_sched_recv_irq_map(void)
 	return irq;
 }
 
-static void ffa_sched_recv_irq_unmap(void)
+static void ffa_irq_unmap(unsigned int irq)
 {
-	if (drv_info->sched_recv_irq) {
-		irq_dispose_mapping(drv_info->sched_recv_irq);
-		drv_info->sched_recv_irq = 0;
-	}
+	if (!irq)
+		return;
+	irq_dispose_mapping(irq);
 }
 
 static int ffa_cpuhp_pcpu_irq_enable(unsigned int cpu)
@@ -1402,13 +1409,14 @@ static int ffa_init_pcpu_irq(unsigned int irq)
 
 	drv_info->irq_pcpu = irq_pcpu;
 
-	ret = request_percpu_irq(irq, irq_handler, "ARM-FFA", irq_pcpu);
+	ret = request_percpu_irq(irq, ffa_sched_recv_irq_handler, "ARM-FFA-SRI",
+				 irq_pcpu);
 	if (ret) {
 		pr_err("Error registering notification IRQ %d: %d\n", irq, ret);
 		return ret;
 	}
 
-	INIT_WORK(&drv_info->irq_work, ffa_sched_recv_irq_work_fn);
+	INIT_WORK(&drv_info->sched_recv_irq_work, ffa_sched_recv_irq_work_fn);
 	INIT_WORK(&drv_info->notif_pcpu_work, notif_pcpu_irq_work_fn);
 	drv_info->notif_pcpu_wq = create_workqueue("ffa_pcpu_irq_notification");
 	if (!drv_info->notif_pcpu_wq)
@@ -1428,7 +1436,8 @@ static int ffa_init_pcpu_irq(unsigned int irq)
 static void ffa_notifications_cleanup(void)
 {
 	ffa_uninit_pcpu_irq();
-	ffa_sched_recv_irq_unmap();
+	ffa_irq_unmap(drv_info->sched_recv_irq);
+	drv_info->sched_recv_irq = 0;
 
 	if (drv_info->bitmap_created) {
 		ffa_notification_bitmap_destroy();
@@ -1452,7 +1461,7 @@ static void ffa_notifications_setup(void)
 		drv_info->bitmap_created = true;
 	}
 
-	irq = ffa_sched_recv_irq_map();
+	irq = ffa_irq_map(FFA_FEAT_SCHEDULE_RECEIVER_INT);
 	if (irq <= 0) {
 		ret = irq;
 		goto cleanup;

-- 
2.43.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/3] firmware: arm_ffa: Add support for handling notification pending interrupt(NPI)

[Sudeep Holla]

From: Jens Wiklander <jens.wiklander@linaro.org>

The FF-A uses the notification pending interrupt to inform the receiver
that it has a pending notification. This is a virtual interrupt and is
used by the following type of receivers:
1. A guest/VM running under a hypervisor.
2. An S-EL1 SP running under a S-EL2 SPMC.

The rules that govern the properties of the NPI are the same as the
rules for the SRI with couple of exceptions. Both SRI and NPI can be
supported simultaneously.

The handling of NPI is also same as the handling of notification for the
self/primary VM with ID 0 except the absence of global notification.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
[sudeep.holla: minor refactoring]
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_ffa/driver.c | 71 ++++++++++++++++++++++++++++++---------
 1 file changed, 55 insertions(+), 16 deletions(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 1ef5d6752c35..87bd30c4bb67 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -101,6 +101,7 @@ struct ffa_drv_info {
 	bool bitmap_created;
 	bool notif_enabled;
 	unsigned int sched_recv_irq;
+	unsigned int notif_pend_irq;
 	unsigned int cpuhp_state;
 	struct ffa_pcpu_irq __percpu *irq_pcpu;
 	struct workqueue_struct *notif_pcpu_wq;
@@ -1301,6 +1302,15 @@ static irqreturn_t ffa_sched_recv_irq_handler(int irq, void *irq_data)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t notif_pend_irq_handler(int irq, void *irq_data)
+{
+	struct ffa_pcpu_irq *pcpu = irq_data;
+
+	ffa_self_notif_handle(smp_processor_id(), true, pcpu->info);
+
+	return IRQ_HANDLED;
+}
+
 static void ffa_sched_recv_irq_work_fn(struct work_struct *work)
 {
 	ffa_notification_info_get();
@@ -1364,13 +1374,19 @@ static void ffa_irq_unmap(unsigned int irq)
 
 static int ffa_cpuhp_pcpu_irq_enable(unsigned int cpu)
 {
-	enable_percpu_irq(drv_info->sched_recv_irq, IRQ_TYPE_NONE);
+	if (drv_info->sched_recv_irq)
+		enable_percpu_irq(drv_info->sched_recv_irq, IRQ_TYPE_NONE);
+	if (drv_info->notif_pend_irq)
+		enable_percpu_irq(drv_info->notif_pend_irq, IRQ_TYPE_NONE);
 	return 0;
 }
 
 static int ffa_cpuhp_pcpu_irq_disable(unsigned int cpu)
 {
-	disable_percpu_irq(drv_info->sched_recv_irq);
+	if (drv_info->sched_recv_irq)
+		disable_percpu_irq(drv_info->sched_recv_irq);
+	if (drv_info->notif_pend_irq)
+		disable_percpu_irq(drv_info->notif_pend_irq);
 	return 0;
 }
 
@@ -1389,13 +1405,16 @@ static void ffa_uninit_pcpu_irq(void)
 	if (drv_info->sched_recv_irq)
 		free_percpu_irq(drv_info->sched_recv_irq, drv_info->irq_pcpu);
 
+	if (drv_info->notif_pend_irq)
+		free_percpu_irq(drv_info->notif_pend_irq, drv_info->irq_pcpu);
+
 	if (drv_info->irq_pcpu) {
 		free_percpu(drv_info->irq_pcpu);
 		drv_info->irq_pcpu = NULL;
 	}
 }
 
-static int ffa_init_pcpu_irq(unsigned int irq)
+static int ffa_init_pcpu_irq(void)
 {
 	struct ffa_pcpu_irq __percpu *irq_pcpu;
 	int ret, cpu;
@@ -1409,11 +1428,26 @@ static int ffa_init_pcpu_irq(unsigned int irq)
 
 	drv_info->irq_pcpu = irq_pcpu;
 
-	ret = request_percpu_irq(irq, ffa_sched_recv_irq_handler, "ARM-FFA-SRI",
-				 irq_pcpu);
-	if (ret) {
-		pr_err("Error registering notification IRQ %d: %d\n", irq, ret);
-		return ret;
+	if (drv_info->sched_recv_irq) {
+		ret = request_percpu_irq(drv_info->sched_recv_irq,
+					 ffa_sched_recv_irq_handler,
+					 "ARM-FFA-SRI", irq_pcpu);
+		if (ret) {
+			pr_err("Error registering percpu SRI nIRQ %d : %d\n",
+			       drv_info->sched_recv_irq, ret);
+			return ret;
+		}
+	}
+
+	if (drv_info->notif_pend_irq) {
+		ret = request_percpu_irq(drv_info->notif_pend_irq,
+					 notif_pend_irq_handler,
+					 "ARM-FFA-NPI", irq_pcpu);
+		if (ret) {
+			pr_err("Error registering percpu NPI nIRQ %d : %d\n",
+			       drv_info->notif_pend_irq, ret);
+			return ret;
+		}
 	}
 
 	INIT_WORK(&drv_info->sched_recv_irq_work, ffa_sched_recv_irq_work_fn);
@@ -1438,6 +1472,8 @@ static void ffa_notifications_cleanup(void)
 	ffa_uninit_pcpu_irq();
 	ffa_irq_unmap(drv_info->sched_recv_irq);
 	drv_info->sched_recv_irq = 0;
+	ffa_irq_unmap(drv_info->notif_pend_irq);
+	drv_info->notif_pend_irq = 0;
 
 	if (drv_info->bitmap_created) {
 		ffa_notification_bitmap_destroy();
@@ -1448,7 +1484,7 @@ static void ffa_notifications_cleanup(void)
 
 static void ffa_notifications_setup(void)
 {
-	int ret, irq;
+       int ret;
 
 	ret = ffa_features(FFA_NOTIFICATION_BITMAP_CREATE, 0, NULL, NULL);
 	if (!ret) {
@@ -1461,15 +1497,18 @@ static void ffa_notifications_setup(void)
 		drv_info->bitmap_created = true;
 	}
 
-	irq = ffa_irq_map(FFA_FEAT_SCHEDULE_RECEIVER_INT);
-	if (irq <= 0) {
-		ret = irq;
-		goto cleanup;
-	}
+	ret = ffa_irq_map(FFA_FEAT_SCHEDULE_RECEIVER_INT);
+	if (ret > 0)
+		drv_info->sched_recv_irq = ret;
 
-	drv_info->sched_recv_irq = irq;
+	ret = ffa_irq_map(FFA_FEAT_NOTIFICATION_PENDING_INT);
+	if (ret > 0)
+		drv_info->notif_pend_irq = ret;
+
+	if (!drv_info->sched_recv_irq && !drv_info->notif_pend_irq)
+		goto cleanup;
 
-	ret = ffa_init_pcpu_irq(irq);
+	ret = ffa_init_pcpu_irq();
 	if (ret)
 		goto cleanup;
 

-- 
2.43.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 0/3] firmware: arm_ffa: Add support to run inside a guest/VM under a hypervisor

[Jens Wiklander]

Hi Sudeep,

On Thu, Apr 11, 2024 at 2:57 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> Add support for running the driver inside a guest/VM under a hypervisor.
>
> The main difference include:
> 1. supporting introducing notification pending interrupt and
> 2. Avoiding creation of all the notification bitmaps as they must be
>    created by the hypervisor before the VM is initialised
>
> The guest may need to use a notification pending interrupt instead of
> or in addition to the schedule receiver interrupt.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> Hi Jens,
>
> This is almost identical to you single patch now. Just split into
> 3 patches with refactoring kept separate. Can you please check if
> this continues to work on your setup(unlikely to break anything).
> Once you confirm, I am happy to queue it.

Works for me:
Tested-by: Jens Wiklander <jens.wiklander@linaro.org>

Thanks,
Jens

>
> Regards,
> Sudeep
>
> Changes in v2:
> - Dropped the idea of SRI or NPI support
> - Added back both SRI and NPI to co-exist as in the original patch
>   from Jens
>   https://lore.kernel.org/all/20240325081335.2326979-1-jens.wiklander@linaro.org/
> - Link to v1: https://lore.kernel.org/r/20240410-ffa_npi_support-v1-0-1a5223391bd1@arm.com
>
> ---
> Jens Wiklander (2):
>       firmware: arm_ffa: Skip creation of the notification bitmaps
>       firmware: arm_ffa: Add support for handling notification pending interrupt(NPI)
>
> Sudeep Holla (1):
>       firmware: arm_ffa: Refactor SRI handling in prepartion to add NPI support
>
>  drivers/firmware/arm_ffa/driver.c | 128 ++++++++++++++++++++++++++------------
>  1 file changed, 87 insertions(+), 41 deletions(-)
> ---
> base-commit: 2c71fdf02a95b3dd425b42f28fd47fb2b1d22702
> change-id: 20240410-ffa_npi_support-98edfdcc4882
>
> Best regards,
> --
> Regards,
> Sudeep
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 0/3] firmware: arm_ffa: Add support to run inside a guest/VM under a hypervisor

[Sudeep Holla]

On Thu, Apr 11, 2024 at 03:30:41PM +0200, Jens Wiklander wrote:
> Hi Sudeep,
> 
> On Thu, Apr 11, 2024 at 2:57 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > Add support for running the driver inside a guest/VM under a hypervisor.
> >
> > The main difference include:
> > 1. supporting introducing notification pending interrupt and
> > 2. Avoiding creation of all the notification bitmaps as they must be
> >    created by the hypervisor before the VM is initialised
> >
> > The guest may need to use a notification pending interrupt instead of
> > or in addition to the schedule receiver interrupt.
> >
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> > Hi Jens,
> >
> > This is almost identical to you single patch now. Just split into
> > 3 patches with refactoring kept separate. Can you please check if
> > this continues to work on your setup(unlikely to break anything).
> > Once you confirm, I am happy to queue it.
> 
> Works for me:
> Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
> 

Thanks !

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/3] firmware: arm_ffa: Skip creation of the notification bitmaps

[Sebastian Ene]

On Thu, Apr 11, 2024 at 01:57:32PM +0100, Sudeep Holla wrote:
> From: Jens Wiklander <jens.wiklander@linaro.org>
> 
> When the FF-A driver is running inside a guest VM under an hypervisor,
> the driver/guest VM doesn't have the permission/capability to request
> the creation of notification bitmaps. For a VM, the hypervisor reserves

Who restrains them from having this capability ? (to use the
FFA_NOTIFICATION_BITMAP_CREATE API).

> memory for its VM and hypervisor framework notification bitmaps and the
> SPMC reserves memory for its SP and SPMC framework notification bitmaps
> before the hypervisor initializes it.
> 
> The hypervisor does not initialize a VM if memory cannot be reserved
> for all its notification bitmaps. So the creation of all the necessary
> bitmaps are already done when the driver initialises and hence it can be
> skipped. We rely on FFA_FEATURES(FFA_NOTIFICATION_BITMAP_CREATE) to fail
> when running in the guest to handle this in the driver.
> 

This implies that the hypervisor reserves memory in advance to keep the
notification bitmaps even though the guest might decide not to use them.
For pKVM this assumption does not work because we might have guests that will not be
allowed to talk to TZ. 

> Signed-off-by:Jens Wiklander <jens.wiklander@linaro.org>
> [sudeep.holla: Updated the commit message]
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/firmware/arm_ffa/driver.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> index f2556a8e9401..4a576af7b8fd 100644
> --- a/drivers/firmware/arm_ffa/driver.c
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -1442,17 +1442,15 @@ static void ffa_notifications_setup(void)
>  	int ret, irq;
>  
>  	ret = ffa_features(FFA_NOTIFICATION_BITMAP_CREATE, 0, NULL, NULL);
> -	if (ret) {
> -		pr_info("Notifications not supported, continuing with it ..\n");
> -		return;
> -	}
> +	if (!ret) {
> +		ret = ffa_notification_bitmap_create();
> +		if (ret) {
> +			pr_info("Notification bitmap create error %d\n", ret);

Not a big deal but shouldn't this be a pr_err(..) 

Thanks,
Seb

> +			return;
> +		}
>  
> -	ret = ffa_notification_bitmap_create();
> -	if (ret) {
> -		pr_info("Notification bitmap create error %d\n", ret);
> -		return;
> +		drv_info->bitmap_created = true;
>  	}
> -	drv_info->bitmap_created = true;
>  
>  	irq = ffa_sched_recv_irq_map();
>  	if (irq <= 0) {
> 
> -- 
> 2.43.2
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/3] firmware: arm_ffa: Skip creation of the notification bitmaps

[Sudeep Holla]

On Fri, Apr 12, 2024 at 10:42:07AM +0000, Sebastian Ene wrote:
> On Thu, Apr 11, 2024 at 01:57:32PM +0100, Sudeep Holla wrote:
> > From: Jens Wiklander <jens.wiklander@linaro.org>
> > 
> > When the FF-A driver is running inside a guest VM under an hypervisor,
> > the driver/guest VM doesn't have the permission/capability to request
> > the creation of notification bitmaps. For a VM, the hypervisor reserves
> 
> Who restrains them from having this capability ? (to use the
> FFA_NOTIFICATION_BITMAP_CREATE API).

The hypervisor has to IIUC from the spec as it is responsible for creating
the bitmaps on its behalf. It can trap and handle those calls if it prefers
to fail for FFA_FEATURES(FFA_NOTIFICATION_BITMAP_CREATE).

> > memory for its VM and hypervisor framework notification bitmaps and the
> > SPMC reserves memory for its SP and SPMC framework notification bitmaps
> > before the hypervisor initializes it.
> > 
> > The hypervisor does not initialize a VM if memory cannot be reserved
> > for all its notification bitmaps. So the creation of all the necessary
> > bitmaps are already done when the driver initialises and hence it can be
> > skipped. We rely on FFA_FEATURES(FFA_NOTIFICATION_BITMAP_CREATE) to fail
> > when running in the guest to handle this in the driver.
> > 
> 
> This implies that the hypervisor reserves memory in advance to keep the
> notification bitmaps even though the guest might decide not to use them.
> For pKVM this assumption does not work because we might have guests that will not be
> allowed to talk to TZ.
>

Sure, in such case pKVM can always trap and fail for FFA_VERSION call so
that no further FFA transactions happen from that guest or just fail for
notification APIs if the notifications alone are not needed.

Hope that clarifies.

> > Signed-off-by:Jens Wiklander <jens.wiklander@linaro.org>
> > [sudeep.holla: Updated the commit message]
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >  drivers/firmware/arm_ffa/driver.c | 16 +++++++---------
> >  1 file changed, 7 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> > index f2556a8e9401..4a576af7b8fd 100644
> > --- a/drivers/firmware/arm_ffa/driver.c
> > +++ b/drivers/firmware/arm_ffa/driver.c
> > @@ -1442,17 +1442,15 @@ static void ffa_notifications_setup(void)
> >  	int ret, irq;
> >  
> >  	ret = ffa_features(FFA_NOTIFICATION_BITMAP_CREATE, 0, NULL, NULL);
> > -	if (ret) {
> > -		pr_info("Notifications not supported, continuing with it ..\n");
> > -		return;
> > -	}
> > +	if (!ret) {
> > +		ret = ffa_notification_bitmap_create();
> > +		if (ret) {
> > +			pr_info("Notification bitmap create error %d\n", ret);
> 
> Not a big deal but shouldn't this be a pr_err(..) 
>

Sure I can fixup when I apply.

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 0/3] firmware: arm_ffa: Add support to run inside a guest/VM under a hypervisor

[Sudeep Holla]

On Thu, 11 Apr 2024 13:57:31 +0100, Sudeep Holla wrote:
> Add support for running the driver inside a guest/VM under a hypervisor.
>
> The main difference include:
> 1. supporting introducing notification pending interrupt and
> 2. Avoiding creation of all the notification bitmaps as they must be
>    created by the hypervisor before the VM is initialised
>
> [...]

Applied to sudeep.holla/linux (for-next/ffa/updates), thanks!

[1/3] firmware: arm_ffa: Skip creation of the notification bitmaps
      https://git.kernel.org/sudeep.holla/c/2b9c66d1abac
[2/3] firmware: arm_ffa: Refactor SRI handling in prepartion to add NPI support
      https://git.kernel.org/sudeep.holla/c/f936c242553f
[3/3] firmware: arm_ffa: Add support for handling notification pending interrupt(NPI)
      https://git.kernel.org/sudeep.holla/c/08530a2aa921
--
Regards,
Sudeep


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel