* [PATCH v2 11/11] coresight: etm4x: docs: Adds detailed document for programming etm4x.
From: Mike Leach @ 2019-08-29 21:33 UTC (permalink / raw)
To: mike.leach, mathieu.poirier, linux-arm-kernel, coresight,
linux-doc
Cc: suzuki.poulose, corbet, gregkh
In-Reply-To: <20190829213321.4092-1-mike.leach@linaro.org>
Add in detailed programmers reference for users wanting to program the
CoreSight ETM 4.x driver using sysfs.
Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
.../coresight/coresight-etm4x-reference.txt | 458 ++++++++++++++++++
1 file changed, 458 insertions(+)
create mode 100644 Documentation/trace/coresight/coresight-etm4x-reference.txt
diff --git a/Documentation/trace/coresight/coresight-etm4x-reference.txt b/Documentation/trace/coresight/coresight-etm4x-reference.txt
new file mode 100644
index 000000000000..f0c370870992
--- /dev/null
+++ b/Documentation/trace/coresight/coresight-etm4x-reference.txt
@@ -0,0 +1,458 @@
+ETMv4 sysfs linux driver programming reference - v2.
+====================================================
+
+Supplement to existing ETMv4 driver documentation.
+
+Sysfs files and directories
+---------------------------
+
+Root: /sys/bus/coresight/devices/etm<N>
+
+
+The following paragraphs explain the association between sysfs files and the
+ETMv4 registers that they effect. Note the register names are given without
+the ‘TRC’ prefix.
+
+File : mode (rw)
+Trace Registers : {CONFIGR + others}
+Notes : Bit select trace features. See ‘mode’ section below. Bits
+ in this will cause equivalent programming of trace config and
+ other registers to enable the features requested.
+Syntax & eg : 'echo bitfield > mode'
+ bitfield up to 32 bits setting trace features.
+Example : $> echo 0x > mode
+
+File : reset (wo)
+Trace Registers : All
+Notes : Reset all programming to trace nothing / no logic programmed.
+Syntax : 'echo 1 > reset'
+
+File : enable_source (wo)
+Trace Registers : PRGCTLR, All hardware regs.
+Notes : >0: Programs up the hardware with the current values held in
+ the driver and enables trace.
+ 0: disable trace hardware.
+Syntax : 'echo 1 > enable_source'
+
+File : cpu (ro)
+Trace Registers : None.
+Notes : CPU ID that this ETM is attached to.
+Example :$> cat cpu
+ $> 0
+
+File : addr_idx (rw)
+Trace Registers : None.
+Notes : Virtual register to index address comparator and range
+ features. Set index for first of the pair in a range.
+Syntax : 'echo idx > addr_idx'
+ Where idx < nr_addr_cmp x 2
+
+File : addr_range (rw)
+Trace Registers : ACVR[idx, idx+1], VIIECTLR
+Notes : Pair of addresses for a range selected by addr_idx. Include
+ / exclude according to the optional parameter, or if omitted
+ uses the current ‘mode’ setting. Select comparator range in
+ control register. Error if index is odd value.
+Depends : mode, addr_idx
+Syntax : 'echo addr1 addr2 [exclude] > addr_range'
+ Where addr1 and addr2 define the range and addr1 < addr2.
+ Optional exclude value - 0 for include, 1 for exclude.
+Example : $> echo 0x0000 0x2000 0 > addr_range
+
+File : addr_single (rw)
+Trace Registers : ACVR[idx]
+Notes : Set a single address comparator according to addr_idx. This
+ is used if the address comparator is used as part of event
+ generation logic etc.
+Depends : addr_idx
+Syntax : 'echo addr1 > addr_single'
+
+File : addr_start (rw)
+Trace Registers : ACVR[idx], VISSCTLR
+Notes : Set a trace start address comparator according to addr_idx.
+ Select comparator in control register.
+Depends : addr_idx
+Syntax : 'echo addr1 > addr_start'
+
+File : addr_stop (rw)
+Trace Registers : ACVR[idx], VISSCTLR
+Notes : Set a trace stop address comparator according to addr_idx.
+ Select comparator in control register.
+Depends : addr_idx
+Syntax : 'echo addr1 > addr_stop'
+
+File : addr_context (rw)
+Trace Registers : ACATR[idx,{6:4}]
+Notes : Link context ID comparator to address comparator addr_idx
+Depends : addr_idx.
+Syntax : 'echo ctxt_idx > addr_context'
+ Where ctxt_idx is the index of the linked context id / vmid
+ comparator.
+
+File : addr_ctxtype (rw)
+Trace Registers : ACATR[idx,{3:2}]
+Notes : Input value string. Set type for linked context ID comparator
+Depends : addr_idx
+Syntax : 'echo type > addr_ctxtype'
+ Type one of {all, vmid, ctxid, none}
+Example : $> echo ctxid > addr_ctxtype
+
+File : addr_exlevel_s_ns (rw)
+Trace Registers : ACATR[idx,{14:8}]
+Notes : Set the ELx secure and non-secure matching bits for the
+ selected address comparator
+Depends : addr_idx
+Syntax : 'echo val > addr_exlevel_s_ns'
+ val is a 7 bit value for exception levels to exclude. Input
+ value shifted to correct bits in register.
+Example : $> echo 0x4F > addr_exlevel_s_ns
+
+File : addr_instdatatype (rw)
+Trace Registers : ACATR[idx,{1:0}]
+Notes : Set the comparator address type for matching. Driver only
+ supports setting instruction address type.
+Depends : addr_idx
+
+File : addr_cmp_view (ro)
+Trace Registers : ACVR[idx, idx+1], ACATR[idx], VIIECTLR
+Notes : Read the currently selected address comparator. If part of
+ address range then display both addresses.
+Depends : addr_idx
+Syntax : 'cat addr_cmp_view'
+Example : $> cat addr_cmp_view
+ addr_cmp[0] range 0x0 0xffffffffffffffff include ctrl(0x4b00)
+
+File : nr_addr_cmp (ro)
+Trace Registers : From IDR4
+Notes : Number of address comparator pairs
+
+File : sshot_idx (rw)
+Trace Registers : None
+Notes : Select single shot register set.
+
+File : sshot_ctrl (rw)
+Trace Registers : SSCCR[idx]
+Notes : Access a single shot comparator control register.
+Depends : sshot_idx
+Syntax : 'echo val > sshot_ctrl'
+ Writes val into the selected control register.
+
+File : sshot_status (ro)
+Trace Registers : SSCSR[idx]
+Notes : Read a single shot comparator status register
+Depends : sshot_idx
+Syntax : 'cat sshot_status'
+ Read status.
+Example : $> cat sshot_status
+ 0x1
+
+File : sshot_pe_ctrl (rw)
+Trace Registers : SSPCICR[idx]
+Notes : Access a single shot PE comparator input control register.
+Depends : sshot_idx
+Syntax : echo val > sshot_pe_ctrl
+ Writes val into the selected control register.
+
+File : ns_exlevel_vinst (rw)
+Trace Registers : VICTLR{23:20}
+Notes : Program non-secure exception level filters. Set / clear NS
+ exception filter bits. Setting ‘1’ excludes trace from the
+ exception level.
+Syntax : 'echo bitfield > ns_exlevel_viinst'
+ Where bitfield contains bits to set clear for EL0 to EL2
+Example : %> echo 0x4 > ns_exlevel_viinst
+ ; Exclude EL2 NS trace.
+
+File : vinst_pe_cmp_start_stop (rw)
+Trace Registers : VIPCSSCTLR
+Notes : Access PE start stop comparator input control registers
+
+File : bb_ctrl (rw)
+Trace Registers : BBCTLR
+Notes : Define ranges that Branch Broadcast will operate in.
+ Default (0x0) is all addresses.
+Depends : BB enabled.
+
+File : cyc_threshold (rw)
+Trace Registers : CCCTLR
+Notes : Set the threshold for which cycle counts will be emitted.
+ Error if attempt to set below minimum defined in IDR3, masked
+ to width of valid bits.
+Depends : CC enabled.
+
+File : syncfreq (rw)
+Trace Registers : SYNCPR
+Notes : Set trace synchronisation period. Power of 2 value, 0 (off)
+ or 8-20. Driver defaults to 12 (every 4096 bytes).
+
+File : cntr_idx (rw)
+Trace Registers : none
+Notes : Select the counter to access
+Syntax : 'echo idx > cntr_idx'
+ Where idx < nr_cntr
+
+File : cntr_ctrl (rw)
+Trace Registers : CNTCTLR[idx]
+Notes : Set counter control value
+Depends : cntr_idx
+Syntax : 'echo val > cntr_ctrl'
+ Where val is per ETMv4 spec.
+
+File : cntrldvr (rw)
+Trace Registers : CNTRLDVR[idx]
+Notes : Set counter reload value
+Depends : cntr_idx
+Syntax : 'echo val > cntrldvr'
+ Where val is per ETMv4 spec.
+
+File : nr_cntr (ro)
+Trace Registers : From IDR5
+Notes : Number of counters implemented.
+
+File : ctxid_idx (rw)
+Trace Registers : None
+Notes : Select the context ID comparator to access
+Syntax : 'echo idx > ctxid_idx'
+ Where idx < numcidc
+
+File : ctxid_pid (rw)
+Trace Registers : CIDCVR[idx]
+Notes : Set the context ID comparator value
+Depends : ctxid_idx
+
+File : ctxid_masks (rw)
+Trace Registers : CIDCCTLR0, CIDCCTLR1, CIDCVR<0-7>
+Notes : Pair of values to set the byte masks for 1-8 context ID
+ comparators. Automatically clears masked bytes to 0 in CID
+ value registers.
+Syntax : 'echo m3m2m1m0 [m7m6m5m4] > ctxid_masks'
+ 32 bit values made up of mask bytes, where mN represents a
+ byte mask value for Ctxt ID comparator N.
+ Second value not required on systems that have fewer than 4
+ context ID comparators
+
+File : numcidc (ro)
+Trace Registers : From IDR4
+Notes : Number of Context ID comparators
+
+File : vmid_idx (rw)
+Trace Registers : None
+Notes : Select the VM ID comparator to access.
+Syntax : 'echo idx > vmid_idx'
+ Where idx < numvmidc
+
+File : vmid_val (rw)
+Trace Registers : VMIDCVR[idx]
+Notes : Set the VM ID comparator value
+Depends : vmid_idx
+
+File : vmid_masks (rw)
+Trace Registers : VMIDCCTLR0, VMIDCCTLR1, VMIDCVR<0-7>
+Notes : Pair of values to set the byte masks for 1-8 VM ID
+ comparators. Automatically clears masked bytes to 0 in VMID
+ value registers.
+Syntax : 'echo m3m2m1m0 [m7m6m5m4] > vmid_masks'
+ Where mN represents a byte mask value for VMID comparator N.
+ Second value not required on systems that have fewer than
+ 4 VMID comparators.
+
+File : numvmidc (ro)
+Trace Registers : From IDR4
+Notes : Number of VMID comparators
+
+File : res_idx (rw)
+Trace Registers : None.
+Notes : Select the resource selector control to access. Must be 2 or
+ higher as selectors 0 and 1 are hardwired.
+Syntax : 'echo idx > res_idx'
+ Where 2 <= idx < nr_resource x 2
+
+File : res_ctrl (rw)
+Trace Registers : RSCTLR[idx]
+Notes : Set resource selector control value. Value per ETMv4 spec.
+Depends : res_idx
+Syntax : 'echo val > res_cntr'
+ Where val is per ETMv4 spec.
+
+File : nr_resource (ro)
+Trace Registers : From IDR4
+Notes : Number of resource selector pairs
+
+File : event (rw)
+Trace Registers : EVENTCTRL0R
+Notes : Set up to 4 implemented event fields.
+Syntax : 'echo ev3ev2ev1ev0 > event'
+ Where evN is an 8 bit event field. Up to 4 event fields make up
+ the 32bit input value. Number of valid fields implementation
+ dependent defined in IDR0.
+
+File : event_instren (rw)
+Trace Registers : EVENTCTRL1R
+Notes : Choose events which insert event packets into trace stream.
+Depends : EVENTCTRL0R
+Syntax : 'echo bitfield > event_instren'
+ Where bitfield is up to 4 bits according to number of event
+ fields.
+
+File : event_ts (rw)
+Trace Registers : TSCTLR
+Notes : Set the event that will generate timestamp requests.
+Depends : TS activated
+Syntax : 'echo evfield > event_ts'
+ Where evfield is an 8 bit event selector.
+
+File : seq_idx (rw)
+Trace Registers : None
+Notes : Sequencer event register select - 0 to 2
+
+
+File : seq_state (rw)
+Trace Registers : SEQSTR
+Notes : Sequencer current state - 0 to 3.
+
+File : seq_event (rw)
+Trace Registers : SEQEVR[idx]
+Notes : State transition event registers
+Depends : seq_idx
+Syntax : 'echo evBevF > seq_event'
+ Where evBevF is a 16 bit value made up of two event selectors,
+ evB - back, evF - forwards.
+
+File : seq_reset_event (rw)
+Trace Registers : SEQRSTEVR
+Notes : Sequencer reset event
+Syntax : 'echo evfield > seq_reset_event'
+ Where evfield is an 8 bit event selector.
+
+File : nrseqstate (ro)
+Trace Registers : From IDR5
+Notes : Number of sequencer states (0 or 4)
+
+File : nr_pe_cmp (ro)
+Trace Registers : From IDR4
+Notes : Number of PE comparator inputs
+
+File : nr_ext_inp (ro)
+Trace Registers : From IDR5
+Notes : Number of external inputs
+
+File : nr_ss_cmp (ro)
+Trace Registers : From IDR4
+Notes : Number of Single Shot control registers
+
+Note: When programming any address comparator the driver will tag the
+comparator with a type used - i.e. RANGE, SINGLE, START, STOP. Once this tag
+is set, then only the values can be changed using the same sysfs file / type
+used to program it.
+
+Thus:-
+% echo 0 > addr_idx ; select address comparator 0
+% echo 0x1000 0x5000 0 > addr_range ; set address range on comparators 0 and 1.
+% echo 0x2000 > addr_start ; this will error as comparator 0 is a
+ ; range comparator
+% echo 2 > addr_idx ; select address comparator 2
+% echo 0x2000 > addr_start ; this is OK as comparator 2 is unused,
+% echo 0x3000 > addr_stop ; this will error as comparator 2 a start
+ ; address comparator
+% echo 2 > addr_idx ; select address comparator 3
+% echo 0x3000 > addr_stop ; this is OK
+
+To remove programming on all the comparators (and all the other hardware) use
+the reset parameter:
+
+% echo 1 > reset
+
+The ‘mode’ sysfs parameter.
+---------------------------
+
+This is a bitfield selection parameter that sets the overall trace mode for the
+ETM. The table below describes the bits, using the defines from the driver
+source file, along with a description of the feature these represent. Many
+features are optional and therefore dependent on implementation in the
+hardware.
+
+Bit assignements shown below:-
+
+bit (0) : #define ETM_MODE_EXCLUDE
+description : This is the default value for the include / exclude function when
+ setting address ranges. Set 1 for exclude range. When the mode
+ parameter is set this value is applied to the currently indexed
+ address range.
+
+bit (4) : #define ETM_MODE_BB
+description : Set to enable branch broadcast if supported in hardware [IDR0].
+
+bit (5) : #define ETMv4_MODE_CYCACC
+description : Set to enable cycle accurate trace if supported [IDR0].
+
+bit (6) : ETMv4_MODE_CTXID
+description : Set to enable context ID tracing if supported in hardware [IDR2].
+
+bit (7) : ETM_MODE_VMID
+description : Set to enable virtual machine ID tracing if supported [IDR2].
+
+bit (11) : ETMv4_MODE_TIMESTAMP
+description : Set to enable timestamp generation if supported [IDR0].
+
+bit (12) : ETM_MODE_RETURNSTACK
+description : Set to enable trace return stack use if supported [IDR0].
+
+bit (13-14) : ETM_MODE_QELEM(val)
+description : ‘val’ determines level of Q element support enabled if
+ implemented by the ETM [IDR0]
+
+bit (19) : ETM_MODE_ATB_TRIGGER
+description : Set to enable the ATBTRIGGER bit in the event control register
+ [EVENTCTLR1] if supported [IDR5].
+
+bit (20) : ETM_MODE_LPOVERRIDE
+description : Set to enable the LPOVERRIDE bit in the event control register
+ [EVENTCTLR1], if supported [IDR5].
+
+bit (21) : ETM_MODE_ISTALL_EN
+description : Set to enable the ISTALL bit in the stall control register
+ [STALLCTLR]
+
+bit (23) : ETM_MODE_INSTPRIO
+description : Set to enable the INSTPRIORITY bit in the stall control register
+ [STALLCTLR] , if supported [IDR0].
+
+bit (24) : ETM_MODE_NOOVERFLOW
+description : Set to enable the NOOVERFLOW bit in the stall control register
+ [STALLCTLR], if supported [IDR3].
+
+bit (25) : ETM_MODE_TRACE_RESET
+description : Set to enable the TRCRESET bit in the viewinst control register
+ [VICTLR] , if supported [IDR3].
+
+bit (26) : ETM_MODE_TRACE_ERR
+description : Set to enable the TRCCTRL bit in the viewinst control register
+ [VICTLR].
+
+bit (27) : ETM_MODE_VIEWINST_STARTSTOP
+description : Set the initial state value of the ViewInst start / stop logic
+ in the viewinst control register [VICTLR]
+
+bit (30) : ETM_MODE_EXCL_KERN
+description : Set default trace setup to exclude kernel mode trace (see note a)
+
+bit (31) : ETM_MODE_EXCL_USER
+description : Set default trace setup to exclude user space trace (see note a)
+
+Note a) On startup the ETM is programmed to trace the complete address space
+using address range comparator 0. ‘mode’ bits 30 / 31 modify this setting to
+set EL exclude bits for NS state in either user space (EL0) or kernel space
+(EL1) in the address range comparator. (the default setting excludes all
+secure EL, and NS EL2)
+
+Once the reset parameter has been used, and/or custom programming has been
+implemented - using these bits will result in the EL bits for address
+comparator 0 being set in the same way.
+
+Note b) Bits 2-3, 8-10, 15-16, 18, 22, control features that only work with
+data trace. As A profile data trace is architecturally prohibited in ETMv4,
+these have been omitted here. Possible uses could be where a kernel has
+support for control of R or M profile infrastructure as part of a heterogeneous
+system.
+
+Bits 17, 28-29 are unused.
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 4/5] rcu: Remove kfree_rcu() special casing and lazy handling
From: Joel Fernandes @ 2019-08-29 22:13 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, byungchul.park, Josh Triplett, Lai Jiangshan,
linux-doc, Mathieu Desnoyers, rcu, Steven Rostedt, kernel-team
In-Reply-To: <20190828214629.GZ26530@linux.ibm.com>
All comments addressed on this one, thanks!
- Joel
On Wed, Aug 28, 2019 at 02:46:29PM -0700, Paul E. McKenney wrote:
> On Tue, Aug 27, 2019 at 03:01:58PM -0400, Joel Fernandes (Google) wrote:
> > Remove kfree_rcu() special casing and lazy handling from RCU.
> > For Tiny RCU we fold the special handling into just Tiny RCU code.
> >
> > Suggested-by: Paul E. McKenney <paulmck@linux.ibm.com>
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>
> Nice! Several comments inline below.
>
> Thanx, Paul
>
> > ---
> > Documentation/RCU/stallwarn.txt | 13 +++------
> > include/linux/rcu_segcblist.h | 2 --
> > include/trace/events/rcu.h | 32 +++++++++-------------
> > kernel/rcu/rcu.h | 27 -------------------
> > kernel/rcu/rcu_segcblist.c | 25 +++--------------
> > kernel/rcu/rcu_segcblist.h | 25 ++---------------
> > kernel/rcu/srcutree.c | 4 +--
> > kernel/rcu/tiny.c | 29 +++++++++++++++++++-
> > kernel/rcu/tree.c | 48 ++++++++++++++++++++++++---------
> > kernel/rcu/tree.h | 1 -
> > kernel/rcu/tree_plugin.h | 42 +++++++----------------------
> > kernel/rcu/tree_stall.h | 6 ++---
> > 12 files changed, 97 insertions(+), 157 deletions(-)
> >
> > diff --git a/Documentation/RCU/stallwarn.txt b/Documentation/RCU/stallwarn.txt
> > index f48f4621ccbc..2c02b8de01e7 100644
> > --- a/Documentation/RCU/stallwarn.txt
> > +++ b/Documentation/RCU/stallwarn.txt
> > @@ -225,18 +225,12 @@ an estimate of the total number of RCU callbacks queued across all CPUs
> > In kernels with CONFIG_RCU_FAST_NO_HZ, more information is printed
> > for each CPU:
> >
> > - 0: (64628 ticks this GP) idle=dd5/3fffffffffffffff/0 softirq=82/543 last_accelerate: a345/d342 Nonlazy posted: ..D
> > + 0: (64628 ticks this GP) idle=dd5/3fffffffffffffff/0 softirq=82/543 last_accelerate: a345/d342 dyntick_enabled: 1
> >
> > The "last_accelerate:" prints the low-order 16 bits (in hex) of the
> > jiffies counter when this CPU last invoked rcu_try_advance_all_cbs()
> > from rcu_needs_cpu() or last invoked rcu_accelerate_cbs() from
> > -rcu_prepare_for_idle(). The "Nonlazy posted:" indicates lazy-callback
> > -status, so that an "l" indicates that all callbacks were lazy at the start
> > -of the last idle period and an "L" indicates that there are currently
> > -no non-lazy callbacks (in both cases, "." is printed otherwise, as
> > -shown above) and "D" indicates that dyntick-idle processing is enabled
> > -("." is printed otherwise, for example, if disabled via the "nohz="
> > -kernel boot parameter).
> > +rcu_prepare_for_idle().
> >
> > If the grace period ends just as the stall warning starts printing,
> > there will be a spurious stall-warning message, which will include
> > @@ -249,7 +243,8 @@ possible for a zero-jiffy stall to be flagged in this case, depending
> > on how the stall warning and the grace-period initialization happen to
> > interact. Please note that it is not possible to entirely eliminate this
> > sort of false positive without resorting to things like stop_machine(),
> > -which is overkill for this sort of problem.
> > +which is overkill for this sort of problem. "dyntick_enabled: 1" indicates that
> > +dyntick-idle processing is enabled.
>
> OK, I'll bite... Why not add the sentence to the earlier paragraph that
> used to discuss its "D" counterpart?
>
> > If all CPUs and tasks have passed through quiescent states, but the
> > grace period has nevertheless failed to end, the stall-warning splat
> > diff --git a/include/linux/rcu_segcblist.h b/include/linux/rcu_segcblist.h
> > index 646759042333..b36afe7b22c9 100644
> > --- a/include/linux/rcu_segcblist.h
> > +++ b/include/linux/rcu_segcblist.h
> > @@ -22,7 +22,6 @@ struct rcu_cblist {
> > struct rcu_head *head;
> > struct rcu_head **tail;
> > long len;
> > - long len_lazy;
> > };
> >
> > #define RCU_CBLIST_INITIALIZER(n) { .head = NULL, .tail = &n.head }
> > @@ -73,7 +72,6 @@ struct rcu_segcblist {
> > #else
> > long len;
> > #endif
> > - long len_lazy;
> > u8 enabled;
> > u8 offloaded;
> > };
> > diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h
> > index 694bd040cf51..0dd3478597ee 100644
> > --- a/include/trace/events/rcu.h
> > +++ b/include/trace/events/rcu.h
> > @@ -474,16 +474,14 @@ TRACE_EVENT_RCU(rcu_dyntick,
> > */
> > TRACE_EVENT_RCU(rcu_callback,
> >
> > - TP_PROTO(const char *rcuname, struct rcu_head *rhp, long qlen_lazy,
> > - long qlen),
> > + TP_PROTO(const char *rcuname, struct rcu_head *rhp, long qlen),
> >
> > - TP_ARGS(rcuname, rhp, qlen_lazy, qlen),
> > + TP_ARGS(rcuname, rhp, qlen),
> >
> > TP_STRUCT__entry(
> > __field(const char *, rcuname)
> > __field(void *, rhp)
> > __field(void *, func)
> > - __field(long, qlen_lazy)
> > __field(long, qlen)
> > ),
> >
> > @@ -491,13 +489,12 @@ TRACE_EVENT_RCU(rcu_callback,
> > __entry->rcuname = rcuname;
> > __entry->rhp = rhp;
> > __entry->func = rhp->func;
> > - __entry->qlen_lazy = qlen_lazy;
> > __entry->qlen = qlen;
> > ),
> >
> > - TP_printk("%s rhp=%p func=%ps %ld/%ld",
> > + TP_printk("%s rhp=%p func=%ps %ld",
> > __entry->rcuname, __entry->rhp, __entry->func,
> > - __entry->qlen_lazy, __entry->qlen)
> > + __entry->qlen)
> > );
> >
> > /*
> > @@ -511,15 +508,14 @@ TRACE_EVENT_RCU(rcu_callback,
> > TRACE_EVENT_RCU(rcu_kfree_callback,
> >
> > TP_PROTO(const char *rcuname, struct rcu_head *rhp, unsigned long offset,
> > - long qlen_lazy, long qlen),
> > + long qlen),
> >
> > - TP_ARGS(rcuname, rhp, offset, qlen_lazy, qlen),
> > + TP_ARGS(rcuname, rhp, offset, qlen),
> >
> > TP_STRUCT__entry(
> > __field(const char *, rcuname)
> > __field(void *, rhp)
> > __field(unsigned long, offset)
> > - __field(long, qlen_lazy)
> > __field(long, qlen)
> > ),
> >
> > @@ -527,13 +523,12 @@ TRACE_EVENT_RCU(rcu_kfree_callback,
> > __entry->rcuname = rcuname;
> > __entry->rhp = rhp;
> > __entry->offset = offset;
> > - __entry->qlen_lazy = qlen_lazy;
> > __entry->qlen = qlen;
> > ),
> >
> > - TP_printk("%s rhp=%p func=%ld %ld/%ld",
> > + TP_printk("%s rhp=%p func=%ld %ld",
> > __entry->rcuname, __entry->rhp, __entry->offset,
> > - __entry->qlen_lazy, __entry->qlen)
> > + __entry->qlen)
> > );
> >
> > /*
> > @@ -545,27 +540,24 @@ TRACE_EVENT_RCU(rcu_kfree_callback,
> > */
> > TRACE_EVENT_RCU(rcu_batch_start,
> >
> > - TP_PROTO(const char *rcuname, long qlen_lazy, long qlen, long blimit),
> > + TP_PROTO(const char *rcuname, long qlen, long blimit),
> >
> > - TP_ARGS(rcuname, qlen_lazy, qlen, blimit),
> > + TP_ARGS(rcuname, qlen, blimit),
> >
> > TP_STRUCT__entry(
> > __field(const char *, rcuname)
> > - __field(long, qlen_lazy)
> > __field(long, qlen)
> > __field(long, blimit)
> > ),
> >
> > TP_fast_assign(
> > __entry->rcuname = rcuname;
> > - __entry->qlen_lazy = qlen_lazy;
> > __entry->qlen = qlen;
> > __entry->blimit = blimit;
> > ),
> >
> > - TP_printk("%s CBs=%ld/%ld bl=%ld",
> > - __entry->rcuname, __entry->qlen_lazy, __entry->qlen,
> > - __entry->blimit)
> > + TP_printk("%s CBs=%ld bl=%ld",
> > + __entry->rcuname, __entry->qlen, __entry->blimit)
> > );
> >
> > /*
> > diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
> > index aeec70fda82c..3f5a30ca7ed7 100644
> > --- a/kernel/rcu/rcu.h
> > +++ b/kernel/rcu/rcu.h
> > @@ -198,33 +198,6 @@ static inline void debug_rcu_head_unqueue(struct rcu_head *head)
> > }
> > #endif /* #else !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> >
> > -void kfree(const void *);
> > -
> > -/*
> > - * Reclaim the specified callback, either by invoking it (non-lazy case)
> > - * or freeing it directly (lazy case). Return true if lazy, false otherwise.
> > - */
> > -static inline bool __rcu_reclaim(const char *rn, struct rcu_head *head)
> > -{
> > - rcu_callback_t f;
> > - unsigned long offset = (unsigned long)head->func;
> > -
> > - rcu_lock_acquire(&rcu_callback_map);
> > - if (__is_kfree_rcu_offset(offset)) {
> > - trace_rcu_invoke_kfree_callback(rn, head, offset);
> > - kfree((void *)head - offset);
> > - rcu_lock_release(&rcu_callback_map);
> > - return true;
> > - } else {
> > - trace_rcu_invoke_callback(rn, head);
> > - f = head->func;
> > - WRITE_ONCE(head->func, (rcu_callback_t)0L);
> > - f(head);
> > - rcu_lock_release(&rcu_callback_map);
> > - return false;
> > - }
> > -}
>
> Nice to see this one go! ;-)
>
> > #ifdef CONFIG_RCU_STALL_COMMON
> >
> > extern int rcu_cpu_stall_ftrace_dump;
> > diff --git a/kernel/rcu/rcu_segcblist.c b/kernel/rcu/rcu_segcblist.c
> > index cbc87b804db9..5f4fd3b8777c 100644
> > --- a/kernel/rcu/rcu_segcblist.c
> > +++ b/kernel/rcu/rcu_segcblist.c
> > @@ -20,14 +20,10 @@ void rcu_cblist_init(struct rcu_cblist *rclp)
> > rclp->head = NULL;
> > rclp->tail = &rclp->head;
> > rclp->len = 0;
> > - rclp->len_lazy = 0;
> > }
> >
> > /*
> > * Enqueue an rcu_head structure onto the specified callback list.
> > - * This function assumes that the callback is non-lazy because it
> > - * is intended for use by no-CBs CPUs, which do not distinguish
> > - * between lazy and non-lazy RCU callbacks.
> > */
> > void rcu_cblist_enqueue(struct rcu_cblist *rclp, struct rcu_head *rhp)
> > {
> > @@ -54,7 +50,6 @@ void rcu_cblist_flush_enqueue(struct rcu_cblist *drclp,
> > else
> > drclp->tail = &drclp->head;
> > drclp->len = srclp->len;
> > - drclp->len_lazy = srclp->len_lazy;
> > if (!rhp) {
> > rcu_cblist_init(srclp);
> > } else {
> > @@ -62,16 +57,12 @@ void rcu_cblist_flush_enqueue(struct rcu_cblist *drclp,
> > srclp->head = rhp;
> > srclp->tail = &rhp->next;
> > WRITE_ONCE(srclp->len, 1);
> > - srclp->len_lazy = 0;
> > }
> > }
> >
> > /*
> > * Dequeue the oldest rcu_head structure from the specified callback
> > - * list. This function assumes that the callback is non-lazy, but
> > - * the caller can later invoke rcu_cblist_dequeued_lazy() if it
> > - * finds otherwise (and if it cares about laziness). This allows
> > - * different users to have different ways of determining laziness.
> > + * list.
> > */
> > struct rcu_head *rcu_cblist_dequeue(struct rcu_cblist *rclp)
> > {
> > @@ -161,7 +152,6 @@ void rcu_segcblist_init(struct rcu_segcblist *rsclp)
> > for (i = 0; i < RCU_CBLIST_NSEGS; i++)
> > rsclp->tails[i] = &rsclp->head;
> > rcu_segcblist_set_len(rsclp, 0);
> > - rsclp->len_lazy = 0;
> > rsclp->enabled = 1;
> > }
> >
> > @@ -173,7 +163,6 @@ void rcu_segcblist_disable(struct rcu_segcblist *rsclp)
> > {
> > WARN_ON_ONCE(!rcu_segcblist_empty(rsclp));
> > WARN_ON_ONCE(rcu_segcblist_n_cbs(rsclp));
> > - WARN_ON_ONCE(rcu_segcblist_n_lazy_cbs(rsclp));
> > rsclp->enabled = 0;
> > }
> >
> > @@ -253,11 +242,9 @@ bool rcu_segcblist_nextgp(struct rcu_segcblist *rsclp, unsigned long *lp)
> > * absolutely not OK for it to ever miss posting a callback.
> > */
> > void rcu_segcblist_enqueue(struct rcu_segcblist *rsclp,
> > - struct rcu_head *rhp, bool lazy)
> > + struct rcu_head *rhp)
> > {
> > rcu_segcblist_inc_len(rsclp);
> > - if (lazy)
> > - rsclp->len_lazy++;
> > smp_mb(); /* Ensure counts are updated before callback is enqueued. */
> > rhp->next = NULL;
> > WRITE_ONCE(*rsclp->tails[RCU_NEXT_TAIL], rhp);
> > @@ -275,15 +262,13 @@ void rcu_segcblist_enqueue(struct rcu_segcblist *rsclp,
> > * period. You have been warned.
> > */
> > bool rcu_segcblist_entrain(struct rcu_segcblist *rsclp,
> > - struct rcu_head *rhp, bool lazy)
> > + struct rcu_head *rhp)
> > {
> > int i;
> >
> > if (rcu_segcblist_n_cbs(rsclp) == 0)
> > return false;
> > rcu_segcblist_inc_len(rsclp);
> > - if (lazy)
> > - rsclp->len_lazy++;
> > smp_mb(); /* Ensure counts are updated before callback is entrained. */
> > rhp->next = NULL;
> > for (i = RCU_NEXT_TAIL; i > RCU_DONE_TAIL; i--)
> > @@ -307,8 +292,6 @@ bool rcu_segcblist_entrain(struct rcu_segcblist *rsclp,
> > void rcu_segcblist_extract_count(struct rcu_segcblist *rsclp,
> > struct rcu_cblist *rclp)
> > {
> > - rclp->len_lazy += rsclp->len_lazy;
> > - rsclp->len_lazy = 0;
> > rclp->len = rcu_segcblist_xchg_len(rsclp, 0);
> > }
> >
> > @@ -361,9 +344,7 @@ void rcu_segcblist_extract_pend_cbs(struct rcu_segcblist *rsclp,
> > void rcu_segcblist_insert_count(struct rcu_segcblist *rsclp,
> > struct rcu_cblist *rclp)
> > {
> > - rsclp->len_lazy += rclp->len_lazy;
> > rcu_segcblist_add_len(rsclp, rclp->len);
> > - rclp->len_lazy = 0;
> > rclp->len = 0;
> > }
> >
> > diff --git a/kernel/rcu/rcu_segcblist.h b/kernel/rcu/rcu_segcblist.h
> > index 815c2fdd3fcc..5c293afc07b8 100644
> > --- a/kernel/rcu/rcu_segcblist.h
> > +++ b/kernel/rcu/rcu_segcblist.h
> > @@ -15,15 +15,6 @@ static inline long rcu_cblist_n_cbs(struct rcu_cblist *rclp)
> > return READ_ONCE(rclp->len);
> > }
> >
> > -/*
> > - * Account for the fact that a previously dequeued callback turned out
> > - * to be marked as lazy.
> > - */
> > -static inline void rcu_cblist_dequeued_lazy(struct rcu_cblist *rclp)
> > -{
> > - rclp->len_lazy--;
> > -}
> > -
> > void rcu_cblist_init(struct rcu_cblist *rclp);
> > void rcu_cblist_enqueue(struct rcu_cblist *rclp, struct rcu_head *rhp);
> > void rcu_cblist_flush_enqueue(struct rcu_cblist *drclp,
> > @@ -59,18 +50,6 @@ static inline long rcu_segcblist_n_cbs(struct rcu_segcblist *rsclp)
> > #endif
> > }
> >
> > -/* Return number of lazy callbacks in segmented callback list. */
> > -static inline long rcu_segcblist_n_lazy_cbs(struct rcu_segcblist *rsclp)
> > -{
> > - return rsclp->len_lazy;
> > -}
> > -
> > -/* Return number of lazy callbacks in segmented callback list. */
> > -static inline long rcu_segcblist_n_nonlazy_cbs(struct rcu_segcblist *rsclp)
> > -{
> > - return rcu_segcblist_n_cbs(rsclp) - rsclp->len_lazy;
> > -}
> > -
> > /*
> > * Is the specified rcu_segcblist enabled, for example, not corresponding
> > * to an offline CPU?
> > @@ -106,9 +85,9 @@ struct rcu_head *rcu_segcblist_first_cb(struct rcu_segcblist *rsclp);
> > struct rcu_head *rcu_segcblist_first_pend_cb(struct rcu_segcblist *rsclp);
> > bool rcu_segcblist_nextgp(struct rcu_segcblist *rsclp, unsigned long *lp);
> > void rcu_segcblist_enqueue(struct rcu_segcblist *rsclp,
> > - struct rcu_head *rhp, bool lazy);
> > + struct rcu_head *rhp);
> > bool rcu_segcblist_entrain(struct rcu_segcblist *rsclp,
> > - struct rcu_head *rhp, bool lazy);
> > + struct rcu_head *rhp);
> > void rcu_segcblist_extract_count(struct rcu_segcblist *rsclp,
> > struct rcu_cblist *rclp);
> > void rcu_segcblist_extract_done_cbs(struct rcu_segcblist *rsclp,
> > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > index 5dffade2d7cd..d0a9d5b69087 100644
> > --- a/kernel/rcu/srcutree.c
> > +++ b/kernel/rcu/srcutree.c
> > @@ -853,7 +853,7 @@ static void __call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
> > local_irq_save(flags);
> > sdp = this_cpu_ptr(ssp->sda);
> > spin_lock_rcu_node(sdp);
> > - rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp, false);
> > + rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
> > rcu_segcblist_advance(&sdp->srcu_cblist,
> > rcu_seq_current(&ssp->srcu_gp_seq));
> > s = rcu_seq_snap(&ssp->srcu_gp_seq);
> > @@ -1052,7 +1052,7 @@ void srcu_barrier(struct srcu_struct *ssp)
> > sdp->srcu_barrier_head.func = srcu_barrier_cb;
> > debug_rcu_head_queue(&sdp->srcu_barrier_head);
> > if (!rcu_segcblist_entrain(&sdp->srcu_cblist,
> > - &sdp->srcu_barrier_head, 0)) {
> > + &sdp->srcu_barrier_head)) {
> > debug_rcu_head_unqueue(&sdp->srcu_barrier_head);
> > atomic_dec(&ssp->srcu_barrier_cpu_cnt);
> > }
> > diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> > index 477b4eb44af5..cc64dc8b3893 100644
> > --- a/kernel/rcu/tiny.c
> > +++ b/kernel/rcu/tiny.c
> > @@ -73,6 +73,33 @@ void rcu_sched_clock_irq(int user)
> > }
> > }
> >
> > +void kfree(const void *);
> > +
> > +/*
> > + * Reclaim the specified callback, either by invoking it for non-kfree cases or
> > + * freeing it directly (for kfree). Return true if kfreeing, false otherwise.
> > + */
> > +static inline bool rcu_reclaim_tiny(struct rcu_head *head)
> > +{
> > + rcu_callback_t f;
> > + unsigned long offset = (unsigned long)head->func;
> > +
> > + rcu_lock_acquire(&rcu_callback_map);
> > + if (__is_kfree_rcu_offset(offset)) {
> > + trace_rcu_invoke_kfree_callback("", head, offset);
> > + kfree((void *)head - offset);
> > + rcu_lock_release(&rcu_callback_map);
> > + return true;
> > + }
> > +
> > + trace_rcu_invoke_callback("", head);
> > + f = head->func;
> > + WRITE_ONCE(head->func, (rcu_callback_t)0L);
> > + f(head);
> > + rcu_lock_release(&rcu_callback_map);
> > + return false;
> > +}
>
> Even if it did show up here in the meantime. ;-)
>
> > /* Invoke the RCU callbacks whose grace period has elapsed. */
> > static __latent_entropy void rcu_process_callbacks(struct softirq_action *unused)
> > {
> > @@ -100,7 +127,7 @@ static __latent_entropy void rcu_process_callbacks(struct softirq_action *unused
> > prefetch(next);
> > debug_rcu_head_unqueue(list);
> > local_bh_disable();
> > - __rcu_reclaim("", list);
> > + rcu_reclaim_tiny(list);
> > local_bh_enable();
> > list = next;
> > }
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 64568f12641d..12c17e10f2b4 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -2129,6 +2129,23 @@ int rcutree_dead_cpu(unsigned int cpu)
> > return 0;
> > }
> >
> > +/*
> > + * Reclaim the specified callback.
> > + */
> > +static inline void rcu_reclaim_tree(const char *rn, struct rcu_head *head)
> > +{
> > + rcu_callback_t f;
> > +
> > + rcu_lock_acquire(&rcu_callback_map);
> > + trace_rcu_invoke_callback(rn, head);
> > +
> > + f = head->func;
> > + WRITE_ONCE(head->func, (rcu_callback_t)0L);
> > + f(head);
> > +
> > + rcu_lock_release(&rcu_callback_map);
> > +}
>
> Might be worth inlining this one, as you did for kfree_rcu().
>
> > /*
> > * Invoke any RCU callbacks that have made it to the end of their grace
> > * period. Thottle as specified by rdp->blimit.
> > @@ -2146,7 +2163,6 @@ static void rcu_do_batch(struct rcu_data *rdp)
> > /* If no callbacks are ready, just return. */
> > if (!rcu_segcblist_ready_cbs(&rdp->cblist)) {
> > trace_rcu_batch_start(rcu_state.name,
> > - rcu_segcblist_n_lazy_cbs(&rdp->cblist),
> > rcu_segcblist_n_cbs(&rdp->cblist), 0);
> > trace_rcu_batch_end(rcu_state.name, 0,
> > !rcu_segcblist_empty(&rdp->cblist),
> > @@ -2168,7 +2184,6 @@ static void rcu_do_batch(struct rcu_data *rdp)
> > if (unlikely(bl > 100))
> > tlimit = local_clock() + rcu_resched_ns;
> > trace_rcu_batch_start(rcu_state.name,
> > - rcu_segcblist_n_lazy_cbs(&rdp->cblist),
> > rcu_segcblist_n_cbs(&rdp->cblist), bl);
> > rcu_segcblist_extract_done_cbs(&rdp->cblist, &rcl);
> > if (offloaded)
> > @@ -2181,8 +2196,8 @@ static void rcu_do_batch(struct rcu_data *rdp)
> > rhp = rcu_cblist_dequeue(&rcl);
> > for (; rhp; rhp = rcu_cblist_dequeue(&rcl)) {
> > debug_rcu_head_unqueue(rhp);
> > - if (__rcu_reclaim(rcu_state.name, rhp))
> > - rcu_cblist_dequeued_lazy(&rcl);
> > + rcu_reclaim_tree(rcu_state.name, rhp);
> > +
> > /*
> > * Stop only if limit reached and CPU has something to do.
> > * Note: The rcl structure counts down from zero.
> > @@ -2585,7 +2600,7 @@ static void rcu_leak_callback(struct rcu_head *rhp)
> > * is expected to specify a CPU.
> > */
> > static void
> > -__call_rcu(struct rcu_head *head, rcu_callback_t func, bool lazy)
> > +__call_rcu(struct rcu_head *head, rcu_callback_t func)
> > {
> > unsigned long flags;
> > struct rcu_data *rdp;
> > @@ -2620,18 +2635,17 @@ __call_rcu(struct rcu_head *head, rcu_callback_t func, bool lazy)
> > if (rcu_segcblist_empty(&rdp->cblist))
> > rcu_segcblist_init(&rdp->cblist);
> > }
> > +
> > if (rcu_nocb_try_bypass(rdp, head, &was_alldone, flags))
> > return; // Enqueued onto ->nocb_bypass, so just leave.
> > /* If we get here, rcu_nocb_try_bypass() acquired ->nocb_lock. */
> > - rcu_segcblist_enqueue(&rdp->cblist, head, lazy);
> > + rcu_segcblist_enqueue(&rdp->cblist, head);
> > if (__is_kfree_rcu_offset((unsigned long)func))
> > trace_rcu_kfree_callback(rcu_state.name, head,
> > (unsigned long)func,
> > - rcu_segcblist_n_lazy_cbs(&rdp->cblist),
> > rcu_segcblist_n_cbs(&rdp->cblist));
> > else
> > trace_rcu_callback(rcu_state.name, head,
> > - rcu_segcblist_n_lazy_cbs(&rdp->cblist),
> > rcu_segcblist_n_cbs(&rdp->cblist));
> >
> > /* Go handle any RCU core processing required. */
> > @@ -2681,7 +2695,7 @@ __call_rcu(struct rcu_head *head, rcu_callback_t func, bool lazy)
> > */
> > void call_rcu(struct rcu_head *head, rcu_callback_t func)
> > {
> > - __call_rcu(head, func, 0);
> > + __call_rcu(head, func);
> > }
> > EXPORT_SYMBOL_GPL(call_rcu);
> >
> > @@ -2755,10 +2769,18 @@ static void kfree_rcu_work(struct work_struct *work)
> > * access is Ok.
> > */
> > for (; head; head = next) {
> > + unsigned long offset = (unsigned long)head->func;
> > +
> > next = head->next;
> > - /* Could be possible to optimize with kfree_bulk in future */
> > +
> > debug_rcu_head_unqueue(head);
> > - __rcu_reclaim(rcu_state.name, head);
> > + rcu_lock_acquire(&rcu_callback_map);
> > + trace_rcu_invoke_kfree_callback(rcu_state.name, head, offset);
> > +
> > + /* Could be possible to optimize with kfree_bulk in future */
> > + kfree((void *)head - offset);
> > +
> > + rcu_lock_release(&rcu_callback_map);
> > cond_resched_tasks_rcu_qs();
> > }
> > }
> > @@ -2840,7 +2862,7 @@ static void kfree_rcu_monitor(struct work_struct *work)
> > */
> > void kfree_call_rcu_nobatch(struct rcu_head *head, rcu_callback_t func)
> > {
> > - __call_rcu(head, func, 1);
> > + __call_rcu(head, func);
> > }
> > EXPORT_SYMBOL_GPL(kfree_call_rcu_nobatch);
> >
> > @@ -3094,7 +3116,7 @@ static void rcu_barrier_func(void *unused)
> > debug_rcu_head_queue(&rdp->barrier_head);
> > rcu_nocb_lock(rdp);
> > WARN_ON_ONCE(!rcu_nocb_flush_bypass(rdp, NULL, jiffies));
> > - if (rcu_segcblist_entrain(&rdp->cblist, &rdp->barrier_head, 0)) {
> > + if (rcu_segcblist_entrain(&rdp->cblist, &rdp->barrier_head)) {
> > atomic_inc(&rcu_state.barrier_cpu_count);
> > } else {
> > debug_rcu_head_unqueue(&rdp->barrier_head);
> > diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
> > index 055c31781d3a..15405420b40c 100644
> > --- a/kernel/rcu/tree.h
> > +++ b/kernel/rcu/tree.h
> > @@ -183,7 +183,6 @@ struct rcu_data {
> > bool rcu_urgent_qs; /* GP old need light quiescent state. */
> > bool rcu_forced_tick; /* Forced tick to provide QS. */
> > #ifdef CONFIG_RCU_FAST_NO_HZ
> > - bool all_lazy; /* All CPU's CBs lazy at idle start? */
> > unsigned long last_accelerate; /* Last jiffy CBs were accelerated. */
> > unsigned long last_advance_all; /* Last jiffy CBs were all advanced. */
> > int tick_nohz_enabled_snap; /* Previously seen value from sysfs. */
> > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> > index 2defc7fe74c3..0d0fa615c5fa 100644
> > --- a/kernel/rcu/tree_plugin.h
> > +++ b/kernel/rcu/tree_plugin.h
> > @@ -1274,21 +1274,15 @@ static void rcu_prepare_for_idle(void)
>
> We now have only one preprocessor symbol (as opposed to the erroneous
> "three proprocessor symbols" in my original).
>
> > * number, be warned: Setting RCU_IDLE_GP_DELAY too high can hang your
> > * system. And if you are -that- concerned about energy efficiency,
> > * just power the system down and be done with it!
> > - * RCU_IDLE_LAZY_GP_DELAY gives the number of jiffies that a CPU is
> > - * permitted to sleep in dyntick-idle mode with only lazy RCU
> > - * callbacks pending. Setting this too high can OOM your system.
> > *
> > * The values below work well in practice. If future workloads require
>
> "value below works"
>
> > * adjustment, they can be converted into kernel config parameters, though
> > * making the state machine smarter might be a better option.
> > */
> > #define RCU_IDLE_GP_DELAY 4 /* Roughly one grace period. */
> > -#define RCU_IDLE_LAZY_GP_DELAY (6 * HZ) /* Roughly six seconds. */
> >
> > static int rcu_idle_gp_delay = RCU_IDLE_GP_DELAY;
> > module_param(rcu_idle_gp_delay, int, 0644);
> > -static int rcu_idle_lazy_gp_delay = RCU_IDLE_LAZY_GP_DELAY;
> > -module_param(rcu_idle_lazy_gp_delay, int, 0644);
> >
> > /*
> > * Try to advance callbacks on the current CPU, but only if it has been
> > @@ -1327,8 +1321,7 @@ static bool __maybe_unused rcu_try_advance_all_cbs(void)
> > /*
> > * Allow the CPU to enter dyntick-idle mode unless it has callbacks ready
> > * to invoke. If the CPU has callbacks, try to advance them. Tell the
> > - * caller to set the timeout based on whether or not there are non-lazy
> > - * callbacks.
> > + * caller about what to set the timeout.
> > *
> > * The caller must have disabled interrupts.
> > */
> > @@ -1354,25 +1347,19 @@ int rcu_needs_cpu(u64 basemono, u64 *nextevt)
> > }
> > rdp->last_accelerate = jiffies;
> >
> > - /* Request timer delay depending on laziness, and round. */
> > - rdp->all_lazy = !rcu_segcblist_n_nonlazy_cbs(&rdp->cblist);
> > - if (rdp->all_lazy) {
> > - dj = round_jiffies(rcu_idle_lazy_gp_delay + jiffies) - jiffies;
> > - } else {
> > - dj = round_up(rcu_idle_gp_delay + jiffies,
> > - rcu_idle_gp_delay) - jiffies;
> > - }
> > + /* Request timer and round. */
> > + dj = round_up(rcu_idle_gp_delay + jiffies,
> > + rcu_idle_gp_delay) - jiffies;
>
> I bet that this now fits onto one line.
>
> > +
> > *nextevt = basemono + dj * TICK_NSEC;
> > return 0;
> > }
> >
> > /*
> > - * Prepare a CPU for idle from an RCU perspective. The first major task
> > - * is to sense whether nohz mode has been enabled or disabled via sysfs.
> > - * The second major task is to check to see if a non-lazy callback has
> > - * arrived at a CPU that previously had only lazy callbacks. The third
> > - * major task is to accelerate (that is, assign grace-period numbers to)
> > - * any recently arrived callbacks.
> > + * Prepare a CPU for idle from an RCU perspective. The first major task is to
> > + * sense whether nohz mode has been enabled or disabled via sysfs. The second
> > + * major task is to accelerate (that is, assign grace-period numbers to) any
> > + * recently arrived callbacks.
> > *
> > * The caller must have disabled interrupts.
> > */
> > @@ -1398,17 +1385,6 @@ static void rcu_prepare_for_idle(void)
> > if (!tne)
> > return;
> >
> > - /*
> > - * If a non-lazy callback arrived at a CPU having only lazy
> > - * callbacks, invoke RCU core for the side-effect of recalculating
> > - * idle duration on re-entry to idle.
> > - */
> > - if (rdp->all_lazy && rcu_segcblist_n_nonlazy_cbs(&rdp->cblist)) {
> > - rdp->all_lazy = false;
> > - invoke_rcu_core();
> > - return;
> > - }
> > -
> > /*
> > * If we have not yet accelerated this jiffy, accelerate all
> > * callbacks on this CPU.
> > diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
> > index 841ab43f3e60..67530c23c708 100644
> > --- a/kernel/rcu/tree_stall.h
> > +++ b/kernel/rcu/tree_stall.h
> > @@ -263,11 +263,9 @@ static void print_cpu_stall_fast_no_hz(char *cp, int cpu)
> > {
> > struct rcu_data *rdp = &per_cpu(rcu_data, cpu);
> >
> > - sprintf(cp, "last_accelerate: %04lx/%04lx, Nonlazy posted: %c%c%c",
> > + sprintf(cp, "last_accelerate: %04lx/%04lx dyntick_enabled: %d",
> > rdp->last_accelerate & 0xffff, jiffies & 0xffff,
> > - ".l"[rdp->all_lazy],
> > - ".L"[!rcu_segcblist_n_nonlazy_cbs(&rdp->cblist)],
> > - ".D"[!!rdp->tick_nohz_enabled_snap]);
> > + !!rdp->tick_nohz_enabled_snap);
> > }
> >
> > #else /* #ifdef CONFIG_RCU_FAST_NO_HZ */
> > --
> > 2.23.0.187.g17f5b7556c-goog
> >
^ permalink raw reply
* Re: [PATCH 5/5] rcu: Remove kfree_call_rcu_nobatch()
From: Joel Fernandes @ 2019-08-29 22:23 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, byungchul.park, Josh Triplett, Lai Jiangshan,
linux-doc, Mathieu Desnoyers, rcu, Steven Rostedt, kernel-team
In-Reply-To: <20190828215636.GA26530@linux.ibm.com>
Hi Paul,
I think this is the only contentious patch preventing my resend of the
series, let me know what you think, I replied below:
On Wed, Aug 28, 2019 at 02:56:36PM -0700, Paul E. McKenney wrote:
> On Tue, Aug 27, 2019 at 03:01:59PM -0400, Joel Fernandes (Google) wrote:
[snip]
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 12c17e10f2b4..c767973d62ac 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -2777,8 +2777,10 @@ static void kfree_rcu_work(struct work_struct *work)
> > rcu_lock_acquire(&rcu_callback_map);
> > trace_rcu_invoke_kfree_callback(rcu_state.name, head, offset);
> >
> > - /* Could be possible to optimize with kfree_bulk in future */
> > - kfree((void *)head - offset);
> > + if (!WARN_ON_ONCE(!__is_kfree_rcu_offset(offset))) {
> > + /* Could be optimized with kfree_bulk() in future. */
> > + kfree((void *)head - offset);
> > + }
>
> This really needs to be in the previous patch until such time as Tiny RCU
> no longer needs the restriction.
I was only going by whatever is already committed to the -rcu dev branch. The
series is based on the -dev branch.
The original patch adding the kfree_rcu() batching is already merged into the
-rcu dev branch (that version just had 1 list, this series adds multiple
lists).
In the above diff, I just added the WARN_ON_ONCE() as extra checking for tree
RCU kfree batching. It has nothing to do with tiny RCU per-se. Should I
submit the WARN_ON_ONCE() as a separate patch then?
To prevent confusion, could you let me know if I am supposed to submitting
patches against a branch other than the dev branch?
> > rcu_lock_release(&rcu_callback_map);
> > cond_resched_tasks_rcu_qs();
> > @@ -2856,16 +2858,6 @@ static void kfree_rcu_monitor(struct work_struct *work)
> > spin_unlock_irqrestore(&krcp->lock, flags);
> > }
> >
> > -/*
> > - * This version of kfree_call_rcu does not do batching of kfree_rcu() requests.
> > - * Used only by rcuperf torture test for comparison with kfree_rcu_batch().
> > - */
> > -void kfree_call_rcu_nobatch(struct rcu_head *head, rcu_callback_t func)
> > -{
> > - __call_rcu(head, func);
> > -}
> > -EXPORT_SYMBOL_GPL(kfree_call_rcu_nobatch);
> > -
> > /*
> > * Queue a request for lazy invocation of kfree() after a grace period.
> > *
> > @@ -2885,12 +2877,6 @@ void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> > unsigned long flags;
> > struct kfree_rcu_cpu *krcp;
> >
> > - /* kfree_call_rcu() batching requires timers to be up. If the scheduler
> > - * is not yet up, just skip batching and do the non-batched version.
> > - */
> > - if (rcu_scheduler_active != RCU_SCHEDULER_RUNNING)
> > - return kfree_call_rcu_nobatch(head, func);
> > -
> > if (debug_rcu_head_queue(head)) {
> > /* Probable double kfree_rcu() */
> > WARN_ONCE(1, "kfree_call_rcu(): Double-freed call. rcu_head %p\n",
> > @@ -2909,8 +2895,15 @@ void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> > krcp->head = head;
> >
> > /* Schedule monitor for timely drain after KFREE_DRAIN_JIFFIES. */
> > - if (!xchg(&krcp->monitor_todo, true))
> > - schedule_delayed_work(&krcp->monitor_work, KFREE_DRAIN_JIFFIES);
> > + if (!xchg(&krcp->monitor_todo, true)) {
> > + /* Scheduling the monitor requires scheduler/timers to be up,
> > + * if it is not, just skip it. An eventual kfree_rcu() will
> > + * kick it again.
> > + */
> > + if ((rcu_scheduler_active == RCU_SCHEDULER_RUNNING)) {
> > + schedule_delayed_work(&krcp->monitor_work, KFREE_DRAIN_JIFFIES);
> > + }
> > + }
>
> And this also needs to be in an earlier patch. Bisectability and all that!
>
> Are we really guaranteed that there will be an eventual kfree_rcu()?
> More of a worry for Tiny RCU than for Tree RCU, but still could be
> annoying for someone trying to debug a memory leak.
Same comment as above, the original patch adding the schedule_delayed_work()
is already merged into the -dev branch. This series is based on top of that.
The reason I had to rearrange &krcp->monitor_todo code above is because we no
longer have kfree_rcu_no_batch() which this patch removes.
thanks,
- Joel
^ permalink raw reply
* Re: [RFC v1 2/2] rcu/tree: Remove dynticks_nmi_nesting counter
From: Paul E. McKenney @ 2019-08-30 0:47 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, Frederic Weisbecker, Jonathan Corbet, Josh Triplett,
kernel-team, Lai Jiangshan, linux-doc, Mathieu Desnoyers,
Mauro Carvalho Chehab, rcu, Steven Rostedt
In-Reply-To: <20190829171454.GA115245@google.com>
On Thu, Aug 29, 2019 at 01:14:54PM -0400, Joel Fernandes wrote:
> On Thu, Aug 29, 2019 at 09:13:01AM -0700, Paul E. McKenney wrote:
> > On Thu, Aug 29, 2019 at 11:13:25AM -0400, Joel Fernandes wrote:
> > > On Thu, Aug 29, 2019 at 10:43:55AM -0400, Joel Fernandes wrote:
> > > > On Wed, Aug 28, 2019 at 08:43:36PM -0700, Paul E. McKenney wrote:
> > > > [snip]
> > > > > > > > > This change is not fixing a bug, so there is no need for an emergency fix,
> > > > > > > > > and thus no point in additional churn. I understand that it is a bit
> > > > > > > > > annoying to code and test something and have your friendly maintainer say
> > > > > > > > > "sorry, wrong rocks", and the reason that I understand this is that I do
> > > > > > > > > that to myself rather often.
> > > > > > > >
> > > > > > > > The motivation for me for this change is to avoid future bugs such as with
> > > > > > > > the following patch where "== 2" did not take the force write of
> > > > > > > > DYNTICK_IRQ_NONIDLE into account:
> > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/commit/?h=dev&id=13c4b07593977d9288e5d0c21c89d9ba27e2ea1f
> > > > > > >
> > > > > > > Yes, the current code does need some simplification.
> > > > > > >
> > > > > > > > I still don't see it as pointless churn, it is also a maintenance cost in its
> > > > > > > > current form and the simplification is worth it IMHO both from a readability,
> > > > > > > > and maintenance stand point.
> > > > > > > >
> > > > > > > > I still don't see what's technically wrong with the patch. I could perhaps
> > > > > > > > add the above "== 2" point in the patch?
> > > > > > >
> > > > > > > I don't know of a crash or splat your patch would cause, if that is
> > > > > > > your question. But that is also true of the current code, so the point
> > > > > > > is simplification, not bug fixing. And from what I can see, there is an
> > > > > > > opportunity to simplify quite a bit further. And with something like
> > > > > > > RCU, further simplification is worth -serious- consideration.
> > > > > > >
> > > > > > > > We could also discuss f2f at LPC to see if we can agree about it?
> > > > > > >
> > > > > > > That might make a lot of sense.
> > > > > >
> > > > > > Sure. I am up for a further redesign / simplification. I will think more
> > > > > > about your suggestions and can also further discuss at LPC.
> > > > >
> > > > > One question that might (or might not) help: Given the compound counter,
> > > > > where the low-order hex digit indicates whether the corresponding CPU
> > > > > is running in a non-idle kernel task and the rest of the hex digits
> > > > > indicate the NMI-style nesting counter shifted up by four bits, what
> > > > > could rcu_is_cpu_rrupt_from_idle() be reduced to?
> > > > >
> > > > > > And this patch is on LKML archives and is not going anywhere so there's no
> > > > > > rush I guess ;-)
> > > > >
> > > > > True enough! ;-)
> > > >
> > > > Paul, do we also nuke rcu_eqs_special_set()? Currently I don't see anyone
> > > > using it. And also remove the bottom most bit of dynticks?
> > > >
> > > > Also what happens if a TLB flush broadcast is needed? Do we IPI nohz or idle
> > > > CPUs are the moment?
> > > >
> > > > All of this was introduced in:
> > > > b8c17e6664c4 ("rcu: Maintain special bits at bottom of ->dynticks counter")
> > >
> > >
> > > Paul, also what what happens in the following scenario:
> > >
> > > CPU0 CPU1
> > >
> > > A syscall causes rcu_eqs_exit()
> > > rcu_read_lock();
> > > ---> FQS loop waiting on
> > > dyntick_snap
> > > usermode-upcall entry -->causes rcu_eqs_enter();
> > >
> > > usermode-upcall exit -->causes rcu_eqs_exit();
> > >
> > > ---> FQS loop sees
> > > dyntick snap
> > > increment and
> > > declares CPU0 is
> > > in a QS state
> > > before the
> > > rcu_read_unlock!
> > >
> > > rcu_read_unlock();
> > > ---
> > >
> > > Does the context tracking not call rcu_user_enter() in this case, or did I
> > > really miss something?
> >
> > Holding rcu_read_lock() across usermode execution (in this case,
> > the usermode upcall) is a bad idea. Why is CPU 0 doing that?
>
> Oh, ok. I was just hypothesizing that since usermode upcalls from
> something as heavy as interrupts, it could also mean we had the same from
> some path that held an rcu_read_lock() as well. It was just a theoretical
> concern, if it is not an issue, no problem.
Are there the usual lockdep checks in the upcall code? Holding a spinlock
across them would seem to be at least as bad as holding an rcu_read_lock()
across them.
> The other question I had was, in which cases would dyntick_nesting in current
> RCU code be > 1 (after removing the lower bit and any crowbarring) ? In the
> scenarios I worked out on paper, I can only see this as 1 or 0. But the
> wording of it is 'dynticks_nesting'. May be I am missing a nesting scenario?
> We can exit RCU-idleness into process context only once (either exiting idle
> mode or user mode). Both cases would imply a value of 1.
Interrrupt -> NMI -> certain types of tracing. I believe that can get
it to 5. There might be even more elaborate sequences of events.
Thanx, Paul
^ permalink raw reply
* Re: [RFC v1 2/2] rcu/tree: Remove dynticks_nmi_nesting counter
From: Paul E. McKenney @ 2019-08-30 0:48 UTC (permalink / raw)
To: Joel Fernandes
Cc: Andy Lutomirski, LKML, Frederic Weisbecker, Jonathan Corbet,
Josh Triplett, Android Kernel Team, Lai Jiangshan,
open list:DOCUMENTATION, Mathieu Desnoyers, Mauro Carvalho Chehab,
rcu, Steven Rostedt
In-Reply-To: <20190829190046.GB115245@google.com>
On Thu, Aug 29, 2019 at 03:00:46PM -0400, Joel Fernandes wrote:
> Hi Paul,
>
> On Thu, Aug 29, 2019 at 09:54:07AM -0700, Paul E. McKenney wrote:
> > On Thu, Aug 29, 2019 at 09:21:46AM -0700, Andy Lutomirski wrote:
> > > On Thu, Aug 29, 2019 at 9:10 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > >
> > > > On Thu, Aug 29, 2019 at 10:43:55AM -0400, Joel Fernandes wrote:
> > > >
> > > > [ . . . ]
> > > >
> > > > > Paul, do we also nuke rcu_eqs_special_set()? Currently I don't see anyone
> > > > > using it. And also remove the bottom most bit of dynticks?
> > > > >
> > > > > Also what happens if a TLB flush broadcast is needed? Do we IPI nohz or idle
> > > > > CPUs are the moment?
> > > > >
> > > > > All of this was introduced in:
> > > > > b8c17e6664c4 ("rcu: Maintain special bits at bottom of ->dynticks counter")
> > > >
> > > > Adding Andy Lutomirski on CC.
> > > >
> > > > Andy, is this going to be used in the near term, or should we just get
> > > > rid of it?
> > >
> > > Let's get rid of it. I'm not actually convinced it *can* be used as designed.
> > >
> > > For those who forgot the history or weren't cc'd on all of it: I had
> > > this clever idea about how we could reduce TLB flushes. I implemented
> > > some of it (but not the part that would have used this RCU feature),
> > > and it exploded in nasty and subtle ways. This caused me to learn
> > > that speculative TLB fills were a problem that I had entirely failed
> > > to account for. Then PTI happened and thoroughly muddied the water.
> >
> > Yeah, PTI was quite annoying. Still is, from what I can see. :-/
> >
> > > So I think we should just drop this :(
> >
> > OK, thank you! I will put a tag into -rcu marking its removal in case
> > it should prove useful whenever for whatever.
> >
> > Joel, would you like to remove this, or would you rather that I did?
> > It is in code you are working with right now, so if I do it, I need to
> > wait until yours is finalized. Which wouldn't be a problem.
>
> I can remove it in my series, made a note to do so.
Sounds good!
Thanx, Paul
^ permalink raw reply
* Re: [RFC v1 2/2] rcu/tree: Remove dynticks_nmi_nesting counter
From: Joel Fernandes @ 2019-08-30 1:20 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, Frederic Weisbecker, Jonathan Corbet, Josh Triplett,
kernel-team, Lai Jiangshan, linux-doc, Mathieu Desnoyers,
Mauro Carvalho Chehab, rcu, Steven Rostedt
In-Reply-To: <20190830004756.GW4125@linux.ibm.com>
On Thu, Aug 29, 2019 at 05:47:56PM -0700, Paul E. McKenney wrote:
[snip]
> > > > Paul, also what what happens in the following scenario:
> > > >
> > > > CPU0 CPU1
> > > >
> > > > A syscall causes rcu_eqs_exit()
> > > > rcu_read_lock();
> > > > ---> FQS loop waiting on
> > > > dyntick_snap
> > > > usermode-upcall entry -->causes rcu_eqs_enter();
> > > >
> > > > usermode-upcall exit -->causes rcu_eqs_exit();
> > > >
> > > > ---> FQS loop sees
> > > > dyntick snap
> > > > increment and
> > > > declares CPU0 is
> > > > in a QS state
> > > > before the
> > > > rcu_read_unlock!
> > > >
> > > > rcu_read_unlock();
> > > > ---
> > > >
> > > > Does the context tracking not call rcu_user_enter() in this case, or did I
> > > > really miss something?
> > >
> > > Holding rcu_read_lock() across usermode execution (in this case,
> > > the usermode upcall) is a bad idea. Why is CPU 0 doing that?
> >
> > Oh, ok. I was just hypothesizing that since usermode upcalls from
> > something as heavy as interrupts, it could also mean we had the same from
> > some path that held an rcu_read_lock() as well. It was just a theoretical
> > concern, if it is not an issue, no problem.
>
> Are there the usual lockdep checks in the upcall code? Holding a spinlock
> across them would seem to be at least as bad as holding an rcu_read_lock()
> across them.
Great point, I'll take a look.
> > The other question I had was, in which cases would dyntick_nesting in current
> > RCU code be > 1 (after removing the lower bit and any crowbarring) ? In the
> > scenarios I worked out on paper, I can only see this as 1 or 0. But the
> > wording of it is 'dynticks_nesting'. May be I am missing a nesting scenario?
> > We can exit RCU-idleness into process context only once (either exiting idle
> > mode or user mode). Both cases would imply a value of 1.
>
> Interrrupt -> NMI -> certain types of tracing. I believe that can get
> it to 5. There might be even more elaborate sequences of events.
I am only talking about dynticks_nesting, not dynticks_nmi_nesting. In
current mainline, I see this only 0 or 1. I am running the below patch
overnight on all RCU configurations to see if it is ever any other value.
And, please feel free to ignore my emails as you mentioned you are supposed
to be out next 2 days! Thanks for the replies though!
---8<-----------------------
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 68ebf0eb64c8..8c8ddb6457d5 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -571,6 +571,9 @@ static void rcu_eqs_enter(bool user)
WRITE_ONCE(rdp->dynticks_nmi_nesting, 0);
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
rdp->dynticks_nesting == 0);
+
+ WARN_ON_ONCE(rdp->dynticks_nesting != 1);
+
if (rdp->dynticks_nesting != 1) {
rdp->dynticks_nesting--;
return;
@@ -736,6 +739,9 @@ static void rcu_eqs_exit(bool user)
lockdep_assert_irqs_disabled();
rdp = this_cpu_ptr(&rcu_data);
oldval = rdp->dynticks_nesting;
+
+ WARN_ON_ONCE(rdp->dynticks_nesting != 0);
+
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && oldval < 0);
if (oldval) {
rdp->dynticks_nesting++;
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related
* Re: [RFC v1 2/2] rcu/tree: Remove dynticks_nmi_nesting counter
From: Paul E. McKenney @ 2019-08-30 2:45 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, Frederic Weisbecker, Jonathan Corbet, Josh Triplett,
kernel-team, Lai Jiangshan, linux-doc, Mathieu Desnoyers,
Mauro Carvalho Chehab, rcu, Steven Rostedt
In-Reply-To: <20190830012036.GA184995@google.com>
On Thu, Aug 29, 2019 at 09:20:36PM -0400, Joel Fernandes wrote:
> On Thu, Aug 29, 2019 at 05:47:56PM -0700, Paul E. McKenney wrote:
> [snip]
> > > > > Paul, also what what happens in the following scenario:
> > > > >
> > > > > CPU0 CPU1
> > > > >
> > > > > A syscall causes rcu_eqs_exit()
> > > > > rcu_read_lock();
> > > > > ---> FQS loop waiting on
> > > > > dyntick_snap
> > > > > usermode-upcall entry -->causes rcu_eqs_enter();
> > > > >
> > > > > usermode-upcall exit -->causes rcu_eqs_exit();
> > > > >
> > > > > ---> FQS loop sees
> > > > > dyntick snap
> > > > > increment and
> > > > > declares CPU0 is
> > > > > in a QS state
> > > > > before the
> > > > > rcu_read_unlock!
> > > > >
> > > > > rcu_read_unlock();
> > > > > ---
> > > > >
> > > > > Does the context tracking not call rcu_user_enter() in this case, or did I
> > > > > really miss something?
> > > >
> > > > Holding rcu_read_lock() across usermode execution (in this case,
> > > > the usermode upcall) is a bad idea. Why is CPU 0 doing that?
> > >
> > > Oh, ok. I was just hypothesizing that since usermode upcalls from
> > > something as heavy as interrupts, it could also mean we had the same from
> > > some path that held an rcu_read_lock() as well. It was just a theoretical
> > > concern, if it is not an issue, no problem.
> >
> > Are there the usual lockdep checks in the upcall code? Holding a spinlock
> > across them would seem to be at least as bad as holding an rcu_read_lock()
> > across them.
>
> Great point, I'll take a look.
>
> > > The other question I had was, in which cases would dyntick_nesting in current
> > > RCU code be > 1 (after removing the lower bit and any crowbarring) ? In the
> > > scenarios I worked out on paper, I can only see this as 1 or 0. But the
> > > wording of it is 'dynticks_nesting'. May be I am missing a nesting scenario?
> > > We can exit RCU-idleness into process context only once (either exiting idle
> > > mode or user mode). Both cases would imply a value of 1.
> >
> > Interrrupt -> NMI -> certain types of tracing. I believe that can get
> > it to 5. There might be even more elaborate sequences of events.
>
> I am only talking about dynticks_nesting, not dynticks_nmi_nesting. In
> current mainline, I see this only 0 or 1. I am running the below patch
> overnight on all RCU configurations to see if it is ever any other value.
Ah! Then yes, we never enter non-idle/non-user process-level mode
twice without having exited it. There would have been a splat,
I believe.
> And, please feel free to ignore my emails as you mentioned you are supposed
> to be out next 2 days! Thanks for the replies though!
Actually this day and next. ;-)
Thanx, Paul
> ---8<-----------------------
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 68ebf0eb64c8..8c8ddb6457d5 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -571,6 +571,9 @@ static void rcu_eqs_enter(bool user)
> WRITE_ONCE(rdp->dynticks_nmi_nesting, 0);
> WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> rdp->dynticks_nesting == 0);
> +
> + WARN_ON_ONCE(rdp->dynticks_nesting != 1);
> +
> if (rdp->dynticks_nesting != 1) {
> rdp->dynticks_nesting--;
> return;
> @@ -736,6 +739,9 @@ static void rcu_eqs_exit(bool user)
> lockdep_assert_irqs_disabled();
> rdp = this_cpu_ptr(&rcu_data);
> oldval = rdp->dynticks_nesting;
> +
> + WARN_ON_ONCE(rdp->dynticks_nesting != 0);
> +
> WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && oldval < 0);
> if (oldval) {
> rdp->dynticks_nesting++;
> --
> 2.23.0.187.g17f5b7556c-goog
>
^ permalink raw reply
* Re: [PATCH v10 0/7] Solve postboot supplier cleanup and optimize probe ordering
From: Saravana Kannan @ 2019-08-30 4:32 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, Greg Kroah-Hartman, Rafael J. Wysocki, Frank Rowand,
Jonathan Corbet, Len Brown,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org, Linux Doc Mailing List, linux-acpi,
clang-built-linux, David Collins, Android Kernel Team
In-Reply-To: <CAL_Jsq+2vR75ofq=aKOt1bb1T-JfhiGSR9dnHWQf7VLmgJP4eA@mail.gmail.com>
On Thu, Aug 29, 2019 at 9:43 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Thu, Aug 29, 2019 at 2:46 AM Saravana Kannan <saravanak@google.com> wrote:
> >
> > Add device-links to track functional dependencies between devices
> > after they are created (but before they are probed) by looking at
> > their common DT bindings like clocks, interconnects, etc.
> >
> > Having functional dependencies automatically added before the devices
> > are probed, provides the following benefits:
> >
> > - Optimizes device probe order and avoids the useless work of
> > attempting probes of devices that will not probe successfully
> > (because their suppliers aren't present or haven't probed yet).
> >
> > For example, in a commonly available mobile SoC, registering just
> > one consumer device's driver at an initcall level earlier than the
> > supplier device's driver causes 11 failed probe attempts before the
> > consumer device probes successfully. This was with a kernel with all
> > the drivers statically compiled in. This problem gets a lot worse if
> > all the drivers are loaded as modules without direct symbol
> > dependencies.
> >
> > - Supplier devices like clock providers, interconnect providers, etc
> > need to keep the resources they provide active and at a particular
> > state(s) during boot up even if their current set of consumers don't
> > request the resource to be active. This is because the rest of the
> > consumers might not have probed yet and turning off the resource
> > before all the consumers have probed could lead to a hang or
> > undesired user experience.
> >
> > Some frameworks (Eg: regulator) handle this today by turning off
> > "unused" resources at late_initcall_sync and hoping all the devices
> > have probed by then. This is not a valid assumption for systems with
> > loadable modules. Other frameworks (Eg: clock) just don't handle
> > this due to the lack of a clear signal for when they can turn off
> > resources. This leads to downstream hacks to handle cases like this
> > that can easily be solved in the upstream kernel.
> >
> > By linking devices before they are probed, we give suppliers a clear
> > count of the number of dependent consumers. Once all of the
> > consumers are active, the suppliers can turn off the unused
> > resources without making assumptions about the number of consumers.
> >
> > By default we just add device-links to track "driver presence" (probe
> > succeeded) of the supplier device. If any other functionality provided
> > by device-links are needed, it is left to the consumer/supplier
> > devices to change the link when they probe.
> >
> > v1 -> v2:
> > - Drop patch to speed up of_find_device_by_node()
> > - Drop depends-on property and use existing bindings
> >
> > v2 -> v3:
> > - Refactor the code to have driver core initiate the linking of devs
> > - Have driver core link consumers to supplier before it's probed
> > - Add support for drivers to edit the device links before probing
> >
> > v3 -> v4:
> > - Tested edit_links() on system with cyclic dependency. Works.
> > - Added some checks to make sure device link isn't attempted from
> > parent device node to child device node.
> > - Added way to pause/resume sync_state callbacks across
> > of_platform_populate().
> > - Recursively parse DT node to create device links from parent to
> > suppliers of parent and all child nodes.
> >
> > v4 -> v5:
> > - Fixed copy-pasta bugs with linked list handling
> > - Walk up the phandle reference till I find an actual device (needed
> > for regulators to work)
> > - Added support for linking devices from regulator DT bindings
> > - Tested the whole series again to make sure cyclic dependencies are
> > broken with edit_links() and regulator links are created properly.
> >
> > v5 -> v6:
> > - Split, squashed and reordered some of the patches.
> > - Refactored the device linking code to follow the same code pattern for
> > any property.
> >
> > v6 -> v7:
> > - No functional changes.
> > - Renamed i to index
> > - Added comment to clarify not having to check property name for every
> > index
> > - Added "matched" variable to clarify code. No functional change.
> > - Added comments to include/linux/device.h for add_links()
> >
> > v7 -> v8:
> > - Rebased on top of linux-next to handle device link changes in [1]
> >
> > v8 -> v9:
> > - Fixed kbuild test bot reported errors (docs and const)
> >
> > v9->v10:
> > - Changes made based on reviews on LKML [2] and discussions at ELC [3]
> > - Dropped the edit_links() patch
> > - Dropped the patch that skips linking for default bus nodes
> > - 1/7: Changed from bus.add_links() to fwnode.ops.add_links()
> > - 1/7: Update device link doc
> > - 1/7: Lots of comments/fn doc updates
> > - 1/7: Renamed device_link_check_waiting_consumers() to
> > device_link_add_missing_supplier_links()
> > - 2/7: Moved DT parsing/linking code from of/platform.c to of/property.c
>
> Why? You'll notice that of/property.c doesn't know anything about
> platform_device (and struct device):
>
> $ git grep platform_device -- drivers/of/property.c
> $
>
> Everything related to platform_device goes in of/platform.c.
> Everything related to struct device only goes in of/device.c. I'd be
> okay with a new file for this too.
The only platform_device related code in what got moved to
of/property.c is the call to of_find_device_by_node(). And that's
because I forgot that function returns a platform_device --- it should
really have been called of_find_plat_device_by_node() or something
similar. Outside of that, of/property.c makes sense because that's
where the fwnode ops are implemented.
As you mentioned in the other email, just searching platform_bus is
not sufficient. So I'll have to figure something out for that. Once I
do, I think the code will be fine in of/property.c as it shouldn't
have any reference to platform_device.
Thanks for catching what I missed.
-Saravana
^ permalink raw reply
* Re: [PATCH v3 01/10] KVM: arm64: Document PV-time interface
From: Steven Price @ 2019-08-30 8:35 UTC (permalink / raw)
To: Andrew Jones
Cc: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm, linux-kernel,
kvm, Catalin Marinas, linux-doc, Russell King, Paolo Bonzini
In-Reply-To: <20190829171548.xfk7i2bwnwl4w2po@kamzik.brq.redhat.com>
On 29/08/2019 18:15, Andrew Jones wrote:
> On Wed, Aug 21, 2019 at 04:36:47PM +0100, Steven Price wrote:
>> Introduce a paravirtualization interface for KVM/arm64 based on the
>> "Arm Paravirtualized Time for Arm-Base Systems" specification DEN 0057A.
>>
>> This only adds the details about "Stolen Time" as the details of "Live
>> Physical Time" have not been fully agreed.
>>
>> User space can specify a reserved area of memory for the guest and
>> inform KVM to populate the memory with information on time that the host
>> kernel has stolen from the guest.
>>
>> A hypercall interface is provided for the guest to interrogate the
>> hypervisor's support for this interface and the location of the shared
>> memory structures.
>>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>> Documentation/virt/kvm/arm/pvtime.txt | 100 ++++++++++++++++++++++++++
>> 1 file changed, 100 insertions(+)
>> create mode 100644 Documentation/virt/kvm/arm/pvtime.txt
>>
>> diff --git a/Documentation/virt/kvm/arm/pvtime.txt b/Documentation/virt/kvm/arm/pvtime.txt
>> new file mode 100644
>> index 000000000000..1ceb118694e7
>> --- /dev/null
>> +++ b/Documentation/virt/kvm/arm/pvtime.txt
>> @@ -0,0 +1,100 @@
>> +Paravirtualized time support for arm64
>> +======================================
>> +
>> +Arm specification DEN0057/A defined a standard for paravirtualised time
>> +support for AArch64 guests:
>> +
>> +https://developer.arm.com/docs/den0057/a
>> +
>> +KVM/arm64 implements the stolen time part of this specification by providing
>> +some hypervisor service calls to support a paravirtualized guest obtaining a
>> +view of the amount of time stolen from its execution.
>> +
>> +Two new SMCCC compatible hypercalls are defined:
>> +
>> +PV_FEATURES 0xC5000020
>> +PV_TIME_ST 0xC5000022
>> +
>> +These are only available in the SMC64/HVC64 calling convention as
>> +paravirtualized time is not available to 32 bit Arm guests. The existence of
>> +the PV_FEATURES hypercall should be probed using the SMCCC 1.1 ARCH_FEATURES
>> +mechanism before calling it.
>> +
>> +PV_FEATURES
>> + Function ID: (uint32) : 0xC5000020
>> + PV_func_id: (uint32) : Either PV_TIME_LPT or PV_TIME_ST
>> + Return value: (int32) : NOT_SUPPORTED (-1) or SUCCESS (0) if the relevant
>> + PV-time feature is supported by the hypervisor.
>> +
>> +PV_TIME_ST
>> + Function ID: (uint32) : 0xC5000022
>> + Return value: (int64) : IPA of the stolen time data structure for this
>> + (V)CPU. On failure:
>
> Why the () around the V in VCPU?
There's nothing preventing the same mechanism being used without the
virtualisation of CPUs. For example a hypervisor like Jailhouse could
implement this interface even though there the CPU isn't being
virtualised but is being handed over to the guest. Equally it is
possible for firmware to provide the same mechanism (using the SMC64
calling convention).
Admittedly that's a little confusing here because the rest of this
document is talking about KVM's implementation and normal hypervisors.
So I'll drop the brackets.
>> + NOT_SUPPORTED (-1)
>> +
>> +The IPA returned by PV_TIME_ST should be mapped by the guest as normal memory
>> +with inner and outer write back caching attributes, in the inner shareable
>> +domain. A total of 16 bytes from the IPA returned are guaranteed to be
>> +meaningfully filled by the hypervisor (see structure below).
>> +
>> +PV_TIME_ST returns the structure for the calling VCPU.
>
> The above sentence seems redundant here.
It is an important detail that each VCPU must use PV_TIME_ST to fetch
the address of the structure for that VCPU. E.g. It could have been
implemented so that the hypercall took a VCPU number. So while redundant
I do feel it's worth pointing this out explicitly.
>> +
>> +Stolen Time
>> +-----------
>> +
>> +The structure pointed to by the PV_TIME_ST hypercall is as follows:
>> +
>> + Field | Byte Length | Byte Offset | Description
>> + ----------- | ----------- | ----------- | --------------------------
>> + Revision | 4 | 0 | Must be 0 for version 0.1
>> + Attributes | 4 | 4 | Must be 0
>> + Stolen time | 8 | 8 | Stolen time in unsigned
>> + | | | nanoseconds indicating how
>> + | | | much time this VCPU thread
>> + | | | was involuntarily not
>> + | | | running on a physical CPU.
>> +
>> +The structure will be updated by the hypervisor prior to scheduling a VCPU. It
>> +will be present within a reserved region of the normal memory given to the
>> +guest. The guest should not attempt to write into this memory. There is a
>> +structure per VCPU of the guest.
>> +
>> +User space interface
>> +====================
>> +
>> +User space can request that KVM provide the paravirtualized time interface to
>> +a guest by creating a KVM_DEV_TYPE_ARM_PV_TIME device, for example:
>> +
>> + struct kvm_create_device pvtime_device = {
>> + .type = KVM_DEV_TYPE_ARM_PV_TIME,
>> + .attr = 0,
>> + .flags = 0,
>> + };
>> +
>> + pvtime_fd = ioctl(vm_fd, KVM_CREATE_DEVICE, &pvtime_device);
>
> The ioctl doesn't return the fd. If the ioctl returns zero the fd will be
> in pvtime_device.fd.
Good catch - I'm not sure quite why I wrote that. Anyway I've agreed to
change the interface to operate on the VCPU device instead so this text
is rewritten completely.
>> +
>> +Creation of the device should be done after creating the vCPUs of the virtual
>> +machine.
>
> Or else what? Will an error be reported in that case?
This is now enforced by calling the ioctl on the VCPU device, so it's
impossible to do in the wrong order.
>> +
>> +The IPA of the structures must be given to KVM. This is the base address
>> +of an array of stolen time structures (one for each VCPU). The base address
>> +must be page aligned. The size must be at least 64 * number of VCPUs and be a
>> +multiple of PAGE_SIZE.
>> +
>> +The memory for these structures should be added to the guest in the usual
>> +manner (e.g. using KVM_SET_USER_MEMORY_REGION).
>
> Above it says the guest shouldn't attempt to write the memory. Should
> KVM_MEM_READONLY be used with KVM_SET_USER_MEMORY_REGION for it?
That is optional - the specification states the guest must not attempt
to write to it - so marking it read-only for the guest should work fine
with a conforming guest. But it's not required.
>> +
>> +For example:
>> +
>> + struct kvm_dev_arm_st_region region = {
>> + .gpa = <IPA of guest base address>,
>> + .size = <size in bytes>
>> + };
>> +
>> + struct kvm_device_attr st_base = {
>> + .group = KVM_DEV_ARM_PV_TIME_PADDR,
>
> This is KVM_DEV_ARM_PV_TIME_REGION in the code.
Gah - I obviously missed that when I refactored to define the region
rather than just the base address. Anyway this has all changed (again)
because each VCPU has its own base address so the size is no longer
necessary.
Thanks for the review,
Steve
>> + .attr = KVM_DEV_ARM_PV_TIME_ST,
>> + .addr = (u64)®ion
>> + };
>> +
>> + ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &st_base);
>> --
>> 2.20.1
>>
>
> Thanks,
> drew
>
^ permalink raw reply
* [PATCH v4 00/10] arm64: Stolen time support
From: Steven Price @ 2019-08-30 8:42 UTC (permalink / raw)
To: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: Steven Price, Catalin Marinas, Paolo Bonzini,
Radim Krčmář, Russell King, James Morse,
Julien Thierry, Suzuki K Pouloze, Mark Rutland, kvm, linux-doc,
linux-kernel
This series add support for paravirtualized time for arm64 guests and
KVM hosts following the specification in Arm's document DEN 0057A:
https://developer.arm.com/docs/den0057/a
It implements support for stolen time, allowing the guest to
identify time when it is forcibly not executing.
It doesn't implement support for Live Physical Time (LPT) as there are
some concerns about the overheads and approach in the above
specification, and I expect an updated version of the specification to
be released soon with just the stolen time parts.
NOTE: Patches 8 and 9 will conflict with Mark Rutland's series[1] cleaning
up the SMCCC conduit. I do feel that the addition of an _invoke() call
makes a number of call sites cleaner and it should be possible to
integrate both this and Mark's other cleanups.
[1] https://lore.kernel.org/linux-arm-kernel/20190809132245.43505-1-mark.rutland@arm.com/
Also available as a git tree:
git://linux-arm.org/linux-sp.git stolen_time/v4
Changes from v3:
https://lore.kernel.org/lkml/20190821153656.33429-1-steven.price@arm.com/
* There's no longer a PV_TIME device, instead there are attributes on
the VCPU. This allows the stolen time structures to be places
arbitrarily by user space (subject to 64 byte alignment).
* Split documentation between information on the hypercalls and the
attributes on the VCPU
* Fixed the type of SMCCC functions to return long not int
Changes from v2:
https://lore.kernel.org/lkml/20190819140436.12207-1-steven.price@arm.com/
* Switched from using gfn_to_hva_cache to a new macro kvm_put_guest()
that can provide the single-copy atomicity required (on arm64). This
macro is added in patch 4.
* Tidied up the locking for kvm_update_stolen_time().
pagefault_disable() was unnecessary and the caller didn't need to
take kvm->srcu as the function does it itself.
* Removed struct kvm_arch_pvtime from the arm implementation, replaced
instead with inline static functions which are empty for arm.
* Fixed a few checkpatch --strict warnings.
Changes from v1:
https://lore.kernel.org/lkml/20190802145017.42543-1-steven.price@arm.com/
* Host kernel no longer allocates the stolen time structure, instead it
is allocated by user space. This means the save/restore functionality
can be removed.
* Refactored the code so arm has stub implementations and to avoid
initcall
* Rebased to pick up Documentation/{virt->virtual} change
* Bunch of typo fixes
Christoffer Dall (1):
KVM: arm/arm64: Factor out hypercall handling from PSCI code
Steven Price (9):
KVM: arm64: Document PV-time interface
KVM: arm64: Implement PV_FEATURES call
KVM: Implement kvm_put_guest()
KVM: arm64: Support stolen time reporting via shared structure
KVM: Allow kvm_device_ops to be const
KVM: arm64: Provide VCPU attributes for stolen time
arm/arm64: Provide a wrapper for SMCCC 1.1 calls
arm/arm64: Make use of the SMCCC 1.1 wrapper
arm64: Retrieve stolen time as paravirtualized guest
Documentation/virt/kvm/arm/pvtime.txt | 64 ++++++++++
Documentation/virt/kvm/devices/vcpu.txt | 14 +++
arch/arm/include/asm/kvm_host.h | 26 +++++
arch/arm/kvm/Makefile | 2 +-
arch/arm/kvm/handle_exit.c | 2 +-
arch/arm/mm/proc-v7-bugs.c | 13 +--
arch/arm64/include/asm/kvm_host.h | 30 ++++-
arch/arm64/include/asm/paravirt.h | 9 +-
arch/arm64/include/asm/pvclock-abi.h | 17 +++
arch/arm64/include/uapi/asm/kvm.h | 2 +
arch/arm64/kernel/cpu_errata.c | 80 +++++--------
arch/arm64/kernel/paravirt.c | 148 ++++++++++++++++++++++++
arch/arm64/kernel/time.c | 3 +
arch/arm64/kvm/Kconfig | 1 +
arch/arm64/kvm/Makefile | 2 +
arch/arm64/kvm/guest.c | 9 ++
arch/arm64/kvm/handle_exit.c | 4 +-
include/kvm/arm_hypercalls.h | 43 +++++++
include/kvm/arm_psci.h | 2 +-
include/linux/arm-smccc.h | 58 ++++++++++
include/linux/cpuhotplug.h | 1 +
include/linux/kvm_host.h | 26 ++++-
include/linux/kvm_types.h | 2 +
include/uapi/linux/kvm.h | 2 +
virt/kvm/arm/arm.c | 11 ++
virt/kvm/arm/hypercalls.c | 68 +++++++++++
virt/kvm/arm/psci.c | 84 +-------------
virt/kvm/arm/pvtime.c | 124 ++++++++++++++++++++
virt/kvm/kvm_main.c | 6 +-
29 files changed, 699 insertions(+), 154 deletions(-)
create mode 100644 Documentation/virt/kvm/arm/pvtime.txt
create mode 100644 arch/arm64/include/asm/pvclock-abi.h
create mode 100644 include/kvm/arm_hypercalls.h
create mode 100644 virt/kvm/arm/hypercalls.c
create mode 100644 virt/kvm/arm/pvtime.c
--
2.20.1
^ permalink raw reply
* [PATCH v4 08/10] arm/arm64: Provide a wrapper for SMCCC 1.1 calls
From: Steven Price @ 2019-08-30 8:42 UTC (permalink / raw)
To: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: Steven Price, Catalin Marinas, Paolo Bonzini,
Radim Krčmář, Russell King, James Morse,
Julien Thierry, Suzuki K Pouloze, Mark Rutland, kvm, linux-doc,
linux-kernel
In-Reply-To: <20190830084255.55113-1-steven.price@arm.com>
SMCCC 1.1 calls may use either HVC or SMC depending on the PSCI
conduit. Rather than coding this in every call site provide a macro
which uses the correct instruction. The macro also handles the case
where no PSCI conduit is configured returning a not supported error
in res, along with returning the conduit used for the call.
This allow us to remove some duplicated code and will be useful later
when adding paravirtualized time hypervisor calls.
Signed-off-by: Steven Price <steven.price@arm.com>
Acked-by: Will Deacon <will@kernel.org>
---
include/linux/arm-smccc.h | 44 +++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index e7f129f26ebd..eee1e832221d 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -303,6 +303,50 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
#define SMCCC_RET_NOT_SUPPORTED -1
#define SMCCC_RET_NOT_REQUIRED -2
+/* Like arm_smccc_1_1* but always returns SMCCC_RET_NOT_SUPPORTED.
+ * Used when the PSCI conduit is not defined. The empty asm statement
+ * avoids compiler warnings about unused variables.
+ */
+#define __fail_smccc_1_1(...) \
+ do { \
+ __declare_args(__count_args(__VA_ARGS__), __VA_ARGS__); \
+ asm ("" __constraints(__count_args(__VA_ARGS__))); \
+ if (___res) \
+ ___res->a0 = SMCCC_RET_NOT_SUPPORTED; \
+ } while (0)
+
+/*
+ * arm_smccc_1_1_invoke() - make an SMCCC v1.1 compliant call
+ *
+ * This is a variadic macro taking one to eight source arguments, and
+ * an optional return structure.
+ *
+ * @a0-a7: arguments passed in registers 0 to 7
+ * @res: result values from registers 0 to 3
+ *
+ * This macro will make either an HVC call or an SMC call depending on the
+ * current PSCI conduit. If no valid conduit is available then -1
+ * (SMCCC_RET_NOT_SUPPORTED) is returned in @res.a0 (if supplied).
+ *
+ * The return value also provides the conduit that was used.
+ */
+#define arm_smccc_1_1_invoke(...) ({ \
+ int method = psci_ops.conduit; \
+ switch (method) { \
+ case PSCI_CONDUIT_HVC: \
+ arm_smccc_1_1_hvc(__VA_ARGS__); \
+ break; \
+ case PSCI_CONDUIT_SMC: \
+ arm_smccc_1_1_smc(__VA_ARGS__); \
+ break; \
+ default: \
+ __fail_smccc_1_1(__VA_ARGS__); \
+ method = PSCI_CONDUIT_NONE; \
+ break; \
+ } \
+ method; \
+ })
+
/* Paravirtualised time calls (defined by ARM DEN0057A) */
#define ARM_SMCCC_HV_PV_FEATURES \
ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
--
2.20.1
^ permalink raw reply related
* [PATCH v4 09/10] arm/arm64: Make use of the SMCCC 1.1 wrapper
From: Steven Price @ 2019-08-30 8:42 UTC (permalink / raw)
To: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: Steven Price, Catalin Marinas, Paolo Bonzini,
Radim Krčmář, Russell King, James Morse,
Julien Thierry, Suzuki K Pouloze, Mark Rutland, kvm, linux-doc,
linux-kernel
In-Reply-To: <20190830084255.55113-1-steven.price@arm.com>
Rather than directly choosing which function to use based on
psci_ops.conduit, use the new arm_smccc_1_1 wrapper instead.
In some cases we still need to do some operations based on the
conduit, but the code duplication is removed.
No functional change.
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm/mm/proc-v7-bugs.c | 13 +++---
arch/arm64/kernel/cpu_errata.c | 80 ++++++++++++----------------------
2 files changed, 33 insertions(+), 60 deletions(-)
diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
index 9a07916af8dd..8eb52f3385e7 100644
--- a/arch/arm/mm/proc-v7-bugs.c
+++ b/arch/arm/mm/proc-v7-bugs.c
@@ -78,12 +78,13 @@ static void cpu_v7_spectre_init(void)
if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
break;
+ arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+ ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+ if ((int)res.a0 != 0)
+ return;
+
switch (psci_ops.conduit) {
case PSCI_CONDUIT_HVC:
- arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
- ARM_SMCCC_ARCH_WORKAROUND_1, &res);
- if ((int)res.a0 != 0)
- break;
per_cpu(harden_branch_predictor_fn, cpu) =
call_hvc_arch_workaround_1;
cpu_do_switch_mm = cpu_v7_hvc_switch_mm;
@@ -91,10 +92,6 @@ static void cpu_v7_spectre_init(void)
break;
case PSCI_CONDUIT_SMC:
- arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
- ARM_SMCCC_ARCH_WORKAROUND_1, &res);
- if ((int)res.a0 != 0)
- break;
per_cpu(harden_branch_predictor_fn, cpu) =
call_smc_arch_workaround_1;
cpu_do_switch_mm = cpu_v7_smc_switch_mm;
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 1e43ba5c79b7..400a49aaae85 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -215,40 +215,31 @@ static int detect_harden_bp_fw(void)
if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
return -1;
+ arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+ ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+
+ switch ((int)res.a0) {
+ case 1:
+ /* Firmware says we're just fine */
+ return 0;
+ case 0:
+ break;
+ default:
+ return -1;
+ }
+
switch (psci_ops.conduit) {
case PSCI_CONDUIT_HVC:
- arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
- ARM_SMCCC_ARCH_WORKAROUND_1, &res);
- switch ((int)res.a0) {
- case 1:
- /* Firmware says we're just fine */
- return 0;
- case 0:
- cb = call_hvc_arch_workaround_1;
- /* This is a guest, no need to patch KVM vectors */
- smccc_start = NULL;
- smccc_end = NULL;
- break;
- default:
- return -1;
- }
+ cb = call_hvc_arch_workaround_1;
+ /* This is a guest, no need to patch KVM vectors */
+ smccc_start = NULL;
+ smccc_end = NULL;
break;
case PSCI_CONDUIT_SMC:
- arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
- ARM_SMCCC_ARCH_WORKAROUND_1, &res);
- switch ((int)res.a0) {
- case 1:
- /* Firmware says we're just fine */
- return 0;
- case 0:
- cb = call_smc_arch_workaround_1;
- smccc_start = __smccc_workaround_1_smc_start;
- smccc_end = __smccc_workaround_1_smc_end;
- break;
- default:
- return -1;
- }
+ cb = call_smc_arch_workaround_1;
+ smccc_start = __smccc_workaround_1_smc_start;
+ smccc_end = __smccc_workaround_1_smc_end;
break;
default:
@@ -338,6 +329,7 @@ void __init arm64_enable_wa2_handling(struct alt_instr *alt,
void arm64_set_ssbd_mitigation(bool state)
{
+ int conduit;
if (!IS_ENABLED(CONFIG_ARM64_SSBD)) {
pr_info_once("SSBD disabled by kernel configuration\n");
return;
@@ -351,19 +343,10 @@ void arm64_set_ssbd_mitigation(bool state)
return;
}
- switch (psci_ops.conduit) {
- case PSCI_CONDUIT_HVC:
- arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_2, state, NULL);
- break;
+ conduit = arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_WORKAROUND_2, state,
+ NULL);
- case PSCI_CONDUIT_SMC:
- arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2, state, NULL);
- break;
-
- default:
- WARN_ON_ONCE(1);
- break;
- }
+ WARN_ON_ONCE(conduit == PSCI_CONDUIT_NONE);
}
static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry,
@@ -373,6 +356,7 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry,
bool required = true;
s32 val;
bool this_cpu_safe = false;
+ int conduit;
WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
@@ -397,18 +381,10 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry,
return false;
}
- switch (psci_ops.conduit) {
- case PSCI_CONDUIT_HVC:
- arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
- ARM_SMCCC_ARCH_WORKAROUND_2, &res);
- break;
-
- case PSCI_CONDUIT_SMC:
- arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
- ARM_SMCCC_ARCH_WORKAROUND_2, &res);
- break;
+ conduit = arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+ ARM_SMCCC_ARCH_WORKAROUND_2, &res);
- default:
+ if (conduit == PSCI_CONDUIT_NONE) {
ssbd_state = ARM64_SSBD_UNKNOWN;
if (!this_cpu_safe)
__ssb_safe = false;
--
2.20.1
^ permalink raw reply related
* [PATCH v4 10/10] arm64: Retrieve stolen time as paravirtualized guest
From: Steven Price @ 2019-08-30 8:42 UTC (permalink / raw)
To: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: Steven Price, Catalin Marinas, Paolo Bonzini,
Radim Krčmář, Russell King, James Morse,
Julien Thierry, Suzuki K Pouloze, Mark Rutland, kvm, linux-doc,
linux-kernel
In-Reply-To: <20190830084255.55113-1-steven.price@arm.com>
Enable paravirtualization features when running under a hypervisor
supporting the PV_TIME_ST hypercall.
For each (v)CPU, we ask the hypervisor for the location of a shared
page which the hypervisor will use to report stolen time to us. We set
pv_time_ops to the stolen time function which simply reads the stolen
value from the shared page for a VCPU. We guarantee single-copy
atomicity using READ_ONCE which means we can also read the stolen
time for another VCPU than the currently running one while it is
potentially being updated by the hypervisor.
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm64/include/asm/paravirt.h | 9 +-
arch/arm64/kernel/paravirt.c | 148 ++++++++++++++++++++++++++++++
arch/arm64/kernel/time.c | 3 +
include/linux/cpuhotplug.h | 1 +
4 files changed, 160 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/paravirt.h b/arch/arm64/include/asm/paravirt.h
index 799d9dd6f7cc..125c26c42902 100644
--- a/arch/arm64/include/asm/paravirt.h
+++ b/arch/arm64/include/asm/paravirt.h
@@ -21,6 +21,13 @@ static inline u64 paravirt_steal_clock(int cpu)
{
return pv_ops.time.steal_clock(cpu);
}
-#endif
+
+int __init kvm_guest_init(void);
+
+#else
+
+#define kvm_guest_init()
+
+#endif // CONFIG_PARAVIRT
#endif
diff --git a/arch/arm64/kernel/paravirt.c b/arch/arm64/kernel/paravirt.c
index 4cfed91fe256..5bf3be7ccf7e 100644
--- a/arch/arm64/kernel/paravirt.c
+++ b/arch/arm64/kernel/paravirt.c
@@ -6,13 +6,161 @@
* Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
*/
+#define pr_fmt(fmt) "kvmarm-pv: " fmt
+
+#include <linux/arm-smccc.h>
+#include <linux/cpuhotplug.h>
#include <linux/export.h>
+#include <linux/io.h>
#include <linux/jump_label.h>
+#include <linux/printk.h>
+#include <linux/psci.h>
+#include <linux/reboot.h>
+#include <linux/slab.h>
#include <linux/types.h>
+
#include <asm/paravirt.h>
+#include <asm/pvclock-abi.h>
+#include <asm/smp_plat.h>
struct static_key paravirt_steal_enabled;
struct static_key paravirt_steal_rq_enabled;
struct paravirt_patch_template pv_ops;
EXPORT_SYMBOL_GPL(pv_ops);
+
+struct kvmarm_stolen_time_region {
+ struct pvclock_vcpu_stolen_time *kaddr;
+};
+
+static DEFINE_PER_CPU(struct kvmarm_stolen_time_region, stolen_time_region);
+
+static bool steal_acc = true;
+static int __init parse_no_stealacc(char *arg)
+{
+ steal_acc = false;
+ return 0;
+}
+
+early_param("no-steal-acc", parse_no_stealacc);
+
+/* return stolen time in ns by asking the hypervisor */
+static u64 kvm_steal_clock(int cpu)
+{
+ struct kvmarm_stolen_time_region *reg;
+
+ reg = per_cpu_ptr(&stolen_time_region, cpu);
+ if (!reg->kaddr) {
+ pr_warn_once("stolen time enabled but not configured for cpu %d\n",
+ cpu);
+ return 0;
+ }
+
+ return le64_to_cpu(READ_ONCE(reg->kaddr->stolen_time));
+}
+
+static int disable_stolen_time_current_cpu(void)
+{
+ struct kvmarm_stolen_time_region *reg;
+
+ reg = this_cpu_ptr(&stolen_time_region);
+ if (!reg->kaddr)
+ return 0;
+
+ memunmap(reg->kaddr);
+ memset(reg, 0, sizeof(*reg));
+
+ return 0;
+}
+
+static int stolen_time_dying_cpu(unsigned int cpu)
+{
+ return disable_stolen_time_current_cpu();
+}
+
+static int init_stolen_time_cpu(unsigned int cpu)
+{
+ struct kvmarm_stolen_time_region *reg;
+ struct arm_smccc_res res;
+
+ reg = this_cpu_ptr(&stolen_time_region);
+
+ arm_smccc_1_1_invoke(ARM_SMCCC_HV_PV_TIME_ST, &res);
+
+ if ((long)res.a0 < 0)
+ return -EINVAL;
+
+ reg->kaddr = memremap(res.a0,
+ sizeof(struct pvclock_vcpu_stolen_time),
+ MEMREMAP_WB);
+
+ if (!reg->kaddr) {
+ pr_warn("Failed to map stolen time data structure\n");
+ return -ENOMEM;
+ }
+
+ if (le32_to_cpu(reg->kaddr->revision) != 0 ||
+ le32_to_cpu(reg->kaddr->attributes) != 0) {
+ pr_warn("Unexpected revision or attributes in stolen time data\n");
+ return -ENXIO;
+ }
+
+ return 0;
+}
+
+static int kvm_arm_init_stolen_time(void)
+{
+ int ret;
+
+ ret = cpuhp_setup_state(CPUHP_AP_ARM_KVMPV_STARTING,
+ "hypervisor/kvmarm/pv:starting",
+ init_stolen_time_cpu, stolen_time_dying_cpu);
+ if (ret < 0)
+ return ret;
+ return 0;
+}
+
+static bool has_kvm_steal_clock(void)
+{
+ struct arm_smccc_res res;
+
+ /* To detect the presence of PV time support we require SMCCC 1.1+ */
+ if (psci_ops.smccc_version < SMCCC_VERSION_1_1)
+ return false;
+
+ arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+ ARM_SMCCC_HV_PV_FEATURES, &res);
+
+ if (res.a0 != SMCCC_RET_SUCCESS)
+ return false;
+
+ arm_smccc_1_1_invoke(ARM_SMCCC_HV_PV_FEATURES,
+ ARM_SMCCC_HV_PV_TIME_ST, &res);
+
+ if (res.a0 != SMCCC_RET_SUCCESS)
+ return false;
+
+ return true;
+}
+
+int __init kvm_guest_init(void)
+{
+ int ret;
+
+ if (!has_kvm_steal_clock())
+ return 0;
+
+ ret = kvm_arm_init_stolen_time();
+ if (ret)
+ return ret;
+
+ pv_ops.time.steal_clock = kvm_steal_clock;
+
+ static_key_slow_inc(¶virt_steal_enabled);
+ if (steal_acc)
+ static_key_slow_inc(¶virt_steal_rq_enabled);
+
+ pr_info("using stolen time PV\n");
+
+ return 0;
+}
diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c
index 0b2946414dc9..a52aea14c6ec 100644
--- a/arch/arm64/kernel/time.c
+++ b/arch/arm64/kernel/time.c
@@ -30,6 +30,7 @@
#include <asm/thread_info.h>
#include <asm/stacktrace.h>
+#include <asm/paravirt.h>
unsigned long profile_pc(struct pt_regs *regs)
{
@@ -65,4 +66,6 @@ void __init time_init(void)
/* Calibrate the delay loop directly */
lpj_fine = arch_timer_rate / HZ;
+
+ kvm_guest_init();
}
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 068793a619ca..89d75edb5750 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -136,6 +136,7 @@ enum cpuhp_state {
/* Must be the last timer callback */
CPUHP_AP_DUMMY_TIMER_STARTING,
CPUHP_AP_ARM_XEN_STARTING,
+ CPUHP_AP_ARM_KVMPV_STARTING,
CPUHP_AP_ARM_CORESIGHT_STARTING,
CPUHP_AP_ARM64_ISNDEP_STARTING,
CPUHP_AP_SMPCFD_DYING,
--
2.20.1
^ permalink raw reply related
* [PATCH v4 01/10] KVM: arm64: Document PV-time interface
From: Steven Price @ 2019-08-30 8:42 UTC (permalink / raw)
To: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: Steven Price, Catalin Marinas, Paolo Bonzini,
Radim Krčmář, Russell King, James Morse,
Julien Thierry, Suzuki K Pouloze, Mark Rutland, kvm, linux-doc,
linux-kernel
In-Reply-To: <20190830084255.55113-1-steven.price@arm.com>
Introduce a paravirtualization interface for KVM/arm64 based on the
"Arm Paravirtualized Time for Arm-Base Systems" specification DEN 0057A.
This only adds the details about "Stolen Time" as the details of "Live
Physical Time" have not been fully agreed.
User space can specify a reserved area of memory for the guest and
inform KVM to populate the memory with information on time that the host
kernel has stolen from the guest.
A hypercall interface is provided for the guest to interrogate the
hypervisor's support for this interface and the location of the shared
memory structures.
Signed-off-by: Steven Price <steven.price@arm.com>
---
Documentation/virt/kvm/arm/pvtime.txt | 64 +++++++++++++++++++++++++
Documentation/virt/kvm/devices/vcpu.txt | 14 ++++++
2 files changed, 78 insertions(+)
create mode 100644 Documentation/virt/kvm/arm/pvtime.txt
diff --git a/Documentation/virt/kvm/arm/pvtime.txt b/Documentation/virt/kvm/arm/pvtime.txt
new file mode 100644
index 000000000000..dda3f0f855b9
--- /dev/null
+++ b/Documentation/virt/kvm/arm/pvtime.txt
@@ -0,0 +1,64 @@
+Paravirtualized time support for arm64
+======================================
+
+Arm specification DEN0057/A defined a standard for paravirtualised time
+support for AArch64 guests:
+
+https://developer.arm.com/docs/den0057/a
+
+KVM/arm64 implements the stolen time part of this specification by providing
+some hypervisor service calls to support a paravirtualized guest obtaining a
+view of the amount of time stolen from its execution.
+
+Two new SMCCC compatible hypercalls are defined:
+
+PV_FEATURES 0xC5000020
+PV_TIME_ST 0xC5000022
+
+These are only available in the SMC64/HVC64 calling convention as
+paravirtualized time is not available to 32 bit Arm guests. The existence of
+the PV_FEATURES hypercall should be probed using the SMCCC 1.1 ARCH_FEATURES
+mechanism before calling it.
+
+PV_FEATURES
+ Function ID: (uint32) : 0xC5000020
+ PV_func_id: (uint32) : Either PV_TIME_LPT or PV_TIME_ST
+ Return value: (int32) : NOT_SUPPORTED (-1) or SUCCESS (0) if the relevant
+ PV-time feature is supported by the hypervisor.
+
+PV_TIME_ST
+ Function ID: (uint32) : 0xC5000022
+ Return value: (int64) : IPA of the stolen time data structure for this
+ VCPU. On failure:
+ NOT_SUPPORTED (-1)
+
+The IPA returned by PV_TIME_ST should be mapped by the guest as normal memory
+with inner and outer write back caching attributes, in the inner shareable
+domain. A total of 16 bytes from the IPA returned are guaranteed to be
+meaningfully filled by the hypervisor (see structure below).
+
+PV_TIME_ST returns the structure for the calling VCPU.
+
+Stolen Time
+-----------
+
+The structure pointed to by the PV_TIME_ST hypercall is as follows:
+
+ Field | Byte Length | Byte Offset | Description
+ ----------- | ----------- | ----------- | --------------------------
+ Revision | 4 | 0 | Must be 0 for version 0.1
+ Attributes | 4 | 4 | Must be 0
+ Stolen time | 8 | 8 | Stolen time in unsigned
+ | | | nanoseconds indicating how
+ | | | much time this VCPU thread
+ | | | was involuntarily not
+ | | | running on a physical CPU.
+
+The structure will be updated by the hypervisor prior to scheduling a VCPU. It
+will be present within a reserved region of the normal memory given to the
+guest. The guest should not attempt to write into this memory. There is a
+structure per VCPU of the guest.
+
+For the user space interface see Documentation/virt/kvm/devices/vcpu.txt
+section "3. GROUP: KVM_ARM_VCPU_PVTIME_CTRL".
+
diff --git a/Documentation/virt/kvm/devices/vcpu.txt b/Documentation/virt/kvm/devices/vcpu.txt
index 2b5dab16c4f2..896777f76f36 100644
--- a/Documentation/virt/kvm/devices/vcpu.txt
+++ b/Documentation/virt/kvm/devices/vcpu.txt
@@ -60,3 +60,17 @@ time to use the number provided for a given timer, overwriting any previously
configured values on other VCPUs. Userspace should configure the interrupt
numbers on at least one VCPU after creating all VCPUs and before running any
VCPUs.
+
+3. GROUP: KVM_ARM_VCPU_PVTIME_CTRL
+Architectures: ARM64
+
+3.1 ATTRIBUTE: KVM_ARM_VCPU_PVTIME_SET_IPA
+Parameters: 64-bit base address
+Returns: -ENXIO: Stolen time not implemented
+ -EEXIST: Base address already set for this VCPU
+ -EINVAL: Base address not 64 byte aligned
+
+Specifies the base address of the stolen time structure for this VCPU. The
+base address must be 64 byte aligned and exist within a valid guest memory
+region. See Documentation/virt/kvm/arm/pvtime.txt for more information
+including the layout of the stolen time structure.
--
2.20.1
^ permalink raw reply related
* [PATCH v4 02/10] KVM: arm/arm64: Factor out hypercall handling from PSCI code
From: Steven Price @ 2019-08-30 8:42 UTC (permalink / raw)
To: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: Steven Price, Catalin Marinas, Paolo Bonzini,
Radim Krčmář, Russell King, James Morse,
Julien Thierry, Suzuki K Pouloze, Mark Rutland, kvm, linux-doc,
linux-kernel, Christoffer Dall
In-Reply-To: <20190830084255.55113-1-steven.price@arm.com>
From: Christoffer Dall <christoffer.dall@arm.com>
We currently intertwine the KVM PSCI implementation with the general
dispatch of hypercall handling, which makes perfect sense because PSCI
is the only category of hypercalls we support.
However, as we are about to support additional hypercalls, factor out
this functionality into a separate hypercall handler file.
Signed-off-by: Christoffer Dall <christoffer.dall@arm.com>
[steven.price@arm.com: rebased]
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm/kvm/Makefile | 2 +-
arch/arm/kvm/handle_exit.c | 2 +-
arch/arm64/kvm/Makefile | 1 +
arch/arm64/kvm/handle_exit.c | 4 +-
include/kvm/arm_hypercalls.h | 43 ++++++++++++++++++
include/kvm/arm_psci.h | 2 +-
virt/kvm/arm/hypercalls.c | 59 +++++++++++++++++++++++++
virt/kvm/arm/psci.c | 84 +-----------------------------------
8 files changed, 110 insertions(+), 87 deletions(-)
create mode 100644 include/kvm/arm_hypercalls.h
create mode 100644 virt/kvm/arm/hypercalls.c
diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile
index 531e59f5be9c..ef4d01088efc 100644
--- a/arch/arm/kvm/Makefile
+++ b/arch/arm/kvm/Makefile
@@ -23,7 +23,7 @@ obj-y += kvm-arm.o init.o interrupts.o
obj-y += handle_exit.o guest.o emulate.o reset.o
obj-y += coproc.o coproc_a15.o coproc_a7.o vgic-v3-coproc.o
obj-y += $(KVM)/arm/arm.o $(KVM)/arm/mmu.o $(KVM)/arm/mmio.o
-obj-y += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
+obj-y += $(KVM)/arm/psci.o $(KVM)/arm/perf.o $(KVM)/arm/hypercalls.o
obj-y += $(KVM)/arm/aarch32.o
obj-y += $(KVM)/arm/vgic/vgic.o
diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
index 2a6a1394d26e..e58a89d2f13f 100644
--- a/arch/arm/kvm/handle_exit.c
+++ b/arch/arm/kvm/handle_exit.c
@@ -9,7 +9,7 @@
#include <asm/kvm_emulate.h>
#include <asm/kvm_coproc.h>
#include <asm/kvm_mmu.h>
-#include <kvm/arm_psci.h>
+#include <kvm/arm_hypercalls.h>
#include <trace/events/kvm.h>
#include "trace.h"
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 3ac1a64d2fb9..73dce4d47d47 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_KVM_ARM_HOST) += hyp/
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o $(KVM)/vfio.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arm.o $(KVM)/arm/mmu.o $(KVM)/arm/mmio.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
+kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hypercalls.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
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 706cca23f0d2..aacfc55de44c 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -11,8 +11,6 @@
#include <linux/kvm.h>
#include <linux/kvm_host.h>
-#include <kvm/arm_psci.h>
-
#include <asm/esr.h>
#include <asm/exception.h>
#include <asm/kvm_asm.h>
@@ -22,6 +20,8 @@
#include <asm/debug-monitors.h>
#include <asm/traps.h>
+#include <kvm/arm_hypercalls.h>
+
#define CREATE_TRACE_POINTS
#include "trace.h"
diff --git a/include/kvm/arm_hypercalls.h b/include/kvm/arm_hypercalls.h
new file mode 100644
index 000000000000..0e2509d27910
--- /dev/null
+++ b/include/kvm/arm_hypercalls.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2019 Arm Ltd. */
+
+#ifndef __KVM_ARM_HYPERCALLS_H
+#define __KVM_ARM_HYPERCALLS_H
+
+#include <asm/kvm_emulate.h>
+
+int kvm_hvc_call_handler(struct kvm_vcpu *vcpu);
+
+static inline u32 smccc_get_function(struct kvm_vcpu *vcpu)
+{
+ return vcpu_get_reg(vcpu, 0);
+}
+
+static inline unsigned long smccc_get_arg1(struct kvm_vcpu *vcpu)
+{
+ return vcpu_get_reg(vcpu, 1);
+}
+
+static inline unsigned long smccc_get_arg2(struct kvm_vcpu *vcpu)
+{
+ return vcpu_get_reg(vcpu, 2);
+}
+
+static inline unsigned long smccc_get_arg3(struct kvm_vcpu *vcpu)
+{
+ return vcpu_get_reg(vcpu, 3);
+}
+
+static inline void smccc_set_retval(struct kvm_vcpu *vcpu,
+ unsigned long a0,
+ unsigned long a1,
+ unsigned long a2,
+ unsigned long a3)
+{
+ vcpu_set_reg(vcpu, 0, a0);
+ vcpu_set_reg(vcpu, 1, a1);
+ vcpu_set_reg(vcpu, 2, a2);
+ vcpu_set_reg(vcpu, 3, a3);
+}
+
+#endif
diff --git a/include/kvm/arm_psci.h b/include/kvm/arm_psci.h
index 632e78bdef4d..5b58bd2fe088 100644
--- a/include/kvm/arm_psci.h
+++ b/include/kvm/arm_psci.h
@@ -40,7 +40,7 @@ static inline int kvm_psci_version(struct kvm_vcpu *vcpu, struct kvm *kvm)
}
-int kvm_hvc_call_handler(struct kvm_vcpu *vcpu);
+int kvm_psci_call(struct kvm_vcpu *vcpu);
struct kvm_one_reg;
diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
new file mode 100644
index 000000000000..f875241bd030
--- /dev/null
+++ b/virt/kvm/arm/hypercalls.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2019 Arm Ltd.
+
+#include <linux/arm-smccc.h>
+#include <linux/kvm_host.h>
+
+#include <asm/kvm_emulate.h>
+
+#include <kvm/arm_hypercalls.h>
+#include <kvm/arm_psci.h>
+
+int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
+{
+ u32 func_id = smccc_get_function(vcpu);
+ u32 val = SMCCC_RET_NOT_SUPPORTED;
+ u32 feature;
+
+ switch (func_id) {
+ case ARM_SMCCC_VERSION_FUNC_ID:
+ val = ARM_SMCCC_VERSION_1_1;
+ break;
+ case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
+ feature = smccc_get_arg1(vcpu);
+ switch (feature) {
+ case ARM_SMCCC_ARCH_WORKAROUND_1:
+ switch (kvm_arm_harden_branch_predictor()) {
+ case KVM_BP_HARDEN_UNKNOWN:
+ break;
+ case KVM_BP_HARDEN_WA_NEEDED:
+ val = SMCCC_RET_SUCCESS;
+ break;
+ case KVM_BP_HARDEN_NOT_REQUIRED:
+ val = SMCCC_RET_NOT_REQUIRED;
+ break;
+ }
+ break;
+ case ARM_SMCCC_ARCH_WORKAROUND_2:
+ switch (kvm_arm_have_ssbd()) {
+ case KVM_SSBD_FORCE_DISABLE:
+ case KVM_SSBD_UNKNOWN:
+ break;
+ case KVM_SSBD_KERNEL:
+ val = SMCCC_RET_SUCCESS;
+ break;
+ case KVM_SSBD_FORCE_ENABLE:
+ case KVM_SSBD_MITIGATED:
+ val = SMCCC_RET_NOT_REQUIRED;
+ break;
+ }
+ break;
+ }
+ break;
+ default:
+ return kvm_psci_call(vcpu);
+ }
+
+ smccc_set_retval(vcpu, val, 0, 0, 0);
+ return 1;
+}
diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index 87927f7e1ee7..17e2bdd4b76f 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -15,6 +15,7 @@
#include <asm/kvm_host.h>
#include <kvm/arm_psci.h>
+#include <kvm/arm_hypercalls.h>
/*
* This is an implementation of the Power State Coordination Interface
@@ -23,38 +24,6 @@
#define AFFINITY_MASK(level) ~((0x1UL << ((level) * MPIDR_LEVEL_BITS)) - 1)
-static u32 smccc_get_function(struct kvm_vcpu *vcpu)
-{
- return vcpu_get_reg(vcpu, 0);
-}
-
-static unsigned long smccc_get_arg1(struct kvm_vcpu *vcpu)
-{
- return vcpu_get_reg(vcpu, 1);
-}
-
-static unsigned long smccc_get_arg2(struct kvm_vcpu *vcpu)
-{
- return vcpu_get_reg(vcpu, 2);
-}
-
-static unsigned long smccc_get_arg3(struct kvm_vcpu *vcpu)
-{
- return vcpu_get_reg(vcpu, 3);
-}
-
-static void smccc_set_retval(struct kvm_vcpu *vcpu,
- unsigned long a0,
- unsigned long a1,
- unsigned long a2,
- unsigned long a3)
-{
- vcpu_set_reg(vcpu, 0, a0);
- vcpu_set_reg(vcpu, 1, a1);
- vcpu_set_reg(vcpu, 2, a2);
- vcpu_set_reg(vcpu, 3, a3);
-}
-
static unsigned long psci_affinity_mask(unsigned long affinity_level)
{
if (affinity_level <= 3)
@@ -373,7 +342,7 @@ static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
* Errors:
* -EINVAL: Unrecognized PSCI function
*/
-static int kvm_psci_call(struct kvm_vcpu *vcpu)
+int kvm_psci_call(struct kvm_vcpu *vcpu)
{
switch (kvm_psci_version(vcpu, vcpu->kvm)) {
case KVM_ARM_PSCI_1_0:
@@ -387,55 +356,6 @@ static int kvm_psci_call(struct kvm_vcpu *vcpu)
};
}
-int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
-{
- u32 func_id = smccc_get_function(vcpu);
- u32 val = SMCCC_RET_NOT_SUPPORTED;
- u32 feature;
-
- switch (func_id) {
- case ARM_SMCCC_VERSION_FUNC_ID:
- val = ARM_SMCCC_VERSION_1_1;
- break;
- case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
- feature = smccc_get_arg1(vcpu);
- switch(feature) {
- case ARM_SMCCC_ARCH_WORKAROUND_1:
- switch (kvm_arm_harden_branch_predictor()) {
- case KVM_BP_HARDEN_UNKNOWN:
- break;
- case KVM_BP_HARDEN_WA_NEEDED:
- val = SMCCC_RET_SUCCESS;
- break;
- case KVM_BP_HARDEN_NOT_REQUIRED:
- val = SMCCC_RET_NOT_REQUIRED;
- break;
- }
- break;
- case ARM_SMCCC_ARCH_WORKAROUND_2:
- switch (kvm_arm_have_ssbd()) {
- case KVM_SSBD_FORCE_DISABLE:
- case KVM_SSBD_UNKNOWN:
- break;
- case KVM_SSBD_KERNEL:
- val = SMCCC_RET_SUCCESS;
- break;
- case KVM_SSBD_FORCE_ENABLE:
- case KVM_SSBD_MITIGATED:
- val = SMCCC_RET_NOT_REQUIRED;
- break;
- }
- break;
- }
- break;
- default:
- return kvm_psci_call(vcpu);
- }
-
- smccc_set_retval(vcpu, val, 0, 0, 0);
- return 1;
-}
-
int kvm_arm_get_fw_num_regs(struct kvm_vcpu *vcpu)
{
return 3; /* PSCI version and two workaround registers */
--
2.20.1
^ permalink raw reply related
* [PATCH v4 04/10] KVM: Implement kvm_put_guest()
From: Steven Price @ 2019-08-30 8:42 UTC (permalink / raw)
To: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: Steven Price, Catalin Marinas, Paolo Bonzini,
Radim Krčmář, Russell King, James Morse,
Julien Thierry, Suzuki K Pouloze, Mark Rutland, kvm, linux-doc,
linux-kernel
In-Reply-To: <20190830084255.55113-1-steven.price@arm.com>
kvm_put_guest() is analogous to put_user() - it writes a single value to
the guest physical address. The implementation is built upon put_user()
and so it has the same single copy atomic properties.
Signed-off-by: Steven Price <steven.price@arm.com>
---
include/linux/kvm_host.h | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index fcb46b3374c6..bf0ae1825b9c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -746,6 +746,28 @@ int kvm_write_guest_offset_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
unsigned long len);
int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
gpa_t gpa, unsigned long len);
+
+#define __kvm_put_guest(kvm, gfn, offset, value, type) \
+({ \
+ unsigned long __addr = gfn_to_hva(kvm, gfn); \
+ type __user *__uaddr = (type __user *)(__addr + offset); \
+ int __ret = -EFAULT; \
+ \
+ if (!kvm_is_error_hva(__addr)) \
+ __ret = put_user(value, __uaddr); \
+ if (!__ret) \
+ mark_page_dirty(kvm, gfn); \
+ __ret; \
+})
+
+#define kvm_put_guest(kvm, gpa, value, type) \
+({ \
+ gpa_t __gpa = gpa; \
+ struct kvm *__kvm = kvm; \
+ __kvm_put_guest(__kvm, __gpa >> PAGE_SHIFT, \
+ offset_in_page(__gpa), (value), type); \
+})
+
int kvm_clear_guest_page(struct kvm *kvm, gfn_t gfn, int offset, int len);
int kvm_clear_guest(struct kvm *kvm, gpa_t gpa, unsigned long len);
struct kvm_memory_slot *gfn_to_memslot(struct kvm *kvm, gfn_t gfn);
--
2.20.1
^ permalink raw reply related
* [PATCH v4 03/10] KVM: arm64: Implement PV_FEATURES call
From: Steven Price @ 2019-08-30 8:42 UTC (permalink / raw)
To: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: Steven Price, Catalin Marinas, Paolo Bonzini,
Radim Krčmář, Russell King, James Morse,
Julien Thierry, Suzuki K Pouloze, Mark Rutland, kvm, linux-doc,
linux-kernel
In-Reply-To: <20190830084255.55113-1-steven.price@arm.com>
This provides a mechanism for querying which paravirtualized features
are available in this hypervisor.
Also add the header file which defines the ABI for the paravirtualized
clock features we're about to add.
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm/include/asm/kvm_host.h | 6 ++++++
arch/arm64/include/asm/kvm_host.h | 2 ++
arch/arm64/include/asm/pvclock-abi.h | 17 +++++++++++++++++
arch/arm64/kvm/Makefile | 1 +
include/linux/arm-smccc.h | 14 ++++++++++++++
virt/kvm/arm/hypercalls.c | 8 +++++++-
virt/kvm/arm/pvtime.c | 21 +++++++++++++++++++++
7 files changed, 68 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/pvclock-abi.h
create mode 100644 virt/kvm/arm/pvtime.c
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 8a37c8e89777..5a0c3569ebde 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -7,6 +7,7 @@
#ifndef __ARM_KVM_HOST_H__
#define __ARM_KVM_HOST_H__
+#include <linux/arm-smccc.h>
#include <linux/errno.h>
#include <linux/types.h>
#include <linux/kvm_types.h>
@@ -323,6 +324,11 @@ static inline int kvm_arch_vm_ioctl_check_extension(struct kvm *kvm, long ext)
int kvm_perf_init(void);
int kvm_perf_teardown(void);
+static inline long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
+{
+ return SMCCC_RET_NOT_SUPPORTED;
+}
+
void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index f656169db8c3..93b46d9526d0 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -478,6 +478,8 @@ void handle_exit_early(struct kvm_vcpu *vcpu, struct kvm_run *run,
int kvm_perf_init(void);
int kvm_perf_teardown(void);
+long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu);
+
void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
diff --git a/arch/arm64/include/asm/pvclock-abi.h b/arch/arm64/include/asm/pvclock-abi.h
new file mode 100644
index 000000000000..c4f1c0a0789c
--- /dev/null
+++ b/arch/arm64/include/asm/pvclock-abi.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2019 Arm Ltd. */
+
+#ifndef __ASM_PVCLOCK_ABI_H
+#define __ASM_PVCLOCK_ABI_H
+
+/* The below structure is defined in ARM DEN0057A */
+
+struct pvclock_vcpu_stolen_time {
+ __le32 revision;
+ __le32 attributes;
+ __le64 stolen_time;
+ /* Structure must be 64 byte aligned, pad to that size */
+ u8 padding[48];
+} __packed;
+
+#endif
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 73dce4d47d47..5ffbdc39e780 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -14,6 +14,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/e
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arm.o $(KVM)/arm/mmu.o $(KVM)/arm/mmio.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hypercalls.o
+kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/pvtime.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
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 080012a6f025..e7f129f26ebd 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -45,6 +45,7 @@
#define ARM_SMCCC_OWNER_SIP 2
#define ARM_SMCCC_OWNER_OEM 3
#define ARM_SMCCC_OWNER_STANDARD 4
+#define ARM_SMCCC_OWNER_STANDARD_HYP 5
#define ARM_SMCCC_OWNER_TRUSTED_APP 48
#define ARM_SMCCC_OWNER_TRUSTED_APP_END 49
#define ARM_SMCCC_OWNER_TRUSTED_OS 50
@@ -302,5 +303,18 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
#define SMCCC_RET_NOT_SUPPORTED -1
#define SMCCC_RET_NOT_REQUIRED -2
+/* Paravirtualised time calls (defined by ARM DEN0057A) */
+#define ARM_SMCCC_HV_PV_FEATURES \
+ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+ ARM_SMCCC_SMC_64, \
+ ARM_SMCCC_OWNER_STANDARD_HYP, \
+ 0x20)
+
+#define ARM_SMCCC_HV_PV_TIME_ST \
+ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+ ARM_SMCCC_SMC_64, \
+ ARM_SMCCC_OWNER_STANDARD_HYP, \
+ 0x22)
+
#endif /*__ASSEMBLY__*/
#endif /*__LINUX_ARM_SMCCC_H*/
diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
index f875241bd030..e2521e0d3978 100644
--- a/virt/kvm/arm/hypercalls.c
+++ b/virt/kvm/arm/hypercalls.c
@@ -12,7 +12,7 @@
int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
{
u32 func_id = smccc_get_function(vcpu);
- u32 val = SMCCC_RET_NOT_SUPPORTED;
+ long val = SMCCC_RET_NOT_SUPPORTED;
u32 feature;
switch (func_id) {
@@ -48,8 +48,14 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
break;
}
break;
+ case ARM_SMCCC_HV_PV_FEATURES:
+ val = SMCCC_RET_SUCCESS;
+ break;
}
break;
+ case ARM_SMCCC_HV_PV_FEATURES:
+ val = kvm_hypercall_pv_features(vcpu);
+ break;
default:
return kvm_psci_call(vcpu);
}
diff --git a/virt/kvm/arm/pvtime.c b/virt/kvm/arm/pvtime.c
new file mode 100644
index 000000000000..7887a61651c6
--- /dev/null
+++ b/virt/kvm/arm/pvtime.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2019 Arm Ltd.
+
+#include <linux/arm-smccc.h>
+
+#include <kvm/arm_hypercalls.h>
+
+long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
+{
+ u32 feature = smccc_get_arg1(vcpu);
+ long val = SMCCC_RET_NOT_SUPPORTED;
+
+ switch (feature) {
+ case ARM_SMCCC_HV_PV_FEATURES:
+ val = SMCCC_RET_SUCCESS;
+ break;
+ }
+
+ return val;
+}
+
--
2.20.1
^ permalink raw reply related
* [PATCH v4 05/10] KVM: arm64: Support stolen time reporting via shared structure
From: Steven Price @ 2019-08-30 8:42 UTC (permalink / raw)
To: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: Steven Price, Catalin Marinas, Paolo Bonzini,
Radim Krčmář, Russell King, James Morse,
Julien Thierry, Suzuki K Pouloze, Mark Rutland, kvm, linux-doc,
linux-kernel
In-Reply-To: <20190830084255.55113-1-steven.price@arm.com>
Implement the service call for configuring a shared structure between a
VCPU and the hypervisor in which the hypervisor can write the time
stolen from the VCPU's execution time by other tasks on the host.
The hypervisor allocates memory which is placed at an IPA chosen by user
space. The hypervisor then updates the shared structure using
kvm_put_guest() to ensure single copy atomicity of the 64-bit value
reporting the stolen time in nanoseconds.
Whenever stolen time is enabled by the guest, the stolen time counter is
reset.
The stolen time itself is retrieved from the sched_info structure
maintained by the Linux scheduler code. We enable SCHEDSTATS when
selecting KVM Kconfig to ensure this value is meaningful.
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm/include/asm/kvm_host.h | 20 +++++++++++
arch/arm64/include/asm/kvm_host.h | 21 +++++++++++-
arch/arm64/kvm/Kconfig | 1 +
include/linux/kvm_types.h | 2 ++
virt/kvm/arm/arm.c | 11 ++++++
virt/kvm/arm/hypercalls.c | 3 ++
virt/kvm/arm/pvtime.c | 56 +++++++++++++++++++++++++++++++
7 files changed, 113 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 5a0c3569ebde..5c401482d62d 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -39,6 +39,7 @@
KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
#define KVM_REQ_IRQ_PENDING KVM_ARCH_REQ(1)
#define KVM_REQ_VCPU_RESET KVM_ARCH_REQ(2)
+#define KVM_REQ_RECORD_STEAL KVM_ARCH_REQ(3)
DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
@@ -329,6 +330,25 @@ static inline long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
return SMCCC_RET_NOT_SUPPORTED;
}
+static inline long kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu)
+{
+ return SMCCC_RET_NOT_SUPPORTED;
+}
+
+static inline int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init)
+{
+ return -ENOTSUPP;
+}
+
+static inline void kvm_arm_pvtime_vcpu_init(struct kvm_vcpu_arch *vcpu_arch)
+{
+}
+
+static inline bool kvm_arm_is_pvtime_enabled(struct kvm_vcpu_arch *vcpu_arch)
+{
+ return false;
+}
+
void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 93b46d9526d0..1697e63f6dd8 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -44,6 +44,7 @@
KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
#define KVM_REQ_IRQ_PENDING KVM_ARCH_REQ(1)
#define KVM_REQ_VCPU_RESET KVM_ARCH_REQ(2)
+#define KVM_REQ_RECORD_STEAL KVM_ARCH_REQ(3)
DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
@@ -338,8 +339,14 @@ struct kvm_vcpu_arch {
/* True when deferrable sysregs are loaded on the physical CPU,
* see kvm_vcpu_load_sysregs and kvm_vcpu_put_sysregs. */
bool sysregs_loaded_on_cpu;
-};
+ /* Guest PV state */
+ struct {
+ u64 steal;
+ u64 last_steal;
+ gpa_t base;
+ } steal;
+};
/* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
#define vcpu_sve_pffr(vcpu) ((void *)((char *)((vcpu)->arch.sve_state) + \
sve_ffr_offset((vcpu)->arch.sve_max_vl)))
@@ -479,6 +486,18 @@ int kvm_perf_init(void);
int kvm_perf_teardown(void);
long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu);
+long kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu);
+int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init);
+
+static inline void kvm_arm_pvtime_vcpu_init(struct kvm_vcpu_arch *vcpu_arch)
+{
+ vcpu_arch->steal.base = GPA_INVALID;
+}
+
+static inline bool kvm_arm_is_pvtime_enabled(struct kvm_vcpu_arch *vcpu_arch)
+{
+ return (vcpu_arch->steal.base != GPA_INVALID);
+}
void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index a67121d419a2..d8b88e40d223 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -39,6 +39,7 @@ config KVM
select IRQ_BYPASS_MANAGER
select HAVE_KVM_IRQ_BYPASS
select HAVE_KVM_VCPU_RUN_PID_CHANGE
+ select SCHEDSTATS
---help---
Support hosting virtualized guest machines.
We don't support KVM with 16K page tables yet, due to the multiple
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index bde5374ae021..1c88e69db3d9 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -35,6 +35,8 @@ typedef unsigned long gva_t;
typedef u64 gpa_t;
typedef u64 gfn_t;
+#define GPA_INVALID (~(gpa_t)0)
+
typedef unsigned long hva_t;
typedef u64 hpa_t;
typedef u64 hfn_t;
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 35a069815baf..eaceb2d0f0c0 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -40,6 +40,10 @@
#include <asm/kvm_coproc.h>
#include <asm/sections.h>
+#include <kvm/arm_hypercalls.h>
+#include <kvm/arm_pmu.h>
+#include <kvm/arm_psci.h>
+
#ifdef REQUIRES_VIRT
__asm__(".arch_extension virt");
#endif
@@ -350,6 +354,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
kvm_arm_reset_debug_ptr(vcpu);
+ kvm_arm_pvtime_vcpu_init(&vcpu->arch);
+
return kvm_vgic_vcpu_init(vcpu);
}
@@ -379,6 +385,8 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
kvm_vcpu_load_sysregs(vcpu);
kvm_arch_vcpu_load_fp(vcpu);
kvm_vcpu_pmu_restore_guest(vcpu);
+ if (kvm_arm_is_pvtime_enabled(&vcpu->arch))
+ kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu);
if (single_task_running())
vcpu_clear_wfe_traps(vcpu);
@@ -644,6 +652,9 @@ static void check_vcpu_requests(struct kvm_vcpu *vcpu)
* that a VCPU sees new virtual interrupts.
*/
kvm_check_request(KVM_REQ_IRQ_PENDING, vcpu);
+
+ if (kvm_check_request(KVM_REQ_RECORD_STEAL, vcpu))
+ kvm_update_stolen_time(vcpu, false);
}
}
diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
index e2521e0d3978..3091a5d2e842 100644
--- a/virt/kvm/arm/hypercalls.c
+++ b/virt/kvm/arm/hypercalls.c
@@ -56,6 +56,9 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
case ARM_SMCCC_HV_PV_FEATURES:
val = kvm_hypercall_pv_features(vcpu);
break;
+ case ARM_SMCCC_HV_PV_TIME_ST:
+ val = kvm_hypercall_stolen_time(vcpu);
+ break;
default:
return kvm_psci_call(vcpu);
}
diff --git a/virt/kvm/arm/pvtime.c b/virt/kvm/arm/pvtime.c
index 7887a61651c6..d9d0dbc6994b 100644
--- a/virt/kvm/arm/pvtime.c
+++ b/virt/kvm/arm/pvtime.c
@@ -3,8 +3,45 @@
#include <linux/arm-smccc.h>
+#include <asm/pvclock-abi.h>
+
#include <kvm/arm_hypercalls.h>
+int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init)
+{
+ struct kvm *kvm = vcpu->kvm;
+ u64 steal;
+ u64 steal_le;
+ u64 offset;
+ int idx;
+ u64 base = vcpu->arch.steal.base;
+
+ if (base == GPA_INVALID)
+ return -ENOTSUPP;
+
+ /* Let's do the local bookkeeping */
+ steal = vcpu->arch.steal.steal;
+ steal += current->sched_info.run_delay - vcpu->arch.steal.last_steal;
+ vcpu->arch.steal.last_steal = current->sched_info.run_delay;
+ vcpu->arch.steal.steal = steal;
+
+ steal_le = cpu_to_le64(steal);
+ idx = srcu_read_lock(&kvm->srcu);
+ if (init) {
+ struct pvclock_vcpu_stolen_time init_values = {
+ .revision = 0,
+ .attributes = 0
+ };
+ kvm_write_guest(kvm, base, &init_values,
+ sizeof(init_values));
+ }
+ offset = offsetof(struct pvclock_vcpu_stolen_time, stolen_time);
+ kvm_put_guest(kvm, base + offset, steal_le, u64);
+ srcu_read_unlock(&kvm->srcu, idx);
+
+ return 0;
+}
+
long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
{
u32 feature = smccc_get_arg1(vcpu);
@@ -12,6 +49,7 @@ long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
switch (feature) {
case ARM_SMCCC_HV_PV_FEATURES:
+ case ARM_SMCCC_HV_PV_TIME_ST:
val = SMCCC_RET_SUCCESS;
break;
}
@@ -19,3 +57,21 @@ long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
return val;
}
+long kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu)
+{
+ int err;
+
+ /*
+ * Start counting stolen time from the time the guest requests
+ * the feature enabled.
+ */
+ vcpu->arch.steal.steal = 0;
+ vcpu->arch.steal.last_steal = current->sched_info.run_delay;
+
+ err = kvm_update_stolen_time(vcpu, true);
+
+ if (err)
+ return SMCCC_RET_NOT_SUPPORTED;
+
+ return vcpu->arch.steal.base;
+}
--
2.20.1
^ permalink raw reply related
* [PATCH v4 07/10] KVM: arm64: Provide VCPU attributes for stolen time
From: Steven Price @ 2019-08-30 8:42 UTC (permalink / raw)
To: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: Steven Price, Catalin Marinas, Paolo Bonzini,
Radim Krčmář, Russell King, James Morse,
Julien Thierry, Suzuki K Pouloze, Mark Rutland, kvm, linux-doc,
linux-kernel
In-Reply-To: <20190830084255.55113-1-steven.price@arm.com>
Allow user space to inform the KVM host where in the physical memory
map the paravirtualized time structures should be located.
User space can set an attribute on the VCPU providing the IPA base
address of the stolen time structure for that VCPU. This must be
repeated for every VCPU in the VM.
The address is given in terms of the physical address visible to
the guest and must be 64 byte aligned. The guest will discover the
address via a hypercall.
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm64/include/asm/kvm_host.h | 7 +++++
arch/arm64/include/uapi/asm/kvm.h | 2 ++
arch/arm64/kvm/guest.c | 9 ++++++
include/uapi/linux/kvm.h | 2 ++
virt/kvm/arm/pvtime.c | 47 +++++++++++++++++++++++++++++++
5 files changed, 67 insertions(+)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 1697e63f6dd8..6af16b29a41f 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -489,6 +489,13 @@ long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu);
long kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu);
int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init);
+int kvm_arm_pvtime_set_attr(struct kvm_vcpu *vcpu,
+ struct kvm_device_attr *attr);
+int kvm_arm_pvtime_get_attr(struct kvm_vcpu *vcpu,
+ struct kvm_device_attr *attr);
+int kvm_arm_pvtime_has_attr(struct kvm_vcpu *vcpu,
+ struct kvm_device_attr *attr);
+
static inline void kvm_arm_pvtime_vcpu_init(struct kvm_vcpu_arch *vcpu_arch)
{
vcpu_arch->steal.base = GPA_INVALID;
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 9a507716ae2f..bde9f165ad3a 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -323,6 +323,8 @@ struct kvm_vcpu_events {
#define KVM_ARM_VCPU_TIMER_CTRL 1
#define KVM_ARM_VCPU_TIMER_IRQ_VTIMER 0
#define KVM_ARM_VCPU_TIMER_IRQ_PTIMER 1
+#define KVM_ARM_VCPU_PVTIME_CTRL 2
+#define KVM_ARM_VCPU_PVTIME_SET_IPA 0
/* KVM_IRQ_LINE irq field index values */
#define KVM_ARM_IRQ_TYPE_SHIFT 24
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index dfd626447482..d3ac9d2fd405 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -858,6 +858,9 @@ int kvm_arm_vcpu_arch_set_attr(struct kvm_vcpu *vcpu,
case KVM_ARM_VCPU_TIMER_CTRL:
ret = kvm_arm_timer_set_attr(vcpu, attr);
break;
+ case KVM_ARM_VCPU_PVTIME_CTRL:
+ ret = kvm_arm_pvtime_set_attr(vcpu, attr);
+ break;
default:
ret = -ENXIO;
break;
@@ -878,6 +881,9 @@ int kvm_arm_vcpu_arch_get_attr(struct kvm_vcpu *vcpu,
case KVM_ARM_VCPU_TIMER_CTRL:
ret = kvm_arm_timer_get_attr(vcpu, attr);
break;
+ case KVM_ARM_VCPU_PVTIME_CTRL:
+ ret = kvm_arm_pvtime_get_attr(vcpu, attr);
+ break;
default:
ret = -ENXIO;
break;
@@ -898,6 +904,9 @@ int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu,
case KVM_ARM_VCPU_TIMER_CTRL:
ret = kvm_arm_timer_has_attr(vcpu, attr);
break;
+ case KVM_ARM_VCPU_PVTIME_CTRL:
+ ret = kvm_arm_pvtime_has_attr(vcpu, attr);
+ break;
default:
ret = -ENXIO;
break;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 5e3f12d5359e..265156a984f2 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1222,6 +1222,8 @@ enum kvm_device_type {
#define KVM_DEV_TYPE_ARM_VGIC_ITS KVM_DEV_TYPE_ARM_VGIC_ITS
KVM_DEV_TYPE_XIVE,
#define KVM_DEV_TYPE_XIVE KVM_DEV_TYPE_XIVE
+ KVM_DEV_TYPE_ARM_PV_TIME,
+#define KVM_DEV_TYPE_ARM_PV_TIME KVM_DEV_TYPE_ARM_PV_TIME
KVM_DEV_TYPE_MAX,
};
diff --git a/virt/kvm/arm/pvtime.c b/virt/kvm/arm/pvtime.c
index d9d0dbc6994b..7b1834b98a68 100644
--- a/virt/kvm/arm/pvtime.c
+++ b/virt/kvm/arm/pvtime.c
@@ -2,7 +2,9 @@
// Copyright (C) 2019 Arm Ltd.
#include <linux/arm-smccc.h>
+#include <linux/kvm_host.h>
+#include <asm/kvm_mmu.h>
#include <asm/pvclock-abi.h>
#include <kvm/arm_hypercalls.h>
@@ -75,3 +77,48 @@ long kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu)
return vcpu->arch.steal.base;
}
+
+int kvm_arm_pvtime_set_attr(struct kvm_vcpu *vcpu,
+ struct kvm_device_attr *attr)
+{
+ u64 __user *user = (u64 __user *)attr->addr;
+ u64 ipa;
+
+ if (attr->attr != KVM_ARM_VCPU_PVTIME_SET_IPA)
+ return -ENXIO;
+
+ if (get_user(ipa, user))
+ return -EFAULT;
+ if (ipa & 63)
+ return -EINVAL;
+ if (vcpu->arch.steal.base != GPA_INVALID)
+ return -EEXIST;
+ vcpu->arch.steal.base = ipa;
+ return 0;
+}
+
+int kvm_arm_pvtime_get_attr(struct kvm_vcpu *vcpu,
+ struct kvm_device_attr *attr)
+{
+ u64 __user *user = (u64 __user *)attr->addr;
+ u64 ipa;
+
+ if (attr->attr != KVM_ARM_VCPU_PVTIME_SET_IPA)
+ return -ENXIO;
+
+ ipa = vcpu->arch.steal.base;
+
+ if (put_user(ipa, user))
+ return -EFAULT;
+ return 0;
+}
+
+int kvm_arm_pvtime_has_attr(struct kvm_vcpu *vcpu,
+ struct kvm_device_attr *attr)
+{
+ switch (attr->attr) {
+ case KVM_ARM_VCPU_PVTIME_SET_IPA:
+ return 0;
+ }
+ return -ENXIO;
+}
--
2.20.1
^ permalink raw reply related
* [PATCH v4 06/10] KVM: Allow kvm_device_ops to be const
From: Steven Price @ 2019-08-30 8:42 UTC (permalink / raw)
To: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: Steven Price, Catalin Marinas, Paolo Bonzini,
Radim Krčmář, Russell King, James Morse,
Julien Thierry, Suzuki K Pouloze, Mark Rutland, kvm, linux-doc,
linux-kernel
In-Reply-To: <20190830084255.55113-1-steven.price@arm.com>
Currently a kvm_device_ops structure cannot be const without triggering
compiler warnings. However the structure doesn't need to be written to
and, by marking it const, it can be read-only in memory. Add some more
const keywords to allow this.
Signed-off-by: Steven Price <steven.price@arm.com>
---
include/linux/kvm_host.h | 4 ++--
virt/kvm/kvm_main.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index bf0ae1825b9c..4e65f7566913 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1260,7 +1260,7 @@ extern unsigned int halt_poll_ns_grow_start;
extern unsigned int halt_poll_ns_shrink;
struct kvm_device {
- struct kvm_device_ops *ops;
+ const struct kvm_device_ops *ops;
struct kvm *kvm;
void *private;
struct list_head vm_node;
@@ -1313,7 +1313,7 @@ struct kvm_device_ops {
void kvm_device_get(struct kvm_device *dev);
void kvm_device_put(struct kvm_device *dev);
struct kvm_device *kvm_device_from_filp(struct file *filp);
-int kvm_register_device_ops(struct kvm_device_ops *ops, u32 type);
+int kvm_register_device_ops(const struct kvm_device_ops *ops, u32 type);
void kvm_unregister_device_ops(u32 type);
extern struct kvm_device_ops kvm_mpic_ops;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index c6a91b044d8d..75488ebb87c9 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3046,14 +3046,14 @@ struct kvm_device *kvm_device_from_filp(struct file *filp)
return filp->private_data;
}
-static struct kvm_device_ops *kvm_device_ops_table[KVM_DEV_TYPE_MAX] = {
+static const struct kvm_device_ops *kvm_device_ops_table[KVM_DEV_TYPE_MAX] = {
#ifdef CONFIG_KVM_MPIC
[KVM_DEV_TYPE_FSL_MPIC_20] = &kvm_mpic_ops,
[KVM_DEV_TYPE_FSL_MPIC_42] = &kvm_mpic_ops,
#endif
};
-int kvm_register_device_ops(struct kvm_device_ops *ops, u32 type)
+int kvm_register_device_ops(const struct kvm_device_ops *ops, u32 type)
{
if (type >= ARRAY_SIZE(kvm_device_ops_table))
return -ENOSPC;
@@ -3074,7 +3074,7 @@ void kvm_unregister_device_ops(u32 type)
static int kvm_ioctl_create_device(struct kvm *kvm,
struct kvm_create_device *cd)
{
- struct kvm_device_ops *ops = NULL;
+ const struct kvm_device_ops *ops = NULL;
struct kvm_device *dev;
bool test = cd->flags & KVM_CREATE_DEVICE_TEST;
int type;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2] vsprintf: introduce %dE for error constants
From: Petr Mladek @ 2019-08-30 9:06 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Rasmus Villemoes, Juergen Gross, Sergey Senozhatsky,
Steven Rostedt, Andrew Morton, Jani Nikula, Jonathan Corbet,
metux IT consult Enrico Weigelt, linux-doc, linux-kernel
In-Reply-To: <002dc2a7-40a3-f52a-c8fa-5dbb42e6dd7b@kleine-koenig.org>
On Thu 2019-08-29 19:39:45, Uwe Kleine-König wrote:
> On 8/29/19 11:09 AM, Rasmus Villemoes wrote:
> > On 29/08/2019 10.27, Juergen Gross wrote:
> >> On 29.08.19 10:12, Petr Mladek wrote:
> >>> On Wed 2019-08-28 21:18:37, Uwe Kleine-König wrote:
> >>>>
> >>>> I'd like to postpone the discussion about "how" until we agreed about
> >>>> the "if at all".
> >>>
> >>> It seems that all people like this feature.
> >>
> >> Hmm, what about already existing format strings conatining "%dE"?
> >>
> >> Yes, I could find only one (drivers/staging/speakup/speakup_bns.c), but
> >> nevertheless...
> >
> > Indeed, Uwe still needs to respond to how he wants to handle that. I
>
> This is indeed bad and I didn't expect that. I just took a quick look
> and this string is indeed used as sprintf format string.
Hmm, it seems that solving this might be pretty tricky.
I see this as a warning that we should not play with fire.
There might be a reason why all format modifiers are put
between % and the format identifier.
> > still prefer making it %pE, both because it's easier to convert integers
> > to ERR_PTRs than having to worry about the type of PTR_ERR() being long
> > and not int, and because alphanumerics after %p have been ignored for a
> > long time (10 years?) whether or not those characters have been
> > recognized as a %p extension, so nobody relies on %pE putting an E after
> > the %p output. It also keeps the non-standard extensions in the same
> > "namespace", so to speak.
>
> > Oh, 'E' is taken, well, make it 'e' then.
>
> I like having %pe to print error valued pointers. Then maybe we could
> have both %de for ints and %pe for pointers. :-)
I would prefer to avoid %pe. It would make sense only when the value
really contains error id. It means that it has to be used as:
if (IS_ERR(p)) {
pr_warn(...);
The error path might handle the error using PTR_ERR() also
on other locations. Also PTR_ERR() will make it clear that we
are trying to print the error code.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v3 01/10] KVM: arm64: Document PV-time interface
From: Christoffer Dall @ 2019-08-30 9:22 UTC (permalink / raw)
To: Steven Price
Cc: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm, linux-kernel,
kvm, Catalin Marinas, linux-doc, Russell King, Paolo Bonzini
In-Reply-To: <37eaf54b-8a22-8483-a372-419bfa1475f1@arm.com>
On Wed, Aug 28, 2019 at 01:09:15PM +0100, Steven Price wrote:
> On 27/08/2019 09:57, Christoffer Dall wrote:
> > On Wed, Aug 21, 2019 at 04:36:47PM +0100, Steven Price wrote:
> >> Introduce a paravirtualization interface for KVM/arm64 based on the
> >> "Arm Paravirtualized Time for Arm-Base Systems" specification DEN 0057A.
> >>
> >> This only adds the details about "Stolen Time" as the details of "Live
> >> Physical Time" have not been fully agreed.
> >>
> >> User space can specify a reserved area of memory for the guest and
> >> inform KVM to populate the memory with information on time that the host
> >> kernel has stolen from the guest.
> >>
> >> A hypercall interface is provided for the guest to interrogate the
> >> hypervisor's support for this interface and the location of the shared
> >> memory structures.
> >>
> >> Signed-off-by: Steven Price <steven.price@arm.com>
> >> ---
> >> Documentation/virt/kvm/arm/pvtime.txt | 100 ++++++++++++++++++++++++++
> >> 1 file changed, 100 insertions(+)
> >> create mode 100644 Documentation/virt/kvm/arm/pvtime.txt
> >>
> >> diff --git a/Documentation/virt/kvm/arm/pvtime.txt b/Documentation/virt/kvm/arm/pvtime.txt
> >> new file mode 100644
> >> index 000000000000..1ceb118694e7
> >> --- /dev/null
> >> +++ b/Documentation/virt/kvm/arm/pvtime.txt
> >> @@ -0,0 +1,100 @@
> >> +Paravirtualized time support for arm64
> >> +======================================
> >> +
> >> +Arm specification DEN0057/A defined a standard for paravirtualised time
> >> +support for AArch64 guests:
> >> +
> >> +https://developer.arm.com/docs/den0057/a
> >> +
> >> +KVM/arm64 implements the stolen time part of this specification by providing
> >> +some hypervisor service calls to support a paravirtualized guest obtaining a
> >> +view of the amount of time stolen from its execution.
> >> +
> >> +Two new SMCCC compatible hypercalls are defined:
> >> +
> >> +PV_FEATURES 0xC5000020
> >> +PV_TIME_ST 0xC5000022
> >> +
> >> +These are only available in the SMC64/HVC64 calling convention as
> >> +paravirtualized time is not available to 32 bit Arm guests. The existence of
> >> +the PV_FEATURES hypercall should be probed using the SMCCC 1.1 ARCH_FEATURES
> >> +mechanism before calling it.
> >> +
> >> +PV_FEATURES
> >> + Function ID: (uint32) : 0xC5000020
> >> + PV_func_id: (uint32) : Either PV_TIME_LPT or PV_TIME_ST
> >> + Return value: (int32) : NOT_SUPPORTED (-1) or SUCCESS (0) if the relevant
> >> + PV-time feature is supported by the hypervisor.
> >> +
> >> +PV_TIME_ST
> >> + Function ID: (uint32) : 0xC5000022
> >> + Return value: (int64) : IPA of the stolen time data structure for this
> >> + (V)CPU. On failure:
> >> + NOT_SUPPORTED (-1)
> >> +
> >> +The IPA returned by PV_TIME_ST should be mapped by the guest as normal memory
> >> +with inner and outer write back caching attributes, in the inner shareable
> >> +domain. A total of 16 bytes from the IPA returned are guaranteed to be
> >> +meaningfully filled by the hypervisor (see structure below).
> >> +
> >> +PV_TIME_ST returns the structure for the calling VCPU.
> >> +
> >> +Stolen Time
> >> +-----------
> >> +
> >> +The structure pointed to by the PV_TIME_ST hypercall is as follows:
> >> +
> >> + Field | Byte Length | Byte Offset | Description
> >> + ----------- | ----------- | ----------- | --------------------------
> >> + Revision | 4 | 0 | Must be 0 for version 0.1
> >> + Attributes | 4 | 4 | Must be 0
> >> + Stolen time | 8 | 8 | Stolen time in unsigned
> >> + | | | nanoseconds indicating how
> >> + | | | much time this VCPU thread
> >> + | | | was involuntarily not
> >> + | | | running on a physical CPU.
> >> +
> >> +The structure will be updated by the hypervisor prior to scheduling a VCPU. It
> >> +will be present within a reserved region of the normal memory given to the
> >> +guest. The guest should not attempt to write into this memory. There is a
> >> +structure per VCPU of the guest.
> >> +
> >> +User space interface
> >> +====================
> >> +
> >> +User space can request that KVM provide the paravirtualized time interface to
> >> +a guest by creating a KVM_DEV_TYPE_ARM_PV_TIME device, for example:
> >> +
> >> + struct kvm_create_device pvtime_device = {
> >> + .type = KVM_DEV_TYPE_ARM_PV_TIME,
> >> + .attr = 0,
> >> + .flags = 0,
> >> + };
> >> +
> >> + pvtime_fd = ioctl(vm_fd, KVM_CREATE_DEVICE, &pvtime_device);
> >> +
> >> +Creation of the device should be done after creating the vCPUs of the virtual
> >> +machine.
> >> +
> >> +The IPA of the structures must be given to KVM. This is the base address
> >> +of an array of stolen time structures (one for each VCPU). The base address
> >> +must be page aligned. The size must be at least 64 * number of VCPUs and be a
> >> +multiple of PAGE_SIZE.
> >> +
> >> +The memory for these structures should be added to the guest in the usual
> >> +manner (e.g. using KVM_SET_USER_MEMORY_REGION).
> >> +
> >> +For example:
> >> +
> >> + struct kvm_dev_arm_st_region region = {
> >> + .gpa = <IPA of guest base address>,
> >> + .size = <size in bytes>
> >> + };
> >
> > This feel fragile; how are you handling userspace creating VCPUs after
> > setting this up,
>
> In this case as long as the structures all fit within the region created
> VCPUs can be created/destroyed at will. If the VCPU index is too high
> then the kernel will bail out in kvm_update_stolen_time() so the
> structure will not be written. I consider this case as user space
> messing up, so beyond protecting the host from the mess, user space gets
> to keep the pieces.
>
> > the GPA overlapping guest memory, etc.
>
> Again, the (host) kernel is protected against this, but clearly this
> will end badly for the guest.
>
> > Is the
> > philosophy here that the VMM can mess up the VM if it wants, but that
> > this should never lead attacks on the host (we better hope not) and so
> > we don't care?
>
> Yes. For things like GPA overlapping guest memory it's not really the
> host's position to work out what is "guest memory". It's quite possible
> that user space could decide to place the stolen time structures right
> in the middle of guest memory - it's just up to user space to inform the
> guest what memory is usable. Obviously the expectation is that the
> shared structures would be positioned "out of the way" in GPA space in
> any normal arrangement.
>
> > It seems to me setting the IPA per vcpu throught the VCPU device would
> > avoid a lot of these issues. See
> > Documentation/virt/kvm/devices/vcpu.txt.
>
> That is certainly a possibility, I'm not really sure what the benefit is
> though? It would still lead to corner cases:
>
> * What if only some VCPUs had stolen time setup on them?
> * What if multiple VCPUs pointed to the same location?
> * The structures can still overlap with guest memory
>
> It's also more work to setup in user space with the only "benefit" being
> that user space could choose to organise the structures however it sees
> fit (e.g. no need for them to be contiguous in memory). But I'm not sure
> I see a use case for that flexibility.
>
> Perhaps there's some benefit I'm not seeing?
>
So this is now mostly moot as you said you were going to change this,
but overall I think the assumption that it's trivial to maintain an
interface such the one proposed here to never be an attack vector for
the host is not as easy as it may sound. In the past, we've had
numerous bugs related to things like calculating an offset into some
region based on some index and a size, because suddenly someone changed
how an index is calculated, and it had unintended side-effects etc.
So if an API is used to specify something which is effectively per-CPU,
it's better to use the per-CPU handles (VCPU fds) to directly describe
this relationship.
Thanks,
Christoffer
^ permalink raw reply
* Re: [PATCH v4 05/10] KVM: arm64: Support stolen time reporting via shared structure
From: Christoffer Dall @ 2019-08-30 9:42 UTC (permalink / raw)
To: Steven Price
Cc: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm, linux-kernel,
kvm, Catalin Marinas, linux-doc, Russell King, Paolo Bonzini
In-Reply-To: <20190830084255.55113-6-steven.price@arm.com>
On Fri, Aug 30, 2019 at 09:42:50AM +0100, Steven Price wrote:
> Implement the service call for configuring a shared structure between a
> VCPU and the hypervisor in which the hypervisor can write the time
> stolen from the VCPU's execution time by other tasks on the host.
>
> The hypervisor allocates memory which is placed at an IPA chosen by user
> space. The hypervisor then updates the shared structure using
> kvm_put_guest() to ensure single copy atomicity of the 64-bit value
> reporting the stolen time in nanoseconds.
>
> Whenever stolen time is enabled by the guest, the stolen time counter is
> reset.
>
> The stolen time itself is retrieved from the sched_info structure
> maintained by the Linux scheduler code. We enable SCHEDSTATS when
> selecting KVM Kconfig to ensure this value is meaningful.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> arch/arm/include/asm/kvm_host.h | 20 +++++++++++
> arch/arm64/include/asm/kvm_host.h | 21 +++++++++++-
> arch/arm64/kvm/Kconfig | 1 +
> include/linux/kvm_types.h | 2 ++
> virt/kvm/arm/arm.c | 11 ++++++
> virt/kvm/arm/hypercalls.c | 3 ++
> virt/kvm/arm/pvtime.c | 56 +++++++++++++++++++++++++++++++
> 7 files changed, 113 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 5a0c3569ebde..5c401482d62d 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -39,6 +39,7 @@
> KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
> #define KVM_REQ_IRQ_PENDING KVM_ARCH_REQ(1)
> #define KVM_REQ_VCPU_RESET KVM_ARCH_REQ(2)
> +#define KVM_REQ_RECORD_STEAL KVM_ARCH_REQ(3)
>
> DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
>
> @@ -329,6 +330,25 @@ static inline long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
> return SMCCC_RET_NOT_SUPPORTED;
> }
>
> +static inline long kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu)
> +{
> + return SMCCC_RET_NOT_SUPPORTED;
> +}
> +
> +static inline int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline void kvm_arm_pvtime_vcpu_init(struct kvm_vcpu_arch *vcpu_arch)
> +{
> +}
> +
> +static inline bool kvm_arm_is_pvtime_enabled(struct kvm_vcpu_arch *vcpu_arch)
> +{
> + return false;
> +}
> +
> void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>
> struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 93b46d9526d0..1697e63f6dd8 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -44,6 +44,7 @@
> KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
> #define KVM_REQ_IRQ_PENDING KVM_ARCH_REQ(1)
> #define KVM_REQ_VCPU_RESET KVM_ARCH_REQ(2)
> +#define KVM_REQ_RECORD_STEAL KVM_ARCH_REQ(3)
>
> DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
>
> @@ -338,8 +339,14 @@ struct kvm_vcpu_arch {
> /* True when deferrable sysregs are loaded on the physical CPU,
> * see kvm_vcpu_load_sysregs and kvm_vcpu_put_sysregs. */
> bool sysregs_loaded_on_cpu;
> -};
>
> + /* Guest PV state */
> + struct {
> + u64 steal;
> + u64 last_steal;
> + gpa_t base;
> + } steal;
> +};
> /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
> #define vcpu_sve_pffr(vcpu) ((void *)((char *)((vcpu)->arch.sve_state) + \
> sve_ffr_offset((vcpu)->arch.sve_max_vl)))
> @@ -479,6 +486,18 @@ int kvm_perf_init(void);
> int kvm_perf_teardown(void);
>
> long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu);
> +long kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu);
> +int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init);
> +
> +static inline void kvm_arm_pvtime_vcpu_init(struct kvm_vcpu_arch *vcpu_arch)
> +{
> + vcpu_arch->steal.base = GPA_INVALID;
> +}
> +
> +static inline bool kvm_arm_is_pvtime_enabled(struct kvm_vcpu_arch *vcpu_arch)
> +{
> + return (vcpu_arch->steal.base != GPA_INVALID);
> +}
>
> void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
>
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index a67121d419a2..d8b88e40d223 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -39,6 +39,7 @@ config KVM
> select IRQ_BYPASS_MANAGER
> select HAVE_KVM_IRQ_BYPASS
> select HAVE_KVM_VCPU_RUN_PID_CHANGE
> + select SCHEDSTATS
> ---help---
> Support hosting virtualized guest machines.
> We don't support KVM with 16K page tables yet, due to the multiple
> diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
> index bde5374ae021..1c88e69db3d9 100644
> --- a/include/linux/kvm_types.h
> +++ b/include/linux/kvm_types.h
> @@ -35,6 +35,8 @@ typedef unsigned long gva_t;
> typedef u64 gpa_t;
> typedef u64 gfn_t;
>
> +#define GPA_INVALID (~(gpa_t)0)
> +
> typedef unsigned long hva_t;
> typedef u64 hpa_t;
> typedef u64 hfn_t;
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 35a069815baf..eaceb2d0f0c0 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -40,6 +40,10 @@
> #include <asm/kvm_coproc.h>
> #include <asm/sections.h>
>
> +#include <kvm/arm_hypercalls.h>
> +#include <kvm/arm_pmu.h>
> +#include <kvm/arm_psci.h>
> +
> #ifdef REQUIRES_VIRT
> __asm__(".arch_extension virt");
> #endif
> @@ -350,6 +354,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
>
> kvm_arm_reset_debug_ptr(vcpu);
>
> + kvm_arm_pvtime_vcpu_init(&vcpu->arch);
> +
> return kvm_vgic_vcpu_init(vcpu);
> }
>
> @@ -379,6 +385,8 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> kvm_vcpu_load_sysregs(vcpu);
> kvm_arch_vcpu_load_fp(vcpu);
> kvm_vcpu_pmu_restore_guest(vcpu);
> + if (kvm_arm_is_pvtime_enabled(&vcpu->arch))
> + kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu);
>
> if (single_task_running())
> vcpu_clear_wfe_traps(vcpu);
> @@ -644,6 +652,9 @@ static void check_vcpu_requests(struct kvm_vcpu *vcpu)
> * that a VCPU sees new virtual interrupts.
> */
> kvm_check_request(KVM_REQ_IRQ_PENDING, vcpu);
> +
> + if (kvm_check_request(KVM_REQ_RECORD_STEAL, vcpu))
> + kvm_update_stolen_time(vcpu, false);
> }
> }
>
> diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
> index e2521e0d3978..3091a5d2e842 100644
> --- a/virt/kvm/arm/hypercalls.c
> +++ b/virt/kvm/arm/hypercalls.c
> @@ -56,6 +56,9 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> case ARM_SMCCC_HV_PV_FEATURES:
> val = kvm_hypercall_pv_features(vcpu);
> break;
> + case ARM_SMCCC_HV_PV_TIME_ST:
> + val = kvm_hypercall_stolen_time(vcpu);
> + break;
> default:
> return kvm_psci_call(vcpu);
> }
> diff --git a/virt/kvm/arm/pvtime.c b/virt/kvm/arm/pvtime.c
> index 7887a61651c6..d9d0dbc6994b 100644
> --- a/virt/kvm/arm/pvtime.c
> +++ b/virt/kvm/arm/pvtime.c
> @@ -3,8 +3,45 @@
>
> #include <linux/arm-smccc.h>
>
> +#include <asm/pvclock-abi.h>
> +
> #include <kvm/arm_hypercalls.h>
>
> +int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init)
> +{
> + struct kvm *kvm = vcpu->kvm;
> + u64 steal;
> + u64 steal_le;
> + u64 offset;
> + int idx;
> + u64 base = vcpu->arch.steal.base;
> +
> + if (base == GPA_INVALID)
> + return -ENOTSUPP;
> +
> + /* Let's do the local bookkeeping */
> + steal = vcpu->arch.steal.steal;
> + steal += current->sched_info.run_delay - vcpu->arch.steal.last_steal;
> + vcpu->arch.steal.last_steal = current->sched_info.run_delay;
> + vcpu->arch.steal.steal = steal;
> +
> + steal_le = cpu_to_le64(steal);
> + idx = srcu_read_lock(&kvm->srcu);
> + if (init) {
> + struct pvclock_vcpu_stolen_time init_values = {
> + .revision = 0,
> + .attributes = 0
> + };
> + kvm_write_guest(kvm, base, &init_values,
> + sizeof(init_values));
> + }
> + offset = offsetof(struct pvclock_vcpu_stolen_time, stolen_time);
> + kvm_put_guest(kvm, base + offset, steal_le, u64);
Let's hope we don't have thousands of memslots through which we have to
do a linear scan on every vcpu load after this. If that were the case,
I think the memslot search path would have to be updated anyhow.
Otherwise looks reasonable to me.
Thanks,
Christoffer
> + srcu_read_unlock(&kvm->srcu, idx);
> +
> + return 0;
> +}
> +
> long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
> {
> u32 feature = smccc_get_arg1(vcpu);
> @@ -12,6 +49,7 @@ long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
>
> switch (feature) {
> case ARM_SMCCC_HV_PV_FEATURES:
> + case ARM_SMCCC_HV_PV_TIME_ST:
> val = SMCCC_RET_SUCCESS;
> break;
> }
> @@ -19,3 +57,21 @@ long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
> return val;
> }
>
> +long kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu)
> +{
> + int err;
> +
> + /*
> + * Start counting stolen time from the time the guest requests
> + * the feature enabled.
> + */
> + vcpu->arch.steal.steal = 0;
> + vcpu->arch.steal.last_steal = current->sched_info.run_delay;
> +
> + err = kvm_update_stolen_time(vcpu, true);
> +
> + if (err)
> + return SMCCC_RET_NOT_SUPPORTED;
> +
> + return vcpu->arch.steal.base;
> +}
> --
> 2.20.1
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply
* Re: [PATCH v4 05/10] KVM: arm64: Support stolen time reporting via shared structure
From: Steven Price @ 2019-08-30 9:52 UTC (permalink / raw)
To: Christoffer Dall
Cc: kvm, linux-doc, Marc Zyngier, linux-kernel, Russell King,
Catalin Marinas, Paolo Bonzini, Will Deacon, kvmarm,
linux-arm-kernel
In-Reply-To: <20190830094245.GB5307@e113682-lin.lund.arm.com>
On 30/08/2019 10:42, Christoffer Dall wrote:
> On Fri, Aug 30, 2019 at 09:42:50AM +0100, Steven Price wrote:
>> Implement the service call for configuring a shared structure between a
>> VCPU and the hypervisor in which the hypervisor can write the time
>> stolen from the VCPU's execution time by other tasks on the host.
>>
>> The hypervisor allocates memory which is placed at an IPA chosen by user
>> space. The hypervisor then updates the shared structure using
>> kvm_put_guest() to ensure single copy atomicity of the 64-bit value
>> reporting the stolen time in nanoseconds.
>>
>> Whenever stolen time is enabled by the guest, the stolen time counter is
>> reset.
>>
>> The stolen time itself is retrieved from the sched_info structure
>> maintained by the Linux scheduler code. We enable SCHEDSTATS when
>> selecting KVM Kconfig to ensure this value is meaningful.
>>
>> Signed-off-by: Steven Price <steven.price@arm.com>
[...]
>> +int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init)
>> +{
>> + struct kvm *kvm = vcpu->kvm;
>> + u64 steal;
>> + u64 steal_le;
>> + u64 offset;
>> + int idx;
>> + u64 base = vcpu->arch.steal.base;
>> +
>> + if (base == GPA_INVALID)
>> + return -ENOTSUPP;
>> +
>> + /* Let's do the local bookkeeping */
>> + steal = vcpu->arch.steal.steal;
>> + steal += current->sched_info.run_delay - vcpu->arch.steal.last_steal;
>> + vcpu->arch.steal.last_steal = current->sched_info.run_delay;
>> + vcpu->arch.steal.steal = steal;
>> +
>> + steal_le = cpu_to_le64(steal);
>> + idx = srcu_read_lock(&kvm->srcu);
>> + if (init) {
>> + struct pvclock_vcpu_stolen_time init_values = {
>> + .revision = 0,
>> + .attributes = 0
>> + };
>> + kvm_write_guest(kvm, base, &init_values,
>> + sizeof(init_values));
>> + }
>> + offset = offsetof(struct pvclock_vcpu_stolen_time, stolen_time);
>> + kvm_put_guest(kvm, base + offset, steal_le, u64);
>
> Let's hope we don't have thousands of memslots through which we have to
> do a linear scan on every vcpu load after this. If that were the case,
> I think the memslot search path would have to be updated anyhow.
Yes I'm not sure with the current memslot implementation it is actually
beneficial to split up the stolen time structures into separate
memslots. But there's nothing requiring the use of so many memslots.
If this is really a problem it would be possible to implement a
memslot-caching kvm_put_guest(), but I'd want to wait until someone
shows there's actually a problem first.
> Otherwise looks reasonable to me.
Great, thanks for the review.
Steve
^ permalink raw reply
* Re: [PATCH v4 07/10] KVM: arm64: Provide VCPU attributes for stolen time
From: Marc Zyngier @ 2019-08-30 10:02 UTC (permalink / raw)
To: Steven Price, Will Deacon, linux-arm-kernel, kvmarm
Cc: Catalin Marinas, Paolo Bonzini, Radim Krčmář,
Russell King, James Morse, Julien Thierry, Suzuki K Pouloze,
Mark Rutland, kvm, linux-doc, linux-kernel
In-Reply-To: <20190830084255.55113-8-steven.price@arm.com>
On 30/08/2019 09:42, Steven Price wrote:
> Allow user space to inform the KVM host where in the physical memory
> map the paravirtualized time structures should be located.
>
> User space can set an attribute on the VCPU providing the IPA base
> address of the stolen time structure for that VCPU. This must be
> repeated for every VCPU in the VM.
>
> The address is given in terms of the physical address visible to
> the guest and must be 64 byte aligned. The guest will discover the
> address via a hypercall.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> arch/arm64/include/asm/kvm_host.h | 7 +++++
> arch/arm64/include/uapi/asm/kvm.h | 2 ++
> arch/arm64/kvm/guest.c | 9 ++++++
> include/uapi/linux/kvm.h | 2 ++
> virt/kvm/arm/pvtime.c | 47 +++++++++++++++++++++++++++++++
> 5 files changed, 67 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 1697e63f6dd8..6af16b29a41f 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -489,6 +489,13 @@ long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu);
> long kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu);
> int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init);
>
> +int kvm_arm_pvtime_set_attr(struct kvm_vcpu *vcpu,
> + struct kvm_device_attr *attr);
> +int kvm_arm_pvtime_get_attr(struct kvm_vcpu *vcpu,
> + struct kvm_device_attr *attr);
> +int kvm_arm_pvtime_has_attr(struct kvm_vcpu *vcpu,
> + struct kvm_device_attr *attr);
> +
> static inline void kvm_arm_pvtime_vcpu_init(struct kvm_vcpu_arch *vcpu_arch)
> {
> vcpu_arch->steal.base = GPA_INVALID;
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 9a507716ae2f..bde9f165ad3a 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -323,6 +323,8 @@ struct kvm_vcpu_events {
> #define KVM_ARM_VCPU_TIMER_CTRL 1
> #define KVM_ARM_VCPU_TIMER_IRQ_VTIMER 0
> #define KVM_ARM_VCPU_TIMER_IRQ_PTIMER 1
> +#define KVM_ARM_VCPU_PVTIME_CTRL 2
> +#define KVM_ARM_VCPU_PVTIME_SET_IPA 0
>
> /* KVM_IRQ_LINE irq field index values */
> #define KVM_ARM_IRQ_TYPE_SHIFT 24
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index dfd626447482..d3ac9d2fd405 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -858,6 +858,9 @@ int kvm_arm_vcpu_arch_set_attr(struct kvm_vcpu *vcpu,
> case KVM_ARM_VCPU_TIMER_CTRL:
> ret = kvm_arm_timer_set_attr(vcpu, attr);
> break;
> + case KVM_ARM_VCPU_PVTIME_CTRL:
> + ret = kvm_arm_pvtime_set_attr(vcpu, attr);
> + break;
> default:
> ret = -ENXIO;
> break;
> @@ -878,6 +881,9 @@ int kvm_arm_vcpu_arch_get_attr(struct kvm_vcpu *vcpu,
> case KVM_ARM_VCPU_TIMER_CTRL:
> ret = kvm_arm_timer_get_attr(vcpu, attr);
> break;
> + case KVM_ARM_VCPU_PVTIME_CTRL:
> + ret = kvm_arm_pvtime_get_attr(vcpu, attr);
> + break;
> default:
> ret = -ENXIO;
> break;
> @@ -898,6 +904,9 @@ int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu,
> case KVM_ARM_VCPU_TIMER_CTRL:
> ret = kvm_arm_timer_has_attr(vcpu, attr);
> break;
> + case KVM_ARM_VCPU_PVTIME_CTRL:
> + ret = kvm_arm_pvtime_has_attr(vcpu, attr);
> + break;
> default:
> ret = -ENXIO;
> break;
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 5e3f12d5359e..265156a984f2 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1222,6 +1222,8 @@ enum kvm_device_type {
> #define KVM_DEV_TYPE_ARM_VGIC_ITS KVM_DEV_TYPE_ARM_VGIC_ITS
> KVM_DEV_TYPE_XIVE,
> #define KVM_DEV_TYPE_XIVE KVM_DEV_TYPE_XIVE
> + KVM_DEV_TYPE_ARM_PV_TIME,
> +#define KVM_DEV_TYPE_ARM_PV_TIME KVM_DEV_TYPE_ARM_PV_TIME
> KVM_DEV_TYPE_MAX,
> };
>
> diff --git a/virt/kvm/arm/pvtime.c b/virt/kvm/arm/pvtime.c
> index d9d0dbc6994b..7b1834b98a68 100644
> --- a/virt/kvm/arm/pvtime.c
> +++ b/virt/kvm/arm/pvtime.c
> @@ -2,7 +2,9 @@
> // Copyright (C) 2019 Arm Ltd.
>
> #include <linux/arm-smccc.h>
> +#include <linux/kvm_host.h>
>
> +#include <asm/kvm_mmu.h>
> #include <asm/pvclock-abi.h>
>
> #include <kvm/arm_hypercalls.h>
> @@ -75,3 +77,48 @@ long kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu)
>
> return vcpu->arch.steal.base;
> }
> +
> +int kvm_arm_pvtime_set_attr(struct kvm_vcpu *vcpu,
> + struct kvm_device_attr *attr)
> +{
> + u64 __user *user = (u64 __user *)attr->addr;
> + u64 ipa;
> +
> + if (attr->attr != KVM_ARM_VCPU_PVTIME_SET_IPA)
> + return -ENXIO;
> +
> + if (get_user(ipa, user))
> + return -EFAULT;
> + if (ipa & 63)
nit: Please express this as !IS_ALIGNED(ipa, 64) instead.
> + return -EINVAL;
> + if (vcpu->arch.steal.base != GPA_INVALID)
> + return -EEXIST;
> + vcpu->arch.steal.base = ipa;
I'm still worried that you end-up not knowing whether the IPA is valid
or not at this stage, nor that we check about overlapping vcpus. How do
we validate that?
I also share Christoffer's concern that the memslot parsing may be
expensive on a system with multiple memslots. But maybe that can be
solved by adding some caching capabilities to your kvm_put_guest(),
should this become a problem.
> + return 0;
> +}
> +
> +int kvm_arm_pvtime_get_attr(struct kvm_vcpu *vcpu,
> + struct kvm_device_attr *attr)
> +{
> + u64 __user *user = (u64 __user *)attr->addr;
> + u64 ipa;
> +
> + if (attr->attr != KVM_ARM_VCPU_PVTIME_SET_IPA)
It is a bit odd that this is using "SET_IPA" as a way to GET it.
> + return -ENXIO;
> +
> + ipa = vcpu->arch.steal.base;
> +
> + if (put_user(ipa, user))
> + return -EFAULT;
> + return 0;
> +}
> +
> +int kvm_arm_pvtime_has_attr(struct kvm_vcpu *vcpu,
> + struct kvm_device_attr *attr)
> +{
> + switch (attr->attr) {
> + case KVM_ARM_VCPU_PVTIME_SET_IPA:
> + return 0;
> + }
> + return -ENXIO;
> +}
>
Thanks,
M.
--
Jazz is not dead, it just smells funny...
^ permalink raw reply
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