Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] Documentation: DT: Add optional 'timeout-sec' property for sp805
From: Guenter Roeck @ 2018-05-24 16:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <91d27df4-29d0-de7f-6805-c3e5f329caf3@arm.com>

On Thu, May 24, 2018 at 02:25:34PM +0100, Robin Murphy wrote:
> On 23/05/18 19:10, Guenter Roeck wrote:
> >On Wed, May 23, 2018 at 11:57:25AM +0100, Robin Murphy wrote:
> >>On 22/05/18 19:47, Ray Jui wrote:
> >>>Update the SP805 binding document to add optional 'timeout-sec'
> >>>devicetree property
> >>>
> >>>Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> >>>Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> >>>---
> >>>  Documentation/devicetree/bindings/watchdog/sp805-wdt.txt | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>>diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> >>>index edc4f0e..f898a86 100644
> >>>--- a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> >>>+++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> >>>@@ -19,6 +19,8 @@ Required properties:
> >>>  Optional properties:
> >>>  - interrupts : Should specify WDT interrupt number.
> >>>+- timeout-sec : Should specify default WDT timeout in seconds. If unset, the
> >>>+                default timeout is 30 seconds
> >>
> >>According to the SP805 TRM, the default interval is dependent on the rate of
> >>WDOGCLK, but would typically be a lot longer than that :/
> >>
> >Depends on the definition of "default". In the context of watchdog drivers,
> >it is (or should be) a driver default, not a chip default.
> 
> DT describes hardware, not driver behaviour.
> 

In this case it describes expected system behavior. Most definitely
it does not describe some hardware default.

Please note that I do not engage in discussions I consider bike-shedding.
This is one of those. Dropping out.

Guenter

^ permalink raw reply

* [PATCH v3 2/6] Documentation: DT: Add optional 'timeout-sec' property for sp805
From: Guenter Roeck @ 2018-05-24 16:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527120924-12131-3-git-send-email-ray.jui@broadcom.com>

On Wed, May 23, 2018 at 05:15:20PM -0700, Ray Jui wrote:
> Update the SP805 binding document to add optional 'timeout-sec'
> devicetree property
> 
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> ---
>  Documentation/devicetree/bindings/watchdog/arm,sp805.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> index 0fa3629..1debea3 100644
> --- a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> +++ b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> @@ -19,6 +19,8 @@ Required properties:
>  
>  Optional properties:
>  - interrupts:  Should specify WDT interrupt number
> +- timeout-sec: Should specify default WDT timeout in seconds. If unset, the
> +               default timeout in the driver is 30 seconds

"... the default timeout is determined by the driver" might be better.
If you want to mandate a default here (not sure if that is a good idea),
I would suggest to use something like "should be 30 seconds".

Guenter

>  
>  Example:
>  	watchdog at 66090000 {
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v3 4/6] watchdog: sp805: set WDOG_HW_RUNNING when appropriate
From: Guenter Roeck @ 2018-05-24 16:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527120924-12131-5-git-send-email-ray.jui@broadcom.com>

On Wed, May 23, 2018 at 05:15:22PM -0700, Ray Jui wrote:
> If the watchdog hardware is already enabled during the boot process,
> when the Linux watchdog driver loads, it should reset the watchdog and
> tell the watchdog framework. As a result, ping can be generated from
> the watchdog framework, until the userspace watchdog daemon takes over
> control
> 
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

I have one question, though: Is it really correct that both
INT_ENABLE _and_ RESET_ENABLE have to be set to enable the watdog ?
What if only RESET_ENABLE is set ?

Thanks,
Guenter

> ---
>  drivers/watchdog/sp805_wdt.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
> index 1484609..d662a6f 100644
> --- a/drivers/watchdog/sp805_wdt.c
> +++ b/drivers/watchdog/sp805_wdt.c
> @@ -42,6 +42,7 @@
>  	/* control register masks */
>  	#define	INT_ENABLE	(1 << 0)
>  	#define	RESET_ENABLE	(1 << 1)
> +	#define	ENABLE_MASK	(INT_ENABLE | RESET_ENABLE)
>  #define WDTINTCLR		0x00C
>  #define WDTRIS			0x010
>  #define WDTMIS			0x014
> @@ -74,6 +75,15 @@ module_param(nowayout, bool, 0);
>  MODULE_PARM_DESC(nowayout,
>  		"Set to 1 to keep watchdog running after device release");
>  
> +/* returns true if wdt is running; otherwise returns false */
> +static bool wdt_is_running(struct watchdog_device *wdd)
> +{
> +	struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
> +	u32 wdtcontrol = readl_relaxed(wdt->base + WDTCONTROL);
> +
> +	return (wdtcontrol & ENABLE_MASK) == ENABLE_MASK;
> +}
> +
>  /* This routine finds load value that will reset system in required timout */
>  static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout)
>  {
> @@ -239,6 +249,15 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
>  	watchdog_init_timeout(&wdt->wdd, 0, &adev->dev);
>  	wdt_setload(&wdt->wdd, wdt->wdd.timeout);
>  
> +	/*
> +	 * If HW is already running, enable/reset the wdt and set the running
> +	 * bit to tell the wdt subsystem
> +	 */
> +	if (wdt_is_running(&wdt->wdd)) {
> +		wdt_enable(&wdt->wdd);
> +		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
> +	}
> +
>  	ret = watchdog_register_device(&wdt->wdd);
>  	if (ret) {
>  		dev_err(&adev->dev, "watchdog_register_device() failed: %d\n",
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v3 3/6] arm64: irqflags: Use ICC sysregs to implement IRQ masking
From: Marc Zyngier @ 2018-05-24 16:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526902515-13769-4-git-send-email-julien.thierry@arm.com>

Hi Julien,

On 21/05/18 12:35, Julien Thierry wrote:
> From: Daniel Thompson <daniel.thompson@linaro.org>
> 
> Currently irqflags is implemented using the PSR's I bit. It is possible
> to implement irqflags by using the co-processor interface to the GIC.
> Using the co-processor interface makes it feasible to simulate NMIs
> using GIC interrupt prioritization.
> 
> This patch changes the irqflags macros to modify, save and restore
> ICC_PMR_EL1. This has a substantial knock on effect for the rest of
> the kernel. There are four reasons for this:
> 
> 1. The state of the PMR becomes part of the interrupt context and must be
>    saved and restored during exceptions. It is saved on the stack as part
>    of the saved context when an interrupt/exception is taken.
> 
> 2. The hardware automatically masks the I bit (at boot, during traps, etc).
>    When the I bit is set by hardware we must add code to switch from I
>    bit masking and PMR masking:
>        - For IRQs, this is done after the interrupt has been acknowledged
>        	 avoiding the need to unmask.
>        - For other exceptions, this is done right after saving the context.
> 
> 3. Some instructions, such as wfi, require that the PMR not be used
>    for interrupt masking. Before calling these instructions we must
>    switch from PMR masking to I bit masking.
>    This is also the case when KVM runs a guest, if the CPU receives
>    an interrupt from the host, interrupts must not be masked in PMR
>    otherwise the GIC will not signal it to the CPU.
> 
> 4. We use the alternatives system to allow a single kernel to boot and
>    be switched to the alternative masking approach at runtime.
> 
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> [julien.thierry at arm.com: changes reflected in commit,
> 			 message, fixes, renaming]
> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Christoffer Dall <christoffer.dall@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: James Morse <james.morse@arm.com>
> ---
>  arch/arm64/Kconfig                     |  15 ++++
>  arch/arm64/include/asm/arch_gicv3.h    |  20 ++++++
>  arch/arm64/include/asm/assembler.h     |  25 ++++++-
>  arch/arm64/include/asm/daifflags.h     |  36 +++++++---
>  arch/arm64/include/asm/efi.h           |   5 ++
>  arch/arm64/include/asm/irqflags.h      | 125 +++++++++++++++++++++++++++++++++
>  arch/arm64/include/asm/kvm_host.h      |  14 ++++
>  arch/arm64/include/asm/processor.h     |   4 ++
>  arch/arm64/include/asm/ptrace.h        |  14 +++-
>  arch/arm64/kernel/asm-offsets.c        |   1 +
>  arch/arm64/kernel/entry.S              |  28 ++++++--
>  arch/arm64/kernel/head.S               |  37 ++++++++++
>  arch/arm64/kernel/process.c            |   6 ++
>  arch/arm64/kernel/smp.c                |   8 +++
>  arch/arm64/kvm/hyp/switch.c            |  25 +++++++
>  arch/arm64/mm/fault.c                  |   5 +-
>  arch/arm64/mm/proc.S                   |  23 ++++++
>  drivers/irqchip/irq-gic-v3-its.c       |   2 +-
>  drivers/irqchip/irq-gic-v3.c           |  82 +++++++++++----------
>  include/linux/irqchip/arm-gic-common.h |   6 ++
>  include/linux/irqchip/arm-gic.h        |   5 --
>  21 files changed, 423 insertions(+), 63 deletions(-)
I've commented about this particular patch offline, but let me state it
on the list:

As it is, this patch is almost impossible to review. It turns the
interrupt masking upside down, messes with the GIC, hacks KVM... Too
many things change at once, and I find it very hard to build a mental
picture of the changes just by staring at it.

Can you please try to split it into related chunks, moving the enabling
of the feature right at the end, so that the reviewers can have a chance
to understand it? It should make it much easier to review.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH] arm64: dts: msm8916: fix Coresight ETF graph connections
From: Mathieu Poirier @ 2018-05-24 16:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524044853.713.23166@silver>

On 23 May 2018 at 22:48, Ivan T. Ivanov <iivanov.xz@gmail.com> wrote:
> Quoting Rob Herring (2018-05-23 23:29:29)
>> On Tue, May 8, 2018 at 10:09 AM, Rob Herring <robh@kernel.org> wrote:
>> > The ETF input should be connected to the funnel output, and the ETF
>> > output should be connected to the replicator input. The labels are wrong
>> > and these got swapped:
>> >
>> > Warning (graph_endpoint): /soc/funnel at 821000/ports/port at 8/endpoint: graph connection to node '/soc/etf at 825000/ports/port at 1/endpoint' is not bidirectional
>> > Warning (graph_endpoint): /soc/replicator at 824000/ports/port at 2/endpoint: graph connection to node '/soc/etf at 825000/ports/port at 0/endpoint' is not bidirectional
>> >
>> > Fixes: 7c10da373698 ("arm64: dts: qcom: Add msm8916 CoreSight components")
>> > Cc: Ivan T. Ivanov <ivan.ivanov@linaro.org>
>> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> > Cc: Andy Gross <andy.gross@linaro.org>
>> > Cc: David Brown <david.brown@linaro.org>
>> > Cc: linux-arm-msm at vger.kernel.org
>> > Signed-off-by: Rob Herring <robh@kernel.org>
>> > ---
>> >  arch/arm64/boot/dts/qcom/msm8916.dtsi | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> Ping. Still seeing warnings in -next for these.
>>
>
>  Hi Rob,
>
>  I no longer have access to this hardware and documentation.
>  I am sure that Mathieu and friends will take care for verification
>  of this patch :-)

The code triggers on the "slave-mode" property rather than the labels,
so this patch has no effect on how a path is established.  I've tested
this on a 410c and things look good.

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>
>  Regards,
>  Ivan
>

^ permalink raw reply

* [PATCHv5 0/8] Add Intel Stratix10 FPGA manager and service layer
From: richard.gong at linux.intel.com @ 2018-05-24 16:33 UTC (permalink / raw)
  To: linux-arm-kernel

From: Richard Gong <richard.gong@intel.com>

This is the 5th submission of Intel stratix10 service layer patches. Intel
Stratix10 FPGA manager, which is 1st Stratix10 service layer client, is
included in this submission.

Stratix10 service layer patches have been reviewed internally by Alan Tull
and other colleagues at Intel.

Some features of the Intel Stratix10 SoC require a level of privilege
higher than the kernel is granted. Such secure features include
FPGA programming. In terms of the ARMv8 architecture, the kernel runs
at Exception Level 1 (EL1), access to the features requires
Exception Level 3 (EL3).

The Intel Stratix10 service layer provides an in kernel API for drivers to
request access to the secure features. The requests are queued and
processed one by one. ARM?s SMCCC is used to pass the execution of the
requests on to a secure monitor (EL3).

Later the Intel Stratix10 service layer driver will be extended to provide
services for QSPI, Crypto and warm reset.

v2: add patches for FPGA manager, FPGA manager binding, dts and defconfig
    remove intel-service subdirectory and intel-service.h, move intel-smc.h
    and intel-service.c to driver/misc subdirectory
    remove global variables
    change service layer driver be 'default n'
    correct SPDX markers
    add timeout for do..while() loop
    add kernel-doc for the functions and structs, correct multiline comments
    replace kfifo_in/kfifo_out with kfifo_in_spinlocked/kfifo_out_spinlocked
    rename struct intel_svc_data (at client header) to intel_svc_client_msg
    rename struct intel_svc_private_mem to intel_svc_data
    other corrections/changes from Intel internal code reviews
v3: change all exported functions with "intel_svc_" as the prefix
    increase timeout values for claiming back submitted buffer(s)
    rename struct intel_command_reconfig_payload to
    struct intel_svc_command_reconfig_payload
    add pr_err() to provide the error return value
    change to put fpga_mgr node under firmware/svc node
    change to FPGA manager to align the update of service client APIs, and the
    update of fpga_mgr device node
    Other corrections/changes
v4: s/intel/stratix10/ on some variables, structs, functions, and file names
    intel-service.c -> stratix10-svc.c
    intel-smc.h -> stratix10-smc.h
    intel-service-client.h -> stratix10-svc-client.h
    remove non-kernel-doc formatting
    s/fpga-mgr at 0/fpga-mgr/ to remove unit_address at fpga_mgr node
    add Rob's Reviewed-by
    add Richard's signed-off-by
v5: add a new API statix10_svc_done() which is called by service client
    when client request is completed or error occurs during request
    process. Which allows service layer to free its resources.
    remove dummy client from service layer client header and service layer
    source file.
    add Rob's Reviewed-by
    add a new file stratix10-svc.rst and add that to driver-api/index.rst
    kernel-doc fixes

Alan Tull (3):
  dt-bindings: fpga: add Stratix10 SoC FPGA manager binding
  arm64: dts: stratix10: add fpga manager and region
  fpga: add intel stratix10 soc fpga manager driver

Richard Gong (5):
  dt-bindings, firmware: add Intel Stratix10 service layer binding
  arm64: dts: stratix10: add stratix10 service driver binding to base
    dtsi
  driver, misc: add Intel Stratix10 service layer driver
  defconfig: enable fpga and service layer
  Documentation: driver-api: add stratix10 service layer

 .../bindings/firmware/intel,stratix10-svc.txt      |  57 ++
 .../bindings/fpga/intel-stratix10-soc-fpga-mgr.txt |  17 +
 Documentation/driver-api/index.rst                 |   1 +
 Documentation/driver-api/stratix10-svc.rst         |  32 +
 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi  |  33 +
 arch/arm64/configs/defconfig                       |   6 +
 drivers/fpga/Kconfig                               |   6 +
 drivers/fpga/Makefile                              |   1 +
 drivers/fpga/stratix10-soc.c                       | 545 ++++++++++++
 drivers/misc/Kconfig                               |  12 +
 drivers/misc/Makefile                              |   1 +
 drivers/misc/stratix10-smc.h                       | 205 +++++
 drivers/misc/stratix10-svc.c                       | 984 +++++++++++++++++++++
 include/linux/stratix10-svc-client.h               | 199 +++++
 14 files changed, 2099 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/intel,stratix10-svc.txt
 create mode 100644 Documentation/devicetree/bindings/fpga/intel-stratix10-soc-fpga-mgr.txt
 create mode 100644 Documentation/driver-api/stratix10-svc.rst
 create mode 100644 drivers/fpga/stratix10-soc.c
 create mode 100644 drivers/misc/stratix10-smc.h
 create mode 100644 drivers/misc/stratix10-svc.c
 create mode 100644 include/linux/stratix10-svc-client.h

-- 
2.7.4

^ permalink raw reply

* [PATCHv5 1/8] dt-bindings, firmware: add Intel Stratix10 service layer binding
From: richard.gong at linux.intel.com @ 2018-05-24 16:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527179600-26441-1-git-send-email-richard.gong@linux.intel.com>

From: Richard Gong <richard.gong@intel.com>

Add a device tree binding for the Intel Stratix10 service layer driver

Signed-off-by: Richard Gong <richard.gong@intel.com>
Signed-off-by: Alan Tull <atull@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
v2: Change to put service layer driver node under the firmware node
    Change compatible to "intel, stratix10-svc"
v3: No change
v4: Add Rob's Reviewed-by
v5: No change
---
 .../bindings/firmware/intel,stratix10-svc.txt      | 57 ++++++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/intel,stratix10-svc.txt

diff --git a/Documentation/devicetree/bindings/firmware/intel,stratix10-svc.txt b/Documentation/devicetree/bindings/firmware/intel,stratix10-svc.txt
new file mode 100644
index 0000000..1fa6606
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/intel,stratix10-svc.txt
@@ -0,0 +1,57 @@
+Intel Service Layer Driver for Stratix10 SoC
+============================================
+Intel Stratix10 SoC is composed of a 64 bit quad-core ARM Cortex A53 hard
+processor system (HPS) and Secure Device Manager (SDM). When the FPGA is
+configured from HPS, there needs to be a way for HPS to notify SDM the
+location and size of the configuration data. Then SDM will get the
+configuration data from that location and perform the FPGA configuration.
+
+To meet the whole system security needs and support virtual machine requesting
+communication with SDM, only the secure world of software (EL3, Exception
+Layer 3) can interface with SDM. All software entities running on other
+exception layers must channel through the EL3 software whenever it needs
+service from SDM.
+
+Intel Stratix10 service layer driver, running at privileged exception level
+(EL1, Exception Layer 1), interfaces with the service providers and provides
+the services for FPGA configuration, QSPI, Crypto and warm reset. Service layer
+driver also manages secure monitor call (SMC) to communicate with secure monitor
+code running in EL3.
+
+Required properties:
+-------------------
+The svc node has the following mandatory properties, must be located under
+the firmware node.
+
+- compatible: "intel,stratix10-svc"
+- method: smc or hvc
+        smc - Secure Monitor Call
+        hvc - Hypervisor Call
+- memory-region:
+	phandle to the reserved memory node. See
+	Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+	for details
+
+Example:
+-------
+
+	reserved-memory {
+                #address-cells = <2>;
+                #size-cells = <2>;
+                ranges;
+
+                service_reserved: svcbuffer at 0 {
+                        compatible = "shared-dma-pool";
+                        reg = <0x0 0x0 0x0 0x1000000>;
+                        alignment = <0x1000>;
+                        no-map;
+                };
+        };
+
+	firmware {
+		svc {
+			compatible = "intel,stratix10-svc";
+			method = "smc";
+			memory-region = <&service_reserved>;
+		};
+	};
-- 
2.7.4

^ permalink raw reply related

* [PATCHv5 2/8] arm64: dts: stratix10: add stratix10 service driver binding to base dtsi
From: richard.gong at linux.intel.com @ 2018-05-24 16:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527179600-26441-1-git-send-email-richard.gong@linux.intel.com>

From: Richard Gong <richard.gong@intel.com>

Add Intel Stratix10 service layer to the device tree

Signed-off-by: Richard Gong <richard.gong@intel.com>
Signed-off-by: Alan Tull <atull@kernel.org>
---
v2: Change to put service layer driver node under the firmware node
    Change compatible to "intel, stratix10-svc"
v3: No change
v4: s/service driver/stratix10 service driver/ in subject line
v5: No change
---
 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
index d8c94d5..c257287 100644
--- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
+++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
@@ -24,6 +24,19 @@
 	#address-cells = <2>;
 	#size-cells = <2>;
 
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		service_reserved: svcbuffer at 0 {
+			compatible = "shared-dma-pool";
+			reg = <0x0 0x0 0x0 0x1000000>;
+			alignment = <0x1000>;
+			no-map;
+		};
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -487,5 +500,13 @@
 
 			status = "disabled";
 		};
+
+		firmware {
+			svc {
+				compatible = "intel,stratix10-svc";
+				method = "smc";
+				memory-region = <&service_reserved>;
+			};
+		};
 	};
 };
-- 
2.7.4

^ permalink raw reply related

* [PATCHv5 3/8] driver, misc: add Intel Stratix10 service layer driver
From: richard.gong at linux.intel.com @ 2018-05-24 16:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527179600-26441-1-git-send-email-richard.gong@linux.intel.com>

From: Richard Gong <richard.gong@intel.com>

Some features of the Intel Stratix10 SoC require a level of privilege
higher than the kernel is granted. Such secure features include
FPGA programming. In terms of the ARMv8 architecture, the kernel runs
at Exception Level 1 (EL1), access to the features requires
Exception Level 3 (EL3).

The Intel Stratix10 SoC service layer provides an in kernel API for
drivers to request access to the secure features. The requests are queued
and processed one by one. ARM?s SMCCC is used to pass the execution
of the requests on to a secure monitor (EL3).

The header file stratix10-sve-client.h defines the interface between
service providers (FPGA manager is one of them) and service layer.

The header file stratix10-smc.h defines the secure monitor call (SMC)
message protocols used for service layer driver in normal world
(EL1) to communicate with secure monitor SW in secure monitor exception
level 3 (EL3).

Signed-off-by: Richard Gong <richard.gong@intel.com>
Signed-off-by: Alan Tull <atull@kernel.org>
---
v2: Remove intel-service subdirectory and intel-service.h, move
    intel-smc.h and intel-service.c to driver/misc subdirectory
    Correct SPDX markers
    Change service layer driver be 'default n'
    Remove global variables
    Add timeout for do..while() loop
    Add kernel-doc for the functions and structs, correct multiline comments
    Replace kfifo_in/kfifo_out with kfifo_in_spinlocked/kfifo_out_spinlocked
    rename struct intel_svc_data (at client header) to intel_svc_client_msg
    rename struct intel_svc_private_mem to intel_svc_data
    Other corrections/changes from Intel internal code reviews
v3: Change all exported functions with "intel_svc_" as the prefix
    Increase timeout values for claiming back submitted buffer(s)
    Rename struct intel_command_reconfig_payload to
    struct intel_svc_command_reconfig_payload
    Add pr_err() to provide the error return value
    Other corrections/changes
v4: s/intel/stratix10/ on some variables, structs, functions, and file names
    intel-service.c -> stratix10-svc.c
    intel-smc.h -> stratix10-smc.h
    intel-service-client.h -> stratix10-svc-client.h
    Remove non-kernel-doc formatting
v5: add a new API statix10_svc_done() which is called by service client
    when client request is completed or error occurs during request
    process. Which allows service layer to free its resources.
    remove dummy client from service layer client header and service
    layer source file.
    kernel-doc fixes
---
 drivers/misc/Kconfig                 |  12 +
 drivers/misc/Makefile                |   1 +
 drivers/misc/stratix10-smc.h         | 205 ++++++++
 drivers/misc/stratix10-svc.c         | 984 +++++++++++++++++++++++++++++++++++
 include/linux/stratix10-svc-client.h | 199 +++++++
 5 files changed, 1401 insertions(+)
 create mode 100644 drivers/misc/stratix10-smc.h
 create mode 100644 drivers/misc/stratix10-svc.c
 create mode 100644 include/linux/stratix10-svc-client.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 5d71300..5d5b648 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -138,6 +138,18 @@ config INTEL_MID_PTI
 	  an Intel Atom (non-netbook) mobile device containing a MIPI
 	  P1149.7 standard implementation.
 
+config STRATIX10_SERVICE
+	tristate "Stratix10 Service Layer"
+	depends on HAVE_ARM_SMCCC
+	default n
+	help
+	 Stratix10 service layer runs at privileged exception level, interfaces with
+	 the service providers (FPGA manager is one of them) and manages secure
+	 monitor call to communicate with secure monitor software at secure monitor
+	 exception level.
+
+	 Say Y here if you want Stratix10 service layer support.
+
 config SGI_IOC4
 	tristate "SGI IOC4 Base IO support"
 	depends on PCI
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 20be70c..99fed8b 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_AD525X_DPOT)	+= ad525x_dpot.o
 obj-$(CONFIG_AD525X_DPOT_I2C)	+= ad525x_dpot-i2c.o
 obj-$(CONFIG_AD525X_DPOT_SPI)	+= ad525x_dpot-spi.o
 obj-$(CONFIG_INTEL_MID_PTI)	+= pti.o
+obj-$(CONFIG_STRATIX10_SERVICE) += stratix10-svc.o
 obj-$(CONFIG_ATMEL_SSC)		+= atmel-ssc.o
 obj-$(CONFIG_ATMEL_TCLIB)	+= atmel_tclib.o
 obj-$(CONFIG_DUMMY_IRQ)		+= dummy-irq.o
diff --git a/drivers/misc/stratix10-smc.h b/drivers/misc/stratix10-smc.h
new file mode 100644
index 0000000..94615f4
--- /dev/null
+++ b/drivers/misc/stratix10-smc.h
@@ -0,0 +1,205 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2017-2018, Intel Corporation
+ */
+
+#ifndef __STRATIX10_SMC_H
+#define __STRATIX10_SMC_H
+
+#include <linux/arm-smccc.h>
+#include <linux/bitops.h>
+
+/**
+ * This file defines the Secure Monitor Call (SMC) message protocol used for
+ * service layer driver in normal world (EL1) to communicate with secure
+ * monitor software in Secure Monitor Exception Level 3 (EL3).
+ *
+ * This file is shared with secure firmware (FW) which is out of kernel tree.
+ *
+ * An ARM SMC instruction takes a function identifier and up to 6 64-bit
+ * register values as arguments, and can return up to 4 64-bit register
+ * value. The operation of the secure monitor is determined by the parameter
+ * values passed in through registers.
+ *
+ * EL1 and EL3 communicates pointer as physical address rather than the
+ * virtual address.
+ *
+ * Functions specified by ARM SMC Calling convention:
+ *
+ * FAST call executes atomic operations, returns when the requested operation
+ * has completed.
+ * STD call starts a operation which can be preempted by a non-secure
+ * interrupt. The call can return before the requested operation has
+ * completed.
+ *
+ * a0..a7 is used as register names in the descriptions below, on arm32
+ * that translates to r0..r7 and on arm64 to w0..w7.
+ */
+
+/**
+ * @func_num: function ID
+ */
+#define INTEL_SIP_SMC_STD_CALL_VAL(func_num) \
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_STD_CALL, ARM_SMCCC_SMC_64, \
+	ARM_SMCCC_OWNER_SIP, (func_num))
+
+#define INTEL_SIP_SMC_FAST_CALL_VAL(func_num) \
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_SMC_64, \
+	ARM_SMCCC_OWNER_SIP, (func_num))
+
+/**
+ * Return values in INTEL_SIP_SMC_* call
+ *
+ * INTEL_SIP_SMC_RETURN_UNKNOWN_FUNCTION:
+ * Secure monitor software doesn't recognize the request.
+ *
+ * INTEL_SIP_SMC_STATUS_OK:
+ * FPGA configuration completed successfully,
+ * In case of FPGA configuration write operation, it means secure monitor
+ * software can accept the next chunk of FPGA configuration data.
+ *
+ * INTEL_SIP_SMC_FPGA_CONFIG_STATUS_BUSY:
+ * In case of FPGA configuration write operation, it means secure monitor
+ * software is still processing previous data & can't accept the next chunk
+ * of data. Service driver needs to issue
+ * INTEL_SIP_SMC_FPGA_CONFIG_COMPLETED_WRITE call to query the
+ * completed block(s).
+ *
+ * INTEL_SIP_SMC_FPGA_CONFIG_STATUS_ERROR:
+ * There is error during the FPGA configuration process.
+ */
+#define INTEL_SIP_SMC_RETURN_UNKNOWN_FUNCTION		0xFFFFFFFF
+#define INTEL_SIP_SMC_STATUS_OK				0x0
+#define INTEL_SIP_SMC_FPGA_CONFIG_STATUS_BUSY		0x1
+#define INTEL_SIP_SMC_FPGA_CONFIG_STATUS_REJECTED       0x2
+#define INTEL_SIP_SMC_FPGA_CONFIG_STATUS_ERROR		0x4
+
+/**
+ * Request INTEL_SIP_SMC_FPGA_CONFIG_START
+ *
+ * Sync call used by service driver at EL1 to request the FPGA in EL3 to
+ * be prepare to receive a new configuration.
+ *
+ * Call register usage:
+ * a0: INTEL_SIP_SMC_FPGA_CONFIG_START.
+ * a1: flag for full or partial configuration. 0 for full and 1 for partial
+ * configuration.
+ * a2-7: not used.
+ *
+ * Return status:
+ * a0: INTEL_SIP_SMC_STATUS_OK, or INTEL_SIP_SMC_FPGA_CONFIG_STATUS_ERROR.
+ * a1-3: not used.
+ */
+#define INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_START 1
+#define INTEL_SIP_SMC_FPGA_CONFIG_START \
+	INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_START)
+
+/**
+ * Request INTEL_SIP_SMC_FPGA_CONFIG_WRITE
+ *
+ * Async call used by service driver at EL1 to provide FPGA configuration data
+ * to secure world.
+ *
+ * Call register usage:
+ * a0: INTEL_SIP_SMC_FPGA_CONFIG_WRITE.
+ * a1: 64bit physical address of the configuration data memory block
+ * a2: Size of configuration data block.
+ * a3-7: not used.
+ *
+ * Return status:
+ * a0: INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_FPGA_CONFIG_STATUS_BUSY or
+ * INTEL_SIP_SMC_FPGA_CONFIG_STATUS_ERROR.
+ * a1: 64bit physical address of 1st completed memory block if any completed
+ * block, otherwise zero value.
+ * a2: 64bit physical address of 2nd completed memory block if any completed
+ * block, otherwise zero value.
+ * a3: 64bit physical address of 3rd completed memory block if any completed
+ * block, otherwise zero value.
+ */
+#define INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_WRITE 2
+#define INTEL_SIP_SMC_FPGA_CONFIG_WRITE \
+	INTEL_SIP_SMC_STD_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_WRITE)
+
+/**
+ * Request INTEL_SIP_SMC_FPGA_CONFIG_COMPLETED_WRITE
+ *
+ * Sync call used by service driver at EL1 to track the completed write
+ * transactions. This request is called after INTEL_SIP_SMC_FPGA_CONFIG_WRITE
+ * call returns INTEL_SIP_SMC_FPGA_CONFIG_STATUS_BUSY.
+ *
+ * Call register usage:
+ * a0: INTEL_SIP_SMC_FPGA_CONFIG_COMPLETED_WRITE.
+ * a1-7: not used.
+ *
+ * Return status:
+ * a0: INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_FPGA_CONFIG_STATUS_BUSY or
+ * INTEL_SIP_SMC_FPGA_CONFIG_STATUS_ERROR.
+ * a1: 64bit physical address of 1st completed memory block.
+ * a2: 64bit physical address of 2nd completed memory block if
+ * any completed block, otherwise zero value.
+ * a3: 64bit physical address of 3rd completed memory block if
+ * any completed block, otherwise zero value.
+ */
+#define INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE 3
+#define INTEL_SIP_SMC_FPGA_CONFIG_COMPLETED_WRITE \
+INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
+
+/**
+ * Request INTEL_SIP_SMC_FPGA_CONFIG_ISDONE
+ *
+ * Sync call used by service driver at EL1 to inform secure world that all
+ * data are sent, to check whether or not the secure world had completed
+ * the FPGA configuration process.
+ *
+ * Call register usage:
+ * a0: INTEL_SIP_SMC_FPGA_CONFIG_ISDONE.
+ * a1-7: not used.
+ *
+ * Return status:
+ * a0: INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_FPGA_CONFIG_STATUS_BUSY or
+ * INTEL_SIP_SMC_FPGA_CONFIG_STATUS_ERROR.
+ * a1-3: not used.
+ */
+#define INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_ISDONE 4
+#define INTEL_SIP_SMC_FPGA_CONFIG_ISDONE \
+	INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_ISDONE)
+
+/**
+ * Request INTEL_SIP_SMC_FPGA_CONFIG_GET_MEM
+ *
+ * Sync call used by service driver at EL1 to query the physical address of
+ * memory block reserved by secure monitor software.
+ *
+ * Call register usage:
+ * a0:INTEL_SIP_SMC_FPGA_CONFIG_GET_MEM.
+ * a1-7: not used.
+ *
+ * Return status:
+ * a0: INTEL_SIP_SMC_STATUS_OK or INTEL_SIP_SMC_FPGA_CONFIG_STATUS_ERROR.
+ * a1: start of physical address of reserved memory block.
+ * a2: size of reserved memory block.
+ * a3: not used.
+ */
+#define INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_GET_MEM 5
+#define INTEL_SIP_SMC_FPGA_CONFIG_GET_MEM \
+	INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_GET_MEM)
+
+/**
+ * Request INTEL_SIP_SMC_FPGA_CONFIG_LOOPBACK
+ *
+ * For SMC loop-back mode only, used for internal integration, debugging
+ * or troubleshooting.
+ *
+ * Call register usage:
+ * a0: INTEL_SIP_SMC_FPGA_CONFIG_LOOPBACK.
+ * a1-7: not used.
+ *
+ * Return status:
+ * a0: INTEL_SIP_SMC_STATUS_OK or INTEL_SIP_SMC_FPGA_CONFIG_STATUS_ERROR.
+ * a1-3: not used.
+ */
+#define INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_LOOPBACK 6
+#define INTEL_SIP_SMC_FPGA_CONFIG_LOOPBACK \
+	INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_LOOPBACK)
+
+#endif
diff --git a/drivers/misc/stratix10-svc.c b/drivers/misc/stratix10-svc.c
new file mode 100644
index 0000000..5d07994
--- /dev/null
+++ b/drivers/misc/stratix10-svc.c
@@ -0,0 +1,984 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2017-2018, Intel Corporation
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/genalloc.h>
+#include <linux/stratix10-svc-client.h>
+#include <linux/io.h>
+#include <linux/kfifo.h>
+#include <linux/kthread.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#include "stratix10-smc.h"
+
+/**
+ * SVC_NUM_DATA_IN_FIFO - number of struct stratix10_svc_data in the FIFO
+ *
+ * SVC_NUM_CHANNEL - number of channel supported by service layer driver
+ *
+ * FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS - claim back the submitted buffer(s)
+ * from the secure world for FPGA manager to reuse, or to free the buffer(s)
+ * when all bit-stream data had be send.
+ *
+ * FPGA_CONFIG_STATUS_TIMEOUT_SEC - poll the FPGA configuration status,
+ * service layer will return error to FPGA manager when timeout occurs,
+ * timeout is set to 30 seconds (30 * 1000) at Intel Stratix10 SoC.
+ */
+#define SVC_NUM_DATA_IN_FIFO			32
+#define SVC_NUM_CHANNEL				1
+#define FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS	200
+#define FPGA_CONFIG_STATUS_TIMEOUT_SEC		30
+
+typedef void (svc_invoke_fn)(unsigned long, unsigned long, unsigned long,
+			     unsigned long, unsigned long, unsigned long,
+			     unsigned long, unsigned long,
+			     struct arm_smccc_res *);
+struct stratix10_svc_chan;
+
+/**
+ * struct stratix10_svc_sh_memory - service shared memory structure
+ * @sync_complete: state for a completion
+ * @addr: physical address of shared memory block
+ * @size: size of shared memory block
+ * @invoke_fn: function to issue secure monitor or hypervisor call
+ *
+ * This struct is used to save physical address and size of shared memory
+ * block. The shared memory blocked is allocated by secure monitor software
+ * at secure world.
+ *
+ * Service layer driver uses the physical address and size to create a memory
+ * pool, then allocates data buffer from that memory pool for service client.
+ */
+struct stratix10_svc_sh_memory {
+	struct completion sync_complete;
+	unsigned long addr;
+	unsigned long size;
+	svc_invoke_fn *invoke_fn;
+};
+
+/**
+ * struct stratix10_svc_data_mem - service memory structure
+ * @vaddr: virtual address
+ * @paddr: physical address
+ * @size: size of memory
+ * @node: link list head node
+ *
+ * This struct is used in a list that keeps track of buffers which have
+ * been allocated or freed from the memory pool. Service layer driver also
+ * uses this struct to transfer physical address to virtual address.
+ */
+struct stratix10_svc_data_mem {
+	void *vaddr;
+	phys_addr_t paddr;
+	size_t size;
+	struct list_head node;
+};
+
+/**
+ * struct stratix10_svc_data - service data structure
+ * @chan: service channel
+ * @paddr: playload physical address
+ * @size: playload size
+ * @command: service command requested by client
+ *
+ * This struct is used in service FIFO for inter-process communication.
+ */
+struct stratix10_svc_data {
+	struct stratix10_svc_chan *chan;
+	phys_addr_t paddr;
+	size_t size;
+	u32 command;
+};
+
+/**
+ * struct stratix10_svc_controller - service controller
+ * @dev: device
+ * @chans: array of service channels
+ * @num_chans: number of channels in 'chans' array
+ * @num_active_client: number of active service client
+ * @node: list management
+ * @genpool: memory pool pointing to the memory region
+ * @task: pointer to the thread task which handles SMC or HVC call
+ * @svc_fifo: a queue for storing service message data
+ * @complete_status: state for completion
+ * @svc_fifo_lock: protect access to service message data queue
+ * @invoke_fn: function to issue secure monitor call or hypervisor call
+ *
+ * This struct is used to create communication channels for service clients, to
+ * handle secure monitor or hypervisor call.
+ */
+struct stratix10_svc_controller {
+	struct device *dev;
+	struct stratix10_svc_chan *chans;
+	int num_chans;
+	int num_active_client;
+	struct list_head node;
+	struct gen_pool *genpool;
+	struct task_struct *task;
+	struct kfifo svc_fifo;
+	struct completion complete_status;
+	spinlock_t svc_fifo_lock;
+	svc_invoke_fn *invoke_fn;
+};
+
+/**
+ * struct stratix10_svc_chan - service communication channel
+ * @ctrl: pointer to service controller which is the provider of this channel
+ * @scl: pointer to service client which owns the channel
+ * @name: service client name associated with the channel
+ * @lock: protect access to the channel
+ *
+ * This struct is used by service client to communicate with service layer, each
+ * service client has its own channel created by service controller.
+ */
+struct stratix10_svc_chan {
+	struct stratix10_svc_controller *ctrl;
+	struct stratix10_svc_client *scl;
+	char *name;
+	spinlock_t lock;
+};
+
+static LIST_HEAD(svc_ctrl);
+static LIST_HEAD(svc_data_mem);
+
+/**
+ * svc_pa_to_va() - translate physical address to virtual address
+ * @addr: to be translated physical address
+ *
+ * Return: valid virtual address or NULL if the provided physical
+ * address doesn't exist.
+ */
+static void *svc_pa_to_va(unsigned long addr)
+{
+	struct stratix10_svc_data_mem *pmem;
+
+	pr_debug("claim back P-addr=0x%016x\n", (unsigned int)addr);
+	list_for_each_entry(pmem, &svc_data_mem, node) {
+		if (pmem->paddr == addr)
+			return pmem->vaddr;
+	}
+
+	/* physical address is not found */
+	return NULL;
+}
+
+/**
+ * svc_thread_cmd_data_claim() - claim back buffer from the secure world
+ * @ctrl: pointer to service layer controller
+ * @p_data: pointer to service data structure
+ * @cb_data: pointer to callback data structure to service client
+ *
+ * Claim back the submitted buffers from the secure world and pass buffer
+ * back to service client (FPGA manager, etc) for reuse.
+ */
+static void svc_thread_cmd_data_claim(struct stratix10_svc_controller *ctrl,
+				      struct stratix10_svc_data *p_data,
+				      struct stratix10_svc_cb_data *cb_data)
+{
+	struct arm_smccc_res res;
+	unsigned long timeout;
+
+	reinit_completion(&ctrl->complete_status);
+	timeout = msecs_to_jiffies(FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS);
+
+	pr_debug("%s: claim back the submitted buffer\n", __func__);
+	do {
+		ctrl->invoke_fn(INTEL_SIP_SMC_FPGA_CONFIG_COMPLETED_WRITE,
+				0, 0, 0, 0, 0, 0, 0, &res);
+
+		if (res.a0 == INTEL_SIP_SMC_STATUS_OK) {
+			if (!res.a1) {
+				complete(&ctrl->complete_status);
+				break;
+			}
+			cb_data->status = BIT(SVC_STATUS_RECONFIG_BUFFER_DONE);
+			cb_data->kaddr1 = svc_pa_to_va(res.a1);
+			cb_data->kaddr2 = (res.a2) ?
+					  svc_pa_to_va(res.a2) : NULL;
+			cb_data->kaddr3 = (res.a3) ?
+					  svc_pa_to_va(res.a3) : NULL;
+			p_data->chan->scl->receive_cb(p_data->chan->scl,
+						      cb_data);
+		} else {
+			pr_debug("%s: secure world busy, polling again\n",
+				 __func__);
+		}
+	} while (res.a0 == INTEL_SIP_SMC_STATUS_OK ||
+		 res.a0 == INTEL_SIP_SMC_FPGA_CONFIG_STATUS_BUSY ||
+		 wait_for_completion_timeout(&ctrl->complete_status, timeout));
+}
+
+/**
+ * svc_thread_cmd_config_status() - check configuration status
+ * @ctrl: pointer to service layer controller
+ * @p_data: pointer to service data structure
+ * @cb_data: pointer to callback data structure to service client
+ *
+ * Check whether the secure firmware at secure world has finished the FPGA
+ * configuration, and then inform FPGA manager the configuration status.
+ */
+static void svc_thread_cmd_config_status(struct stratix10_svc_controller *ctrl,
+					 struct stratix10_svc_data *p_data,
+					 struct stratix10_svc_cb_data *cb_data)
+{
+	struct arm_smccc_res res;
+	int count_in_sec;
+
+	cb_data->kaddr1 = NULL;
+	cb_data->kaddr2 = NULL;
+	cb_data->kaddr3 = NULL;
+	cb_data->status = BIT(SVC_STATUS_RECONFIG_ERROR);
+
+	pr_debug("%s: polling config status\n", __func__);
+
+	count_in_sec = FPGA_CONFIG_STATUS_TIMEOUT_SEC;
+	while (count_in_sec) {
+		ctrl->invoke_fn(INTEL_SIP_SMC_FPGA_CONFIG_ISDONE,
+				0, 0, 0, 0, 0, 0, 0, &res);
+		if ((res.a0 == INTEL_SIP_SMC_STATUS_OK) ||
+		    (res.a0 == INTEL_SIP_SMC_FPGA_CONFIG_STATUS_ERROR))
+			break;
+
+		/*
+		 * configuration is still in progress, wait one second then
+		 * poll again
+		 */
+		msleep(1000);
+		count_in_sec--;
+	};
+
+	if (res.a0 == INTEL_SIP_SMC_STATUS_OK && count_in_sec)
+		cb_data->status = BIT(SVC_STATUS_RECONFIG_COMPLETED);
+
+	p_data->chan->scl->receive_cb(p_data->chan->scl, cb_data);
+}
+
+/**
+ * svc_thread_recv_status_ok() - handle the successful status
+ * @p_data: pointer to service data structure
+ * @cb_data: pointer to callback data structure to service client
+ * @res: result from SMC or HVC call
+ *
+ * Send back the correspond status to the service client (FPGA manager etc).
+ */
+static void svc_thread_recv_status_ok(struct stratix10_svc_data *p_data,
+				      struct stratix10_svc_cb_data *cb_data,
+				      struct arm_smccc_res res)
+{
+	cb_data->kaddr1 = NULL;
+	cb_data->kaddr2 = NULL;
+	cb_data->kaddr3 = NULL;
+
+	switch (p_data->command) {
+	case COMMAND_RECONFIG:
+		cb_data->status = BIT(SVC_STATUS_RECONFIG_REQUEST_OK);
+		break;
+	case COMMAND_RECONFIG_DATA_SUBMIT:
+		cb_data->status = BIT(SVC_STATUS_RECONFIG_BUFFER_SUBMITTED);
+		break;
+	case COMMAND_NOOP:
+		cb_data->status = BIT(SVC_STATUS_RECONFIG_BUFFER_SUBMITTED);
+		cb_data->kaddr1 = svc_pa_to_va(res.a1);
+		break;
+	case COMMAND_RECONFIG_STATUS:
+		cb_data->status = BIT(SVC_STATUS_RECONFIG_COMPLETED);
+		break;
+	default:
+		break;
+	}
+
+	pr_debug("%s: call receive_cb\n", __func__);
+	p_data->chan->scl->receive_cb(p_data->chan->scl, cb_data);
+}
+
+/**
+ * svc_normal_to_secure_thread() - the function to run in the kthread
+ * @data: data pointer for kthread function
+ *
+ * Service layer driver creates stratix10_svc_smc_hvc_call kthread on CPU
+ * node 0, its function stratix10_svc_secure_call_thread is used to handle
+ * SMC or HVC calls between kernel driver and secure monitor software.
+ *
+ * Return: 0 for success or -ENOMEM on error.
+ */
+static int svc_normal_to_secure_thread(void *data)
+{
+	struct stratix10_svc_controller
+			*ctrl = (struct stratix10_svc_controller *)data;
+	struct stratix10_svc_data *pdata;
+	struct stratix10_svc_cb_data *cbdata;
+	struct arm_smccc_res res;
+	unsigned long a0, a1, a2;
+	int ret_fifo = 0;
+
+	pdata =  kmalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	cbdata = kmalloc(sizeof(*cbdata), GFP_KERNEL);
+	if (!cbdata)
+		return -ENOMEM;
+
+	/* default set, to remove build warning */
+	a0 = INTEL_SIP_SMC_FPGA_CONFIG_LOOPBACK;
+	a1 = 0;
+	a2 = 0;
+
+	pr_debug("smc_hvc_shm_thread is running\n");
+
+	while (!kthread_should_stop()) {
+		ret_fifo = kfifo_out_spinlocked(&ctrl->svc_fifo,
+						pdata, sizeof(*pdata),
+						&ctrl->svc_fifo_lock);
+
+		if (!ret_fifo)
+			continue;
+
+		pr_debug("get from FIFO pa=0x%016x, command=%u, size=%u\n",
+			 (unsigned int)pdata->paddr, pdata->command,
+			 (unsigned int)pdata->size);
+
+		switch (pdata->command) {
+		case COMMAND_RECONFIG_DATA_CLAIM:
+			svc_thread_cmd_data_claim(ctrl, pdata, cbdata);
+			continue;
+		case COMMAND_RECONFIG:
+			a0 = INTEL_SIP_SMC_FPGA_CONFIG_START;
+			a1 = 0;
+			a2 = 0;
+			break;
+		case COMMAND_RECONFIG_DATA_SUBMIT:
+			a0 = INTEL_SIP_SMC_FPGA_CONFIG_WRITE;
+			a1 = (unsigned long)pdata->paddr;
+			a2 = (unsigned long)pdata->size;
+			break;
+		case COMMAND_RECONFIG_STATUS:
+			a0 = INTEL_SIP_SMC_FPGA_CONFIG_ISDONE;
+			a1 = 0;
+			a2 = 0;
+			break;
+		default:
+			/* it shouldn't happen */
+			break;
+		}
+		pr_debug("%s: before SMC call -- a0=0x%016x a1=0x%016x",
+			 __func__, (unsigned int)a0, (unsigned int)a1);
+		pr_debug(" a2=0x%016x\n", (unsigned int)a2);
+
+		ctrl->invoke_fn(a0, a1, a2, 0, 0, 0, 0, 0, &res);
+
+		pr_debug("%s: after SMC call -- res.a0=0x%016x",
+			 __func__, (unsigned int)res.a0);
+		pr_debug(" res.a1=0x%016x, res.a2=0x%016x",
+			 (unsigned int)res.a1, (unsigned int)res.a2);
+		pr_debug(" res.a3=0x%016x\n", (unsigned int)res.a3);
+
+		switch (res.a0) {
+		case INTEL_SIP_SMC_STATUS_OK:
+			svc_thread_recv_status_ok(pdata, cbdata, res);
+			break;
+		case INTEL_SIP_SMC_FPGA_CONFIG_STATUS_BUSY:
+			switch (pdata->command) {
+			case COMMAND_RECONFIG_DATA_SUBMIT:
+				svc_thread_cmd_data_claim(ctrl,
+							  pdata, cbdata);
+				break;
+			case COMMAND_RECONFIG_STATUS:
+				svc_thread_cmd_config_status(ctrl,
+							     pdata, cbdata);
+				break;
+			default:
+				break;
+			}
+			break;
+		case INTEL_SIP_SMC_FPGA_CONFIG_STATUS_REJECTED:
+			pr_debug("%s: STATUS_REJECTED\n", __func__);
+			break;
+		case INTEL_SIP_SMC_FPGA_CONFIG_STATUS_ERROR:
+			pr_err("%s: STATUS_ERROR\n", __func__);
+			cbdata->status = BIT(SVC_STATUS_RECONFIG_ERROR);
+			cbdata->kaddr1 = NULL;
+			cbdata->kaddr2 = NULL;
+			cbdata->kaddr3 = NULL;
+			pdata->chan->scl->receive_cb(pdata->chan->scl, cbdata);
+			break;
+		default:
+			break;
+		}
+	};
+
+	kfree(cbdata);
+	kfree(pdata);
+
+	return 0;
+}
+
+/**
+ * svc_normal_to_secure_shm_thread() - the function to run in the kthread
+ * @data: data pointer for kthread function
+ *
+ * Service layer driver creates stratix10_svc_smc_hvc_shm kthread on CPU
+ * node 0, its function stratix10_svc_secure_shm_thread is used to query the
+ * physical address of memory block reserved by secure monitor software at
+ * secure world.
+ *
+ * svc_normal_to_secure_shm_thread() calls do_exit() directly since it is a
+ * standlone thread for which no one will call kthread_stop() or return when
+ * 'kthread_should_stop()' is true.
+ */
+static int svc_normal_to_secure_shm_thread(void *data)
+{
+	struct stratix10_svc_sh_memory
+			*sh_mem = (struct stratix10_svc_sh_memory *)data;
+	struct arm_smccc_res res;
+
+	/* SMC or HVC call to get shared memory info from secure world */
+	sh_mem->invoke_fn(INTEL_SIP_SMC_FPGA_CONFIG_GET_MEM,
+			  0, 0, 0, 0, 0, 0, 0, &res);
+	if (res.a0 == INTEL_SIP_SMC_STATUS_OK) {
+		sh_mem->addr = res.a1;
+		sh_mem->size = res.a2;
+	} else {
+		pr_err("%s: after SMC call -- res.a0=0x%016x",  __func__,
+		       (unsigned int)res.a0);
+		sh_mem->addr = 0;
+		sh_mem->size = 0;
+	}
+
+	complete(&sh_mem->sync_complete);
+	do_exit(0);
+}
+
+/**
+ * svc_get_sh_memory() - get memory block reserved by secure monitor SW
+ * @pdev: pointer to service layer device
+ * @sh_memory: pointer to service shared memory structure
+ *
+ * Return: zero for successfully getting the physical address of memory block
+ * reserved by secure monitor software, or negative value on error.
+ */
+static int svc_get_sh_memory(struct platform_device *pdev,
+				    struct stratix10_svc_sh_memory *sh_memory)
+{
+	struct device *dev = &pdev->dev;
+	struct task_struct *sh_memory_task;
+
+	init_completion(&sh_memory->sync_complete);
+
+	/* smc or hvc call happens on cpu 0 bound kthread */
+	sh_memory_task = kthread_create_on_cpu(svc_normal_to_secure_shm_thread,
+					       (void *)sh_memory,
+						0, "svc_smc_hvc_shm_thread");
+	if (IS_ERR(sh_memory_task)) {
+		dev_err(dev, "fail to create stratix10_svc_smc_shm_thread\n");
+		return -EINVAL;
+	}
+
+	wake_up_process(sh_memory_task);
+
+	if (!wait_for_completion_timeout(&sh_memory->sync_complete, 10 * HZ)) {
+		dev_err(dev,
+			"timeout to get sh-memory paras from secure world\n");
+		return -ETIMEDOUT;
+	}
+
+	if (!sh_memory->addr || !sh_memory->size) {
+		dev_err(dev,
+			"fails to get shared memory info from secure world\n");
+		return -ENOMEM;
+	}
+
+	dev_dbg(dev, "SM software provides paddr: 0x%016x, size: 0x%08x\n",
+		(unsigned int)sh_memory->addr,
+		(unsigned int)sh_memory->size);
+
+	return 0;
+}
+
+/**
+ * svc_create_memory_pool() - create a memory pool from reserved memory block
+ * @pdev: pointer to service layer device
+ * @sh_memory: pointer to service shared memory structure
+ *
+ * Return: pool allocated from reserved memory block or ERR_PTR() on error.
+ */
+static struct gen_pool *
+svc_create_memory_pool(struct platform_device *pdev,
+		       struct stratix10_svc_sh_memory *sh_memory)
+{
+	struct device *dev = &pdev->dev;
+	struct gen_pool *genpool;
+	unsigned long vaddr;
+	phys_addr_t paddr;
+	size_t size;
+	phys_addr_t begin;
+	phys_addr_t end;
+	void *va;
+	size_t page_mask = PAGE_SIZE - 1;
+	int min_alloc_order = 3;
+	int ret;
+
+	begin = roundup(sh_memory->addr, PAGE_SIZE);
+	end = rounddown(sh_memory->addr + sh_memory->size, PAGE_SIZE);
+	paddr = begin;
+	size = end - begin;
+	va = memremap(paddr, size, MEMREMAP_WC);
+	if (!va) {
+		dev_err(dev, "fail to remap shared memory\n");
+		return ERR_PTR(-EINVAL);
+	}
+	vaddr = (unsigned long)va;
+	dev_dbg(dev,
+		"reserved memory vaddr: %p, paddr: 0x%16x size: 0x%8x\n",
+		va, (unsigned int)paddr, (unsigned int)size);
+	if ((vaddr & page_mask) || (paddr & page_mask) ||
+	    (size & page_mask)) {
+		dev_err(dev, "page is not aligned\n");
+		return ERR_PTR(-EINVAL);
+	}
+	genpool = gen_pool_create(min_alloc_order, -1);
+	if (!genpool) {
+		dev_err(dev, "fail to create genpool\n");
+		return ERR_PTR(-ENOMEM);
+	}
+	gen_pool_set_algo(genpool, gen_pool_best_fit, NULL);
+	ret = gen_pool_add_virt(genpool, vaddr, paddr, size, -1);
+	if (ret) {
+		dev_err(dev, "fail to add memory chunk to the pool\n");
+		gen_pool_destroy(genpool);
+		return ERR_PTR(ret);
+	}
+
+	return genpool;
+}
+
+/**
+ * svc_smccc_smc() - secure monitor call between normal and secure world
+ * @a0: argument passed in registers 0
+ * @a1: argument passed in registers 1
+ * @a2: argument passed in registers 2
+ * @a3: argument passed in registers 3
+ * @a4: argument passed in registers 4
+ * @a5: argument passed in registers 5
+ * @a6: argument passed in registers 6
+ * @a7: argument passed in registers 7
+ * @res: result values from register 0 to 3
+ */
+static void svc_smccc_smc(unsigned long a0, unsigned long a1,
+			  unsigned long a2, unsigned long a3,
+			  unsigned long a4, unsigned long a5,
+			  unsigned long a6, unsigned long a7,
+			  struct arm_smccc_res *res)
+{
+	arm_smccc_smc(a0, a1, a2, a3, a4, a5, a6, a7, res);
+}
+
+/**
+ * svc_smccc_hvc() - hypervisor call between normal and secure world
+ * @a0: argument passed in registers 0
+ * @a1: argument passed in registers 1
+ * @a2: argument passed in registers 2
+ * @a3: argument passed in registers 3
+ * @a4: argument passed in registers 4
+ * @a5: argument passed in registers 5
+ * @a6: argument passed in registers 6
+ * @a7: argument passed in registers 7
+ * @res: result values from register 0 to 3
+ */
+static void svc_smccc_hvc(unsigned long a0, unsigned long a1,
+			  unsigned long a2, unsigned long a3,
+			  unsigned long a4, unsigned long a5,
+			  unsigned long a6, unsigned long a7,
+			  struct arm_smccc_res *res)
+{
+	arm_smccc_hvc(a0, a1, a2, a3, a4, a5, a6, a7, res);
+}
+
+/**
+ * get_invoke_func() - invoke SMC or HVC call
+ * @dev: pointer to device
+ *
+ * Return: function pointer to svc_smccc_smc or svc_smccc_hvc.
+ */
+static svc_invoke_fn *get_invoke_func(struct device *dev)
+{
+	const char *method;
+
+	if (of_property_read_string(dev->of_node, "method", &method)) {
+		dev_warn(dev, "missing \"method\" property\n");
+		return ERR_PTR(-ENXIO);
+	}
+
+	if (!strcmp(method, "smc"))
+		return svc_smccc_smc;
+	if (!strcmp(method, "hvc"))
+		return svc_smccc_hvc;
+
+	dev_warn(dev, "invalid \"method\" property: %s\n", method);
+
+	return ERR_PTR(-EINVAL);
+}
+
+/**
+ * stratix10_svc_request_channel_byname() - request a service channel
+ * @client: pointer to service client
+ * @name: service client name
+ *
+ * This function is used by service client to request a service channel.
+ *
+ * Return: a pointer to channel assigned to the client on success,
+ * or ERR_PTR() on error.
+ */
+struct stratix10_svc_chan *stratix10_svc_request_channel_byname(
+	struct stratix10_svc_client *client, const char *name)
+{
+	struct device *dev = client->dev;
+	struct stratix10_svc_controller *controller;
+	struct stratix10_svc_chan *chan;
+	unsigned long flag;
+	int i;
+
+	chan = ERR_PTR(-EPROBE_DEFER);
+	if (list_empty(&svc_ctrl))
+		return ERR_PTR(-ENODEV);
+
+	controller = list_first_entry(&svc_ctrl,
+				      struct stratix10_svc_controller, node);
+	for (i = 0; i < SVC_NUM_CHANNEL; i++) {
+		if (!strcmp(controller->chans[i].name, name)) {
+			chan = &controller->chans[i];
+			break;
+		}
+	}
+
+	if (chan->scl || !try_module_get(controller->dev->driver->owner)) {
+		dev_dbg(dev, "%s: svc not free\n", __func__);
+		return ERR_PTR(-EBUSY);
+	}
+
+	spin_lock_irqsave(&chan->lock, flag);
+	chan->scl = client;
+	chan->ctrl->num_active_client++;
+	spin_unlock_irqrestore(&chan->lock, flag);
+
+	return chan;
+}
+EXPORT_SYMBOL_GPL(stratix10_svc_request_channel_byname);
+
+/**
+ * stratix10_svc_free_channel() - free service channel
+ * @chan: service channel to be freed
+ *
+ * This function is used by service client to free a service channel.
+ */
+void stratix10_svc_free_channel(struct stratix10_svc_chan *chan)
+{
+	unsigned long flag;
+
+	spin_lock_irqsave(&chan->lock, flag);
+	chan->scl = NULL;
+	chan->ctrl->num_active_client--;
+	module_put(chan->ctrl->dev->driver->owner);
+	spin_unlock_irqrestore(&chan->lock, flag);
+}
+EXPORT_SYMBOL_GPL(stratix10_svc_free_channel);
+
+/**
+ * stratix10_svc_send() - send a message data to the remote
+ * @chan: service channel assigned to the client
+ * @msg: message data to be sent, in the format of
+ * "struct stratix10_svc_client_msg"
+ *
+ * This function is used by service client to add a message to the service
+ * layer driver's queue for being sent to the secure world.
+ *
+ * Return: 0 for success, -ENOMEM or -ENOBUFS on error.
+ */
+int stratix10_svc_send(struct stratix10_svc_chan *chan, void *msg)
+{
+	struct stratix10_svc_client_msg
+		*p_msg = (struct stratix10_svc_client_msg *)msg;
+	struct stratix10_svc_data_mem *p_mem;
+	struct stratix10_svc_data *p_data;
+	int ret = 0;
+
+	p_data = kmalloc(sizeof(*p_data), GFP_KERNEL);
+	if (!p_data)
+		return -ENOMEM;
+
+	/* first client will create kernel thread */
+	if (!chan->ctrl->task) {
+		chan->ctrl->task =
+			kthread_create_on_cpu(svc_normal_to_secure_thread,
+					      (void *)chan->ctrl, 0,
+					      "svc_smc_hvc_thread");
+			if (IS_ERR(chan->ctrl->task)) {
+				dev_err(chan->ctrl->dev,
+					"fails to create svc_smc_hvc_thread\n");
+				return -EINVAL;
+			}
+		wake_up_process(chan->ctrl->task);
+	}
+
+	pr_debug("%s: sent P-va=%p, P-com=%x, P-size=%u\n", __func__,
+		 p_msg->payload, p_msg->command,
+		 (unsigned int)p_msg->payload_length);
+
+	p_data->paddr = 0;
+	list_for_each_entry(p_mem, &svc_data_mem, node) {
+		if (p_mem->vaddr == p_msg->payload) {
+			p_data->paddr = p_mem->paddr;
+			break;
+		}
+	}
+
+	p_data->command = p_msg->command;
+	p_data->size = p_msg->payload_length;
+	p_data->chan = chan;
+	pr_debug("%s: put to FIFO pa=0x%016x, cmd=%x, size=%u\n", __func__,
+	       (unsigned int)p_data->paddr, p_data->command,
+	       (unsigned int)p_data->size);
+	ret = kfifo_in_spinlocked(&chan->ctrl->svc_fifo, p_data,
+				  sizeof(*p_data),
+				  &chan->ctrl->svc_fifo_lock);
+
+	kfree(p_data);
+
+	if (!ret)
+		return -ENOBUFS;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(stratix10_svc_send);
+
+/**
+ * stratix10_svc_done() - complete service request transactions
+ * @chan: service channel assigned to the client
+ *
+ * This function should be called when client has finished its request
+ * or there is an error in the request process. It allows the service layer
+ * to stop the running thread to have maximize savings in kernel resources.
+ */
+void stratix10_svc_done(struct stratix10_svc_chan *chan)
+{
+	/* stop thread when thread is running AND only one active client */
+	if (chan->ctrl->task && chan->ctrl->num_active_client <= 1) {
+		pr_debug("svc_smc_hvc_shm_thread is stopped\n");
+		kthread_stop(chan->ctrl->task);
+		chan->ctrl->task = NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(stratix10_svc_done);
+
+/**
+ * stratix10_svc_allocate_memory() - allocate memory
+ * @chan: service channel assigned to the client
+ * @size: memory size requested by a specific service client
+ *
+ * Service layer allocates the requested number of bytes buffer from the
+ * memory pool, service client uses this function to get allocated buffers.
+ *
+ * Return: address of allocated memory on success, or ERR_PTR() on error.
+ */
+void *stratix10_svc_allocate_memory(struct stratix10_svc_chan *chan,
+				    size_t size)
+{
+	struct stratix10_svc_data_mem *pmem;
+	unsigned long va;
+	phys_addr_t pa;
+	struct gen_pool *genpool = chan->ctrl->genpool;
+	size_t s = roundup(size, 1 << genpool->min_alloc_order);
+
+	pmem = devm_kzalloc(chan->ctrl->dev, sizeof(*pmem), GFP_KERNEL);
+	if (!pmem)
+		return ERR_PTR(-ENOMEM);
+
+	va = gen_pool_alloc(genpool, s);
+	if (!va)
+		return ERR_PTR(-ENOMEM);
+
+	memset((void *)va, 0, s);
+	pa = gen_pool_virt_to_phys(genpool, va);
+
+	pmem->vaddr = (void *)va;
+	pmem->paddr = pa;
+	pmem->size = s;
+	list_add_tail(&pmem->node, &svc_data_mem);
+	pr_debug("%s: va=%p, pa=0x%016x\n", __func__,
+		 pmem->vaddr, (unsigned int)pmem->paddr);
+
+	return (void *)va;
+}
+EXPORT_SYMBOL_GPL(stratix10_svc_allocate_memory);
+
+/**
+ * stratix10_svc_free_memory() - free allocated memory
+ * @chan: service channel assigned to the client
+ * @kaddr: memory to be freed
+ *
+ * This function is used by service client to free allocated buffers.
+ */
+void stratix10_svc_free_memory(struct stratix10_svc_chan *chan, void *kaddr)
+{
+	struct stratix10_svc_data_mem *pmem;
+	size_t size = 0;
+
+	list_for_each_entry(pmem, &svc_data_mem, node)
+		if (pmem->vaddr == kaddr) {
+			size = pmem->size;
+			break;
+		}
+
+	gen_pool_free(chan->ctrl->genpool, (unsigned long)kaddr, size);
+	pmem->vaddr = NULL;
+	list_del(&pmem->node);
+}
+EXPORT_SYMBOL_GPL(stratix10_svc_free_memory);
+
+static const struct of_device_id stratix10_svc_drv_match[] = {
+	{.compatible = "intel,stratix10-svc"},
+	{},
+};
+
+static int stratix10_svc_drv_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct stratix10_svc_controller *controller;
+	struct stratix10_svc_chan *chans;
+	struct gen_pool *genpool;
+	struct stratix10_svc_sh_memory *sh_memory;
+	svc_invoke_fn *invoke_fn;
+	size_t fifo_size;
+	int ret;
+
+	/* get SMC or HVC function */
+	invoke_fn = get_invoke_func(dev);
+	if (IS_ERR(invoke_fn))
+		return -EINVAL;
+
+	sh_memory = devm_kzalloc(dev, sizeof(*sh_memory), GFP_KERNEL);
+	if (!sh_memory)
+		return -ENOMEM;
+
+	sh_memory->invoke_fn = invoke_fn;
+	ret = svc_get_sh_memory(pdev, sh_memory);
+	if (ret)
+		return ret;
+
+	genpool = svc_create_memory_pool(pdev, sh_memory);
+	if (!genpool)
+		return -ENOMEM;
+
+	/* allocate service controller and supporting channel */
+	controller = devm_kzalloc(dev, sizeof(*controller), GFP_KERNEL);
+	if (!controller)
+		return -ENOMEM;
+
+	chans = devm_kmalloc_array(dev, SVC_NUM_CHANNEL,
+				   sizeof(*chans), GFP_KERNEL | __GFP_ZERO);
+	if (!chans)
+		return -ENOMEM;
+
+	controller->dev = dev;
+	controller->num_chans = SVC_NUM_CHANNEL;
+	controller->num_active_client = 0;
+	controller->chans = chans;
+	controller->genpool = genpool;
+	controller->task = NULL;
+	controller->invoke_fn = invoke_fn;
+	init_completion(&controller->complete_status);
+
+	fifo_size = sizeof(struct stratix10_svc_data) * SVC_NUM_DATA_IN_FIFO;
+	ret = kfifo_alloc(&controller->svc_fifo, fifo_size, GFP_KERNEL);
+	if (ret) {
+		dev_err(dev, "fails to allocate FIFO\n");
+		return ret;
+	}
+	spin_lock_init(&controller->svc_fifo_lock);
+
+	chans[0].scl = NULL;
+	chans[0].ctrl = controller;
+	chans[0].name = SVC_CLIENT_FPGA;
+	spin_lock_init(&chans[0].lock);
+
+	list_add_tail(&controller->node, &svc_ctrl);
+	platform_set_drvdata(pdev, controller);
+
+	pr_info("Intel Service Layer Driver Initialized\n");
+
+	return ret;
+}
+
+static int stratix10_svc_drv_remove(struct platform_device *pdev)
+{
+	struct stratix10_svc_controller *ctrl = platform_get_drvdata(pdev);
+
+	kfifo_free(&ctrl->svc_fifo);
+	if (ctrl->task) {
+		kthread_stop(ctrl->task);
+		ctrl->task = NULL;
+	}
+	if (ctrl->genpool)
+		gen_pool_destroy(ctrl->genpool);
+	list_del(&ctrl->node);
+
+	return 0;
+}
+
+static struct platform_driver stratix10_svc_driver = {
+	.probe = stratix10_svc_drv_probe,
+	.remove = stratix10_svc_drv_remove,
+	.driver = {
+		.name = "stratix10-svc",
+		.of_match_table = stratix10_svc_drv_match,
+	},
+};
+
+static int __init stratix10_svc_init(void)
+{
+	struct device_node *fw_np;
+	struct device_node *np;
+	int ret;
+
+	fw_np = of_find_node_by_name(NULL, "firmware");
+	if (!fw_np)
+		return -ENODEV;
+
+	np = of_find_matching_node(fw_np, stratix10_svc_drv_match);
+	if (!np) {
+		of_node_put(fw_np);
+		return -ENODEV;
+	}
+
+	of_node_put(np);
+	ret = of_platform_populate(fw_np, stratix10_svc_drv_match, NULL, NULL);
+	of_node_put(fw_np);
+	if (ret)
+		return ret;
+
+	return platform_driver_register(&stratix10_svc_driver);
+}
+
+static void __exit stratix10_svc_exit(void)
+{
+	return platform_driver_unregister(&stratix10_svc_driver);
+}
+
+subsys_initcall(stratix10_svc_init);
+module_exit(stratix10_svc_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Intel Stratix10 Service Layer Driver");
+MODULE_AUTHOR("Richard Gong <richard.gong@intel.com>");
+MODULE_ALIAS("platform:stratix10-svc");
diff --git a/include/linux/stratix10-svc-client.h b/include/linux/stratix10-svc-client.h
new file mode 100644
index 0000000..0ac6b2c
--- /dev/null
+++ b/include/linux/stratix10-svc-client.h
@@ -0,0 +1,199 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2017-2018, Intel Corporation
+ */
+
+#ifndef __STRATIX10_SVC_CLIENT_H
+#define __STRATIX10_SVC_CLIENT_H
+
+/**
+ * Service layer driver supports client names
+ *
+ * fpga: for FPGA configuration
+ */
+#define SVC_CLIENT_FPGA			"fpga"
+
+/**
+ * Status of the sent command, in bit number
+ *
+ * SVC_COMMAND_STATUS_RECONFIG_REQUEST_OK:
+ * Secure firmware accepts the request of FPGA reconfiguration.
+ *
+ * SVC_STATUS_RECONFIG_BUFFER_SUBMITTED:
+ * Service client successfully submits FPGA configuration
+ * data buffer to secure firmware.
+ *
+ * SVC_COMMAND_STATUS_RECONFIG_BUFFER_DONE:
+ * Secure firmware completes data process, ready to accept the
+ * next WRITE transaction.
+ *
+ * SVC_COMMAND_STATUS_RECONFIG_COMPLETED:
+ * Secure firmware completes FPGA configuration successfully, FPGA should
+ * be in user mode.
+ *
+ * SVC_COMMAND_STATUS_RECONFIG_BUSY:
+ * FPGA configuration is still in process.
+ *
+ * SVC_COMMAND_STATUS_RECONFIG_ERROR:
+ * Error encountered during FPGA configuration.
+ */
+#define SVC_STATUS_RECONFIG_REQUEST_OK		0
+#define SVC_STATUS_RECONFIG_BUFFER_SUBMITTED	1
+#define SVC_STATUS_RECONFIG_BUFFER_DONE		2
+#define SVC_STATUS_RECONFIG_COMPLETED		3
+#define SVC_STATUS_RECONFIG_BUSY		4
+#define SVC_STATUS_RECONFIG_ERROR		5
+
+/**
+ * Flag bit for COMMAND_RECONFIG
+ *
+ * COMMAND_RECONFIG_FLAG_PARTIAL:
+ * Set to FPGA configuration type (full or partial), the default
+ * is full reconfig.
+ */
+#define COMMAND_RECONFIG_FLAG_PARTIAL   0
+
+/**
+ * Timeout settings for service clients:
+ * timeout value used in Stratix10 FPGA manager driver.
+ */
+#define SVC_RECONFIG_REQUEST_TIMEOUT_MS         100
+#define SVC_RECONFIG_BUFFER_TIMEOUT_MS          240
+
+struct stratix10_svc_chan;
+
+/**
+ * enum stratix10_svc_command_code - supported service commands
+ *
+ * @COMMAND_NOOP: do 'dummy' request for integration/debug/trouble-shooting
+ *
+ * @COMMAND_RECONFIG: ask for FPGA configuration preparation, return status
+ * is SVC_STATUS_RECONFIG_REQUEST_OK
+ *
+ * @COMMAND_RECONFIG_DATA_SUBMIT: submit buffer(s) of bit-stream data for the
+ * FPGA configuration, return status is SVC_STATUS_RECONFIG_BUFFER_SUBMITTED,
+ * or SVC_STATUS_RECONFIG_ERROR
+ *
+ * @COMMAND_RECONFIG_DATA_CLAIM: check the status of the configuration, return
+ * status is SVC_STATUS_RECONFIG_COMPLETED, or SVC_STATUS_RECONFIG_BUSY, or
+ * SVC_STATUS_RECONFIG_ERROR
+ *
+ * @COMMAND_RECONFIG_STATUS: check the status of the configuration, return
+ * status is SVC_STATUS_RECONFIG_COMPLETED, or  SVC_STATUS_RECONFIG_BUSY, or
+ * SVC_STATUS_RECONFIG_ERROR
+ */
+enum stratix10_svc_command_code {
+	COMMAND_NOOP = 0,
+	COMMAND_RECONFIG,
+	COMMAND_RECONFIG_DATA_SUBMIT,
+	COMMAND_RECONFIG_DATA_CLAIM,
+	COMMAND_RECONFIG_STATUS
+};
+
+/**
+ * struct stratix10_svc_client_msg - message sent by client to service
+ * @command: service command
+ * @payload: starting address of data need be processed
+ * @payload_length: data size in bytes
+ */
+struct stratix10_svc_client_msg {
+	void *payload;
+	size_t payload_length;
+	enum stratix10_svc_command_code command;
+};
+
+/**
+ * struct stratix10_svc_command_reconfig_payload - reconfig payload
+ * @flags: flag bit for the type of FPGA configuration
+ */
+struct stratix10_svc_command_reconfig_payload {
+	u32 flags;
+};
+
+/**
+ * struct stratix10_svc_cb_data - callback data structure from service layer
+ * @status: the status of sent command
+ * @kaddr1: address of 1st completed data block
+ * @kaddr2: address of 2nd completed data block
+ * @kaddr3: address of 3rd completed data block
+ */
+struct stratix10_svc_cb_data {
+	u32 status;
+	void *kaddr1;
+	void *kaddr2;
+	void *kaddr3;
+};
+
+/**
+ * struct stratix10_svc_client - service client structure
+ * @dev: the client device
+ * @receive_cb: callback to provide service client the received data
+ * @priv: client private data
+ */
+struct stratix10_svc_client {
+	struct device *dev;
+	void (*receive_cb)(struct stratix10_svc_client *client,
+			   struct stratix10_svc_cb_data *cb_data);
+	void *priv;
+};
+
+/**
+ * stratix10_svc_request_channel_byname() - request service channel
+ * @client: identity of the client requesting the channel
+ * @name: supporting client name defined above
+ *
+ * Return: a pointer to channel assigned to the client on success,
+ * or ERR_PTR() on error.
+ */
+struct stratix10_svc_chan
+*stratix10_svc_request_channel_byname(struct stratix10_svc_client *client,
+	const char *name);
+
+/**
+ * stratix10_svc_free_channel() - free service channel.
+ * @chan: service channel to be freed
+ */
+void stratix10_svc_free_channel(struct stratix10_svc_chan *chan);
+
+/**
+ * stratix10_svc_allocate_memory() - allocate the momory
+ * @chan: service channel assigned to the client
+ * @size: number of bytes client requests
+ *
+ * Service layer allocates the requested number of bytes from the memory
+ * pool for the client.
+ *
+ * Return: the starting address of allocated memory on success, or
+ * ERR_PTR() on error.
+ */
+void *stratix10_svc_allocate_memory(struct stratix10_svc_chan *chan,
+				    size_t size);
+
+/**
+ * stratix10_svc_free_memory() - free allocated memory
+ * @chan: service channel assigned to the client
+ * @kaddr: starting address of memory to be free back to pool
+ */
+void stratix10_svc_free_memory(struct stratix10_svc_chan *chan, void *kaddr);
+
+/**
+ * stratix10_svc_send() - send a message to the remote
+ * @chan: service channel assigned to the client
+ * @msg: message data to be sent, in the format of
+ * struct stratix10_svc_client_msg
+ *
+ * Return: 0 for success, -ENOMEM or -ENOBUFS on error.
+ */
+int stratix10_svc_send(struct stratix10_svc_chan *chan, void *msg);
+
+/**
+ * intel_svc_done() - complete service request
+ * @chan: service channel assigned to the client
+ *
+ * This function is used by service client to inform service layer that
+ * client's service requests are completed, or there is an error in the
+ * request process.
+ */
+void stratix10_svc_done(struct stratix10_svc_chan *chan);
+#endif
+
-- 
2.7.4

^ permalink raw reply related

* [PATCHv5 4/8] dt-bindings: fpga: add Stratix10 SoC FPGA manager binding
From: richard.gong at linux.intel.com @ 2018-05-24 16:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527179600-26441-1-git-send-email-richard.gong@linux.intel.com>

From: Alan Tull <atull@kernel.org>

Add a Device Tree binding for the Intel Stratix10 SoC FPGA manager.

Signed-off-by: Alan Tull <atull@kernel.org>
Signed-off-by: Richard Gong <richard.gong@intel.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
v2: this patch is added in patch set version 2
v3: change to put fpga_mgr node under firmware/svc node
v4: s/fpga-mgr at 0/fpga-mgr/ to remove unit_address
    add Richard's signed-off-by
v5: add Reviewed-by Rob Herring
---
 .../bindings/fpga/intel-stratix10-soc-fpga-mgr.txt      | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/intel-stratix10-soc-fpga-mgr.txt

diff --git a/Documentation/devicetree/bindings/fpga/intel-stratix10-soc-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/intel-stratix10-soc-fpga-mgr.txt
new file mode 100644
index 0000000..6e03f79
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/intel-stratix10-soc-fpga-mgr.txt
@@ -0,0 +1,17 @@
+Intel Stratix10 SoC FPGA Manager
+
+Required properties:
+The fpga_mgr node has the following mandatory property, must be located under
+firmware/svc node.
+
+- compatible : should contain "intel,stratix10-soc-fpga-mgr"
+
+Example:
+
+	firmware {
+		svc {
+			fpga_mgr: fpga-mgr {
+				compatible = "intel,stratix10-soc-fpga-mgr";
+			};
+		};
+	};
-- 
2.7.4

^ permalink raw reply related

* [PATCHv5 5/8] arm64: dts: stratix10: add fpga manager and region
From: richard.gong at linux.intel.com @ 2018-05-24 16:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527179600-26441-1-git-send-email-richard.gong@linux.intel.com>

From: Alan Tull <atull@kernel.org>

Add the Stratix10 FPGA manager and a FPGA region to the
device tree.

Signed-off-by: Alan Tull <atull@kernel.org>
Signed-off-by: Richard Gong <richard.gong@intel.com>
---
v2: this patch is added in patch set version 2
v3: change to put fpga_mgr node under firmware/svc node
v4: s/fpga-mgr at 0/fpga-mgr/ to remove unit_address
    add Richard's signed-off-by
v5: no change
---
 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
index c257287..8f8f409 100644
--- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
+++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
@@ -106,6 +106,14 @@
 		interrupt-parent = <&intc>;
 		ranges = <0 0 0 0xffffffff>;
 
+		base_fpga_region {
+			#address-cells = <0x1>;
+			#size-cells = <0x1>;
+
+			compatible = "fpga-region";
+			fpga-mgr = <&fpga_mgr>;
+		};
+
 		clkmgr: clock-controller at ffd10000 {
 			compatible = "intel,stratix10-clkmgr";
 			reg = <0xffd10000 0x1000>;
@@ -506,6 +514,10 @@
 				compatible = "intel,stratix10-svc";
 				method = "smc";
 				memory-region = <&service_reserved>;
+
+				fpga_mgr: fpga-mgr {
+					compatible = "intel,stratix10-soc-fpga-mgr";
+				};
 			};
 		};
 	};
-- 
2.7.4

^ permalink raw reply related

* [PATCHv5 6/8] fpga: add intel stratix10 soc fpga manager driver
From: richard.gong at linux.intel.com @ 2018-05-24 16:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527179600-26441-1-git-send-email-richard.gong@linux.intel.com>

From: Alan Tull <atull@kernel.org>

Add driver for reconfiguring Intel Stratix10 SoC FPGA devices.
This driver communicates through the Intel Service Driver which
does communication with privileged hardware (that does the
FPGA programming) through a secure mailbox.

Signed-off-by: Alan Tull <atull@kernel.org>
Signed-off-by: Richard Gong <richard.gong@intel.com>
---
v2: this patch is added in patch set version 2
v3: change to align to the update of service client APIs, and the
    update of fpga_mgr device node
v4: changes to align with stratix10-svc-client API updates
    add Richard's signed-off-by
v5: update to align changes at service layer to minimize service
    layer thread usages
---
 drivers/fpga/Kconfig         |   6 +
 drivers/fpga/Makefile        |   1 +
 drivers/fpga/stratix10-soc.c | 545 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 552 insertions(+)
 create mode 100644 drivers/fpga/stratix10-soc.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index f47ef84..1624a73 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -57,6 +57,12 @@ config FPGA_MGR_ZYNQ_FPGA
 	help
 	  FPGA manager driver support for Xilinx Zynq FPGAs.
 
+config FPGA_MGR_STRATIX10_SOC
+	tristate "Intel Stratix10 SoC FPGA Manager"
+	depends on (ARCH_STRATIX10 && STRATIX10_SERVICE)
+	help
+	  FPGA manager driver support for the Intel Stratix10 SoC.
+
 config FPGA_MGR_XILINX_SPI
 	tristate "Xilinx Configuration over Slave Serial (SPI)"
 	depends on SPI
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 3cb276a..6eef670 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_FPGA_MGR_ALTERA_PS_SPI)	+= altera-ps-spi.o
 obj-$(CONFIG_FPGA_MGR_ICE40_SPI)	+= ice40-spi.o
 obj-$(CONFIG_FPGA_MGR_SOCFPGA)		+= socfpga.o
 obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10)	+= socfpga-a10.o
+obj-$(CONFIG_FPGA_MGR_STRATIX10_SOC)	+= stratix10-soc.o
 obj-$(CONFIG_FPGA_MGR_TS73XX)		+= ts73xx-fpga.o
 obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
 obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
new file mode 100644
index 0000000..d645ef7
--- /dev/null
+++ b/drivers/fpga/stratix10-soc.c
@@ -0,0 +1,545 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * FPGA Manager Driver for Intel Stratix10 SoC
+ *
+ *  Copyright (C) 2018 Intel Corporation
+ */
+#include <linux/completion.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/stratix10-svc-client.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+/*
+ * FPGA programming requires a higher level of privilege (EL3), per the SoC
+ * design.
+ */
+#define NUM_SVC_BUFS	4
+#define SVC_BUF_SIZE	SZ_512K
+
+/* Indicates buffer is in use if set */
+#define SVC_BUF_LOCK	0
+
+/**
+ * struct s10_svc_buf
+ * @buf: virtual address of buf provided by service layer
+ * @lock: locked if buffer is in use
+ */
+struct s10_svc_buf {
+	char *buf;
+	unsigned long lock;
+};
+
+struct s10_priv {
+	struct stratix10_svc_chan *chan;
+	struct stratix10_svc_client client;
+	struct completion status_return_completion;
+	struct s10_svc_buf svc_bufs[NUM_SVC_BUFS];
+	unsigned long status;
+};
+
+static int s10_svc_send_msg(struct s10_priv *priv,
+			    enum stratix10_svc_command_code command,
+			    void *payload, u32 payload_length)
+{
+	struct stratix10_svc_chan *chan = priv->chan;
+	struct stratix10_svc_client_msg msg;
+	int ret;
+
+	pr_debug("%s cmd=%d payload=%p legnth=%d\n",
+		 __func__, command, payload, payload_length);
+
+	msg.command = command;
+	msg.payload = payload;
+	msg.payload_length = payload_length;
+
+	ret = stratix10_svc_send(chan, &msg);
+	pr_debug("stratix10_svc_send returned status %d\n", ret);
+
+	return ret;
+}
+
+/**
+ * s10_free_buffers
+ * Free buffers allocated from the service layer's pool that are not in use.
+ * @mgr: fpga manager struct
+ * Free all buffers that are not in use.
+ * Return true when all buffers are freed.
+ */
+static bool s10_free_buffers(struct fpga_manager *mgr)
+{
+	struct s10_priv *priv = mgr->priv;
+	uint num_free = 0;
+	uint i;
+
+	for (i = 0; i < NUM_SVC_BUFS; i++) {
+		if (!priv->svc_bufs[i].buf) {
+			num_free++;
+			continue;
+		}
+
+		if (!test_and_set_bit_lock(SVC_BUF_LOCK,
+					   &priv->svc_bufs[i].lock)) {
+			stratix10_svc_free_memory(priv->chan,
+					      priv->svc_bufs[i].buf);
+			priv->svc_bufs[i].buf = NULL;
+			num_free++;
+		}
+	}
+
+	return num_free == NUM_SVC_BUFS;
+}
+
+/**
+ * s10_free_buffer_count
+ * Count how many buffers are not in use.
+ * @mgr: fpga manager struct
+ * Return # of buffers that are not in use.
+ */
+static uint s10_free_buffer_count(struct fpga_manager *mgr)
+{
+	struct s10_priv *priv = mgr->priv;
+	uint num_free = 0;
+	uint i;
+
+	for (i = 0; i < NUM_SVC_BUFS; i++)
+		if (!priv->svc_bufs[i].buf)
+			num_free++;
+
+	return num_free;
+}
+
+/**
+ * s10_unlock_bufs
+ * Given the returned buffer address, match that address to our buffer struct
+ * and unlock that buffer.  This marks it as available to be refilled and sent
+ * (or freed).
+ * @priv: private data
+ * @kaddr: kernel address of buffer that was returned from service layer
+ */
+static void s10_unlock_bufs(struct s10_priv *priv, void *kaddr)
+{
+	uint i;
+
+	if (!kaddr)
+		return;
+
+	for (i = 0; i < NUM_SVC_BUFS; i++)
+		if (priv->svc_bufs[i].buf == kaddr) {
+			clear_bit_unlock(SVC_BUF_LOCK,
+					 &priv->svc_bufs[i].lock);
+			return;
+		}
+
+	WARN(1, "Unknown buffer returned from service layer %p\n", kaddr);
+}
+
+/**
+ * s10_receive_callback
+ * Callback for service layer to use to provide client (this driver) messages
+ * received through the mailbox.
+ * @client: service layer client struct
+ * @data: message
+ */
+static void s10_receive_callback(struct stratix10_svc_client *client,
+				 struct stratix10_svc_cb_data *data)
+{
+	struct s10_priv *priv = client->priv;
+	u32 status;
+	int i;
+
+	WARN_ONCE(!data, "%s: stratix10_svc_rc_data = NULL", __func__);
+
+	status = data->status;
+
+	/*
+	 * Here we set status bits as we receive them.  Elsewhere, we always use
+	 * test_and_clear_bit() to check status in priv->status
+	 */
+	for (i = 0; i <= SVC_STATUS_RECONFIG_ERROR; i++)
+		if (status & (1 << i))
+			set_bit(i, &priv->status);
+
+	if (status & BIT(SVC_STATUS_RECONFIG_BUFFER_DONE)) {
+		s10_unlock_bufs(priv, data->kaddr1);
+		s10_unlock_bufs(priv, data->kaddr2);
+		s10_unlock_bufs(priv, data->kaddr3);
+	}
+
+	complete(&priv->status_return_completion);
+}
+
+/**
+ * s10_ops_write_init
+ * Prepare for FPGA reconfiguration by requesting partial reconfig and
+ * allocating buffers from the service layer.
+ * @mgr: fpga manager
+ * @info: fpga image info
+ * @buf: fpga image buffer
+ * @count: size of buf in bytes
+ */
+static int s10_ops_write_init(struct fpga_manager *mgr,
+			      struct fpga_image_info *info,
+			      const char *buf, size_t count)
+{
+	struct s10_priv *priv = mgr->priv;
+	struct device *dev = priv->client.dev;
+	unsigned long timeout;
+	struct stratix10_svc_command_reconfig_payload payload;
+	char *kbuf;
+	uint i;
+	int ret;
+
+	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
+		dev_info(dev, "Requesting partial reconfiguration.\n");
+		payload.flags |= BIT(COMMAND_RECONFIG_FLAG_PARTIAL);
+	} else {
+		dev_info(dev, "Requesting full reconfiguration.\n");
+	}
+
+	reinit_completion(&priv->status_return_completion);
+	ret = s10_svc_send_msg(priv, COMMAND_RECONFIG,
+			       &payload, sizeof(payload));
+	if (ret)
+		goto init_done;
+
+	timeout = msecs_to_jiffies(SVC_RECONFIG_REQUEST_TIMEOUT_MS);
+	ret = wait_for_completion_interruptible_timeout(
+		&priv->status_return_completion, timeout);
+	if (!ret) {
+		dev_err(dev, "timeout waiting for RECONFIG_REQUEST\n");
+		ret = -ETIMEDOUT;
+		goto init_done;
+	}
+	if (ret < 0) {
+		dev_err(dev, "error (%d) waiting for RECONFIG_REQUEST\n", ret);
+		goto init_done;
+	}
+
+	ret = 0;
+	if (!test_and_clear_bit(SVC_STATUS_RECONFIG_REQUEST_OK,
+				&priv->status)) {
+		ret = -ETIMEDOUT;
+		goto init_done;
+	}
+
+	/* Allocate buffers from the service layer's pool. */
+	for (i = 0; i < NUM_SVC_BUFS; i++) {
+		kbuf = stratix10_svc_allocate_memory(priv->chan, SVC_BUF_SIZE);
+		if (!kbuf) {
+			s10_free_buffers(mgr);
+			ret = -ENOMEM;
+			goto init_done;
+		}
+
+		priv->svc_bufs[i].buf = kbuf;
+		priv->svc_bufs[i].lock = 0;
+	}
+
+init_done:
+	stratix10_svc_done(priv->chan);
+	return ret;
+}
+
+/**
+ * s10_send_buf
+ * Send a buffer to the service layer queue
+ * @mgr: fpga manager struct
+ * @buf_num: index of buffer in svc_bufs array
+ * @buf: fpga image buffer
+ * @count: size of buf in bytes
+ * Returns # of bytes transferred or -errno, never 0
+ */
+static int s10_send_buf(struct fpga_manager *mgr, uint buf_num,
+			const char *buf, size_t count)
+
+{
+	struct s10_priv *priv = mgr->priv;
+	struct device *dev = priv->client.dev;
+	void *svc_buf;
+	size_t xfer_sz;
+	int ret;
+
+	xfer_sz = count < SVC_BUF_SIZE ? count : SVC_BUF_SIZE;
+
+	svc_buf = priv->svc_bufs[buf_num].buf;
+	memcpy(svc_buf, buf, xfer_sz);
+	ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_DATA_SUBMIT,
+			       svc_buf, xfer_sz);
+	if (ret) {
+		dev_err(dev,
+			"Error while sending data to service layer (%d)", ret);
+		return ret;
+	}
+
+	return xfer_sz;
+}
+
+/**
+ * s10_ops_write
+ * Send a FPGA image to privileged layers to write to the FPGA.  When done
+ * sending, free all service layer buffers we allocated in write_init.
+ * @mgr: fpga manager
+ * @buf: fpga image buffer
+ * @count: size of buf in bytes
+ * Returns 0 for success or negative errno.
+ */
+static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
+			 size_t count)
+{
+	struct s10_priv *priv = mgr->priv;
+	struct device *dev = priv->client.dev;
+	unsigned long timeout;
+	size_t sent = 0;
+	int ret = 0;
+	uint i;
+
+	timeout = msecs_to_jiffies(SVC_RECONFIG_BUFFER_TIMEOUT_MS);
+
+	/* Buffer loop: either send buffers or free them. */
+	while (1) {
+		reinit_completion(&priv->status_return_completion);
+
+		if (count > 0) {
+			for (i = 0; i < NUM_SVC_BUFS; i++)
+				if (!test_and_set_bit_lock(
+					 SVC_BUF_LOCK, &priv->svc_bufs[i].lock))
+					break;
+
+			if (i == NUM_SVC_BUFS)
+				/* wait for a free buffer */
+				continue;
+
+			sent = s10_send_buf(mgr, i, buf, count);
+			/*
+			 * If service queue was full, we won't get a callback.
+			 * Wait and try again
+			 */
+			if (sent < 0)
+				continue;
+
+			count -= sent;
+			buf += sent;
+		} else {
+			s10_free_buffers(mgr);
+			if (s10_free_buffer_count(mgr) == NUM_SVC_BUFS)
+				return 0;
+
+			ret = s10_svc_send_msg(
+				priv, COMMAND_RECONFIG_DATA_CLAIM,
+				NULL, 0);
+			if (ret)
+				break;
+		}
+
+		/*
+		 * If callback hasn't already happened, wait for buffers to be
+		 * returned from service layer
+		 */
+		if (priv->status)
+			ret = 0;
+		else
+			ret = wait_for_completion_interruptible_timeout(
+				&priv->status_return_completion, timeout);
+
+		if (test_and_clear_bit(
+				SVC_STATUS_RECONFIG_BUFFER_DONE, &priv->status))
+			continue;
+
+		if (test_and_clear_bit(SVC_STATUS_RECONFIG_BUFFER_SUBMITTED,
+				       &priv->status))
+			continue;
+
+		if (test_and_clear_bit(SVC_STATUS_RECONFIG_ERROR,
+				       &priv->status)) {
+			dev_err(dev, "ERROR - giving up - SVC_STATUS_RECONFIG_ERROR\n");
+			ret = -EFAULT;
+			break;
+		}
+
+		if (!ret) {
+			dev_err(dev, "timeout waiting for svc layer buffers\n");
+			ret = -ETIMEDOUT;
+			break;
+		}
+		if (ret < 0) {
+			dev_err(dev,
+				"error (%d) waiting for svc layer buffers\n",
+				ret);
+			break;
+		}
+	}
+
+	s10_free_buffers(mgr);
+	if (s10_free_buffer_count(mgr) != NUM_SVC_BUFS)
+		dev_err(dev, "%s not all buffers were freed\n", __func__);
+
+	return ret;
+}
+
+/**
+ * s10_ops_write_complete
+ * Wait for FPGA configuration to be done
+ * @mgr: fpga manager
+ * @info: fpga image info
+ * Returns 0 for success negative errno.
+ */
+static int s10_ops_write_complete(struct fpga_manager *mgr,
+				  struct fpga_image_info *info)
+{
+	struct s10_priv *priv = mgr->priv;
+	struct device *dev = priv->client.dev;
+	unsigned long timeout;
+	int ret;
+
+	timeout = usecs_to_jiffies(info->config_complete_timeout_us);
+
+	do {
+		reinit_completion(&priv->status_return_completion);
+
+		ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_STATUS, NULL, 0);
+		if (ret)
+			break;
+
+		ret = wait_for_completion_interruptible_timeout(
+			&priv->status_return_completion, timeout);
+		if (!ret) {
+			dev_err(dev,
+				"timeout waiting for RECONFIG_COMPLETED\n");
+			ret = -ETIMEDOUT;
+			break;
+		}
+		if (ret < 0) {
+			dev_err(dev,
+				"error (%d) waiting for RECONFIG_COMPLETED\n",
+				ret);
+			break;
+		}
+		/* Not error or timeout, so ret is # of jiffies until timeout */
+		timeout = ret;
+		ret = 0;
+
+		if (test_and_clear_bit(SVC_STATUS_RECONFIG_COMPLETED,
+				       &priv->status))
+			break;
+
+		if (test_and_clear_bit(SVC_STATUS_RECONFIG_ERROR,
+				       &priv->status)) {
+			dev_err(dev, "ERROR - giving up - SVC_STATUS_RECONFIG_ERROR\n");
+			ret = -EFAULT;
+			break;
+		}
+	} while (1);
+
+	stratix10_svc_done(priv->chan);
+	return ret;
+}
+
+static enum fpga_mgr_states s10_ops_state(struct fpga_manager *mgr)
+{
+	return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static const struct fpga_manager_ops s10_ops = {
+	.state = s10_ops_state,
+	.write_init = s10_ops_write_init,
+	.write = s10_ops_write,
+	.write_complete = s10_ops_write_complete,
+};
+
+static int s10_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct s10_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->client.dev = dev;
+	priv->client.receive_cb = s10_receive_callback;
+	priv->client.priv = priv;
+
+	priv->chan = stratix10_svc_request_channel_byname(&priv->client,
+						SVC_CLIENT_FPGA);
+	if (IS_ERR(priv->chan)) {
+		dev_err(dev, "couldn't get service channel (%s)\n",
+			SVC_CLIENT_FPGA);
+		return PTR_ERR(priv->chan);
+	}
+
+	init_completion(&priv->status_return_completion);
+
+	ret = fpga_mgr_register(dev, "Stratix10 SOC FPGA Manager",
+				&s10_ops, priv);
+
+	if (ret)
+		stratix10_svc_free_channel(priv->chan);
+
+	return ret;
+}
+
+static int s10_remove(struct platform_device *pdev)
+{
+	struct fpga_manager *mgr = platform_get_drvdata(pdev);
+	struct s10_priv *priv = mgr->priv;
+
+	fpga_mgr_unregister(&pdev->dev);
+	stratix10_svc_free_channel(priv->chan);
+
+	return 0;
+}
+
+static const struct of_device_id s10_of_match[] = {
+	{ .compatible = "intel,stratix10-soc-fpga-mgr", },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, s10_of_match);
+
+static struct platform_driver s10_driver = {
+	.probe = s10_probe,
+	.remove = s10_remove,
+	.driver = {
+		.name	= "Stratix10 SoC FPGA manager",
+		.of_match_table = of_match_ptr(s10_of_match),
+	},
+};
+
+static int __init s10_init(void)
+{
+	struct device_node *fw_np;
+	struct device_node *np;
+	int ret;
+
+	fw_np = of_find_node_by_name(NULL, "svc");
+	if (!fw_np)
+		return -ENODEV;
+
+	np = of_find_matching_node(fw_np, s10_of_match);
+	if (!np) {
+		of_node_put(fw_np);
+		return -ENODEV;
+	}
+
+	of_node_put(np);
+	ret = of_platform_populate(fw_np, s10_of_match, NULL, NULL);
+	of_node_put(fw_np);
+	if (ret)
+		return ret;
+
+	return platform_driver_register(&s10_driver);
+}
+
+static void __exit s10_exit(void)
+{
+	return platform_driver_unregister(&s10_driver);
+}
+
+module_init(s10_init);
+module_exit(s10_exit);
+
+MODULE_AUTHOR("Alan Tull <atull@kernel.org>");
+MODULE_DESCRIPTION("Intel Stratix 10 SOC FPGA Manager");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

^ permalink raw reply related

* [PATCHv5 7/8] defconfig: enable fpga and service layer
From: richard.gong at linux.intel.com @ 2018-05-24 16:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527179600-26441-1-git-send-email-richard.gong@linux.intel.com>

From: Richard Gong <richard.gong@intel.com>

Enable fpga framework, Stratix 10 SoC FPGA manager and Stratix10
Service Layer

Signed-off-by: Richard Gong <richard.gong@intel.com>
Signed-off-by: Alan Tull <atull@kernel.org>
---
v2: this patch is added in patch set version 2
v3: no change
v4: s/CONFIG_INTEL_SERVICE/CONFIG_STRATIX10_SERVICE/
    add CONFIG_OF_FPGA_REGION=y
    s/Intel/Stratix10/ in subject line
v5: no change
---
 arch/arm64/configs/defconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index ecf6137..5f7a9b7 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -180,6 +180,7 @@ CONFIG_BLK_DEV_LOOP=y
 CONFIG_BLK_DEV_NBD=m
 CONFIG_VIRTIO_BLK=y
 CONFIG_BLK_DEV_NVME=m
+CONFIG_STRATIX10_SERVICE=y
 CONFIG_SRAM=y
 CONFIG_EEPROM_AT25=m
 # CONFIG_SCSI_PROC_FS is not set
@@ -595,6 +596,11 @@ CONFIG_PHY_TEGRA_XUSB=y
 CONFIG_QCOM_L2_PMU=y
 CONFIG_QCOM_L3_PMU=y
 CONFIG_MESON_EFUSE=m
+CONFIG_FPGA=y
+CONFIG_FPGA_MGR_STRATIX10_SOC=y
+CONFIG_FPGA_REGION=y
+CONFIG_FPGA_BRIDGE=y
+CONFIG_OF_FPGA_REGION=y
 CONFIG_QCOM_QFPROM=y
 CONFIG_UNIPHIER_EFUSE=y
 CONFIG_TEE=y
-- 
2.7.4

^ permalink raw reply related

* [PATCHv5 8/8] Documentation: driver-api: add stratix10 service layer
From: richard.gong at linux.intel.com @ 2018-05-24 16:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527179600-26441-1-git-send-email-richard.gong@linux.intel.com>

From: Richard Gong <richard.gong@intel.com>

Add new file stratix10-svc.rst
Add stratix10-svc.rst to driver-api/index.rst

Signed-off-by: Richard Gong <richard.gong@intel.com>
Signed-off-by: Alan Tull <atull@kernel.org>
---
v5: this patch is added in patch set version 5
---
 Documentation/driver-api/index.rst         |  1 +
 Documentation/driver-api/stratix10-svc.rst | 32 ++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)
 create mode 100644 Documentation/driver-api/stratix10-svc.rst

diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
index 6d8352c..4b31109 100644
--- a/Documentation/driver-api/index.rst
+++ b/Documentation/driver-api/index.rst
@@ -49,6 +49,7 @@ available subsections can be seen below.
    dmaengine/index
    slimbus
    soundwire/index
+   stratix10-svc
 
 .. only::  subproject and html
 
diff --git a/Documentation/driver-api/stratix10-svc.rst b/Documentation/driver-api/stratix10-svc.rst
new file mode 100644
index 0000000..ed361d8
--- /dev/null
+++ b/Documentation/driver-api/stratix10-svc.rst
@@ -0,0 +1,32 @@
+
+Intel Stratix10 SoC Service Layer
+=================================
+
+Some features of the Intel Stratix10 SoC require a level of privilege
+higher than the kernel is granted. Such secure features include
+FPGA programming. In terms of the ARMv8 architecture, the kernel runs
+at Exception Level 1 (EL1), access to the features requires
+Exception Level 3 (EL3).
+
+The Intel Stratix10 SoC service layer provides an in kernel API for
+drivers to request access to the secure features. The requests are queued
+and processed one by one. ARM?s SMCCC is used to pass the execution
+of the requests on to a secure monitor (EL3).
+
+.. kernel-doc:: include/linux/stratix10-svc-client.h
+   :functions: stratix10_svc_command_code
+
+.. kernel-doc:: include/linux/stratix10-svc-client.h
+   :functions: stratix10_svc_client_msg
+
+.. kernel-doc:: include/linux/stratix10-svc-client.h
+   :functions: stratix10_svc_command_reconfig_payload
+
+.. kernel-doc:: include/linux/stratix10-svc-client.h
+   :functions: stratix10_svc_cb_data
+
+.. kernel-doc:: include/linux/stratix10-svc-client.h
+   :functions: stratix10_svc_client
+
+.. kernel-doc:: drivers/misc/stratix10-svc.c
+   :export:
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 4/6] watchdog: sp805: set WDOG_HW_RUNNING when appropriate
From: Ray Jui @ 2018-05-24 16:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524161901.GB28425@roeck-us.net>



On 5/24/2018 9:19 AM, Guenter Roeck wrote:
> On Wed, May 23, 2018 at 05:15:22PM -0700, Ray Jui wrote:
>> If the watchdog hardware is already enabled during the boot process,
>> when the Linux watchdog driver loads, it should reset the watchdog and
>> tell the watchdog framework. As a result, ping can be generated from
>> the watchdog framework, until the userspace watchdog daemon takes over
>> control
>>
>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 
> I have one question, though: Is it really correct that both
> INT_ENABLE _and_ RESET_ENABLE have to be set to enable the watdog ?
> What if only RESET_ENABLE is set ?

According to the SP805 TRM, INT_ENABLE needs to be set to high to enable 
the counter and the interrupt. Counter will be stopped if INT_ENABLE is 
cleared. So yes, INT_ENABLE needs to be set.

Thanks,

Ray

> 
> Thanks,
> Guenter
> 
>> ---
>>   drivers/watchdog/sp805_wdt.c | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
>> index 1484609..d662a6f 100644
>> --- a/drivers/watchdog/sp805_wdt.c
>> +++ b/drivers/watchdog/sp805_wdt.c
>> @@ -42,6 +42,7 @@
>>   	/* control register masks */
>>   	#define	INT_ENABLE	(1 << 0)
>>   	#define	RESET_ENABLE	(1 << 1)
>> +	#define	ENABLE_MASK	(INT_ENABLE | RESET_ENABLE)
>>   #define WDTINTCLR		0x00C
>>   #define WDTRIS			0x010
>>   #define WDTMIS			0x014
>> @@ -74,6 +75,15 @@ module_param(nowayout, bool, 0);
>>   MODULE_PARM_DESC(nowayout,
>>   		"Set to 1 to keep watchdog running after device release");
>>   
>> +/* returns true if wdt is running; otherwise returns false */
>> +static bool wdt_is_running(struct watchdog_device *wdd)
>> +{
>> +	struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>> +	u32 wdtcontrol = readl_relaxed(wdt->base + WDTCONTROL);
>> +
>> +	return (wdtcontrol & ENABLE_MASK) == ENABLE_MASK;
>> +}
>> +
>>   /* This routine finds load value that will reset system in required timout */
>>   static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout)
>>   {
>> @@ -239,6 +249,15 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
>>   	watchdog_init_timeout(&wdt->wdd, 0, &adev->dev);
>>   	wdt_setload(&wdt->wdd, wdt->wdd.timeout);
>>   
>> +	/*
>> +	 * If HW is already running, enable/reset the wdt and set the running
>> +	 * bit to tell the wdt subsystem
>> +	 */
>> +	if (wdt_is_running(&wdt->wdd)) {
>> +		wdt_enable(&wdt->wdd);
>> +		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
>> +	}
>> +
>>   	ret = watchdog_register_device(&wdt->wdd);
>>   	if (ret) {
>>   		dev_err(&adev->dev, "watchdog_register_device() failed: %d\n",
>> -- 
>> 2.1.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH 3/4] KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
From: Marc Zyngier @ 2018-05-24 16:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524155117.qdi25z33lnmkrsoe@armageddon.cambridge.arm.com>

On 24/05/18 16:51, Catalin Marinas wrote:
> On Thu, May 17, 2018 at 11:35:47AM +0100, Marc Zyngier wrote:
>> There is no need to perform cache maintenance operations when
>> creating the HYP page tables if we have the multiprocessing
>> extensions. ARMv7 mandates them with the virtualization support,
>> and ARMv8 just mandates them unconditionally.
>>
>> Let's remove these operations.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  virt/kvm/arm/mmu.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>> index ba66bf7ae299..acbfea09578c 100644
>> --- a/virt/kvm/arm/mmu.c
>> +++ b/virt/kvm/arm/mmu.c
>> @@ -578,7 +578,6 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start,
>>  		pte = pte_offset_kernel(pmd, addr);
>>  		kvm_set_pte(pte, pfn_pte(pfn, prot));
>>  		get_page(virt_to_page(pte));
>> -		kvm_flush_dcache_to_poc(pte, sizeof(*pte));
>>  		pfn++;
>>  	} while (addr += PAGE_SIZE, addr != end);
>>  }
>> @@ -605,7 +604,6 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start,
>>  			}
>>  			pmd_populate_kernel(NULL, pmd, pte);
>>  			get_page(virt_to_page(pmd));
>> -			kvm_flush_dcache_to_poc(pmd, sizeof(*pmd));
>>  		}
>>  
>>  		next = pmd_addr_end(addr, end);
>> @@ -638,7 +636,6 @@ static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start,
>>  			}
>>  			pud_populate(NULL, pud, pmd);
>>  			get_page(virt_to_page(pud));
>> -			kvm_flush_dcache_to_poc(pud, sizeof(*pud));
>>  		}
>>  
>>  		next = pud_addr_end(addr, end);
>> @@ -675,7 +672,6 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>>  			}
>>  			pgd_populate(NULL, pgd, pud);
>>  			get_page(virt_to_page(pgd));
>> -			kvm_flush_dcache_to_poc(pgd, sizeof(*pgd));
>>  		}
>>  
>>  		next = pgd_addr_end(addr, end);
>> @@ -685,6 +681,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>>  		pfn += (next - addr) >> PAGE_SHIFT;
>>  	} while (addr = next, addr != end);
>>  out:
>> +	dsb(ishst);
>>  	mutex_unlock(&kvm_hyp_pgd_mutex);
> 
> Why do we need the DSB here? A comment would help.

That's a combination of me being paranoid (we've just dropped a bunch of
CMOs that had DSBs there), and prior art (see dcadda146f4f -- adding
Mark who authored that change).

My understanding is also that we do need this as per K11.5.3 (ARMv8 ARM
rev C.a), which indicates that a DSB ISH "ensures visibility of the
update to translation table walks".

There is also this text:

<quote>

A translation table walk is considered to be a separate observer, and:
-  A write to the translation tables can be observed by that separate
observer at any time after the execution of the instruction that
performed that write, but is only guaranteed to be observable after the
execution of a DSB instruction by the PE that executed the instruction
that performed that write to the translation tables.

</quote>

I'm happy to add comment though.

> If a DMB is sufficient, I think mutex_unlock has release semantics.

I don't think it is, unfortunately.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH v3 2/6] Documentation: DT: Add optional 'timeout-sec' property for sp805
From: Ray Jui @ 2018-05-24 16:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524161628.GA28425@roeck-us.net>



On 5/24/2018 9:16 AM, Guenter Roeck wrote:
> On Wed, May 23, 2018 at 05:15:20PM -0700, Ray Jui wrote:
>> Update the SP805 binding document to add optional 'timeout-sec'
>> devicetree property
>>
>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>> ---
>>   Documentation/devicetree/bindings/watchdog/arm,sp805.txt | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
>> index 0fa3629..1debea3 100644
>> --- a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
>> +++ b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
>> @@ -19,6 +19,8 @@ Required properties:
>>   
>>   Optional properties:
>>   - interrupts:  Should specify WDT interrupt number
>> +- timeout-sec: Should specify default WDT timeout in seconds. If unset, the
>> +               default timeout in the driver is 30 seconds
> 
> "... the default timeout is determined by the driver" might be better.
> If you want to mandate a default here (not sure if that is a good idea),
> I would suggest to use something like "should be 30 seconds".
> 

Okay. This can be changed to:

- timeout-sec: Should specify default WDT timeout in seconds. If unset, 
the default timeout is determined by the driver.

Please advise how to proceed with this patch series. Should I make the 
above modification and send out v4?

Thanks,

Ray

> Guenter
> 
>>   
>>   Example:
>>   	watchdog at 66090000 {
>> -- 
>> 2.1.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v9 02/15] clk: mux: Split out register accessors for reuse
From: Bjorn Andersson @ 2018-05-24 16:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1520347148-27852-3-git-send-email-sricharan@codeaurora.org>

On Tue 06 Mar 06:38 PST 2018, Sricharan R wrote:

> From: Stephen Boyd <sboyd@codeaurora.org>
> 
> We want to reuse the logic in clk-mux.c for other clock drivers
> that don't use readl as register accessors. Fortunately, there
> really isn't much to the mux code besides the table indirection
> and quirk flags if you assume any bit shifting and masking has
> been done already. Pull that logic out into reusable functions
> that operate on an optional table and some flags so that other
> drivers can use the same logic.
> 
> [Sricharan: Rebased for mainline]
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>

This should read as a log, where the first entry is Stephen stating that
he acquired or wrote the code and can release it according to the
license requirements. Then you state that you acquired it, changed it
and are releasing it according to the license requirements.


PS. Please expand your last name.

Regards,
Bjorn

> ---
>  drivers/clk/clk-mux.c         | 74 +++++++++++++++++++++++++++----------------
>  drivers/clk/nxp/clk-lpc32xx.c | 21 +++---------
>  include/linux/clk-provider.h  |  6 ++++
>  3 files changed, 57 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index 39cabe1..28223fa 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -26,35 +26,25 @@
>   * parent - parent is adjustable through clk_set_parent
>   */
>  
> -static u8 clk_mux_get_parent(struct clk_hw *hw)
> +unsigned int clk_mux_get_parent(struct clk_hw *hw, unsigned int val,
> +				unsigned int *table,
> +				unsigned long flags)
>  {
> -	struct clk_mux *mux = to_clk_mux(hw);
>  	int num_parents = clk_hw_get_num_parents(hw);
> -	u32 val;
> -
> -	/*
> -	 * FIXME need a mux-specific flag to determine if val is bitwise or numeric
> -	 * e.g. sys_clkin_ck's clksel field is 3 bits wide, but ranges from 0x1
> -	 * to 0x7 (index starts at one)
> -	 * OTOH, pmd_trace_clk_mux_ck uses a separate bit for each clock, so
> -	 * val = 0x4 really means "bit 2, index starts at bit 0"
> -	 */
> -	val = clk_readl(mux->reg) >> mux->shift;
> -	val &= mux->mask;
>  
> -	if (mux->table) {
> +	if (table) {
>  		int i;
>  
>  		for (i = 0; i < num_parents; i++)
> -			if (mux->table[i] == val)
> +			if (table[i] == val)
>  				return i;
>  		return -EINVAL;
>  	}
>  
> -	if (val && (mux->flags & CLK_MUX_INDEX_BIT))
> +	if (val && (flags & CLK_MUX_INDEX_BIT))
>  		val = ffs(val) - 1;
>  
> -	if (val && (mux->flags & CLK_MUX_INDEX_ONE))
> +	if (val && (flags & CLK_MUX_INDEX_ONE))
>  		val--;
>  
>  	if (val >= num_parents)
> @@ -62,23 +52,53 @@ static u8 clk_mux_get_parent(struct clk_hw *hw)
>  
>  	return val;
>  }
> +EXPORT_SYMBOL_GPL(clk_mux_get_parent);
>  
> -static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
> +static u8 _clk_mux_get_parent(struct clk_hw *hw)
>  {
>  	struct clk_mux *mux = to_clk_mux(hw);
>  	u32 val;
> -	unsigned long flags = 0;
>  
> -	if (mux->table) {
> -		index = mux->table[index];
> +	/*
> +	 * FIXME need a mux-specific flag to determine if val is bitwise or
> +	 * numeric e.g. sys_clkin_ck's clksel field is 3 bits wide,
> +	 * but ranges from 0x1 to 0x7 (index starts at one)
> +	 * OTOH, pmd_trace_clk_mux_ck uses a separate bit for each clock, so
> +	 * val = 0x4 really means "bit 2, index starts at bit 0"
> +	 */
> +	val = clk_readl(mux->reg) >> mux->shift;
> +	val &= mux->mask;
> +
> +	return clk_mux_get_parent(hw, val, mux->table, mux->flags);
> +}
> +
> +unsigned int clk_mux_reindex(u8 index, unsigned int *table,
> +			     unsigned long flags)
> +{
> +	unsigned int val = index;
> +
> +	if (table) {
> +		val = table[val];
>  	} else {
> -		if (mux->flags & CLK_MUX_INDEX_BIT)
> -			index = 1 << index;
> +		if (flags & CLK_MUX_INDEX_BIT)
> +			val = 1 << index;
>  
> -		if (mux->flags & CLK_MUX_INDEX_ONE)
> -			index++;
> +		if (flags & CLK_MUX_INDEX_ONE)
> +			val++;
>  	}
>  
> +	return val;
> +}
> +EXPORT_SYMBOL_GPL(clk_mux_reindex);
> +
> +static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
> +{
> +	struct clk_mux *mux = to_clk_mux(hw);
> +	u32 val;
> +	unsigned long flags = 0;
> +
> +	index = clk_mux_reindex(index, mux->table, mux->flags);
> +
>  	if (mux->lock)
>  		spin_lock_irqsave(mux->lock, flags);
>  	else
> @@ -102,14 +122,14 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>  }
>  
>  const struct clk_ops clk_mux_ops = {
> -	.get_parent = clk_mux_get_parent,
> +	.get_parent = _clk_mux_get_parent,
>  	.set_parent = clk_mux_set_parent,
>  	.determine_rate = __clk_mux_determine_rate,
>  };
>  EXPORT_SYMBOL_GPL(clk_mux_ops);
>  
>  const struct clk_ops clk_mux_ro_ops = {
> -	.get_parent = clk_mux_get_parent,
> +	.get_parent = _clk_mux_get_parent,
>  };
>  EXPORT_SYMBOL_GPL(clk_mux_ro_ops);
>  
> diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c
> index f5d815f..9b34150 100644
> --- a/drivers/clk/nxp/clk-lpc32xx.c
> +++ b/drivers/clk/nxp/clk-lpc32xx.c
> @@ -999,29 +999,16 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
>  	.set_rate = clk_divider_set_rate,
>  };
>  
> -static u8 clk_mux_get_parent(struct clk_hw *hw)
> +static u8 _clk_mux_get_parent(struct clk_hw *hw)
>  {
>  	struct lpc32xx_clk_mux *mux = to_lpc32xx_mux(hw);
> -	u32 num_parents = clk_hw_get_num_parents(hw);
>  	u32 val;
>  
>  	regmap_read(clk_regmap, mux->reg, &val);
>  	val >>= mux->shift;
>  	val &= mux->mask;
>  
> -	if (mux->table) {
> -		u32 i;
> -
> -		for (i = 0; i < num_parents; i++)
> -			if (mux->table[i] == val)
> -				return i;
> -		return -EINVAL;
> -	}
> -
> -	if (val >= num_parents)
> -		return -EINVAL;
> -
> -	return val;
> +	return clk_mux_get_parent(hw, val, mux->table, 0);
>  }
>  
>  static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
> @@ -1036,11 +1023,11 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>  }
>  
>  static const struct clk_ops lpc32xx_clk_mux_ro_ops = {
> -	.get_parent = clk_mux_get_parent,
> +	.get_parent = _clk_mux_get_parent,
>  };
>  
>  static const struct clk_ops lpc32xx_clk_mux_ops = {
> -	.get_parent = clk_mux_get_parent,
> +	.get_parent = _clk_mux_get_parent,
>  	.set_parent = clk_mux_set_parent,
>  	.determine_rate = __clk_mux_determine_rate,
>  };
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index f711be6..344ad92 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -488,6 +488,12 @@ struct clk_mux {
>  extern const struct clk_ops clk_mux_ops;
>  extern const struct clk_ops clk_mux_ro_ops;
>  
> +unsigned int clk_mux_get_parent(struct clk_hw *hw, unsigned int val,
> +				unsigned int *table,
> +				unsigned long flags);
> +unsigned int clk_mux_reindex(u8 index, unsigned int *table,
> +			     unsigned long flags);
> +
>  struct clk *clk_register_mux(struct device *dev, const char *name,
>  		const char * const *parent_names, u8 num_parents,
>  		unsigned long flags,
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
> 

^ permalink raw reply

* [PATCH v4 2/2] arm64: signal: Report signal frame size to userspace via auxv
From: Will Deacon @ 2018-05-24 16:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524155516.GW13470@e103592.cambridge.arm.com>

On Thu, May 24, 2018 at 04:55:17PM +0100, Dave Martin wrote:
> On Thu, May 24, 2018 at 01:49:21PM +0100, Will Deacon wrote:
> > On Wed, May 23, 2018 at 06:46:56PM +0100, Dave Martin wrote:
> > > Stateful CPU architecture extensions may require the signal frame
> > > to grow to a size that exceeds the arch's MINSIGSTKSZ #define.
> > > However, changing this #define is an ABI break.
> > > 
> > > To allow userspace the option of determining the signal frame size
> > > in a more forwards-compatible way, this patch adds a new auxv entry
> > > tagged with AT_MINSIGSTKSZ, which provides the maximum signal frame
> > > size that the process can observe during its lifetime.
> > > 
> > > If AT_MINSIGSTKSZ is absent from the aux vector, the caller can
> > > assume that the MINSIGSTKSZ #define is sufficient.  This allows for
> > > a consistent interface with older kernels that do not provide
> > > AT_MINSIGSTKSZ.
> > > 
> > > The idea is that libc could expose this via sysconf() or some
> > > similar mechanism.
> > > 
> > > There is deliberately no AT_SIGSTKSZ.  The kernel knows nothing
> > > about userspace's own stack overheads and should not pretend to
> > > know.
> > 
> > [...]
> > 
> > > diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> > > index fac1c4d..9c18f0e 100644
> > > --- a/arch/arm64/include/asm/elf.h
> > > +++ b/arch/arm64/include/asm/elf.h
> > > @@ -121,6 +121,9 @@
> > >  
> > >  #ifndef __ASSEMBLY__
> > >  
> > > +#include <linux/bug.h>
> > > +#include <asm/processor.h> /* for signal_minsigstksz, used by ARCH_DLINFO */
> > > +
> > >  typedef unsigned long elf_greg_t;
> > >  
> > >  #define ELF_NGREG (sizeof(struct user_pt_regs) / sizeof(elf_greg_t))
> > > @@ -148,6 +151,14 @@ typedef struct user_fpsimd_state elf_fpregset_t;
> > >  do {									\
> > >  	NEW_AUX_ENT(AT_SYSINFO_EHDR,					\
> > >  		    (elf_addr_t)current->mm->context.vdso);		\
> > > +									\
> > > +	/*								\
> > > +	 * Should always be nonzero unless there's a kernel bug.  If	\
> > > +	 * the we haven't determined a sensible value to give to	\
> > 
> > "If the we"?
> 
> Dang, fixed locally now.
> 
> [...]
> 
> > > diff --git a/arch/arm64/include/uapi/asm/auxvec.h b/arch/arm64/include/uapi/asm/auxvec.h
> > > index ec0a86d..743c0b8 100644
> > > --- a/arch/arm64/include/uapi/asm/auxvec.h
> > > +++ b/arch/arm64/include/uapi/asm/auxvec.h
> > > @@ -19,7 +19,8 @@
> > >  
> > >  /* vDSO location */
> > >  #define AT_SYSINFO_EHDR	33
> > > +#define AT_MINSIGSTKSZ	51	/* stack needed for signal delivery */
> > 
> > Curious: but how do we avoid/detect conflicts at -rc1? I guess somebody just
> > needs to remember to run grep? (I know you have another series consolidating
> > the ID allocations).
> 
> We basically can't.  These are spread over various arch headers today,
> so the solution is to (a) grep, and (b) know that you needed to do that.
> 
> This is the main motivation for collecting the definitions together.
> 
> Short of having some script that checks these at build-time, I couldn't
> see another obvious solution.  It's nonetheless a bit ugly because of
> things like AT_VECTOR_SIZE_ARCH which is masquerading a tag but isn't
> one, and obviously does vary across arches...
> 
> [...]
> 
> > > diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> > > index 154b7d3..00b9990 100644
> > > --- a/arch/arm64/kernel/signal.c
> > > +++ b/arch/arm64/kernel/signal.c
> 
> [...]
> 
> > > @@ -936,3 +949,28 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
> > >  		thread_flags = READ_ONCE(current_thread_info()->flags);
> > >  	} while (thread_flags & _TIF_WORK_MASK);
> > >  }
> > > +
> > > +unsigned long __ro_after_init signal_minsigstksz;
> > > +
> > > +/*
> > > + * Determine the stack space required for guaranteed signal devliery.
> > > + * This function is used to populate AT_MINSIGSTKSZ at process startup.
> > > + * cpufeatures setup is assumed to be complete.
> > > + */
> > > +void __init minsigstksz_setup(void)
> > > +{
> > > +	struct rt_sigframe_user_layout user;
> > > +
> > > +	init_user_layout(&user);
> > > +
> > > +	/*
> > > +	 * If this fails, SIGFRAME_MAXSZ needs to be enlarged.  It won't
> > > +	 * be big enough, but it's our best guess:
> > > +	 */
> > > +	if (WARN_ON(setup_sigframe_layout(&user, true)))
> > > +		signal_minsigstksz = SIGFRAME_MAXSZ;
> > 
> > Can we not leave signal_minsigstksz as zero in this case?
> 
> I prefer to distinguish the "kernel went wrong" case (where we just omit
> AT_MINSIGSTKSZ for backwards compatibilty) from the "sigframe too
> large" case.

Hmm, so I'm confused as to the distinction here. Wouldn't an allocation
failure in setup_sigframe_layout be indicative of "kernel went wrong"?

To put it another way, if we could determine the maximum sigframe size
at build time, surely we'd fail the build if SIGFRAME_MAXSZ wasn't big
enough? In that case, detecting this at runtime is also pretty bad (hence
the WARN_ON) and I think we should drop the aux entry rather than provide
a value that is known to be incorrect.

Will

^ permalink raw reply

* [PATCH v11 00/19] KVM: arm64: Optimise FPSIMD context switching
From: Dave Martin @ 2018-05-24 16:56 UTC (permalink / raw)
  To: linux-arm-kernel

Note: Most of these patches are Arm-specific.  People not Cc'd on the
whole series can find it in the linux-arm-kernel archive [2].

Git: see [3]

This series aims to improve the way FPSIMD context is handled by KVM.
Changes since the previous v10 [1] are minor, but one patch has been
split.

** To Catalin Marinas, Alex Benn?e: **

I have retained your Reviewed-bys on patches 7 and 8: these were split
from patch 7 of v9.  Although the code is (almost) the same as noted
below, I advise you to take a look.

(all) reviewers please note:

 * Patches 7 and 8 have been split from the previous patch 7, since
   the update to INIT_THREAD_INFO is logically a fix and deserves
   a Fixes: tag.

   I have also moved the definition of INIT_THREAD_INFO to after the
   TIF_* definitions that it now depends on (mysteriously this wasn't
   causing a compile error anyway, but I've not dug into why).

 * Other changes are all additions or rewordings of comments, and
   amendments commit messages.

Cheers
---Dave

[1] [PATCH v10 00/18] KVM: arm64: Optimise FPSIMD context switching
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/579899.html

[2] linux-arm-kernel archive
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/thread.html

[3] http://linux-arm.org/git?p=linux-dm.git;a=shortlog;h=refs/heads/kvm-vhe-fpsimd/v11

Christoffer Dall (1):
  KVM: arm/arm64: Introduce kvm_arch_vcpu_run_pid_change

Dave Martin (18):
  arm64: fpsimd: Fix TIF_FOREIGN_FPSTATE after invalidating cpu regs
  thread_info: Add update_thread_flag() helpers
  arm64: Use update{,_tsk}_thread_flag()
  KVM: arm64: Convert lazy FPSIMD context switch trap to C
  arm64: fpsimd: Generalise context saving for non-task contexts
  arm64: fpsimd: Avoid FPSIMD context leakage for the init task
  arm64: fpsimd: Eliminate task->mm checks
  arm64/sve: Refactor user SVE trap maintenance for external use
  KVM: arm64: Repurpose vcpu_arch.debug_flags for general-purpose flags
  KVM: arm64: Optimise FPSIMD handling to reduce guest/host thrashing
  arm64/sve: Move read_zcr_features() out of cpufeature.h
  arm64/sve: Switch sve_pffr() argument from task to thread
  arm64/sve: Move sve_pffr() to fpsimd.h and make inline
  KVM: arm64: Save host SVE context as appropriate
  KVM: arm64: Remove eager host SVE state saving
  KVM: arm64: Remove redundant *exit_code changes in fpsimd_guest_exit()
  KVM: arm64: Fold redundant exit code checks out of fixup_guest_exit()
  KVM: arm64: Invoke FPSIMD context switch trap from C

 arch/arm/include/asm/kvm_host.h      |  10 +-
 arch/arm64/Kconfig                   |   7 ++
 arch/arm64/include/asm/cpufeature.h  |  29 ------
 arch/arm64/include/asm/fpsimd.h      |  21 +++++
 arch/arm64/include/asm/kvm_asm.h     |   3 -
 arch/arm64/include/asm/kvm_host.h    |  45 +++++++--
 arch/arm64/include/asm/processor.h   |  15 ++-
 arch/arm64/include/asm/thread_info.h |  13 +--
 arch/arm64/kernel/fpsimd.c           | 177 +++++++++++++++++------------------
 arch/arm64/kernel/ptrace.c           |   1 +
 arch/arm64/kvm/Kconfig               |   1 +
 arch/arm64/kvm/Makefile              |   2 +-
 arch/arm64/kvm/debug.c               |   8 +-
 arch/arm64/kvm/fpsimd.c              | 110 ++++++++++++++++++++++
 arch/arm64/kvm/hyp/debug-sr.c        |   6 +-
 arch/arm64/kvm/hyp/entry.S           |  43 ---------
 arch/arm64/kvm/hyp/hyp-entry.S       |  19 ----
 arch/arm64/kvm/hyp/switch.c          | 124 ++++++++++++++++--------
 arch/arm64/kvm/hyp/sysreg-sr.c       |   4 +-
 arch/arm64/kvm/sys_regs.c            |   9 +-
 include/linux/kvm_host.h             |   9 ++
 include/linux/sched.h                |   6 ++
 include/linux/thread_info.h          |  11 +++
 virt/kvm/Kconfig                     |   3 +
 virt/kvm/arm/arm.c                   |  14 ++-
 virt/kvm/kvm_main.c                  |   7 +-
 26 files changed, 437 insertions(+), 260 deletions(-)
 create mode 100644 arch/arm64/kvm/fpsimd.c

-- 
2.1.4

^ permalink raw reply

* [PATCH v11 01/19] arm64: fpsimd: Fix TIF_FOREIGN_FPSTATE after invalidating cpu regs
From: Dave Martin @ 2018-05-24 16:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527181008-13549-1-git-send-email-Dave.Martin@arm.com>

fpsimd_last_state.st is set to NULL as a way of indicating that
current's FPSIMD registers are no longer loaded in the cpu.  In
particular, this is done when the kernel temporarily uses or
clobbers the FPSIMD registers for its own purposes, as in CPU PM or
kernel-mode NEON, resulting in them being populated with garbage
data not belonging to a task.

Commit 17eed27b02da ("arm64/sve: KVM: Prevent guests from using
SVE") factors this operation out as a new helper
fpsimd_flush_cpu_state() to make it clearer what is being done
here, and on SVE systems this helper is now used, via
kvm_fpsimd_flush_cpu_state(), to invalidate the registers after KVM
has run a vcpu.  The reason for this is that KVM does not yet
understand how to restore the full host SVE registers itself after
loading the guest FPSIMD context into them.

This exposes a particular problem: if fpsimd_last_state.st is set
to NULL without also setting TIF_FOREIGN_FPSTATE, the kernel may
continue to think that current's FPSIMD registers are live even
though they have actually been clobbered.

Prior to the aforementioned commit, the only path where
fpsimd_last_state.st is set to NULL without setting
TIF_FOREIGN_FPSTATE is when kernel_neon_begin() is called by a
kernel thread (where current->mm can be NULL).  This does not
matter, because the only harm is that at context-switch time
fpsimd_thread_switch() may unnecessarily save the FPSIMD registers
back to current's thread_struct (even though kernel threads are not
considered to have any FPSIMD context of their own and the
registers will never be reloaded).

Note that although CPU_PM_ENTER lacks the TIF_FOREIGN_FPSTATE
setting, every CPU passing through that path must subsequently pass
through CPU_PM_EXIT before it can re-enter the kernel proper.
CPU_PM_EXIT sets the flag.

The sve_flush_cpu_state() function added by commit 17eed27b02da
also lacks the proper maintenance of TIF_FOREIGN_FPSTATE.  This may
cause the bits of a host task's SVE registers that do not alias the
FPSIMD register file to spontaneously appear zeroed if a KVM vcpu
runs in the same task in the meantime.  Although this effect is
hidden by the fact that the non-FPSIMD bits of the SVE registers
are zeroed by a syscall anyway, it is doubtless a bad idea to rely
on these different code paths interacting correctly under future
maintenance.

This patch makes TIF_FOREIGN_FPSTATE an unconditional side-effect
of fpsimd_flush_cpu_state(), and removes the set_thread_flag()
calls that become redundant as a result.  This ensures that
TIF_FOREIGN_FPSTATE cannot remain clear if the FPSIMD state in the
FPSIMD registers is invalid.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
Reviewed-by: Alex Benn?e <alex.bennee@linaro.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/kernel/fpsimd.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 87a3536..12e1c96 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1067,6 +1067,7 @@ void fpsimd_flush_task_state(struct task_struct *t)
 static inline void fpsimd_flush_cpu_state(void)
 {
 	__this_cpu_write(fpsimd_last_state.st, NULL);
+	set_thread_flag(TIF_FOREIGN_FPSTATE);
 }
 
 /*
@@ -1121,10 +1122,8 @@ void kernel_neon_begin(void)
 	__this_cpu_write(kernel_neon_busy, true);
 
 	/* Save unsaved task fpsimd state, if any: */
-	if (current->mm) {
+	if (current->mm)
 		task_fpsimd_save();
-		set_thread_flag(TIF_FOREIGN_FPSTATE);
-	}
 
 	/* Invalidate any task state remaining in the fpsimd regs: */
 	fpsimd_flush_cpu_state();
@@ -1251,8 +1250,6 @@ static int fpsimd_cpu_pm_notifier(struct notifier_block *self,
 		fpsimd_flush_cpu_state();
 		break;
 	case CPU_PM_EXIT:
-		if (current->mm)
-			set_thread_flag(TIF_FOREIGN_FPSTATE);
 		break;
 	case CPU_PM_ENTER_FAILED:
 	default:
-- 
2.1.4

^ permalink raw reply related

* [PATCH v11 02/19] thread_info: Add update_thread_flag() helpers
From: Dave Martin @ 2018-05-24 16:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527181008-13549-1-git-send-email-Dave.Martin@arm.com>

There are a number of bits of code sprinkled around the kernel to
set a thread flag if a certain condition is true, and clear it
otherwise.

To help make those call sites terser and less cumbersome, this
patch adds a new family of thread flag manipulators

	update*_thread_flag([...,] flag, cond)

which do the equivalent of:

	if (cond)
		set*_thread_flag([...,] flag);
	else
		clear*_thread_flag([...,] flag);

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Alex Benn?e <alex.bennee@linaro.org>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Oleg Nesterov <oleg@redhat.com>
---
 include/linux/sched.h       |  6 ++++++
 include/linux/thread_info.h | 11 +++++++++++
 2 files changed, 17 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index b3d697f..c2c3051 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1578,6 +1578,12 @@ static inline void clear_tsk_thread_flag(struct task_struct *tsk, int flag)
 	clear_ti_thread_flag(task_thread_info(tsk), flag);
 }
 
+static inline void update_tsk_thread_flag(struct task_struct *tsk, int flag,
+					  bool value)
+{
+	update_ti_thread_flag(task_thread_info(tsk), flag, value);
+}
+
 static inline int test_and_set_tsk_thread_flag(struct task_struct *tsk, int flag)
 {
 	return test_and_set_ti_thread_flag(task_thread_info(tsk), flag);
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index cf2862b..8d8821b 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -60,6 +60,15 @@ static inline void clear_ti_thread_flag(struct thread_info *ti, int flag)
 	clear_bit(flag, (unsigned long *)&ti->flags);
 }
 
+static inline void update_ti_thread_flag(struct thread_info *ti, int flag,
+					 bool value)
+{
+	if (value)
+		set_ti_thread_flag(ti, flag);
+	else
+		clear_ti_thread_flag(ti, flag);
+}
+
 static inline int test_and_set_ti_thread_flag(struct thread_info *ti, int flag)
 {
 	return test_and_set_bit(flag, (unsigned long *)&ti->flags);
@@ -79,6 +88,8 @@ static inline int test_ti_thread_flag(struct thread_info *ti, int flag)
 	set_ti_thread_flag(current_thread_info(), flag)
 #define clear_thread_flag(flag) \
 	clear_ti_thread_flag(current_thread_info(), flag)
+#define update_thread_flag(flag, value) \
+	update_ti_thread_flag(current_thread_info(), flag, value)
 #define test_and_set_thread_flag(flag) \
 	test_and_set_ti_thread_flag(current_thread_info(), flag)
 #define test_and_clear_thread_flag(flag) \
-- 
2.1.4

^ permalink raw reply related

* [PATCH v11 03/19] arm64: Use update{,_tsk}_thread_flag()
From: Dave Martin @ 2018-05-24 16:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527181008-13549-1-git-send-email-Dave.Martin@arm.com>

This patch uses the new update_thread_flag() helpers to simplify a
couple of if () set; else clear; constructs.

No functional change.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Alex Benn?e <alex.bennee@linaro.org>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/fpsimd.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 12e1c96..9d85373 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -618,10 +618,8 @@ int sve_set_vector_length(struct task_struct *task,
 	task->thread.sve_vl = vl;
 
 out:
-	if (flags & PR_SVE_VL_INHERIT)
-		set_tsk_thread_flag(task, TIF_SVE_VL_INHERIT);
-	else
-		clear_tsk_thread_flag(task, TIF_SVE_VL_INHERIT);
+	update_tsk_thread_flag(task, TIF_SVE_VL_INHERIT,
+			       flags & PR_SVE_VL_INHERIT);
 
 	return 0;
 }
@@ -910,12 +908,12 @@ void fpsimd_thread_switch(struct task_struct *next)
 		 * the TIF_FOREIGN_FPSTATE flag so the state will be loaded
 		 * upon the next return to userland.
 		 */
-		if (__this_cpu_read(fpsimd_last_state.st) ==
-			&next->thread.uw.fpsimd_state
-		    && next->thread.fpsimd_cpu == smp_processor_id())
-			clear_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE);
-		else
-			set_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE);
+		bool wrong_task = __this_cpu_read(fpsimd_last_state.st) !=
+					&next->thread.uw.fpsimd_state;
+		bool wrong_cpu = next->thread.fpsimd_cpu != smp_processor_id();
+
+		update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE,
+				       wrong_task || wrong_cpu);
 	}
 }
 
-- 
2.1.4

^ permalink raw reply related

* [PATCH v11 04/19] KVM: arm/arm64: Introduce kvm_arch_vcpu_run_pid_change
From: Dave Martin @ 2018-05-24 16:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527181008-13549-1-git-send-email-Dave.Martin@arm.com>

From: Christoffer Dall <christoffer.dall@linaro.org>

KVM/ARM differs from other architectures in having to maintain an
additional virtual address space from that of the host and the
guest, because we split the execution of KVM across both EL1 and
EL2.

This results in a need to explicitly map data structures into EL2
(hyp) which are accessed from the hyp code.  As we are about to be
more clever with our FPSIMD handling on arm64, which stores data in
the task struct and uses thread_info flags, we will have to map
parts of the currently executing task struct into the EL2 virtual
address space.

However, we don't want to do this on every KVM_RUN, because it is a
fairly expensive operation to walk the page tables, and the common
execution mode is to map a single thread to a VCPU.  By introducing
a hook that architectures can select with
HAVE_KVM_VCPU_RUN_PID_CHANGE, we do not introduce overhead for
other architectures, but have a simple way to only map the data we
need when required for arm64.

This patch introduces the framework only, and wires it up in the
arm/arm64 KVM common code.

No functional change.

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Alex Benn?e <alex.bennee@linaro.org>
---
 include/linux/kvm_host.h | 9 +++++++++
 virt/kvm/Kconfig         | 3 +++
 virt/kvm/kvm_main.c      | 7 ++++++-
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 6930c63..4268ace 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1276,4 +1276,13 @@ static inline long kvm_arch_vcpu_async_ioctl(struct file *filp,
 void kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm,
 		unsigned long start, unsigned long end);
 
+#ifdef CONFIG_HAVE_KVM_VCPU_RUN_PID_CHANGE
+int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu);
+#else
+static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
+{
+	return 0;
+}
+#endif /* CONFIG_HAVE_KVM_VCPU_RUN_PID_CHANGE */
+
 #endif
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index cca7e06..72143cf 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -54,3 +54,6 @@ config HAVE_KVM_IRQ_BYPASS
 
 config HAVE_KVM_VCPU_ASYNC_IOCTL
        bool
+
+config HAVE_KVM_VCPU_RUN_PID_CHANGE
+       bool
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index c7b2e92..c32e240 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2550,8 +2550,13 @@ static long kvm_vcpu_ioctl(struct file *filp,
 		oldpid = rcu_access_pointer(vcpu->pid);
 		if (unlikely(oldpid != current->pids[PIDTYPE_PID].pid)) {
 			/* The thread running this VCPU changed. */
-			struct pid *newpid = get_task_pid(current, PIDTYPE_PID);
+			struct pid *newpid;
 
+			r = kvm_arch_vcpu_run_pid_change(vcpu);
+			if (r)
+				break;
+
+			newpid = get_task_pid(current, PIDTYPE_PID);
 			rcu_assign_pointer(vcpu->pid, newpid);
 			if (oldpid)
 				synchronize_rcu();
-- 
2.1.4

^ permalink raw reply related

* [PATCH v11 05/19] KVM: arm64: Convert lazy FPSIMD context switch trap to C
From: Dave Martin @ 2018-05-24 16:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527181008-13549-1-git-send-email-Dave.Martin@arm.com>

To make the lazy FPSIMD context switch trap code easier to hack on,
this patch converts it to C.

This is not amazingly efficient, but the trap should typically only
be taken once per host context switch.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Alex Benn?e <alex.bennee@linaro.org>
---
 arch/arm64/kvm/hyp/entry.S  | 57 +++++++++++++++++----------------------------
 arch/arm64/kvm/hyp/switch.c | 24 +++++++++++++++++++
 2 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
index e41a161..40f349b 100644
--- a/arch/arm64/kvm/hyp/entry.S
+++ b/arch/arm64/kvm/hyp/entry.S
@@ -172,40 +172,27 @@ ENTRY(__fpsimd_guest_restore)
 	// x1: vcpu
 	// x2-x29,lr: vcpu regs
 	// vcpu x0-x1 on the stack
-	stp	x2, x3, [sp, #-16]!
-	stp	x4, lr, [sp, #-16]!
-
-alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
-	mrs	x2, cptr_el2
-	bic	x2, x2, #CPTR_EL2_TFP
-	msr	cptr_el2, x2
-alternative_else
-	mrs	x2, cpacr_el1
-	orr	x2, x2, #CPACR_EL1_FPEN
-	msr	cpacr_el1, x2
-alternative_endif
-	isb
-
-	mov	x3, x1
-
-	ldr	x0, [x3, #VCPU_HOST_CONTEXT]
-	kern_hyp_va x0
-	add	x0, x0, #CPU_GP_REG_OFFSET(CPU_FP_REGS)
-	bl	__fpsimd_save_state
-
-	add	x2, x3, #VCPU_CONTEXT
-	add	x0, x2, #CPU_GP_REG_OFFSET(CPU_FP_REGS)
-	bl	__fpsimd_restore_state
-
-	// Skip restoring fpexc32 for AArch64 guests
-	mrs	x1, hcr_el2
-	tbnz	x1, #HCR_RW_SHIFT, 1f
-	ldr	x4, [x3, #VCPU_FPEXC32_EL2]
-	msr	fpexc32_el2, x4
-1:
-	ldp	x4, lr, [sp], #16
-	ldp	x2, x3, [sp], #16
-	ldp	x0, x1, [sp], #16
-
+	stp	x2, x3, [sp, #-144]!
+	stp	x4, x5, [sp, #16]
+	stp	x6, x7, [sp, #32]
+	stp	x8, x9, [sp, #48]
+	stp	x10, x11, [sp, #64]
+	stp	x12, x13, [sp, #80]
+	stp	x14, x15, [sp, #96]
+	stp	x16, x17, [sp, #112]
+	stp	x18, lr, [sp, #128]
+
+	bl	__hyp_switch_fpsimd
+
+	ldp	x4, x5, [sp, #16]
+	ldp	x6, x7, [sp, #32]
+	ldp	x8, x9, [sp, #48]
+	ldp	x10, x11, [sp, #64]
+	ldp	x12, x13, [sp, #80]
+	ldp	x14, x15, [sp, #96]
+	ldp	x16, x17, [sp, #112]
+	ldp	x18, lr, [sp, #128]
+	ldp	x0, x1, [sp, #144]
+	ldp	x2, x3, [sp], #160
 	eret
 ENDPROC(__fpsimd_guest_restore)
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index d964523..c0796c4 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -318,6 +318,30 @@ static bool __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
 	}
 }
 
+void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
+				    struct kvm_vcpu *vcpu)
+{
+	kvm_cpu_context_t *host_ctxt;
+
+	if (has_vhe())
+		write_sysreg(read_sysreg(cpacr_el1) | CPACR_EL1_FPEN,
+			     cpacr_el1);
+	else
+		write_sysreg(read_sysreg(cptr_el2) & ~(u64)CPTR_EL2_TFP,
+			     cptr_el2);
+
+	isb();
+
+	host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
+	__fpsimd_save_state(&host_ctxt->gp_regs.fp_regs);
+	__fpsimd_restore_state(&vcpu->arch.ctxt.gp_regs.fp_regs);
+
+	/* Skip restoring fpexc32 for AArch64 guests */
+	if (!(read_sysreg(hcr_el2) & HCR_RW))
+		write_sysreg(vcpu->arch.ctxt.sys_regs[FPEXC32_EL2],
+			     fpexc32_el2);
+}
+
 /*
  * Return true when we were able to fixup the guest exit and should return to
  * the guest, false when we should restore the host state and return to the
-- 
2.1.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox