Linux Documentation
 help / color / mirror / Atom feed
* 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

* Re: [PATCH] tools: memory-model: add it to the Documentation body
From: Peter Zijlstra @ 2019-08-30 11:32 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: Akira Yokosawa, Alan Stern, Mauro Carvalho Chehab, Joel Fernandes,
	Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
	Jonathan Corbet, Andrea Parri, Will Deacon, Boqun Feng,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Daniel Lustig, Ingo Molnar, Jason Gunthorpe, SeongJae Park,
	linux-arch
In-Reply-To: <20190731202517.GF5913@linux.ibm.com>

On Wed, Jul 31, 2019 at 01:25:17PM -0700, Paul E. McKenney wrote:
> Looks like a pretty clear consensus thus far.  Any objections to keeping
> these .txt for the time being?

Obviously I'm a huge proponent of abandoning RST and an advocate for
normal .txt files ;-)

^ permalink raw reply

* [PATCH v5 0/2] Add CCPI2 PMU support
From: ganapat @ 2019-08-30 12:54 UTC (permalink / raw)
  To: linux-doc, linux-kernel, linux-arm-kernel
  Cc: will, mark.rutland, corbet, jnair, rrichter, jglauber, gkulkarni

From: Ganapatrao Kulkarni <gkulkarni@marvell.com>

Add Cavium Coherent Processor Interconnect (CCPI2) PMU
support in ThunderX2 Uncore driver.

v5:
	Fixed minor bug of v4 (timer callback fuction
	was getting initialized to NULL for all PMUs).

v4:
	Update with review comments [2].
	Changed Counter read to 2 word read since single dword read is misbhehaving(hw issue).

[2] https://lkml.org/lkml/2019/7/23/231

v3: Rebased to 5.3-rc1

v2: Updated with review comments [1]

[1] https://lkml.org/lkml/2019/6/14/965

v1: initial patch


Ganapatrao Kulkarni (2):
  Documentation: perf: Update documentation for ThunderX2 PMU uncore
    driver
  drivers/perf: Add CCPI2 PMU support in ThunderX2 UNCORE driver.

 .../admin-guide/perf/thunderx2-pmu.rst        |  20 +-
 drivers/perf/thunderx2_pmu.c                  | 267 +++++++++++++++---
 2 files changed, 245 insertions(+), 42 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH v5 1/2] Documentation: perf: Update documentation for ThunderX2 PMU uncore driver
From: ganapat @ 2019-08-30 12:54 UTC (permalink / raw)
  To: linux-doc, linux-kernel, linux-arm-kernel
  Cc: will, mark.rutland, corbet, jnair, rrichter, jglauber, gkulkarni
In-Reply-To: <20190830125436.16959-1-ganapat@localhost.localdomain>

From: Ganapatrao Kulkarni <ganapatrao.kulkarni@marvell.com>

Add documentation for Cavium Coherent Processor Interconnect (CCPI2) PMU.

Signed-off-by: Ganapatrao Kulkarni <gkulkarni@marvell.com>
---
 .../admin-guide/perf/thunderx2-pmu.rst        | 20 ++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/Documentation/admin-guide/perf/thunderx2-pmu.rst b/Documentation/admin-guide/perf/thunderx2-pmu.rst
index 08e33675853a..01f158238ae1 100644
--- a/Documentation/admin-guide/perf/thunderx2-pmu.rst
+++ b/Documentation/admin-guide/perf/thunderx2-pmu.rst
@@ -3,24 +3,26 @@ Cavium ThunderX2 SoC Performance Monitoring Unit (PMU UNCORE)
 =============================================================
 
 The ThunderX2 SoC PMU consists of independent, system-wide, per-socket
-PMUs such as the Level 3 Cache (L3C) and DDR4 Memory Controller (DMC).
+PMUs such as the Level 3 Cache (L3C), DDR4 Memory Controller (DMC) and
+Cavium Coherent Processor Interconnect (CCPI2).
 
 The DMC has 8 interleaved channels and the L3C has 16 interleaved tiles.
 Events are counted for the default channel (i.e. channel 0) and prorated
 to the total number of channels/tiles.
 
-The DMC and L3C support up to 4 counters. Counters are independently
-programmable and can be started and stopped individually. Each counter
-can be set to a different event. Counters are 32-bit and do not support
-an overflow interrupt; they are read every 2 seconds.
+The DMC and L3C support up to 4 counters, while the CCPI2 supports up to 8
+counters. Counters are independently programmable to different events and
+can be started and stopped individually. None of the counters support an
+overflow interrupt. DMC and L3C counters are 32-bit and read every 2 seconds.
+The CCPI2 counters are 64-bit and assumed not to overflow in normal operation.
 
 PMU UNCORE (perf) driver:
 
 The thunderx2_pmu driver registers per-socket perf PMUs for the DMC and
-L3C devices.  Each PMU can be used to count up to 4 events
-simultaneously. The PMUs provide a description of their available events
-and configuration options under sysfs, see
-/sys/devices/uncore_<l3c_S/dmc_S/>; S is the socket id.
+L3C devices.  Each PMU can be used to count up to 4 (DMC/L3C) or up to 8
+(CCPI2) events simultaneously. The PMUs provide a description of their
+available events and configuration options under sysfs, see
+/sys/devices/uncore_<l3c_S/dmc_S/ccpi2_S/>; S is the socket id.
 
 The driver does not support sampling, therefore "perf record" will not
 work. Per-task perf sessions are also not supported.
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 2/2] drivers/perf: Add CCPI2 PMU support in ThunderX2 UNCORE driver.
From: ganapat @ 2019-08-30 12:54 UTC (permalink / raw)
  To: linux-doc, linux-kernel, linux-arm-kernel
  Cc: will, mark.rutland, corbet, jnair, rrichter, jglauber, gkulkarni
In-Reply-To: <20190830125436.16959-1-ganapat@localhost.localdomain>

From: Ganapatrao Kulkarni <gkulkarni@marvell.com>

CCPI2 is a low-latency high-bandwidth serial interface for inter socket
connectivity of ThunderX2 processors.

CCPI2 PMU supports up to 8 counters per socket. Counters are
independently programmable to different events and can be started and
stopped individually. The CCPI2 counters are 64-bit and do not overflow
in normal operation.

Signed-off-by: Ganapatrao Kulkarni <gkulkarni@marvell.com>
---
 drivers/perf/thunderx2_pmu.c | 267 ++++++++++++++++++++++++++++++-----
 1 file changed, 234 insertions(+), 33 deletions(-)

diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
index 43d76c85da56..67c6a7140130 100644
--- a/drivers/perf/thunderx2_pmu.c
+++ b/drivers/perf/thunderx2_pmu.c
@@ -16,23 +16,36 @@
  * they need to be sampled before overflow(i.e, at every 2 seconds).
  */
 
-#define TX2_PMU_MAX_COUNTERS		4
+#define TX2_PMU_DMC_L3C_MAX_COUNTERS	4
+#define TX2_PMU_CCPI2_MAX_COUNTERS	8
+#define TX2_PMU_MAX_COUNTERS		TX2_PMU_CCPI2_MAX_COUNTERS
+
+
 #define TX2_PMU_DMC_CHANNELS		8
 #define TX2_PMU_L3_TILES		16
 
 #define TX2_PMU_HRTIMER_INTERVAL	(2 * NSEC_PER_SEC)
-#define GET_EVENTID(ev)			((ev->hw.config) & 0x1f)
-#define GET_COUNTERID(ev)		((ev->hw.idx) & 0x3)
+#define GET_EVENTID(ev, mask)		((ev->hw.config) & mask)
+#define GET_COUNTERID(ev, mask)		((ev->hw.idx) & mask)
  /* 1 byte per counter(4 counters).
   * Event id is encoded in bits [5:1] of a byte,
   */
 #define DMC_EVENT_CFG(idx, val)		((val) << (((idx) * 8) + 1))
 
+/* bits[3:0] to select counters, are indexed from 8 to 15. */
+#define CCPI2_COUNTER_OFFSET		8
+
 #define L3C_COUNTER_CTL			0xA8
 #define L3C_COUNTER_DATA		0xAC
 #define DMC_COUNTER_CTL			0x234
 #define DMC_COUNTER_DATA		0x240
 
+#define CCPI2_PERF_CTL			0x108
+#define CCPI2_COUNTER_CTL		0x10C
+#define CCPI2_COUNTER_SEL		0x12c
+#define CCPI2_COUNTER_DATA_L		0x130
+#define CCPI2_COUNTER_DATA_H		0x134
+
 /* L3C event IDs */
 #define L3_EVENT_READ_REQ		0xD
 #define L3_EVENT_WRITEBACK_REQ		0xE
@@ -51,15 +64,28 @@
 #define DMC_EVENT_READ_TXNS		0xF
 #define DMC_EVENT_MAX			0x10
 
+#define CCPI2_EVENT_REQ_PKT_SENT	0x3D
+#define CCPI2_EVENT_SNOOP_PKT_SENT	0x65
+#define CCPI2_EVENT_DATA_PKT_SENT	0x105
+#define CCPI2_EVENT_GIC_PKT_SENT	0x12D
+#define CCPI2_EVENT_MAX			0x200
+
+#define CCPI2_PERF_CTL_ENABLE		BIT(0)
+#define CCPI2_PERF_CTL_START		BIT(1)
+#define CCPI2_PERF_CTL_RESET		BIT(4)
+#define CCPI2_EVENT_LEVEL_RISING_EDGE	BIT(10)
+#define CCPI2_EVENT_TYPE_EDGE_SENSITIVE	BIT(11)
+
 enum tx2_uncore_type {
 	PMU_TYPE_L3C,
 	PMU_TYPE_DMC,
+	PMU_TYPE_CCPI2,
 	PMU_TYPE_INVALID,
 };
 
 /*
- * pmu on each socket has 2 uncore devices(dmc and l3c),
- * each device has 4 counters.
+ * Each socket has 3 uncore devices associated with a PMU. The DMC and
+ * L3C have 4 32-bit counters and the CCPI2 has 8 64-bit counters.
  */
 struct tx2_uncore_pmu {
 	struct hlist_node hpnode;
@@ -69,8 +95,10 @@ struct tx2_uncore_pmu {
 	int node;
 	int cpu;
 	u32 max_counters;
+	u32 counters_mask;
 	u32 prorate_factor;
 	u32 max_events;
+	u32 events_mask;
 	u64 hrtimer_interval;
 	void __iomem *base;
 	DECLARE_BITMAP(active_counters, TX2_PMU_MAX_COUNTERS);
@@ -79,6 +107,7 @@ struct tx2_uncore_pmu {
 	struct hrtimer hrtimer;
 	const struct attribute_group **attr_groups;
 	enum tx2_uncore_type type;
+	enum hrtimer_restart (*hrtimer_callback)(struct hrtimer *cb);
 	void (*init_cntr_base)(struct perf_event *event,
 			struct tx2_uncore_pmu *tx2_pmu);
 	void (*stop_event)(struct perf_event *event);
@@ -92,7 +121,21 @@ static inline struct tx2_uncore_pmu *pmu_to_tx2_pmu(struct pmu *pmu)
 	return container_of(pmu, struct tx2_uncore_pmu, pmu);
 }
 
-PMU_FORMAT_ATTR(event,	"config:0-4");
+#define TX2_PMU_FORMAT_ATTR(_var, _name, _format)			\
+static ssize_t								\
+__tx2_pmu_##_var##_show(struct device *dev,				\
+			       struct device_attribute *attr,		\
+			       char *page)				\
+{									\
+	BUILD_BUG_ON(sizeof(_format) >= PAGE_SIZE);			\
+	return sprintf(page, _format "\n");				\
+}									\
+									\
+static struct device_attribute format_attr_##_var =			\
+	__ATTR(_name, 0444, __tx2_pmu_##_var##_show, NULL)
+
+TX2_PMU_FORMAT_ATTR(event, event, "config:0-4");
+TX2_PMU_FORMAT_ATTR(event_ccpi2, event, "config:0-9");
 
 static struct attribute *l3c_pmu_format_attrs[] = {
 	&format_attr_event.attr,
@@ -104,6 +147,11 @@ static struct attribute *dmc_pmu_format_attrs[] = {
 	NULL,
 };
 
+static struct attribute *ccpi2_pmu_format_attrs[] = {
+	&format_attr_event_ccpi2.attr,
+	NULL,
+};
+
 static const struct attribute_group l3c_pmu_format_attr_group = {
 	.name = "format",
 	.attrs = l3c_pmu_format_attrs,
@@ -114,6 +162,11 @@ static const struct attribute_group dmc_pmu_format_attr_group = {
 	.attrs = dmc_pmu_format_attrs,
 };
 
+static const struct attribute_group ccpi2_pmu_format_attr_group = {
+	.name = "format",
+	.attrs = ccpi2_pmu_format_attrs,
+};
+
 /*
  * sysfs event attributes
  */
@@ -164,6 +217,19 @@ static struct attribute *dmc_pmu_events_attrs[] = {
 	NULL,
 };
 
+TX2_EVENT_ATTR(req_pktsent, CCPI2_EVENT_REQ_PKT_SENT);
+TX2_EVENT_ATTR(snoop_pktsent, CCPI2_EVENT_SNOOP_PKT_SENT);
+TX2_EVENT_ATTR(data_pktsent, CCPI2_EVENT_DATA_PKT_SENT);
+TX2_EVENT_ATTR(gic_pktsent, CCPI2_EVENT_GIC_PKT_SENT);
+
+static struct attribute *ccpi2_pmu_events_attrs[] = {
+	&tx2_pmu_event_attr_req_pktsent.attr.attr,
+	&tx2_pmu_event_attr_snoop_pktsent.attr.attr,
+	&tx2_pmu_event_attr_data_pktsent.attr.attr,
+	&tx2_pmu_event_attr_gic_pktsent.attr.attr,
+	NULL,
+};
+
 static const struct attribute_group l3c_pmu_events_attr_group = {
 	.name = "events",
 	.attrs = l3c_pmu_events_attrs,
@@ -174,6 +240,11 @@ static const struct attribute_group dmc_pmu_events_attr_group = {
 	.attrs = dmc_pmu_events_attrs,
 };
 
+static const struct attribute_group ccpi2_pmu_events_attr_group = {
+	.name = "events",
+	.attrs = ccpi2_pmu_events_attrs,
+};
+
 /*
  * sysfs cpumask attributes
  */
@@ -213,6 +284,13 @@ static const struct attribute_group *dmc_pmu_attr_groups[] = {
 	NULL
 };
 
+static const struct attribute_group *ccpi2_pmu_attr_groups[] = {
+	&ccpi2_pmu_format_attr_group,
+	&pmu_cpumask_attr_group,
+	&ccpi2_pmu_events_attr_group,
+	NULL
+};
+
 static inline u32 reg_readl(unsigned long addr)
 {
 	return readl((void __iomem *)addr);
@@ -245,33 +323,58 @@ static void init_cntr_base_l3c(struct perf_event *event,
 		struct tx2_uncore_pmu *tx2_pmu)
 {
 	struct hw_perf_event *hwc = &event->hw;
+	u32 cmask;
+
+	tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+	cmask = tx2_pmu->counters_mask;
 
 	/* counter ctrl/data reg offset at 8 */
 	hwc->config_base = (unsigned long)tx2_pmu->base
-		+ L3C_COUNTER_CTL + (8 * GET_COUNTERID(event));
+		+ L3C_COUNTER_CTL + (8 * GET_COUNTERID(event, cmask));
 	hwc->event_base =  (unsigned long)tx2_pmu->base
-		+ L3C_COUNTER_DATA + (8 * GET_COUNTERID(event));
+		+ L3C_COUNTER_DATA + (8 * GET_COUNTERID(event, cmask));
 }
 
 static void init_cntr_base_dmc(struct perf_event *event,
 		struct tx2_uncore_pmu *tx2_pmu)
 {
 	struct hw_perf_event *hwc = &event->hw;
+	u32 cmask;
+
+	tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+	cmask = tx2_pmu->counters_mask;
 
 	hwc->config_base = (unsigned long)tx2_pmu->base
 		+ DMC_COUNTER_CTL;
 	/* counter data reg offset at 0xc */
 	hwc->event_base = (unsigned long)tx2_pmu->base
-		+ DMC_COUNTER_DATA + (0xc * GET_COUNTERID(event));
+		+ DMC_COUNTER_DATA + (0xc * GET_COUNTERID(event, cmask));
+}
+
+static void init_cntr_base_ccpi2(struct perf_event *event,
+		struct tx2_uncore_pmu *tx2_pmu)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	u32 cmask;
+
+	cmask = tx2_pmu->counters_mask;
+
+	hwc->config_base = (unsigned long)tx2_pmu->base
+		+ CCPI2_COUNTER_CTL + (4 * GET_COUNTERID(event, cmask));
+	hwc->event_base =  (unsigned long)tx2_pmu->base;
 }
 
 static void uncore_start_event_l3c(struct perf_event *event, int flags)
 {
-	u32 val;
+	u32 val, emask;
 	struct hw_perf_event *hwc = &event->hw;
+	struct tx2_uncore_pmu *tx2_pmu;
+
+	tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+	emask = tx2_pmu->events_mask;
 
 	/* event id encoded in bits [07:03] */
-	val = GET_EVENTID(event) << 3;
+	val = GET_EVENTID(event, emask) << 3;
 	reg_writel(val, hwc->config_base);
 	local64_set(&hwc->prev_count, 0);
 	reg_writel(0, hwc->event_base);
@@ -284,10 +387,17 @@ static inline void uncore_stop_event_l3c(struct perf_event *event)
 
 static void uncore_start_event_dmc(struct perf_event *event, int flags)
 {
-	u32 val;
+	u32 val, cmask, emask;
 	struct hw_perf_event *hwc = &event->hw;
-	int idx = GET_COUNTERID(event);
-	int event_id = GET_EVENTID(event);
+	struct tx2_uncore_pmu *tx2_pmu;
+	int idx, event_id;
+
+	tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+	cmask = tx2_pmu->counters_mask;
+	emask = tx2_pmu->events_mask;
+
+	idx = GET_COUNTERID(event, cmask);
+	event_id = GET_EVENTID(event, emask);
 
 	/* enable and start counters.
 	 * 8 bits for each counter, bits[05:01] of a counter to set event type.
@@ -302,9 +412,14 @@ static void uncore_start_event_dmc(struct perf_event *event, int flags)
 
 static void uncore_stop_event_dmc(struct perf_event *event)
 {
-	u32 val;
+	u32 val, cmask;
 	struct hw_perf_event *hwc = &event->hw;
-	int idx = GET_COUNTERID(event);
+	struct tx2_uncore_pmu *tx2_pmu;
+	int idx;
+
+	tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+	cmask = tx2_pmu->counters_mask;
+	idx = GET_COUNTERID(event, cmask);
 
 	/* clear event type(bits[05:01]) to stop counter */
 	val = reg_readl(hwc->config_base);
@@ -312,27 +427,72 @@ static void uncore_stop_event_dmc(struct perf_event *event)
 	reg_writel(val, hwc->config_base);
 }
 
+static void uncore_start_event_ccpi2(struct perf_event *event, int flags)
+{
+	u32 emask;
+	struct hw_perf_event *hwc = &event->hw;
+	struct tx2_uncore_pmu *tx2_pmu;
+
+	tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+	emask = tx2_pmu->events_mask;
+
+	/* Bit [09:00] to set event id.
+	 * Bits [10], set level to rising edge.
+	 * Bits [11], set type to edge sensitive.
+	 */
+	reg_writel((CCPI2_EVENT_TYPE_EDGE_SENSITIVE |
+			CCPI2_EVENT_LEVEL_RISING_EDGE |
+			GET_EVENTID(event, emask)), hwc->config_base);
+
+	/* reset[4], enable[0] and start[1] counters */
+	reg_writel(CCPI2_PERF_CTL_RESET |
+			CCPI2_PERF_CTL_START |
+			CCPI2_PERF_CTL_ENABLE,
+			hwc->event_base + CCPI2_PERF_CTL);
+	local64_set(&event->hw.prev_count, 0ULL);
+}
+
+static void uncore_stop_event_ccpi2(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+
+	/* disable and stop counter */
+	reg_writel(0, hwc->event_base + CCPI2_PERF_CTL);
+}
+
 static void tx2_uncore_event_update(struct perf_event *event)
 {
-	s64 prev, delta, new = 0;
+	u64 prev, delta, new = 0;
 	struct hw_perf_event *hwc = &event->hw;
 	struct tx2_uncore_pmu *tx2_pmu;
 	enum tx2_uncore_type type;
 	u32 prorate_factor;
+	u32 cmask, emask;
 
 	tx2_pmu = pmu_to_tx2_pmu(event->pmu);
 	type = tx2_pmu->type;
+	cmask = tx2_pmu->counters_mask;
+	emask = tx2_pmu->events_mask;
 	prorate_factor = tx2_pmu->prorate_factor;
-
-	new = reg_readl(hwc->event_base);
-	prev = local64_xchg(&hwc->prev_count, new);
-
-	/* handles rollover of 32 bit counter */
-	delta = (u32)(((1UL << 32) - prev) + new);
+	if (type == PMU_TYPE_CCPI2) {
+		reg_writel(CCPI2_COUNTER_OFFSET +
+				GET_COUNTERID(event, cmask),
+				hwc->event_base + CCPI2_COUNTER_SEL);
+		new = reg_readl(hwc->event_base + CCPI2_COUNTER_DATA_H);
+		new = (new << 32) +
+			reg_readl(hwc->event_base + CCPI2_COUNTER_DATA_L);
+		prev = local64_xchg(&hwc->prev_count, new);
+		delta = new - prev;
+	} else {
+		new = reg_readl(hwc->event_base);
+		prev = local64_xchg(&hwc->prev_count, new);
+		/* handles rollover of 32 bit counter */
+		delta = (u32)(((1UL << 32) - prev) + new);
+	}
 
 	/* DMC event data_transfers granularity is 16 Bytes, convert it to 64 */
 	if (type == PMU_TYPE_DMC &&
-			GET_EVENTID(event) == DMC_EVENT_DATA_TRANSFERS)
+			GET_EVENTID(event, emask) == DMC_EVENT_DATA_TRANSFERS)
 		delta = delta/4;
 
 	/* L3C and DMC has 16 and 8 interleave channels respectively.
@@ -351,6 +511,7 @@ static enum tx2_uncore_type get_tx2_pmu_type(struct acpi_device *adev)
 	} devices[] = {
 		{"CAV901D", PMU_TYPE_L3C},
 		{"CAV901F", PMU_TYPE_DMC},
+		{"CAV901E", PMU_TYPE_CCPI2},
 		{"", PMU_TYPE_INVALID}
 	};
 
@@ -380,7 +541,8 @@ static bool tx2_uncore_validate_event(struct pmu *pmu,
  * Make sure the group of events can be scheduled at once
  * on the PMU.
  */
-static bool tx2_uncore_validate_event_group(struct perf_event *event)
+static bool tx2_uncore_validate_event_group(struct perf_event *event,
+		int max_counters)
 {
 	struct perf_event *sibling, *leader = event->group_leader;
 	int counters = 0;
@@ -403,7 +565,7 @@ static bool tx2_uncore_validate_event_group(struct perf_event *event)
 	 * If the group requires more counters than the HW has,
 	 * it cannot ever be scheduled.
 	 */
-	return counters <= TX2_PMU_MAX_COUNTERS;
+	return counters <= max_counters;
 }
 
 
@@ -439,7 +601,7 @@ static int tx2_uncore_event_init(struct perf_event *event)
 	hwc->config = event->attr.config;
 
 	/* Validate the group */
-	if (!tx2_uncore_validate_event_group(event))
+	if (!tx2_uncore_validate_event_group(event, tx2_pmu->max_counters))
 		return -EINVAL;
 
 	return 0;
@@ -456,6 +618,10 @@ static void tx2_uncore_event_start(struct perf_event *event, int flags)
 	tx2_pmu->start_event(event, flags);
 	perf_event_update_userpage(event);
 
+	/* No hrtimer needed for CCPI2, 64-bit counters */
+	if (!tx2_pmu->hrtimer_callback)
+		return;
+
 	/* Start timer for first event */
 	if (bitmap_weight(tx2_pmu->active_counters,
 				tx2_pmu->max_counters) == 1) {
@@ -510,15 +676,23 @@ static void tx2_uncore_event_del(struct perf_event *event, int flags)
 {
 	struct tx2_uncore_pmu *tx2_pmu = pmu_to_tx2_pmu(event->pmu);
 	struct hw_perf_event *hwc = &event->hw;
+	u32 cmask;
 
+	cmask = tx2_pmu->counters_mask;
 	tx2_uncore_event_stop(event, PERF_EF_UPDATE);
 
 	/* clear the assigned counter */
-	free_counter(tx2_pmu, GET_COUNTERID(event));
+	free_counter(tx2_pmu, GET_COUNTERID(event, cmask));
 
 	perf_event_update_userpage(event);
 	tx2_pmu->events[hwc->idx] = NULL;
 	hwc->idx = -1;
+
+	if (!tx2_pmu->hrtimer_callback)
+		return;
+
+	if (bitmap_empty(tx2_pmu->active_counters, tx2_pmu->max_counters))
+		hrtimer_cancel(&tx2_pmu->hrtimer);
 }
 
 static void tx2_uncore_event_read(struct perf_event *event)
@@ -580,8 +754,12 @@ static int tx2_uncore_pmu_add_dev(struct tx2_uncore_pmu *tx2_pmu)
 			cpu_online_mask);
 
 	tx2_pmu->cpu = cpu;
-	hrtimer_init(&tx2_pmu->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
-	tx2_pmu->hrtimer.function = tx2_hrtimer_callback;
+
+	if (tx2_pmu->hrtimer_callback) {
+		hrtimer_init(&tx2_pmu->hrtimer,
+				CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		tx2_pmu->hrtimer.function = tx2_pmu->hrtimer_callback;
+	}
 
 	ret = tx2_uncore_pmu_register(tx2_pmu);
 	if (ret) {
@@ -653,10 +831,13 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev,
 
 	switch (tx2_pmu->type) {
 	case PMU_TYPE_L3C:
-		tx2_pmu->max_counters = TX2_PMU_MAX_COUNTERS;
+		tx2_pmu->max_counters = TX2_PMU_DMC_L3C_MAX_COUNTERS;
+		tx2_pmu->counters_mask = 0x3;
 		tx2_pmu->prorate_factor = TX2_PMU_L3_TILES;
 		tx2_pmu->max_events = L3_EVENT_MAX;
+		tx2_pmu->events_mask = 0x1f;
 		tx2_pmu->hrtimer_interval = TX2_PMU_HRTIMER_INTERVAL;
+		tx2_pmu->hrtimer_callback = tx2_hrtimer_callback;
 		tx2_pmu->attr_groups = l3c_pmu_attr_groups;
 		tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL,
 				"uncore_l3c_%d", tx2_pmu->node);
@@ -665,10 +846,13 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev,
 		tx2_pmu->stop_event = uncore_stop_event_l3c;
 		break;
 	case PMU_TYPE_DMC:
-		tx2_pmu->max_counters = TX2_PMU_MAX_COUNTERS;
+		tx2_pmu->max_counters = TX2_PMU_DMC_L3C_MAX_COUNTERS;
+		tx2_pmu->counters_mask = 0x3;
 		tx2_pmu->prorate_factor = TX2_PMU_DMC_CHANNELS;
 		tx2_pmu->max_events = DMC_EVENT_MAX;
+		tx2_pmu->events_mask = 0x1f;
 		tx2_pmu->hrtimer_interval = TX2_PMU_HRTIMER_INTERVAL;
+		tx2_pmu->hrtimer_callback = tx2_hrtimer_callback;
 		tx2_pmu->attr_groups = dmc_pmu_attr_groups;
 		tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL,
 				"uncore_dmc_%d", tx2_pmu->node);
@@ -676,6 +860,21 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev,
 		tx2_pmu->start_event = uncore_start_event_dmc;
 		tx2_pmu->stop_event = uncore_stop_event_dmc;
 		break;
+	case PMU_TYPE_CCPI2:
+		/* CCPI2 has 8 counters */
+		tx2_pmu->max_counters = TX2_PMU_CCPI2_MAX_COUNTERS;
+		tx2_pmu->counters_mask = 0x7;
+		tx2_pmu->prorate_factor = 1;
+		tx2_pmu->max_events = CCPI2_EVENT_MAX;
+		tx2_pmu->events_mask = 0x1ff;
+		tx2_pmu->attr_groups = ccpi2_pmu_attr_groups;
+		tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL,
+				"uncore_ccpi2_%d", tx2_pmu->node);
+		tx2_pmu->init_cntr_base = init_cntr_base_ccpi2;
+		tx2_pmu->start_event = uncore_start_event_ccpi2;
+		tx2_pmu->stop_event = uncore_stop_event_ccpi2;
+		tx2_pmu->hrtimer_callback = NULL;
+		break;
 	case PMU_TYPE_INVALID:
 		devm_kfree(dev, tx2_pmu);
 		return NULL;
@@ -744,7 +943,9 @@ static int tx2_uncore_pmu_offline_cpu(unsigned int cpu,
 	if (cpu != tx2_pmu->cpu)
 		return 0;
 
-	hrtimer_cancel(&tx2_pmu->hrtimer);
+	if (tx2_pmu->hrtimer_callback)
+		hrtimer_cancel(&tx2_pmu->hrtimer);
+
 	cpumask_copy(&cpu_online_mask_temp, cpu_online_mask);
 	cpumask_clear_cpu(cpu, &cpu_online_mask_temp);
 	new_cpu = cpumask_any_and(
-- 
2.17.1


^ permalink raw reply related

* RE: [PATCH v1 1/2] vsprintf: introduce %dE for error constants
From: David Laight @ 2019-08-30 13:21 UTC (permalink / raw)
  To: 'Enrico Weigelt, metux IT consult', Uwe Kleine-König,
	Andrew Morton
  Cc: Jonathan Corbet, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
	Linus Walleij, Bartosz Golaszewski
In-Reply-To: <a5f535af-09f7-e65b-1527-7d6dd8553c1d@metux.net>

From: Enrico Weigelt, metux IT consult
> Sent: 26 August 2019 14:29
> On 25.08.19 01:37, Uwe Kleine-König wrote:
> 
> Hi,
> 
> > +static noinline_for_stack > +char *errstr(char *buf, char *end, unsigned long long num,> +
> struct printf_spec spec)> +{
> #1: why not putting that into some separate strerror() lib function ?
>      This is something I've been looking for quite some time (actually
>      already hacked it up somewhere, sometime, but forgotten ...)
> 
> #2: why not just having a big case statement and leave the actual lookup
>      logic to the compiler ? IMHO, could be written in a very compact way
>      by some macro magic

And generate an enormous amount of code and long chains of mispredicted branches.

Is it also worth looking at how long the strings are.
If they can be truncated to 16 bytes then char[][16] will generate
much better code than the array of pointers.

OTOH I'm not really sure it is all a good idea.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: [PATCH v4 01/10] KVM: arm64: Document PV-time interface
From: Andrew Jones @ 2019-08-30 14:47 UTC (permalink / raw)
  To: Steven Price
  Cc: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm,
	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-2-steven.price@arm.com>

On Fri, Aug 30, 2019 at 09:42:46AM +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   | 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

PV_TIME_LPT doesn't exist

> +    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

The above fields don't appear to be exposed to userspace in anyway. How
will we handle migration from one KVM with one version of the structure
to another?

> +  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.

Should we provide a recommendation as to how that reserved memory is
provided? One memslot divided into NR_VCPUS subregions? Should the
reserved region be described to the guest kernel with DT/ACPI? Or
should userspace ensure the region is not within any DT/ACPI described
regions?

> +
> +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
>

Thanks,
drew 

^ permalink raw reply

* Re: [PATCH v4 07/10] KVM: arm64: Provide VCPU attributes for stolen time
From: Steven Price @ 2019-08-30 15:04 UTC (permalink / raw)
  To: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
  Cc: Mark Rutland, kvm, Radim Krčmář, Catalin Marinas,
	Suzuki K Pouloze, linux-doc, Russell King, linux-kernel,
	James Morse, Paolo Bonzini, Julien Thierry
In-Reply-To: <36104ec0-2237-fb0e-376f-ab50c23c6101@kernel.org>

On 30/08/2019 11:02, Marc Zyngier wrote:
> 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.

Sure

>> +		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?

Considering we really can't reasonably validate IPA overlapping with
guest memory (how is the host to know what is 'guest memory'), I'm not
convinced it's worth the code to detect overlapping vcpus. Nothing bad
will happen to the host in this case (the kvm_put_guest() calls will
just clobber each other).

In terms of checking the IPA is valid - again this is something that can
change in the lifetime of the VM, so the check at setup time isn't
particularly useful. Currently it's also possible to create the vcpus
(including setting up the stolen time) before the memory is assigned to
the guest as long as the vcpus are not started. This seems like a useful
level of flexibility.

> 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.

Yes it should be possible to add it - I'd like to wait to see whether
user actually want to use multiple memslots though - it's quite possible
to use a single memslot for both guest memory and stolen time structures.

>> +	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.

Yes that does look weird. I'll drop the "SET_" part from the symbol. I'm
not sure what I was thinking when I named that.

Thanks,

Steve

>> +		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.
> 


^ permalink raw reply

* Re: [PATCH v4 01/10] KVM: arm64: Document PV-time interface
From: Steven Price @ 2019-08-30 15:25 UTC (permalink / raw)
  To: Andrew Jones
  Cc: Mark Rutland, kvm, Radim Krčmář, Marc Zyngier,
	Suzuki K Pouloze, linux-doc, Russell King, linux-kernel,
	James Morse, Julien Thierry, Catalin Marinas, Paolo Bonzini,
	Will Deacon, kvmarm, linux-arm-kernel
In-Reply-To: <20190830144734.kvj4dvt32qzmhw32@kamzik.brq.redhat.com>

On 30/08/2019 15:47, Andrew Jones wrote:
> On Fri, Aug 30, 2019 at 09:42:46AM +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   | 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
> 
> PV_TIME_LPT doesn't exist

Thanks, will remove.

>> +    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
> 
> The above fields don't appear to be exposed to userspace in anyway. How
> will we handle migration from one KVM with one version of the structure
> to another?

Interesting question. User space does have access to them now it is
providing the memory, but it's not exactly an easy method. In particular
user space has no (simple) way of probing the kernel's supported version.

I guess one solution would be to add an extra attribute on the VCPU
which would provide the revision information. The current kernel would
then reject any revision other than 0, but this could then be extended
to support other revision numbers in the future.

Although there's some logic in saying we could add the extra attribute
when(/if) there is a new version. Future kernels would then be expected
to use the current version unless user space explicitly set the new
attribute.

Do you feel this is something that needs to be addressed now, or can it
be deferred until another version is proposed?

>> +  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.
> 
> Should we provide a recommendation as to how that reserved memory is
> provided? One memslot divided into NR_VCPUS subregions? Should the
> reserved region be described to the guest kernel with DT/ACPI? Or
> should userspace ensure the region is not within any DT/ACPI described
> regions?

I'm open to providing a recommendation, but I'm not entirely sure I know
enough here to provide one.

There is an obvious efficiency argument for minimizing memslots with the
current code. But if someone has a reason for using multiple memslots
then that's probably a good argument for implementing a memslot-caching
kvm_put_user() rather than to be dis-recommended.

My assumption (and testing) has been with a single memslot divided into
NR_VCPUS (or more accurately the number of VCPUs in the VM) subregions.

For testing DT I've tested both methods: an explicit reserved region or
just ensuring it's not in any DT described region. Both seem reasonable,
but it might be easier to integrate into existing migration mechanisms
if it's simply a reserved region (then the memory block of the guest is
just as it always was).

For ACPI the situation should be similar, but my testing has been with DT.

Thanks,

Steve

>> +
>> +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
>>
> 
> Thanks,
> drew 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 


^ permalink raw reply

* Re: [PATCH v2] vsprintf: introduce %dE for error constants
From: Uwe Kleine-König @ 2019-08-30 15:47 UTC (permalink / raw)
  To: Petr Mladek
  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: <20190830090624.tb2tmhbt3wzwz6rp@pathway.suse.cz>


[-- Attachment #1.1: Type: text/plain, Size: 3022 bytes --]

Hello Petr,

On 8/30/19 11:06 AM, Petr Mladek wrote:
> 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:
>>>> 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 thought about this and we could make it possible by some syntax. Such that

	someint = 42
	pr_info("%d}E\n", someint)

emits

	42E

(I'm open to use a different "end-of-fmt-specifier" char, a } might
confuse source editors when highlighting matching braces. Maybe '#'?

This idea could be transferred to %p, too, which then lift the
limitation that some strings cannot easily be produced by printk et al.
(Of course this makes the format string parsing still more complicated
which I expect you won't like.) This would make it possible then to
adapt drivers/staging/speakup/speakup_bns.c before introducing the
format for error ints.

> 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.

AFAIK they are put after the format specifier in the kernel to still be
able to benefit from the compiler's printf attribute.

>>> 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.

The same holds true for %dE. Something has to happen if an int is passed
that isn't an error code. I'm emitting it as is in my patch, the same
could be done for a pointer.

> 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.

I suggest to postpone this until we have %dE. (But I consider using %de
instead as then if we later chose that %pe is a nice idea they can use
the same modifier.)

Best regards
Uwe


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH 0/9] drivers: add new variants of devm_platform_ioremap_resource()
From: Christoph Hellwig @ 2019-08-30 15:59 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Bartosz Golaszewski, Jonathan Corbet, Greg Kroah-Hartman,
	Rafael J . Wysocki, Alban Bedel, Linus Walleij, Arnd Bergmann,
	Geert Uytterhoeven, open list:DOCUMENTATION,
	Linux Kernel Mailing List, open list:GPIO SUBSYSTEM, Julia Lawall,
	Bartosz Golaszewski, Christoph Hellwig
In-Reply-To: <CAMuHMdW8d1h-81jy-dgDiLfGB3MGPx+f-Zqz+4D5S+gtmk3-BQ@mail.gmail.com>

On Thu, Aug 29, 2019 at 04:48:36PM +0200, Geert Uytterhoeven wrote:
> Hi Bartosz,
> 
> On Thu, Aug 29, 2019 at 4:38 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > The new devm_platform_ioremap_resource() helper has now been widely
> > adopted and used in many drivers. Users of nocache and write-combined
> > ioremap() variants could profit from the same code shrinkage. This
> > series provides two new versions of devm_platform_ioremap_resource()
> > and uses it in a few example drivers with the assumption that - just
> > like was the case previously - a coccinelle script will be developed
> > to ease the transition for others.
> 
> Please be aware that the number of ioremap() variants is being
> reduced, as some of them are redundant (e.g. ioremap() already creates
> an uncached mapping, so ioremap_nocache() is not needed).
> So less is better than more ;-)

Yes.  If I can get the ia64 and openrisc patch in I plan to send Linus
a scripted removal of ioremap_nocache after -rc1.  I already have a
local patch for current mainline as of about two weeks ago.

^ permalink raw reply

* Re: [PATCH v2] vsprintf: introduce %dE for error constants
From: Rasmus Villemoes @ 2019-08-30 21:45 UTC (permalink / raw)
  To: Uwe Kleine-König, Rasmus Villemoes, Juergen Gross,
	Petr Mladek
  Cc: 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 29/08/2019 19.39, Uwe Kleine-König wrote:
> On 8/29/19 11:09 AM, Rasmus Villemoes wrote:

>> 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. :-)

Oh no. And actually, come to think of it, we don't even need to extend
%p at all (taking away yet another letter for future expansion...), we
should simply make %p DTRT when passed an ERR_PTR - currently, if it's
some %p<extension> that would normally derefence it, there's sanity
checking in place which makes it print (efault), while if it's plain %p,
it just does the hashing, making it impossible to figure out that it was
an errno value (or which it was).

I've cooked up what I mean, sending in separate thread.

Rasmus



^ permalink raw reply

* Re: [PATCH] docs: ftrace: clarify when tracing is disabled by the trace file
From: Steven Rostedt @ 2019-08-30 21:51 UTC (permalink / raw)
  To: Peter Wu
  Cc: Ingo Molnar, Jonathan Corbet, linux-doc, linux-kernel,
	Alexei Starovoitov
In-Reply-To: <20190822234823.18594-1-peter@lekensteyn.nl>

On Fri, 23 Aug 2019 00:48:23 +0100
Peter Wu <peter@lekensteyn.nl> wrote:

> The current text could mislead the user into believing that only read()
> disables tracing. Clarify that any open() call that requests read access
> disables tracing.
> 
> Link: https://lkml.kernel.org/r/CAADnVQ+hU6QOC_dPmpjnuv=9g4SQEeaMEMqXOS2WpMj=q=LdiQ@mail.gmail.com
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
>  Documentation/trace/ftrace.rst | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
> index f60079259669..965be5c9afb3 100644
> --- a/Documentation/trace/ftrace.rst
> +++ b/Documentation/trace/ftrace.rst
> @@ -125,7 +125,8 @@ of ftrace. Here is a list of some of the key files:
>  
>  	This file holds the output of the trace in a human
>  	readable format (described below). Note, tracing is temporarily
> -	disabled while this file is being read (opened).
> +	disabled when the file is open for reading. Once all readers
> +	are closed, tracing is re-enabled.
>  
>    trace_pipe:
>  
> @@ -139,8 +140,9 @@ of ftrace. Here is a list of some of the key files:
>  	will not be read again with a sequential read. The
>  	"trace" file is static, and if the tracer is not
>  	adding more data, it will display the same
> -	information every time it is read. This file will not
> -	disable tracing while being read.
> +	information every time it is read. Unlike the
> +	"trace" file, opening this file for reading will not
> +	temporarily disable tracing.
>  
>    trace_options:
>  
> @@ -3153,7 +3155,10 @@ different. The trace is live.
>  
>  
>  Note, reading the trace_pipe file will block until more input is
> -added.
> +added. This is contrary to the trace file. If any process opened
> +the trace file for reading, it will actually disable tracing and
> +prevent new entries from being added. The trace_file file does

I was just about to ack this, and then I saw the above.

  s/trace_file/trace_pipe/

Other than that, it looks good!

-- Steve

> +not have this limitation.
>  
>  trace entries
>  -------------


^ permalink raw reply

* [PATCH] doc:lock: remove reference to clever use of read-write lock
From: Federico Vaga @ 2019-08-31 13:41 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, linux-kernel, linux-doc,
	Federico Vaga

Remove the clever example about read-write lock because these type of
lock is not reccomended anymore (according to the very same document).
So there is no reason to teach cleaver things that people should not do.

(and by the way there was a little typo)

Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
---
 Documentation/locking/spinlocks.rst | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/Documentation/locking/spinlocks.rst b/Documentation/locking/spinlocks.rst
index e93ec6645238..8053fd4c3544 100644
--- a/Documentation/locking/spinlocks.rst
+++ b/Documentation/locking/spinlocks.rst
@@ -106,7 +106,7 @@ and on other architectures it can be worse).
 
 If you have a case where you have to protect a data structure across
 several CPU's and you want to use spinlocks you can potentially use
-cheaper versions of the spinlocks. IFF you know that the spinlocks are
+cheaper versions of the spinlocks. If you know that the spinlocks are
 never used in interrupt handlers, you can use the non-irq versions::
 
 	spin_lock(&lock);
@@ -139,18 +139,6 @@ on other CPU's, because an interrupt on another CPU doesn't interrupt the
 CPU that holds the lock, so the lock-holder can continue and eventually
 releases the lock).
 
-Note that you can be clever with read-write locks and interrupts. For
-example, if you know that the interrupt only ever gets a read-lock, then
-you can use a non-irq version of read locks everywhere - because they
-don't block on each other (and thus there is no dead-lock wrt interrupts.
-But when you do the write-lock, you have to use the irq-safe version.
-
-For an example of being clever with rw-locks, see the "waitqueue_lock"
-handling in kernel/sched/core.c - nothing ever _changes_ a wait-queue from
-within an interrupt, they only read the queue in order to know whom to
-wake up. So read-locks are safe (which is good: they are very common
-indeed), while write-locks need to protect themselves against interrupts.
-
 		Linus
 
 ----
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH] doc:lock: remove reference to clever use of read-write lock
From: Jonathan Corbet @ 2019-08-31 14:43 UTC (permalink / raw)
  To: Federico Vaga
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, linux-kernel, linux-doc
In-Reply-To: <20190831134116.25417-1-federico.vaga@vaga.pv.it>

On Sat, 31 Aug 2019 15:41:16 +0200
Federico Vaga <federico.vaga@vaga.pv.it> wrote:

>  several CPU's and you want to use spinlocks you can potentially use
> -cheaper versions of the spinlocks. IFF you know that the spinlocks are
> +cheaper versions of the spinlocks. If you know that the spinlocks are
>  never used in interrupt handlers, you can use the non-irq versions::

I suspect that was not actually a typo; "iff" is a way for the
mathematically inclined to say "if and only if".

jon

^ permalink raw reply

* [PATCH v2] docs: ftrace: clarify when tracing is disabled by the trace file
From: Peter Wu @ 2019-08-31 15:35 UTC (permalink / raw)
  To: Steven Rostedt, Ingo Molnar, Jonathan Corbet
  Cc: linux-doc, linux-kernel, Alexei Starovoitov
In-Reply-To: <20190830175108.0ffa6ef1@gandalf.local.home>

The current text could mislead the user into believing that only read()
disables tracing. Clarify that any open() call that requests read access
disables tracing.

Link: https://lkml.kernel.org/r/CAADnVQ+hU6QOC_dPmpjnuv=9g4SQEeaMEMqXOS2WpMj=q=LdiQ@mail.gmail.com
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
v2: fix typo s/trace_file/trace_pipe/ (spotted by Steven)
---
 Documentation/trace/ftrace.rst | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
index f60079259669..e3060eedb22d 100644
--- a/Documentation/trace/ftrace.rst
+++ b/Documentation/trace/ftrace.rst
@@ -125,7 +125,8 @@ of ftrace. Here is a list of some of the key files:
 
 	This file holds the output of the trace in a human
 	readable format (described below). Note, tracing is temporarily
-	disabled while this file is being read (opened).
+	disabled when the file is open for reading. Once all readers
+	are closed, tracing is re-enabled.
 
   trace_pipe:
 
@@ -139,8 +140,9 @@ of ftrace. Here is a list of some of the key files:
 	will not be read again with a sequential read. The
 	"trace" file is static, and if the tracer is not
 	adding more data, it will display the same
-	information every time it is read. This file will not
-	disable tracing while being read.
+	information every time it is read. Unlike the
+	"trace" file, opening this file for reading will not
+	temporarily disable tracing.
 
   trace_options:
 
@@ -3153,7 +3155,10 @@ different. The trace is live.
 
 
 Note, reading the trace_pipe file will block until more input is
-added.
+added. This is contrary to the trace file. If any process opened
+the trace file for reading, it will actually disable tracing and
+prevent new entries from being added. The trace_pipe file does
+not have this limitation.
 
 trace entries
 -------------
-- 
2.22.0


^ permalink raw reply related

* [PATCH] Ext4 Docs: Add missing bigalloc documentation.
From: Ayush Ranjan @ 2019-08-31 15:44 UTC (permalink / raw)
  To: Theodore Ts'o, Andreas Dilger, Jonathan Corbet
  Cc: linux-ext4, linux-doc, linux-kernel

There was a broken link for bigalloc. The page
https://ext4.wiki.kernel.org/index.php/Bigalloc was not migrated into
the current documentation sources. This patch adds the contents of that
missing page into the section for Bigalloc itself.

Signed-off-by: Ayush Ranjan <ayushr2@illinois.edu>
---
Please note that I have not included changes from the "Kernel Support"
and "E2fsprogs Support" sections of the original page because the
comments there seemed outdated.

 Documentation/filesystems/ext4/bigalloc.rst | 32 ++++++++++++++-------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/Documentation/filesystems/ext4/bigalloc.rst b/Documentation/filesystems/ext4/bigalloc.rst
index c6d885575..72075aa60 100644
--- a/Documentation/filesystems/ext4/bigalloc.rst
+++ b/Documentation/filesystems/ext4/bigalloc.rst
@@ -9,14 +9,26 @@ ext4 code is not prepared to handle the case where the block size
 exceeds the page size. However, for a filesystem of mostly huge files,
 it is desirable to be able to allocate disk blocks in units of multiple
 blocks to reduce both fragmentation and metadata overhead. The
-`bigalloc <Bigalloc>`__ feature provides exactly this ability. The
-administrator can set a block cluster size at mkfs time (which is stored
-in the s\_log\_cluster\_size field in the superblock); from then on, the
-block bitmaps track clusters, not individual blocks. This means that
-block groups can be several gigabytes in size (instead of just 128MiB);
-however, the minimum allocation unit becomes a cluster, not a block,
-even for directories. TaoBao had a patchset to extend the “use units of
-clusters instead of blocks” to the extent tree, though it is not clear
-where those patches went-- they eventually morphed into “extent tree v2”
-but that code has not landed as of May 2015.
+bigalloc feature provides exactly this ability.
+
+The bigalloc feature (EXT4_FEATURE_RO_COMPAT_BIGALLOC) changes ext4 to
+use clustered allocation, so that each bit in the ext4 block allocation
+bitmap addresses a power of two number of blocks. For example, if the
+file system is mainly going to be storing large files in the 4-32
+megabyte range, it might make sense to set a cluster size of 1 megabyte.
+This means that each bit in the block allocation bitmap now addresses
+256 4k blocks. This shrinks the total size of the block allocation
+bitmaps for a 2T file system from 64 megabytes to 256 kilobytes. It also
+means that a block group addresses 32 gigabytes instead of 128 megabytes,
+also shrinking the amount of file system overhead for metadata.
+
+The administrator can set a block cluster size at mkfs time (which is
+stored in the s\_log\_cluster\_size field in the superblock); from then
+on, the block bitmaps track clusters, not individual blocks. This means
+that block groups can be several gigabytes in size (instead of just
+128MiB); however, the minimum allocation unit becomes a cluster, not a
+block, even for directories. TaoBao had a patchset to extend the “use
+units of clusters instead of blocks” to the extent tree, though it is
+not clear where those patches went-- they eventually morphed into
+“extent tree v2” but that code has not landed as of May 2015.
 
-- 
2.23.0


^ permalink raw reply related

* Re: [PATCH v2] docs: ftrace: clarify when tracing is disabled by the trace file
From: Steven Rostedt @ 2019-08-31 15:58 UTC (permalink / raw)
  To: Peter Wu
  Cc: Ingo Molnar, Jonathan Corbet, linux-doc, linux-kernel,
	Alexei Starovoitov
In-Reply-To: <20190831153500.7399-1-peter@lekensteyn.nl>

On Sat, 31 Aug 2019 16:35:00 +0100
Peter Wu <peter@lekensteyn.nl> wrote:

> The current text could mislead the user into believing that only read()
> disables tracing. Clarify that any open() call that requests read access
> disables tracing.
> 
> Link: https://lkml.kernel.org/r/CAADnVQ+hU6QOC_dPmpjnuv=9g4SQEeaMEMqXOS2WpMj=q=LdiQ@mail.gmail.com
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

Jon care to take this in your tree?

Thanks!

-- Steve

> ---
> v2: fix typo s/trace_file/trace_pipe/ (spotted by Steven)
> ---
>  Documentation/trace/ftrace.rst | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
> index f60079259669..e3060eedb22d 100644
> --- a/Documentation/trace/ftrace.rst
> +++ b/Documentation/trace/ftrace.rst
> @@ -125,7 +125,8 @@ of ftrace. Here is a list of some of the key files:
>  
>  	This file holds the output of the trace in a human
>  	readable format (described below). Note, tracing is temporarily
> -	disabled while this file is being read (opened).
> +	disabled when the file is open for reading. Once all readers
> +	are closed, tracing is re-enabled.
>  
>    trace_pipe:
>  
> @@ -139,8 +140,9 @@ of ftrace. Here is a list of some of the key files:
>  	will not be read again with a sequential read. The
>  	"trace" file is static, and if the tracer is not
>  	adding more data, it will display the same
> -	information every time it is read. This file will not
> -	disable tracing while being read.
> +	information every time it is read. Unlike the
> +	"trace" file, opening this file for reading will not
> +	temporarily disable tracing.
>  
>    trace_options:
>  
> @@ -3153,7 +3155,10 @@ different. The trace is live.
>  
>  
>  Note, reading the trace_pipe file will block until more input is
> -added.
> +added. This is contrary to the trace file. If any process opened
> +the trace file for reading, it will actually disable tracing and
> +prevent new entries from being added. The trace_pipe file does
> +not have this limitation.
>  
>  trace entries
>  -------------


^ permalink raw reply

* [PATCH v3 2/2] kbuild: rename KBUILD_ENABLE_EXTRA_GCC_CHECKS to KBUILD_EXTRA_WARN
From: Masahiro Yamada @ 2019-08-31 16:25 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Nick Desaulniers, Nathan Chancellor, Sedat Dilek, Masahiro Yamada,
	Jonathan Corbet, Michal Marek, clang-built-linux, linux-doc,
	linux-kernel
In-Reply-To: <20190831162555.31887-1-yamada.masahiro@socionext.com>

KBUILD_ENABLE_EXTRA_GCC_CHECKS started as a switch to add extra warning
options for GCC, but now it is a historical misnomer since we use it
also for Clang, DTC, and even kernel-doc.

Rename it to more sensible, and shorter KBUILD_EXTRA_WARN.

For the backward compatibility, KBUILD_ENABLE_EXTRA_GCC_CHECKS is still
supported (but not advertised in the documentation).

I also fixed up 'make help', and updated the documentation.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v3:
  - new patch

Changes in v2: None

 Documentation/kbuild/kbuild.rst | 14 +++++++++-----
 Makefile                        |  2 +-
 scripts/Makefile.build          |  2 +-
 scripts/Makefile.extrawarn      | 13 +++++++++----
 scripts/Makefile.lib            |  4 ++--
 scripts/genksyms/Makefile       |  2 +-
 6 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst
index 62f9d86c082c..f1e5dce86af7 100644
--- a/Documentation/kbuild/kbuild.rst
+++ b/Documentation/kbuild/kbuild.rst
@@ -105,6 +105,15 @@ The output directory can also be specified using "O=...".
 
 Setting "O=..." takes precedence over KBUILD_OUTPUT.
 
+KBUILD_EXTRA_WARN
+-----------------
+Specify the extra build checks. The same value can be assigned by passing
+W=... from the command line.
+
+See `make help` for the list of the supported values.
+
+Setting "W=..." takes precedence over KBUILD_EXTRA_WARN.
+
 KBUILD_DEBARCH
 --------------
 For the deb-pkg target, allows overriding the normal heuristics deployed by
@@ -241,11 +250,6 @@ To get all available archs you can also specify all. E.g.::
 
     $ make ALLSOURCE_ARCHS=all tags
 
-KBUILD_ENABLE_EXTRA_GCC_CHECKS
-------------------------------
-If enabled over the make command line with "W=1", it turns on additional
-gcc -W... options for more extensive build-time checking.
-
 KBUILD_BUILD_TIMESTAMP
 ----------------------
 Setting this to a date string overrides the timestamp used in the
diff --git a/Makefile b/Makefile
index 06e1e21c0f45..016d72eb3ddf 100644
--- a/Makefile
+++ b/Makefile
@@ -1538,7 +1538,7 @@ help:
 	@echo  '  make C=1   [targets] Check re-compiled c source with $$CHECK (sparse by default)'
 	@echo  '  make C=2   [targets] Force check of all c source with $$CHECK'
 	@echo  '  make RECORDMCOUNT_WARN=1 [targets] Warn about ignored mcount sections'
-	@echo  '  make W=n   [targets] Enable extra gcc checks, n=1,2,3 where'
+	@echo  '  make W=n   [targets] Enable extra checks, n=1,2,3 where'
 	@echo  '		1: warnings which may be relevant and do not occur too often'
 	@echo  '		2: warnings which occur quite often but may still be relevant'
 	@echo  '		3: more obscure warnings, can most likely be ignored'
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 2a21ca86b720..f72aba64d611 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -85,7 +85,7 @@ else ifeq ($(KBUILD_CHECKSRC),2)
         cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $<
 endif
 
-ifneq ($(KBUILD_ENABLE_EXTRA_GCC_CHECKS),)
+ifneq ($(KBUILD_EXTRA_WARN),)
   cmd_checkdoc = $(srctree)/scripts/kernel-doc -none $<
 endif
 
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index d226c5fb13e2..53eb7e0c6a5a 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -8,14 +8,19 @@
 
 KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned)
 
+# backward compatibility
+KBUILD_EXTRA_WARN ?= $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)
+
 ifeq ("$(origin W)", "command line")
-  export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W)
+  KBUILD_EXTRA_WARN := $(W)
 endif
 
+export KBUILD_EXTRA_WARN
+
 #
 # W=1 - warnings which may be relevant and do not occur too often
 #
-ifneq ($(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
+ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)
 
 KBUILD_CFLAGS += -Wextra -Wunused -Wno-unused-parameter
 KBUILD_CFLAGS += -Wmissing-declarations
@@ -48,7 +53,7 @@ endif
 #
 # W=2 - warnings which occur quite often but may still be relevant
 #
-ifneq ($(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
+ifneq ($(findstring 2, $(KBUILD_EXTRA_WARN)),)
 
 KBUILD_CFLAGS += -Wcast-align
 KBUILD_CFLAGS += -Wdisabled-optimization
@@ -65,7 +70,7 @@ endif
 #
 # W=3 - more obscure warnings, can most likely be ignored
 #
-ifneq ($(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
+ifneq ($(findstring 3, $(KBUILD_EXTRA_WARN)),)
 
 KBUILD_CFLAGS += -Wbad-function-cast
 KBUILD_CFLAGS += -Wcast-qual
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 7ab17712ab24..df83967268ba 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -248,7 +248,7 @@ quiet_cmd_gzip = GZIP    $@
 DTC ?= $(objtree)/scripts/dtc/dtc
 
 # Disable noisy checks by default
-ifeq ($(findstring 1,$(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
+ifeq ($(findstring 1,$(KBUILD_EXTRA_WARN)),)
 DTC_FLAGS += -Wno-unit_address_vs_reg \
 	-Wno-unit_address_format \
 	-Wno-avoid_unnecessary_addr_size \
@@ -259,7 +259,7 @@ DTC_FLAGS += -Wno-unit_address_vs_reg \
 	-Wno-pci_device_reg
 endif
 
-ifneq ($(findstring 2,$(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
+ifneq ($(findstring 2,$(KBUILD_EXTRA_WARN)),)
 DTC_FLAGS += -Wnode_name_chars_strict \
 	-Wproperty_name_chars_strict
 endif
diff --git a/scripts/genksyms/Makefile b/scripts/genksyms/Makefile
index baf44ed0a93a..78629f515e78 100644
--- a/scripts/genksyms/Makefile
+++ b/scripts/genksyms/Makefile
@@ -12,7 +12,7 @@ genksyms-objs	:= genksyms.o parse.tab.o lex.lex.o
 #
 # Just in case, run "$(YACC) --version" without suppressing stderr
 # so that 'bison: not found' will be displayed if it is missing.
-ifeq ($(findstring 1,$(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
+ifeq ($(findstring 1,$(KBUILD_EXTRA_WARN)),)
 
 quiet_cmd_bison_no_warn = $(quiet_cmd_bison)
       cmd_bison_no_warn = $(YACC) --version >/dev/null; \
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH] tools: memory-model: add it to the Documentation body
From: Paul E. McKenney @ 2019-09-01 13:35 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Akira Yokosawa, Alan Stern, Mauro Carvalho Chehab, Joel Fernandes,
	Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
	Jonathan Corbet, Andrea Parri, Will Deacon, Boqun Feng,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Daniel Lustig, Ingo Molnar, Jason Gunthorpe, SeongJae Park,
	linux-arch
In-Reply-To: <20190830113216.GE2369@hirez.programming.kicks-ass.net>

On Fri, Aug 30, 2019 at 01:32:16PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 31, 2019 at 01:25:17PM -0700, Paul E. McKenney wrote:
> > Looks like a pretty clear consensus thus far.  Any objections to keeping
> > these .txt for the time being?
> 
> Obviously I'm a huge proponent of abandoning RST and an advocate for
> normal .txt files ;-)

Oh?  I hadn't noticed.  ;-)

							Thanx, Paul

^ permalink raw reply

* Re: [PATCH] [RFC] i2c: imx: make use of format specifier %dE
From: Oleksij Rempel @ 2019-09-02  5:58 UTC (permalink / raw)
  To: Wolfram Sang, Uwe Kleine-König
  Cc: Oleksij Rempel, kernel, Shawn Guo, Fabio Estevam, NXP Linux Team,
	linux-i2c, linux-arm-kernel, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt, Andrew Morton, Jani Nikula, Enrico Weigelt,
	Jonathan Corbet, linux-doc, linux-kernel
In-Reply-To: <20190829203912.GU3740@ninjato>



On 29.08.19 22:39, Wolfram Sang wrote:
> On Thu, Aug 29, 2019 at 06:29:05AM +0200, Uwe Kleine-König wrote:
>> I created a patch that teaches printk et al to emit a symbolic error
>> name for an error valued integer[1]. With that applied
>>
>> 	dev_err(&pdev->dev, "can't enable I2C clock, ret=%dE\n", ret);
>>
>> emits
>>
>> 	... can't enable I2C clock, ret=EIO
>>
>> if ret is -EIO. Petr Mladek (i.e. one of the printk maintainers) had
>> concerns if this would be well received and worth the effort. He asked
>> to present it to a few subsystems. So for now, this patch converting the
>> i2c-imx driver shouldn't be applied yet but it would be great to get
>> some feedback about if you think that being able to easily printk (for
>> example) "EIO" instead of "-5" is a good idea. Would it help you? Do you
>> think it helps your users?
> 
> Yes, it would help me. And users, too, I am quite sure. For me, if I mix
> up two numbers while debugging, I am hunting ghosts for a while until I
> realize my mistake. So:
> 
> Acked-by: Wolfram Sang <wsa@the-dreams.de>
> 
> I think the main drawback is that ERRORCODES in vsprintf.c now need
> maintenance, but I think it is worth the effort. I'd be interested in
> the overhead in size this causes, but I also think it is worth the
> effort. (It could even be compiled out if we have some generic Kconfig
> symbol for smaller kernels).


I like it, at least it will safe me some time.
I tested this patch together with the vprintf patch, so result looks like:
[    0.281843] imx-i2c 21a0000.i2c: can't enable I2C clock, ret=EIO
[    0.281891] imx-i2c: probe of 21a0000.i2c failed with error -5

Tested-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

Kind regards,
Oleksij Rempel

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH] doc:lock: remove reference to clever use of read-write lock
From: Federico Vaga @ 2019-09-02  7:01 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, linux-kernel, linux-doc
In-Reply-To: <20190831084344.6fd7c039@lwn.net>

On Saturday, August 31, 2019 4:43:44 PM CEST Jonathan Corbet wrote:
> On Sat, 31 Aug 2019 15:41:16 +0200
> 
> Federico Vaga <federico.vaga@vaga.pv.it> wrote:
> >  several CPU's and you want to use spinlocks you can potentially use
> > 
> > -cheaper versions of the spinlocks. IFF you know that the spinlocks are
> > +cheaper versions of the spinlocks. If you know that the spinlocks are
> > 
> >  never used in interrupt handlers, you can use the non-irq versions::
> I suspect that was not actually a typo; "iff" is a way for the
> mathematically inclined to say "if and only if".
> 
> jon

I learned something new today :)

I am not used to the mathematical English jargon. It make sense, but then I 
would replace it with "If and only if": for clarity.


-- 
Federico Vaga




^ permalink raw reply

* Re: [PATCH v4 02/10] KVM: arm/arm64: Factor out hypercall handling from PSCI code
From: kbuild test robot @ 2019-09-02  7:06 UTC (permalink / raw)
  To: Steven Price
  Cc: kbuild-all, Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm,
	Steven Price, Catalin Marinas, Paolo Bonzini,
	Radim =?unknown-8bit?B?S3LEjW3DocWZ?=, Russell King, James Morse,
	Julien Thierry, Suzuki K Pouloze, Mark Rutland, kvm, linux-doc,
	linux-kernel, Christoffer Dall
In-Reply-To: <20190830084255.55113-3-steven.price@arm.com>

[-- Attachment #1: Type: text/plain, Size: 11454 bytes --]

Hi Steven,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc6 next-20190830]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Steven-Price/arm64-Stolen-time-support/20190901-185152
config: i386-randconfig-a002-201935 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-11) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from include/kvm/arm_hypercalls.h:7:0,
                    from <command-line>:0:
>> arch/x86/include/asm/kvm_emulate.h:349:22: error: 'NR_VCPU_REGS' undeclared here (not in a function)
     unsigned long _regs[NR_VCPU_REGS];
                         ^~~~~~~~~~~~
   In file included from <command-line>:0:0:
>> include/kvm/arm_hypercalls.h:9:33: warning: 'struct kvm_vcpu' declared inside parameter list will not be visible outside of this definition or declaration
    int kvm_hvc_call_handler(struct kvm_vcpu *vcpu);
                                    ^~~~~~~~
   include/kvm/arm_hypercalls.h:11:45: warning: 'struct kvm_vcpu' declared inside parameter list will not be visible outside of this definition or declaration
    static inline u32 smccc_get_function(struct kvm_vcpu *vcpu)
                                                ^~~~~~~~
   include/kvm/arm_hypercalls.h: In function 'smccc_get_function':
>> include/kvm/arm_hypercalls.h:13:9: error: implicit declaration of function 'vcpu_get_reg' [-Werror=implicit-function-declaration]
     return vcpu_get_reg(vcpu, 0);
            ^~~~~~~~~~~~
   include/kvm/arm_hypercalls.h: At top level:
   include/kvm/arm_hypercalls.h:16:51: warning: 'struct kvm_vcpu' declared inside parameter list will not be visible outside of this definition or declaration
    static inline unsigned long smccc_get_arg1(struct kvm_vcpu *vcpu)
                                                      ^~~~~~~~
   include/kvm/arm_hypercalls.h:21:51: warning: 'struct kvm_vcpu' declared inside parameter list will not be visible outside of this definition or declaration
    static inline unsigned long smccc_get_arg2(struct kvm_vcpu *vcpu)
                                                      ^~~~~~~~
   include/kvm/arm_hypercalls.h:26:51: warning: 'struct kvm_vcpu' declared inside parameter list will not be visible outside of this definition or declaration
    static inline unsigned long smccc_get_arg3(struct kvm_vcpu *vcpu)
                                                      ^~~~~~~~
   include/kvm/arm_hypercalls.h:31:44: warning: 'struct kvm_vcpu' declared inside parameter list will not be visible outside of this definition or declaration
    static inline void smccc_set_retval(struct kvm_vcpu *vcpu,
                                               ^~~~~~~~
   include/kvm/arm_hypercalls.h: In function 'smccc_set_retval':
>> include/kvm/arm_hypercalls.h:37:2: error: implicit declaration of function 'vcpu_set_reg'; did you mean 'smccc_set_retval'? [-Werror=implicit-function-declaration]
     vcpu_set_reg(vcpu, 0, a0);
     ^~~~~~~~~~~~
     smccc_set_retval
   cc1: some warnings being treated as errors

vim +/NR_VCPU_REGS +349 arch/x86/include/asm/kvm_emulate.h

a584539b24b87d arch/x86/include/asm/kvm_emulate.h     Paolo Bonzini       2015-04-01  290  
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  291  struct x86_emulate_ctxt {
0225fb509d51fc arch/x86/include/asm/kvm_emulate.h     Mathias Krause      2012-08-30  292  	const struct x86_emulate_ops *ops;
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  293  
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  294  	/* Register state before/after emulation. */
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  295  	unsigned long eflags;
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  296  	unsigned long eip; /* eip before instruction emulation */
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  297  	/* Emulated execution mode, represented by an X86EMUL_MODE value. */
9d1b39a967871b arch/x86/include/asm/kvm_emulate.h     Gleb Natapov        2012-09-03  298  	enum x86emul_mode mode;
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  299  
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  300  	/* interruptibility state, as a result of execution of STI or MOV SS */
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  301  	int interruptibility;
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  302  
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  303  	bool perm_ok; /* do not check permissions if true */
b51e974fcdabd0 arch/x86/include/asm/kvm_emulate.h     Borislav Petkov     2013-09-22  304  	bool ud;	/* inject an #UD if host doesn't support insn */
c8401dda2f0a00 arch/x86/include/asm/kvm_emulate.h     Paolo Bonzini       2017-06-07  305  	bool tf;	/* TF value before instruction (after for syscall/sysret) */
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  306  
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  307  	bool have_exception;
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  308  	struct x86_exception exception;
9dac77fa4011bd arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  309  
1ce19dc16ce913 arch/x86/include/asm/kvm_emulate.h     Borislav Petkov     2013-09-22  310  	/*
1ce19dc16ce913 arch/x86/include/asm/kvm_emulate.h     Borislav Petkov     2013-09-22  311  	 * decode cache
1ce19dc16ce913 arch/x86/include/asm/kvm_emulate.h     Borislav Petkov     2013-09-22  312  	 */
1ce19dc16ce913 arch/x86/include/asm/kvm_emulate.h     Borislav Petkov     2013-09-22  313  
1ce19dc16ce913 arch/x86/include/asm/kvm_emulate.h     Borislav Petkov     2013-09-22  314  	/* current opcode length in bytes */
1ce19dc16ce913 arch/x86/include/asm/kvm_emulate.h     Borislav Petkov     2013-09-22  315  	u8 opcode_len;
e4e03deda83b1f drivers/kvm/x86_emulate.h              Laurent Vivier      2007-09-18  316  	u8 b;
c4f035c60dad45 arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-04-04  317  	u8 intercept;
e4e03deda83b1f drivers/kvm/x86_emulate.h              Laurent Vivier      2007-09-18  318  	u8 op_bytes;
e4e03deda83b1f drivers/kvm/x86_emulate.h              Laurent Vivier      2007-09-18  319  	u8 ad_bytes;
e4e03deda83b1f drivers/kvm/x86_emulate.h              Laurent Vivier      2007-09-18  320  	struct operand src;
0dc8d10f7d848b arch/x86/include/asm/kvm_x86_emulate.h Guillaume Thouvenin 2008-12-04  321  	struct operand src2;
e4e03deda83b1f drivers/kvm/x86_emulate.h              Laurent Vivier      2007-09-18  322  	struct operand dst;
ef65c88912cafe arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2010-07-29  323  	int (*execute)(struct x86_emulate_ctxt *ctxt);
d09beabd7cd4cf arch/x86/include/asm/kvm_emulate.h     Joerg Roedel        2011-04-04  324  	int (*check_perm)(struct x86_emulate_ctxt *ctxt);
41061cdb98a0be arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  325  	/*
41061cdb98a0be arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  326  	 * The following six fields are cleared together,
41061cdb98a0be arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  327  	 * the rest are initialized unconditionally in x86_decode_insn
41061cdb98a0be arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  328  	 * or elsewhere
41061cdb98a0be arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  329  	 */
c44b4c6ab80eef arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  330  	bool rip_relative;
c44b4c6ab80eef arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  331  	u8 rex_prefix;
c44b4c6ab80eef arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  332  	u8 lock_prefix;
c44b4c6ab80eef arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  333  	u8 rep_prefix;
c44b4c6ab80eef arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  334  	/* bitmaps of registers in _regs[] that can be read */
c44b4c6ab80eef arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  335  	u32 regs_valid;
c44b4c6ab80eef arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  336  	/* bitmaps of registers in _regs[] that have been written */
c44b4c6ab80eef arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  337  	u32 regs_dirty;
e4e03deda83b1f drivers/kvm/x86_emulate.h              Laurent Vivier      2007-09-18  338  	/* modrm */
e4e03deda83b1f drivers/kvm/x86_emulate.h              Laurent Vivier      2007-09-18  339  	u8 modrm;
e4e03deda83b1f drivers/kvm/x86_emulate.h              Laurent Vivier      2007-09-18  340  	u8 modrm_mod;
e4e03deda83b1f drivers/kvm/x86_emulate.h              Laurent Vivier      2007-09-18  341  	u8 modrm_reg;
e4e03deda83b1f drivers/kvm/x86_emulate.h              Laurent Vivier      2007-09-18  342  	u8 modrm_rm;
09ee57cdae3156 arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2010-08-01  343  	u8 modrm_seg;
573e80fe04db1a arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  344  	u8 seg_override;
c44b4c6ab80eef arch/x86/include/asm/kvm_emulate.h     Bandan Das          2014-04-16  345  	u64 d;
36dd9bb5ce32bc arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-06-01  346  	unsigned long _eip;
cbd27ee783f1e5 arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2012-06-10  347  	struct operand memop;
b5c9ff731f3cee arch/x86/include/asm/kvm_emulate.h     Takuya Yoshikawa    2011-05-25  348  	/* Fields above regs are cleared together. */
dd856efafe6097 arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2012-08-27 @349  	unsigned long _regs[NR_VCPU_REGS];
f09ed83e211d25 arch/x86/include/asm/kvm_emulate.h     Avi Kivity          2011-09-13  350  	struct operand *memopp;
6226686954c4cc drivers/kvm/x86_emulate.h              Avi Kivity          2007-11-20  351  	struct fetch_cache fetch;
7b262e90fc20a4 arch/x86/include/asm/kvm_emulate.h     Gleb Natapov        2010-03-18  352  	struct read_cache io_read;
9de41573675cba arch/x86/include/asm/kvm_emulate.h     Gleb Natapov        2010-04-28  353  	struct read_cache mem_read;
e4e03deda83b1f drivers/kvm/x86_emulate.h              Laurent Vivier      2007-09-18  354  };
e4e03deda83b1f drivers/kvm/x86_emulate.h              Laurent Vivier      2007-09-18  355  

:::::: The code at line 349 was first introduced by commit
:::::: dd856efafe6097a5c9104725c2bca74430423db8 KVM: x86 emulator: access GPRs on demand

:::::: TO: Avi Kivity <avi@redhat.com>
:::::: CC: Marcelo Tosatti <mtosatti@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30130 bytes --]

^ permalink raw reply

* Re: [PATCH] nvmem: core: add nvmem_device_find
From: Thomas Bogendoerfer @ 2019-09-02  8:50 UTC (permalink / raw)
  To: Jonathan Corbet, Srinivas Kandagatla, linux-doc, linux-kernel
In-Reply-To: <20190826130829.21073-1-tbogendoerfer@suse.de>

On Mon, Aug 26, 2019 at 03:08:28PM +0200, Thomas Bogendoerfer wrote:
> nvmem_device_find provides a way to search for nvmem devices with
> the help of a match function simlair to bus_find_device.
> 
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> ---
>  Documentation/driver-api/nvmem.rst |  2 ++
>  drivers/nvmem/core.c               | 61 +++++++++++++++++---------------------

Any chance to get this into 5.4 ?

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

^ permalink raw reply

* Re: [PATCH v2 1/1] counter: cros_ec: Add synchronization sensor
From: Lee Jones @ 2019-09-02  9:40 UTC (permalink / raw)
  To: Fabien Lahoudere
  Cc: gwendal, egranata, kernel, William Breathitt Gray,
	Jonathan Corbet, Benson Leung, Enric Balletbo i Serra,
	Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Mauro Carvalho Chehab,
	David S. Miller, Greg Kroah-Hartman, Nicolas Ferre, Nick Vaccaro,
	linux-iio, linux-doc, linux-kernel
In-Reply-To: <d985a8a811996148e8cda78b9fe47bb87b884b56.1566563833.git.fabien.lahoudere@collabora.com>

On Fri, 23 Aug 2019, Fabien Lahoudere wrote:

> From: Gwendal Grignou <gwendal@chromium.org>
> 
> EC returns a counter when there is an event on camera vsync.
> This patch comes from chromeos kernel 4.4
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.com>
> 
> CROS EC sync sensor was originally designed as an IIO device.
> Now that the counter subsystem will replace IIO_COUNTER, we
> have to implement a new way to get sync count.
> 
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.com>
> ---
>  Documentation/driver-api/generic-counter.rst  |   3 +
>  MAINTAINERS                                   |   7 +
>  drivers/counter/Kconfig                       |   9 +
>  drivers/counter/Makefile                      |   1 +
>  drivers/counter/counter.c                     |   2 +
>  drivers/counter/cros_ec_sensors_sync.c        | 208 ++++++++++++++++++
>  .../cros_ec_sensors/cros_ec_sensors_core.c    |   1 +

>  drivers/mfd/cros_ec_dev.c                     |   3 +

I can't see any reason for this change to be squashed into this patch.

Please separate it out.

>  include/linux/counter.h                       |   1 +
>  9 files changed, 235 insertions(+)
>  create mode 100644 drivers/counter/cros_ec_sensors_sync.c

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply


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