[PATCH v2 10/21] arm: socfpga: Add secure register access helper functions for SoC 64bits

[Ang, Chee Hong]

> On 2/19/20 1:25 PM, chee.hong.ang at intel.com wrote:
> > From: Chee Hong Ang <chee.hong.ang at intel.com>
> >
> > These secure register access functions allow U-Boot proper running at
> > EL2 (non-secure) to access System Manager's secure registers by
> > calling the ATF's PSCI runtime services (EL3/secure). If these helper
> > functions are called from secure mode (EL3), the helper function will
> > direct access the secure registers without calling the ATF's PSCI
> > runtime services.
> >
> > Signed-off-by: Chee Hong Ang <chee.hong.ang at intel.com>
> > ---
> >  arch/arm/mach-socfpga/Makefile                     |  6 +++
> >  .../mach-socfpga/include/mach/secure_reg_helper.h  | 20 ++++++++
> >  arch/arm/mach-socfpga/secure_reg_helper.c          | 57
> ++++++++++++++++++++++
> >  3 files changed, 83 insertions(+)
> >  create mode 100644
> > arch/arm/mach-socfpga/include/mach/secure_reg_helper.h
> >  create mode 100644 arch/arm/mach-socfpga/secure_reg_helper.c
> >
> > diff --git a/arch/arm/mach-socfpga/Makefile
> > b/arch/arm/mach-socfpga/Makefile index 3310e92..e59587b 100644
> > --- a/arch/arm/mach-socfpga/Makefile
> > +++ b/arch/arm/mach-socfpga/Makefile
> > @@ -53,6 +53,12 @@ obj-y	+= wrap_pinmux_config_s10.o
> >  obj-y	+= wrap_pll_config_s10.o
> >  endif
> >
> > +ifndef CONFIG_SPL_BUILD
> > +ifdef CONFIG_SPL_ATF
> > +obj-y	+= secure_reg_helper.o
> 
> obj-$(FOO) += bar.o , you don't need the inner ifdef
OK. It's cleaner. Thanks.
> 
> > +endif
> > +endif
> 
> [...]
> 
> > +++ b/arch/arm/mach-socfpga/include/mach/secure_reg_helper.h
> > @@ -0,0 +1,20 @@
> > +/* SPDX-License-Identifier: GPL-2.0
> > + *
> > + * Copyright (C) 2019 Intel Corporation <www.intel.com>
> > + *
> > + */
> > +
> > +#ifndef	_SECURE_REG_HELPER_H_
> > +#define	_SECURE_REG_HELPER_H_
> > +
> > +#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_ATF)
> > +u32 socfpga_secure_reg_read32(phys_addr_t reg_addr); void
> > +socfpga_secure_reg_write32(u32 val, phys_addr_t reg_addr); void
> > +socfpga_secure_reg_update32(phys_addr_t reg_addr, u32 mask, u32 val);
> > +#else
> > +#define socfpga_secure_reg_read32	readl
> > +#define socfpga_secure_reg_write32	writel
> > +#define socfpga_secure_reg_update32	clrsetbits_le32
> > +#endif
> 
> I think I don't understand how this is supposed to work. Would every place in U-
> Boot have to be patched to call these functions now ?

Not every register access need this. Only those accessing registers in secure zone such
as 'System Manager' registers need to call this. It's basically determine whether the driver
should issue SMC/PSCI call if it's running in EL2 (non-secure) or access the registers directly
by simply using readl/writel and etc if it's running in EL3 (secure).
Accessing those registers in secure zone in non-secure mode (EL2) will cause SError exception.
So we can determine this behaviour in compile time:
SPL always running in EL3. So it just simply fallback to use readl/writel/clrsetbits_le32.

For U-Boot proper (SSBL), there are 2 scenarios:
1) If CONFIG_SPL_ATF is defined, it means ATF is supported. It implies that U-Boot proper will be
running in EL2 (non-secure), then it will use SMC/PSCI calls to access the secure registers.

2) CONFIG_SPL_ATF is not defined, no ATF support. U-Boot proper will be running in EL3 which
will fall back to simply using the direct access functions (readl/writel and etc).
> 
> [...]