mbox series

[XEN,v6,0/7] FF-A notifications

Message ID 20240610065343.2594943-1-jens.wiklander@linaro.org (mailing list archive)
Headers show
Series FF-A notifications | expand

Message

Jens Wiklander June 10, 2024, 6:53 a.m. UTC
Hi,

This patch set adds support for FF-A notifications. We only support
global notifications, per vCPU notifications remain unsupported.

The first three patches are further cleanup and can be merged before the
rest if desired.

A physical SGI is used to make Xen aware of pending FF-A notifications. The
physical SGI is selected by the SPMC in the secure world. Since it must not
already be used by Xen the SPMC is in practice forced to donate one of the
secure SGIs, but that's normally not a problem. The SGI handling in Xen is
updated to support registration of handlers for SGIs that aren't statically
assigned, that is, SGI IDs above GIC_SGI_MAX.

The patch "xen/arm: add and call init_tee_secondary()" provides a hook for
register the needed per-cpu interrupt handler in "xen/arm: ffa: support
notification".

The patch "xen/arm: add and call tee_free_domain_ctx()" provides a hook for
later freeing of the TEE context. This hook is used in "xen/arm: ffa:
support notification" and avoids the problem with TEE context being freed
while we need to access it when handling a Schedule Receiver interrupt. It
was suggested as an alternative in [1] that the TEE context could be freed
from complete_domain_destroy().

[1] https://lore.kernel.org/all/CAHUa44H4YpoxYT7e6WNH5XJFpitZQjqP9Ng4SmTy4eWhyN+F+w@mail.gmail.com/

Thanks,
Jens

v5->v6:
- Added Betrands R-B for "xen/arm: add and call tee_free_domain_ctx()",
  "xen/arm: add and call init_tee_secondary()"
- Added Juliens A-B for "xen/arm: allow dynamically assigned SGI handlers"
- Updated "xen/arm: ffa: support notification", details in the patch

v4->v5:
- Added two new patches "xen/arm: add and call init_tee_interrupt()" and
  "xen/arm: add and call tee_free_domain_ctx()"
- Updated "xen/arm: ffa: support notification", details in the patch

v3->v4:
- "xen/arm: ffa: support notification" and
  "xen/arm: allow dynamically assigned SGI handlers" updated as requestsed,
  details in each patch.

v2->v3:
- "xen/arm: ffa: support notification" and
  "xen/arm: allow dynamically assigned SGI handlers" updated as requestsed,
  details in each patch.

v1->v2:
- "xen/arm: ffa: support notification" and
  "xen/arm: allow dynamically assigned SGI handlers" updated as requestsed,
  details in each patch.
- Added Bertrands R-B for "xen/arm: ffa: refactor ffa_handle_call()",
  "xen/arm: ffa: use ACCESS_ONCE()", and
  "xen/arm: ffa: simplify ffa_handle_mem_share()"



Jens Wiklander (7):
  xen/arm: ffa: refactor ffa_handle_call()
  xen/arm: ffa: use ACCESS_ONCE()
  xen/arm: ffa: simplify ffa_handle_mem_share()
  xen/arm: allow dynamically assigned SGI handlers
  xen/arm: add and call init_tee_secondary()
  xen/arm: add and call tee_free_domain_ctx()
  xen/arm: ffa: support notification

 xen/arch/arm/domain.c              |   1 +
 xen/arch/arm/gic.c                 |  12 +-
 xen/arch/arm/include/asm/gic.h     |   2 +-
 xen/arch/arm/include/asm/tee/tee.h |  14 +
 xen/arch/arm/irq.c                 |  18 +-
 xen/arch/arm/smpboot.c             |   2 +
 xen/arch/arm/tee/Makefile          |   1 +
 xen/arch/arm/tee/ffa.c             | 105 +++++--
 xen/arch/arm/tee/ffa_notif.c       | 425 +++++++++++++++++++++++++++++
 xen/arch/arm/tee/ffa_partinfo.c    |   9 +-
 xen/arch/arm/tee/ffa_private.h     |  56 +++-
 xen/arch/arm/tee/ffa_shm.c         |  33 +--
 xen/arch/arm/tee/tee.c             |  14 +-
 xen/include/public/arch-arm.h      |  14 +
 14 files changed, 643 insertions(+), 63 deletions(-)
 create mode 100644 xen/arch/arm/tee/ffa_notif.c

Comments

Bertrand Marquis June 10, 2024, 3:54 p.m. UTC | #1
Hi Jens,

> On 10 Jun 2024, at 08:53, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> 
> Hi,
> 
> This patch set adds support for FF-A notifications. We only support
> global notifications, per vCPU notifications remain unsupported.
> 
> The first three patches are further cleanup and can be merged before the
> rest if desired.
> 
> A physical SGI is used to make Xen aware of pending FF-A notifications. The
> physical SGI is selected by the SPMC in the secure world. Since it must not
> already be used by Xen the SPMC is in practice forced to donate one of the
> secure SGIs, but that's normally not a problem. The SGI handling in Xen is
> updated to support registration of handlers for SGIs that aren't statically
> assigned, that is, SGI IDs above GIC_SGI_MAX.
> 
> The patch "xen/arm: add and call init_tee_secondary()" provides a hook for
> register the needed per-cpu interrupt handler in "xen/arm: ffa: support
> notification".
> 
> The patch "xen/arm: add and call tee_free_domain_ctx()" provides a hook for
> later freeing of the TEE context. This hook is used in "xen/arm: ffa:
> support notification" and avoids the problem with TEE context being freed
> while we need to access it when handling a Schedule Receiver interrupt. It
> was suggested as an alternative in [1] that the TEE context could be freed
> from complete_domain_destroy().
> 
> [1] https://lore.kernel.org/all/CAHUa44H4YpoxYT7e6WNH5XJFpitZQjqP9Ng4SmTy4eWhyN+F+w@mail.gmail.com/
> 
> Thanks,
> Jens

All patches are now reviewed and/or acked so I think they can get in for the release.

Regards
Bertrand
Julien Grall June 10, 2024, 8:38 p.m. UTC | #2
Hi Bertrand,

On 10/06/2024 16:54, Bertrand Marquis wrote:
> Hi Jens,
> 
>> On 10 Jun 2024, at 08:53, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>>
>> Hi,
>>
>> This patch set adds support for FF-A notifications. We only support
>> global notifications, per vCPU notifications remain unsupported.
>>
>> The first three patches are further cleanup and can be merged before the
>> rest if desired.
>>
>> A physical SGI is used to make Xen aware of pending FF-A notifications. The
>> physical SGI is selected by the SPMC in the secure world. Since it must not
>> already be used by Xen the SPMC is in practice forced to donate one of the
>> secure SGIs, but that's normally not a problem. The SGI handling in Xen is
>> updated to support registration of handlers for SGIs that aren't statically
>> assigned, that is, SGI IDs above GIC_SGI_MAX.
>>
>> The patch "xen/arm: add and call init_tee_secondary()" provides a hook for
>> register the needed per-cpu interrupt handler in "xen/arm: ffa: support
>> notification".
>>
>> The patch "xen/arm: add and call tee_free_domain_ctx()" provides a hook for
>> later freeing of the TEE context. This hook is used in "xen/arm: ffa:
>> support notification" and avoids the problem with TEE context being freed
>> while we need to access it when handling a Schedule Receiver interrupt. It
>> was suggested as an alternative in [1] that the TEE context could be freed
>> from complete_domain_destroy().
>>
>> [1] https://lore.kernel.org/all/CAHUa44H4YpoxYT7e6WNH5XJFpitZQjqP9Ng4SmTy4eWhyN+F+w@mail.gmail.com/
>>
>> Thanks,
>> Jens
> 
> All patches are now reviewed and/or acked so I think they can get in for the release.

This would need a release-ack from Oleksii (I can't seem to find already 
one).

As we discussed last week, I am fine with the idea to merge the FFA 
patches as the feature is tech-preview. But there are some changes in 
the arm generic code. Do you (or Jens) have an assessment of the risk of 
the changes?

Cheers,
Bertrand Marquis June 11, 2024, 7:09 a.m. UTC | #3
Hi Julien and Oleksii,

@Oleksii: Could we consider having this serie merged for next release ?

It is a feature that is in tech-preview at the moment and as such should not have any
consequences on existing system unless it is activated explicitly in the Xen configuration.

There are some changes in the arm common code introducing support to register SGI
interrupt handlers in drivers. As no drivers appart from FF-A is trying to register such
handlers, the risk is minimal for existing systems.


> On 10 Jun 2024, at 22:38, Julien Grall <julien@xen.org> wrote:
> 
> Hi Bertrand,
> 
> On 10/06/2024 16:54, Bertrand Marquis wrote:
>> Hi Jens,
>>> On 10 Jun 2024, at 08:53, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>>> 
>>> Hi,
>>> 
>>> This patch set adds support for FF-A notifications. We only support
>>> global notifications, per vCPU notifications remain unsupported.
>>> 
>>> The first three patches are further cleanup and can be merged before the
>>> rest if desired.
>>> 
>>> A physical SGI is used to make Xen aware of pending FF-A notifications. The
>>> physical SGI is selected by the SPMC in the secure world. Since it must not
>>> already be used by Xen the SPMC is in practice forced to donate one of the
>>> secure SGIs, but that's normally not a problem. The SGI handling in Xen is
>>> updated to support registration of handlers for SGIs that aren't statically
>>> assigned, that is, SGI IDs above GIC_SGI_MAX.
>>> 
>>> The patch "xen/arm: add and call init_tee_secondary()" provides a hook for
>>> register the needed per-cpu interrupt handler in "xen/arm: ffa: support
>>> notification".
>>> 
>>> The patch "xen/arm: add and call tee_free_domain_ctx()" provides a hook for
>>> later freeing of the TEE context. This hook is used in "xen/arm: ffa:
>>> support notification" and avoids the problem with TEE context being freed
>>> while we need to access it when handling a Schedule Receiver interrupt. It
>>> was suggested as an alternative in [1] that the TEE context could be freed
>>> from complete_domain_destroy().
>>> 
>>> [1] https://lore.kernel.org/all/CAHUa44H4YpoxYT7e6WNH5XJFpitZQjqP9Ng4SmTy4eWhyN+F+w@mail.gmail.com/
>>> 
>>> Thanks,
>>> Jens
>> All patches are now reviewed and/or acked so I think they can get in for the release.
> 
> This would need a release-ack from Oleksii (I can't seem to find already one).

You are right, i do not know why in my mind we already got one.

> 
> As we discussed last week, I am fine with the idea to merge the FFA patches as the feature is tech-preview. But there are some changes in the arm generic code. Do you (or Jens) have an assessment of the risk of the changes?

Agree.

In my view, the changes are changing the behaviour of some internal functions if an interrupt handler is register for SGI but should not have any impact for other kind of interrupts.
As there was nothing before the FF-A driver registering such interrupts, the risk of the changes having any impact on existing configurations not activating FF-A is fairly reduced.

@Jens: do you agree with my analysis.

I made a text for Oleksii at the beginning of the mail.

Cheers

Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall
Jens Wiklander June 11, 2024, 7:17 a.m. UTC | #4
Hi all,

On Tue, Jun 11, 2024 at 9:09 AM Bertrand Marquis
<Bertrand.Marquis@arm.com> wrote:
>
> Hi Julien and Oleksii,
>
> @Oleksii: Could we consider having this serie merged for next release ?
>
> It is a feature that is in tech-preview at the moment and as such should not have any
> consequences on existing system unless it is activated explicitly in the Xen configuration.
>
> There are some changes in the arm common code introducing support to register SGI
> interrupt handlers in drivers. As no drivers appart from FF-A is trying to register such
> handlers, the risk is minimal for existing systems.
>
>
> > On 10 Jun 2024, at 22:38, Julien Grall <julien@xen.org> wrote:
> >
> > Hi Bertrand,
> >
> > On 10/06/2024 16:54, Bertrand Marquis wrote:
> >> Hi Jens,
> >>> On 10 Jun 2024, at 08:53, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> >>>
> >>> Hi,
> >>>
> >>> This patch set adds support for FF-A notifications. We only support
> >>> global notifications, per vCPU notifications remain unsupported.
> >>>
> >>> The first three patches are further cleanup and can be merged before the
> >>> rest if desired.
> >>>
> >>> A physical SGI is used to make Xen aware of pending FF-A notifications. The
> >>> physical SGI is selected by the SPMC in the secure world. Since it must not
> >>> already be used by Xen the SPMC is in practice forced to donate one of the
> >>> secure SGIs, but that's normally not a problem. The SGI handling in Xen is
> >>> updated to support registration of handlers for SGIs that aren't statically
> >>> assigned, that is, SGI IDs above GIC_SGI_MAX.
> >>>
> >>> The patch "xen/arm: add and call init_tee_secondary()" provides a hook for
> >>> register the needed per-cpu interrupt handler in "xen/arm: ffa: support
> >>> notification".
> >>>
> >>> The patch "xen/arm: add and call tee_free_domain_ctx()" provides a hook for
> >>> later freeing of the TEE context. This hook is used in "xen/arm: ffa:
> >>> support notification" and avoids the problem with TEE context being freed
> >>> while we need to access it when handling a Schedule Receiver interrupt. It
> >>> was suggested as an alternative in [1] that the TEE context could be freed
> >>> from complete_domain_destroy().
> >>>
> >>> [1] https://lore.kernel.org/all/CAHUa44H4YpoxYT7e6WNH5XJFpitZQjqP9Ng4SmTy4eWhyN+F+w@mail.gmail.com/
> >>>
> >>> Thanks,
> >>> Jens
> >> All patches are now reviewed and/or acked so I think they can get in for the release.
> >
> > This would need a release-ack from Oleksii (I can't seem to find already one).
>
> You are right, i do not know why in my mind we already got one.
>
> >
> > As we discussed last week, I am fine with the idea to merge the FFA patches as the feature is tech-preview. But there are some changes in the arm generic code. Do you (or Jens) have an assessment of the risk of the changes?
>
> Agree.
>
> In my view, the changes are changing the behaviour of some internal functions if an interrupt handler is register for SGI but should not have any impact for other kind of interrupts.
> As there was nothing before the FF-A driver registering such interrupts, the risk of the changes having any impact on existing configurations not activating FF-A is fairly reduced.
>
> @Jens: do you agree with my analysis.

Yes, I agree.

Cheers,
Jens
Oleksii June 11, 2024, 10:36 a.m. UTC | #5
Hi Bertrand and Julien,

On Tue, 2024-06-11 at 07:09 +0000, Bertrand Marquis wrote:
> Hi Julien and Oleksii,
> 
> @Oleksii: Could we consider having this serie merged for next release
> ?
We can consider including it in Xen 4.19 as it has a low impact on
existing systems and needs to be explicitly activated:
 Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

~ Oleksii

> 
> It is a feature that is in tech-preview at the moment and as such
> should not have any
> consequences on existing system unless it is activated explicitly in
> the Xen configuration.
> 
> There are some changes in the arm common code introducing support to
> register SGI
> interrupt handlers in drivers. As no drivers appart from FF-A is
> trying to register such
> handlers, the risk is minimal for existing systems.
> 
> 
> > On 10 Jun 2024, at 22:38, Julien Grall <julien@xen.org> wrote:
> > 
> > Hi Bertrand,
> > 
> > On 10/06/2024 16:54, Bertrand Marquis wrote:
> > > Hi Jens,
> > > > On 10 Jun 2024, at 08:53, Jens Wiklander
> > > > <jens.wiklander@linaro.org> wrote:
> > > > 
> > > > Hi,
> > > > 
> > > > This patch set adds support for FF-A notifications. We only
> > > > support
> > > > global notifications, per vCPU notifications remain
> > > > unsupported.
> > > > 
> > > > The first three patches are further cleanup and can be merged
> > > > before the
> > > > rest if desired.
> > > > 
> > > > A physical SGI is used to make Xen aware of pending FF-A
> > > > notifications. The
> > > > physical SGI is selected by the SPMC in the secure world. Since
> > > > it must not
> > > > already be used by Xen the SPMC is in practice forced to donate
> > > > one of the
> > > > secure SGIs, but that's normally not a problem. The SGI
> > > > handling in Xen is
> > > > updated to support registration of handlers for SGIs that
> > > > aren't statically
> > > > assigned, that is, SGI IDs above GIC_SGI_MAX.
> > > > 
> > > > The patch "xen/arm: add and call init_tee_secondary()" provides
> > > > a hook for
> > > > register the needed per-cpu interrupt handler in "xen/arm: ffa:
> > > > support
> > > > notification".
> > > > 
> > > > The patch "xen/arm: add and call tee_free_domain_ctx()"
> > > > provides a hook for
> > > > later freeing of the TEE context. This hook is used in
> > > > "xen/arm: ffa:
> > > > support notification" and avoids the problem with TEE context
> > > > being freed
> > > > while we need to access it when handling a Schedule Receiver
> > > > interrupt. It
> > > > was suggested as an alternative in [1] that the TEE context
> > > > could be freed
> > > > from complete_domain_destroy().
> > > > 
> > > > [1]
> > > > https://lore.kernel.org/all/CAHUa44H4YpoxYT7e6WNH5XJFpitZQjqP9Ng4SmTy4eWhyN+F+w@mail.gmail.com/
> > > > 
> > > > Thanks,
> > > > Jens
> > > All patches are now reviewed and/or acked so I think they can get
> > > in for the release.
> > 
> > This would need a release-ack from Oleksii (I can't seem to find
> > already one).
> 
> You are right, i do not know why in my mind we already got one.
> 
> > 
> > As we discussed last week, I am fine with the idea to merge the FFA
> > patches as the feature is tech-preview. But there are some changes
> > in the arm generic code. Do you (or Jens) have an assessment of the
> > risk of the changes?
> 
> Agree.
> 
> In my view, the changes are changing the behaviour of some internal
> functions if an interrupt handler is register for SGI but should not
> have any impact for other kind of interrupts.
> As there was nothing before the FF-A driver registering such
> interrupts, the risk of the changes having any impact on existing
> configurations not activating FF-A is fairly reduced.
> 
> @Jens: do you agree with my analysis.
> 
> I made a text for Oleksii at the beginning of the mail.
> 
> Cheers
> 
> Bertrand
> 
> > 
> > Cheers,
> > 
> > -- 
> > Julien Grall
> 
>
Julien Grall June 13, 2024, 12:44 p.m. UTC | #6
Hi,

On 11/06/2024 11:36, Oleksii K. wrote:
> Hi Bertrand and Julien,
> 
> On Tue, 2024-06-11 at 07:09 +0000, Bertrand Marquis wrote:
>> Hi Julien and Oleksii,
>>
>> @Oleksii: Could we consider having this serie merged for next release
>> ?
> We can consider including it in Xen 4.19 as it has a low impact on
> existing systems and needs to be explicitly activated:
>   Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

It is now merged.

Cheers,
Bertrand Marquis June 13, 2024, 1:02 p.m. UTC | #7
Hi Julien,

> On 13 Jun 2024, at 14:44, Julien Grall <julien@xen.org> wrote:
> 
> Hi,
> 
> On 11/06/2024 11:36, Oleksii K. wrote:
>> Hi Bertrand and Julien,
>> On Tue, 2024-06-11 at 07:09 +0000, Bertrand Marquis wrote:
>>> Hi Julien and Oleksii,
>>> 
>>> @Oleksii: Could we consider having this serie merged for next release
>>> ?
>> We can consider including it in Xen 4.19 as it has a low impact on
>> existing systems and needs to be explicitly activated:
>>  Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> 
> It is now merged.

Great, thanks a lot :-)

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall