* [PATCH 0/2] arm64: fatal kernel fault reporting improvements
From: Mark Rutland @ 2018-05-21 13:14 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Catalin noticed that do_page_fault() can die() for some faults without
reporting the faulting address, which is unfortunate. We also don't
decode the ESR, or attempt to dump the page tables.
These patches unify the handling of those faults with the ones we handle
in __do_kernel_fault(), making things a little more consistent.
Thanks,
Mark.
Mark Rutland (2):
arm64: make is_permission_fault() name clearer
arm64: Unify kernel fault reporting
arch/arm64/mm/fault.c | 46 ++++++++++++++++++++++++++++------------------
1 file changed, 28 insertions(+), 18 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH 1/2] arm64: make is_permission_fault() name clearer
From: Mark Rutland @ 2018-05-21 13:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180521131451.41040-1-mark.rutland@arm.com>
The naming of is_permission_fault() makes it sound like it should return
true for permission faults from EL0, but by design, it only does so for
faults from EL1.
Let's make this clear by dropping el1 in the name, as we do for
is_el1_instruction_abort().
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
arch/arm64/mm/fault.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 4165485e8b6e..59990491e72a 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -235,8 +235,9 @@ static bool is_el1_instruction_abort(unsigned int esr)
return ESR_ELx_EC(esr) == ESR_ELx_EC_IABT_CUR;
}
-static inline bool is_permission_fault(unsigned int esr, struct pt_regs *regs,
- unsigned long addr)
+static inline bool is_el1_permission_fault(unsigned int esr,
+ struct pt_regs *regs,
+ unsigned long addr)
{
unsigned int ec = ESR_ELx_EC(esr);
unsigned int fsc_type = esr & ESR_ELx_FSC_TYPE;
@@ -268,7 +269,7 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
bust_spinlocks(1);
- if (is_permission_fault(esr, regs, addr)) {
+ if (is_el1_permission_fault(esr, regs, addr)) {
if (esr & ESR_ELx_WNR)
msg = "write to read-only memory";
else
@@ -395,7 +396,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
mm_flags |= FAULT_FLAG_WRITE;
}
- if (addr < TASK_SIZE && is_permission_fault(esr, regs, addr)) {
+ if (addr < TASK_SIZE && is_el1_permission_fault(esr, regs, addr)) {
/* regs->orig_addr_limit may be 0 if we entered from EL0 */
if (regs->orig_addr_limit == KERNEL_DS)
die("Accessing user space memory with fs=KERNEL_DS", regs, esr);
--
2.11.0
^ permalink raw reply related
* [PATCH 2/2] arm64: Unify kernel fault reporting
From: Mark Rutland @ 2018-05-21 13:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180521131451.41040-1-mark.rutland@arm.com>
In do_page_fault(), we handle some kernel faults early, and simply
die() with a message. For faults handled later, we dump the faulting
address, decode the ESR, walk the page tables, and perform a number of
steps to ensure that this data is reported.
Let's unify the handling of fatal kernel faults with a new
die_kernel_fault() helper, handling all of these details. This is
largely the same as the existing logic in __do_kernel_fault(), except
that addresses are consistently padded to 16 hex characters, as would be
expected for a 64-bit address.
The messages currently logged in do_page_fault are adjusted to fit into
the die_kernel_fault() message template.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
arch/arm64/mm/fault.c | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 59990491e72a..27cbe0b38960 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -255,6 +255,22 @@ static inline bool is_el1_permission_fault(unsigned int esr,
return false;
}
+static void die_kernel_fault(const char *msg, unsigned long addr,
+ unsigned int esr, struct pt_regs *regs)
+{
+ bust_spinlocks(1);
+
+ pr_alert("Unable to handle kernel %s at virtual address %016lx\n", msg,
+ addr);
+
+ mem_abort_decode(esr);
+
+ show_pte(addr);
+ die("Oops", regs, esr);
+ bust_spinlocks(0);
+ do_exit(SIGKILL);
+}
+
static void __do_kernel_fault(unsigned long addr, unsigned int esr,
struct pt_regs *regs)
{
@@ -267,8 +283,6 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
if (!is_el1_instruction_abort(esr) && fixup_exception(regs))
return;
- bust_spinlocks(1);
-
if (is_el1_permission_fault(esr, regs, addr)) {
if (esr & ESR_ELx_WNR)
msg = "write to read-only memory";
@@ -280,15 +294,7 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
msg = "paging request";
}
- pr_alert("Unable to handle kernel %s at virtual address %08lx\n", msg,
- addr);
-
- mem_abort_decode(esr);
-
- show_pte(addr);
- die("Oops", regs, esr);
- bust_spinlocks(0);
- do_exit(SIGKILL);
+ die_kernel_fault(msg, addr, esr, regs);
}
static void __do_user_fault(struct siginfo *info, unsigned int esr)
@@ -399,13 +405,16 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
if (addr < TASK_SIZE && is_el1_permission_fault(esr, regs, addr)) {
/* regs->orig_addr_limit may be 0 if we entered from EL0 */
if (regs->orig_addr_limit == KERNEL_DS)
- die("Accessing user space memory with fs=KERNEL_DS", regs, esr);
+ die_kernel_fault("access to user memory with fs=KERNEL_DS",
+ addr, esr, regs);
if (is_el1_instruction_abort(esr))
- die("Attempting to execute userspace memory", regs, esr);
+ die_kernel_fault("execution of user memory",
+ addr, esr, regs);
if (!search_exception_tables(regs->pc))
- die("Accessing user space memory outside uaccess.h routines", regs, esr);
+ die_kernel_fault("access to user memory outside uaccess routines",
+ addr, esr, regs);
}
perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, addr);
--
2.11.0
^ permalink raw reply related
* [PATCH v9 01/15] soc: qcom: Separate kryo l2 accessors from PMU driver
From: Will Deacon @ 2018-05-21 13:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526901932-9514-2-git-send-email-ilialin@codeaurora.org>
On Mon, May 21, 2018 at 02:25:18PM +0300, Ilia Lin wrote:
> The driver provides kernel level API for other drivers
> to access the MSM8996 L2 cache registers.
> Separating the L2 access code from the PMU driver and
> making it public to allow other drivers use it.
> The accesses must be separated with a single spinlock,
> maintained in this driver.
>
> Signed-off-by: Ilia Lin <ilialin@codeaurora.org>
> ---
> drivers/perf/Kconfig | 1 +
> drivers/perf/qcom_l2_pmu.c | 90 ++++++++++--------------------------
I'm fine with the perf bits:
Acked-by: Will Deacon <will.deacon@arm.com>
Will
^ permalink raw reply
* [PATCH] PCI: Do not enable PASID when End-to-End TLP is not supported
From: Sinan Kaya @ 2018-05-21 13:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526748769-701-1-git-send-email-okaya@codeaurora.org>
+iommu folks.
On 5/19/2018 12:52 PM, Sinan Kaya wrote:
> A PCIe endpoint carries the process address space identifier (PASID) in
> the TLP prefix as part of the memory read/write transaction. The address
> information in the TLP is relevant only for a given PASID context.
>
> A translation agent takes PASID value and the address information from the
> TLP to look up the physical address in the system.
>
> If a bridge drops the TLP prefix, the translation agent can resolve the
> address to an incorrect location and cause data corruption. Prevent
> this condition by requiring End-to-End TLP prefix to be supported on the
> entire data path between the endpoint and the root port.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/pci/ats.c | 16 ++++++++++++++++
> include/uapi/linux/pci_regs.h | 1 +
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> index 89305b5..0bcded5 100644
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
> @@ -265,7 +265,9 @@ EXPORT_SYMBOL_GPL(pci_reset_pri);
> int pci_enable_pasid(struct pci_dev *pdev, int features)
> {
> u16 control, supported;
> + struct pci_dev *bridge;
> int pos;
> + u32 cap;
>
> if (WARN_ON(pdev->pasid_enabled))
> return -EBUSY;
> @@ -274,6 +276,20 @@ int pci_enable_pasid(struct pci_dev *pdev, int features)
> if (!pos)
> return -EINVAL;
>
> + bridge = pci_upstream_bridge(pdev);
> + while (bridge) {
> + if (!pci_find_capability(bridge, PCI_CAP_ID_EXP))
> + return -EINVAL;
> +
> + if (pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap))
> + return -EINVAL;
> +
> + if (!(cap & PCI_EXP_DEVCAP2_E2ETLP))
> + return -EINVAL;
> +
> + bridge = pci_upstream_bridge(bridge);
> + }
> +
> pci_read_config_word(pdev, pos + PCI_PASID_CAP, &supported);
> supported &= PCI_PASID_CAP_EXEC | PCI_PASID_CAP_PRIV;
>
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 103ba79..d91dea5 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -634,6 +634,7 @@
> #define PCI_EXP_DEVCAP2_OBFF_MASK 0x000c0000 /* OBFF support mechanism */
> #define PCI_EXP_DEVCAP2_OBFF_MSG 0x00040000 /* New message signaling */
> #define PCI_EXP_DEVCAP2_OBFF_WAKE 0x00080000 /* Re-use WAKE# for OBFF */
> +#define PCI_EXP_DEVCAP2_E2ETLP 0x00200000 /* End-to-End TLP Prefix */
> #define PCI_EXP_DEVCTL2 40 /* Device Control 2 */
> #define PCI_EXP_DEVCTL2_COMP_TIMEOUT 0x000f /* Completion Timeout Value */
> #define PCI_EXP_DEVCTL2_COMP_TMOUT_DIS 0x0010 /* Completion Timeout Disable */
>
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH 6/6] arm64: perf: Add support for chaining counters
From: Suzuki K Poulose @ 2018-05-21 13:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5d53fe1d-dbd9-2aad-73bd-0c4284540240@arm.com>
On 18/05/18 16:57, Suzuki K Poulose wrote:
> Hi Robin,
>
> On 18/05/18 14:49, Robin Murphy wrote:
>> On 18/05/18 11:22, Suzuki K Poulose wrote:
>>> Add support for chained event counters. PMUv3 allows chaining
>>> a pair of adjacent PMU counters (with the lower counter number
>>> being always "even"). The low counter is programmed to count
>>> the event of interest and the high counter(odd numbered) is
>>> programmed with a special event code (0x1e - Chain). Thus
>>> we need special allocation schemes to make the full use of
>>> available counters. So, we allocate the counters from either
>>> ends. i.e, chained counters are allocated from the lower
>>> end in pairs of two and the normal counters are allocated
>>> from the higher number. Also makes necessary changes to
>>> handle the chained events as a single event with 2 counters.
>>>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Will Deacon <will.deacon@arm.com>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>
>>> ? /*
>>> @@ -845,8 +1016,14 @@ static int __armv8_pmuv3_map_event(struct perf_event *event,
>>> ???????????????????????? &armv8_pmuv3_perf_cache_map,
>>> ???????????????????????? ARMV8_PMU_EVTYPE_EVENT);
>>> -??? if (hw_event_id == ARMV8_PMUV3_PERFCTR_CPU_CYCLES)
>>> +??? if (hw_event_id == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) {
>>> +??????? /* Prevent chaining for cycle counter */
>>
>> Why? Sure, we want to avoid executing the chaining logic if we're scheduling a cycles event in the dedicated counter (which is perhaps what the comment above wanted to say), but if one ends up allocated into a regular counter (e.g. if the user asks for multiple cycle counts with different filters), then I don't see any reason to forbid that being chained.
>
> Ah, I didn't think about that case. I was under the assumption that the
> cycles are *only* placed on the cycle counter. I will take care of that.
> Thanks for the review.
Robin, Mark, Will
One potential problem I see with allowing chaining of the cycle counter
*and* the promotion of cycle event to 64bit by default is when the user
may actually be able to count 1 less event (due to the promotion of
cycle event to 64bit and thus forcing to use chain, if the cycle counter
is unavailable).
So one option is to drop automatic promotion of the cycle counter to
64bit and do it only when it is requested by the user and use either the
Cycle counter (preferred) or fall back to chaining. That way, the user
has the control over the number of events he can count using the given
set of counters.
Let me know your thoughts.
Suzuki
^ permalink raw reply
* [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
From: Gilad Ben-Yossef @ 2018-05-21 13:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdVM55S7+PdnKusX-qkTxioc=0kQ6EGSbwv+kbLk5RCUYw@mail.gmail.com>
On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c
> does not distinguish between the absence of the clock property, and an
> actual error in getting the clock, and never considers any error a failure
> (incl. -PROBE_DEFER).
>
> As of_clk_get() returns -ENOENT for both a missing clock property and a
> missing clock, you should use (devm_)clk_get() instead, and distinguish
> between NULL (no clock property) and IS_ERR() (actual failure -> abort).
>
I was trying to do as you suggested but I didn't quite get what is the
dev_id (2nd) parameter to devm_clk_get parameter is supposed to be.
I see what of_clk_get() is doing, so can replicate that but it seems
an over kill.
Any ideas?
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply
* [PATCH v6 3/9] docs: Add Generic Counter interface documentation
From: William Breathitt Gray @ 2018-05-21 13:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180520163109.22b11af8@archlinux>
On Sun, May 20, 2018 at 04:31:09PM +0100, Jonathan Cameron wrote:
>On Wed, 16 May 2018 13:51:06 -0400
>William Breathitt Gray <vilhelm.gray@gmail.com> wrote:
>
>> This patch adds high-level documentation about the Generic Counter
>> interface.
>>
>> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
>
>Various comments inline. I've been doing a lot long reviews recently
>(outside of the kernel world) and keep discovering the old rule that
>everytime you read a document you'll find something else to
>improve :(
>
>Jonathan
But it is good too to have multiple eyes on this -- I have found as an
author my brain tends to skip over minor errors while rereading
passages, so having persons reading it for both the first and subsequent
times helps catch those mistakes I may have overlooked in my mind. :)
>> ---
>> Documentation/driver-api/generic-counter.rst | 336 +++++++++++++++++++
>> Documentation/driver-api/index.rst | 1 +
>> MAINTAINERS | 1 +
>> 3 files changed, 338 insertions(+)
>> create mode 100644 Documentation/driver-api/generic-counter.rst
>>
>> diff --git a/Documentation/driver-api/generic-counter.rst b/Documentation/driver-api/generic-counter.rst
>> new file mode 100644
>> index 000000000000..5c6b9c008c06
>> --- /dev/null
>> +++ b/Documentation/driver-api/generic-counter.rst
>> @@ -0,0 +1,336 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +=========================
>> +Generic Counter Interface
>> +=========================
>> +
>> +Introduction
>> +============
>> +
>> +Counter devices are prevalent within a diverse spectrum of industries.
>> +The ubiquitous presence of these devices necessitates a common interface
>> +and standard of interaction and exposure. This driver API attempts to
>> +resolve the issue of duplicate code found among existing counter device
>> +drivers by introducing a generic counter interface for consumption. The
>> +Generic Counter interface enables drivers to support and expose a common
>> +set of components and functionality present in counter devices.
>> +
>> +Theory
>> +======
>> +
>> +Counter devices can vary greatly in design, but regardless of whether
>> +some devices are quadrature encoder counters or tally counters, all
>> +counter devices consist of a core set of components. This core set of
>> +components, shared by all counter devices, is what forms the essence of
>> +the Generic Counter interface.
>> +
>> +There are three core components to a counter:
>
>Enumerate them here. If people are reading this as a paged document (pdf etc)
>then the list of 3 as titles of next few sections may not be clear.
>
>* Count
>
>* Signal
>
>* Synapse
>
>> +
>> +COUNT
>> +-----
>> +A Count represents the count data for a set of Signals. The Generic
>> +Counter interface provides the following available count data types:
>> +
>> +* COUNT_POSITION_UNSIGNED:
>> + Unsigned integer value representing position.
>> +
>> +* COUNT_POSITION_SIGNED:
>> + Signed integer value representing position.
>
>Just a thought: As the '0' position is effectively arbitrary is there any
>actual difference between signed and unsigned? If we defined all counters
>to be unsigned and just offset any signed ones so the range still fitted
>would we end up with a simpler interface - there would be no real loss
>of meaning that I can see.. I suppose it might not be what people expect
>though when they see their counters start at a large positive value...
This is something I've been on the fence about for a while. I would
actually prefer the interface to have simply a COUNT_POSITION data type
to represent position and leave it as unsigned; distinguishing between
signed and unsigned position seems ultimately arbitrary given that it's
mathematically just an offset as you have pointed out.
If we were to go down this route, then we'd have a count value that may
always be represented using an unsigned data type, with an offset value
that may always be represented using a signed data type -- the
relationship being such: position = count + offset. Is that correct?
My reason for giving the option for either signed or unsigned position
was to help minimize the work userspace would have to do in order to get
the value in which they're actually interested. I suppose it was a
question of how abstract I want to make the interface -- although,
making it simpler for userspace put more of a burden on the kernel side.
However, the "offset" value route may actually be more robust in the end
because userspace would have to know whether they want a signed or
unsigned position regardless in order to parse, so with count and offet
defined we know they will always be unsigned and signed respectively.
>
>
>
>
>> +
>> +A Count has a count function mode which represents the update behavior
>> +for the count data. The Generic Counter interface provides the following
>> +available count function modes:
>> +
>> +* Increase:
>> + Accumulated count is incremented.
>> +
>> +* Decrease:
>> + Accumulated count is decremented.
>> +
>> +* Pulse-Direction:
>> + Rising edges on quadrature pair signal A updates the respective count.
>> + The input level of quadrature pair signal B determines direction.
>> +
>Perhaps group the quadrature modes for the point of view of this document?
>Might be slightly easier to read?
>
>* Quadrature Modes
>
> - x1 A: etc.
>
>> +* Quadrature x1 A:
>> + If direction is forward, rising edges on quadrature pair signal A
>> + updates the respective count; if the direction is backward, falling
>> + edges on quadrature pair signal A updates the respective count.
>> + Quadrature encoding determines the direction.
>> +
>> +* Quadrature x1 B:
>> + If direction is forward, rising edges on quadrature pair signal B
>> + updates the respective count; if the direction is backward, falling
>> + edges on quadrature pair signal B updates the respective count.
>> + Quadrature encoding determines the direction.
>> +
>> +* Quadrature x2 A:
>> + Any state transition on quadrature pair signal A updates the
>> + respective count. Quadrature encoding determines the direction.
>> +
>> +* Quadrature x2 B:
>> + Any state transition on quadrature pair signal B updates the
>> + respective count. Quadrature encoding determines the direction.
>> +
>> +* Quadrature x2 Rising:
>> + Rising edges on either quadrature pair signals updates the respective
>> + count. Quadrature encoding determines the direction.
>
>This one I've never met. Really? There are devices who do this form
>of crazy? It gives really uneven counting and I'm failing to see when
>it would ever make sense... References for these odd corner cases
>would be good.
>
>
>__|---|____|-----|____
>____|----|____|-----|____
>
>001122222223334444444
That's the same reaction I had when I discovered this -- in fact the
STM32 LP Timer is the first time I've come across such a quadrature
mode. I'm not sure of the use case for this mode, because positioning
wouldn't be precise as you've pointed out. Perhaps Fabrice or Benjamin
can probe the ST guys responsible for this design choice to figure out
the rationale.
I'm leaving in these modes for now, as they do exist in the STM32 LP
Timer, but it does make me curious what the intentions for them were
(perhaps use cases outside of traditional quadrature encoder
positioning).
>
>
>> +
>> +* Quadrature x2 Falling:
>> + Falling edges on either quadrature pair signals updates the respective
>> + count. Quadrature encoding determines the direction.
>> +
>> +* Quadrature x4:
>> + Any state transition on either quadrature pair signals updates the
>> + respective count. Quadrature encoding determines the direction.
>> +
>> +A Count has a set of one or more associated Signals.
>> +
>> +SIGNAL
>> +------
>> +A Signal represents a counter input data; this is the input data that is
>> +analyzed by the counter to determine the count data; e.g. a quadrature
>> +signal output line of a rotary encoder. Not all counter devices provide
>> +user access to the Signal data.
>> +
>> +The Generic Counter interface provides the following available signal
>> +data types for when the Signal data is available for user access:
>> +
>> +* SIGNAL_LEVEL_LOW:
>> + Signal line is in a low state.
>> +
>> +* SIGNAL_LEVEL_HIGH:
>> + Signal line is in a high state.
>> +
>> +A Signal may be associated with one or more Counts.
>> +
>> +SYNAPSE
>> +-------
>> +A Synapse represents the association of a Signal with a respective
>> +Count. Signal data affects respective Count data, and the Synapse
>> +represents this relationship.
>> +
>> +The Synapse action mode specifies the Signal data condition which
>> +triggers the respective Count's count function evaluation to update the
>> +count data. The Generic Counter interface provides the following
>> +available action modes:
>> +
>> +* None:
>> + Signal does not trigger the count function. In Pulse-Direction count
>> + function mode, this Signal is evaluated as Direction.
>> +
>> +* Rising Edge:
>> + Low state transitions to high state.
>> +
>> +* Falling Edge:
>> + High state transitions to low state.
>> +
>> +* Both Edges:
>> + Any state transition.
>> +
>> +A counter is defined as a set of input signals associated with count
>> +data that are generated by the evaluation of the state of the associated
>> +input signals as defined by the respective count functions. Within the
>> +context of the Generic Counter interface, a counter consists of Counts
>> +each associated with a set of Signals, whose respective Synapse
>> +instances represent the count function update conditions for the
>> +associated Counts.
>> +
>> +Paradigm
>> +========
>> +
>> +The most basic counter device may be expressed as a single Count
>> +associated with a single Signal via a single Synapse. Take for example
>> +a counter device which simply accumulates a count of rising edges on a
>> +source input line::
>> +
>> + Count Synapse Signal
>> + ----- ------- ------
>> + +---------------------+
>> + | Data: Count | Rising Edge ________
>> + | Function: Increase | <------------- / Source \
>> + | | ____________
>> + +---------------------+
>> +
>> +In this example, the Signal is a source input line with a pulsing
>> +voltage, while the Count is a persistent count value which is repeatedly
>> +incremented. The Signal is associated with the respective Count via a
>> +Synapse. The increase function is triggered by the Signal data condition
>> +specified by the Synapse -- in this case a rising edge condition on the
>> +voltage input line. In summary, the counter device existence and
>> +behavior is aptly represented by respective Count, Signal, and Synapse
>> +components: a rising edge condition triggers an increase function on an
>> +accumulating count datum.
>> +
>> +A counter device is not limited to a single Signal; in fact, in theory
>> +many Signals may be associated with even a single Count. For example, a
>> +quadrature encoder counter device can keep track of position based on
>> +the states of two input lines::
>> +
>> + Count Synapse Signal
>> + ----- ------- ------
>> + +-------------------------+
>> + | Data: Position | Both Edges ___
>> + | Function: Quadrature x4 | <------------ / A \
>> + | | _______
>> + | |
>> + | | Both Edges ___
>> + | | <------------ / B \
>> + | | _______
>> + +-------------------------+
>> +
>> +In this example, two Signals (quadrature encoder lines A and B) are
>> +associated with a single Count: a rising or falling edge on either A or
>> +B triggers the "Quadrature x4" function which determines the direction
>> +of movement and updates the respective position data. The "Quadrature
>> +x4" function is likely implemented in the hardware of the quadrature
>> +encoder counter device; the Count, Signals, and Synapses simply
>> +represent this hardware behavior and functionality.
>> +
>> +Signals associated with the same Count can have differing Synapse action
>> +mode conditions. For example, a quadrature encoder counter device
>> +operating in a non-quadrature Pulse-Direction mode could have one input
>> +line dedicated for movement and a second input line dedicated for
>> +direction::
>> +
>> + Count Synapse Signal
>> + ----- ------- ------
>> + +---------------------------+
>> + | Data: Position | Rising Edge ___
>> + | Function: Pulse-Direction | <------------- / A \ (Movement)
>> + | | _______
>> + | |
>> + | | None ___
>> + | | <------------- / B \ (Direction)
>> + | | _______
>> + +---------------------------+
>> +
>> +Only Signal A triggers the "Pulse-Direction" update function, but the
>> +instantaneous state of Signal B is still required in order to know the
>> +direction so that the position data may be properly updated. Ultimately,
>> +both Signals are associated with the same Count via two respective
>> +Synapses, but only one Synapse has an active action mode condition which
>> +triggers the respective count function while the other is left with a
>> +"None" condition action mode to indicate its respective Signal's
>> +availability for state evaluation despite its non-triggering mode.
>> +
>> +Keep in mind that the Signal, Synapse, and Count are abstract
>> +representations which do not need to be closely married to their
>> +respective physical sources. This allows the user of a counter to
>> +divorce themselves from the nuances of physical components (such as
>> +whether an input line is differential or single-ended) and instead focus
>> +on the core idea of what the data and process represent (e.g. position
>> +as interpreted from quadrature encoding data).
>> +
>> +Userspace Interface
>> +===================
>> +
>> +Several sysfs attributes are generated by the Generic Counter interface,
>> +and reside under the /sys/bus/counter/devices/counterX directory, where
>> +counterX refers to the respective counter device. Please see
>> +Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed
>> +information on each Generic Counter interface sysfs attribute.
>> +
>> +Through these sysfs attributes, programs and scripts may interact with
>> +the Generic Counter paradigm Counts, Signals, and Synapses of respective
>> +counter devices.
>> +
>> +Driver API
>> +==========
>> +
>> +Driver authors may utilize the Generic Counter interface in their code
>> +by including the include/linux/iio/counter.h header file. This header
>
>Didn't this move?
Yes you are correct, looks like an oversight I made. I'll cleanup this
and the rest with the next revision then.
William Breathitt Gray
>
>> +file provides several core data structures, function prototypes, and
>> +macros for defining a counter device.
>> +
>> +.. kernel-doc:: include/linux/counter.h
>> + :internal:
>> +
>> +.. kernel-doc:: drivers/counter/generic-counter.c
>> + :export:
>> +
>> +Implementation
>> +==============
>> +
>> +To support a counter device, a driver must first allocate the available
>> +Counter Signals via counter_signal structures. These Signals should
>> +be stored as an array and set to the signals array member of an
>> +allocated counter_device structure before the Counter is registered to
>> +the system.
>> +
>> +Counter Counts may be allocated via counter_count structures, and
>> +respective Counter Signal associations (Synapses) made via
>> +counter_synapse structures. Associated counter_synapse structures are
>> +stored as an array and set to the the synapses array member of the
>> +respective counter_count structure. These counter_count structures are
>> +set to the counts array member of an allocated counter_device structure
>> +before the Counter is registered to the system.
>> +
>> +Driver callbacks should be provided to the counter_device structure via
>> +a constant counter_ops structure in order to communicate with the
>> +device: to read and write various Signals and Counts, and to set and get
>> +the "action mode" and "function mode" for various Synapses and Counts
>> +respectively.
>> +
>> +A defined counter_device structure may be registered to the system by
>> +passing it to the counter_register function, and unregistered by passing
>> +it to the counter_unregister function. Similarly, the
>> +devm_counter_register and devm_counter_unregister functions may be used
>> +if device memory-managed registration is desired.
>> +
>> +Extension sysfs attributes can be created for auxiliary functionality
>> +and data by passing in defined counter_device_ext, counter_count_ext,
>> +and counter_signal_ext structures. In these cases, the
>> +counter_device_ext structure is used for global configuration of the
>> +respective Counter device, while the counter_count_ext and
>> +counter_signal_ext structures allow for auxiliary exposure and
>> +configuration of a specific Count or Signal respectively.
>> +
>> +Architecture
>> +============
>> +
>> +When the Generic Counter interface counter module is loaded, the
>> +counter_init function is called which registers a bus_type named
>> +"counter" to the system. Subsequently, when the module is unloaded, the
>> +counter_exit function is called which unregisters the bus_type named
>> +"counter" from the system.
>> +
>> +Counter devices are registered to the system via the counter_register
>> +function, and later removed via the counter_unregister function. The
>> +counter_register function establishes a unique ID for the Counter
>> +device and creates a respective sysfs directory, where X is the
>> +mentioned unique ID:
>> +
>> + /sys/bus/counter/devices/counterX
>> +
>> +Sysfs attributes are created within the counterX directory to expose
>> +functionality, configurations, and data relating to the Counts, Signals,
>> +and Synapses of the Counter device, as well as options and information
>> +for the Counter device itself.
>> +
>> +Each Signal has a directory created to house its relevant sysfs
>> +attributes, where Y is the unique ID of the respective Signal:
>> +
>> + /sys/bus/counter/devices/counterX/signalY
>> +
>> +Similarly, each Count has a directory created to house its relevant
>> +sysfs attributes, where Y is the unique ID of the respective Count:
>> +
>> + /sys/bus/counter/devices/counterX/countY
>> +
>> +For a more detailed breakdown of the available Generic Counter interface
>> +sysfs attributes, please refer to the
>> +Documentation/ABI/testing/sys-bus-counter file.
>> +
>> +The Signals and Counts associated with the Counter device are registered
>> +to the system as well by the counter_register function. The
>> +signal_read/signal_write driver callbacks are associated with their
>> +respective Signal attributes, while the count_read/count_write and
>> +function_get/function_set driver callbacks are associated with their
>> +respective Count attributes; similarly, the same is true for the
>> +action_get/action_set driver callbacks and their respective Synapse
>> +attributes. If a driver callback is left undefined, then the respective
>> +read/write permission is left disabled for the relevant attributes.
>> +
>> +Similarly, extension sysfs attributes are created for the defined
>> +counter_device_ext, counter_count_ext, and counter_signal_ext
>> +structures that are passed in.
>> diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
>> index 6d8352c0f354..5fd747c4f2ce 100644
>> --- a/Documentation/driver-api/index.rst
>> +++ b/Documentation/driver-api/index.rst
>> @@ -25,6 +25,7 @@ available subsections can be seen below.
>> frame-buffer
>> regulator
>> iio/index
>> + generic-counter
>> input
>> usb/index
>> pci
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 1413e3eb49e5..7a01aa63fb33 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -3674,6 +3674,7 @@ M: William Breathitt Gray <vilhelm.gray@gmail.com>
>> L: linux-iio at vger.kernel.org
>> S: Maintained
>> F: Documentation/ABI/testing/sysfs-bus-counter*
>> +F: Documentation/driver-api/generic-counter.rst
>> F: drivers/counter/
>> F: include/linux/counter.h
>>
>
^ permalink raw reply
* [PATCH] EDAC, ghes: Make platform-based whitelisting x86-only
From: Tyler Baicar @ 2018-05-21 13:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518112028.GD17285@pd.tnic>
On 5/18/2018 7:20 AM, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
>
> ARM machines all have DMI tables so if they request hw error reporting
> through GHES, then the driver should be able to detect DIMMs and report
> errors successfully (famous last words :)).
>
> Make the platform-based list x86-specific so that ghes_edac can load on
> ARM.
>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Reviewed-by: James Morse <james.morse@arm.com>
> Tested-by: James Morse <james.morse@arm.com>
> Cc: Qiang Zheng <zhengqiang10@huawei.com>
Tested-by: Tyler Baicar <tbaicar@codeaurora.org>
Was it intentional to make idx=0 so that the pr_info later in this function no
longer hits?
I don't see an issue with not printing out the long BIOS statement, but the
number of DIMM sockets print could still be useful.
Thanks,
Tyler
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH v6 3/5] soc: rockchip: split rockchip_typec_phy struct to separate header
From: Enric Balletbo Serra @ 2018-05-21 13:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526895424-22894-3-git-send-email-hl@rock-chips.com>
Hi Lin,
2018-05-21 11:37 GMT+02:00 Lin Huang <hl@rock-chips.com>:
> we may use rockchip_phy_typec struct in other driver, so split
> it to separate header.
>
That patch does more than just split some structs to a public header,
it also introduces new structs and new parameters related to the
phy_config feature. IMHO you should first move the current structs and
introduce the new phy_config stuff in the following patch (4/5). I am
not sure about the maintainer preferences, but at least, if the
maintainer is fine like is now, I'd explain that you introduce new
elements in the commit message.
Best regards,
Enric
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
> Changes in v2:
> - None
> Changes in v3:
> - None
> Changes in v4:
> - None
> Changes in v5:
> - None
> Changes in v6:
> - new patch here
>
> drivers/phy/rockchip/phy-rockchip-typec.c | 47 +----------------------
> include/soc/rockchip/rockchip_phy_typec.h | 63 +++++++++++++++++++++++++++++++
> 2 files changed, 64 insertions(+), 46 deletions(-)
> create mode 100644 include/soc/rockchip/rockchip_phy_typec.h
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
> index 76a4b58..795055f 100644
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
> @@ -63,6 +63,7 @@
>
> #include <linux/mfd/syscon.h>
> #include <linux/phy/phy.h>
> +#include <soc/rockchip/rockchip_phy_typec.h>
>
> #define CMN_SSM_BANDGAP (0x21 << 2)
> #define CMN_SSM_BIAS (0x22 << 2)
> @@ -349,52 +350,6 @@
> #define MODE_DFP_USB BIT(1)
> #define MODE_DFP_DP BIT(2)
>
> -struct usb3phy_reg {
> - u32 offset;
> - u32 enable_bit;
> - u32 write_enable;
> -};
> -
> -/**
> - * struct rockchip_usb3phy_port_cfg: usb3-phy port configuration.
> - * @reg: the base address for usb3-phy config.
> - * @typec_conn_dir: the register of type-c connector direction.
> - * @usb3tousb2_en: the register of type-c force usb2 to usb2 enable.
> - * @external_psm: the register of type-c phy external psm clock.
> - * @pipe_status: the register of type-c phy pipe status.
> - * @usb3_host_disable: the register of type-c usb3 host disable.
> - * @usb3_host_port: the register of type-c usb3 host port.
> - * @uphy_dp_sel: the register of type-c phy DP select control.
> - */
> -struct rockchip_usb3phy_port_cfg {
> - unsigned int reg;
> - struct usb3phy_reg typec_conn_dir;
> - struct usb3phy_reg usb3tousb2_en;
> - struct usb3phy_reg external_psm;
> - struct usb3phy_reg pipe_status;
> - struct usb3phy_reg usb3_host_disable;
> - struct usb3phy_reg usb3_host_port;
> - struct usb3phy_reg uphy_dp_sel;
> -};
> -
> -struct rockchip_typec_phy {
> - struct device *dev;
> - void __iomem *base;
> - struct extcon_dev *extcon;
> - struct regmap *grf_regs;
> - struct clk *clk_core;
> - struct clk *clk_ref;
> - struct reset_control *uphy_rst;
> - struct reset_control *pipe_rst;
> - struct reset_control *tcphy_rst;
> - const struct rockchip_usb3phy_port_cfg *port_cfgs;
> - /* mutex to protect access to individual PHYs */
> - struct mutex lock;
> -
> - bool flip;
> - u8 mode;
> -};
> -
> struct phy_reg {
> u16 value;
> u32 addr;
> diff --git a/include/soc/rockchip/rockchip_phy_typec.h b/include/soc/rockchip/rockchip_phy_typec.h
> new file mode 100644
> index 0000000..be6af0e
> --- /dev/null
> +++ b/include/soc/rockchip/rockchip_phy_typec.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
> + * Author: Lin Huang <hl@rock-chips.com>
> + */
> +
> +#ifndef __SOC_ROCKCHIP_PHY_TYPEC_H
> +#define __SOC_ROCKCHIP_PHY_TYPEC_H
> +
> +struct usb3phy_reg {
> + u32 offset;
> + u32 enable_bit;
> + u32 write_enable;
> +};
> +
> +/**
> + * struct rockchip_usb3phy_port_cfg: usb3-phy port configuration.
> + * @reg: the base address for usb3-phy config.
> + * @typec_conn_dir: the register of type-c connector direction.
> + * @usb3tousb2_en: the register of type-c force usb2 to usb2 enable.
> + * @external_psm: the register of type-c phy external psm clock.
> + * @pipe_status: the register of type-c phy pipe status.
> + * @usb3_host_disable: the register of type-c usb3 host disable.
> + * @usb3_host_port: the register of type-c usb3 host port.
> + * @uphy_dp_sel: the register of type-c phy DP select control.
> + */
> +struct rockchip_usb3phy_port_cfg {
> + unsigned int reg;
> + struct usb3phy_reg typec_conn_dir;
> + struct usb3phy_reg usb3tousb2_en;
> + struct usb3phy_reg external_psm;
> + struct usb3phy_reg pipe_status;
> + struct usb3phy_reg usb3_host_disable;
> + struct usb3phy_reg usb3_host_port;
> + struct usb3phy_reg uphy_dp_sel;
> +};
> +
> +struct phy_config {
> + int swing;
> + int pe;
> +};
> +
> +struct rockchip_typec_phy {
> + struct device *dev;
> + void __iomem *base;
> + struct extcon_dev *extcon;
> + struct regmap *grf_regs;
> + struct clk *clk_core;
> + struct clk *clk_ref;
> + struct reset_control *uphy_rst;
> + struct reset_control *pipe_rst;
> + struct reset_control *tcphy_rst;
> + const struct rockchip_usb3phy_port_cfg *port_cfgs;
> + /* mutex to protect access to individual PHYs */
> + struct mutex lock;
> + struct phy_config config[3][4];
> + bool flip;
> + u8 mode;
> + int (*typec_phy_config)(struct phy *phy, int link_rate,
> + int lanes, u8 swing, u8 pre_emp);
> +};
> +
> +#endif
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v6 4/9] counter: 104-quad-8: Add Generic Counter interface support
From: William Breathitt Gray @ 2018-05-21 13:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180520164253.5432d2a4@archlinux>
On Sun, May 20, 2018 at 04:42:53PM +0100, Jonathan Cameron wrote:
>On Wed, 16 May 2018 13:51:25 -0400
>William Breathitt Gray <vilhelm.gray@gmail.com> wrote:
>
>> This patch adds support for the Generic Counter interface to the
>> 104-QUAD-8 driver. The existing 104-QUAD-8 device interface should not
>> be affected by this patch; all changes are intended as supplemental
>> additions as perceived by the user.
>>
>> Generic Counter Counts are created for the eight quadrature channel
>> counts, as well as their respective quadrature A and B Signals (which
>> are associated via respective Synapse structures) and respective index
>> Signals.
>>
>> The new Generic Counter interface sysfs attributes are intended to
>> expose the same functionality and data available via the existing
>> 104-QUAD-8 IIO device interface; the Generic Counter interface serves
>> to provide the respective functionality and data in a standard way
>> expected of counter devices.
>>
>> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
>
>A few general comments that applied just as well to the original driver
>as they do to the modified version.
>
>I wonder if this would be easier to review as two patches.
>Move the driver then add the counter interfaces?
>
>Right now people kind of have to review the old IIO driver and
>all the new stuff which is a big job..
>
>Jonathan
This looks like more simple cleanup, so I expect to incorporate your
suggestions without problem here as well. I'll also try to make the code
easier to read by writing some defines for the magic numbers throughout.
William Breathitt Gray
>> ---
>> MAINTAINERS | 4 +-
>> drivers/counter/104-quad-8.c | 1335 ++++++++++++++++++++++++++++++
>> drivers/counter/Kconfig | 21 +
>> drivers/counter/Makefile | 2 +
>> drivers/iio/counter/104-quad-8.c | 596 -------------
>> drivers/iio/counter/Kconfig | 17 -
>> drivers/iio/counter/Makefile | 1 -
>> 7 files changed, 1360 insertions(+), 616 deletions(-)
>> create mode 100644 drivers/counter/104-quad-8.c
>> delete mode 100644 drivers/iio/counter/104-quad-8.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 7a01aa63fb33..f11bf7885aeb 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -266,12 +266,12 @@ L: linux-gpio at vger.kernel.org
>> S: Maintained
>> F: drivers/gpio/gpio-104-idio-16.c
>>
>> -ACCES 104-QUAD-8 IIO DRIVER
>> +ACCES 104-QUAD-8 DRIVER
>> M: William Breathitt Gray <vilhelm.gray@gmail.com>
>> L: linux-iio at vger.kernel.org
>> S: Maintained
>> F: Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8
>> -F: drivers/iio/counter/104-quad-8.c
>> +F: drivers/counter/104-quad-8.c
>>
>> ACCES PCI-IDIO-16 GPIO DRIVER
>> M: William Breathitt Gray <vilhelm.gray@gmail.com>
>> diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c
>> new file mode 100644
>> index 000000000000..7c72fb72d660
>> --- /dev/null
>> +++ b/drivers/counter/104-quad-8.c
>> @@ -0,0 +1,1335 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>
>If you are happy with SPDX drop the GPL text below to keep things
>short.
>
>> +/*
>> + * IIO driver for the ACCES 104-QUAD-8
>> + * Copyright (C) 2016 William Breathitt Gray
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License, version 2, as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + * General Public License for more details.
>> + *
>> + * This driver supports the ACCES 104-QUAD-8 and ACCES 104-QUAD-4.
>> + */
>> +#include <linux/bitops.h>
>...
>> +static int quad8_probe(struct device *dev, unsigned int id)
>> +{
>> + struct iio_dev *indio_dev;
>> + struct quad8_iio *quad8iio;
>> + int i, j;
>> + unsigned int base_offset;
>> + int err;
>> +
>> + if (!devm_request_region(dev, base[id], QUAD8_EXTENT, dev_name(dev))) {
>> + dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n",
>> + base[id], base[id] + QUAD8_EXTENT);
>> + return -EBUSY;
>> + }
>> +
>> + /* Allocate IIO device; this also allocates driver data structure */
>> + indio_dev = devm_iio_device_alloc(dev, sizeof(*quad8iio));
>> + if (!indio_dev)
>> + return -ENOMEM;
>> +
>> + /* Initialize IIO device */
>> + indio_dev->info = &quad8_info;
>> + indio_dev->modes = INDIO_DIRECT_MODE;
>> + indio_dev->num_channels = ARRAY_SIZE(quad8_channels);
>> + indio_dev->channels = quad8_channels;
>> + indio_dev->name = dev_name(dev);
>> + indio_dev->dev.parent = dev;
>> +
>> + /* Initialize Counter device and driver data */
>> + quad8iio = iio_priv(indio_dev);
>> + quad8iio->counter.name = dev_name(dev);
>> + quad8iio->counter.parent = dev;
>> + quad8iio->counter.ops = &quad8_ops;
>> + quad8iio->counter.counts = quad8_counts;
>> + quad8iio->counter.num_counts = ARRAY_SIZE(quad8_counts);
>> + quad8iio->counter.signals = quad8_signals;
>> + quad8iio->counter.num_signals = ARRAY_SIZE(quad8_signals);
>> + quad8iio->counter.priv = quad8iio;
>> + quad8iio->base = base[id];
>> +
>> + /* Reset all counters and disable interrupt function */
>> + outb(0x01, base[id] + 0x11);
>> + /* Set initial configuration for all counters */
>> + for (i = 0; i < QUAD8_NUM_COUNTERS; i++) {
>> + base_offset = base[id] + 2 * i;
>> + /* Reset Byte Pointer */
>> + outb(0x01, base_offset + 1);
>
>I'm going to be fussy. There are lots of values
>in here that look like register bits and you could exchange much of
>this documentation for a some good defines...
>
>Taking base_offset + 1 bits 5 and 6 look to select the actual register
>and the rest of them do the control.
>
>Anyhow, not critical but the readability of this code could be improved
>somewhat.
>
>> + /* Reset Preset Register */
>> + for (j = 0; j < 3; j++)
>> + outb(0x00, base_offset);
>> + /* Reset Borrow, Carry, Compare, and Sign flags */
>> + outb(0x04, base_offset + 1);
>> + /* Reset Error flag */
>> + outb(0x06, base_offset + 1);
>> + /* Binary encoding; Normal count; non-quadrature mode */
>> + outb(0x20, base_offset + 1);
>> + /* Disable A and B inputs; preset on index; FLG1 as Carry */
>> + outb(0x40, base_offset + 1);
>> + /* Disable index function; negative index polarity */
>> + outb(0x60, base_offset + 1);
>> + }
>> + /* Enable all counters */
>> + outb(0x00, base[id] + 0x11);
>> +
>> + /* Register IIO device */
>> + err = devm_iio_device_register(dev, indio_dev);
>> + if (err)
>> + return err;
>> +
>> + /* Register Counter device */
>> + return devm_counter_register(dev, &quad8iio->counter);
>> +}
>> +
>> +static struct isa_driver quad8_driver = {
>> + .probe = quad8_probe,
>> + .driver = {
>> + .name = "104-quad-8"
>> + }
>> +};
>> +
>> +module_isa_driver(quad8_driver, num_quad8);
>> +
>> +MODULE_AUTHOR("William Breathitt Gray <vilhelm.gray@gmail.com>");
>> +MODULE_DESCRIPTION("ACCES 104-QUAD-8 IIO driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
>> index 65fa92abd5a4..73f03372484f 100644
>> --- a/drivers/counter/Kconfig
>> +++ b/drivers/counter/Kconfig
>> @@ -16,3 +16,24 @@ menuconfig COUNTER
>> consumption. The Generic Counter interface enables drivers to support
>> and expose a common set of components and functionality present in
>> counter devices.
>> +
>> +if COUNTER
>> +
>> +config 104_QUAD_8
>> + tristate "ACCES 104-QUAD-8 driver"
>> + depends on PC104 && X86 && IIO
>> + select ISA_BUS_API
>> + help
>> + Say yes here to build support for the ACCES 104-QUAD-8 quadrature
>> + encoder counter/interface device family (104-QUAD-8, 104-QUAD-4).
>> +
>> + Performing a write to a counter's IIO_CHAN_INFO_RAW sets the counter and
>> + also clears the counter's respective error flag. Although the counters
>> + have a 25-bit range, only the lower 24 bits may be set, either directly
>> + or via a counter's preset attribute. Interrupts are not supported by
>> + this driver.
>
>This text probably wants to be updated to reflect the new counter subsystem support..
>
>> +
>> + The base port addresses for the devices may be configured via the base
>> + array module parameter.
>> +
>> +endif # COUNTER
>> diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
>> index ad1ba7109cdc..23a4f6263e45 100644
>> --- a/drivers/counter/Makefile
>> +++ b/drivers/counter/Makefile
>> @@ -6,3 +6,5 @@
>>
>> obj-$(CONFIG_COUNTER) += counter.o
>> counter-y := generic-counter.o
>> +
>> +obj-$(CONFIG_104_QUAD_8) += 104-quad-8.o
>> diff --git a/drivers/iio/counter/104-quad-8.c b/drivers/iio/counter/104-quad-8.c
>...
^ permalink raw reply
* [PATCH v6 9/9] iio: counter: Remove IIO counter subdirectory
From: William Breathitt Gray @ 2018-05-21 13:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180520165302.101d37ce@archlinux>
On Sun, May 20, 2018 at 04:53:02PM +0100, Jonathan Cameron wrote:
>On Wed, 16 May 2018 13:52:39 -0400
>William Breathitt Gray <vilhelm.gray@gmail.com> wrote:
>
>> This patch removes the IIO counter subdirectory which is now superceded
>> by the Counter subsystem. Deprecation warnings are added to the
>> documentation of the relevant IIO counter sysfs attributes.
>>
>> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
>
>Please drop the directory when it becomes empty rather than in a later
>patch. IIRC there are some issues with empty Makefiles that will
>make building inbetween tricky.
>
>For the deprecated markings.
>
>Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
I'll have the directory removal occur with the removal of the last
module then when the directory becomes empty.
Regarding the deprecation markings, should I select a specific kernel
version to date the removal of these attributes, or leave the future
date open as this patch is now?
William Breathitt Gray
>
>> ---
>> Documentation/ABI/testing/sysfs-bus-iio | 8 ++++++++
>> .../ABI/testing/sysfs-bus-iio-counter-104-quad-8 | 16 ++++++++++++++++
>> drivers/iio/Kconfig | 1 -
>> drivers/iio/Makefile | 1 -
>> drivers/iio/counter/Kconfig | 8 --------
>> drivers/iio/counter/Makefile | 5 -----
>> 6 files changed, 24 insertions(+), 15 deletions(-)
>> delete mode 100644 drivers/iio/counter/Kconfig
>> delete mode 100644 drivers/iio/counter/Makefile
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
>> index 731146c3b138..6115d97b075e 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>> @@ -1637,6 +1637,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_raw
>> KernelVersion: 4.10
>> Contact: linux-iio at vger.kernel.org
>> Description:
>> + This interface is deprecated; please use the Counter subsystem.
>> +
>> Raw counter device counts from channel Y. For quadrature
>> counters, multiplication by an available [Y]_scale results in
>> the counts of a single quadrature signal phase from channel Y.
>> @@ -1645,6 +1647,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_raw
>> KernelVersion: 4.10
>> Contact: linux-iio at vger.kernel.org
>> Description:
>> + This interface is deprecated; please use the Counter subsystem.
>> +
>> Raw counter device index value from channel Y. This attribute
>> provides an absolute positional reference (e.g. a pulse once per
>> revolution) which may be used to home positional systems as
>> @@ -1654,6 +1658,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_count_count_direction_available
>> KernelVersion: 4.12
>> Contact: linux-iio at vger.kernel.org
>> Description:
>> + This interface is deprecated; please use the Counter subsystem.
>> +
>> A list of possible counting directions which are:
>> - "up" : counter device is increasing.
>> - "down": counter device is decreasing.
>> @@ -1662,4 +1668,6 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_count_direction
>> KernelVersion: 4.12
>> Contact: linux-iio at vger.kernel.org
>> Description:
>> + This interface is deprecated; please use the Counter subsystem.
>> +
>> Raw counter device counters direction for channel Y.
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8 b/Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8
>> index 7fac2c268d9a..bac3d0d48b7b 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8
>> @@ -6,6 +6,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_index_synchronous_mode_available
>> KernelVersion: 4.10
>> Contact: linux-iio at vger.kernel.org
>> Description:
>> + This interface is deprecated; please use the Counter subsystem.
>> +
>> Discrete set of available values for the respective counter
>> configuration are listed in this file.
>>
>> @@ -13,6 +15,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_count_mode
>> KernelVersion: 4.10
>> Contact: linux-iio at vger.kernel.org
>> Description:
>> + This interface is deprecated; please use the Counter subsystem.
>> +
>> Count mode for channel Y. Four count modes are available:
>> normal, range limit, non-recycle, and modulo-n. The preset value
>> for channel Y is used by the count mode where required.
>> @@ -47,6 +51,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_noise_error
>> KernelVersion: 4.10
>> Contact: linux-iio at vger.kernel.org
>> Description:
>> + This interface is deprecated; please use the Counter subsystem.
>> +
>> Read-only attribute that indicates whether excessive noise is
>> present at the channel Y count inputs in quadrature clock mode;
>> irrelevant in non-quadrature clock mode.
>> @@ -55,6 +61,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_preset
>> KernelVersion: 4.10
>> Contact: linux-iio at vger.kernel.org
>> Description:
>> + This interface is deprecated; please use the Counter subsystem.
>> +
>> If the counter device supports preset registers, the preset
>> count for channel Y is provided by this attribute.
>>
>> @@ -62,6 +70,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_quadrature_mode
>> KernelVersion: 4.10
>> Contact: linux-iio at vger.kernel.org
>> Description:
>> + This interface is deprecated; please use the Counter subsystem.
>> +
>> Configure channel Y counter for non-quadrature or quadrature
>> clock mode. Selecting non-quadrature clock mode will disable
>> synchronous load mode. In quadrature clock mode, the channel Y
>> @@ -83,6 +93,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_set_to_preset_on_index
>> KernelVersion: 4.10
>> Contact: linux-iio at vger.kernel.org
>> Description:
>> + This interface is deprecated; please use the Counter subsystem.
>> +
>> Whether to set channel Y counter with channel Y preset value
>> when channel Y index input is active, or continuously count.
>> Valid attribute values are boolean.
>> @@ -91,6 +103,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_index_polarity
>> KernelVersion: 4.10
>> Contact: linux-iio at vger.kernel.org
>> Description:
>> + This interface is deprecated; please use the Counter subsystem.
>> +
>> Active level of channel Y index input; irrelevant in
>> non-synchronous load mode.
>>
>> @@ -98,6 +112,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_synchronous_mode
>> KernelVersion: 4.10
>> Contact: linux-iio at vger.kernel.org
>> Description:
>> + This interface is deprecated; please use the Counter subsystem.
>> +
>> Configure channel Y counter for non-synchronous or synchronous
>> load mode. Synchronous load mode cannot be selected in
>> non-quadrature clock mode.
>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>> index d69e85a8bdc3..1152efad91a1 100644
>> --- a/drivers/iio/Kconfig
>> +++ b/drivers/iio/Kconfig
>> @@ -74,7 +74,6 @@ source "drivers/iio/afe/Kconfig"
>> source "drivers/iio/amplifiers/Kconfig"
>> source "drivers/iio/chemical/Kconfig"
>> source "drivers/iio/common/Kconfig"
>> -source "drivers/iio/counter/Kconfig"
>> source "drivers/iio/dac/Kconfig"
>> source "drivers/iio/dummy/Kconfig"
>> source "drivers/iio/frequency/Kconfig"
>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
>> index d8cba9c229c0..7bdd31f1b88f 100644
>> --- a/drivers/iio/Makefile
>> +++ b/drivers/iio/Makefile
>> @@ -20,7 +20,6 @@ obj-y += amplifiers/
>> obj-y += buffer/
>> obj-y += chemical/
>> obj-y += common/
>> -obj-y += counter/
>> obj-y += dac/
>> obj-y += dummy/
>> obj-y += gyro/
>> diff --git a/drivers/iio/counter/Kconfig b/drivers/iio/counter/Kconfig
>> deleted file mode 100644
>> index 95a7a0df6cac..000000000000
>> --- a/drivers/iio/counter/Kconfig
>> +++ /dev/null
>> @@ -1,8 +0,0 @@
>> -#
>> -# Counter devices
>> -#
>> -# When adding new entries keep the list in alphabetical order
>> -
>> -menu "Counters"
>> -
>> -endmenu
>> diff --git a/drivers/iio/counter/Makefile b/drivers/iio/counter/Makefile
>> deleted file mode 100644
>> index 8fd3d954775a..000000000000
>> --- a/drivers/iio/counter/Makefile
>> +++ /dev/null
>> @@ -1,5 +0,0 @@
>> -#
>> -# Makefile for IIO counter devices
>> -#
>> -
>> -# When adding new entries keep the list in alphabetical order
>
^ permalink raw reply
* [PATCH 6/6] arm64: perf: Add support for chaining counters
From: Robin Murphy @ 2018-05-21 14:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c8cb9280-c93d-89c4-43ed-179de8d46dfd@arm.com>
On 21/05/18 14:42, Suzuki K Poulose wrote:
> On 18/05/18 16:57, Suzuki K Poulose wrote:
>> Hi Robin,
>>
>> On 18/05/18 14:49, Robin Murphy wrote:
>>> On 18/05/18 11:22, Suzuki K Poulose wrote:
>>>> Add support for chained event counters. PMUv3 allows chaining
>>>> a pair of adjacent PMU counters (with the lower counter number
>>>> being always "even"). The low counter is programmed to count
>>>> the event of interest and the high counter(odd numbered) is
>>>> programmed with a special event code (0x1e - Chain). Thus
>>>> we need special allocation schemes to make the full use of
>>>> available counters. So, we allocate the counters from either
>>>> ends. i.e, chained counters are allocated from the lower
>>>> end in pairs of two and the normal counters are allocated
>>>> from the higher number. Also makes necessary changes to
>>>> handle the chained events as a single event with 2 counters.
>>>>
>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>> Cc: Will Deacon <will.deacon@arm.com>
>>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>
>>
>>>> ? /*
>>>> @@ -845,8 +1016,14 @@ static int __armv8_pmuv3_map_event(struct
>>>> perf_event *event,
>>>> ???????????????????????? &armv8_pmuv3_perf_cache_map,
>>>> ???????????????????????? ARMV8_PMU_EVTYPE_EVENT);
>>>> -??? if (hw_event_id == ARMV8_PMUV3_PERFCTR_CPU_CYCLES)
>>>> +??? if (hw_event_id == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) {
>>>> +??????? /* Prevent chaining for cycle counter */
>>>
>>> Why? Sure, we want to avoid executing the chaining logic if we're
>>> scheduling a cycles event in the dedicated counter (which is perhaps
>>> what the comment above wanted to say), but if one ends up allocated
>>> into a regular counter (e.g. if the user asks for multiple cycle
>>> counts with different filters), then I don't see any reason to forbid
>>> that being chained.
>>
>> Ah, I didn't think about that case. I was under the assumption that the
>> cycles are *only* placed on the cycle counter. I will take care of that.
>> Thanks for the review.
>
> Robin, Mark, Will
>
> One potential problem I see with allowing chaining of the cycle counter
> *and* the promotion of cycle event to 64bit by default is when the user
> may actually be able to count 1 less event (due to the promotion of
> cycle event to 64bit and thus forcing to use chain, if the cycle counter
> is unavailable).
Right, I didn't mean to imply we should inject the "chain" attr
automatically for all cycles events, just that we shouldn't be rejecting
it if the user does explicitly set it (but then just ignore it if using
the dedicated counter).
> So one option is to drop automatic promotion of the cycle counter to
> 64bit and do it only when it is requested by the user and use either the
> Cycle counter (preferred) or fall back to chaining. That way, the user
> has the control over the number of events he can count using the given
> set of counters.
Naively, there doesn't seem to be any inherent harm in always using
64-bit arithmetic for the dedicated counter, but it would mean that with
multiple (non-chained) cycles events, some would be taking an interrupt
every few seconds while one would effectively never overflow. I guess
the question is whether that matters or not.
Robin.
^ permalink raw reply
* [PATCH v7 2/2] PCI: mediatek: Using chained IRQ to setup IRQ handle
From: Lorenzo Pieralisi @ 2018-05-21 14:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518195154.GB41790@bhelgaas-glaptop.roam.corp.google.com>
On Fri, May 18, 2018 at 02:51:54PM -0500, Bjorn Helgaas wrote:
> On Fri, May 04, 2018 at 01:47:33PM +0800, honghui.zhang at mediatek.com wrote:
> > From: Honghui Zhang <honghui.zhang@mediatek.com>
> >
> > Using irq_chip solution to setup IRQs in order to consist
> > with IRQ framework.
> >
> > Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> > Acked-by: Ryder Lee <ryder.lee@mediatek.com>
> > ---
> > drivers/pci/host/pcie-mediatek.c | 206 ++++++++++++++++++++++-----------------
> > 1 file changed, 115 insertions(+), 91 deletions(-)
> >
> > diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
> > index c3dc549..dabf1086 100644
> > --- a/drivers/pci/host/pcie-mediatek.c
> > +++ b/drivers/pci/host/pcie-mediatek.c
> > ...
>
> > -static int mtk_pcie_msi_map(struct irq_domain *domain, unsigned int irq,
> > - irq_hw_number_t hwirq)
> > +static struct msi_domain_info mtk_msi_domain_info = {
>
> I think this patch should be amended to include this:
>
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index 0d0177ce436c..368b70d9371b 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -193,7 +193,7 @@ config PCIE_MEDIATEK
> bool "MediaTek PCIe controller"
> depends on (ARM || ARM64) && (ARCH_MEDIATEK || COMPILE_TEST)
> depends on OF
> - depends on PCI
Actually I amended your patch, since I do not think the deletion above
belongs in this commit (I agree that's completely useless but that's
not this patch that we should remove it).
Lorenzo
> + depends on PCI_MSI_IRQ_DOMAIN
^ permalink raw reply
* [PATCH v6 4/5] phy: rockchip-typec: support variable phy config value
From: Enric Balletbo Serra @ 2018-05-21 14:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526895424-22894-4-git-send-email-hl@rock-chips.com>
Hi Lin,
2018-05-21 11:37 GMT+02:00 Lin Huang <hl@rock-chips.com>:
> the phy config values used to fix in dp firmware, but some boards
> need change these values to do training and get the better eye diagram
> result. So support that in phy driver.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
> Changes in v2:
> - update patch following Enric suggest
> Changes in v3:
> - delete need_software_training variable
> - add default phy config value, if dts do not define phy config value, use these value
> Changes in v4:
> - rename variable config to tcphy_default_config
> Changes in v5:
> - None
> Changes in v6:
> - split the header file to new patch
>
> drivers/phy/rockchip/phy-rockchip-typec.c | 261 ++++++++++++++++++++++++------
> 1 file changed, 208 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
> index 795055f..4c4b925 100644
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
> @@ -324,21 +324,29 @@
> * clock 0: PLL 0 div 1
> * clock 1: PLL 1 div 2
> */
> -#define CLK_PLL_CONFIG 0X30
> +#define CLK_PLL1_DIV1 0x20
> +#define CLK_PLL1_DIV2 0x30
> #define CLK_PLL_MASK 0x33
>
> #define CMN_READY BIT(0)
>
> +#define DP_PLL_CLOCK_ENABLE_ACK BIT(3)
> #define DP_PLL_CLOCK_ENABLE BIT(2)
> +#define DP_PLL_ENABLE_ACK BIT(1)
> #define DP_PLL_ENABLE BIT(0)
> #define DP_PLL_DATA_RATE_RBR ((2 << 12) | (4 << 8))
> #define DP_PLL_DATA_RATE_HBR ((2 << 12) | (4 << 8))
> #define DP_PLL_DATA_RATE_HBR2 ((1 << 12) | (2 << 8))
> +#define DP_PLL_DATA_RATE_MASK 0xff00
>
> -#define DP_MODE_A0 BIT(4)
> -#define DP_MODE_A2 BIT(6)
> -#define DP_MODE_ENTER_A0 0xc101
> -#define DP_MODE_ENTER_A2 0xc104
> +#define DP_MODE_MASK 0xf
> +#define DP_MODE_ENTER_A0 BIT(0)
> +#define DP_MODE_ENTER_A2 BIT(2)
> +#define DP_MODE_ENTER_A3 BIT(3)
> +#define DP_MODE_A0_ACK BIT(4)
> +#define DP_MODE_A2_ACK BIT(6)
> +#define DP_MODE_A3_ACK BIT(7)
> +#define DP_LINK_RESET_DEASSERTED BIT(8)
>
> #define PHY_MODE_SET_TIMEOUT 100000
>
> @@ -350,6 +358,8 @@
> #define MODE_DFP_USB BIT(1)
> #define MODE_DFP_DP BIT(2)
>
> +#define DP_DEFAULT_RATE 162000
> +
> struct phy_reg {
> u16 value;
> u32 addr;
> @@ -372,15 +382,15 @@ struct phy_reg usb3_pll_cfg[] = {
> { 0x8, CMN_DIAG_PLL0_LF_PROG },
> };
>
> -struct phy_reg dp_pll_cfg[] = {
> +struct phy_reg dp_pll_rbr_cfg[] = {
> { 0xf0, CMN_PLL1_VCOCAL_INIT },
> { 0x18, CMN_PLL1_VCOCAL_ITER },
> { 0x30b9, CMN_PLL1_VCOCAL_START },
> - { 0x21c, CMN_PLL1_INTDIV },
> + { 0x87, CMN_PLL1_INTDIV },
> { 0, CMN_PLL1_FRACDIV },
> - { 0x5, CMN_PLL1_HIGH_THR },
> - { 0x35, CMN_PLL1_SS_CTRL1 },
> - { 0x7f1e, CMN_PLL1_SS_CTRL2 },
> + { 0x22, CMN_PLL1_HIGH_THR },
> + { 0x8000, CMN_PLL1_SS_CTRL1 },
> + { 0, CMN_PLL1_SS_CTRL2 },
> { 0x20, CMN_PLL1_DSM_DIAG },
> { 0, CMN_PLLSM1_USER_DEF_CTRL },
> { 0, CMN_DIAG_PLL1_OVRD },
> @@ -391,9 +401,52 @@ struct phy_reg dp_pll_cfg[] = {
> { 0x8, CMN_DIAG_PLL1_LF_PROG },
> { 0x100, CMN_DIAG_PLL1_PTATIS_TUNE1 },
> { 0x7, CMN_DIAG_PLL1_PTATIS_TUNE2 },
> - { 0x4, CMN_DIAG_PLL1_INCLK_CTRL },
> + { 0x1, CMN_DIAG_PLL1_INCLK_CTRL },
> };
>
> +struct phy_reg dp_pll_hbr_cfg[] = {
> + { 0xf0, CMN_PLL1_VCOCAL_INIT },
> + { 0x18, CMN_PLL1_VCOCAL_ITER },
> + { 0x30b4, CMN_PLL1_VCOCAL_START },
> + { 0xe1, CMN_PLL1_INTDIV },
> + { 0, CMN_PLL1_FRACDIV },
> + { 0x5, CMN_PLL1_HIGH_THR },
> + { 0x8000, CMN_PLL1_SS_CTRL1 },
> + { 0, CMN_PLL1_SS_CTRL2 },
> + { 0x20, CMN_PLL1_DSM_DIAG },
> + { 0x1000, CMN_PLLSM1_USER_DEF_CTRL },
> + { 0, CMN_DIAG_PLL1_OVRD },
> + { 0, CMN_DIAG_PLL1_FBH_OVRD },
> + { 0, CMN_DIAG_PLL1_FBL_OVRD },
> + { 0x7, CMN_DIAG_PLL1_V2I_TUNE },
> + { 0x45, CMN_DIAG_PLL1_CP_TUNE },
> + { 0x8, CMN_DIAG_PLL1_LF_PROG },
> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE1 },
> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE2 },
> + { 0x1, CMN_DIAG_PLL1_INCLK_CTRL },
> +};
> +
> +struct phy_reg dp_pll_hbr2_cfg[] = {
> + { 0xf0, CMN_PLL1_VCOCAL_INIT },
> + { 0x18, CMN_PLL1_VCOCAL_ITER },
> + { 0x30b4, CMN_PLL1_VCOCAL_START },
> + { 0xe1, CMN_PLL1_INTDIV },
> + { 0, CMN_PLL1_FRACDIV },
> + { 0x5, CMN_PLL1_HIGH_THR },
> + { 0x8000, CMN_PLL1_SS_CTRL1 },
> + { 0, CMN_PLL1_SS_CTRL2 },
> + { 0x20, CMN_PLL1_DSM_DIAG },
> + { 0x1000, CMN_PLLSM1_USER_DEF_CTRL },
> + { 0, CMN_DIAG_PLL1_OVRD },
> + { 0, CMN_DIAG_PLL1_FBH_OVRD },
> + { 0, CMN_DIAG_PLL1_FBL_OVRD },
> + { 0x7, CMN_DIAG_PLL1_V2I_TUNE },
> + { 0x45, CMN_DIAG_PLL1_CP_TUNE },
> + { 0x8, CMN_DIAG_PLL1_LF_PROG },
> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE1 },
> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE2 },
> + { 0x1, CMN_DIAG_PLL1_INCLK_CTRL },
> +};
> static const struct rockchip_usb3phy_port_cfg rk3399_usb3phy_port_cfgs[] = {
> {
> .reg = 0xff7c0000,
> @@ -418,6 +471,24 @@ static const struct rockchip_usb3phy_port_cfg rk3399_usb3phy_port_cfgs[] = {
> { /* sentinel */ }
> };
>
> +/* default phy config */
> +static const struct phy_config tcphy_default_config[3][4] = {
> + {{ .swing = 0x2a, .pe = 0x00 },
> + { .swing = 0x1f, .pe = 0x15 },
> + { .swing = 0x14, .pe = 0x22 },
> + { .swing = 0x02, .pe = 0x2b } },
> +
> + {{ .swing = 0x21, .pe = 0x00 },
> + { .swing = 0x12, .pe = 0x15 },
> + { .swing = 0x02, .pe = 0x22 },
> + { .swing = 0, .pe = 0 } },
> +
> + {{ .swing = 0x15, .pe = 0x00 },
> + { .swing = 0x00, .pe = 0x15 },
> + { .swing = 0, .pe = 0 },
> + { .swing = 0, .pe = 0 } },
> +};
> +
> static void tcphy_cfg_24m(struct rockchip_typec_phy *tcphy)
> {
> u32 i, rdata;
> @@ -439,7 +510,7 @@ static void tcphy_cfg_24m(struct rockchip_typec_phy *tcphy)
>
> rdata = readl(tcphy->base + CMN_DIAG_HSCLK_SEL);
> rdata &= ~CLK_PLL_MASK;
> - rdata |= CLK_PLL_CONFIG;
> + rdata |= CLK_PLL1_DIV2;
> writel(rdata, tcphy->base + CMN_DIAG_HSCLK_SEL);
> }
>
> @@ -453,17 +524,44 @@ static void tcphy_cfg_usb3_pll(struct rockchip_typec_phy *tcphy)
> tcphy->base + usb3_pll_cfg[i].addr);
> }
>
> -static void tcphy_cfg_dp_pll(struct rockchip_typec_phy *tcphy)
> +static void tcphy_cfg_dp_pll(struct rockchip_typec_phy *tcphy, int link_rate)
> {
> - u32 i;
> + struct phy_reg *phy_cfg;
> + u32 clk_ctrl;
> + u32 i, cfg_size, hsclk_sel;
> +
> + hsclk_sel = readl(tcphy->base + CMN_DIAG_HSCLK_SEL);
> + hsclk_sel &= ~CLK_PLL_MASK;
> +
> + switch (link_rate) {
> + case 162000:
> + clk_ctrl = DP_PLL_DATA_RATE_RBR;
> + hsclk_sel |= CLK_PLL1_DIV2;
> + phy_cfg = dp_pll_rbr_cfg;
> + cfg_size = ARRAY_SIZE(dp_pll_rbr_cfg);
> + break;
> + case 270000:
> + clk_ctrl = DP_PLL_DATA_RATE_HBR;
> + hsclk_sel |= CLK_PLL1_DIV2;
> + phy_cfg = dp_pll_hbr_cfg;
> + cfg_size = ARRAY_SIZE(dp_pll_hbr_cfg);
> + break;
> + case 540000:
> + clk_ctrl = DP_PLL_DATA_RATE_HBR2;
> + hsclk_sel |= CLK_PLL1_DIV1;
> + phy_cfg = dp_pll_hbr2_cfg;
> + cfg_size = ARRAY_SIZE(dp_pll_hbr2_cfg);
> + break;
If someone calls this function with a link_rate different to the ones
in the switch statement, some variables will be uninitialized and the
kernel will crash. I'd add a default case or return an error if a
different link_rate is passed to the function.
> + }
>
> - /* set the default mode to RBR */
> - writel(DP_PLL_CLOCK_ENABLE | DP_PLL_ENABLE | DP_PLL_DATA_RATE_RBR,
> - tcphy->base + DP_CLK_CTL);
> + clk_ctrl |= DP_PLL_CLOCK_ENABLE | DP_PLL_ENABLE;
> + writel(clk_ctrl, tcphy->base + DP_CLK_CTL);
> +
> + writel(hsclk_sel, tcphy->base + CMN_DIAG_HSCLK_SEL);
>
> /* load the configuration of PLL1 */
> - for (i = 0; i < ARRAY_SIZE(dp_pll_cfg); i++)
> - writel(dp_pll_cfg[i].value, tcphy->base + dp_pll_cfg[i].addr);
> + for (i = 0; i < cfg_size; i++)
> + writel(phy_cfg[i].value, tcphy->base + phy_cfg[i].addr);
> }
>
> static void tcphy_tx_usb3_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
> @@ -490,9 +588,10 @@ static void tcphy_rx_usb3_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
> writel(0xfb, tcphy->base + XCVR_DIAG_BIDI_CTRL(lane));
> }
>
> -static void tcphy_dp_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
> +static void tcphy_dp_cfg_lane(struct rockchip_typec_phy *tcphy, int link_rate,
> + u8 swing, u8 pre_emp, u32 lane)
> {
> - u16 rdata;
> + u16 val;
>
Just use rdata in your added code, you don't need to rename the
variable. That will result in a smaller diff.
> writel(0xbefc, tcphy->base + XCVR_PSM_RCTRL(lane));
> writel(0x6799, tcphy->base + TX_PSC_A0(lane));
> @@ -500,25 +599,31 @@ static void tcphy_dp_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
> writel(0x98, tcphy->base + TX_PSC_A2(lane));
> writel(0x98, tcphy->base + TX_PSC_A3(lane));
>
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_000(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_001(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_010(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_011(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_100(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_101(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_110(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_111(lane));
> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_10(lane));
> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_01(lane));
> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_00(lane));
> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_11(lane));
> -
> - writel(0x128, tcphy->base + TX_TXCC_CAL_SCLR_MULT(lane));
> - writel(0x400, tcphy->base + TX_DIAG_TX_DRV(lane));
> -
> - rdata = readl(tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
> - rdata = (rdata & 0x8fff) | 0x6000;
> - writel(rdata, tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
> + writel(tcphy->config[swing][pre_emp].swing,
> + tcphy->base + TX_TXCC_MGNFS_MULT_000(lane));
> + writel(tcphy->config[swing][pre_emp].pe,
> + tcphy->base + TX_TXCC_CPOST_MULT_00(lane));
> +
> + if (swing == 2 && pre_emp == 0 && link_rate != 540000) {
> + writel(0x700, tcphy->base + TX_DIAG_TX_DRV(lane));
> + writel(0x13c, tcphy->base + TX_TXCC_CAL_SCLR_MULT(lane));
> + } else {
> + writel(0x128, tcphy->base + TX_TXCC_CAL_SCLR_MULT(lane));
> + writel(0x0400, tcphy->base + TX_DIAG_TX_DRV(lane));
> + }
> +
> + val = readl(tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
> + val = val & 0x8fff;
> + switch (link_rate) {
> + case 162000:
> + case 270000:
> + val |= (6 << 12);
> + break;
> + case 540000:
> + val |= (4 << 12);
> + break;
If link_rate is another value you will write (val & 0x8fff), is ok?
Before this patch, we were writing (rdata & 0x8fff) | 0x6000 by
default, maybe we should default to this value?
> + }
> + writel(val, tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
> }
>
> static inline int property_enable(struct rockchip_typec_phy *tcphy,
> @@ -709,30 +814,33 @@ static int tcphy_phy_init(struct rockchip_typec_phy *tcphy, u8 mode)
> tcphy_cfg_24m(tcphy);
>
> if (mode == MODE_DFP_DP) {
> - tcphy_cfg_dp_pll(tcphy);
> + tcphy_cfg_dp_pll(tcphy, DP_DEFAULT_RATE);
> for (i = 0; i < 4; i++)
> - tcphy_dp_cfg_lane(tcphy, i);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, i);
>
> writel(PIN_ASSIGN_C_E, tcphy->base + PMA_LANE_CFG);
> } else {
> tcphy_cfg_usb3_pll(tcphy);
> - tcphy_cfg_dp_pll(tcphy);
> + tcphy_cfg_dp_pll(tcphy, DP_DEFAULT_RATE);
> if (tcphy->flip) {
> tcphy_tx_usb3_cfg_lane(tcphy, 3);
> tcphy_rx_usb3_cfg_lane(tcphy, 2);
> - tcphy_dp_cfg_lane(tcphy, 0);
> - tcphy_dp_cfg_lane(tcphy, 1);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, 0);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, 1);
> } else {
> tcphy_tx_usb3_cfg_lane(tcphy, 0);
> tcphy_rx_usb3_cfg_lane(tcphy, 1);
> - tcphy_dp_cfg_lane(tcphy, 2);
> - tcphy_dp_cfg_lane(tcphy, 3);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, 2);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, 3);
> }
>
> writel(PIN_ASSIGN_D_F, tcphy->base + PMA_LANE_CFG);
> }
>
> - writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
> + val = readl(tcphy->base + DP_MODE_CTL);
> + val &= ~DP_MODE_MASK;
> + val |= DP_MODE_ENTER_A2 | DP_LINK_RESET_DEASSERTED;
> + writel(val, tcphy->base + DP_MODE_CTL);
>
> reset_control_deassert(tcphy->uphy_rst);
>
> @@ -945,7 +1053,7 @@ static int rockchip_dp_phy_power_on(struct phy *phy)
> property_enable(tcphy, &cfg->uphy_dp_sel, 1);
>
> ret = readx_poll_timeout(readl, tcphy->base + DP_MODE_CTL,
> - val, val & DP_MODE_A2, 1000,
> + val, val & DP_MODE_A2_ACK, 1000,
> PHY_MODE_SET_TIMEOUT);
> if (ret < 0) {
> dev_err(tcphy->dev, "failed to wait TCPHY enter A2\n");
> @@ -954,13 +1062,19 @@ static int rockchip_dp_phy_power_on(struct phy *phy)
>
> tcphy_dp_aux_calibration(tcphy);
>
> - writel(DP_MODE_ENTER_A0, tcphy->base + DP_MODE_CTL);
> + /* enter A0 mode */
> + val = readl(tcphy->base + DP_MODE_CTL);
> + val &= ~DP_MODE_MASK;
> + val |= DP_MODE_ENTER_A0;
> + writel(val, tcphy->base + DP_MODE_CTL);
>
> ret = readx_poll_timeout(readl, tcphy->base + DP_MODE_CTL,
> - val, val & DP_MODE_A0, 1000,
> + val, val & DP_MODE_A0_ACK, 1000,
> PHY_MODE_SET_TIMEOUT);
> if (ret < 0) {
> - writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
> + val &= ~DP_MODE_MASK;
> + val |= DP_MODE_ENTER_A2;
> + writel(val, tcphy->base + DP_MODE_CTL);
> dev_err(tcphy->dev, "failed to wait TCPHY enter A0\n");
> goto power_on_finish;
> }
> @@ -978,6 +1092,7 @@ static int rockchip_dp_phy_power_on(struct phy *phy)
> static int rockchip_dp_phy_power_off(struct phy *phy)
> {
> struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
> + u32 val;
>
> mutex_lock(&tcphy->lock);
>
> @@ -986,7 +1101,10 @@ static int rockchip_dp_phy_power_off(struct phy *phy)
>
> tcphy->mode &= ~MODE_DFP_DP;
>
> - writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
> + val = readl(tcphy->base + DP_MODE_CTL);
> + val &= ~DP_MODE_MASK;
> + val |= DP_MODE_ENTER_A2;
> + writel(val, tcphy->base + DP_MODE_CTL);
>
> if (tcphy->mode == MODE_DISCONNECT)
> tcphy_phy_deinit(tcphy);
> @@ -1002,9 +1120,35 @@ static const struct phy_ops rockchip_dp_phy_ops = {
> .owner = THIS_MODULE,
> };
>
> +static int typec_dp_phy_config(struct phy *phy, int link_rate,
> + int lanes, u8 swing, u8 pre_emp)
> +{
> + struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
> + u8 i;
> +
> + tcphy_cfg_dp_pll(tcphy, link_rate);
> +
> + if (tcphy->mode == MODE_DFP_DP) {
> + for (i = 0; i < 4; i++)
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, i);
> + } else {
> + if (tcphy->flip) {
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 0);
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 1);
> + } else {
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 2);
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 3);
> + }
> + }
> +
> + return 0;
> +}
> +
> static int tcphy_parse_dt(struct rockchip_typec_phy *tcphy,
> struct device *dev)
> {
> + int ret;
> +
> tcphy->grf_regs = syscon_regmap_lookup_by_phandle(dev->of_node,
> "rockchip,grf");
> if (IS_ERR(tcphy->grf_regs)) {
> @@ -1042,6 +1186,16 @@ static int tcphy_parse_dt(struct rockchip_typec_phy *tcphy,
> return PTR_ERR(tcphy->tcphy_rst);
> }
>
> + /*
> + * check if phy_config pass from dts, if no,
> + * use default phy config value.
nit: Check if phy-config is passed from the dts, if no, use the
default phy configuration.
> + */
> + ret = of_property_read_u32_array(dev->of_node, "rockchip,phy-config",
> + (u32 *)tcphy->config, sizeof(tcphy->config) / sizeof(u32));
> + if (ret)
> + memcpy(tcphy->config, tcphy_default_config,
> + sizeof(tcphy->config));
> +
> return 0;
> }
>
> @@ -1126,6 +1280,7 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev)
> }
> }
>
> + tcphy->typec_phy_config = typec_dp_phy_config;
> pm_runtime_enable(dev);
>
> for_each_available_child_of_node(np, child_np) {
> --
> 2.7.4
>
Best regards,
Enric
^ permalink raw reply
* [PATCH v9 00/16] KVM: arm64: Optimise FPSIMD context switching
From: Dave Martin @ 2018-05-21 14:17 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].
This series aims to improve the way FPSIMD context is handled by KVM.
Only minor changes have been made since the previous v8 [1], though
this posting does apply a couple of fixes.
This is a minor update to fix a couple of build breaks introduced late
in the evolution of the series. [3] [4]
There is also an unexplained NULL-dereference bug observed by Marc on
ESPRESSOBin [5] that I need to look into. We've not been able to
reproduce this so far.
In the meantime, this post will give people an opportunity to look over
the current changes.
The changes are summarised in the individual patches.
Reviewers please note:
* **Only patches 8 and 12** have changed since v8.
* I have stripped tags from patch 12 due to the introduction of a new
source file, which may not be the best approach.
If Christoffer, Marc and Catalin could take another look that
would be much appreciated.
Cheers
---Dave
[1] [PULL v8] KVM: arm64: Optimise FPSIMD context switching
lists.infradead.org/pipermail/linux-arm-kernel/2018-May/578184.html
[2] linux-arm-kernel archive
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/thread.html
[3] [kvmarm:queue 9/29] arch/arm/kvm/../../../virt/kvm/arm/arm.c:783:3: error: implicit declaration of function 'kvm_arch_vcpu_ctxsync_fp'; did you mean 'kvm_arch_vcpu_put_fp'?
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/579400.html
[4] [kvmarm:queue 13/29] arch/arm/kvm/../../../virt/kvm/arm/arm.c:1598:6: error: implicit declaration of function 'system_supports_sve'
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/579399.html
[5] [PULL v8] KVM: arm64: Optimise FPSIMD context switching
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/579353.html
Christoffer Dall (1):
KVM: arm/arm64: Introduce kvm_arch_vcpu_run_pid_change
Dave Martin (15):
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/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 | 33 +++++---
arch/arm64/include/asm/processor.h | 2 +
arch/arm64/kernel/fpsimd.c | 147 +++++++++++++++++++-----------------
arch/arm64/kernel/ptrace.c | 1 +
arch/arm64/kvm/Kconfig | 1 +
arch/arm64/kvm/Makefile | 4 +-
arch/arm64/kvm/debug.c | 8 +-
arch/arm64/kvm/fpsimd.c | 111 +++++++++++++++++++++++++++
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/init.c | 33 ++++++++
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 | 13 +++-
virt/kvm/kvm_main.c | 7 +-
26 files changed, 430 insertions(+), 234 deletions(-)
create mode 100644 arch/arm64/kvm/fpsimd.c
create mode 100644 arch/arm64/kvm/init.c
--
2.1.4
^ permalink raw reply
* [PATCH v9 01/16] thread_info: Add update_thread_flag() helpers
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-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>
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 v9 02/16] arm64: Use update{,_tsk}_thread_flag()
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-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>
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 87a3536..9647fab 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 v9 03/16] KVM: arm/arm64: Introduce kvm_arch_vcpu_run_pid_change
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-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>
---
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 v9 04/16] KVM: arm64: Convert lazy FPSIMD context switch trap to C
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-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>
---
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
* [PATCH v9 05/16] arm64: fpsimd: Generalise context saving for non-task contexts
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-1-git-send-email-Dave.Martin@arm.com>
In preparation for allowing non-task (i.e., KVM vcpu) FPSIMD
contexts to be handled by the fpsimd common code, this patch adapts
task_fpsimd_save() to save back the currently loaded context,
removing the explicit dependency on current.
The relevant storage to write back to in memory is now found by
examining the fpsimd_last_state percpu struct.
fpsimd_save() does nothing unless TIF_FOREIGN_FPSTATE is clear, and
fpsimd_last_state is updated under local_bh_disable() or
local_irq_disable() everywhere that TIF_FOREIGN_FPSTATE is cleared:
thus, fpsimd_save() will write back to the correct storage for the
loaded context.
No functional change.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/kernel/fpsimd.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 9647fab..7700076 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -270,13 +270,15 @@ static void task_fpsimd_load(void)
}
/*
- * Ensure current's FPSIMD/SVE storage in thread_struct is up to date
- * with respect to the CPU registers.
+ * Ensure FPSIMD/SVE storage in memory for the loaded context is up to
+ * date with respect to the CPU registers.
*
* Softirqs (and preemption) must be disabled.
*/
-static void task_fpsimd_save(void)
+static void fpsimd_save(void)
{
+ struct user_fpsimd_state *st = __this_cpu_read(fpsimd_last_state.st);
+
WARN_ON(!in_softirq() && !irqs_disabled());
if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) {
@@ -291,10 +293,9 @@ static void task_fpsimd_save(void)
return;
}
- sve_save_state(sve_pffr(current),
- ¤t->thread.uw.fpsimd_state.fpsr);
+ sve_save_state(sve_pffr(current), &st->fpsr);
} else
- fpsimd_save_state(¤t->thread.uw.fpsimd_state);
+ fpsimd_save_state(st);
}
}
@@ -598,7 +599,7 @@ int sve_set_vector_length(struct task_struct *task,
if (task == current) {
local_bh_disable();
- task_fpsimd_save();
+ fpsimd_save();
set_thread_flag(TIF_FOREIGN_FPSTATE);
}
@@ -837,7 +838,7 @@ asmlinkage void do_sve_acc(unsigned int esr, struct pt_regs *regs)
local_bh_disable();
- task_fpsimd_save();
+ fpsimd_save();
fpsimd_to_sve(current);
/* Force ret_to_user to reload the registers: */
@@ -898,7 +899,7 @@ void fpsimd_thread_switch(struct task_struct *next)
* 'current'.
*/
if (current->mm)
- task_fpsimd_save();
+ fpsimd_save();
if (next->mm) {
/*
@@ -980,7 +981,7 @@ void fpsimd_preserve_current_state(void)
return;
local_bh_disable();
- task_fpsimd_save();
+ fpsimd_save();
local_bh_enable();
}
@@ -1120,7 +1121,7 @@ void kernel_neon_begin(void)
/* Save unsaved task fpsimd state, if any: */
if (current->mm) {
- task_fpsimd_save();
+ fpsimd_save();
set_thread_flag(TIF_FOREIGN_FPSTATE);
}
@@ -1245,7 +1246,7 @@ static int fpsimd_cpu_pm_notifier(struct notifier_block *self,
switch (cmd) {
case CPU_PM_ENTER:
if (current->mm)
- task_fpsimd_save();
+ fpsimd_save();
fpsimd_flush_cpu_state();
break;
case CPU_PM_EXIT:
--
2.1.4
^ permalink raw reply related
* [PATCH v9 06/16] arm64/sve: Refactor user SVE trap maintenance for external use
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-1-git-send-email-Dave.Martin@arm.com>
In preparation for optimising the way KVM manages switching the
guest and host FPSIMD state, it is necessary to provide a means for
code outside arch/arm64/kernel/fpsimd.c to restore the user trap
configuration for SVE correctly for the current task.
Rather than requiring external code to duplicate the maintenance
explicitly, this patch wraps moves the trap maintenenace to
fpsimd_bind_to_cpu(), since it is logically part of the work of
associating the current task with the cpu.
Because fpsimd_bind_to_cpu() is rather a cryptic name to publish
alongside fpsimd_bind_state_to_cpu(), the former function is
renamed to fpsimd_bind_task_to_cpu() to make its purpose more
explicit.
This patch makes appropriate changes to ensure that
fpsimd_bind_task_to_cpu() is always called alongside
task_fpsimd_load(), so that the trap maintenance continues to be
done in every situation where it was done prior to this patch.
As a side-effect, the metadata updates done by
fpsimd_bind_task_to_cpu() now change from conditional to
unconditional in the "already bound" case of sigreturn. This is
harmless, and a couple of extra stores on this slow path will not
impact performance. I consider this a reasonable price to pay for
a slightly cleaner interface.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/kernel/fpsimd.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 7700076..f98b9a4 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -257,16 +257,6 @@ static void task_fpsimd_load(void)
sve_vq_from_vl(current->thread.sve_vl) - 1);
else
fpsimd_load_state(¤t->thread.uw.fpsimd_state);
-
- if (system_supports_sve()) {
- /* Toggle SVE trapping for userspace if needed */
- if (test_thread_flag(TIF_SVE))
- sve_user_enable();
- else
- sve_user_disable();
-
- /* Serialised by exception return to user */
- }
}
/*
@@ -1001,7 +991,7 @@ void fpsimd_signal_preserve_current_state(void)
* Associate current's FPSIMD context with this cpu
* Preemption must be disabled when calling this function.
*/
-static void fpsimd_bind_to_cpu(void)
+static void fpsimd_bind_task_to_cpu(void)
{
struct fpsimd_last_state_struct *last =
this_cpu_ptr(&fpsimd_last_state);
@@ -1009,6 +999,16 @@ static void fpsimd_bind_to_cpu(void)
last->st = ¤t->thread.uw.fpsimd_state;
last->sve_in_use = test_thread_flag(TIF_SVE);
current->thread.fpsimd_cpu = smp_processor_id();
+
+ if (system_supports_sve()) {
+ /* Toggle SVE trapping for userspace if needed */
+ if (test_thread_flag(TIF_SVE))
+ sve_user_enable();
+ else
+ sve_user_disable();
+
+ /* Serialised by exception return to user */
+ }
}
/*
@@ -1025,7 +1025,7 @@ void fpsimd_restore_current_state(void)
if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) {
task_fpsimd_load();
- fpsimd_bind_to_cpu();
+ fpsimd_bind_task_to_cpu();
}
local_bh_enable();
@@ -1048,9 +1048,9 @@ void fpsimd_update_current_state(struct user_fpsimd_state const *state)
fpsimd_to_sve(current);
task_fpsimd_load();
+ fpsimd_bind_task_to_cpu();
- if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE))
- fpsimd_bind_to_cpu();
+ clear_thread_flag(TIF_FOREIGN_FPSTATE);
local_bh_enable();
}
--
2.1.4
^ permalink raw reply related
* [PATCH v9 07/16] KVM: arm64: Repurpose vcpu_arch.debug_flags for general-purpose flags
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-1-git-send-email-Dave.Martin@arm.com>
In struct vcpu_arch, the debug_flags field is used to store
debug-related flags about the vcpu state.
Since we are about to add some more flags related to FPSIMD and
SVE, it makes sense to add them to the existing flags field rather
than adding new fields. Since there is only one debug_flags flag
defined so far, there is plenty of free space for expansion.
In preparation for adding more flags, this patch renames the
debug_flags field to simply "flags", and updates comments
appropriately.
The flag definitions are also moved to <asm/kvm_host.h>, since
their presence in <asm/kvm_asm.h> was for purely historical
reasons: these definitions are not used from asm any more, and not
very likely to be as more Hyp asm is migrated to C.
KVM_ARM64_DEBUG_DIRTY_SHIFT has not been used since commit
1ea66d27e7b0 ("arm64: KVM: Move away from the assembly version of
the world switch"), so this patch gets rid of that too.
No functional change.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@arm.com>
---
arch/arm64/include/asm/kvm_asm.h | 3 ---
arch/arm64/include/asm/kvm_host.h | 7 +++++--
arch/arm64/kvm/debug.c | 8 ++++----
arch/arm64/kvm/hyp/debug-sr.c | 6 +++---
arch/arm64/kvm/hyp/sysreg-sr.c | 4 ++--
arch/arm64/kvm/sys_regs.c | 9 ++++-----
6 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index f6648a3..f62ccbf 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -30,9 +30,6 @@
/* The hyp-stub will return this for any kvm_call_hyp() call */
#define ARM_EXCEPTION_HYP_GONE HVC_STUB_ERR
-#define KVM_ARM64_DEBUG_DIRTY_SHIFT 0
-#define KVM_ARM64_DEBUG_DIRTY (1 << KVM_ARM64_DEBUG_DIRTY_SHIFT)
-
/* Translate a kernel address of @sym into its equivalent linear mapping */
#define kvm_ksym_ref(sym) \
({ \
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 469de8a..146c167 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -216,8 +216,8 @@ struct kvm_vcpu_arch {
/* Exception Information */
struct kvm_vcpu_fault_info fault;
- /* Guest debug state */
- u64 debug_flags;
+ /* Miscellaneous vcpu state flags */
+ u64 flags;
/*
* We maintain more than a single set of debug registers to support
@@ -293,6 +293,9 @@ struct kvm_vcpu_arch {
bool sysregs_loaded_on_cpu;
};
+/* vcpu_arch flags field values: */
+#define KVM_ARM64_DEBUG_DIRTY (1 << 0)
+
#define vcpu_gp_regs(v) (&(v)->arch.ctxt.gp_regs)
/*
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index a1f4ebd..00d4223 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -103,7 +103,7 @@ void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
*
* Additionally, KVM only traps guest accesses to the debug registers if
* the guest is not actively using them (see the KVM_ARM64_DEBUG_DIRTY
- * flag on vcpu->arch.debug_flags). Since the guest must not interfere
+ * flag on vcpu->arch.flags). Since the guest must not interfere
* with the hardware state when debugging the guest, we must ensure that
* trapping is enabled whenever we are debugging the guest using the
* debug registers.
@@ -111,7 +111,7 @@ void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
{
- bool trap_debug = !(vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY);
+ bool trap_debug = !(vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY);
unsigned long mdscr;
trace_kvm_arm_setup_debug(vcpu, vcpu->guest_debug);
@@ -184,7 +184,7 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
vcpu_write_sys_reg(vcpu, mdscr, MDSCR_EL1);
vcpu->arch.debug_ptr = &vcpu->arch.external_debug_state;
- vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
+ vcpu->arch.flags |= KVM_ARM64_DEBUG_DIRTY;
trap_debug = true;
trace_kvm_arm_set_regset("BKPTS", get_num_brps(),
@@ -206,7 +206,7 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
/* If KDE or MDE are set, perform a full save/restore cycle. */
if (vcpu_read_sys_reg(vcpu, MDSCR_EL1) & (DBG_MDSCR_KDE | DBG_MDSCR_MDE))
- vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
+ vcpu->arch.flags |= KVM_ARM64_DEBUG_DIRTY;
trace_kvm_arm_set_dreg32("MDCR_EL2", vcpu->arch.mdcr_el2);
trace_kvm_arm_set_dreg32("MDSCR_EL1", vcpu_read_sys_reg(vcpu, MDSCR_EL1));
diff --git a/arch/arm64/kvm/hyp/debug-sr.c b/arch/arm64/kvm/hyp/debug-sr.c
index 3e717f6..5000976 100644
--- a/arch/arm64/kvm/hyp/debug-sr.c
+++ b/arch/arm64/kvm/hyp/debug-sr.c
@@ -163,7 +163,7 @@ void __hyp_text __debug_switch_to_guest(struct kvm_vcpu *vcpu)
if (!has_vhe())
__debug_save_spe_nvhe(&vcpu->arch.host_debug_state.pmscr_el1);
- if (!(vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY))
+ if (!(vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY))
return;
host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
@@ -185,7 +185,7 @@ void __hyp_text __debug_switch_to_host(struct kvm_vcpu *vcpu)
if (!has_vhe())
__debug_restore_spe_nvhe(vcpu->arch.host_debug_state.pmscr_el1);
- if (!(vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY))
+ if (!(vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY))
return;
host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
@@ -196,7 +196,7 @@ void __hyp_text __debug_switch_to_host(struct kvm_vcpu *vcpu)
__debug_save_state(vcpu, guest_dbg, guest_ctxt);
__debug_restore_state(vcpu, host_dbg, host_ctxt);
- vcpu->arch.debug_flags &= ~KVM_ARM64_DEBUG_DIRTY;
+ vcpu->arch.flags &= ~KVM_ARM64_DEBUG_DIRTY;
}
u32 __hyp_text __kvm_get_mdcr_el2(void)
diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
index b3894df..35bc168 100644
--- a/arch/arm64/kvm/hyp/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/sysreg-sr.c
@@ -196,7 +196,7 @@ void __hyp_text __sysreg32_save_state(struct kvm_vcpu *vcpu)
sysreg[DACR32_EL2] = read_sysreg(dacr32_el2);
sysreg[IFSR32_EL2] = read_sysreg(ifsr32_el2);
- if (has_vhe() || vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
+ if (has_vhe() || vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY)
sysreg[DBGVCR32_EL2] = read_sysreg(dbgvcr32_el2);
}
@@ -218,7 +218,7 @@ void __hyp_text __sysreg32_restore_state(struct kvm_vcpu *vcpu)
write_sysreg(sysreg[DACR32_EL2], dacr32_el2);
write_sysreg(sysreg[IFSR32_EL2], ifsr32_el2);
- if (has_vhe() || vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
+ if (has_vhe() || vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY)
write_sysreg(sysreg[DBGVCR32_EL2], dbgvcr32_el2);
}
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 6e3b969..a436373 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -31,7 +31,6 @@
#include <asm/debug-monitors.h>
#include <asm/esr.h>
#include <asm/kvm_arm.h>
-#include <asm/kvm_asm.h>
#include <asm/kvm_coproc.h>
#include <asm/kvm_emulate.h>
#include <asm/kvm_host.h>
@@ -338,7 +337,7 @@ static bool trap_debug_regs(struct kvm_vcpu *vcpu,
{
if (p->is_write) {
vcpu_write_sys_reg(vcpu, p->regval, r->reg);
- vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
+ vcpu->arch.flags |= KVM_ARM64_DEBUG_DIRTY;
} else {
p->regval = vcpu_read_sys_reg(vcpu, r->reg);
}
@@ -369,7 +368,7 @@ static void reg_to_dbg(struct kvm_vcpu *vcpu,
}
*dbg_reg = val;
- vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
+ vcpu->arch.flags |= KVM_ARM64_DEBUG_DIRTY;
}
static void dbg_to_reg(struct kvm_vcpu *vcpu,
@@ -1441,7 +1440,7 @@ static bool trap_debug32(struct kvm_vcpu *vcpu,
{
if (p->is_write) {
vcpu_cp14(vcpu, r->reg) = p->regval;
- vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
+ vcpu->arch.flags |= KVM_ARM64_DEBUG_DIRTY;
} else {
p->regval = vcpu_cp14(vcpu, r->reg);
}
@@ -1473,7 +1472,7 @@ static bool trap_xvr(struct kvm_vcpu *vcpu,
val |= p->regval << 32;
*dbg_reg = val;
- vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
+ vcpu->arch.flags |= KVM_ARM64_DEBUG_DIRTY;
} else {
p->regval = *dbg_reg >> 32;
}
--
2.1.4
^ permalink raw reply related
* [PATCH v9 08/16] KVM: arm64: Optimise FPSIMD handling to reduce guest/host thrashing
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-1-git-send-email-Dave.Martin@arm.com>
This patch refactors KVM to align the host and guest FPSIMD
save/restore logic with each other for arm64. This reduces the
number of redundant save/restore operations that must occur, and
reduces the common-case IRQ blackout time during guest exit storms
by saving the host state lazily and optimising away the need to
restore the host state before returning to the run loop.
Four hooks are defined in order to enable this:
* kvm_arch_vcpu_run_map_fp():
Called on PID change to map necessary bits of current to Hyp.
* kvm_arch_vcpu_load_fp():
Set up FP/SIMD for entering the KVM run loop (parse as
"vcpu_load fp").
* kvm_arch_vcpu_ctxsync_fp():
Get FP/SIMD into a safe state for re-enabling interrupts after a
guest exit back to the run loop.
For arm64 specifically, this involves updating the host kernel's
FPSIMD context tracking metadata so that kernel-mode NEON use
will cause the vcpu's FPSIMD state to be saved back correctly
into the vcpu struct. This must be done before re-enabling
interrupts because kernel-mode NEON may be used by softirqs.
* kvm_arch_vcpu_put_fp():
Save guest FP/SIMD state back to memory and dissociate from the
CPU ("vcpu_put fp").
Also, the arm64 FPSIMD context switch code is updated to enable it
to save back FPSIMD state for a vcpu, not just current. A few
helpers drive this:
* fpsimd_bind_state_to_cpu(struct user_fpsimd_state *fp):
mark this CPU as having context fp (which may belong to a vcpu)
currently loaded in its registers. This is the non-task
equivalent of the static function fpsimd_bind_to_cpu() in
fpsimd.c.
* task_fpsimd_save():
exported to allow KVM to save the guest's FPSIMD state back to
memory on exit from the run loop.
* fpsimd_flush_state():
invalidate any context's FPSIMD state that is currently loaded.
Used to disassociate the vcpu from the CPU regs on run loop exit.
These changes allow the run loop to enable interrupts (and thus
softirqs that may use kernel-mode NEON) without having to save the
guest's FPSIMD state eagerly.
Some new vcpu_arch fields are added to make all this work. Because
host FPSIMD state can now be saved back directly into current's
thread_struct as appropriate, host_cpu_context is no longer used
for preserving the FPSIMD state. However, it is still needed for
preserving other things such as the host's system registers. To
avoid ABI churn, the redundant storage space in host_cpu_context is
not removed for now.
arch/arm is not addressed by this patch and continues to use its
current save/restore logic. It could provide implementations of
the helpers later if desired.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
squash! KVM: arm64: Optimise FPSIMD handling to reduce guest/host thrashing
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
Changes since v8:
* Fix arch/arm version of kvm_arch_vcpu_park_fp() to match new naming.
Without this, the build was breaking on arm.
Acks retained, since this is a trivial change.
---
arch/arm/include/asm/kvm_host.h | 8 +++
arch/arm64/include/asm/fpsimd.h | 6 ++
arch/arm64/include/asm/kvm_host.h | 21 +++++++
arch/arm64/kernel/fpsimd.c | 17 +++++-
arch/arm64/kvm/Kconfig | 1 +
arch/arm64/kvm/Makefile | 2 +-
arch/arm64/kvm/fpsimd.c | 112 ++++++++++++++++++++++++++++++++++++++
arch/arm64/kvm/hyp/switch.c | 51 +++++++++--------
virt/kvm/arm/arm.c | 4 ++
9 files changed, 192 insertions(+), 30 deletions(-)
create mode 100644 arch/arm64/kvm/fpsimd.c
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index c7c28c8..ac870b2 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -303,6 +303,14 @@ int kvm_arm_vcpu_arch_get_attr(struct kvm_vcpu *vcpu,
int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu,
struct kvm_device_attr *attr);
+/*
+ * VFP/NEON switching is all done by the hyp switch code, so no need to
+ * coordinate with host context handling for this state:
+ */
+static inline void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) {}
+static inline void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) {}
+static inline void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) {}
+
/* All host FP/SIMD state is restored on guest exit, so nothing to save: */
static inline void kvm_fpsimd_flush_cpu_state(void) {}
diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
index aa7162a..3e00f70 100644
--- a/arch/arm64/include/asm/fpsimd.h
+++ b/arch/arm64/include/asm/fpsimd.h
@@ -41,6 +41,8 @@ struct task_struct;
extern void fpsimd_save_state(struct user_fpsimd_state *state);
extern void fpsimd_load_state(struct user_fpsimd_state *state);
+extern void fpsimd_save(void);
+
extern void fpsimd_thread_switch(struct task_struct *next);
extern void fpsimd_flush_thread(void);
@@ -49,7 +51,11 @@ extern void fpsimd_preserve_current_state(void);
extern void fpsimd_restore_current_state(void);
extern void fpsimd_update_current_state(struct user_fpsimd_state const *state);
+extern void fpsimd_bind_task_to_cpu(void);
+extern void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *state);
+
extern void fpsimd_flush_task_state(struct task_struct *target);
+extern void fpsimd_flush_cpu_state(void);
extern void sve_flush_cpu_state(void);
/* Maximum VL that SVE VL-agnostic software can transparently support */
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 146c167..b3fe730 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -30,6 +30,7 @@
#include <asm/kvm.h>
#include <asm/kvm_asm.h>
#include <asm/kvm_mmio.h>
+#include <asm/thread_info.h>
#define __KVM_HAVE_ARCH_INTC_INITIALIZED
@@ -238,6 +239,10 @@ struct kvm_vcpu_arch {
/* Pointer to host CPU context */
kvm_cpu_context_t *host_cpu_context;
+
+ struct thread_info *host_thread_info; /* hyp VA */
+ struct user_fpsimd_state *host_fpsimd_state; /* hyp VA */
+
struct {
/* {Break,watch}point registers */
struct kvm_guest_debug_arch regs;
@@ -295,6 +300,9 @@ struct kvm_vcpu_arch {
/* vcpu_arch flags field values: */
#define KVM_ARM64_DEBUG_DIRTY (1 << 0)
+#define KVM_ARM64_FP_ENABLED (1 << 1) /* guest FP regs loaded */
+#define KVM_ARM64_FP_HOST (1 << 2) /* host FP regs loaded */
+#define KVM_ARM64_HOST_SVE_IN_USE (1 << 3) /* backup for host TIF_SVE */
#define vcpu_gp_regs(v) (&(v)->arch.ctxt.gp_regs)
@@ -423,6 +431,19 @@ static inline void __cpu_init_stage2(void)
"PARange is %d bits, unsupported configuration!", parange);
}
+/* Guest/host FPSIMD coordination helpers */
+int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu);
+void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu);
+void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu);
+void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
+
+#ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
+static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
+{
+ return kvm_arch_vcpu_run_map_fp(vcpu);
+}
+#endif
+
/*
* All host FP/SIMD state is restored on guest exit, so nothing needs
* doing here except in the SVE case:
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index f98b9a4..051b455 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -265,7 +265,7 @@ static void task_fpsimd_load(void)
*
* Softirqs (and preemption) must be disabled.
*/
-static void fpsimd_save(void)
+void fpsimd_save(void)
{
struct user_fpsimd_state *st = __this_cpu_read(fpsimd_last_state.st);
@@ -991,7 +991,7 @@ void fpsimd_signal_preserve_current_state(void)
* Associate current's FPSIMD context with this cpu
* Preemption must be disabled when calling this function.
*/
-static void fpsimd_bind_task_to_cpu(void)
+void fpsimd_bind_task_to_cpu(void)
{
struct fpsimd_last_state_struct *last =
this_cpu_ptr(&fpsimd_last_state);
@@ -1011,6 +1011,17 @@ static void fpsimd_bind_task_to_cpu(void)
}
}
+void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *st)
+{
+ struct fpsimd_last_state_struct *last =
+ this_cpu_ptr(&fpsimd_last_state);
+
+ WARN_ON(!in_softirq() && !irqs_disabled());
+
+ last->st = st;
+ last->sve_in_use = false;
+}
+
/*
* Load the userland FPSIMD state of 'current' from memory, but only if the
* FPSIMD state already held in the registers is /not/ the most recent FPSIMD
@@ -1063,7 +1074,7 @@ void fpsimd_flush_task_state(struct task_struct *t)
t->thread.fpsimd_cpu = NR_CPUS;
}
-static inline void fpsimd_flush_cpu_state(void)
+void fpsimd_flush_cpu_state(void)
{
__this_cpu_write(fpsimd_last_state.st, NULL);
}
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index a2e3a5a..47b23bf 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -39,6 +39,7 @@ config KVM
select HAVE_KVM_IRQ_ROUTING
select IRQ_BYPASS_MANAGER
select HAVE_KVM_IRQ_BYPASS
+ select HAVE_KVM_VCPU_RUN_PID_CHANGE
---help---
Support hosting virtualized guest machines.
We don't support KVM with 16K page tables yet, due to the multiple
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 93afff9..0f2a135 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -19,7 +19,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o va_layout.o
kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
kvm-$(CONFIG_KVM_ARM_HOST) += guest.o debug.o reset.o sys_regs.o sys_regs_generic_v8.o
-kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o
+kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/aarch32.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic.o
diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
new file mode 100644
index 0000000..d9b5f73
--- /dev/null
+++ b/arch/arm64/kvm/fpsimd.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * arch/arm64/kvm/fpsimd.c: Guest/host FPSIMD context coordination helpers
+ *
+ * Copyright 2018 Arm Limited
+ * Author: Dave Martin <Dave.Martin@arm.com>
+ */
+#include <linux/bottom_half.h>
+#include <linux/sched.h>
+#include <linux/thread_info.h>
+#include <linux/kvm_host.h>
+#include <asm/kvm_asm.h>
+#include <asm/kvm_host.h>
+#include <asm/kvm_mmu.h>
+
+/*
+ * Called on entry to KVM_RUN unless this vcpu previously ran at least
+ * once and the most recent prior KVM_RUN for this vcpu was called from
+ * the same task as current (highly likely).
+ *
+ * This is guaranteed to execute before kvm_arch_vcpu_load_fp(vcpu),
+ * such that on entering hyp the relevant parts of current are already
+ * mapped.
+ */
+int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)
+{
+ int ret;
+
+ struct thread_info *ti = ¤t->thread_info;
+ struct user_fpsimd_state *fpsimd = ¤t->thread.uw.fpsimd_state;
+
+ /*
+ * Make sure the host task thread flags and fpsimd state are
+ * visible to hyp:
+ */
+ ret = create_hyp_mappings(ti, ti + 1, PAGE_HYP);
+ if (ret)
+ goto error;
+
+ ret = create_hyp_mappings(fpsimd, fpsimd + 1, PAGE_HYP);
+ if (ret)
+ goto error;
+
+ vcpu->arch.host_thread_info = kern_hyp_va(ti);
+ vcpu->arch.host_fpsimd_state = kern_hyp_va(fpsimd);
+error:
+ return ret;
+}
+
+/*
+ * Prepare vcpu for saving the host's FPSIMD state and loading the guest's.
+ * The actual loading is done by the FPSIMD access trap taken to hyp.
+ *
+ * Here, we just set the correct metadata to indicate that the FPSIMD
+ * state in the cpu regs (if any) belongs to current on the host.
+ *
+ * TIF_SVE is backed up here, since it may get clobbered with guest state.
+ * This flag is restored by kvm_arch_vcpu_put_fp(vcpu).
+ */
+void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
+{
+ BUG_ON(system_supports_sve());
+ BUG_ON(!current->mm);
+
+ vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED | KVM_ARM64_HOST_SVE_IN_USE);
+ vcpu->arch.flags |= KVM_ARM64_FP_HOST;
+ if (test_thread_flag(TIF_SVE))
+ vcpu->arch.flags |= KVM_ARM64_HOST_SVE_IN_USE;
+}
+
+/*
+ * If the guest FPSIMD state was loaded, update the host's context
+ * tracking data mark the CPU FPSIMD regs as dirty and belonging to vcpu
+ * so that they will be written back if the kernel clobbers them due to
+ * kernel-mode NEON before re-entry into the guest.
+ */
+void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu)
+{
+ WARN_ON_ONCE(!irqs_disabled());
+
+ if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
+ fpsimd_bind_state_to_cpu(&vcpu->arch.ctxt.gp_regs.fp_regs);
+ clear_thread_flag(TIF_FOREIGN_FPSTATE);
+ clear_thread_flag(TIF_SVE);
+ }
+}
+
+/*
+ * Write back the vcpu FPSIMD regs if they are dirty, and invalidate the
+ * cpu FPSIMD regs so that they can't be spuriously reused if this vcpu
+ * disappears and another task or vcpu appears that recycles the same
+ * struct fpsimd_state.
+ */
+void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
+{
+ local_bh_disable();
+
+ update_thread_flag(TIF_SVE,
+ vcpu->arch.flags & KVM_ARM64_HOST_SVE_IN_USE);
+
+ if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
+ /* Clean guest FP state to memory and invalidate cpu view */
+ fpsimd_save();
+ fpsimd_flush_cpu_state();
+ set_thread_flag(TIF_FOREIGN_FPSTATE);
+ } else if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) {
+ /* Ensure user trap controls are correctly restored */
+ fpsimd_bind_task_to_cpu();
+ }
+
+ local_bh_enable();
+}
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index c0796c4..118f300 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -23,19 +23,21 @@
#include <asm/kvm_asm.h>
#include <asm/kvm_emulate.h>
+#include <asm/kvm_host.h>
#include <asm/kvm_hyp.h>
#include <asm/kvm_mmu.h>
#include <asm/fpsimd.h>
#include <asm/debug-monitors.h>
+#include <asm/thread_info.h>
-static bool __hyp_text __fpsimd_enabled_nvhe(void)
+/* Check whether the FP regs were dirtied while in the host-side run loop: */
+static bool __hyp_text update_fp_enabled(struct kvm_vcpu *vcpu)
{
- return !(read_sysreg(cptr_el2) & CPTR_EL2_TFP);
-}
+ if (vcpu->arch.host_thread_info->flags & _TIF_FOREIGN_FPSTATE)
+ vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED |
+ KVM_ARM64_FP_HOST);
-static bool fpsimd_enabled_vhe(void)
-{
- return !!(read_sysreg(cpacr_el1) & CPACR_EL1_FPEN);
+ return !!(vcpu->arch.flags & KVM_ARM64_FP_ENABLED);
}
/* Save the 32-bit only FPSIMD system register state */
@@ -92,7 +94,10 @@ static void activate_traps_vhe(struct kvm_vcpu *vcpu)
val = read_sysreg(cpacr_el1);
val |= CPACR_EL1_TTA;
- val &= ~(CPACR_EL1_FPEN | CPACR_EL1_ZEN);
+ val &= ~CPACR_EL1_ZEN;
+ if (!update_fp_enabled(vcpu))
+ val &= ~CPACR_EL1_FPEN;
+
write_sysreg(val, cpacr_el1);
write_sysreg(kvm_get_hyp_vector(), vbar_el1);
@@ -105,7 +110,10 @@ static void __hyp_text __activate_traps_nvhe(struct kvm_vcpu *vcpu)
__activate_traps_common(vcpu);
val = CPTR_EL2_DEFAULT;
- val |= CPTR_EL2_TTA | CPTR_EL2_TFP | CPTR_EL2_TZ;
+ val |= CPTR_EL2_TTA | CPTR_EL2_TZ;
+ if (!update_fp_enabled(vcpu))
+ val |= CPTR_EL2_TFP;
+
write_sysreg(val, cptr_el2);
}
@@ -321,8 +329,6 @@ 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);
@@ -332,14 +338,19 @@ void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
isb();
- host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
- __fpsimd_save_state(&host_ctxt->gp_regs.fp_regs);
+ if (vcpu->arch.flags & KVM_ARM64_FP_HOST) {
+ __fpsimd_save_state(vcpu->arch.host_fpsimd_state);
+ vcpu->arch.flags &= ~KVM_ARM64_FP_HOST;
+ }
+
__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);
+
+ vcpu->arch.flags |= KVM_ARM64_FP_ENABLED;
}
/*
@@ -418,7 +429,6 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
{
struct kvm_cpu_context *host_ctxt;
struct kvm_cpu_context *guest_ctxt;
- bool fp_enabled;
u64 exit_code;
host_ctxt = vcpu->arch.host_cpu_context;
@@ -440,19 +450,14 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
/* And we're baaack! */
} while (fixup_guest_exit(vcpu, &exit_code));
- fp_enabled = fpsimd_enabled_vhe();
-
sysreg_save_guest_state_vhe(guest_ctxt);
__deactivate_traps(vcpu);
sysreg_restore_host_state_vhe(host_ctxt);
- if (fp_enabled) {
- __fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
- __fpsimd_restore_state(&host_ctxt->gp_regs.fp_regs);
+ if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED)
__fpsimd_save_fpexc32(vcpu);
- }
__debug_switch_to_host(vcpu);
@@ -464,7 +469,6 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
{
struct kvm_cpu_context *host_ctxt;
struct kvm_cpu_context *guest_ctxt;
- bool fp_enabled;
u64 exit_code;
vcpu = kern_hyp_va(vcpu);
@@ -496,8 +500,6 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
/* And we're baaack! */
} while (fixup_guest_exit(vcpu, &exit_code));
- fp_enabled = __fpsimd_enabled_nvhe();
-
__sysreg_save_state_nvhe(guest_ctxt);
__sysreg32_save_state(vcpu);
__timer_disable_traps(vcpu);
@@ -508,11 +510,8 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
__sysreg_restore_state_nvhe(host_ctxt);
- if (fp_enabled) {
- __fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
- __fpsimd_restore_state(&host_ctxt->gp_regs.fp_regs);
+ if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED)
__fpsimd_save_fpexc32(vcpu);
- }
/*
* This must come after restoring the host sysregs, since a non-VHE
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index a4c1b76..bee226c 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -363,10 +363,12 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
kvm_vgic_load(vcpu);
kvm_timer_vcpu_load(vcpu);
kvm_vcpu_load_sysregs(vcpu);
+ kvm_arch_vcpu_load_fp(vcpu);
}
void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
{
+ kvm_arch_vcpu_put_fp(vcpu);
kvm_vcpu_put_sysregs(vcpu);
kvm_timer_vcpu_put(vcpu);
kvm_vgic_put(vcpu);
@@ -778,6 +780,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
if (static_branch_unlikely(&userspace_irqchip_in_use))
kvm_timer_sync_hwstate(vcpu);
+ kvm_arch_vcpu_ctxsync_fp(vcpu);
+
/*
* We may have taken a host interrupt in HYP mode (ie
* while executing the guest). This interrupt is still
--
2.1.4
^ permalink raw reply related
* [PATCH v9 09/16] arm64/sve: Move read_zcr_features() out of cpufeature.h
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-1-git-send-email-Dave.Martin@arm.com>
Having read_zcr_features() inline in cpufeature.h results in that
header requiring #includes which make it hard to include
<asm/fpsimd.h> elsewhere without triggering header inclusion
cycles.
This is not a hot-path function and arguably should not be in
cpufeature.h in the first place, so this patch moves it to
fpsimd.c, compiled conditionally if CONFIG_ARM64_SVE=y.
This allows some SVE-related #includes to be dropped from
cpufeature.h, which will ease future maintenance.
A couple of missing #includes of <asm/fpsimd.h> are exposed by this
change under arch/arm64/. This patch adds the missing #includes as
necessary.
No functional change.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/include/asm/cpufeature.h | 29 -----------------------------
arch/arm64/include/asm/fpsimd.h | 2 ++
arch/arm64/include/asm/processor.h | 1 +
arch/arm64/kernel/fpsimd.c | 28 ++++++++++++++++++++++++++++
arch/arm64/kernel/ptrace.c | 1 +
5 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 09b0f2a..0a6b713 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -11,9 +11,7 @@
#include <asm/cpucaps.h>
#include <asm/cputype.h>
-#include <asm/fpsimd.h>
#include <asm/hwcap.h>
-#include <asm/sigcontext.h>
#include <asm/sysreg.h>
/*
@@ -510,33 +508,6 @@ static inline bool system_supports_sve(void)
cpus_have_const_cap(ARM64_SVE);
}
-/*
- * Read the pseudo-ZCR used by cpufeatures to identify the supported SVE
- * vector length.
- *
- * Use only if SVE is present.
- * This function clobbers the SVE vector length.
- */
-static inline u64 read_zcr_features(void)
-{
- u64 zcr;
- unsigned int vq_max;
-
- /*
- * Set the maximum possible VL, and write zeroes to all other
- * bits to see if they stick.
- */
- sve_kernel_enable(NULL);
- write_sysreg_s(ZCR_ELx_LEN_MASK, SYS_ZCR_EL1);
-
- zcr = read_sysreg_s(SYS_ZCR_EL1);
- zcr &= ~(u64)ZCR_ELx_LEN_MASK; /* find sticky 1s outside LEN field */
- vq_max = sve_vq_from_vl(sve_get_vl());
- zcr |= vq_max - 1; /* set LEN field to maximum effective value */
-
- return zcr;
-}
-
#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
index 3e00f70..fb60b22 100644
--- a/arch/arm64/include/asm/fpsimd.h
+++ b/arch/arm64/include/asm/fpsimd.h
@@ -69,6 +69,8 @@ extern unsigned int sve_get_vl(void);
struct arm64_cpu_capabilities;
extern void sve_kernel_enable(const struct arm64_cpu_capabilities *__unused);
+extern u64 read_zcr_features(void);
+
extern int __ro_after_init sve_max_vl;
#ifdef CONFIG_ARM64_SVE
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 7675989..f902b6d 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -40,6 +40,7 @@
#include <asm/alternative.h>
#include <asm/cpufeature.h>
+#include <asm/fpsimd.h>
#include <asm/hw_breakpoint.h>
#include <asm/lse.h>
#include <asm/pgtable-hwdef.h>
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 051b455..5c33667 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -37,6 +37,7 @@
#include <linux/sched/task_stack.h>
#include <linux/signal.h>
#include <linux/slab.h>
+#include <linux/stddef.h>
#include <linux/sysctl.h>
#include <asm/esr.h>
@@ -754,6 +755,33 @@ void sve_kernel_enable(const struct arm64_cpu_capabilities *__always_unused p)
isb();
}
+/*
+ * Read the pseudo-ZCR used by cpufeatures to identify the supported SVE
+ * vector length.
+ *
+ * Use only if SVE is present.
+ * This function clobbers the SVE vector length.
+ */
+u64 read_zcr_features(void)
+{
+ u64 zcr;
+ unsigned int vq_max;
+
+ /*
+ * Set the maximum possible VL, and write zeroes to all other
+ * bits to see if they stick.
+ */
+ sve_kernel_enable(NULL);
+ write_sysreg_s(ZCR_ELx_LEN_MASK, SYS_ZCR_EL1);
+
+ zcr = read_sysreg_s(SYS_ZCR_EL1);
+ zcr &= ~(u64)ZCR_ELx_LEN_MASK; /* find sticky 1s outside LEN field */
+ vq_max = sve_vq_from_vl(sve_get_vl());
+ zcr |= vq_max - 1; /* set LEN field to maximum effective value */
+
+ return zcr;
+}
+
void __init sve_setup(void)
{
u64 zcr;
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 7ff81fe..78889c4 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -44,6 +44,7 @@
#include <asm/compat.h>
#include <asm/cpufeature.h>
#include <asm/debug-monitors.h>
+#include <asm/fpsimd.h>
#include <asm/pgtable.h>
#include <asm/stacktrace.h>
#include <asm/syscall.h>
--
2.1.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox