Re: [edk2-devel] [PATCH v2 3/8] ArmPkg: Add Arm Firmware TRNG library

["Leif Lindholm" <quic_llindhol@quicinc.com>]

On Thu, Mar 24, 2022 at 15:56:32 +0100, Pierre Gondois wrote:
> > > > > +/** Invoke the monitor call using the appropriate conduit.
> > > > > +    If PcdMonitorConduitHvc is TRUE use the HVC conduit else use SMC conduit.
> > > > > +
> > > > > +  @param [in, out]  Args    Arguments passed to and returned from the monitor.
> > > > > +
> > > > > +  @return  VOID
> > > > > +**/
> > > > > +STATIC
> > > > > +VOID
> > > > > +ArmCallMonitor (
> > > > > +  IN OUT ARM_MONITOR_ARGS   *Args
> > > > > +  )
> > > > > +{
> > > > > +  if (FeaturePcdGet (PcdMonitorConduitHvc)) {
> > > > > +    ArmCallHvc ((ARM_HVC_ARGS*)Args);
> > > > > +  } else {
> > > > > +    ArmCallSmc ((ARM_SMC_ARGS*)Args);
> > > > > +  }
> > > > > +}
> > > > Should this be in (a potentially renamed) ArmSmcLib?
> > > [SAMI] Looking at ArmSmcLib and ArmHvcLib libraries there is not much
> > > difference in the code other than the SMC/HVC call. Please let me know
> > > if I should submit a patch to unify these in ArmMonitorLib?
> > > The ArmCall<Smc|Hvc> APIs would still remain the same but moved to
> > > ArmMonitorLib.
> > 
> > Hello Leif,
> > About your comment, I am not sure I understand it correctly. Assuming the
> > function allowing to choose the conduit looks like:
> > 
> > VOID
> > EFIAPI
> > ArmConduitCall (UINT8 Conduit) {
> >     if (Conduit == 0) {
> >       ArmCallHvc ((ARM_HVC_ARGS*)Args);
> >     } else if (Conduit == 1) {
> >       ArmCallSmc ((ARM_SMC_ARGS*)Args);
> >     } else {
> >      ASSERT (FALSE);
> >     }
> > }
> > 
> > Do you suggest to:
> > 1. Make ArmSmcLib dependent on ArmHvcLib and add ArmConduitCall()
> >      in ArmSmcLib (or do the opposite with the ArmHvcLib)
> > 2. Merge ArmSmcLib and ArmHvcLib in a new ArmConduitLibrary and add
> >      ArmConduitCall() in this new library.
> > 3. Add an ArmConduitLibrary, relying on ArmSmcLib and ArmHvcLib, and having
> >      only one function: ArmConduitCall()
> >
> > 
> > 2. would make the Hvc and Smc calls really tied together.
> > 3. would avoid creating new dependencies on existing libraries (i.e. a
> > platform only using the ArmSmcLib would not require to have a NULL instance of
> > ArmHvcLib). I assume you meant 3.

I think 2 and 3 both capture the spirit of my request.

2 could potentially be achieved with a FixedPcd and conditionals in
asm (less sure about the armasm/vs situation).

I guess the question is if we think it plausible that a platform might
both want to use this new ArmConduitCall *and* directly make use of
ArmHvcLib/ArmSmcLib.

Best Regards,

Leif