* [PATCH 1/2] arm64: dts: NS2: enable GICv2m for PAXB/PAXC interfaces
From: Jon Mason @ 2016-11-28 19:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480361491-22221-1-git-send-email-jon.mason@broadcom.com>
PAXB and PAXC PCIe interfaces on NS2 have been using the iProc event
queue to handle MSI. With the gicv2m support ready, we should now switch
to gicv2m for MSI handling
Signed-off-by: Ray Jui <ray.jui@broadcom.com>
Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
arch/arm64/boot/dts/broadcom/ns2.dtsi | 104 ++++++++++++++++++++++++++--------
1 file changed, 80 insertions(+), 24 deletions(-)
diff --git a/arch/arm64/boot/dts/broadcom/ns2.dtsi b/arch/arm64/boot/dts/broadcom/ns2.dtsi
index 4fcdeca..69775a8 100644
--- a/arch/arm64/boot/dts/broadcom/ns2.dtsi
+++ b/arch/arm64/boot/dts/broadcom/ns2.dtsi
@@ -115,7 +115,7 @@
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 0>;
- interrupt-map = <0 0 0 0 &gic GIC_SPI 281 IRQ_TYPE_NONE>;
+ interrupt-map = <0 0 0 0 &gic 0 GIC_SPI 281 IRQ_TYPE_NONE>;
linux,pci-domain = <0>;
@@ -136,18 +136,7 @@
phys = <&pci_phy0>;
phy-names = "pcie-phy";
- msi-parent = <&msi0>;
- msi0: msi at 20020000 {
- compatible = "brcm,iproc-msi";
- msi-controller;
- interrupt-parent = <&gic>;
- interrupts = <GIC_SPI 277 IRQ_TYPE_NONE>,
- <GIC_SPI 278 IRQ_TYPE_NONE>,
- <GIC_SPI 279 IRQ_TYPE_NONE>,
- <GIC_SPI 280 IRQ_TYPE_NONE>;
- brcm,num-eq-region = <1>;
- brcm,num-msi-msg-region = <1>;
- };
+ msi-parent = <&v2m0>;
};
pcie4: pcie at 50020000 {
@@ -156,7 +145,7 @@
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 0>;
- interrupt-map = <0 0 0 0 &gic GIC_SPI 305 IRQ_TYPE_NONE>;
+ interrupt-map = <0 0 0 0 &gic 0 GIC_SPI 305 IRQ_TYPE_NONE>;
linux,pci-domain = <4>;
@@ -177,16 +166,7 @@
phys = <&pci_phy1>;
phy-names = "pcie-phy";
- msi-parent = <&msi4>;
- msi4: msi at 50020000 {
- compatible = "brcm,iproc-msi";
- msi-controller;
- interrupt-parent = <&gic>;
- interrupts = <GIC_SPI 301 IRQ_TYPE_NONE>,
- <GIC_SPI 302 IRQ_TYPE_NONE>,
- <GIC_SPI 303 IRQ_TYPE_NONE>,
- <GIC_SPI 304 IRQ_TYPE_NONE>;
- };
+ msi-parent = <&v2m0>;
};
soc: soc {
@@ -331,6 +311,82 @@
<0x65260000 0x1000>;
interrupts = <GIC_PPI 9 (GIC_CPU_MASK_RAW(0xf) |
IRQ_TYPE_LEVEL_HIGH)>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x652e0000 0x80000>;
+
+ v2m0: v2m at 00000 {
+ compatible = "arm,gic-v2m-frame";
+ interrupt-parent = <&gic>;
+ msi-controller;
+ reg = <0x00000 0x1000>;
+ arm,msi-base-spi = <72>;
+ arm,msi-num-spis = <16>;
+ };
+
+ v2m1: v2m at 10000 {
+ compatible = "arm,gic-v2m-frame";
+ interrupt-parent = <&gic>;
+ msi-controller;
+ reg = <0x10000 0x1000>;
+ arm,msi-base-spi = <88>;
+ arm,msi-num-spis = <16>;
+ };
+
+ v2m2: v2m at 20000 {
+ compatible = "arm,gic-v2m-frame";
+ interrupt-parent = <&gic>;
+ msi-controller;
+ reg = <0x20000 0x1000>;
+ arm,msi-base-spi = <104>;
+ arm,msi-num-spis = <16>;
+ };
+
+ v2m3: v2m at 30000 {
+ compatible = "arm,gic-v2m-frame";
+ interrupt-parent = <&gic>;
+ msi-controller;
+ reg = <0x30000 0x1000>;
+ arm,msi-base-spi = <120>;
+ arm,msi-num-spis = <16>;
+ };
+
+ v2m4: v2m at 40000 {
+ compatible = "arm,gic-v2m-frame";
+ interrupt-parent = <&gic>;
+ msi-controller;
+ reg = <0x40000 0x1000>;
+ arm,msi-base-spi = <136>;
+ arm,msi-num-spis = <16>;
+ };
+
+ v2m5: v2m at 50000 {
+ compatible = "arm,gic-v2m-frame";
+ interrupt-parent = <&gic>;
+ msi-controller;
+ reg = <0x50000 0x1000>;
+ arm,msi-base-spi = <152>;
+ arm,msi-num-spis = <16>;
+ };
+
+ v2m6: v2m at 60000 {
+ compatible = "arm,gic-v2m-frame";
+ interrupt-parent = <&gic>;
+ msi-controller;
+ reg = <0x60000 0x1000>;
+ arm,msi-base-spi = <168>;
+ arm,msi-num-spis = <16>;
+ };
+
+ v2m7: v2m at 70000 {
+ compatible = "arm,gic-v2m-frame";
+ interrupt-parent = <&gic>;
+ msi-controller;
+ reg = <0x70000 0x1000>;
+ arm,msi-base-spi = <184>;
+ arm,msi-num-spis = <16>;
+ };
};
cci at 65590000 {
--
2.7.4
^ permalink raw reply related
* [PATCH 2/2] arm64: dts: NS2: enable PAXC on NS2 SVK
From: Jon Mason @ 2016-11-28 19:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480361491-22221-1-git-send-email-jon.mason@broadcom.com>
This enables the PAXC based PCIe root complex on NS2 SVK. The PAXC based
root complex is connected to internally emulated PCIe endpoints
Signed-off-by: Ray Jui <rjui@broadcom.com>
Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
arch/arm64/boot/dts/broadcom/ns2-svk.dts | 4 ++++
arch/arm64/boot/dts/broadcom/ns2.dtsi | 17 +++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/arch/arm64/boot/dts/broadcom/ns2-svk.dts b/arch/arm64/boot/dts/broadcom/ns2-svk.dts
index de8d379..5ae0816 100644
--- a/arch/arm64/boot/dts/broadcom/ns2-svk.dts
+++ b/arch/arm64/boot/dts/broadcom/ns2-svk.dts
@@ -76,6 +76,10 @@
status = "ok";
};
+&pcie8 {
+ status = "ok";
+};
+
&i2c0 {
status = "ok";
};
diff --git a/arch/arm64/boot/dts/broadcom/ns2.dtsi b/arch/arm64/boot/dts/broadcom/ns2.dtsi
index 69775a8..96ed47b 100644
--- a/arch/arm64/boot/dts/broadcom/ns2.dtsi
+++ b/arch/arm64/boot/dts/broadcom/ns2.dtsi
@@ -169,6 +169,23 @@
msi-parent = <&v2m0>;
};
+ pcie8: pcie at 60c00000 {
+ compatible = "brcm,iproc-pcie-paxc";
+ reg = <0 0x60c00000 0 0x1000>;
+ linux,pci-domain = <8>;
+
+ bus-range = <0x0 0x1>;
+
+ #address-cells = <3>;
+ #size-cells = <2>;
+ device_type = "pci";
+ ranges = <0x83000000 0 0x00000000 0 0x60000000 0 0x00c00000>;
+
+ status = "disabled";
+
+ msi-parent = <&v2m0>;
+ };
+
soc: soc {
compatible = "simple-bus";
#address-cells = <1>;
--
2.7.4
^ permalink raw reply related
* Adding a .platform_init callback to sdhci_arasan_ops
From: Mason @ 2016-11-28 19:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=XrJHFbpbhsJZAEE-HLMe47Ov=BgvHw8ZtfCazzT75pKQ@mail.gmail.com>
On 28/11/2016 18:46, Doug Anderson wrote:
> As argued in my original patch the field "corecfg_baseclkfreq" is
> documented in the generic Arasan document
> <https://arasan.com/wp-content/media/eMMC-5-1-Total-Solution_Rev-1-3.pdf>
> and thus is unlikely to be Rockchip specific.
I downloaded the data sheet, which doesn't mention registers,
but "pins" and "signals". Does that mean it is up to every
platform to decide how to group these wires into individual
registers?
corecfg_baseclkfreq[7:0]
Base Clock Frequency for SD Clock.
This is the frequency of the xin_clk.
How can 8 bits encode a frequency?
Is there an implicit LUT? Is it a MHz count?
"For maximum efficiency this should be around 200 MHz for eMMC
or 208MHz (for SD3.0)"
Regards.
^ permalink raw reply
* [PATCH v9 06/11] arm/arm64: vgic: Implement VGICv3 CPU interface access
From: Christoffer Dall @ 2016-11-28 19:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479906118-15832-7-git-send-email-vijay.kilari@gmail.com>
On Wed, Nov 23, 2016 at 06:31:53PM +0530, vijay.kilari at gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>
> VGICv3 CPU interface registers are accessed using
> KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
> as 64-bit. The cpu MPIDR value is passed along with register id.
> is used to identify the cpu for registers access.
>
> The VM that supports SEIs expect it on destination machine to handle
> guest aborts and hence checked for ICC_CTLR_EL1.SEIS compatibility.
> Similarly, VM that supports Affinity Level 3 that is required for AArch64
> mode, is required to be supported on destination machine. Hence checked
> for ICC_CTLR_EL1.A3V compatibility.
>
> The CPU system register handling is spitted into two files
spitted? Did you mean 'split into' ?
> vgic-sys-reg-common.c and vgic-sys-reg-v3.c.
> The vgic-sys-reg-common.c handles read and write of VGIC CPU registers
So this is weird because everything in virt/kvm/arm/ is exactly supposed
to be common between arm and arm64 already.
I would rather that you had a copy of vgic-sys-reg-v3.c in arch/arm/kvm/
and in arch/arm64/kvm/ each taking care of its own architecture.
But note that I didn't actually require that you implemented support for
GICv3 migration on AArch32 hosts for these patches, I just didn't want
thigns to silently break.
If we cannot test the AArch32 implementation, we should potentially just
make sure that is not supported yet, return a proper error to userspace
and get the AArch64 host implementation correct.
I suggest you move your:
virt/kvm/arm/vgic/vgic-sys-reg-v3.c to
arch/arm64/kvm/vgic-sys-reg-v3.c
and rename
virt/kvm/arm/vgic/vgic-sys-reg-common.c to
virt/kvm/arm/vgic/vgic-sys-reg-v3.c
And then wait with the AArch32 host side for now, but just make sure it
compiles and returns an error as opposed to crashing the system if
someone tries to excercise this interface on an AArch32 host.
> for both AArch64 and AArch32 mode. The vgic-sys-reg-v3.c handles AArch64
> mode and is compiled only for AArch64 mode.
>
> Updated arch/arm/include/uapi/asm/kvm.h with new definitions
> required to compile for AArch32.
>
> The version of VGIC v3 specification is define here
> Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> ---
> arch/arm/include/uapi/asm/kvm.h | 2 +
> arch/arm64/include/uapi/asm/kvm.h | 3 +
> arch/arm64/kvm/Makefile | 2 +
> include/kvm/arm_vgic.h | 9 ++
> virt/kvm/arm/vgic/vgic-kvm-device.c | 28 ++++
> virt/kvm/arm/vgic/vgic-mmio-v3.c | 18 +++
> virt/kvm/arm/vgic/vgic-sys-reg-common.c | 258 ++++++++++++++++++++++++++++++++
> virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 142 ++++++++++++++++++
> virt/kvm/arm/vgic/vgic-v3.c | 8 +
> virt/kvm/arm/vgic/vgic.h | 22 +++
> 10 files changed, 492 insertions(+)
>
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index 0ae6035..98658d9d 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -186,9 +186,11 @@ struct kvm_arch_memory_slot {
> (0xffffffffULL << KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT)
> #define KVM_DEV_ARM_VGIC_OFFSET_SHIFT 0
> #define KVM_DEV_ARM_VGIC_OFFSET_MASK (0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
> +#define KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK (0xffff)
> #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS 3
> #define KVM_DEV_ARM_VGIC_GRP_CTRL 4
> #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
> +#define KVM_DEV_ARM_VGIC_CPU_SYSREGS 6
> #define KVM_DEV_ARM_VGIC_CTRL_INIT 0
>
> /* KVM_IRQ_LINE irq field index values */
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 56dc08d..91c7137 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -206,9 +206,12 @@ struct kvm_arch_memory_slot {
> (0xffffffffULL << KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT)
> #define KVM_DEV_ARM_VGIC_OFFSET_SHIFT 0
> #define KVM_DEV_ARM_VGIC_OFFSET_MASK (0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
> +#define KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK (0xffff)
> #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS 3
> #define KVM_DEV_ARM_VGIC_GRP_CTRL 4
> #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
> +#define KVM_DEV_ARM_VGIC_CPU_SYSREGS 6
> +
> #define KVM_DEV_ARM_VGIC_CTRL_INIT 0
>
> /* Device Control API on vcpu fd */
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index d50a82a..5c8580e 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -32,5 +32,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o
> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o
> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-its.o
> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o
> +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-sys-reg-common.o
> +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-sys-reg-v3.o
> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o
> kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 002f092..730a18a 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -71,6 +71,9 @@ struct vgic_global {
>
> /* GIC system register CPU interface */
> struct static_key_false gicv3_cpuif;
> +
> + /* Cache ICH_VTR_EL2 reg value */
> + u32 ich_vtr_el2;
> };
>
> extern struct vgic_global kvm_vgic_global_state;
> @@ -269,6 +272,12 @@ struct vgic_cpu {
> u64 pendbaser;
>
> bool lpis_enabled;
> +
> + /* Cache guest priority bits */
> + u32 num_pri_bits;
> +
> + /* Cache guest interrupt ID bits */
> + u32 num_id_bits;
> };
>
> extern struct static_key_false vgic_v2_cpuif_trap;
> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
> index bc7de95..b6266fe 100644
> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
> @@ -16,6 +16,7 @@
> #include <linux/kvm_host.h>
> #include <kvm/arm_vgic.h>
> #include <linux/uaccess.h>
> +#include <asm/kvm_emulate.h>
> #include <asm/kvm_mmu.h>
> #include "vgic.h"
>
> @@ -501,6 +502,14 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
> if (!is_write)
> *reg = tmp32;
> break;
> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> + u64 regid;
> +
> + regid = (attr->attr & KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK);
> + ret = vgic_v3_cpu_sysregs_uaccess(vcpu, is_write,
> + regid, reg);
> + break;
> + }
> default:
> ret = -EINVAL;
> break;
> @@ -534,6 +543,15 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
> reg = tmp32;
> return vgic_v3_attr_regs_access(dev, attr, ®, true);
> }
> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> + u64 __user *uaddr = (u64 __user *)(long)attr->addr;
> + u64 reg;
> +
> + if (get_user(reg, uaddr))
> + return -EFAULT;
> +
> + return vgic_v3_attr_regs_access(dev, attr, ®, true);
> + }
> }
> return -ENXIO;
> }
> @@ -560,6 +578,15 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
> tmp32 = reg;
> return put_user(tmp32, uaddr);
> }
> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> + u64 __user *uaddr = (u64 __user *)(long)attr->addr;
> + u64 reg;
> +
> + ret = vgic_v3_attr_regs_access(dev, attr, ®, false);
> + if (ret)
> + return ret;
> + return put_user(reg, uaddr);
> + }
> }
>
> return -ENXIO;
> @@ -578,6 +605,7 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
> break;
> case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
> case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS:
> return vgic_v3_has_attr_regs(dev, attr);
> case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
> return 0;
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> index 2a7cd62..2f7b4ed 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> @@ -641,6 +641,24 @@ int vgic_v3_has_attr_regs(struct kvm_device *dev, struct kvm_device_attr *attr)
> nr_regions = ARRAY_SIZE(vgic_v3_rdbase_registers);
> break;
> }
> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> + u64 reg, id;
> + unsigned long vgic_mpidr, mpidr_reg;
> + struct kvm_vcpu *vcpu;
> +
> + vgic_mpidr = (attr->attr & KVM_DEV_ARM_VGIC_V3_MPIDR_MASK) >>
> + KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT;
> +
> + /* Convert plain mpidr value to MPIDR reg format */
> + mpidr_reg = VGIC_TO_MPIDR(vgic_mpidr);
> +
> + vcpu = kvm_mpidr_to_vcpu(dev->kvm, mpidr_reg);
> + if (!vcpu)
> + return -EINVAL;
> +
> + id = (attr->attr & KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK);
> + return vgic_v3_has_cpu_sysregs_attr(vcpu, 0, id, ®);
> + }
> default:
> return -ENXIO;
> }
> diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-common.c b/virt/kvm/arm/vgic/vgic-sys-reg-common.c
> new file mode 100644
> index 0000000..a1fc370c
> --- /dev/null
> +++ b/virt/kvm/arm/vgic/vgic-sys-reg-common.c
> @@ -0,0 +1,258 @@
> +/*
> + * VGIC system registers handling functions
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/irqchip/arm-gic-v3.h>
> +#include <linux/kvm.h>
> +#include <linux/kvm_host.h>
> +#include <asm/kvm_emulate.h>
> +#include "vgic.h"
> +
> +bool access_gic_ctlr_reg(struct kvm_vcpu *vcpu, bool is_write,
> + unsigned long *reg)
> +{
> + struct vgic_cpu *vgic_v3_cpu = &vcpu->arch.vgic_cpu;
> + struct vgic_vmcr vmcr;
> + u64 val;
> + u32 valid_bits, seis, a3v;
> +
> + vgic_get_vmcr(vcpu, &vmcr);
> + if (is_write) {
> + val = *reg;
> +
> + /*
> + * Disallow restoring VM state if not supported by this
> + * hardware.
> + */
> + valid_bits = ((val & ICC_CTLR_EL1_PRI_BITS_MASK) >>
> + ICC_CTLR_EL1_PRI_BITS_SHIFT) + 1;
> + if (valid_bits > vgic_v3_cpu->num_pri_bits)
> + return false;
> +
> + vgic_v3_cpu->num_pri_bits = valid_bits;
> +
> + valid_bits = (val & ICC_CTLR_EL1_ID_BITS_MASK) >>
> + ICC_CTLR_EL1_ID_BITS_SHIFT;
> + if (valid_bits > vgic_v3_cpu->num_id_bits)
> + return false;
> +
> + vgic_v3_cpu->num_id_bits = valid_bits;
nit: could you just define the variables your need instead of reusing
this valid_bits thing, that would make the code a lot more readable.
> +
> + valid_bits = ((kvm_vgic_global_state.ich_vtr_el2 &
> + ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT);
it's especially confusing that you now use valid_bits for the host's
values...
> + seis = (val & ICC_CTLR_EL1_SEIS_MASK) >>
> + ICC_CTLR_EL1_SEIS_SHIFT;
> + if (valid_bits != seis)
> + return false;
> +
> + valid_bits = ((kvm_vgic_global_state.ich_vtr_el2 &
> + ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT);
> + a3v = (val & ICC_CTLR_EL1_A3V_MASK) >>
> + ICC_CTLR_EL1_A3V_SHIFT;
> + if (valid_bits != a3v)
> + return false;
> +
> + vmcr.ctlr = (val & ICC_CTLR_EL1_CBPR_MASK);
> + vmcr.ctlr |= (val & ICC_CTLR_EL1_EOImode_MASK);
> + vgic_set_vmcr(vcpu, &vmcr);
> + } else {
> + val = 0;
> + val |= (vgic_v3_cpu->num_pri_bits - 1) <<
> + ICC_CTLR_EL1_PRI_BITS_SHIFT;
> + val |= vgic_v3_cpu->num_id_bits <<
> + ICC_CTLR_EL1_ID_BITS_SHIFT;
> + val |= ((kvm_vgic_global_state.ich_vtr_el2 &
> + ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT) <<
> + ICC_CTLR_EL1_SEIS_SHIFT;
> + val |= ((kvm_vgic_global_state.ich_vtr_el2 &
> + ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT) <<
> + ICC_CTLR_EL1_A3V_SHIFT;
> + val |= (vmcr.ctlr & ICC_CTLR_EL1_CBPR_MASK);
> + val |= (vmcr.ctlr & ICC_CTLR_EL1_EOImode_MASK);
> +
> + *reg = val;
> + }
> +
> + return true;
> +}
> +
> +bool access_gic_pmr_reg(struct kvm_vcpu *vcpu, bool is_write,
> + unsigned long *reg)
> +{
> + struct vgic_vmcr vmcr;
> +
> + vgic_get_vmcr(vcpu, &vmcr);
> + if (is_write) {
> + vmcr.pmr = (*reg & ICC_PMR_EL1_MASK) >> ICC_PMR_EL1_SHIFT;
> + vgic_set_vmcr(vcpu, &vmcr);
> + } else {
> + *reg = (vmcr.pmr << ICC_PMR_EL1_SHIFT) & ICC_PMR_EL1_MASK;
> + }
> +
> + return true;
> +}
> +
> +bool access_gic_bpr0_reg(struct kvm_vcpu *vcpu, bool is_write,
> + unsigned long *reg)
> +{
> + struct vgic_vmcr vmcr;
> +
> + vgic_get_vmcr(vcpu, &vmcr);
> + if (is_write) {
> + vmcr.bpr = (*reg & ICC_BPR0_EL1_MASK) >>
> + ICC_BPR0_EL1_SHIFT;
> + vgic_set_vmcr(vcpu, &vmcr);
> + } else {
> + *reg = (vmcr.bpr << ICC_BPR0_EL1_SHIFT) & ICC_BPR0_EL1_MASK;
> + }
> +
> + return true;
> +}
> +
> +bool access_gic_bpr1_reg(struct kvm_vcpu *vcpu, bool is_write,
> + unsigned long *reg)
> +{
> + struct vgic_vmcr vmcr;
> +
> + if (!is_write)
> + *reg = 0;
> +
> + vgic_get_vmcr(vcpu, &vmcr);
> + if (!((vmcr.ctlr & ICH_VMCR_CBPR_MASK) >> ICH_VMCR_CBPR_SHIFT)) {
> + if (is_write) {
> + vmcr.abpr = (*reg & ICC_BPR1_EL1_MASK) >>
> + ICC_BPR1_EL1_SHIFT;
> + vgic_set_vmcr(vcpu, &vmcr);
> + } else {
> + *reg = (vmcr.abpr << ICC_BPR1_EL1_SHIFT) &
> + ICC_BPR1_EL1_MASK;
> + }
> + } else {
> + if (!is_write)
> + *reg = min((vmcr.bpr + 1), 7U);
> + }
> +
> + return true;
> +}
> +
> +bool access_gic_grpen0_reg(struct kvm_vcpu *vcpu, bool is_write,
> + unsigned long *reg)
> +{
> + struct vgic_vmcr vmcr;
> +
> + vgic_get_vmcr(vcpu, &vmcr);
> + if (is_write) {
> + vmcr.grpen0 = (*reg & ICC_IGRPEN0_EL1_MASK) >>
> + ICC_IGRPEN0_EL1_SHIFT;
> + vgic_set_vmcr(vcpu, &vmcr);
> + } else {
> + *reg = (vmcr.grpen0 << ICC_IGRPEN0_EL1_SHIFT) &
> + ICC_IGRPEN0_EL1_MASK;
> + }
> +
> + return true;
> +}
> +
> +bool access_gic_grpen1_reg(struct kvm_vcpu *vcpu, bool is_write,
> + unsigned long *reg)
> +{
> + struct vgic_vmcr vmcr;
> +
> + vgic_get_vmcr(vcpu, &vmcr);
> + if (is_write) {
> + vmcr.grpen1 = (*reg & ICC_IGRPEN1_EL1_MASK) >>
> + ICC_IGRPEN1_EL1_SHIFT;
> + vgic_set_vmcr(vcpu, &vmcr);
> + } else {
> + *reg = (vmcr.grpen1 << ICC_IGRPEN1_EL1_SHIFT) &
> + ICC_IGRPEN1_EL1_MASK;
> + }
> +
> + return true;
> +}
> +
> +static void vgic_v3_access_apr_reg(struct kvm_vcpu *vcpu, bool is_write,
> + u8 apr, u8 idx, unsigned long *reg)
> +{
> + struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
> + uint32_t *ap_reg;
> +
> + if (apr)
> + ap_reg = &vgicv3->vgic_ap1r[idx];
> + else
> + ap_reg = &vgicv3->vgic_ap0r[idx];
> +
> + if (is_write)
> + *ap_reg = *reg;
> + else
> + *reg = *ap_reg;
> +}
> +
> +static bool access_gic_aprn(struct kvm_vcpu *vcpu, bool is_write, u8 apr,
> + u8 idx, unsigned long *reg)
> +{
> + struct vgic_cpu *vgic_v3_cpu = &vcpu->arch.vgic_cpu;
> +
> + /* num_pri_bits are initialized with HW supported values.
> + * We can rely safely on num_pri_bits even if VM has not
> + * restored ICC_CTLR_EL1 before restoring APnR registers.
> + */
nit: commenting style
> + switch (vgic_v3_cpu->num_pri_bits) {
> + case 7:
> + vgic_v3_access_apr_reg(vcpu, is_write, apr, idx, reg);
> + break;
> + case 6:
> + if (idx > 1)
> + goto err;
> + vgic_v3_access_apr_reg(vcpu, is_write, apr, idx, reg);
> + break;
> + default:
> + if (idx > 0)
> + goto err;
> + vgic_v3_access_apr_reg(vcpu, is_write, apr, idx, reg);
> + }
It looks to me like userspace can then program active priorities with
higher numbers than what it will program num_pri_bits to later. Is that
not weird, or am I missing something?
> +
> + return true;
> +err:
> + if (!is_write)
> + *reg = 0;
> +
> + return false;
> +}
> +
> +bool access_gic_ap0r_reg(struct kvm_vcpu *vcpu, bool is_write, u8 idx,
> + unsigned long *reg)
> +{
> + return access_gic_aprn(vcpu, is_write, 0, idx, reg);
> +}
> +
> +bool access_gic_ap1r_reg(struct kvm_vcpu *vcpu, bool is_write, u8 idx,
> + unsigned long *reg)
> +{
> + return access_gic_aprn(vcpu, is_write, 1, idx, reg);
> +}
> +
> +bool access_gic_sre_reg(struct kvm_vcpu *vcpu, bool is_write,
> + unsigned long *reg)
> +{
> + struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
> +
> + /* Validate SRE bit */
> + if (is_write) {
> + if (!(*reg & ICC_SRE_EL1_SRE))
> + return false;
> + } else {
> + *reg = vgicv3->vgic_sre;
> + }
> +
> + return true;
> +}
> diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
> new file mode 100644
> index 0000000..82c2f02
> --- /dev/null
> +++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
> @@ -0,0 +1,142 @@
> +/*
> + * VGIC system registers handling functions
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/kvm.h>
> +#include <linux/kvm_host.h>
> +#include <asm/kvm_emulate.h>
> +#include "vgic.h"
> +#include "sys_regs.h"
> +
> +#define ACCESS_SYS_REG(REG) \
> +static bool access_gic_##REG##_sys_reg(struct kvm_vcpu *vcpu, \
> + struct sys_reg_params *p, \
> + const struct sys_reg_desc *r) \
> +{ \
> + unsigned long tmp; \
> + bool ret; \
> + \
> + if (p->is_write) \
> + tmp = p->regval; \
> + ret = access_gic_##REG##_reg(vcpu, p->is_write, &tmp); \
> + if (!p->is_write) \
> + p->regval = tmp; \
> + \
> + return ret; \
> +}
> +
> +ACCESS_SYS_REG(ctlr)
> +ACCESS_SYS_REG(pmr)
> +ACCESS_SYS_REG(bpr0)
> +ACCESS_SYS_REG(bpr1)
> +ACCESS_SYS_REG(sre)
> +ACCESS_SYS_REG(grpen0)
> +ACCESS_SYS_REG(grpen1)
> +
> +#define ACCESS_APNR_SYS_REG(REG) \
> +static bool access_gic_##REG##_sys_reg(struct kvm_vcpu *vcpu, \
> + struct sys_reg_params *p, \
> + const struct sys_reg_desc *r) \
> +{ \
> + unsigned long tmp; \
> + u8 idx = p->Op2 & 3; \
> + bool ret; \
> + \
> + if (p->is_write) \
> + tmp = p->regval; \
> + ret = access_gic_##REG##_reg(vcpu, p->is_write, idx, &tmp); \
> + if (!p->is_write) \
> + p->regval = tmp; \
> + \
> + return ret; \
> +}
> +
> +ACCESS_APNR_SYS_REG(ap0r)
> +ACCESS_APNR_SYS_REG(ap1r)
I don't get these indirections. Why can't you call the functions
directly?
> +
> +static const struct sys_reg_desc gic_v3_icc_reg_descs[] = {
> + /* ICC_PMR_EL1 */
> + { Op0(3), Op1(0), CRn(4), CRm(6), Op2(0), access_gic_pmr_sys_reg },
> + /* ICC_BPR0_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(3), access_gic_bpr0_sys_reg },
> + /* ICC_AP0R0_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(4), access_gic_ap0r_sys_reg },
> + /* ICC_AP0R1_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(5), access_gic_ap0r_sys_reg },
> + /* ICC_AP0R2_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(6), access_gic_ap0r_sys_reg },
> + /* ICC_AP0R3_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(7), access_gic_ap0r_sys_reg },
> + /* ICC_AP1R0_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(0), access_gic_ap1r_sys_reg },
> + /* ICC_AP1R1_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(1), access_gic_ap1r_sys_reg },
> + /* ICC_AP1R2_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(2), access_gic_ap1r_sys_reg },
> + /* ICC_AP1R3_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(3), access_gic_ap1r_sys_reg },
> + /* ICC_BPR1_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(3), access_gic_bpr1_sys_reg },
> + /* ICC_CTLR_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(4), access_gic_ctlr_sys_reg },
> + /* ICC_SRE_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(5), access_gic_sre_sys_reg },
> + /* ICC_IGRPEN0_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(6), access_gic_grpen0_sys_reg },
> + /* ICC_GRPEN1_EL1 */
> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(7), access_gic_grpen1_sys_reg },
> +};
> +
> +int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
> + u64 *reg)
> +{
> + struct sys_reg_params params;
> + u64 sysreg = (id & KVM_DEV_ARM_VGIC_SYSREG_MASK) | KVM_REG_SIZE_U64;
> +
> + params.regval = *reg;
> + params.is_write = is_write;
> + params.is_aarch32 = false;
> + params.is_32bit = false;
> +
> + if (find_reg_by_id(sysreg, ¶ms, gic_v3_icc_reg_descs,
> + ARRAY_SIZE(gic_v3_icc_reg_descs)))
> + return 0;
> +
> + return -ENXIO;
> +}
> +
> +int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write, u64 id,
> + u64 *reg)
> +{
> + struct sys_reg_params params;
> + const struct sys_reg_desc *r;
> + u64 sysreg = (id & KVM_DEV_ARM_VGIC_SYSREG_MASK) | KVM_REG_SIZE_U64;
> +
> + if (is_write)
> + params.regval = *reg;
> + params.is_write = is_write;
> + params.is_aarch32 = false;
> + params.is_32bit = false;
> +
> + r = find_reg_by_id(sysreg, ¶ms, gic_v3_icc_reg_descs,
> + ARRAY_SIZE(gic_v3_icc_reg_descs));
> + if (!r)
> + return -ENXIO;
> +
> + if (!r->access(vcpu, ¶ms, r))
> + return -EINVAL;
> +
> + if (!is_write)
> + *reg = params.regval;
> +
> + return 0;
> +}
> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
> index a3ff04b..6e7400e 100644
> --- a/virt/kvm/arm/vgic/vgic-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-v3.c
> @@ -240,6 +240,13 @@ void vgic_v3_enable(struct kvm_vcpu *vcpu)
> vgic_v3->vgic_sre = 0;
> }
>
> + vcpu->arch.vgic_cpu.num_id_bits = (kvm_vgic_global_state.ich_vtr_el2 &
> + ICH_VTR_ID_BITS_MASK) >>
> + ICH_VTR_ID_BITS_SHIFT;
> + vcpu->arch.vgic_cpu.num_pri_bits = ((kvm_vgic_global_state.ich_vtr_el2 &
> + ICH_VTR_PRI_BITS_MASK) >>
> + ICH_VTR_PRI_BITS_SHIFT) + 1;
> +
> /* Get the show on the road... */
> vgic_v3->vgic_hcr = ICH_HCR_EN;
> }
> @@ -340,6 +347,7 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
> */
> kvm_vgic_global_state.nr_lr = (ich_vtr_el2 & 0xf) + 1;
> kvm_vgic_global_state.can_emulate_gicv2 = false;
> + kvm_vgic_global_state.ich_vtr_el2 = ich_vtr_el2;
>
> if (!info->vcpu.start) {
> kvm_info("GICv3: no GICV resource entry\n");
> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
> index 9232791..af23399 100644
> --- a/virt/kvm/arm/vgic/vgic.h
> +++ b/virt/kvm/arm/vgic/vgic.h
> @@ -140,6 +140,28 @@ int vgic_v3_dist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
> int offset, u32 *val);
> int vgic_v3_redist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
> int offset, u32 *val);
> +int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write,
> + u64 id, u64 *val);
> +int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
> + u64 *reg);
> +bool access_gic_ctlr_reg(struct kvm_vcpu *vcpu, bool is_write,
> + unsigned long *reg);
> +bool access_gic_pmr_reg(struct kvm_vcpu *vcpu, bool is_write,
> + unsigned long *reg);
> +bool access_gic_bpr0_reg(struct kvm_vcpu *vcpu, bool is_write,
> + unsigned long *reg);
> +bool access_gic_bpr1_reg(struct kvm_vcpu *vcpu, bool is_write,
> + unsigned long *reg);
> +bool access_gic_grpen0_reg(struct kvm_vcpu *vcpu, bool is_write,
> + unsigned long *reg);
> +bool access_gic_grpen1_reg(struct kvm_vcpu *vcpu, bool is_write,
> + unsigned long *reg);
> +bool access_gic_ap0r_reg(struct kvm_vcpu *vcpu, bool is_write,
> + u8 idx, unsigned long *reg);
> +bool access_gic_ap1r_reg(struct kvm_vcpu *vcpu, bool is_write,
> + u8 idx, unsigned long *reg);
> +bool access_gic_sre_reg(struct kvm_vcpu *vcpu, bool is_write,
> + unsigned long *reg);
nit: since all of this is exported, I would name them vgic_access_ctlr()
and so on. The _reg postfix is probably also unnecessary for all of
these.
> int kvm_register_vgic_device(unsigned long type);
> void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
> void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
> --
> 1.9.1
>
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH] KVM: arm/arm64: Access CNTHCTL_EL2 bit fields correctly
From: Christoffer Dall @ 2016-11-28 19:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <799d03f5-a929-9547-1ae7-94026b76f116@arm.com>
On Mon, Nov 28, 2016 at 06:39:04PM +0000, Marc Zyngier wrote:
> On 28/11/16 17:43, Marc Zyngier wrote:
> > Hi Jintack,
> >
> > On 28/11/16 16:46, Jintack Lim wrote:
> >> Bit positions of CNTHCTL_EL2 are changing depending on HCR_EL2.E2H bit.
> >> EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is not set, but they
> >> are 11th and 10th bits respectively when E2H is set. Current code is
> >> unintentionally setting wrong bits to CNTHCTL_EL2 with E2H set, which
> >> may allow guest OS to access physical timer. So, fix it.
> >>
> >> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> >> ---
> >> arch/arm/include/asm/kvm_timer.h | 33 +++++++++++++++++++
> >> arch/arm64/include/asm/kvm_timer.h | 62 ++++++++++++++++++++++++++++++++++++
> >> include/clocksource/arm_arch_timer.h | 6 ++--
> >> virt/kvm/arm/hyp/timer-sr.c | 8 ++---
> >> 4 files changed, 103 insertions(+), 6 deletions(-)
> >> create mode 100644 arch/arm/include/asm/kvm_timer.h
> >> create mode 100644 arch/arm64/include/asm/kvm_timer.h
> >>
>
> [...]
>
> > We could make it nicer (read "faster") by introducing a
> > hyp_alternate_select construct that only returns a value instead
> > of calling a function. I remember writing something like that
> > at some point, and dropping it...
>
> So here's what this could look like (warning, wacky code ahead,
> though I fixed a stupid bug that was present in the previous patch).
> The generated code is quite nice (no branch, only an extra mov
> instruction on the default path)... Of course, completely untested!
Isn't this all about determining which bitmask to use, statically, once,
after the system has booted?
How about a good old fashioned static variable, or global struct like
the global one we use for the VGIC, which sets the proper mit mask
during kvm init, and the world-switch code just uses a variable?
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH v9 07/11] arm/arm64: vgic: Implement KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO ioctl
From: Christoffer Dall @ 2016-11-28 19:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479906118-15832-8-git-send-email-vijay.kilari@gmail.com>
On Wed, Nov 23, 2016 at 06:31:54PM +0530, vijay.kilari at gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>
> Userspace requires to store and restore of line_level for
> level triggered interrupts using ioctl KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO.
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> ---
> arch/arm/include/uapi/asm/kvm.h | 7 ++++++
> arch/arm64/include/uapi/asm/kvm.h | 6 +++++
> virt/kvm/arm/vgic/vgic-kvm-device.c | 49 ++++++++++++++++++++++++++++++++++++-
> virt/kvm/arm/vgic/vgic-mmio-v3.c | 11 +++++++++
> virt/kvm/arm/vgic/vgic-mmio.c | 38 ++++++++++++++++++++++++++++
> virt/kvm/arm/vgic/vgic-mmio.h | 5 ++++
> virt/kvm/arm/vgic/vgic.h | 2 ++
> 7 files changed, 117 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index 98658d9d..f347779 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -191,6 +191,13 @@ struct kvm_arch_memory_slot {
> #define KVM_DEV_ARM_VGIC_GRP_CTRL 4
> #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
> #define KVM_DEV_ARM_VGIC_CPU_SYSREGS 6
> +#define KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO 7
> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT 10
> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \
> + (0x3fffffULL << KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT)
> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK 0x3ff
> +#define VGIC_LEVEL_INFO_LINE_LEVEL 0
> +
> #define KVM_DEV_ARM_VGIC_CTRL_INIT 0
>
> /* KVM_IRQ_LINE irq field index values */
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 91c7137..4100f8c 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -211,6 +211,12 @@ struct kvm_arch_memory_slot {
> #define KVM_DEV_ARM_VGIC_GRP_CTRL 4
> #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
> #define KVM_DEV_ARM_VGIC_CPU_SYSREGS 6
> +#define KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO 7
> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT 10
> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \
> + (0x3fffffULL << KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT)
> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK 0x3ff
> +#define VGIC_LEVEL_INFO_LINE_LEVEL 0
>
> #define KVM_DEV_ARM_VGIC_CTRL_INIT 0
>
> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
> index b6266fe..52ed00b 100644
> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
> @@ -510,6 +510,25 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
> regid, reg);
> break;
> }
> + case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
> + unsigned int info, intid;
> +
> + info = (attr->attr & KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK) >>
> + KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT;
> + if (info == VGIC_LEVEL_INFO_LINE_LEVEL) {
> + if (is_write)
> + tmp32 = *reg;
> + intid = attr->attr &
> + KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK;
> + ret = vgic_v3_line_level_info_uaccess(vcpu, is_write,
> + intid, &tmp32);
> + if (!is_write)
> + *reg = tmp32;
I had a comment here about not having to use the tmp32 by modifying the
line_level_info function, that you seem to have missed.
Hint: The level info is not called from an MMIO path so you should be
able to just write it in a natural way.
> + } else {
> + ret = -EINVAL;
> + }
> + break;
> + }
> default:
> ret = -EINVAL;
> break;
> @@ -552,6 +571,17 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
>
> return vgic_v3_attr_regs_access(dev, attr, ®, true);
> }
> + case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
> + u32 __user *uaddr = (u32 __user *)(long)attr->addr;
> + u64 reg;
> + u32 tmp32;
> +
> + if (get_user(tmp32, uaddr))
> + return -EFAULT;
> +
> + reg = tmp32;
> + return vgic_v3_attr_regs_access(dev, attr, ®, true);
> + }
> }
> return -ENXIO;
> }
> @@ -587,8 +617,18 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
> return ret;
> return put_user(reg, uaddr);
> }
> - }
> + case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
> + u32 __user *uaddr = (u32 __user *)(long)attr->addr;
> + u64 reg;
> + u32 tmp32;
>
> + ret = vgic_v3_attr_regs_access(dev, attr, ®, false);
> + if (ret)
> + return ret;
> + tmp32 = reg;
> + return put_user(tmp32, uaddr);
> + }
> + }
> return -ENXIO;
> }
>
> @@ -609,6 +649,13 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
> return vgic_v3_has_attr_regs(dev, attr);
> case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
> return 0;
> + case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
> + if (((attr->attr & KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK) >>
> + KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT) ==
> + VGIC_LEVEL_INFO_LINE_LEVEL)
> + return 0;
> + break;
> + }
> case KVM_DEV_ARM_VGIC_GRP_CTRL:
> switch (attr->attr) {
> case KVM_DEV_ARM_VGIC_CTRL_INIT:
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> index 2f7b4ed..4d7d93d 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> @@ -808,3 +808,14 @@ int vgic_v3_redist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
> return vgic_uaccess(vcpu, &rd_dev, is_write,
> offset, val);
> }
> +
> +int vgic_v3_line_level_info_uaccess(struct kvm_vcpu *vcpu, bool is_write,
> + u32 intid, u32 *val)
> +{
> + if (is_write)
> + vgic_write_irq_line_level_info(vcpu, intid, *val);
> + else
> + *val = vgic_read_irq_line_level_info(vcpu, intid);
> +
> + return 0;
> +}
> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
> index f81e0e5..d602081 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio.c
> @@ -371,6 +371,44 @@ void vgic_mmio_write_config(struct kvm_vcpu *vcpu,
> }
> }
>
> +unsigned long vgic_read_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid)
> +{
> + int i;
> + unsigned long val = 0;
> +
> + for (i = 0; i < 32; i++) {
> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
> +
> + if (irq->line_level)
> + val |= (1U << i);
> +
> + vgic_put_irq(vcpu->kvm, irq);
> + }
> +
> + return val;
> +}
> +
> +void vgic_write_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid,
> + const unsigned long val)
> +{
> + int i;
> +
> + for (i = 0; i < 32; i++) {
> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
> +
> + spin_lock(&irq->irq_lock);
> + if (val & (1U << i)) {
> + irq->line_level = true;
> + vgic_queue_irq_unlock(vcpu->kvm, irq);
> + } else {
> + irq->line_level = false;
> + spin_unlock(&irq->irq_lock);
> + }
I think you also missed my comment about having to keep the pending
state in sync with the level state.
Which means you have to set the pending state when the line_level goes
up, and lower it when it goes down unless soft_pending is also set,
assuming it's configured as a level triggered interrupt.
If it's an edge-triggered interrupt, I think you only need to set the
pending state on a line being asserted and the rest should be adjusted
in case the user restores the configuration state to level triggered
later.
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH v9 0/11] arm/arm64: vgic: Implement API for vGICv3 live migration
From: Christoffer Dall @ 2016-11-28 19:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479906118-15832-1-git-send-email-vijay.kilari@gmail.com>
Hi Vijaya,
On Wed, Nov 23, 2016 at 06:31:47PM +0530, vijay.kilari at gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>
> This patchset adds API for saving and restoring
> of VGICv3 registers to support live migration with new vgic feature.
> This API definition is as per version of VGICv3 specification
> Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>
> The patch 3 & 4 are picked from the Pavel's previous implementation.
> http://www.spinics.net/lists/kvm/msg122040.html
>
> NOTE: Only compilation tested for AArch32. No hardware to test.
I did not review the 32-bit part, because if you cannot test it, I don't
think we should merge it.
I just want clarity on what happens if someone tries to use this on
32-bit, and I want to make sure it fails gracefully.
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH][v2] arm64: Add DTS support for FSL's LS1012A SoC
From: Leo Li @ 2016-11-28 19:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479320647-24460-1-git-send-email-harninder.rai@nxp.com>
On Wed, Nov 16, 2016 at 12:24 PM, Harninder Rai <harninder.rai@nxp.com> wrote:
> LS1012A features an advanced 64-bit ARM v8 CortexA53 processor
> with 32 KB of parity protected L1-I cache, 32 KB of ECC protected
> L1-D cache, as well as 256 KB of ECC protected L2 cache.
>
> Features summary
> One 64-bit ARM-v8 Cortex-A53 core with the following capabilities
> - Arranged as a cluster of one core supporting a 256 KB L2 cache with ECC
> protection
> - Speed up to 800 MHz
> - Parity-protected 32 KB L1 instruction cache and 32 KB L1 data cache
> - Neon SIMD engine
> - ARM v8 cryptography extensions
> One 16-bit DDR3L SDRAM memory controller
> ARM core-link CCI-400 cache coherent interconnect
> Cryptography acceleration (SEC)
> One Configurable x3 SerDes
> One PCI Express Gen2 controller, supporting x1 operation
> One serial ATA (SATA Gen 3.0) controller
> One USB 3.0/2.0 controller with integrated PHY
>
> Following levels of DTSI/DTS files have been created for the LS1012A
> SoC family:
>
> - fsl-ls1012a.dtsi:
> DTS-Include file for FSL LS1012A SoC.
>
> - fsl-ls1012a-frdm.dts:
> DTS file for FSL LS1012A FRDM board.
>
> - fsl-ls1012a-qds.dts:
> DTS file for FSL LS1012A QDS board.
>
> - fsl-ls1012a-rdb.dts:
> DTS file for FSL LS1012A RDB board.
>
> Signed-off-by: Harninder Rai <harninder.rai@nxp.com>
> Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya@nxp.com>
Hi Shawn,
Any feedback on this version? Is it still possible for having it in
4.9? It will be perfect that we can finalize this base platform
device tree soon so that driver developers can work on on-chip device
specific changes on top of it.
Regards,
Leo
^ permalink raw reply
* [GIT PULL] Amlogic 64-bit DT changes for v4.10, round 2
From: Kevin Hilman @ 2016-11-28 20:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <m2k2btx2c9.fsf@baylibre.com>
Arnd, Olof,
On Wed, Nov 23, 2016 at 2:23 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>
> Here's a final round of arm64 DT changes for Amlogic family SoCs.
>
I guess it's not the final. Please ignore this one. I need to respin
based on some DT comments from Rob H about node names.
Kevin
^ permalink raw reply
* [GIT PULL] Amlogic 64-bit DT changes for v4.10, round 2, v2
From: Kevin Hilman @ 2016-11-28 20:13 UTC (permalink / raw)
To: linux-arm-kernel
Arnd, Olof,
Here's a final round of arm64 DT changes for Amlogic family SoCs. This
superceeds the previous "round 2". The changes are
- Rob H. pointed out some "_" in node names that needed updates
- SCPI compatible string for pre v1.0 (legacy) devices added
Please pull.
Thanks,
Kevin
The following changes since commit ab5b24fdd2d551ea729e8e19e47811a646260331:
ARM64: dts: meson-gxbb-vega-s95: Add SD/SDIO/MMC and PWM nodes (2016-11-15 12:05:54 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-amlogic.git tags/amlogic-dt64-2-v2
for you to fetch changes up to c681ca42bfb74f74ac3187ca5289df25cb67b67f:
ARM64: dts: meson-gxbb: add SCPI pre-1.0 compatible (2016-11-28 12:06:31 -0800)
----------------------------------------------------------------
Amlogic 64-bit DT updates for v4.10, round 2
- new SoC support: S912/GXM series (8x A53)
- new boards: Nexbox A1 (S912), Nexbox A95X (S905X)
- resets for 2nd USB PHY
- update SCPI compatible for pre-v1.0 devices
----------------------------------------------------------------
Kevin Hilman (1):
ARM64: dts: meson-gxbb: add SCPI pre-1.0 compatible
Martin Blumenstingl (1):
ARM64: dts: meson-gxbb: add the USB reset also to the second USB PHY
Neil Armstrong (3):
ARM64: dts: Add support for Meson GXM
ARM64: dts: meson-gxm: Add support for the Nexbox A1
ARM64: dts: meson-gxl: Add support for Nexbox A95X
Documentation/devicetree/bindings/arm/amlogic.txt | 8 +++
arch/arm64/boot/dts/amlogic/Makefile | 4 ++
arch/arm64/boot/dts/amlogic/{meson-gxl-s905d-p23x.dtsi => meson-gx-p23x-q20x.dtsi} | 4 +-
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 3 +-
arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts | 205 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts | 3 +-
arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p231.dts | 3 +-
arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts | 77 +++++++++++++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxm-s912-q201.dts | 58 ++++++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxm.dtsi | 114 +++++++++++++++++++++++++++++++++++++++
11 files changed, 644 insertions(+), 4 deletions(-)
rename arch/arm64/boot/dts/amlogic/{meson-gxl-s905d-p23x.dtsi => meson-gx-p23x-q20x.dtsi} (97%)
create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxm-s912-q201.dts
create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
^ permalink raw reply
* [PATCH 2/2] Add support for the Nexbox A1 board based on the Amlogic S912 SoC.
From: Kevin Hilman @ 2016-11-28 20:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161123225205.re77xff5vcccaltl@rob-hp-laptop>
Rob Herring <robh@kernel.org> writes:
> On Mon, Nov 21, 2016 at 05:29:05PM +0100, Neil Armstrong wrote:
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> Documentation/devicetree/bindings/arm/amlogic.txt | 1 +
>> arch/arm64/boot/dts/amlogic/Makefile | 1 +
>> .../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 169 +++++++++++++++++++++
>> 3 files changed, 171 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
>
> A few nits below, otherwise:
>
> Acked-by: Rob Herring <robh@kernel.org>
>
Fixed up the changes locally, and applied for v4.10.
Kevin
^ permalink raw reply
* [PATCH V8 2/6] thermal: bcm2835: add thermal driver for bcm2835 soc
From: Eric Anholt @ 2016-11-28 20:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161125052008.GA8342@localhost.localdomain>
Eduardo Valentin <edubezval@gmail.com> writes:
> Hello,
>
> On Tue, Nov 22, 2016 at 03:28:04PM +0100, Martin Sperl wrote:
>> Hi Eduardo!
>>
>> On 19.11.2016 05:22, Eduardo Valentin wrote:
>> > Hello Martin,
>
> <cut>
>
>>
>> I was asked to implement the "initialize" case just in case FW ever
>> stopped setting up the device itself, so that is why this code is
>> included.
>
> OK. Looks like we (like, we in the Linux side) do not understand the
> conditions that firmware fails to initialize the thermal device, but we
> still want to force an initialization, hoping to get the device in a sane
> state, even if we do not know if the firmware is correctly booted or
> not. And that is done silently, with no notification to user. I see.
The firmware today always initializes thermal. I suggested adding the
init code because we (myself and the Pi Foundation) would like to reduce
how much closed firmware code is required in the platform, and the Linux
driver doing this would help make that possible in the future.
>> > Who has the ownership of this device?
>>
>> Joined ownership I suppose...
>>
>
> with no synchronization mechanism?
Correct, because none is necessary.
>> >> The above mentioned ?configuration if not running? reflect the values that
>> >> the FW is currently setting. We should not change those values as long as the
>> >> Firmware is also reading the temperature on its own.
>> >
>> > hmm.. that looks like racy to me. Again, How do you synchronize accesses to
>> > this device? What if you configure the device and right after the
>> > firmware updates the configs? How do you make sure the configs you are
>> > writing here are the same used by the firmware? What if the firmware
>> > version changes? What versions of the firmware does this driver support?
>> >
>> > Would it make sense to simply always initialize the device? Do you have
>> > a way to tell the firmware that it should not use the device?
>> >
>> > Or, if you want to keep the device driver simply being a dummy reader,
>> > would it make sense to simply avoid writing configurations to the
>> > device, and simply retry to check if the firmware gets the device
>> > initialized?
>>
>> Again: the device registers are only ever written if the device is not started
>> already. Otherwise the driver only reads for the ADC register, so there
>> is no real race here.
>>
>
> and no race?
>
> To me, there is a race when you write to the config of this device,
> given that there is no sync between the two. We do not know if the
> firmware would be still attempting to initialize the device or not, do
> we?
Either the device was initialized by the firmware before handing off to
ARM (today's firmware) or it never will be (potential future firmware).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161128/af046805/attachment.sig>
^ permalink raw reply
* Adding a .platform_init callback to sdhci_arasan_ops
From: Doug Anderson @ 2016-11-28 20:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <9c783843-d0e4-92ef-d156-6826e38d9fa8@free.fr>
Hi,
On Mon, Nov 28, 2016 at 11:32 AM, Mason <slash.tmp@free.fr> wrote:
> On 28/11/2016 18:46, Doug Anderson wrote:
>
>> As argued in my original patch the field "corecfg_baseclkfreq" is
>> documented in the generic Arasan document
>> <https://arasan.com/wp-content/media/eMMC-5-1-Total-Solution_Rev-1-3.pdf>
>> and thus is unlikely to be Rockchip specific.
>
> I downloaded the data sheet, which doesn't mention registers,
> but "pins" and "signals". Does that mean it is up to every
> platform to decide how to group these wires into individual
> registers?
As I understand it: yes. ...but remember that I'm not arasan nor even
Rockchip and I don't have any access to that they provide their IP
licensees.
That's why I structured my config the way it did. It means that no
matter how a licensee stuffs these configs into their register map we
can support it. Every configuration is individually described by a
register offset, width, and mask. Technically the width _ought_ to be
the same for all users, so I guess you could try to optimize that out.
...but it doesn't hurt the way it is.
> corecfg_baseclkfreq[7:0]
> Base Clock Frequency for SD Clock.
> This is the frequency of the xin_clk.
>
> How can 8 bits encode a frequency?
> Is there an implicit LUT? Is it a MHz count?
Use the source. :)
* The corecfg_baseclkfreq is supposed to contain the MHz of clk_xin.
It also turns out to match the SDHCI spec "Base Clock Frequency for SD
Clock". See below.
---
It actually turns out that both of the "corecfg" bits we're setting
right now for Rockchip are a bit on the silly side. All they do is
get mirrored out the other end in the "caps" register. I found that
out from folks at Rockchip much later after I wrote my patch.
So, for instance, setting corecfg_clockmultiplier (I'm told) does
nothing more than control the bits read here:
(caps[1] & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT;
I can certainly see that they are mirrored, but I have to just take it
on faith that it does nothing else.
>>> w(grf + 0xf02c, 0x00ff0011)
>>> hex((r(sdhci + 0x44) & 0x00ff0000) >> 16)
'0x11L'
...and similar for the speed:
>>> w(grf + 0xf000, 0xff009700)
>>> hex((r(sdhci + 0x40) & 0x0000ff00) >> 8)
'0x97L'
-Doug
^ permalink raw reply
* [PATCH 5/7] add bindings for stm32 IIO timer drivers
From: Rob Herring @ 2016-11-28 21:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+M3ks7MgnUJoDoNG157+MbdhgbqwzRmhPcD-uQKC7_DJVAK6A@mail.gmail.com>
On Wed, Nov 23, 2016 at 09:17:58AM +0100, Benjamin Gaignard wrote:
> If it is ok for you I will add "id" parameter in mfd driver and
> forward it to the sub-devices drivers
> to be able to distinguish the hardware blocks
Please don't top post.
No, it's not okay. If the counter sizes are different, then have a
property for the counter size. Describe how they are different without
numbering them. If you can't describe the differences, then it shouldn't
matter which ones the OS picks to use.
> 2016-11-22 18:18 GMT+01:00 Lee Jones <lee.jones@linaro.org>:
> > On Tue, 22 Nov 2016, Benjamin Gaignard wrote:
> >
> >> [snip]
> >> >> + "st,stm32-iio-timer5"
> >> >> + "st,stm32-iio-timer6"
> >> >> + "st,stm32-iio-timer7"
> >> >> + "st,stm32-iio-timer8"
> >> >> + "st,stm32-iio-timer9"
> >> >> + "st,stm32-iio-timer10"
> >> >> + "st,stm32-iio-timer11"
> >> >> + "st,stm32-iio-timer12"
> >> >> + "st,stm32-iio-timer13"
> >> >> + "st,stm32-iio-timer14"
I doubt the h/w manual calls these "IIO timers".
> >> >
> >> > We can't do this. This is a binding for a driver, not for the hardware.
> >> >
> >>
> >> Unfortunately each instance for the hardware IP have little
> >> differences like which triggers they could accept or size of the
> >> counter register,
> >> and I doesn't have value inside the hardware to distinguish them so
> >> the only way I found is to use compatible.
> >
> > Can't you represent these as properties?
> >
> > --
> > Lee Jones
> > Linaro STMicroelectronics Landing Team Lead
> > Linaro.org ? Open source software for ARM SoCs
> > Follow Linaro: Facebook | Twitter | Blog
>
>
>
> --
> Benjamin Gaignard
>
> Graphic Study Group
>
> Linaro.org ? Open source software for ARM SoCs
>
> Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd
From: Rob Herring @ 2016-11-28 21:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479871767-20160-2-git-send-email-david@lechnology.com>
On Tue, Nov 22, 2016 at 09:29:25PM -0600, David Lechner wrote:
> Device-tree bindings for TI DA8XX/OMAP-L138/AM18XX pullup/pulldown
> pinconf controller.
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
> .../devicetree/bindings/pinctrl/ti,da850-pupd.txt | 55 ++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/ti,da850-pupd.txt
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH V7 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping
From: Agustin Vega-Frias @ 2016-11-28 22:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161125114035.GA13739@red-moon>
Hi Rafael,
Can you chime in on Lorenzo's feedback and the discussion below?
It would be great if you can comment on the reason ACPI does things
in a certain way.
Hi Lorenzo,
On 2016-11-25 06:40, Lorenzo Pieralisi wrote:
> Hi Agustin,
>
> On Thu, Nov 24, 2016 at 04:15:48PM +0000, Lorenzo Pieralisi wrote:
>
> [...]
>
>> > @@ -448,6 +449,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>> > {
>> > struct acpi_resource_irq *irq;
>> > struct acpi_resource_extended_irq *ext_irq;
>> > + struct fwnode_handle *src;
>> >
>> > switch (ares->type) {
>> > case ACPI_RESOURCE_TYPE_IRQ:
>> > @@ -460,7 +462,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>> > acpi_dev_irqresource_disabled(res, 0);
>> > return false;
>> > }
>> > - acpi_dev_get_irqresource(res, irq->interrupts[index],
>> > + acpi_dev_get_irqresource(res, irq->interrupts[index], NULL,
>> > irq->triggering, irq->polarity,
>> > irq->sharable, true);
>> > break;
>> > @@ -470,7 +472,8 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>> > acpi_dev_irqresource_disabled(res, 0);
>> > return false;
>> > }
>> > - acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
>> > + src = acpi_get_irq_source_fwhandle(&ext_irq->resource_source);
>>
>> Is there a reason why we need to do the domain look-up here ?
Because we need to pass the resource down to acpi_dev_get_irqresource
which does the mapping through acpi_register_irq/acpi_register_gsi.
>>
>> I would like to understand if, by reshuffling the code (and by
>> returning
>> the resource_source to the calling code - somehow), it would be
>> possible
>> to just mirror what the OF code does in of_irq_get(), namely:
>>
>> (1) parse the irq entry -> of_irq_parse_one()
>> (2) look the domain up -> irq_find_host()
>> (3) create the mapping -> irq_create_of_mapping()
>>
>> You wrote the code already, I think it is just a matter of shuffling
>> it around (well, minus returning the resource_source to the caller
>> which is phandle equivalent in DT).
This is one area in which DT and ACPI are fundamentally different. In DT
once the flattened blob is expanded the data is fixed. In ACPI the data
returned by a method can change. In reality most methods like CRS return
constants, but given that per-spec they are methods the interpreter has
to be involved, which makes it an expensive operation. I believe that is
the reason the resource parsing code in ACPI attempts all mappings
during
the bus scan. Rafael can you comment on this?
One way to do what you suggest would be to defer IRQ mapping by, e.g.,
populating res->start with the HW IRQ number and res->end with the
fwnode.
That way we can avoid having to walk the resource buffer when a mapping
is needed. I don't think that approach would deviate much more from
the spec from what the current ahead-of-time mapping does, but it would
require more changes in the core code. An alternative would be to do
that only for resources that fail to map.
>>
>> You abstracted away (2) and (3) behind acpi_register_irq(), that
>> on anything than does not use ACPI_GENERIC_GSI is just glue code
>> to acpi_register_gsi().
>>
>> Also, it is not a question on this patch but I ask it here because it
>> is related. On ACPI you are doing the reverse of what is done in
>> DT in platform_get_irq():
>>
>> - get the resources already parsed -> platform_get_resource()
>> - if they are disabled -> acpi_irq_get()
>>
>> and I think the ordering is tied to my question above because
>> you carry out the domain look up in acpi_dev_resource_interrupt()
>> so that if for any reason it fails the corresponding resource
>> is disabled so that we try to get it again through acpi_irq_get().
>>
>> I suspect you did it this way to make sure:
>>
>> a) keep the current ACPI IRQ parsing interface changes to a mininum
>> b) avoid changing the behaviour on x86/ia64; in particular, calling
>> acpi_register_gsi() for the _same_ mapping (an IRQ that was already
>> registered at device creation resource parsing) multiple times can
>> trigger issues on x86/ia64
You are correct about my reasons. I wanted to keep ACPI core code
changes
to a minimum, and I also needed to work within the current
implementation
which uses the pre-converted IRQ resources.
>>
>> I think that's a reasonable approach but I wanted to get these
>> clarifications, I do not think you are far from getting this
>> done but since it is a significant change I think it is worth
>> discussing the points I raised above because I think the DT code
>> sequence in of_irq_get() (1-2-3 above) is cleaner from an IRQ
>> layer perspective (instead of having the domain look-up buried
>> inside the ACPI IRQ resource parsing API).
>
> I had another look and to achieve the above one way of doing that is to
> implement acpi_irq_get() only for ACPI_GENERIC_GSI and stub it out for
> !ACPI_GENERIC_GSI (ie return an error code so that on !ACPI_GENERIC_GSI
> we would fall back to current solution for ACPI). Within acpi_irq_get()
> you can easily carry out the same steps (1->2->3) above in ACPI you
> have
> the code already there I think it is easy to change the
> acpi_irq_get_cb() interface to return a filled in struct irq_fwspec and
> the interface would become identical to of_irq_get() that is an
> advantage to maintain it from an IRQ maintainership perspective I
> think,
> that's my opinion.
I think I get what you mean. I'll take a stab at implementing
acpi_irq_get()
in the way you suggest.
>
> There is still a nagging snag though. When platform devices are
> created, core ACPI code parse the resources through:
>
> acpi_dev_get_resources()
>
> and we _have_ to have way to avoid initializing IRQ resources that
> have a dependency (ie there is a resource_source pointer that is valid
> in their descriptors) that's easy to do if we think that's the right
> thing to do and can hardly break current code (which ignores the
> resource_source altogether).
I'd rather keep the core code as-is with regard to the ahead-of-time
conversion. Whether a resource source is available at the time of the
bus
scan should be transparent to the code in drivers/acpi/resource.c, and
we need the initialization as a disabled resource to signal the need
to retry anyway.
Rafael, do you have any other suggestions/feedback on how to go about
doing this?
Thanks,
Agustin
>
> It is an important difference with DT probing, where the IRQ
> resources are only created if the domain reference (ie interrupt
> controller phandle) is satisfied at of_device_alloc() time
> (see of_device_alloc()).
>
> Thoughts ? Please let me know, the code to implement what I say
> is already in these patches, it is just a matter of reshuffling it.
>
> Thanks !
> Lorenzo
--
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a
Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH] arm64: dts: Add symlinks for cros-ec-keyboard and cros-ec-sbs
From: Brian Norris @ 2016-11-28 23:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1463090530-21864-1-git-send-email-dianders@chromium.org>
Hi Olof, Arnd,
On Thu, May 12, 2016 at 03:02:10PM -0700, Doug Anderson wrote:
> We'd like to be able to use the cros-ec-keyboard.dtsi and
> cros-ec-sbs.dtsi snippets for arm64 devices. Currently those files live
> in the arm/boot/dts directory.
>
> Let's follow the convention set by commit 8ee57b8182c4 ("ARM64: dts:
> vexpress: Use a symlink to vexpress-v2m-rs1.dtsi from arch=arm") and use
> a symlink. Note that in this case we put the files in a new
> "include/common" directory since these snippets may need to be
> referenced by dts files in many different subdirectories.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> Note that, as of right now, there are no users of this. However, given
> development happening it is almost 100% certain that users will arrive
> soon. If we need to wait for the first user before landing this we can
> leave this on the back burner. If it's OK to have no users (yet), let's
> land.
This may not fit your definition of "soon", but I'm looking to start
using these files in arch/arm64/. I'll probably carry along this patch
and resend when I'm ready, but it'd be just as well if you'd merge it
now (or express a preference for a different directory structure).
> arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi | 1 +
> arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi | 1 +
> 2 files changed, 2 insertions(+)
> create mode 120000 arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi
> create mode 120000 arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi
>
> \ No newline at end of file
>
> diff --git a/arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi b/arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi
> new file mode 120000
> index 000000000000..1c1889f0a791
> --- /dev/null
> +++ b/arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi
> @@ -0,0 +1 @@
> +../../../../../arm/boot/dts/cros-ec-keyboard.dtsi
> \ No newline at end of file
> diff --git a/arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi b/arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi
> new file mode 120000
> index 000000000000..3d7ae9c88bcd
> --- /dev/null
> +++ b/arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi
> @@ -0,0 +1 @@
> +../../../../../arm/boot/dts/cros-ec-sbs.dtsi
FWIW:
Reviewed-by: Brian Norris <briannorris@chromium.org>
^ permalink raw reply
* [PATCH v5 8/8] iommu/rockchip: Enable Rockchip IOMMU on ARM64
From: Brian Norris @ 2016-11-29 0:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1466734413-7453-9-git-send-email-zhengsq@rock-chips.com>
Hi,
On Fri, Jun 24, 2016 at 10:13:33AM +0800, Shunqian Zheng wrote:
> From: Simon Xue <xxm@rock-chips.com>
>
> This patch makes it possible to compile the rockchip-iommu driver on
> ARM64, so that it can be used with 64-bit SoCs equipped with this type
> of IOMMU.
>
> Signed-off-by: Simon Xue <xxm@rock-chips.com>
> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> ---
> drivers/iommu/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index ad08603..5572621 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -218,7 +218,7 @@ config OMAP_IOMMU_DEBUG
>
> config ROCKCHIP_IOMMU
> bool "Rockchip IOMMU Support"
> - depends on ARM
> + depends on ARM || ARM64
> depends on ARCH_ROCKCHIP || COMPILE_TEST
> select IOMMU_API
> select ARM_DMA_USE_IOMMU
Whatever happened with the rest of this series? Some of the IOMMU bits
made it, but the DRM fixes never did, and so this didn't get applied.
This leaves the whole DRM stack unusable on ARM64 Rockchip systems.
The patch context has changed a bit on patch 7 (and maybe 6?), so
somebody will need to refresh those on the latest upstream.
Brian
^ permalink raw reply
* [bug report v4.8] fs/locks.c: kernel oops during posix lock stress test
From: Ming Lei @ 2016-11-29 1:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480340400.2606.10.camel@poochiereds.net>
Hi Jeff,
On Mon, Nov 28, 2016 at 9:40 PM, Jeff Layton <jlayton@poochiereds.net> wrote:
> On Mon, 2016-11-28 at 11:10 +0800, Ming Lei wrote:
>> Hi Guys,
>>
>> When I run stress-ng via the following steps on one ARM64 dual
>> socket system(Cavium Thunder), the kernel oops[1] can often be
>> triggered after running the stress test for several hours(sometimes
>> it may take longer):
>>
>> - git clone git://kernel.ubuntu.com/cking/stress-ng.git
>> - apply the attachment patch which just makes the posix file
>> lock stress test more aggressive
>> - run the test via '~/git/stress-ng$./stress-ng --lockf 128 --aggressive'
>>
>>
>> From the oops log, looks one garbage file_lock node is got
>> from the linked list of 'ctx->flc_posix' when the issue happens.
>>
>> BTW, the issue isn't observed on single socket Cavium Thunder yet,
>> and the same issue can be seen on Ubuntu Xenial(v4.4 based kernel)
>> too.
>>
>> Thanks,
>> Ming
>>
>
> Some questions just for clarification:
>
> - I assume this is being run on a local fs of some sort? ext4 or xfs or
> something?
Yes, I just tested it on local ext4, and not test it on other filesystems yet.
>
> - have you seen this on any other arch, besides ARM?
I run the same tests on x86 before, and not see the issue.
>
> The file locking code does do some lockless checking to see whether the
> i_flctx is even present and whether the list is empty in
> locks_remove_posix. It's possible we have some barrier problems there,
> but I don't quite see how that would cause us to have a corrupt lock on
> the flc_posix list.
Yeah, I looked at the function of posix_lock_inode(), seems both add and
remove are protected by the lock.
Thanks,
Ming
^ permalink raw reply
* [PATCH v4] clkdev: add devm_of_clk_get()
From: Kuninori Morimoto @ 2016-11-29 1:21 UTC (permalink / raw)
To: linux-arm-kernel
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Current Linux has of_clk_get(), but doesn't have devm_of_clk_get().
This patch adds it. It is implemeted in clk-devres.c to share
devm_clk_release().
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v3 -> v4
- git log explain why it is implemeted in clk-devres
- it is related to CONFIG_HAVE_CLK
drivers/clk/clk-devres.c | 21 +++++++++++++++++++++
include/linux/clk.h | 27 +++++++++++++++++++++++----
2 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index 8f57154..2449b25 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -53,3 +53,24 @@ void devm_clk_put(struct device *dev, struct clk *clk)
WARN_ON(ret);
}
EXPORT_SYMBOL(devm_clk_put);
+
+struct clk *devm_of_clk_get(struct device *dev,
+ struct device_node *np, int index)
+{
+ struct clk **ptr, *clk;
+
+ ptr = devres_alloc(devm_clk_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ clk = of_clk_get(np, index);
+ if (!IS_ERR(clk)) {
+ *ptr = clk;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return clk;
+}
+EXPORT_SYMBOL(devm_of_clk_get);
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 123c027..7f50c5f 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -17,8 +17,9 @@
#include <linux/notifier.h>
struct device;
-
struct clk;
+struct device_node;
+struct of_phandle_args;
/**
* DOC: clk notifier callback types
@@ -249,6 +250,21 @@ static inline void clk_unprepare(struct clk *clk)
struct clk *devm_clk_get(struct device *dev, const char *id);
/**
+ * devm_clk_get - lookup and obtain a managed reference to a clock producer.
+ * @dev: device for clock "consumer"
+ * @np: pointer to clock consumer node
+ * @index: clock index
+ *
+ * This function parses the clocks, and uses them to look up the
+ * struct clk from the registered list of clock providers by using
+ * @np and @index.
+ *
+ * The clock will automatically be freed when the device is unbound
+ * from the bus.
+ */
+struct clk *devm_of_clk_get(struct device *dev, struct device_node *np, int index);
+
+/**
* clk_enable - inform the system when the clock source should be running.
* @clk: clock source
*
@@ -432,6 +448,12 @@ static inline struct clk *devm_clk_get(struct device *dev, const char *id)
return NULL;
}
+static inline struct clk *devm_of_clk_get(struct device *dev,
+ struct device_node *np, int index)
+{
+ return NULL;
+}
+
static inline void clk_put(struct clk *clk) {}
static inline void devm_clk_put(struct device *dev, struct clk *clk) {}
@@ -501,9 +523,6 @@ static inline void clk_disable_unprepare(struct clk *clk)
clk_unprepare(clk);
}
-struct device_node;
-struct of_phandle_args;
-
#if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
struct clk *of_clk_get(struct device_node *np, int index);
struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
--
1.9.1
^ permalink raw reply related
* [PATCH V8 2/6] thermal: bcm2835: add thermal driver for bcm2835 soc
From: Eduardo Valentin @ 2016-11-29 1:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87twarz6s1.fsf@eliezer.anholt.net>
Hello Eric, Martin,
On Mon, Nov 28, 2016 at 12:30:38PM -0800, Eric Anholt wrote:
> Eduardo Valentin <edubezval@gmail.com> writes:
>
<cut>
> The firmware today always initializes thermal. I suggested adding the
> init code because we (myself and the Pi Foundation) would like to reduce
> how much closed firmware code is required in the platform, and the Linux
> driver doing this would help make that possible in the future.
Oh I see. Backup code for future chips/firmware.
>
> >> > Who has the ownership of this device?
> >>
> >> Joined ownership I suppose...
> >>
> >
> > with no synchronization mechanism?
>
> Correct, because none is necessary.
>
<cut>
>
> Either the device was initialized by the firmware before handing off to
> ARM (today's firmware) or it never will be (potential future firmware).
And do you have a way to check if the firmware has the initialization
code or not? By firmware version, for example. Or even, chip version,
maybe?
If the current firmware will always initialize the chip, I would say,
ARM should simply read the registers, no initialization, unless it is
known that the firmware intentionally left the device uninitialized.
Again, just trying to avoid obscure misbehavior, when running into a
faulty state, and running silently broken.
^ permalink raw reply
* [PATCH] arm64: head.S: Fix CNTHCTL_EL2 access on VHE system
From: Jintack Lim @ 2016-11-29 2:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <871868f8-563c-98a1-e3f7-f3ac9457265e@arm.com>
On Mon, Nov 28, 2016 at 11:56 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 28/11/16 16:43, Jintack Lim wrote:
>> From: Jintack <jintack@cs.columbia.edu>
>>
>> Bit positions of CNTHCTL_EL2 are changing depending on HCR_EL2.E2H bit.
>> EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is not set, but they
>> are 11th and 10th bits respectively when E2H is set. Current code is
>> unintentionally setting wrong bits to CNTHCTL_EL2 with E2H set.
>>
>> In fact, we don't need to set those two bits, which allow EL1 and EL0 to
>> access physical timer and counter respectively, if E2H and TGE are set
>> for the host kernel. They will be configured later as necessary. First,
>> we don't need to configure those bits for EL1, since the host kernel
>> runs in EL2. It is a hypervisor's responsibility to configure them
>> before entering a VM, which runs in EL0 and EL1. Second, EL0 accesses
>> are configured in the later stage of boot process.
>>
>> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
>> ---
>> arch/arm64/kernel/head.S | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> index 332e331..bc3d2db 100644
>> --- a/arch/arm64/kernel/head.S
>> +++ b/arch/arm64/kernel/head.S
>> @@ -524,10 +524,16 @@ set_hcr:
>> msr hcr_el2, x0
>> isb
>>
>> - /* Generic timers. */
>> + /*
>> + * Allow Non-secure EL1 and EL0 to access physical timer and counter.
>> + * This is not necessary for VHE, since the host kernel runs in EL2,
>> + * and EL0 accesses are configured in the later stage of boot process.
>> + */
>> + cbnz x2, 1f
>> mrs x0, cnthctl_el2
>> orr x0, x0, #3 // Enable EL1 physical timers
>> msr cnthctl_el2, x0
>> +1:
>> msr cntvoff_el2, xzr // Clear virtual offset
>>
>> #ifdef CONFIG_ARM_GIC_V3
>>
>
> Nice catch. It may be worth documenting that when HCR_EL2.E2H == 1,
> CNTHCTL_EL2 has the same bit layout as CNTKCTL_EL1, allowing the kernel
> designed to run at EL1 to transparently mess with the EL0 bits (as
> CNTHCTL_EL2 and CNTKCTL_EL1 are the same register in this configuration).
Hi Marc, thanks for the review. I'll add this comment and send out v2.
>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
>
^ permalink raw reply
* [PATCH v2] arm64: head.S: Fix CNTHCTL_EL2 access on VHE system
From: Jintack Lim @ 2016-11-29 2:13 UTC (permalink / raw)
To: linux-arm-kernel
From: Jintack <jintack@cs.columbia.edu>
Bit positions of CNTHCTL_EL2 are changing depending on HCR_EL2.E2H bit.
EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is not set, but they
are 11th and 10th bits respectively when E2H is set. Current code is
unintentionally setting wrong bits to CNTHCTL_EL2 with E2H set.
In fact, we don't need to set those two bits, which allow EL1 and EL0 to
access physical timer and counter respectively, if E2H and TGE are set
for the host kernel. They will be configured later as necessary. First,
we don't need to configure those bits for EL1, since the host kernel
runs in EL2. It is a hypervisor's responsibility to configure them
before entering a VM, which runs in EL0 and EL1. Second, EL0 accesses
are configured in the later stage of boot process.
Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
v2: Add comments about accessing CNTHCTL_EL2 when HCR_EL2.ECH == 1
---
arch/arm64/kernel/head.S | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 332e331..136cb0d 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -524,10 +524,21 @@ set_hcr:
msr hcr_el2, x0
isb
- /* Generic timers. */
+ /*
+ * Allow Non-secure EL1 and EL0 to access physical timer and counter.
+ * This is not necessary for VHE, since the host kernel runs in EL2,
+ * and EL0 accesses are configured in the later stage of boot process.
+ * Note that when HCR_EL2.E2H == 1, CNTHCTL_EL2 has the same bit layout
+ * as CNTKCTL_EL1, and CNTKCTL_EL1 accessing instructions are redefined
+ * to access CNTHCTL_EL2. This allows the kernel designed to run at EL1
+ * to transparently mess with the EL0 bits via CNTKCTL_EL1 access in
+ * EL2.
+ */
+ cbnz x2, 1f
mrs x0, cnthctl_el2
orr x0, x0, #3 // Enable EL1 physical timers
msr cnthctl_el2, x0
+1:
msr cntvoff_el2, xzr // Clear virtual offset
#ifdef CONFIG_ARM_GIC_V3
--
1.9.1
^ permalink raw reply related
* [PATCH v2] irqchip/gicv3-its: Enable cacheable attribute Read-allocate hints
From: Shanker Donthineni @ 2016-11-29 2:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478589498-17696-1-git-send-email-shankerd@codeaurora.org>
Hi Marc,
Please comment on this patch. I'm seeing ~1000 cpu cycles interrupt
latency improvement on Qualcomm server platforms if the ITS tables are
not in L1/L2/L3 cache. You can find the test code at
https://patchwork.kernel.org/patch/9416793 which I used to validate my
code changes.
Thanks,
Shanker
On 11/08/2016 01:18 AM, Shanker Donthineni wrote:
> Read-allocation hints are not enabled for both the GIC-ITS and GICR
> tables. This forces the hardware to always read the table contents
> from an external memory (DDR) which is slow compared to cache memory.
> Most of the tables are often read by hardware. So, it's better to
> enable Read-allocate hints in addition to Write-allocate hints in
> order to improve the GICR_PEND, GICR_PROP, Collection, Device, and
> vCPU tables lookup time.
>
> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
> ---
> Implemented a test case to prove that enabling Read Allocation hints
> improves ITS lookup time ~15% while delivering a LPI event. Used the
> ITS command INV to analyze time spent in device, collection, prop and
> pending table lookups.
>
> Pseudo code:
> Create a fake ITS device.
> Record PMU cycle counter before sending INV command.
> Build and send ITS INT command.
> ITS hardware triggers device table lookup.
> ITTE table & collection table lookup.
> ITS property table lookup.
> ITS pending table lookup.
> Deliver interrupt to CPU interface.
> do_IRQ() called.
> Measure the total CPU cycle spent to reach this point.
>
> Without ReadAllocation hints:
> /sys/kernel/debug # echo 100 > lpitest
> [ 94.693968] CPU[1] niter=100 cycles=0x8dfc0 avg=0x16b7 min=0x1652
>
> With ReadAllocation hints:
> /sys/kernel/debug # echo 100 > lpitest
> [ 98.617873] CPU[1] niter=100 cycles=0x7df49 avg=0x1427 min=0x1388
>
> drivers/irqchip/irq-gic-v3-its.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c
> b/drivers/irqchip/irq-gic-v3-its.c
> index c5dee30..227a1eb 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -961,7 +961,7 @@ static bool its_parse_baser_device(struct its_node
> *its, struct its_baser *baser
> u32 psz, u32 *order)
> {
> u64 esz = GITS_BASER_ENTRY_SIZE(its_read_baser(its, baser));
> - u64 val = GITS_BASER_InnerShareable | GITS_BASER_WaWb;
> + u64 val = GITS_BASER_InnerShareable | GITS_BASER_RaWaWb;
> u32 ids = its->device_ids;
> u32 new_order = *order;
> bool indirect = false;
> @@ -1026,7 +1026,7 @@ static int its_alloc_tables(struct its_node *its)
> u64 typer = gic_read_typer(its->base + GITS_TYPER);
> u32 ids = GITS_TYPER_DEVBITS(typer);
> u64 shr = GITS_BASER_InnerShareable;
> - u64 cache = GITS_BASER_WaWb;
> + u64 cache = GITS_BASER_RaWaWb;
> u32 psz = SZ_64K;
> int err, i;
>
> @@ -1123,7 +1123,7 @@ static void its_cpu_init_lpis(void)
> /* set PROPBASE */
> val = (page_to_phys(gic_rdists->prop_page) |
> GICR_PROPBASER_InnerShareable |
> - GICR_PROPBASER_WaWb |
> + GICR_PROPBASER_RaWaWb |
> ((LPI_NRBITS - 1) & GICR_PROPBASER_IDBITS_MASK));
>
> writeq_relaxed(val, rbase + GICR_PROPBASER);
> @@ -1148,7 +1148,7 @@ static void its_cpu_init_lpis(void)
> /* set PENDBASE */
> val = (page_to_phys(pend_page) |
> GICR_PENDBASER_InnerShareable |
> - GICR_PENDBASER_WaWb);
> + GICR_PENDBASER_RaWaWb);
>
> writeq_relaxed(val, rbase + GICR_PENDBASER);
> tmp = readq_relaxed(rbase + GICR_PENDBASER);
> @@ -1712,7 +1712,7 @@ static int __init its_probe_one(struct resource
> *res,
> goto out_free_tables;
>
> baser = (virt_to_phys(its->cmd_base) |
> - GITS_CBASER_WaWb |
> + GITS_CBASER_RaWaWb |
> GITS_CBASER_InnerShareable |
> (ITS_CMD_QUEUE_SZ / SZ_4K - 1) |
> GITS_CBASER_VALID);
--
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ziji Hu @ 2016-11-29 2:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFr8rX04iY92OeQpSkS+3HN2-FxijCCiDb2sSKvP+TZPog@mail.gmail.com>
Hi Ulf,
On 2016/11/28 23:16, Ulf Hansson wrote:
> On 28 November 2016 at 12:38, Ziji Hu <huziji@marvell.com> wrote:
>> Hi Ulf,
>>
>> On 2016/11/28 19:13, Ulf Hansson wrote:
>>>>
>>>> As you suggest, I replace mmc_wait_for_cmd() with mmc_send_tuning(), to
>>>> send commands for testing current sampling point set in our host PHY.
>>>>
>>>> According to my test result, it shows that mmc_send_tuning() can only support
>>>> tuning command (CMD21/CMD19).
>>>> As a result, we cannot use mmc_send_tuning() when card is in the speed modes
>>>> which doesn't support tuning, such as eMMC HS SDR, eMMC HS DRR and
>>>> SD SDR 12/SDR25/DDR50. Card will not response to tuning commands in those
>>>> speed modes.
>>>>
>>>> Could you please provide suggestions for the speed mode in which tuning is
>>>> not available?
>>>>
>>>
>>> Normally the mmc host driver shouldn't have to care about what the
>>> card supports, as that is the responsibility of the mmc core to
>>> manage.
>>>
>>> The host should only need to implement the ->execute_tuning() ops,
>>> which gets called when the card supports tuning (CMD19/21). Does it
>>> make sense?
>>>
>> I think it is irrelevant to tuning procedure.
>>
>> Our host requires to adjust PHY setting after each time ios setting
>> (SDCLK/bus width/speed mode) is changed.
>> The simplified sequence is:
>> mmc change ios --> mmc_set_ios() --> ->set_ios() --> after sdhci_set_ios(),
>> adjust PHY setting.
>> During PHY setting adjustment, out host driver has to send commands to
>> test current sampling point. Tuning is another independent step.
>
> For those speed modes (or other ios changes) that *don't* requires
> tuning, then what will you do when you send the command to confirm the
> change of PHY setting and it fails?
>
> My assumption is that you will fail anyway, by propagating the error
> to the mmc core. At least that what was my understanding from your
> earlier replies, right!?
>
> Then, I think there are no point having the host driver sending a
> command to confirm the PHY settings, as the mmc core will anyway
> discover if something goes wrong when the next command is sent.
>
> Please correct me if I am wrong!
>
Sorry that I didn't make myself clear.
Our host PHY delay line consists of hundreds of sampling points.
Each sampling point represents a different phase shift.
In lower speed mode, our host driver will scan the delay line.
It will select and test multiple sampling points, other than testing
only single sampling point.
If a sampling point fails to transfer cmd/data, our host driver will
move to test next sampling point, until we find out a group of successful
sampling points which can transfer cmd/data. At last we will select
a perfect one from them.
Thank you.
Best regards,
Hu Ziji
>>
>> Thus our host needs a valid command in PHY setting adjustment. Tuning command
>> can be borrowed to complete this task in SD SDR50. But for other speed mode,
>> we have to find out a valid command.
>
> I thought we agreed on this wasn't necessary? Please see my upper response.
>
> Kind regards
> Uffe
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox