Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 14/14] KVM: arm64: Invoke FPSIMD context switch trap from C
From: Dave Martin @ 2018-05-04 16:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525449935-31424-1-git-send-email-Dave.Martin@arm.com>

The conversion of the FPSIMD context switch trap code to C has added
some overhead to calling it, due to the need to save registers that
the procedure call standard defines as caller-saved.

So, perhaps it is no longer worth invoking this trap handler quite
so early.

Instead, we can invoke it from fixup_guest_exit(), with little
likelihood of increasing the overhead much further.

As a convenience, this patch gives __hyp_switch_fpsimd() the same
return semantics fixup_guest_exit().  For now there is no
possibility of a spurious FPSIMD trap, so the function always
returns true, but this allows it to be tail-called with a single
return statement.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/kvm/hyp/entry.S     | 30 ------------------------------
 arch/arm64/kvm/hyp/hyp-entry.S | 19 -------------------
 arch/arm64/kvm/hyp/switch.c    | 15 +++++++++++++--
 3 files changed, 13 insertions(+), 51 deletions(-)

diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
index 40f349b..fad1e16 100644
--- a/arch/arm64/kvm/hyp/entry.S
+++ b/arch/arm64/kvm/hyp/entry.S
@@ -166,33 +166,3 @@ abort_guest_exit_end:
 	orr	x0, x0, x5
 1:	ret
 ENDPROC(__guest_exit)
-
-ENTRY(__fpsimd_guest_restore)
-	// x0: esr
-	// x1: vcpu
-	// x2-x29,lr: vcpu regs
-	// vcpu x0-x1 on the stack
-	stp	x2, x3, [sp, #-144]!
-	stp	x4, x5, [sp, #16]
-	stp	x6, x7, [sp, #32]
-	stp	x8, x9, [sp, #48]
-	stp	x10, x11, [sp, #64]
-	stp	x12, x13, [sp, #80]
-	stp	x14, x15, [sp, #96]
-	stp	x16, x17, [sp, #112]
-	stp	x18, lr, [sp, #128]
-
-	bl	__hyp_switch_fpsimd
-
-	ldp	x4, x5, [sp, #16]
-	ldp	x6, x7, [sp, #32]
-	ldp	x8, x9, [sp, #48]
-	ldp	x10, x11, [sp, #64]
-	ldp	x12, x13, [sp, #80]
-	ldp	x14, x15, [sp, #96]
-	ldp	x16, x17, [sp, #112]
-	ldp	x18, lr, [sp, #128]
-	ldp	x0, x1, [sp, #144]
-	ldp	x2, x3, [sp], #160
-	eret
-ENDPROC(__fpsimd_guest_restore)
diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
index bffece2..753b9d2 100644
--- a/arch/arm64/kvm/hyp/hyp-entry.S
+++ b/arch/arm64/kvm/hyp/hyp-entry.S
@@ -113,25 +113,6 @@ el1_hvc_guest:
 
 el1_trap:
 	get_vcpu_ptr	x1, x0
-
-	mrs		x0, esr_el2
-	lsr		x0, x0, #ESR_ELx_EC_SHIFT
-	/*
-	 * x0: ESR_EC
-	 * x1: vcpu pointer
-	 */
-
-	/*
-	 * We trap the first access to the FP/SIMD to save the host context
-	 * and restore the guest context lazily.
-	 * If FP/SIMD is not implemented, handle the trap and inject an
-	 * undefined instruction exception to the guest.
-	 */
-alternative_if_not ARM64_HAS_NO_FPSIMD
-	cmp	x0, #ESR_ELx_EC_FP_ASIMD
-	b.eq	__fpsimd_guest_restore
-alternative_else_nop_endif
-
 	mov	x0, #ARM_EXCEPTION_TRAP
 	b	__guest_exit
 
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index be09c52..03d132b 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -327,8 +327,7 @@ static bool __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
 	}
 }
 
-void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
-				    struct kvm_vcpu *vcpu)
+static bool __hyp_text __hyp_switch_fpsimd(struct kvm_vcpu *vcpu)
 {
 	struct user_fpsimd_state *host_fpsimd = vcpu->arch.host_fpsimd_state;
 
@@ -367,6 +366,8 @@ void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
 			     fpexc32_el2);
 
 	vcpu->arch.fp_enabled = true;
+
+	return true;
 }
 
 /*
@@ -388,6 +389,16 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 	if (*exit_code != ARM_EXCEPTION_TRAP)
 		goto exit;
 
+	/*
+	 * We trap the first access to the FP/SIMD to save the host context
+	 * and restore the guest context lazily.
+	 * If FP/SIMD is not implemented, handle the trap and inject an
+	 * undefined instruction exception to the guest.
+	 */
+	if (system_supports_fpsimd() &&
+	    kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_FP_ASIMD)
+		return __hyp_switch_fpsimd(vcpu);
+
 	if (!__populate_fault_info(vcpu))
 		return true;
 
-- 
2.1.4

^ permalink raw reply related

* [PATCH v8 0/6] optimize memblock_next_valid_pfn and early_pfn_valid on arm and arm64
From: Daniel Vacek @ 2018-05-04 16:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <05b0fcf2-7670-101e-d4ab-1f656ff6b02f@gmail.com>

On Fri, May 4, 2018 at 4:45 AM, Jia He <hejianet@gmail.com> wrote:
> Ping
>
> Sorry if I am a little bit verbose, but it can speedup the arm64 booting
> time indeed.

I'm wondering, ain't simple enabling of config
DEFERRED_STRUCT_PAGE_INIT provide even better speed-up? If that is the
case then it seems like this series is not needed at all, right?
I am not sure why is this config optional. It looks like it could be
enabled by default or even unconditionally considering that with
commit c9e97a1997fb ("mm: initialize pages on demand during boot") the
deferred code is statically disabled after all the pages are
initialized.

--nX

>
> --
> Cheers,
> Jia
>
>
> On 4/11/2018 3:21 PM, Jia He Wrote:
>>
>> Commit b92df1de5d28 ("mm: page_alloc: skip over regions of invalid pfns
>> where possible") tried to optimize the loop in memmap_init_zone(). But
>> there is still some room for improvement.
>>
>> Patch 1 introduce new config to make codes more generic
>> Patch 2 remain the memblock_next_valid_pfn on arm and arm64
>> Patch 3 optimizes the memblock_next_valid_pfn()
>> Patch 4~6 optimizes the early_pfn_valid()
>>
>> As for the performance improvement, after this set, I can see the time
>> overhead of memmap_init() is reduced from 41313 us to 24389 us in my
>> armv8a server(QDF2400 with 96G memory).
>>
>> Without this patchset:
>> [  117.113677] before memmap_init
>> [  117.118195] after  memmap_init
>>>>>
>>>>> memmap_init takes 4518 us
>>
>> [  117.121446] before memmap_init
>> [  117.154992] after  memmap_init
>>>>>
>>>>> memmap_init takes 33546 us
>>
>> [  117.158241] before memmap_init
>> [  117.161490] after  memmap_init
>>>>>
>>>>> memmap_init takes 3249 us
>>>>> totally takes 41313 us
>>
>> With this patchset:
>> [  123.222962] before memmap_init
>> [  123.226819] after  memmap_init
>>>>>
>>>>> memmap_init takes 3857
>>
>> [  123.230070] before memmap_init
>> [  123.247354] after  memmap_init
>>>>>
>>>>> memmap_init takes 17284
>>
>> [  123.250604] before memmap_init
>> [  123.253852] after  memmap_init
>>>>>
>>>>> memmap_init takes 3248
>>>>> totally takes 24389 us
>>
>> Attached the memblock region information in my server.
>> [   86.956758] Zone ranges:
>> [   86.959452]   DMA      [mem 0x0000000000200000-0x00000000ffffffff]
>> [   86.966041]   Normal   [mem 0x0000000100000000-0x00000017ffffffff]
>> [   86.972631] Movable zone start for each node
>> [   86.977179] Early memory node ranges
>> [   86.980985]   node   0: [mem 0x0000000000200000-0x000000000021ffff]
>> [   86.987666]   node   0: [mem 0x0000000000820000-0x000000000307ffff]
>> [   86.994348]   node   0: [mem 0x0000000003080000-0x000000000308ffff]
>> [   87.001029]   node   0: [mem 0x0000000003090000-0x00000000031fffff]
>> [   87.007710]   node   0: [mem 0x0000000003200000-0x00000000033fffff]
>> [   87.014392]   node   0: [mem 0x0000000003410000-0x000000000563ffff]
>> [   87.021073]   node   0: [mem 0x0000000005640000-0x000000000567ffff]
>> [   87.027754]   node   0: [mem 0x0000000005680000-0x00000000056dffff]
>> [   87.034435]   node   0: [mem 0x00000000056e0000-0x00000000086fffff]
>> [   87.041117]   node   0: [mem 0x0000000008700000-0x000000000871ffff]
>> [   87.047798]   node   0: [mem 0x0000000008720000-0x000000000894ffff]
>> [   87.054479]   node   0: [mem 0x0000000008950000-0x0000000008baffff]
>> [   87.061161]   node   0: [mem 0x0000000008bb0000-0x0000000008bcffff]
>> [   87.067842]   node   0: [mem 0x0000000008bd0000-0x0000000008c4ffff]
>> [   87.074524]   node   0: [mem 0x0000000008c50000-0x0000000008e2ffff]
>> [   87.081205]   node   0: [mem 0x0000000008e30000-0x0000000008e4ffff]
>> [   87.087886]   node   0: [mem 0x0000000008e50000-0x0000000008fcffff]
>> [   87.094568]   node   0: [mem 0x0000000008fd0000-0x000000000910ffff]
>> [   87.101249]   node   0: [mem 0x0000000009110000-0x00000000092effff]
>> [   87.107930]   node   0: [mem 0x00000000092f0000-0x000000000930ffff]
>> [   87.114612]   node   0: [mem 0x0000000009310000-0x000000000963ffff]
>> [   87.121293]   node   0: [mem 0x0000000009640000-0x000000000e61ffff]
>> [   87.127975]   node   0: [mem 0x000000000e620000-0x000000000e64ffff]
>> [   87.134657]   node   0: [mem 0x000000000e650000-0x000000000fffffff]
>> [   87.141338]   node   0: [mem 0x0000000010800000-0x0000000017feffff]
>> [   87.148019]   node   0: [mem 0x000000001c000000-0x000000001c00ffff]
>> [   87.154701]   node   0: [mem 0x000000001c010000-0x000000001c7fffff]
>> [   87.161383]   node   0: [mem 0x000000001c810000-0x000000007efbffff]
>> [   87.168064]   node   0: [mem 0x000000007efc0000-0x000000007efdffff]
>> [   87.174746]   node   0: [mem 0x000000007efe0000-0x000000007efeffff]
>> [   87.181427]   node   0: [mem 0x000000007eff0000-0x000000007effffff]
>> [   87.188108]   node   0: [mem 0x000000007f000000-0x00000017ffffffff]
>> [   87.194791] Initmem setup node 0 [mem
>> 0x0000000000200000-0x00000017ffffffff]
>>
>> Changelog:
>> V8: - introduce new config and move generic code to early_pfn.h
>>      - optimize memblock_next_valid_pfn as suggested by Matthew Wilcox
>> V7: - fix i386 compilation error. refine the commit description
>> V6: - simplify the codes, move arm/arm64 common codes to one file.
>>      - refine patches as suggested by Danial Vacek and Ard Biesheuvel
>> V5: - further refining as suggested by Danial Vacek. Make codes
>>        arm/arm64 more arch specific
>> V4: - refine patches as suggested by Danial Vacek and Wei Yang
>>      - optimized on arm besides arm64
>> V3: - fix 2 issues reported by kbuild test robot
>> V2: - rebase to mmotm latest
>>      - remain memblock_next_valid_pfn on arm64
>>      - refine memblock_search_pfn_regions and pfn_valid_region
>>
>> Jia He (6):
>>    arm: arm64: introduce CONFIG_HAVE_MEMBLOCK_PFN_VALID
>>    mm: page_alloc: remain memblock_next_valid_pfn() on arm/arm64
>>    arm: arm64: page_alloc: reduce unnecessary binary search in
>>      memblock_next_valid_pfn()
>>    mm/memblock: introduce memblock_search_pfn_regions()
>>    arm: arm64: introduce pfn_valid_region()
>>    mm: page_alloc: reduce unnecessary binary search in early_pfn_valid()
>>
>>   arch/arm/Kconfig          |  4 +++
>>   arch/arm/mm/init.c        |  1 +
>>   arch/arm64/Kconfig        |  4 +++
>>   arch/arm64/mm/init.c      |  1 +
>>   include/linux/early_pfn.h | 79
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/memblock.h  |  2 ++
>>   include/linux/mmzone.h    | 18 ++++++++++-
>>   mm/Kconfig                |  3 ++
>>   mm/memblock.c             |  9 ++++++
>>   mm/page_alloc.c           |  5 ++-
>>   10 files changed, 124 insertions(+), 2 deletions(-)
>>   create mode 100644 include/linux/early_pfn.h
>>
>

^ permalink raw reply

* [PATCH 1/2] ARM: dts: BCM5301X: Add DT for Luxul XWR-3150 V1
From: Dan Haab @ 2018-05-04 16:08 UTC (permalink / raw)
  To: linux-arm-kernel

Luxul XWR-3150 is a wireless router similar to the XWR-3100 except:
1) It has more RAM
2) Its NAND controller in running in BCH8 mode
3) LAN ports LEDs are hardware controlled

Signed-off-by: Dan Haab <dan.haab@luxul.com>
---
 arch/arm/boot/dts/Makefile                       |  1 +
 arch/arm/boot/dts/bcm47094-luxul-xwr-3150-v1.dts | 77 ++++++++++++++++++++++++
 2 files changed, 78 insertions(+)
 create mode 100644 arch/arm/boot/dts/bcm47094-luxul-xwr-3150-v1.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 7e24249..a00b279 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -104,6 +104,7 @@ dtb-$(CONFIG_ARCH_BCM_5301X) += \
 	bcm47094-luxul-abr-4500.dtb \
 	bcm47094-luxul-xbr-4500.dtb \
 	bcm47094-luxul-xwr-3100.dtb \
+	bcm47094-luxul-xwr-3150-v1.dtb \
 	bcm47094-netgear-r8500.dtb \
 	bcm94708.dtb \
 	bcm94709.dtb \
diff --git a/arch/arm/boot/dts/bcm47094-luxul-xwr-3150-v1.dts b/arch/arm/boot/dts/bcm47094-luxul-xwr-3150-v1.dts
new file mode 100644
index 0000000..bdad726
--- /dev/null
+++ b/arch/arm/boot/dts/bcm47094-luxul-xwr-3150-v1.dts
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Copyright 2018 Luxul Inc.
+ */
+
+/dts-v1/;
+
+#include "bcm47094.dtsi"
+#include "bcm5301x-nand-cs0-bch8.dtsi"
+
+/ {
+	compatible = "luxul,xwr-3150-v1", "brcm,bcm47094", "brcm,bcm4708";
+	model = "Luxul XWR-3150 V1";
+
+	chosen {
+		bootargs = "earlycon";
+	};
+
+	memory {
+		reg = <0x00000000 0x08000000
+		       0x88000000 0x18000000>;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		power	{
+			label = "bcm53xx:green:power";
+			gpios = <&chipcommon 0 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "default-on";
+		};
+
+		usb3	{
+			label = "bcm53xx:green:usb3";
+			gpios = <&chipcommon 8 GPIO_ACTIVE_LOW>;
+			trigger-sources = <&ohci_port1>, <&ehci_port1>,
+					  <&xhci_port1>;
+			linux,default-trigger = "usbport";
+		};
+
+		status	{
+			label = "bcm53xx:green:status";
+			gpios = <&chipcommon 10 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "timer";
+		};
+
+		2ghz {
+			label = "bcm53xx:green:2ghz";
+			gpios = <&chipcommon 13 GPIO_ACTIVE_LOW>;
+		};
+
+		5ghz {
+			label = "bcm53xx:green:5ghz";
+			gpios = <&chipcommon 14 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		restart {
+			label = "Reset";
+			linux,code = <KEY_RESTART>;
+			gpios = <&chipcommon 17 GPIO_ACTIVE_LOW>;
+		};
+	};
+};
+
+&usb3 {
+	vcc-gpio = <&chipcommon 18 GPIO_ACTIVE_HIGH>;
+};
+
+&spi_nor {
+	status = "okay";
+};
-- 
1.9.1

^ permalink raw reply related

* [PATCH 2/2] ARM: dts: BCM5301X: Add DT for Luxul XAP-1610
From: Dan Haab @ 2018-05-04 16:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525450124-21665-1-git-send-email-dan.haab@luxul.com>

It's an access point based on BCM47094 SoC with two BCM4366E wireless
chipsets.

Signed-off-by: Dan Haab <dan.haab@luxul.com>
---
 arch/arm/boot/dts/Makefile                    |  1 +
 arch/arm/boot/dts/bcm47094-luxul-xap-1610.dts | 57 +++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)
 create mode 100644 arch/arm/boot/dts/bcm47094-luxul-xap-1610.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index a00b279..d041508 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -102,6 +102,7 @@ dtb-$(CONFIG_ARCH_BCM_5301X) += \
 	bcm47094-dlink-dir-885l.dtb \
 	bcm47094-linksys-panamera.dtb \
 	bcm47094-luxul-abr-4500.dtb \
+	bcm47094-luxul-xap-1610.dtb \
 	bcm47094-luxul-xbr-4500.dtb \
 	bcm47094-luxul-xwr-3100.dtb \
 	bcm47094-luxul-xwr-3150-v1.dtb \
diff --git a/arch/arm/boot/dts/bcm47094-luxul-xap-1610.dts b/arch/arm/boot/dts/bcm47094-luxul-xap-1610.dts
new file mode 100644
index 0000000..7fd8547
--- /dev/null
+++ b/arch/arm/boot/dts/bcm47094-luxul-xap-1610.dts
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Copyright 2018 Luxul Inc.
+ */
+
+/dts-v1/;
+
+#include "bcm47094.dtsi"
+
+/ {
+	compatible = "luxul,xap-1610-v1", "brcm,bcm47094", "brcm,bcm4708";
+	model = "Luxul XAP-1610 V1";
+
+	chosen {
+		bootargs = "earlycon";
+	};
+
+	memory {
+		reg = <0x00000000 0x08000000>;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		status	{
+			label = "bcm53xx:green:status";
+			gpios = <&chipcommon 0 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "timer";
+		};
+
+		2ghz {
+			label = "bcm53xx:blue:2ghz";
+			gpios = <&chipcommon 13 GPIO_ACTIVE_LOW>;
+		};
+
+		5ghz {
+			label = "bcm53xx:blue:5ghz";
+			gpios = <&chipcommon 14 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		restart {
+			label = "Reset";
+			linux,code = <KEY_RESTART>;
+			gpios = <&chipcommon 17 GPIO_ACTIVE_LOW>;
+		};
+	};
+};
+
+&spi_nor {
+	status = "okay";
+};
-- 
1.9.1

^ permalink raw reply related

* [GIT PULL 0/4] KVM/arm fixes for 4.17-rc4
From: Marc Zyngier @ 2018-05-04 16:15 UTC (permalink / raw)
  To: linux-arm-kernel

Paolo, Radim,

Here is the second batch of KVM/arm fixes for 4.17, mostly fixing a
few fallouts from the merge window.

Please pull.

	M.

The following changes since commit 85bd0ba1ff9875798fad94218b627ea9f768f3c3:

  arm/arm64: KVM: Add PSCI version selection API (2018-04-20 16:32:23 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git tags/kvmarm-fixes-for-4.17-2

for you to fetch changes up to b220244d41798c6592e7d17843256eb0bae456a0:

  arm64: vgic-v2: Fix proxying of cpuif access (2018-05-04 16:45:55 +0100)

----------------------------------------------------------------
KVM/arm fixes for 4.17, take #2

- Fix proxying of GICv2 CPU interface accesses
- Fix crash when switching to BE
- Track source vcpu git GICv2 SGIs
- Fix an outdated bit of documentation

----------------------------------------------------------------
James Morse (2):
      KVM: arm64: Fix order of vcpu_write_sys_reg() arguments
      arm64: vgic-v2: Fix proxying of cpuif access

Marc Zyngier (1):
      KVM: arm/arm64: vgic: Fix source vcpu issues for GICv2 SGI

Valentin Schneider (1):
      KVM: arm/arm64: vgic_init: Cleanup reference to process_maintenance

 arch/arm64/include/asm/kvm_emulate.h     |  2 +-
 arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c | 24 ++++++++++++----
 include/kvm/arm_vgic.h                   |  1 +
 virt/kvm/arm/vgic/vgic-init.c            |  2 +-
 virt/kvm/arm/vgic/vgic-mmio.c            | 10 +++++--
 virt/kvm/arm/vgic/vgic-v2.c              | 38 ++++++++++++++-----------
 virt/kvm/arm/vgic/vgic-v3.c              | 49 +++++++++++++++++++-------------
 virt/kvm/arm/vgic/vgic.c                 | 30 +++++--------------
 virt/kvm/arm/vgic/vgic.h                 | 14 +++++++++
 9 files changed, 102 insertions(+), 68 deletions(-)

^ permalink raw reply

* [PATCH 1/4] KVM: arm/arm64: vgic: Fix source vcpu issues for GICv2 SGI
From: Marc Zyngier @ 2018-05-04 16:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504161559.22852-1-marc.zyngier@arm.com>

Now that we make sure we don't inject multiple instances of the
same GICv2 SGI at the same time, we've made another bug more
obvious:

If we exit with an active SGI, we completely lose track of which
vcpu it came from. On the next entry, we restore it with 0 as a
source, and if that wasn't the right one, too bad. While this
doesn't seem to trouble GIC-400, the architectural model gets
offended and doesn't deactivate the interrupt on EOI.

Another connected issue is that we will happilly make pending
an interrupt from another vcpu, overriding the above zero with
something that is just as inconsistent. Don't do that.

The final issue is that we signal a maintenance interrupt when
no pending interrupts are present in the LR. Assuming we've fixed
the two issues above, we end-up in a situation where we keep
exiting as soon as we've reached the active state, and not be
able to inject the following pending.

The fix comes in 3 parts:
- GICv2 SGIs have their source vcpu saved if they are active on
  exit, and restored on entry
- Multi-SGIs cannot go via the Pending+Active state, as this would
  corrupt the source field
- Multi-SGIs are converted to using MI on EOI instead of NPIE

Fixes: 16ca6a607d84bef0 ("KVM: arm/arm64: vgic: Don't populate multiple LRs with the same vintid")
Reported-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/kvm/arm_vgic.h        |  1 +
 virt/kvm/arm/vgic/vgic-mmio.c | 10 +++++++--
 virt/kvm/arm/vgic/vgic-v2.c   | 38 +++++++++++++++++++--------------
 virt/kvm/arm/vgic/vgic-v3.c   | 49 +++++++++++++++++++++++++------------------
 virt/kvm/arm/vgic/vgic.c      | 30 +++++++-------------------
 virt/kvm/arm/vgic/vgic.h      | 14 +++++++++++++
 6 files changed, 81 insertions(+), 61 deletions(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 24f03941ada8..e7efe12a81bd 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -131,6 +131,7 @@ struct vgic_irq {
 		u32 mpidr;			/* GICv3 target VCPU */
 	};
 	u8 source;			/* GICv2 SGIs only */
+	u8 active_source;		/* GICv2 SGIs only */
 	u8 priority;
 	enum vgic_irq_config config;	/* Level or edge */
 
diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
index dbe99d635c80..ff9655cfeb2f 100644
--- a/virt/kvm/arm/vgic/vgic-mmio.c
+++ b/virt/kvm/arm/vgic/vgic-mmio.c
@@ -289,10 +289,16 @@ static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
 	       irq->vcpu->cpu != -1) /* VCPU thread is running */
 		cond_resched_lock(&irq->irq_lock);
 
-	if (irq->hw)
+	if (irq->hw) {
 		vgic_hw_irq_change_active(vcpu, irq, active, !requester_vcpu);
-	else
+	} else {
+		u32 model = vcpu->kvm->arch.vgic.vgic_model;
+
 		irq->active = active;
+		if (model == KVM_DEV_TYPE_ARM_VGIC_V2 &&
+		    active && vgic_irq_is_sgi(irq->intid))
+			irq->active_source = requester_vcpu->vcpu_id;
+	}
 
 	if (irq->active)
 		vgic_queue_irq_unlock(vcpu->kvm, irq, flags);
diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c
index 45aa433f018f..a5f2e44f1c33 100644
--- a/virt/kvm/arm/vgic/vgic-v2.c
+++ b/virt/kvm/arm/vgic/vgic-v2.c
@@ -37,13 +37,6 @@ void vgic_v2_init_lrs(void)
 		vgic_v2_write_lr(i, 0);
 }
 
-void vgic_v2_set_npie(struct kvm_vcpu *vcpu)
-{
-	struct vgic_v2_cpu_if *cpuif = &vcpu->arch.vgic_cpu.vgic_v2;
-
-	cpuif->vgic_hcr |= GICH_HCR_NPIE;
-}
-
 void vgic_v2_set_underflow(struct kvm_vcpu *vcpu)
 {
 	struct vgic_v2_cpu_if *cpuif = &vcpu->arch.vgic_cpu.vgic_v2;
@@ -71,13 +64,18 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
 	int lr;
 	unsigned long flags;
 
-	cpuif->vgic_hcr &= ~(GICH_HCR_UIE | GICH_HCR_NPIE);
+	cpuif->vgic_hcr &= ~GICH_HCR_UIE;
 
 	for (lr = 0; lr < vgic_cpu->used_lrs; lr++) {
 		u32 val = cpuif->vgic_lr[lr];
-		u32 intid = val & GICH_LR_VIRTUALID;
+		u32 cpuid, intid = val & GICH_LR_VIRTUALID;
 		struct vgic_irq *irq;
 
+		/* Extract the source vCPU id from the LR */
+		cpuid = val & GICH_LR_PHYSID_CPUID;
+		cpuid >>= GICH_LR_PHYSID_CPUID_SHIFT;
+		cpuid &= 7;
+
 		/* Notify fds when the guest EOI'ed a level-triggered SPI */
 		if (lr_signals_eoi_mi(val) && vgic_valid_spi(vcpu->kvm, intid))
 			kvm_notify_acked_irq(vcpu->kvm, 0,
@@ -90,17 +88,16 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
 		/* Always preserve the active bit */
 		irq->active = !!(val & GICH_LR_ACTIVE_BIT);
 
+		if (irq->active && vgic_irq_is_sgi(intid))
+			irq->active_source = cpuid;
+
 		/* Edge is the only case where we preserve the pending bit */
 		if (irq->config == VGIC_CONFIG_EDGE &&
 		    (val & GICH_LR_PENDING_BIT)) {
 			irq->pending_latch = true;
 
-			if (vgic_irq_is_sgi(intid)) {
-				u32 cpuid = val & GICH_LR_PHYSID_CPUID;
-
-				cpuid >>= GICH_LR_PHYSID_CPUID_SHIFT;
+			if (vgic_irq_is_sgi(intid))
 				irq->source |= (1 << cpuid);
-			}
 		}
 
 		/*
@@ -152,8 +149,15 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
 	u32 val = irq->intid;
 	bool allow_pending = true;
 
-	if (irq->active)
+	if (irq->active) {
 		val |= GICH_LR_ACTIVE_BIT;
+		if (vgic_irq_is_sgi(irq->intid))
+			val |= irq->active_source << GICH_LR_PHYSID_CPUID_SHIFT;
+		if (vgic_irq_is_multi_sgi(irq)) {
+			allow_pending = false;
+			val |= GICH_LR_EOI;
+		}
+	}
 
 	if (irq->hw) {
 		val |= GICH_LR_HW;
@@ -190,8 +194,10 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
 			BUG_ON(!src);
 			val |= (src - 1) << GICH_LR_PHYSID_CPUID_SHIFT;
 			irq->source &= ~(1 << (src - 1));
-			if (irq->source)
+			if (irq->source) {
 				irq->pending_latch = true;
+				val |= GICH_LR_EOI;
+			}
 		}
 	}
 
diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index 8195f52ae6f0..c7423f3768e5 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -27,13 +27,6 @@ static bool group1_trap;
 static bool common_trap;
 static bool gicv4_enable;
 
-void vgic_v3_set_npie(struct kvm_vcpu *vcpu)
-{
-	struct vgic_v3_cpu_if *cpuif = &vcpu->arch.vgic_cpu.vgic_v3;
-
-	cpuif->vgic_hcr |= ICH_HCR_NPIE;
-}
-
 void vgic_v3_set_underflow(struct kvm_vcpu *vcpu)
 {
 	struct vgic_v3_cpu_if *cpuif = &vcpu->arch.vgic_cpu.vgic_v3;
@@ -55,17 +48,23 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 	int lr;
 	unsigned long flags;
 
-	cpuif->vgic_hcr &= ~(ICH_HCR_UIE | ICH_HCR_NPIE);
+	cpuif->vgic_hcr &= ~ICH_HCR_UIE;
 
 	for (lr = 0; lr < vgic_cpu->used_lrs; lr++) {
 		u64 val = cpuif->vgic_lr[lr];
-		u32 intid;
+		u32 intid, cpuid;
 		struct vgic_irq *irq;
+		bool is_v2_sgi = false;
 
-		if (model == KVM_DEV_TYPE_ARM_VGIC_V3)
+		cpuid = val & GICH_LR_PHYSID_CPUID;
+		cpuid >>= GICH_LR_PHYSID_CPUID_SHIFT;
+
+		if (model == KVM_DEV_TYPE_ARM_VGIC_V3) {
 			intid = val & ICH_LR_VIRTUAL_ID_MASK;
-		else
+		} else {
 			intid = val & GICH_LR_VIRTUALID;
+			is_v2_sgi = vgic_irq_is_sgi(intid);
+		}
 
 		/* Notify fds when the guest EOI'ed a level-triggered IRQ */
 		if (lr_signals_eoi_mi(val) && vgic_valid_spi(vcpu->kvm, intid))
@@ -81,18 +80,16 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 		/* Always preserve the active bit */
 		irq->active = !!(val & ICH_LR_ACTIVE_BIT);
 
+		if (irq->active && is_v2_sgi)
+			irq->active_source = cpuid;
+
 		/* Edge is the only case where we preserve the pending bit */
 		if (irq->config == VGIC_CONFIG_EDGE &&
 		    (val & ICH_LR_PENDING_BIT)) {
 			irq->pending_latch = true;
 
-			if (vgic_irq_is_sgi(intid) &&
-			    model == KVM_DEV_TYPE_ARM_VGIC_V2) {
-				u32 cpuid = val & GICH_LR_PHYSID_CPUID;
-
-				cpuid >>= GICH_LR_PHYSID_CPUID_SHIFT;
+			if (is_v2_sgi)
 				irq->source |= (1 << cpuid);
-			}
 		}
 
 		/*
@@ -133,10 +130,20 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
 {
 	u32 model = vcpu->kvm->arch.vgic.vgic_model;
 	u64 val = irq->intid;
-	bool allow_pending = true;
+	bool allow_pending = true, is_v2_sgi;
 
-	if (irq->active)
+	is_v2_sgi = (vgic_irq_is_sgi(irq->intid) &&
+		     model == KVM_DEV_TYPE_ARM_VGIC_V2);
+
+	if (irq->active) {
 		val |= ICH_LR_ACTIVE_BIT;
+		if (is_v2_sgi)
+			val |= irq->active_source << GICH_LR_PHYSID_CPUID_SHIFT;
+		if (vgic_irq_is_multi_sgi(irq)) {
+			allow_pending = false;
+			val |= ICH_LR_EOI;
+		}
+	}
 
 	if (irq->hw) {
 		val |= ICH_LR_HW;
@@ -174,8 +181,10 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
 			BUG_ON(!src);
 			val |= (src - 1) << GICH_LR_PHYSID_CPUID_SHIFT;
 			irq->source &= ~(1 << (src - 1));
-			if (irq->source)
+			if (irq->source) {
 				irq->pending_latch = true;
+				val |= ICH_LR_EOI;
+			}
 		}
 	}
 
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index 4b6d72939c42..568c65f852e1 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -719,14 +719,6 @@ static inline void vgic_set_underflow(struct kvm_vcpu *vcpu)
 		vgic_v3_set_underflow(vcpu);
 }
 
-static inline void vgic_set_npie(struct kvm_vcpu *vcpu)
-{
-	if (kvm_vgic_global_state.type == VGIC_V2)
-		vgic_v2_set_npie(vcpu);
-	else
-		vgic_v3_set_npie(vcpu);
-}
-
 /* Requires the ap_list_lock to be held. */
 static int compute_ap_list_depth(struct kvm_vcpu *vcpu,
 				 bool *multi_sgi)
@@ -740,17 +732,15 @@ static int compute_ap_list_depth(struct kvm_vcpu *vcpu,
 	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&vgic_cpu->ap_list_lock));
 
 	list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) {
+		int w;
+
 		spin_lock(&irq->irq_lock);
 		/* GICv2 SGIs can count for more than one... */
-		if (vgic_irq_is_sgi(irq->intid) && irq->source) {
-			int w = hweight8(irq->source);
-
-			count += w;
-			*multi_sgi |= (w > 1);
-		} else {
-			count++;
-		}
+		w = vgic_irq_get_lr_count(irq);
 		spin_unlock(&irq->irq_lock);
+
+		count += w;
+		*multi_sgi |= (w > 1);
 	}
 	return count;
 }
@@ -761,7 +751,6 @@ static void vgic_flush_lr_state(struct kvm_vcpu *vcpu)
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 	struct vgic_irq *irq;
 	int count;
-	bool npie = false;
 	bool multi_sgi;
 	u8 prio = 0xff;
 
@@ -791,10 +780,8 @@ static void vgic_flush_lr_state(struct kvm_vcpu *vcpu)
 		if (likely(vgic_target_oracle(irq) == vcpu)) {
 			vgic_populate_lr(vcpu, irq, count++);
 
-			if (irq->source) {
-				npie = true;
+			if (irq->source)
 				prio = irq->priority;
-			}
 		}
 
 		spin_unlock(&irq->irq_lock);
@@ -807,9 +794,6 @@ static void vgic_flush_lr_state(struct kvm_vcpu *vcpu)
 		}
 	}
 
-	if (npie)
-		vgic_set_npie(vcpu);
-
 	vcpu->arch.vgic_cpu.used_lrs = count;
 
 	/* Nuke remaining LRs */
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index 830e815748a0..32c25d42c93f 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -110,6 +110,20 @@ static inline bool vgic_irq_is_mapped_level(struct vgic_irq *irq)
 	return irq->config == VGIC_CONFIG_LEVEL && irq->hw;
 }
 
+static inline int vgic_irq_get_lr_count(struct vgic_irq *irq)
+{
+	/* Account for the active state as an interrupt */
+	if (vgic_irq_is_sgi(irq->intid) && irq->source)
+		return hweight8(irq->source) + irq->active;
+
+	return irq_is_pending(irq) || irq->active;
+}
+
+static inline bool vgic_irq_is_multi_sgi(struct vgic_irq *irq)
+{
+	return vgic_irq_get_lr_count(irq) > 1;
+}
+
 /*
  * This struct provides an intermediate representation of the fields contained
  * in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC
-- 
2.14.2

^ permalink raw reply related

* [PATCH 2/4] KVM: arm64: Fix order of vcpu_write_sys_reg() arguments
From: Marc Zyngier @ 2018-05-04 16:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504161559.22852-1-marc.zyngier@arm.com>

From: James Morse <james.morse@arm.com>

A typo in kvm_vcpu_set_be()'s call:
| vcpu_write_sys_reg(vcpu, SCTLR_EL1, sctlr)
causes us to use the 32bit register value as an index into the sys_reg[]
array, and sail off the end of the linear map when we try to bring up
big-endian secondaries.

| Unable to handle kernel paging request at virtual address ffff80098b982c00
| Mem abort info:
|  ESR = 0x96000045
|  Exception class = DABT (current EL), IL = 32 bits
|   SET = 0, FnV = 0
|   EA = 0, S1PTW = 0
| Data abort info:
|   ISV = 0, ISS = 0x00000045
|   CM = 0, WnR = 1
| swapper pgtable: 4k pages, 48-bit VAs, pgdp = 000000002ea0571a
| [ffff80098b982c00] pgd=00000009ffff8803, pud=0000000000000000
| Internal error: Oops: 96000045 [#1] PREEMPT SMP
| Modules linked in:
| CPU: 2 PID: 1561 Comm: kvm-vcpu-0 Not tainted 4.17.0-rc3-00001-ga912e2261ca6-dirty #1323
| Hardware name: ARM Juno development board (r1) (DT)
| pstate: 60000005 (nZCv daif -PAN -UAO)
| pc : vcpu_write_sys_reg+0x50/0x134
| lr : vcpu_write_sys_reg+0x50/0x134

| Process kvm-vcpu-0 (pid: 1561, stack limit = 0x000000006df4728b)
| Call trace:
|  vcpu_write_sys_reg+0x50/0x134
|  kvm_psci_vcpu_on+0x14c/0x150
|  kvm_psci_0_2_call+0x244/0x2a4
|  kvm_hvc_call_handler+0x1cc/0x258
|  handle_hvc+0x20/0x3c
|  handle_exit+0x130/0x1ec
|  kvm_arch_vcpu_ioctl_run+0x340/0x614
|  kvm_vcpu_ioctl+0x4d0/0x840
|  do_vfs_ioctl+0xc8/0x8d0
|  ksys_ioctl+0x78/0xa8
|  sys_ioctl+0xc/0x18
|  el0_svc_naked+0x30/0x34
| Code: 73620291 604d00b0 00201891 1ab10194 (957a33f8)
|---[ end trace 4b4a4f9628596602 ]---

Fix the order of the arguments.

Fixes: 8d404c4c24613 ("KVM: arm64: Rewrite system register accessors to read/write functions")
CC: Christoffer Dall <cdall@cs.columbia.edu>
Signed-off-by: James Morse <james.morse@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/kvm_emulate.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 23b33e8ea03a..1dab3a984608 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -333,7 +333,7 @@ static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
 	} else {
 		u64 sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
 		sctlr |= (1 << 25);
-		vcpu_write_sys_reg(vcpu, SCTLR_EL1, sctlr);
+		vcpu_write_sys_reg(vcpu, sctlr, SCTLR_EL1);
 	}
 }
 
-- 
2.14.2

^ permalink raw reply related

* [PATCH 3/4] KVM: arm/arm64: vgic_init: Cleanup reference to process_maintenance
From: Marc Zyngier @ 2018-05-04 16:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504161559.22852-1-marc.zyngier@arm.com>

From: Valentin Schneider <valentin.schneider@arm.com>

One comment still mentioned process_maintenance operations after
commit af0614991ab6 ("KVM: arm/arm64: vgic: Get rid of unnecessary
process_maintenance operation")

Update the comment to point to vgic_fold_lr_state instead, which
is where maintenance interrupts are taken care of.

Acked-by: Christoffer Dall <christoffer.dall@arm.com>
Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 virt/kvm/arm/vgic/vgic-init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index 68378fe17a0e..e07156c30323 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -423,7 +423,7 @@ static irqreturn_t vgic_maintenance_handler(int irq, void *data)
 	 * We cannot rely on the vgic maintenance interrupt to be
 	 * delivered synchronously. This means we can only use it to
 	 * exit the VM, and we perform the handling of EOIed
-	 * interrupts on the exit path (see vgic_process_maintenance).
+	 * interrupts on the exit path (see vgic_fold_lr_state).
 	 */
 	return IRQ_HANDLED;
 }
-- 
2.14.2

^ permalink raw reply related

* [PATCH 4/4] arm64: vgic-v2: Fix proxying of cpuif access
From: Marc Zyngier @ 2018-05-04 16:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504161559.22852-1-marc.zyngier@arm.com>

From: James Morse <james.morse@arm.com>

Proxying the cpuif accesses at EL2 makes use of vcpu_data_guest_to_host
and co, which check the endianness, which call into vcpu_read_sys_reg...
which isn't mapped at EL2 (it was inlined before, and got moved OoL
with the VHE optimizations).

The result is of course a nice panic. Let's add some specialized
cruft to keep the broken platforms that require this hack alive.

But, this code used vcpu_data_guest_to_host(), which expected us to
write the value to host memory, instead we have trapped the guest's
read or write to an mmio-device, and are about to replay it using the
host's readl()/writel() which also perform swabbing based on the host
endianness. This goes wrong when both host and guest are big-endian,
as readl()/writel() will undo the guest's swabbing, causing the
big-endian value to be written to device-memory.

What needs doing?
A big-endian guest will have pre-swabbed data before storing, undo this.
If its necessary for the host, writel() will re-swab it.

For a read a big-endian guest expects to swab the data after the load.
The hosts's readl() will correct for host endianness, giving us the
device-memory's value in the register. For a big-endian guest, swab it
as if we'd only done the load.

For a little-endian guest, nothing needs doing as readl()/writel() leave
the correct device-memory value in registers.

Tested on Juno with that rarest of things: a big-endian 64K host.
Based on a patch from Marc Zyngier.

Reported-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Fixes: bf8feb39642b ("arm64: KVM: vgic-v2: Add GICV access from HYP")
Signed-off-by: James Morse <james.morse@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c b/arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c
index 86801b6055d6..39be799d0417 100644
--- a/arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c
+++ b/arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c
@@ -18,11 +18,20 @@
 #include <linux/compiler.h>
 #include <linux/irqchip/arm-gic.h>
 #include <linux/kvm_host.h>
+#include <linux/swab.h>
 
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_hyp.h>
 #include <asm/kvm_mmu.h>
 
+static bool __hyp_text __is_be(struct kvm_vcpu *vcpu)
+{
+	if (vcpu_mode_is_32bit(vcpu))
+		return !!(read_sysreg_el2(spsr) & COMPAT_PSR_E_BIT);
+
+	return !!(read_sysreg(SCTLR_EL1) & SCTLR_ELx_EE);
+}
+
 /*
  * __vgic_v2_perform_cpuif_access -- perform a GICV access on behalf of the
  *				     guest.
@@ -64,14 +73,19 @@ int __hyp_text __vgic_v2_perform_cpuif_access(struct kvm_vcpu *vcpu)
 	addr += fault_ipa - vgic->vgic_cpu_base;
 
 	if (kvm_vcpu_dabt_iswrite(vcpu)) {
-		u32 data = vcpu_data_guest_to_host(vcpu,
-						   vcpu_get_reg(vcpu, rd),
-						   sizeof(u32));
+		u32 data = vcpu_get_reg(vcpu, rd);
+		if (__is_be(vcpu)) {
+			/* guest pre-swabbed data, undo this for writel() */
+			data = swab32(data);
+		}
 		writel_relaxed(data, addr);
 	} else {
 		u32 data = readl_relaxed(addr);
-		vcpu_set_reg(vcpu, rd, vcpu_data_host_to_guest(vcpu, data,
-							       sizeof(u32)));
+		if (__is_be(vcpu)) {
+			/* guest expects swabbed data */
+			data = swab32(data);
+		}
+		vcpu_set_reg(vcpu, rd, data);
 	}
 
 	return 1;
-- 
2.14.2

^ permalink raw reply related

* [GIT PULL 1/3] ti-sysc driver changes for v4.18 merge window
From: Tony Lindgren @ 2018-05-04 16:17 UTC (permalink / raw)
  To: linux-arm-kernel

From: "Tony Lindgren" <tony@atomide.com>

The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:

  Linux 4.17-rc1 (2018-04-15 18:24:20 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap tags/omap-for-v4.18/ti-sysc-signed

for you to fetch changes up to dc4c85eac6bc8cfe25144936c5636aa1415bbc12:

  bus: ti-sysc: Show module information for suspend if DEBUG is enabled (2018-05-01 06:54:17 -0700)

----------------------------------------------------------------
ti-sysc driver related changes for omap variants

This series improves the ti-sysc interconnect target module driver to
the point where a most of SoC can be booted with interconnect target
module data configured in device tree instead of legacy platform data.
The related device tree changes need some more work though, and can
wait for v4.19. Also some drivers using nested interconnects like DSS
need more work.

We can now remove the unused pm-noop code that is not doing anything
any longer. And we can now initialize things for PM and display pdata
later to prepare things for using ti-sysc driver.

We also need to add  some more quirk handling so we can boot both with
platform data and dts data.

----------------------------------------------------------------
Tony Lindgren (16):
      ARM: OMAP2+: Drop unused pm-noop
      ARM: OMAP2+: Allow using ti-sysc for system timers
      ARM: OMAP2+: Use signed value for sysc register offsets
      ARM: OMAP2+: Only probe SDMA via ti-sysc if configured in dts
      ARM: OMAP2+: Initialize SoC PM later
      ARM: OMAP2+: Make display related init into device_initcall
      bus: ti-sysc: Handle simple-bus for nested children
      bus: ti-sysc: Make child clock alias handling more generic
      bus: ti-sysc: Add handling for clkctrl opt clocks
      bus: ti-sysc: Tag some modules resource providers for noirq suspend
      bus: ti-sysc: Improve suspend and resume handling
      bus: ti-sysc: Add initial support for external resets
      bus: ti-sysc: Detect omap4 type timers for quirk
      bus: ti-sysc: Detect UARTs for SYSC_QUIRK_LEGACY_IDLE quirk on omap4
      bus: ti-sysc: Tag sdio and wdt with legacy mode for suspend
      bus: ti-sysc: Show module information for suspend if DEBUG is enabled

 Documentation/devicetree/bindings/bus/ti-sysc.txt  |   6 +-
 arch/arm/mach-omap2/Makefile                       |   1 -
 arch/arm/mach-omap2/board-generic.c                |   2 -
 arch/arm/mach-omap2/common.h                       |  18 +-
 arch/arm/mach-omap2/display.c                      |  10 +-
 arch/arm/mach-omap2/hsmmc.c                        |   1 -
 arch/arm/mach-omap2/i2c.c                          |   1 -
 arch/arm/mach-omap2/io.c                           |  70 +---
 arch/arm/mach-omap2/omap-pm-noop.c                 | 176 ----------
 arch/arm/mach-omap2/omap-pm.h                      | 161 ---------
 arch/arm/mach-omap2/omap_device.c                  |  22 +-
 arch/arm/mach-omap2/omap_hwmod.c                   |  21 +-
 arch/arm/mach-omap2/omap_hwmod.h                   |   6 +-
 .../mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c |   1 +
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |   4 +
 arch/arm/mach-omap2/omap_hwmod_43xx_data.c         |   1 +
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |   4 +
 arch/arm/mach-omap2/omap_hwmod_54xx_data.c         |   3 +
 arch/arm/mach-omap2/omap_hwmod_7xx_data.c          |   6 +
 arch/arm/mach-omap2/omap_hwmod_81xx_data.c         |   1 +
 arch/arm/mach-omap2/pdata-quirks.c                 |  15 -
 arch/arm/mach-omap2/pm-debug.c                     |   5 -
 arch/arm/mach-omap2/pm.c                           |  21 +-
 arch/arm/mach-omap2/pm33xx-core.c                  |   4 +-
 arch/arm/mach-omap2/timer.c                        |  68 +++-
 arch/arm/plat-omap/Kconfig                         |  10 -
 drivers/bus/ti-sysc.c                              | 388 ++++++++++++++++++---
 drivers/media/rc/ir-rx51.c                         |  17 +-
 include/linux/platform_data/media/ir-rx51.h        |   9 -
 include/linux/platform_data/ti-sysc.h              |   1 +
 30 files changed, 494 insertions(+), 559 deletions(-)
 delete mode 100644 arch/arm/mach-omap2/omap-pm-noop.c
 delete mode 100644 arch/arm/mach-omap2/omap-pm.h
 delete mode 100644 include/linux/platform_data/media/ir-rx51.h

^ permalink raw reply

* [GIT PULL 2/3] omap dts changes for v4.18 merge window
From: Tony Lindgren @ 2018-05-04 16:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <pull-1525450571-674978@atomide.com>

From: "Tony Lindgren" <tony@atomide.com>

The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:

  Linux 4.17-rc1 (2018-04-15 18:24:20 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap tags/omap-for-v4.18/dt-signed

for you to fetch changes up to 8dfa75524a0e0e2b4eaf2a3dc178f6b4d8db85d9:

  ARM: dts: correct invalid I/O definition for MMC/SD card detect on T410 (2018-05-03 10:03:01 -0700)

----------------------------------------------------------------
Device tree changes for omap variants for v4.18 merge window

This series adds support for am335x-pockebeagle and also add missing
pinctrl configuration for am335x evm and beagle bone variants.

There are also changes to add missing omap3 oscillator clocks for audio,
and fixes am437x tps65218 irq type used for various board specific
files.

There are also few minor fixes included that are not urgent. The
fixes for n8x0 audio also depend on driver changes, and the hp t410
mmc card detect mux typo is harmless.

----------------------------------------------------------------
Adam Ford (1):
      ARM: dts: logicpd-som-lv: Enable Touchscreen controller

Faiz Abbas (1):
      ARM: dts: am33xx: Add pinmux data for mmc1 in am335x-evm, evmsk and beaglebone

Graeme Smecher (1):
      ARM: dts: correct invalid I/O definition for MMC/SD card detect on T410

H. Nikolaus Schaller (2):
      ARM: dts: omap3-pandora: Add fixed 26MHz clock as fck for twl
      ARM: dts: omap3-gta04: Add fixed 26MHz clock as fck for twl

Peter Ujfalusi (7):
      ARM: dts: omap3-beagle-xm: Add fixed 26MHz clock as fck for twl
      ARM: dts: am437x-gp-evm: Correct tps65218 irq type
      ARM: dts: am437x-cm-t43: Correct tps65218 irq type
      ARM: dts: am437x-epos-evm: Correct tps65218 irq type
      ARM: dts: am437x-sk-evm: Correct tps65218 irq type
      ARM: dts: omap2420-n810: Enable McBSP2 for audio
      ARM: dts: omap2420-n810: Correct the audio codec (tlv320aic33) node

Robert Nelson (1):
      ARM: dts: Add am335x-pocketbeagle

 arch/arm/boot/dts/Makefile                   |   1 +
 arch/arm/boot/dts/am335x-bone-common.dtsi    |   9 +-
 arch/arm/boot/dts/am335x-evm.dts             |   9 +-
 arch/arm/boot/dts/am335x-evmsk.dts           |   9 +-
 arch/arm/boot/dts/am335x-osd335x-common.dtsi | 124 ++++++++++++++
 arch/arm/boot/dts/am335x-pocketbeagle.dts    | 237 +++++++++++++++++++++++++++
 arch/arm/boot/dts/am437x-cm-t43.dts          |   2 +-
 arch/arm/boot/dts/am437x-gp-evm.dts          |   2 +-
 arch/arm/boot/dts/am437x-sk-evm.dts          |   2 +-
 arch/arm/boot/dts/am43x-epos-evm.dts         |   2 +-
 arch/arm/boot/dts/dm8148-t410.dts            |   2 +-
 arch/arm/boot/dts/logicpd-som-lv.dtsi        |  25 +++
 arch/arm/boot/dts/omap2420-n810.dts          |  63 ++++++-
 arch/arm/boot/dts/omap3-beagle-xm.dts        |  10 ++
 arch/arm/boot/dts/omap3-gta04.dtsi           |  10 ++
 arch/arm/boot/dts/omap3-pandora-common.dtsi  |  10 ++
 16 files changed, 507 insertions(+), 10 deletions(-)
 create mode 100644 arch/arm/boot/dts/am335x-osd335x-common.dtsi
 create mode 100644 arch/arm/boot/dts/am335x-pocketbeagle.dts

^ permalink raw reply

* [GIT PULL 3/3] omap sdhci dts changes for v4.18 merge window
From: Tony Lindgren @ 2018-05-04 16:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <pull-1525450571-674978@atomide.com>

From: "Tony Lindgren" <tony@atomide.com>

The following changes since commit 3f4028780287ff5a7308f40e10bbba9a42b993aa:

  mmc: sdhci-omap: Get IODelay values for 3.3v DDR mode (2018-05-03 09:37:13 +0200)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap tags/omap-for-v4.18/dt-sdhci-signed

for you to fetch changes up to 24a6f1f65ea567d017c598faf1374ee443f73851:

  Documentation: ARM: Add new MMC requirements for DRA7/K2G (2018-05-03 10:32:20 -0700)

----------------------------------------------------------------
Device tree changes for omap variants for SDHCI

This series adds the devicetree configuration needed for pinctrl on
dra7 variants to use the SDHCI SDIO driver instead of mmc-omap-hs
driver. To use SDHCI, both the pins and the iodelay needs to be
configured.

This series is based on the related SDHCI drivers changes on a branch
set up by Ulf.

----------------------------------------------------------------
Hari Nagalla (2):
      ARM: dts: dra72-evm-common: Add wilink8 wlan support
      ARM: dts: dra7-evm: Add wilink8 wlan support

Kishon Vijay Abraham I (10):
      ARM: dts: dra72-evm-common: Remove mmc specific pinmux
      ARM: dts: dra71-evm: Add "vqmmc-supply" property for mmc2
      ARM: dts: dra7-mmc-iodelay: Add a new pinctrl group for clk line without pullup
      ARM: dts: am57xx-idk: Use pinctrl group from dra7-mmc-iodelay.dtsi to select pulldown
      ARM: dts: dra71-evm: Use pinctrl group from dra7-mmc-iodelay.dtsi to select pulldown
      ARM: dts: am57xx-beagle-x15/am57xx-idk: Fix pinctrl-names
      ARM: dts: dra7-evm: Model EVM_3V6 regulator
      ARM: dts: dra7: Use sdhci-omap programming model
      ARM: dts: dra7: Add high speed modes capability to MMC1/MMC2 dt node
      Documentation: ARM: Add new MMC requirements for DRA7/K2G

Sekhar Nori (1):
      ARM: dts: am574x-idk: Add pinmux configuration for MMC

Vishal Mahaveer (1):
      ARM: dts: dra76-evm: Add wilink8 wlan support

 Documentation/arm/OMAP/README                   |  4 ++
 arch/arm/boot/dts/am571x-idk.dts                |  5 +-
 arch/arm/boot/dts/am572x-idk.dts                |  5 +-
 arch/arm/boot/dts/am574x-idk.dts                | 20 +++++++
 arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi |  4 +-
 arch/arm/boot/dts/am57xx-beagle-x15.dts         |  3 +-
 arch/arm/boot/dts/am57xx-idk-common.dtsi        | 14 +----
 arch/arm/boot/dts/dra7-evm-common.dtsi          | 15 ++++++
 arch/arm/boot/dts/dra7-evm.dts                  | 68 +++++++++++++++++++++++
 arch/arm/boot/dts/dra7-mmc-iodelay.dtsi         | 19 +++++++
 arch/arm/boot/dts/dra7.dtsi                     | 32 ++++++-----
 arch/arm/boot/dts/dra71-evm.dts                 | 17 ++----
 arch/arm/boot/dts/dra72-evm-common.dtsi         | 71 +++++++++++++++----------
 arch/arm/boot/dts/dra72x-mmc-iodelay.dtsi       | 11 ++++
 arch/arm/boot/dts/dra76-evm.dts                 | 34 +++++++++++-
 15 files changed, 243 insertions(+), 79 deletions(-)
 create mode 100644 arch/arm/boot/dts/dra7-mmc-iodelay.dtsi

^ permalink raw reply

* [PATCH v2 1/4] KVM: arm/arm64: Share common code in user_mem_abort()
From: Punit Agrawal @ 2018-05-04 16:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504113813.GD10191@C02W217FHV2R.local>

Christoffer Dall <christoffer.dall@arm.com> writes:

> On Tue, May 01, 2018 at 11:26:56AM +0100, Punit Agrawal wrote:
>> The code for operations such as marking the pfn as dirty, and
>> dcache/icache maintenance during stage 2 fault handling is duplicated
>> between normal pages and PMD hugepages.
>> 
>> Instead of creating another copy of the operations when we introduce
>> PUD hugepages, let's share them across the different pagesizes.
>> 
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  virt/kvm/arm/mmu.c | 66 +++++++++++++++++++++++++++-------------------
>>  1 file changed, 39 insertions(+), 27 deletions(-)
>> 
>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>> index 7f6a944db23d..686fc6a4b866 100644
>> --- a/virt/kvm/arm/mmu.c
>> +++ b/virt/kvm/arm/mmu.c

[...]

>> @@ -1517,28 +1533,34 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>  	if (mmu_notifier_retry(kvm, mmu_seq))
>>  		goto out_unlock;
>>  
>> -	if (!hugetlb && !force_pte)
>> +	if (!hugetlb && !force_pte) {
>>  		hugetlb = transparent_hugepage_adjust(&pfn, &fault_ipa);
>> +		/*
>> +		 * Only PMD_SIZE transparent hugepages(THP) are
>> +		 * currently supported. This code will need to be
>> +		 * updated to support other THP sizes.
>> +		 */
>> +		if (hugetlb)
>> +			vma_pagesize = PMD_SIZE;
>
> nit: this is a bit of a trap waiting to happen, as the suggested
> semantics of hugetlb is now hugetlbfs and not THP.
>
> It may be slightly nicer to do do:
>
> 		if (transparent_hugepage_adjust(&pfn, &fault_ipa))
> 			vma_pagesize = PMD_SIZE;

I should've noticed this.

I'll incorporate your suggestion and update the condition below using
hugetlb to rely on vma_pagesize instead.

Thanks,
Punit

>
>> +	}
>> +
>> +	if (writable)
>> +		kvm_set_pfn_dirty(pfn);
>> +
>> +	if (fault_status != FSC_PERM)
>> +		clean_dcache_guest_page(pfn, vma_pagesize);
>> +
>> +	if (exec_fault)
>> +		invalidate_icache_guest_page(pfn, vma_pagesize);
>>  
>>  	if (hugetlb) {
>>  		pmd_t new_pmd = pfn_pmd(pfn, mem_type);
>>  		new_pmd = pmd_mkhuge(new_pmd);
>> -		if (writable) {
>> +		if (writable)
>>  			new_pmd = kvm_s2pmd_mkwrite(new_pmd);
>> -			kvm_set_pfn_dirty(pfn);
>> -		}
>>  
>> -		if (fault_status != FSC_PERM)
>> -			clean_dcache_guest_page(pfn, PMD_SIZE);
>> -
>> -		if (exec_fault) {
>> +		if (stage2_should_exec(kvm, fault_ipa, exec_fault, fault_status))
>>  			new_pmd = kvm_s2pmd_mkexec(new_pmd);
>> -			invalidate_icache_guest_page(pfn, PMD_SIZE);
>> -		} else if (fault_status == FSC_PERM) {
>> -			/* Preserve execute if XN was already cleared */
>> -			if (stage2_is_exec(kvm, fault_ipa))
>> -				new_pmd = kvm_s2pmd_mkexec(new_pmd);
>> -		}
>>  
>>  		ret = stage2_set_pmd_huge(kvm, memcache, fault_ipa, &new_pmd);
>>  	} else {
>> @@ -1546,21 +1568,11 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>  
>>  		if (writable) {
>>  			new_pte = kvm_s2pte_mkwrite(new_pte);
>> -			kvm_set_pfn_dirty(pfn);
>>  			mark_page_dirty(kvm, gfn);
>>  		}
>>  
>> -		if (fault_status != FSC_PERM)
>> -			clean_dcache_guest_page(pfn, PAGE_SIZE);
>> -
>> -		if (exec_fault) {
>> +		if (stage2_should_exec(kvm, fault_ipa, exec_fault, fault_status))
>>  			new_pte = kvm_s2pte_mkexec(new_pte);
>> -			invalidate_icache_guest_page(pfn, PAGE_SIZE);
>> -		} else if (fault_status == FSC_PERM) {
>> -			/* Preserve execute if XN was already cleared */
>> -			if (stage2_is_exec(kvm, fault_ipa))
>> -				new_pte = kvm_s2pte_mkexec(new_pte);
>> -		}
>>  
>>  		ret = stage2_set_pte(kvm, memcache, fault_ipa, &new_pte, flags);
>>  	}
>> -- 
>> 2.17.0
>> 
>
> Otherwise looks good.
>
> Thanks,
> -Christoffer
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply

* Potential deadlock in vgic
From: Jan Glauber @ 2018-05-04 16:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504151740.12165-1-andre.przywara@arm.com>

On Fri, May 04, 2018 at 04:17:40PM +0100, Andre Przywara wrote:
> Hi Jan,
> 
> can you please test this patch with your setup, to see if it still
> screams? That converts two forgotten irq_lock's over to be irqsafe,
> plus lets lpi_list_lock join them (which you already did, IIUC).
> That should appease lockdep, hopefully.

Hi Andre,

that solves the issue for me, no more lockdep complains.

thanks!
Jan

> Cheers,
> Andre.
> ---
>  virt/kvm/arm/vgic/vgic-debug.c |  5 +++--
>  virt/kvm/arm/vgic/vgic-its.c   | 15 +++++++++------
>  virt/kvm/arm/vgic/vgic.c       | 12 +++++++-----
>  3 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c
> index 10b38178cff2..4ffc0b5e6105 100644
> --- a/virt/kvm/arm/vgic/vgic-debug.c
> +++ b/virt/kvm/arm/vgic/vgic-debug.c
> @@ -211,6 +211,7 @@ static int vgic_debug_show(struct seq_file *s, void *v)
>  	struct vgic_state_iter *iter = (struct vgic_state_iter *)v;
>  	struct vgic_irq *irq;
>  	struct kvm_vcpu *vcpu = NULL;
> +	unsigned long flags;
>  
>  	if (iter->dist_id == 0) {
>  		print_dist_state(s, &kvm->arch.vgic);
> @@ -227,9 +228,9 @@ static int vgic_debug_show(struct seq_file *s, void *v)
>  		irq = &kvm->arch.vgic.spis[iter->intid - VGIC_NR_PRIVATE_IRQS];
>  	}
>  
> -	spin_lock(&irq->irq_lock);
> +	spin_lock_irqsave(&irq->irq_lock, flags);
>  	print_irq_state(s, irq, vcpu);
> -	spin_unlock(&irq->irq_lock);
> +	spin_unlock_irqrestore(&irq->irq_lock, flags);
>  
>  	return 0;
>  }
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index a8f07243aa9f..51a80b600632 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -52,6 +52,7 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
>  {
>  	struct vgic_dist *dist = &kvm->arch.vgic;
>  	struct vgic_irq *irq = vgic_get_irq(kvm, NULL, intid), *oldirq;
> +	unsigned long flags;
>  	int ret;
>  
>  	/* In this case there is no put, since we keep the reference. */
> @@ -71,7 +72,7 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
>  	irq->intid = intid;
>  	irq->target_vcpu = vcpu;
>  
> -	spin_lock(&dist->lpi_list_lock);
> +	spin_lock_irqsave(&dist->lpi_list_lock, flags);
>  
>  	/*
>  	 * There could be a race with another vgic_add_lpi(), so we need to
> @@ -99,7 +100,7 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
>  	dist->lpi_list_count++;
>  
>  out_unlock:
> -	spin_unlock(&dist->lpi_list_lock);
> +	spin_unlock_irqrestore(&dist->lpi_list_lock, flags);
>  
>  	/*
>  	 * We "cache" the configuration table entries in our struct vgic_irq's.
> @@ -315,6 +316,7 @@ static int vgic_copy_lpi_list(struct kvm_vcpu *vcpu, u32 **intid_ptr)
>  {
>  	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>  	struct vgic_irq *irq;
> +	unsigned long flags;
>  	u32 *intids;
>  	int irq_count, i = 0;
>  
> @@ -330,7 +332,7 @@ static int vgic_copy_lpi_list(struct kvm_vcpu *vcpu, u32 **intid_ptr)
>  	if (!intids)
>  		return -ENOMEM;
>  
> -	spin_lock(&dist->lpi_list_lock);
> +	spin_lock_irqsave(&dist->lpi_list_lock, flags);
>  	list_for_each_entry(irq, &dist->lpi_list_head, lpi_list) {
>  		if (i == irq_count)
>  			break;
> @@ -339,7 +341,7 @@ static int vgic_copy_lpi_list(struct kvm_vcpu *vcpu, u32 **intid_ptr)
>  			continue;
>  		intids[i++] = irq->intid;
>  	}
> -	spin_unlock(&dist->lpi_list_lock);
> +	spin_unlock_irqrestore(&dist->lpi_list_lock, flags);
>  
>  	*intid_ptr = intids;
>  	return i;
> @@ -348,10 +350,11 @@ static int vgic_copy_lpi_list(struct kvm_vcpu *vcpu, u32 **intid_ptr)
>  static int update_affinity(struct vgic_irq *irq, struct kvm_vcpu *vcpu)
>  {
>  	int ret = 0;
> +	unsigned long flags;
>  
> -	spin_lock(&irq->irq_lock);
> +	spin_lock_irqsave(&irq->irq_lock, flags);
>  	irq->target_vcpu = vcpu;
> -	spin_unlock(&irq->irq_lock);
> +	spin_unlock_irqrestore(&irq->irq_lock, flags);
>  
>  	if (irq->hw) {
>  		struct its_vlpi_map map;
> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> index 5f52a2bca36f..6efcddfb5167 100644
> --- a/virt/kvm/arm/vgic/vgic.c
> +++ b/virt/kvm/arm/vgic/vgic.c
> @@ -75,8 +75,9 @@ static struct vgic_irq *vgic_get_lpi(struct kvm *kvm, u32 intid)
>  {
>  	struct vgic_dist *dist = &kvm->arch.vgic;
>  	struct vgic_irq *irq = NULL;
> +	unsigned long flags;
>  
> -	spin_lock(&dist->lpi_list_lock);
> +	spin_lock_irqsave(&dist->lpi_list_lock, flags);
>  
>  	list_for_each_entry(irq, &dist->lpi_list_head, lpi_list) {
>  		if (irq->intid != intid)
> @@ -92,7 +93,7 @@ static struct vgic_irq *vgic_get_lpi(struct kvm *kvm, u32 intid)
>  	irq = NULL;
>  
>  out_unlock:
> -	spin_unlock(&dist->lpi_list_lock);
> +	spin_unlock_irqrestore(&dist->lpi_list_lock, flags);
>  
>  	return irq;
>  }
> @@ -137,19 +138,20 @@ static void vgic_irq_release(struct kref *ref)
>  void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)
>  {
>  	struct vgic_dist *dist = &kvm->arch.vgic;
> +	unsigned long flags;
>  
>  	if (irq->intid < VGIC_MIN_LPI)
>  		return;
>  
> -	spin_lock(&dist->lpi_list_lock);
> +	spin_lock_irqsave(&dist->lpi_list_lock, flags);
>  	if (!kref_put(&irq->refcount, vgic_irq_release)) {
> -		spin_unlock(&dist->lpi_list_lock);
> +		spin_unlock_irqrestore(&dist->lpi_list_lock, flags);
>  		return;
>  	};
>  
>  	list_del(&irq->lpi_list);
>  	dist->lpi_list_count--;
> -	spin_unlock(&dist->lpi_list_lock);
> +	spin_unlock_irqrestore(&dist->lpi_list_lock, flags);
>  
>  	kfree(irq);
>  }
> -- 
> 2.14.1

^ permalink raw reply

* Potential deadlock in vgic
From: Andre Przywara @ 2018-05-04 16:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504162659.GB14663@hc>

Hi Jan,

On 04/05/18 17:26, Jan Glauber wrote:
> On Fri, May 04, 2018 at 04:17:40PM +0100, Andre Przywara wrote:
>> Hi Jan,
>>
>> can you please test this patch with your setup, to see if it still
>> screams? That converts two forgotten irq_lock's over to be irqsafe,
>> plus lets lpi_list_lock join them (which you already did, IIUC).
>> That should appease lockdep, hopefully.
> 
> Hi Andre,
> 
> that solves the issue for me, no more lockdep complains.

Thanks for the confirmation, will send out a proper patch shortly.

Sch?nes Wochenende!

Andre.

>> ---
>>  virt/kvm/arm/vgic/vgic-debug.c |  5 +++--
>>  virt/kvm/arm/vgic/vgic-its.c   | 15 +++++++++------
>>  virt/kvm/arm/vgic/vgic.c       | 12 +++++++-----
>>  3 files changed, 19 insertions(+), 13 deletions(-)
>>
>> diff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c
>> index 10b38178cff2..4ffc0b5e6105 100644
>> --- a/virt/kvm/arm/vgic/vgic-debug.c
>> +++ b/virt/kvm/arm/vgic/vgic-debug.c
>> @@ -211,6 +211,7 @@ static int vgic_debug_show(struct seq_file *s, void *v)
>>  	struct vgic_state_iter *iter = (struct vgic_state_iter *)v;
>>  	struct vgic_irq *irq;
>>  	struct kvm_vcpu *vcpu = NULL;
>> +	unsigned long flags;
>>  
>>  	if (iter->dist_id == 0) {
>>  		print_dist_state(s, &kvm->arch.vgic);
>> @@ -227,9 +228,9 @@ static int vgic_debug_show(struct seq_file *s, void *v)
>>  		irq = &kvm->arch.vgic.spis[iter->intid - VGIC_NR_PRIVATE_IRQS];
>>  	}
>>  
>> -	spin_lock(&irq->irq_lock);
>> +	spin_lock_irqsave(&irq->irq_lock, flags);
>>  	print_irq_state(s, irq, vcpu);
>> -	spin_unlock(&irq->irq_lock);
>> +	spin_unlock_irqrestore(&irq->irq_lock, flags);
>>  
>>  	return 0;
>>  }
>> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
>> index a8f07243aa9f..51a80b600632 100644
>> --- a/virt/kvm/arm/vgic/vgic-its.c
>> +++ b/virt/kvm/arm/vgic/vgic-its.c
>> @@ -52,6 +52,7 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
>>  {
>>  	struct vgic_dist *dist = &kvm->arch.vgic;
>>  	struct vgic_irq *irq = vgic_get_irq(kvm, NULL, intid), *oldirq;
>> +	unsigned long flags;
>>  	int ret;
>>  
>>  	/* In this case there is no put, since we keep the reference. */
>> @@ -71,7 +72,7 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
>>  	irq->intid = intid;
>>  	irq->target_vcpu = vcpu;
>>  
>> -	spin_lock(&dist->lpi_list_lock);
>> +	spin_lock_irqsave(&dist->lpi_list_lock, flags);
>>  
>>  	/*
>>  	 * There could be a race with another vgic_add_lpi(), so we need to
>> @@ -99,7 +100,7 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
>>  	dist->lpi_list_count++;
>>  
>>  out_unlock:
>> -	spin_unlock(&dist->lpi_list_lock);
>> +	spin_unlock_irqrestore(&dist->lpi_list_lock, flags);
>>  
>>  	/*
>>  	 * We "cache" the configuration table entries in our struct vgic_irq's.
>> @@ -315,6 +316,7 @@ static int vgic_copy_lpi_list(struct kvm_vcpu *vcpu, u32 **intid_ptr)
>>  {
>>  	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>>  	struct vgic_irq *irq;
>> +	unsigned long flags;
>>  	u32 *intids;
>>  	int irq_count, i = 0;
>>  
>> @@ -330,7 +332,7 @@ static int vgic_copy_lpi_list(struct kvm_vcpu *vcpu, u32 **intid_ptr)
>>  	if (!intids)
>>  		return -ENOMEM;
>>  
>> -	spin_lock(&dist->lpi_list_lock);
>> +	spin_lock_irqsave(&dist->lpi_list_lock, flags);
>>  	list_for_each_entry(irq, &dist->lpi_list_head, lpi_list) {
>>  		if (i == irq_count)
>>  			break;
>> @@ -339,7 +341,7 @@ static int vgic_copy_lpi_list(struct kvm_vcpu *vcpu, u32 **intid_ptr)
>>  			continue;
>>  		intids[i++] = irq->intid;
>>  	}
>> -	spin_unlock(&dist->lpi_list_lock);
>> +	spin_unlock_irqrestore(&dist->lpi_list_lock, flags);
>>  
>>  	*intid_ptr = intids;
>>  	return i;
>> @@ -348,10 +350,11 @@ static int vgic_copy_lpi_list(struct kvm_vcpu *vcpu, u32 **intid_ptr)
>>  static int update_affinity(struct vgic_irq *irq, struct kvm_vcpu *vcpu)
>>  {
>>  	int ret = 0;
>> +	unsigned long flags;
>>  
>> -	spin_lock(&irq->irq_lock);
>> +	spin_lock_irqsave(&irq->irq_lock, flags);
>>  	irq->target_vcpu = vcpu;
>> -	spin_unlock(&irq->irq_lock);
>> +	spin_unlock_irqrestore(&irq->irq_lock, flags);
>>  
>>  	if (irq->hw) {
>>  		struct its_vlpi_map map;
>> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
>> index 5f52a2bca36f..6efcddfb5167 100644
>> --- a/virt/kvm/arm/vgic/vgic.c
>> +++ b/virt/kvm/arm/vgic/vgic.c
>> @@ -75,8 +75,9 @@ static struct vgic_irq *vgic_get_lpi(struct kvm *kvm, u32 intid)
>>  {
>>  	struct vgic_dist *dist = &kvm->arch.vgic;
>>  	struct vgic_irq *irq = NULL;
>> +	unsigned long flags;
>>  
>> -	spin_lock(&dist->lpi_list_lock);
>> +	spin_lock_irqsave(&dist->lpi_list_lock, flags);
>>  
>>  	list_for_each_entry(irq, &dist->lpi_list_head, lpi_list) {
>>  		if (irq->intid != intid)
>> @@ -92,7 +93,7 @@ static struct vgic_irq *vgic_get_lpi(struct kvm *kvm, u32 intid)
>>  	irq = NULL;
>>  
>>  out_unlock:
>> -	spin_unlock(&dist->lpi_list_lock);
>> +	spin_unlock_irqrestore(&dist->lpi_list_lock, flags);
>>  
>>  	return irq;
>>  }
>> @@ -137,19 +138,20 @@ static void vgic_irq_release(struct kref *ref)
>>  void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)
>>  {
>>  	struct vgic_dist *dist = &kvm->arch.vgic;
>> +	unsigned long flags;
>>  
>>  	if (irq->intid < VGIC_MIN_LPI)
>>  		return;
>>  
>> -	spin_lock(&dist->lpi_list_lock);
>> +	spin_lock_irqsave(&dist->lpi_list_lock, flags);
>>  	if (!kref_put(&irq->refcount, vgic_irq_release)) {
>> -		spin_unlock(&dist->lpi_list_lock);
>> +		spin_unlock_irqrestore(&dist->lpi_list_lock, flags);
>>  		return;
>>  	};
>>  
>>  	list_del(&irq->lpi_list);
>>  	dist->lpi_list_count--;
>> -	spin_unlock(&dist->lpi_list_lock);
>> +	spin_unlock_irqrestore(&dist->lpi_list_lock, flags);
>>  
>>  	kfree(irq);
>>  }
>> -- 
>> 2.14.1

^ permalink raw reply

* Potential deadlock in vgic
From: Jan Glauber @ 2018-05-04 16:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504151740.12165-1-andre.przywara@arm.com>

On Fri, May 04, 2018 at 04:17:40PM +0100, Andre Przywara wrote:
> Hi Jan,
> 
> can you please test this patch with your setup, to see if it still
> screams? That converts two forgotten irq_lock's over to be irqsafe,
> plus lets lpi_list_lock join them (which you already did, IIUC).
> That should appease lockdep, hopefully.

Hit send too soon, on halting the guest I get:

[ 1025.694857] =============================
[ 1025.694862] WARNING: suspicious RCU usage
[ 1025.694868] 4.17.0-rc3-jang+ #73 Not tainted
[ 1025.694873] -----------------------------
[ 1025.694880] ./include/linux/kvm_host.h:575 suspicious rcu_dereference_check() usage!
[ 1025.694884] 
               other info that might help us debug this:

[ 1025.694890] 
               rcu_scheduler_active = 2, debug_locks = 1
[ 1025.694896] 18 locks held by qemu-system-aar/5540:
[ 1025.694901]  #0: 000000005e03488a (&kvm->lock){+.+.}, at: vgic_its_set_attr+0x230/0x388
[ 1025.694937]  #1: 000000004b1a3bb5 (&its->its_lock){+.+.}, at: vgic_its_set_attr+0x23c/0x388
[ 1025.694965]  #2: 000000003ca8213c (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.694993]  #3: 00000000adb6ae51 (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695021]  #4: 0000000000563df7 (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695048]  #5: 00000000da16277a (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695076]  #6: 00000000bf36d9aa (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695103]  #7: 00000000607eaa4f (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695130]  #8: 0000000046dadf65 (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695157]  #9: 00000000197747b2 (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695184]  #10: 00000000e4f1282c (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695211]  #11: 000000007471b896 (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695239]  #12: 000000005be54486 (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695266]  #13: 000000000f1fa184 (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695293]  #14: 0000000093fdb28b (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695396]  #15: 0000000097cc103c (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695426]  #16: 00000000d24dd32e (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695453]  #17: 000000002606c3a7 (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xc0
[ 1025.695482] 
               stack backtrace:
[ 1025.695489] CPU: 29 PID: 5540 Comm: qemu-system-aar Not tainted 4.17.0-rc3-jang+ #73
[ 1025.695494] Hardware name: To be filled by O.E.M. Saber/To be filled by O.E.M., BIOS 0ACKL018 03/30/2018
[ 1025.695499] Call trace:
[ 1025.695505]  dump_backtrace+0x0/0x160
[ 1025.695510]  show_stack+0x24/0x30
[ 1025.695517]  dump_stack+0x9c/0xd4
[ 1025.695524]  lockdep_rcu_suspicious+0xcc/0x118
[ 1025.695537]  gfn_to_memslot+0x174/0x190
[ 1025.695546]  kvm_read_guest+0x50/0xb0
[ 1025.695553]  vgic_its_check_id.isra.0+0x114/0x148
[ 1025.695560]  vgic_its_save_tables_v0+0x1a0/0x320
[ 1025.695567]  vgic_its_set_attr+0x330/0x388
[ 1025.695573]  kvm_device_ioctl_attr+0x9c/0xd8
[ 1025.695579]  kvm_device_ioctl+0x8c/0xf8
[ 1025.695587]  do_vfs_ioctl+0xc4/0x938
[ 1025.695594]  ksys_ioctl+0x8c/0x98
[ 1025.695601]  sys_ioctl+0x34/0x48
[ 1025.695609]  el0_svc_naked+0x44/0x48

--Jan

^ permalink raw reply

* [PATCH] ARM: dts: imx7s: Pass the 'fsl,sec-era' property
From: Fabio Estevam @ 2018-05-04 16:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504071926.GT3443@dragon>

Hi Shawn,

On Fri, May 4, 2018 at 4:19 AM, Shawn Guo <shawnguo@kernel.org> wrote:

> The property is documented as optional in the bindings.  Missing the
> property shouldn't cause any fatal error, I guess.

The CAAM drivers uses the era information in many places:

drivers/crypto/caam/caamalg.c:  if (ctrlpriv->era >= 6) {
drivers/crypto/caam/caamalg.c:  if (ctrlpriv->era < 3)
drivers/crypto/caam/caamalg.c:  if (priv->era >= 6 && uses_dkp)
drivers/crypto/caam/caamalg_desc.c:     if (era < 6) {
drivers/crypto/caam/caamalg_desc.c:     /* Class 2 operation */
drivers/crypto/caam/caamalg_desc.c:     if (era < 6) {
drivers/crypto/caam/caamalg_desc.c:     /* Class 2 operation */
drivers/crypto/caam/caamalg_desc.c:     if (era < 6) {

If the era information is not provided the value of -524 (-ENOTSUPP)
will be used, so all the above logic will not operate correctly.

I have sent a patch that allows the era information to be retrieved
via CAAM registers:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/crypto/caam/ctrl.c?h=next-20180424&id=654f2b937b389295581bcb4aa26011a63db7bc8f

but this will only land in 4.18.

In order to avoid i.MX7 to report the incorrect era information in
4.17 I suggest that this patch should be applied for 4.17-rc.

Also, the other i.mx SoCs pass the fsl,sec-era in their dts.

Thanks

^ permalink raw reply

* [PATCH v2 07/10] PCI: Convert of_pci_get_host_bridge_resources() users to devm variant
From: Lorenzo Pieralisi @ 2018-05-04 16:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <dbfa80fb-5fe8-3ccd-6886-4604ef9e058b@mentor.com>

On Thu, May 03, 2018 at 10:18:24AM +0300, Vladimir Zapolskiy wrote:
> Hi Jan,
> 
> On 04/30/2018 08:48 AM, Jan Kiszka wrote:
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > Straightforward for all of them, no more leaks afterwards.
> > 
> > CC: Jingoo Han <jingoohan1@gmail.com>
> > CC: Joao Pinto <Joao.Pinto@synopsys.com>
> > CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > Acked-by: Jingoo Han <jingoo1han@gmail.com>
> 
> [snip]
> 
> > diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> > index 6ab28f29ac6a..6eb36c924983 100644
> > --- a/drivers/pci/host/pcie-rcar.c
> > +++ b/drivers/pci/host/pcie-rcar.c
> > @@ -1067,12 +1067,11 @@ static int rcar_pcie_parse_request_of_pci_ranges(struct rcar_pcie *pci)
> >  {
> >  	int err;
> >  	struct device *dev = pci->dev;
> > -	struct device_node *np = dev->of_node;
> >  	resource_size_t iobase;
> >  	struct resource_entry *win, *tmp;
> >  
> > -	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pci->resources,
> > -					       &iobase);
> > +	err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
> > +						    &pci->resources, &iobase);
> >  	if (err)
> >  		return err;
> >  
> 
> this one snippet is obsoleted by https://patchwork.ozlabs.org/patch/904326/
> 
> If 08/10 remains a deprecation, then it is sufficient to exclude the R-Car change,
> otherwise I hope maintainers can deal with the proper ordering.

Isn't applying your patch:

https://patchwork.ozlabs.org/patch/904326/

and dropping the rcar hunk from this patch enough ?

Lorenzo

^ permalink raw reply

* [PATCH v8 0/6] optimize memblock_next_valid_pfn and early_pfn_valid on arm and arm64
From: Pavel Tatashin @ 2018-05-04 16:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACjP9X8bHmrxmd7ZPcfQq6Eq0Mzwmt0saOR3Ph53gp2n-dcKBQ@mail.gmail.com>

> I'm wondering, ain't simple enabling of config
> DEFERRED_STRUCT_PAGE_INIT provide even better speed-up? If that is the
> case then it seems like this series is not needed at all, right?
> I am not sure why is this config optional. It looks like it could be
> enabled by default or even unconditionally considering that with
> commit c9e97a1997fb ("mm: initialize pages on demand during boot") the
> deferred code is statically disabled after all the pages are
> initialized.

Hi Daniel,

Currently, deferred struct pages are initialized in parallel only on NUMA machines. I would like to make a change to use all the available CPUs even on a single socket systems, but that is not there yet. So, I believe Jia's performance improvements are still relevant.

Thank you,
Pavel

^ permalink raw reply

* [PATCH net-next v2 01/13] net: phy: sfp: make the i2c-bus property really optional
From: Florian Fainelli @ 2018-05-04 17:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504135643.23466-2-antoine.tenart@bootlin.com>

On 05/04/2018 06:56 AM, Antoine Tenart wrote:
> The SFF,SFP documentation is clear about making all the DT properties,
> with the exception of the compatible, optional. In practice this is not
> the case and without an i2c-bus property provided the SFP code will
> throw NULL pointer exceptions.
> 
> This patch is an attempt to fix this.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
> ---
>  drivers/net/phy/sfp.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index 4ab6e9a50bbe..4686c443fc22 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -298,11 +298,17 @@ static void sfp_set_state(struct sfp *sfp, unsigned int state)
>  
>  static int sfp_read(struct sfp *sfp, bool a2, u8 addr, void *buf, size_t len)
>  {
> +	if (!sfp->read)
> +		return -EOPNOTSUPP;

-ENODEV would be closer to the intended meaning IMHO, those this could
be argue that this is yet another color to paint the bikeshed with.

> +
>  	return sfp->read(sfp, a2, addr, buf, len);
>  }
>  
>  static int sfp_write(struct sfp *sfp, bool a2, u8 addr, void *buf, size_t len)
>  {
> +	if (!sfp->write)
> +		return -EOPNOTSUPP;
> +
>  	return sfp->write(sfp, a2, addr, buf, len);
>  }
>  
> @@ -533,6 +539,8 @@ static int sfp_sm_mod_hpower(struct sfp *sfp)
>  		return 0;
>  
>  	err = sfp_read(sfp, true, SFP_EXT_STATUS, &val, sizeof(val));
> +	if (err == -EOPNOTSUPP)
> +		goto err;
>  	if (err != sizeof(val)) {
>  		dev_err(sfp->dev, "Failed to read EEPROM: %d\n", err);
>  		err = -EAGAIN;
> @@ -542,6 +550,8 @@ static int sfp_sm_mod_hpower(struct sfp *sfp)
>  	val |= BIT(0);
>  
>  	err = sfp_write(sfp, true, SFP_EXT_STATUS, &val, sizeof(val));
> +	if (err == -EOPNOTSUPP)
> +		goto err;
>  	if (err != sizeof(val)) {
>  		dev_err(sfp->dev, "Failed to write EEPROM: %d\n", err);
>  		err = -EAGAIN;
> @@ -565,6 +575,8 @@ static int sfp_sm_mod_probe(struct sfp *sfp)
>  	int ret;
>  
>  	ret = sfp_read(sfp, false, 0, &id, sizeof(id));
> +	if (ret == -EOPNOTSUPP)
> +		return ret;

Can you find a way such that only sfp_sm_mod_probe() needs to check
whether the sfp read/write operations returned failure and then we just
make sure the SFP state machine does not make any more progress? Having
to check the sfp_read()/sfp_write() operations all over the place sounds
error prone and won't scale in the future.
-- 
Florian

^ permalink raw reply

* [PATCH net-next v2 02/13] net: phy: sfp: handle non-wired SFP connectors
From: Florian Fainelli @ 2018-05-04 17:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504135643.23466-3-antoine.tenart@bootlin.com>

On 05/04/2018 06:56 AM, Antoine Tenart wrote:
> SFP connectors can be solder on a board without having any of their pins
> (LOS, i2c...) wired. In such cases the SFP link state cannot be guessed,
> and the overall link status reporting is left to other layers.
> 
> In order to achieve this, a new SFP_DEV status is added, named UNKNOWN.
> This mode is set when it is not possible for the SFP code to get the
> link status and as a result the link status is reported to be always UP
> from the SFP point of view.

Why represent the SFP in Device Tree then? Why not just declare this is
a fixed link which would avoid having to introduce this "unknown" state.
-- 
Florian

^ permalink raw reply

* [PATCH net-next v2 03/13] net: phy: sfp: warn the user when no tx_disable pin is available
From: Florian Fainelli @ 2018-05-04 17:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180504135643.23466-4-antoine.tenart@bootlin.com>

On 05/04/2018 06:56 AM, Antoine Tenart wrote:
> In case no Tx disable pin is available the SFP modules will always be
> emitting. This could be an issue when using modules using laser as their
> light source as we would have no way to disable it when the fiber is
> removed. This patch adds a warning when registering an SFP cage which do
> not have its tx_disable pin wired or available.

Is this something that was done in a possibly earlier revision of a
given board design and which was finally fixed? Nothing wrong with the
patch, but this seems like a pretty serious board design mistake, that
needs to be addressed.
-- 
Florian

^ permalink raw reply

* [PATCH v4 2/2] ThunderX2: Add Cavium ThunderX2 SoC UNCORE PMU driver
From: Robin Murphy @ 2018-05-04 17:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180503193023.9c27512d6a0e9af85f8c0407@arm.com>

Hi Kim,

On 04/05/18 01:30, Kim Phillips wrote:
> On Tue, 1 May 2018 12:54:05 +0100
> Will Deacon <will.deacon@arm.com> wrote:
> 
>> Hi Kim,
> 
> Hi Will, thanks for responding.
> 
>> On Fri, Apr 27, 2018 at 11:56:25AM -0500, Kim Phillips wrote:
>>> On Fri, 27 Apr 2018 17:09:14 +0100
>>> Will Deacon <will.deacon@arm.com> wrote:
>>>> On Fri, Apr 27, 2018 at 10:46:29AM -0500, Kim Phillips wrote:
>>>>> On Fri, 27 Apr 2018 15:37:20 +0100
>>>>> Will Deacon <will.deacon@arm.com> wrote:
>>>>>> For anything under drivers/perf/, I'd prefer not to have these prints
>>>>>> and instead see efforts to improve error reporting via the perf system
>>>>>> call interface.
>>>>>
>>>>> We'd all prefer that, and for all PMU drivers, why should ones under
>>>>> drivers/perf be treated differently?
>>>>
>>>> Because they're the ones I maintain...
>>>
>>> You represent a minority on your opinion on this matter though.
>>
>> I'm not sure that's true
> 
> I haven't seen another arch/PMU driver maintainer actively oppose it
> other than you (and Mark).
> 
>> -- you tend to be the only one raising this issue;
> 
> If you're basing that on list activity, I wouldn't, since most Arm perf
> users don't subscribe to LKML.  I'm trying to represent current and
> future Arm perf users that shouldn't be expected to hang out here on
> LKML and review PMU drivers.  Couldn't tell you why PMU driver authors
> themselves don't chime in, but will note that some drivers in
> drivers/perf do print things from their event_init functions.

As a PMU driver author who *has* already invested not-inconsiderable 
time and effort in trying to explain to you the technical issues from a 
non-maintainer perspective (and is about to do so again), I can't help 
but feel rather slighted by that comment.

> Also, you have been cc'd on off-list email threads where SPE users had
> to debug the SPE driver's event_init() manually.  There are other SPE
> users I know that have had to go through the same painstaking process of
> debugging the SPE driver.  Last but not least, there's me, and I'd sure
> appreciate more verbose PMU drivers, based on my real-life performance
> monitoring experience(s).
> 
>> I think most people don't actually care one way or the other. If you know of
> 
> Like I said, I think you're limiting your audience to LKML subscribers,
> who are more amenable to being convinced they need to debug their
> kernel.
> 
>> others who care about it too then I suggest you work together as a group to
>> solve the problem in a way which is amenable to the upstream maintainers.
> 
> I know a few off-list people that would like a more user-friendly Arm
> perf experience, but we're not qualified to solve the larger problem
> that perf has had since its inception, and I think it's a little unfair
> for you to suggest that, esp. given you're one of the maintainers, and
> there's Linus on top of you.
> 
>> Then you won't have to worry about fixing it personally. You could even
>> propose a topic for plumbers if there is enough interest in a general
>> framework for propagating error messages to userspace.
> 
> I'm not worried about fixing it personally or not.  I'm worried about
> our perf users' experience given your objection to using the vehicle
> already in use on other architectures and PMU drivers.
> 
> If you - or anyone else for that matter - know of a way that'll get us
> past this, by all means, say something.
> 
> If it helps, I recently asked acme if we could borrow bits from struct
> perf_event and got no response.
> 
>>>>> As you are already aware, I've personally tried to fix this problem -
>>>>> that has existed since before the introduction of the perf tool (I
>>>>> consider it a syscall-independent enhanced error interface), multiple
>>>>> times, and failed.
>>>>
>>>> Why is that my problem? Try harder?
>>>
>>> It's your problem because we're here reviewing a patch that happens to
>>> fall under your maintainership.  I'll be the first person to tell you
>>> I'm obviously incompetent and haven't been able to come up with a
>>> solution that is acceptable for everyone up to and including Linus
>>> Torvalds.  I'm just noticing a chronic usability problem that can be
>>> easily alleviated in the context of this patch review.
>>
>> I don't see how incompetence has anything to do with it and, like most
> 
> Well you told me to try harder, and I'm trying to let you know that I
> can try harder - ad infinitum - and still not get it, so telling me to
> try harder isn't going to necessarily resolve the issue.
> 
>> people (lucky gits...), I doubt Torvalds cares too deeply about PMU driver
>> internals. No arguments on the usability problem, but this ain't the fix
> 
> Like acme, I doubt Linus runs perf on real Arm h/w, so yes, I don't
> think they care that much, but if they did try to do it one day, I'd
> like to that they'd be able to have dmesg contain that extra error
> information for them.
> 
> As you know, David Howells wrote a supplemental error syscall facility
> [1], that Linus nacked (off list no less).  Does Linus care about what
> David Howells was developing it for?  I don't know, but he does have
> enough experience to know whether a certain approach to a problem is
> sustainable in his kernel or not.
> 
>> you're looking for: it's a bodge. We've been over this many times before.
> 
> I think it's OK - necessary even - until this error infrastructure
> problem perf has is fixed, and I think you're being unrealistic if you
> think it will be fixed anytime soon.
> 
>>>>> So until someone comes up with a solution that works for everyone
>>>>> up to and including Linus Torvalds (who hasn't put up a problem
>>>>> pulling PMU drivers emitting things to dmesg so far, by the way), this
>>>>> keep PMU drivers' errors silent preference of yours is unnecessarily
>>>>> impeding people trying to measure system performance on Arm based
>>>>> machines - all other archs' maintainers are fine with PMU drivers using
>>>>> dmesg.
>>>>
>>>> Good for them, although I'm pretty sure that at least the x86 folks are
>>>> against this crap too.
>>>
>>> Unfortunately, it doesn't affect them nearly as much as it does our
>>> more diverse platforms, which is why I don't think they care to do
>>> much about it.
>>
>> x86 has its fair share of PMUs too, so I don't think we're special in this
>> regard. The main difference seems to be that they have better support in
>> the perf tool for diagnosing problems.
> 
> Indeed, their software is as vertically integrated as their hardware,
> and, well, they have more people with more experience working and caring
> about perf running and being more usable.

I note that uncore_pmu_event_init() and snb_uncore_imc_event_init(), to 
pick a couple of Intel examples, contain a lot of the same logic to the 
ARM system PMU drivers you repeatedly request should be more verbose. 
Now, surely either perf tool already has some magic which somehow makes 
those routines "user-friendly", in which case there seems no reason that 
at least any generic event attribute stuff couldn't be applied to ARM 
PMUs as well; or if not then you could easily send a patch adding 
"helpful" printk()s to those drivers too and actively solicit the 
maintainer feedback you lament not having.

>>>>>> Anyway, I think this driver has bigger problems that need addressing.
>>>>>
>>>>> To me it represents yet another PMU driver submission - as the years go
>>>>> by - that is lacking in the user messaging area.  Which reminds me, can
>>>>> you take another look at applying this?:
>>>>
>>>> As I said before, I'm not going to take anything that logs above pr_debug
>>>> for things that are directly triggerable from userspace. Spin a version
>>>
>>> Why?  There are plenty of things that emit stuff into dmesg that are
>>> directly triggerable from userspace.  Is it because it upsets fuzzing
>>> tests?  How about those be run with a patched kernel that somehow
>>> mitigates the printing?
>>
>> [we've been over this before]
>> There are two camps that might find this information useful:
>>
>>    1. People writing userspace support for a new PMU using the
>>       perf_event_open syscall
>>
>>    2. People trying to use a tool which abstracts the PMU details to some
>>       degree (e.g. perf tool)
>>
>> Those in (1) can get by with pr_debug. Sure, they have to recompile, but
>> it's not like there are many people in this camp and they'll probably be
>> working with the PMU driver writer to some degree anyway and taking new
>> kernel drops.
> 
> No, please, that's a worse user experience than necessary.
> 
> FWIW, I see this type of person as e.g., a JIT developer - a
> compiler/toolchain person - who has decided to use h/w monitoring to
> help the JIT engine find hotspots.  The first place they should start
> is to start using the perf tool itself, experimenting with events and
> PMUs of interest.  When satisfied, they should clone the parameters
> the tool makes to the perf_event_open syscall in their JIT engine.  We
> should not be wasting their time forcing them to hack around in the
> kernel trying to debug perf driver semantics.
> 
>> Those in (2) may not have access to dmesg, absolutely should not be able
> 
> I don't think you're being realistic here: we're talking about people
> trying to improve performance: They would likely have kptr_restrict ==
> 0 (unrestricted), so why would they have dmesg_restrict set?  Btw, the
> rationale for dmesg_restrict was kernel pointer visibility [2], but now
> %Kp hashes addresses before printing them, so it's even less likely to
> be used.  Curious, where have you seen dmesg_restrict being set?
> 
>> to spam it (since this could hide other important messages), will probably
> 
> How will it hide other important messages?  Run a logging daemon?
> We're not talking about a lot of messages here: one line per perf
> invocation: see e.g. runs in [2].

Assuming you meant [3], that's a pretty trivial case which doesn't 
really illustrate the whole scope of the problem.

Just for you, I added prints in 4 places where event_init validation 
returns -EINVAL in my not-yet-upstream SMMUv2 PMU driver. Let's capture 
a few events on my Juno board to get some arbitrary cross-sections of 
system-wide I/O activity:

[root at space-channel-5 ~]# perf stat -e arm_smmu/cycles/ -e 
arm_smmu/access,tcefcfg=1/ -e arm_smmu/access,tcefcfg=1,smr_id=1/ -e 
arm_smmu/tlb_alloc,tcefcfg=2,ndx=0/ -e 
arm_smmu/tlb_alloc,tcefcfg=2,ndx=1/ find /usr > /dev/null

  Performance counter stats for 'system wide':

          729128494      arm_smmu/cycles/ 
               (12.08%)
          841301489      arm_smmu/cycles/ 
               (28.10%)
          834458758      arm_smmu/cycles/ 
               (23.40%)
          774373529      arm_smmu/cycles/ 
               (15.69%)
         1028501665      arm_smmu/cycles/ 
               (13.84%)
               5401      arm_smmu/access,tcefcfg=1/ 
                (27.11%)
                  0      arm_smmu/access,tcefcfg=1/ 
                (14.73%)
                  0      arm_smmu/access,tcefcfg=1/ 
                (23.27%)
                  0      arm_smmu/access,tcefcfg=1/ 
                (20.54%)
             220788      arm_smmu/access,tcefcfg=1/ 
                (17.37%)
                  0      arm_smmu/access,tcefcfg=1,smr_id=1/ 
                         (28.18%)
                  0      arm_smmu/access,tcefcfg=1,smr_id=1/ 
                         (24.10%)
                  0      arm_smmu/access,tcefcfg=1,smr_id=1/ 
                         (12.52%)
                  0      arm_smmu/access,tcefcfg=1,smr_id=1/ 
                         (16.17%)
                  0      arm_smmu/access,tcefcfg=1,smr_id=1/ 
                         (19.71%)
                  0      arm_smmu/tlb_alloc,tcefcfg=2,ndx=0/ 
                         (19.68%)
                  0      arm_smmu/tlb_alloc,tcefcfg=2,ndx=0/ 
                         (23.69%)
                  0      arm_smmu/tlb_alloc,tcefcfg=2,ndx=0/ 
                         (29.15%)
                  0      arm_smmu/tlb_alloc,tcefcfg=2,ndx=0/ 
                         (29.19%)
              12092      arm_smmu/tlb_alloc,tcefcfg=2,ndx=0/ 
                         (27.70%)
                  0      arm_smmu/tlb_alloc,tcefcfg=2,ndx=1/ 
                         (16.97%)
                  0      arm_smmu/tlb_alloc,tcefcfg=2,ndx=1/ 
                         (13.36%)
                  0      arm_smmu/tlb_alloc,tcefcfg=2,ndx=1/ 
                         (16.01%)
                  0      arm_smmu/tlb_alloc,tcefcfg=2,ndx=1/ 
                         (24.70%)
                  0      arm_smmu/tlb_alloc,tcefcfg=2,ndx=1/ 
                         (28.39%)

        2.315150449 seconds time elapsed


Wonderful, it worked OK. And yet in that brief time it spammed 
*sixty-one* lines to the kernel log:

[ 2096.220934] event_source arm_smmu_3: Incompatible filtering mode
[ 2096.226893] event_source arm_smmu_1: Incompatible filtering mode
[ 2096.232852] event_source arm_smmu_3: Incompatible filtering mode
[ 2096.238798] event_source arm_smmu_1: Incompatible filtering mode
[ 2096.244744] event_source arm_smmu_3: Incompatible filtering mode
[ 2096.250690] event_source arm_smmu_4: Incompatible filtering mode
[ 2096.256641] event_source arm_smmu_2: Incompatible filtering mode
[ 2096.262594] event_source arm_smmu_0: Incompatible filtering mode
[ 2096.268546] event_source arm_smmu_1: Incompatible filtering mode
[ 2096.274492] event_source arm_smmu_3: Incompatible filtering mode
[ 2096.280437] event_source arm_smmu_4: Incompatible filtering mode
[ 2096.286381] event_source arm_smmu_2: Incompatible filtering mode
[ 2096.292326] event_source arm_smmu_0: Incompatible filtering mode
[ 2096.298271] event_source arm_smmu_1: Incompatible filtering mode
[ 2096.304215] event_source arm_smmu_3: Incompatible filtering mode
[ 2096.310164] event_source arm_smmu_4: Incompatible filtering mode
[ 2096.316109] event_source arm_smmu_2: Incompatible filtering mode
[ 2096.322053] event_source arm_smmu_0: Incompatible filtering mode
[ 2096.327997] event_source arm_smmu_1: Incompatible filtering mode
[ 2096.333941] event_source arm_smmu_3: Incompatible filtering mode
[ 2096.339887] event_source arm_smmu_3: Incompatible SMR value
[ 2096.345408] event_source arm_smmu_1: Incompatible filtering mode
[ 2096.351361] event_source arm_smmu_4: Incompatible filtering mode
[ 2096.357313] event_source arm_smmu_2: Incompatible filtering mode
[ 2096.363263] event_source arm_smmu_0: Incompatible filtering mode
[ 2096.369214] event_source arm_smmu_3: Incompatible filtering mode
[ 2096.375171] event_source arm_smmu_1: Incompatible filtering mode
[ 2096.381124] event_source arm_smmu_4: Incompatible filtering mode
[ 2096.387078] event_source arm_smmu_2: Incompatible filtering mode
[ 2096.393029] event_source arm_smmu_0: Incompatible filtering mode
[ 2096.398984] event_source arm_smmu_3: Incompatible filtering mode
[ 2096.404936] event_source arm_smmu_1: Incompatible filtering mode
[ 2096.410890] event_source arm_smmu_4: Incompatible filtering mode
[ 2096.416840] event_source arm_smmu_4: Incompatible context bank index
[ 2096.423130] event_source arm_smmu_2: Incompatible filtering mode
[ 2096.429075] event_source arm_smmu_0: Incompatible filtering mode
[ 2096.435019] event_source arm_smmu_1: Incompatible context bank index
[ 2096.441308] event_source arm_smmu_3: Incompatible context bank index
[ 2096.447611] event_source arm_smmu_4: Incompatible filtering mode
[ 2096.453557] event_source arm_smmu_2: Incompatible filtering mode
[ 2096.459501] event_source arm_smmu_0: Incompatible filtering mode
[ 2096.465445] event_source arm_smmu_1: Incompatible filtering mode
[ 2096.471390] event_source arm_smmu_3: Incompatible filtering mode
[ 2096.477342] event_source arm_smmu_4: Incompatible filtering mode
[ 2096.483286] event_source arm_smmu_2: Incompatible SMR value
[ 2096.488800] event_source arm_smmu_0: Incompatible SMR value
[ 2096.494316] event_source arm_smmu_2: Incompatible filtering mode
[ 2096.500319] event_source arm_smmu_0: Incompatible filtering mode
[ 2096.571799] event_source arm_smmu_1: Incompatible filtering mode
[ 2096.577749] event_source arm_smmu_3: Incompatible filtering mode
[ 2096.583705] event_source arm_smmu_4: Incompatible SMR value
[ 2096.589221] event_source arm_smmu_2: Incompatible SMR value
[ 2096.594736] event_source arm_smmu_0: Incompatible SMR value
[ 2096.600256] event_source arm_smmu_1: Incompatible SMR value
[ 2096.605770] event_source arm_smmu_3: Incompatible filtering mode
[ 2096.611718] event_source arm_smmu_4: Incompatible filtering mode
[ 2096.617663] event_source arm_smmu_2: Incompatible filtering mode
[ 2096.623607] event_source arm_smmu_0: Incompatible filtering mode
[ 2096.629550] event_source arm_smmu_1: Incompatible filtering mode
[ 2096.635494] event_source arm_smmu_3: Incompatible filtering mode
[ 2096.641443] event_source arm_smmu_4: Incompatible context bank index

(and in case it's not clear, that output is *continual* during the perf 
run; empirically, the runtime only needs to reach about 6 seconds or so 
for the dmesg ringbuffer to wrap so I have to fall back to journalctl to 
see what I missed)

Now how about we try counting those exact same events a slightly 
different way:

[root at space-channel-5 ~]# perf stat -e 
\{arm_smmu/cycles/,arm_smmu/access,tcefcfg=1/,arm_smmu/access,tcefcfg=1,smr_id=1/,arm_smmu/tlb_alloc,tcefcfg=2,ndx=0/,arm_smmu/tlb_alloc,tcefcfg=2,ndx=1/\} 
find /usr > /dev/null

  Performance counter stats for 'system wide':

      <not counted>      arm_smmu/cycles/ 

    <not supported>      arm_smmu/cycles/ 

    <not supported>      arm_smmu/cycles/ 

    <not supported>      arm_smmu/cycles/ 

    <not supported>      arm_smmu/cycles/ 

    <not supported>      arm_smmu/access,tcefcfg=1/ 

    <not supported>      arm_smmu/access,tcefcfg=1/ 

    <not supported>      arm_smmu/access,tcefcfg=1/ 

    <not supported>      arm_smmu/access,tcefcfg=1/ 

    <not supported>      arm_smmu/access,tcefcfg=1/ 

    <not supported>      arm_smmu/access,tcefcfg=1,smr_id=1/ 

    <not supported>      arm_smmu/access,tcefcfg=1,smr_id=1/ 

    <not supported>      arm_smmu/access,tcefcfg=1,smr_id=1/ 

    <not supported>      arm_smmu/access,tcefcfg=1,smr_id=1/ 

    <not supported>      arm_smmu/access,tcefcfg=1,smr_id=1/ 

    <not supported>      arm_smmu/tlb_alloc,tcefcfg=2,ndx=0/ 

    <not supported>      arm_smmu/tlb_alloc,tcefcfg=2,ndx=0/ 

    <not supported>      arm_smmu/tlb_alloc,tcefcfg=2,ndx=0/ 

    <not supported>      arm_smmu/tlb_alloc,tcefcfg=2,ndx=0/ 

    <not supported>      arm_smmu/tlb_alloc,tcefcfg=2,ndx=0/ 

    <not supported>      arm_smmu/tlb_alloc,tcefcfg=2,ndx=1/ 

    <not supported>      arm_smmu/tlb_alloc,tcefcfg=2,ndx=1/ 

    <not supported>      arm_smmu/tlb_alloc,tcefcfg=2,ndx=1/ 

    <not supported>      arm_smmu/tlb_alloc,tcefcfg=2,ndx=1/ 

    <not supported>      arm_smmu/tlb_alloc,tcefcfg=2,ndx=1/ 


        0.721310193 seconds time elapsed


Oh, that didn't work. Let's check dmesg to see why:

[ 2416.494951] event_source arm_smmu_3: Incompatible filtering mode
[ 2416.500933] event_source arm_smmu_3: Incompatible filtering mode
[ 2416.506958] event_source arm_smmu_3: Incompatible filtering mode
[ 2416.513276] event_source arm_smmu_3: Incompatible filtering mode


Well, it said the same thing continually when it worked correctly, so I 
guess it must have failed for some other reason. As the user I'm now not 
only confused but unknowingly heading towards the *wrong* conclusion 
thanks to those "helpful" messages. And that's not even contemplating 
the "what if multiple programs did this at the same time" case.

>> struggle to match an internal message from the PMU driver to their
>> invocation of the high-level tool and may well be in a multi-user
>> environment so will struggle to identify the messages that they are
>> responsible for.
> 
> Again, I think you're being unrealistic.  My experience - in
> multiple performance analysis roles - was that I was always the only
> person using the machine at the time, and can easily correlate perf
> invocations with dmesg output, particularly if I run 'dmesg -w &' prior
> to invoking perf.  In one of those roles, having to mess with the
> kernel was almost inconceivable - it literally 'just worked' for
> everything else.  I was lucky that I had prior kernel experience in
> order to debug the driver and be able find out how to invoke perf.
> 
>> What they actually want is for the perf tool to give
>> them more information, and dmesg is not the right channel for that, for
>> similar reasons.
> 
> We'd all like the perf tool to do things better, and dmesg is all we
> have for now, and now I'm sounding like I did 1 1/2 years ago.
> 
>>>> using pr_debug and I'll queue it.
>>>
>>> How about using a ratelimited dev_err variant?
>>
>> Nah, I don't want dmesg being parsed by users of perf tool. pr_debug should
>> be sufficient for perf tool developers working with a new PMU type.
> 
> OK the question was to the ratelimited part, but I think that even
> might be the default these days [4].
> 
> For reasons (different perceptions?) already mentioned above, I also
> don't agree that this has to do with just perf tool developers, and
> only when working with a new PMU type.  My main concern is new/future
> users to perf on Arm, working with any PMU type - new or old.  If there
> is a perf tool-side component to this driver, I don't see it.  Having
> said that, I think the owners of this and other PMU drivers should have
> a say in what type of experience they want for their users in this
> regard...is that fair?

As I've said before, a significant portion of what you want to improve 
involves generic perf_event properties which would already be much 
better validated in the core than by every driver individually, so the 
first straightforward step would be to improve the interface between 
drivers and perf core, such that the "hard" problem reduces to 
communicating from a single part of perf core to userspace. As it is, 
PMU drivers alone simply don't have enough context to know when an 
event_init "failure" is actually significant to the user or not, thus if 
they display user-visible messages directly then half the time the user 
will be thoroughly misled unless they are intimately familiar with how 
perf core grouping and event rotation work (see above), yet required 
knowledge of perf internals is exactly what you're arguing against!

Robin.

> 
> Thanks,
> 
> Kim
> 
> [1] https://patchwork.kernel.org/patch/9907463/
> [2] https://lwn.net/Articles/414813/
> [3] https://lkml.org/lkml/2017/11/21/385
> [4] https://lwn.net/Articles/693010/
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply

* [PATCH net-next v2 03/13] net: phy: sfp: warn the user when no tx_disable pin is available
From: Andrew Lunn @ 2018-05-04 17:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <ed267042-1a61-7fcc-13ee-3cad2c05c658@gmail.com>

On Fri, May 04, 2018 at 10:07:53AM -0700, Florian Fainelli wrote:
> On 05/04/2018 06:56 AM, Antoine Tenart wrote:
> > In case no Tx disable pin is available the SFP modules will always be
> > emitting. This could be an issue when using modules using laser as their
> > light source as we would have no way to disable it when the fiber is
> > removed. This patch adds a warning when registering an SFP cage which do
> > not have its tx_disable pin wired or available.
> 
> Is this something that was done in a possibly earlier revision of a
> given board design and which was finally fixed? Nothing wrong with the
> patch, but this seems like a pretty serious board design mistake, that
> needs to be addressed.

Hi Florian

Zii Devel B is like this. Only the "Signal Detect" pin is wired to a
GPIO.

	Andrew

^ permalink raw reply

* [PATCH net-next v2 02/13] net: phy: sfp: handle non-wired SFP connectors
From: Andrew Lunn @ 2018-05-04 17:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e338472c-1f26-e49b-9a2d-7942e6ed4288@gmail.com>

On Fri, May 04, 2018 at 10:04:48AM -0700, Florian Fainelli wrote:
> On 05/04/2018 06:56 AM, Antoine Tenart wrote:
> > SFP connectors can be solder on a board without having any of their pins
> > (LOS, i2c...) wired. In such cases the SFP link state cannot be guessed,
> > and the overall link status reporting is left to other layers.
> > 
> > In order to achieve this, a new SFP_DEV status is added, named UNKNOWN.
> > This mode is set when it is not possible for the SFP code to get the
> > link status and as a result the link status is reported to be always UP
> > from the SFP point of view.
> 
> Why represent the SFP in Device Tree then? Why not just declare this is
> a fixed link which would avoid having to introduce this "unknown" state.

Hi Antoine

I agree with Florian here.

It LOS was missing, but i2c worked, i could see some value in using
SFP, or order to be able to read the EEPROM. But if everything is
missing, fixed-link seems a better fit.

	 Andrew

^ 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