* [PATCH 2/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74
From: Geetha sowjanya @ 2017-04-27 11:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1493293584-20287-1-git-send-email-gakula@caviumnetworks.com>
From: Linu Cherian <linu.cherian@cavium.com>
Cavium 99xx SMMU implementation doesn't support page 1 register space.
Based on silicon id, ARM_SMMU_PAGE0_REGS_ONLY macro is set as an errata
workaround.
This macro when set, replaces all page 1 offsets used for
EVTQ_PROD/CONS, PRIQ_PROD/CONS register access with page 0 offsets.
Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
Signed-off-by: Geetha <gakula@cavium.com>
---
Documentation/arm64/silicon-errata.txt | 1 +
drivers/acpi/arm64/iort.c | 14 +++++++++++++-
drivers/iommu/arm-smmu-v3.c | 32 +++++++++++++++++++++++++++-----
3 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 2f66683..629e2ce 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -61,6 +61,7 @@ stable kernels.
| Cavium | ThunderX GICv3 | #23154 | CAVIUM_ERRATUM_23154 |
| Cavium | ThunderX Core | #27456 | CAVIUM_ERRATUM_27456 |
| Cavium | ThunderX SMMUv2 | #27704 | N/A |
+| Cavium | ThunderX2 SMMUv3| #74 | N/A |
| | | | |
| Freescale/NXP | LS2080A/LS1043A | A-008585 | FSL_ERRATUM_A008585 |
| | | | |
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 4a5bb96..a074ce9 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -25,6 +25,7 @@
#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
+#include <asm/cputype.h>
#define IORT_TYPE_MASK(type) (1 << (type))
#define IORT_MSI_TYPE (1 << ACPI_IORT_NODE_ITS_GROUP)
@@ -669,12 +670,23 @@ static void __init arm_smmu_v3_init_resources(struct resource *res,
{
struct acpi_iort_smmu_v3 *smmu;
int num_res = 0;
+ u32 cpu_model;
+ unsigned long size = SZ_128K;
/* Retrieve SMMUv3 specific data */
smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
+ /*
+ * Override the size, for Cavium CN99xx implementations
+ * which doesn't support the page 1 SMMU register space.
+ */
+ cpu_model = read_cpuid_id() & MIDR_CPU_MODEL_MASK;
+ if (cpu_model == MIDR_THUNDERX_99XX ||
+ cpu_model == MIDR_BRCM_VULCAN)
+ size = SZ_64K;
+
res[num_res].start = smmu->base_address;
- res[num_res].end = smmu->base_address + SZ_128K - 1;
+ res[num_res].end = smmu->base_address + size - 1;
res[num_res].flags = IORESOURCE_MEM;
num_res++;
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 1dcd154..ee23ccd 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -38,6 +38,7 @@
#include <linux/platform_device.h>
#include <linux/amba/bus.h>
+#include <asm/cputype.h>
#include "io-pgtable.h"
@@ -176,15 +177,15 @@
#define ARM_SMMU_CMDQ_CONS 0x9c
#define ARM_SMMU_EVTQ_BASE 0xa0
-#define ARM_SMMU_EVTQ_PROD 0x100a8
-#define ARM_SMMU_EVTQ_CONS 0x100ac
+#define ARM_SMMU_EVTQ_PROD (page1_offset_adjust(0x100a8))
+#define ARM_SMMU_EVTQ_CONS (page1_offset_adjust(0x100ac))
#define ARM_SMMU_EVTQ_IRQ_CFG0 0xb0
#define ARM_SMMU_EVTQ_IRQ_CFG1 0xb8
#define ARM_SMMU_EVTQ_IRQ_CFG2 0xbc
#define ARM_SMMU_PRIQ_BASE 0xc0
-#define ARM_SMMU_PRIQ_PROD 0x100c8
-#define ARM_SMMU_PRIQ_CONS 0x100cc
+#define ARM_SMMU_PRIQ_PROD (page1_offset_adjust(0x100c8))
+#define ARM_SMMU_PRIQ_CONS (page1_offset_adjust(0x100cc))
#define ARM_SMMU_PRIQ_IRQ_CFG0 0xd0
#define ARM_SMMU_PRIQ_IRQ_CFG1 0xd8
#define ARM_SMMU_PRIQ_IRQ_CFG2 0xdc
@@ -412,6 +413,10 @@
#define MSI_IOVA_BASE 0x8000000
#define MSI_IOVA_LENGTH 0x100000
+#define ARM_SMMU_PAGE0_REGS_ONLY \
+ (((read_cpuid_id() & MIDR_CPU_MODEL_MASK) == MIDR_THUNDERX_99XX) \
+ || ((read_cpuid_id() & MIDR_CPU_MODEL_MASK) == MIDR_BRCM_VULCAN))
+
static bool disable_bypass;
module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
MODULE_PARM_DESC(disable_bypass,
@@ -660,6 +665,15 @@ struct arm_smmu_option_prop {
{ 0, NULL},
};
+static inline unsigned long page1_offset_adjust(
+ unsigned long off)
+{
+ if (!ARM_SMMU_PAGE0_REGS_ONLY)
+ return off;
+ else
+ return (off - SZ_64K);
+}
+
static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
{
return container_of(dom, struct arm_smmu_domain, domain);
@@ -2631,6 +2645,14 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
return ret;
}
+static unsigned long arm_smmu_resource_size(void)
+{
+ if (ARM_SMMU_PAGE0_REGS_ONLY)
+ return SZ_64K;
+ else
+ return SZ_128K;
+}
+
static int arm_smmu_device_probe(struct platform_device *pdev)
{
int irq, ret;
@@ -2649,7 +2671,7 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
/* Base address */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (resource_size(res) + 1 < SZ_128K) {
+ if (resource_size(res) + 1 < arm_smmu_resource_size()) {
dev_err(dev, "MMIO region too small (%pr)\n", res);
return -EINVAL;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 1/3] arm64: Add MIDR values for Cavium cn99xx SoCs
From: Geetha sowjanya @ 2017-04-27 11:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1493293584-20287-1-git-send-email-gakula@caviumnetworks.com>
From: Geetha <gakula@cavium.com>
Add MIDR values for Cavium cn99xx SoCs
Signed-off-by: Geetha <gakula@cavium.com>
---
arch/arm64/include/asm/cputype.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index fc50271..066fad0 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -85,6 +85,7 @@
#define CAVIUM_CPU_PART_THUNDERX 0x0A1
#define CAVIUM_CPU_PART_THUNDERX_81XX 0x0A2
+#define CAVIUM_CPU_PART_THUNDERX_99XX 0x0AF
#define BRCM_CPU_PART_VULCAN 0x516
@@ -94,6 +95,8 @@
#define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
#define MIDR_THUNDERX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
#define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
+#define MIDR_THUNDERX_99XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_99XX)
+#define MIDR_BRCM_VULCAN MIDR_CPU_MODEL(ARM_CPU_IMP_BRCM, BRCM_CPU_PART_VULCAN)
#define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR_V1)
#ifndef __ASSEMBLY__
--
1.9.1
^ permalink raw reply related
* [PATCH 0/3] Cavium ThunderX2 SMMUv3 errata workarounds
From: Geetha sowjanya @ 2017-04-27 11:46 UTC (permalink / raw)
To: linux-arm-kernel
From: Geetha <gakula@cavium.com>
Cavium CN99xx SMMUv3 implementation has two Silicon Erratas.
1. Errata ID #74
SMMU register alias Page 1 is not implemented
2. Errata ID #126
SMMU doesnt support unique IRQ lines for gerror, eventq and cmdq-sync
The following patchset does software workaround for these two erratas.
This series is based on RFC patch.
https://www.spinics.net/lists/arm-kernel/msg575739.html
As suggested by Will Deacon, code is modified to use silicon id to
enable errata#74 workaround.
Linu Cherian (1):
iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74
Geetha (2):
arm64: Add MIDR values for Cavium cn99xx SoCs
iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126
Documentation/arm64/silicon-errata.txt | 2 ++
arch/arm64/include/asm/cputype.h | 3 ++
drivers/acpi/arm64/iort.c | 14 +++++++-
drivers/iommu/arm-smmu-v3.c | 64 +++++++++++++++++++++++++++++-----
4 files changed, 73 insertions(+), 10 deletions(-)
--
1.9.1
^ permalink raw reply
* stable/linux-3.18.y build: 204 builds: 5 failed, 199 passed, 35 errors, 212 warnings (v3.18.49)
From: Arnd Bergmann @ 2017-04-27 11:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170427103348.GA9881@kroah.com>
On Thu, Apr 27, 2017 at 12:33 PM, gregkh <gregkh@linuxfoundation.org> wrote:
> On Fri, Apr 21, 2017 at 04:27:14PM +0200, Arnd Bergmann wrote:
>> On Thu, Apr 20, 2017 at 7:04 PM, kernelci.org bot <bot@kernelci.org> wrote:
>> > stable/linux-3.18.y build: 204 builds: 5 failed, 199 passed, 35 errors, 212 warnings (v3.18.49)
>>
>> I've gone through all these now and found a fix. In three cases, there is no
>> fix yet since the respective drivers got removed before the warning was
>> noticed. Do we have a policy for how to deal with those? Should I just
>> send patches to address the warnings for 3.18?
>
> I've wondered about this, and yeah, I would like to see the number drop
> to 0 if at all possible (the scsi driver will not change), so i'll be
> glad to take patches for the code that is no longer in upstream.
Ok, I'll have a go at this after the build report.
>> > drivers/scsi/advansys.c:71:2: warning: #warning this driver is still not
>> > properly converted to the DMA API [-Wcpp]
>>
>> The driver was properly converted in v4.2 and the warning removed, but the
>> conversion would be outside of stable-kernel-rules.
>
> Yeah, this one is going to have to stay as-is :(
How about just shutting up the #warning then, based on the argument that
the warning isn't helping anyone fix it, and all the other drivers that had not
been converted at the time don't come with a #warning?
>> > Section mismatches:
>> > WARNING: arch/x86/kernel/built-in.o(.text.unlikely+0x157f): Section mismatch
>> > in reference from the function cpumask_empty.constprop.3() to the variable
>> > .init.data:nmi_ipi_mask
>> > WARNING: arch/x86/built-in.o(.text.unlikely+0x189b): Section mismatch in
>> > reference from the function cpumask_empty.constprop.3() to the variable
>> > .init.data:nmi_ipi_mask
>> > WARNING: vmlinux.o(.text.unlikely+0x1962): Section mismatch in reference
>> > from the function cpumask_empty.constprop.3() to the variable
>> > .init.data:nmi_ipi_mask
>>
>> f0ba662a6e06f2 x86: Properly _init-annotate NMI selftest code
>
> That commit is from 3.4, so how can I add it to 3.18? :)
Very odd, my search was definitely going wrong there. The file that
we get the warning for was last changed in 3.6, so I accidentally looked
at ancient changes.
Upon a closer look it looks like the problem is the 'cpumask_empty()'
helper function getting uninlined with CONFIG_OPTIMIZE_INLINING,
but I cannot see why the warning is gone in later kernels. I'll probably
have to bisect it.
>> > allnoconfig (mips) ? PASS, 0 errors, 1 warning, 0 section mismatches
>> >
>> > Warnings:
>> > mm/page_alloc.c:5346:34: warning: array subscript is below array bounds
>> > [-Warray-bounds]
>>
>> Also bisected this one now, this is also missing on 3.16:
>>
>> 90cae1fe1c35 ("mm/init: fix zone boundary creation")
>
> Now applied to 3.1_8_ :)
Ok. To clarify, I was taking note of the fact this 3.16 needs it since this
is the most frequent warning I still see on Ben's 3.16.y tree, and the
last time I went through the 3.16 build report, I did not succeed in finding
this (without bisecting).
>> > ath79_defconfig (mips) ? PASS, 0 errors, 2 warnings, 0 section mismatches
>> >
>> > Warnings:
>> > arch/mips/kernel/entry.S:170: Warning: tried to set unrecognized symbol:
>> > MIPS_ISA_LEVEL_RAW
>>
>> aebac99384f7 ("MIPS: kernel: entry.S: Set correct ISA level for mips_ihb")
>
> That was in 3.18.14, what kernel are you looking at here???
For most of the changes, I tried looking at 'git log v3.18..stable/linux-4.4.y'
and immediately found the obvious fix. If that didn't help, I tried a few other
things, but I usually did not look in 3.18.y to see if it was already there
if I found something at first that looked obviously right.
This is another case where I confused the patch that introduced the
warning with the one that fixed it. This one requires a another patch that
got merged into 3.20:
be5136988e25 ("MIPS: asm: compiler: Add new macros to set ISA and arch
asm annotations")
>> > cerfcube_defconfig (arm) ? PASS, 0 errors, 2 warnings, 0 section mismatches
>> >
>> > Warnings:
>> > fs/nfsd/nfs4state.c:3781:3: warning: 'old_deny_bmap' may be used
>> > uninitialized in this function [-Wmaybe-uninitialized]
>>
>> 5368e1a6 ("nfsd: work around a gcc-5.1 warning")
It's a copy-paste mistake, missing the first digits of the commit ID,
I found the correct one now:
6ac75368e1a6 nfsd: work around a gcc-5.1 warning
> That commit id isn't in Linus's tree, where did you get it from?
>> > defconfig+CONFIG_LKDTM=y (mips) ? PASS, 0 errors, 3 warnings, 0 section
>> > mismatches
>> >
>> > Warnings:
>> > include/asm-generic/div64.h:43:28: warning: comparison of distinct pointer
>> > types lacks a cast
>> > include/asm-generic/div64.h:43:28: warning: comparison of distinct pointer
>> > types lacks a cast
>>
>> 2ae83bf93882 ("[CIFS] Fix setting time before epoch (negative time values)")
>
> That was in 3.17, are you sure you are looking at 3.18 like the subject
> says???
Another similar mistake on my end, 2ae83bf93882 introduced the problem,
the fix we need was
97c7134ae22f ("Fix signed/unsigned pointer warning")
Arnd
^ permalink raw reply
* [PATCH v5 08/22] KVM: arm64: vgic-its: Implement vgic_mmio_uaccess_write_its_creadr
From: Christoffer Dall @ 2017-04-27 11:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1492164934-988-9-git-send-email-eric.auger@redhat.com>
On Fri, Apr 14, 2017 at 12:15:20PM +0200, Eric Auger wrote:
> GITS_CREADR needs to be restored so let's implement the associated
> uaccess_write_its callback. The write only is allowed if the its
> is disabled.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
> v4 -> v5:
> - keep Stalled bit
> - vgic_mmio_uaccess_write_its_creadr can now return an error
>
> v3 -> v4:
> - REGISTER_ITS_DESC_UACCESS now introduced in this patch
> - we now check the its is disabled
> ---
> virt/kvm/arm/vgic/vgic-its.c | 42 ++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index a9a2c12..79ed1c2 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -1213,6 +1213,33 @@ static unsigned long vgic_mmio_read_its_creadr(struct kvm *kvm,
> return extract_bytes(its->creadr, addr & 0x7, len);
> }
>
> +static int vgic_mmio_uaccess_write_its_creadr(struct kvm *kvm,
> + struct vgic_its *its,
> + gpa_t addr, unsigned int len,
> + unsigned long val)
> +{
> + int ret = 0;
> + u32 reg;
> +
> + mutex_lock(&its->cmd_lock);
> +
> + if (its->enabled) {
> + ret = -EBUSY;
> + goto out;
> + }
> +
> + reg = update_64bit_reg(its->creadr, addr & 7, len, val);
you theoretically don't need this, since you prevent 32-bit accesses to
this register, but I guess it doesn't hurt...
> + if (ITS_CMD_OFFSET(reg) >= ITS_CMD_BUFFER_SIZE(its->cbaser)) {
> + ret = -EINVAL;
> + goto out;
> + }
can the creadr value be unaligned to the command size? I don't think
you check that anywhere here?
Thanks,
-Christoffer
> +
> + its->creadr = reg;
> +out:
> + mutex_unlock(&its->cmd_lock);
> + return ret;
> +}
> +
> #define BASER_INDEX(addr) (((addr) / sizeof(u64)) & 0x7)
> static unsigned long vgic_mmio_read_its_baser(struct kvm *kvm,
> struct vgic_its *its,
> @@ -1317,6 +1344,16 @@ static void vgic_mmio_write_its_ctlr(struct kvm *kvm, struct vgic_its *its,
> .its_write = wr, \
> }
>
> +#define REGISTER_ITS_DESC_UACCESS(off, rd, wr, uwr, length, acc)\
> +{ \
> + .reg_offset = off, \
> + .len = length, \
> + .access_flags = acc, \
> + .its_read = rd, \
> + .its_write = wr, \
> + .uaccess_its_write = uwr, \
> +}
> +
> static void its_mmio_write_wi(struct kvm *kvm, struct vgic_its *its,
> gpa_t addr, unsigned int len, unsigned long val)
> {
> @@ -1339,8 +1376,9 @@ static struct vgic_register_region its_registers[] = {
> REGISTER_ITS_DESC(GITS_CWRITER,
> vgic_mmio_read_its_cwriter, vgic_mmio_write_its_cwriter, 8,
> VGIC_ACCESS_64bit | VGIC_ACCESS_32bit),
> - REGISTER_ITS_DESC(GITS_CREADR,
> - vgic_mmio_read_its_creadr, its_mmio_write_wi, 8,
> + REGISTER_ITS_DESC_UACCESS(GITS_CREADR,
> + vgic_mmio_read_its_creadr, its_mmio_write_wi,
> + vgic_mmio_uaccess_write_its_creadr, 8,
> VGIC_ACCESS_64bit | VGIC_ACCESS_32bit),
> REGISTER_ITS_DESC(GITS_BASER,
> vgic_mmio_read_its_baser, vgic_mmio_write_its_baser, 0x40,
> --
> 2.5.5
>
^ permalink raw reply
* [PATCH] iommu/arm-smmu-v3: Poll for CMDQ drain completion more effectively
From: sunil.kovvuri at gmail.com @ 2017-04-27 11:13 UTC (permalink / raw)
To: linux-arm-kernel
From: Sunil Goutham <sgoutham@cavium.com>
Modified polling on CMDQ consumer similar to how polling is done for TLB SYNC
completion in SMMUv2 driver. Code changes are done with reference to
8513c8930069 iommu/arm-smmu: Poll for TLB sync completion more effectively
Poll timeout has been increased which addresses issue of 100us timeout not
sufficient, when command queue is full with TLB invalidation commands.
Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
Signed-off-by: Geetha <gakula@cavium.com>
---
drivers/iommu/arm-smmu-v3.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index d412bdd..34599d4 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -379,6 +379,9 @@
#define CMDQ_SYNC_0_CS_NONE (0UL << CMDQ_SYNC_0_CS_SHIFT)
#define CMDQ_SYNC_0_CS_SEV (2UL << CMDQ_SYNC_0_CS_SHIFT)
+#define CMDQ_DRAIN_TIMEOUT_US 1000
+#define CMDQ_SPIN_COUNT 10
+
/* Event queue */
#define EVTQ_ENT_DWORDS 4
#define EVTQ_MAX_SZ_SHIFT 7
@@ -737,7 +740,8 @@ static void queue_inc_prod(struct arm_smmu_queue *q)
*/
static int queue_poll_cons(struct arm_smmu_queue *q, bool drain, bool wfe)
{
- ktime_t timeout = ktime_add_us(ktime_get(), ARM_SMMU_POLL_TIMEOUT_US);
+ ktime_t timeout = ktime_add_us(ktime_get(), CMDQ_DRAIN_TIMEOUT_US);
+ unsigned int spin_cnt, delay = 1;
while (queue_sync_cons(q), (drain ? !queue_empty(q) : queue_full(q))) {
if (ktime_compare(ktime_get(), timeout) > 0)
@@ -746,8 +750,13 @@ static int queue_poll_cons(struct arm_smmu_queue *q, bool drain, bool wfe)
if (wfe) {
wfe();
} else {
- cpu_relax();
- udelay(1);
+ for (spin_cnt = 0;
+ spin_cnt < CMDQ_SPIN_COUNT; spin_cnt++) {
+ cpu_relax();
+ continue;
+ }
+ udelay(delay);
+ delay *= 2;
}
}
--
2.7.4
^ permalink raw reply related
* [PATCH, 3.18-stable regression] ARM: psci: fix header file
From: Arnd Bergmann @ 2017-04-27 11:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170427100124.GE1755@kroah.com>
On Thu, Apr 27, 2017 at 12:01 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Apr 21, 2017 at 03:59:17PM +0200, Arnd Bergmann wrote:
>> Commit be95485a0b828 was incorrectly backported to 3.18 and
>> now causes a different build error when CONFIG_SMP is disabled:
>>
>> arch/arm/kernel/psci.c:287:12: error: redefinition of 'psci_init'
>>
>> This version is how it should be on 3.18, with the main psci
>> code built for both SMP and UP, but the psci_smp_available()
>> function only being available for SMP builds.
>>
>> Fixes: dbcfee724255 ("ARM: 8457/1: psci-smp is built only for SMP")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> arch/arm/include/asm/psci.h | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> This should have already been fixed in 3.18.50 as dbcfee724255 was
> reverted there. Because of that, this patch does not apply to the
> 3.18-stable tree.
Confirmed. The report was for 3.18.49 and I already noticed the error
is gone in the 3.18.50 kernelci build report.
Arnd
^ permalink raw reply
* [PATCH] mtd: spi-nor: stm32: remove broken MODULE_ALIAS
From: Arnd Bergmann @ 2017-04-27 11:09 UTC (permalink / raw)
To: linux-arm-kernel
The MODULE_ALIAS statement refers to a macro that has never been defined
in this driver, causing a build error:
drivers/mtd/spi-nor/stm32-quadspi.c:694:150: error: expected ',' or ';' before 'DRIVER_NAME'
Unless there is a specific alias we need other then the driver name,
we don't need an alias here and can simply remove the line to fix the
build.
Fixes: dc5e400611c5 ("mtd: spi-nor: add driver for STM32 quad spi flash controller")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/mtd/spi-nor/stm32-quadspi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/stm32-quadspi.c b/drivers/mtd/spi-nor/stm32-quadspi.c
index 90d9152ddf98..1056e7408d2a 100644
--- a/drivers/mtd/spi-nor/stm32-quadspi.c
+++ b/drivers/mtd/spi-nor/stm32-quadspi.c
@@ -691,7 +691,6 @@ static struct platform_driver stm32_qspi_driver = {
};
module_platform_driver(stm32_qspi_driver);
-MODULE_ALIAS("platform:" DRIVER_NAME);
MODULE_AUTHOR("Ludovic Barre <ludovic.barre@st.com>");
MODULE_DESCRIPTION("STMicroelectronics STM32 quad spi driver");
MODULE_LICENSE("GPL v2");
--
2.9.0
^ permalink raw reply related
* [PATCH v5 03/22] KVM: arm/arm64: vgic-its: rename itte into ite
From: Christoffer Dall @ 2017-04-27 11:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <82b7eb70-1705-8fc5-7ec1-12ba58418389@redhat.com>
On Thu, Apr 27, 2017 at 11:40:16AM +0200, Auger Eric wrote:
> Hi Christoffer, Andre,
>
> On 27/04/2017 11:20, Andre Przywara wrote:
> > Hi,
> >
> > On 27/04/17 10:05, Christoffer Dall wrote:
> >> On Fri, Apr 14, 2017 at 12:15:15PM +0200, Eric Auger wrote:
> >>> The actual abbreviation for the interrupt translation table entry
> >>> is ITE. Let's rename all itte instances by ite.
> >>
> >> Is there really any confusion or problems with using itte? This is a
> >> lot of churn...
> >
> > I tend to agree (just didn't dare to mention this before).
> > I see that the spec speaks of "ITE", but the spelled out term hints more
> > at ITTE (because it's a "translation table").
> > Besides three letters tend to be more ambiguous than a four letter
> > identifier.
> >
> > Would adding a comment to the structure definition help?
> >
> > But speaking of churn I am not sure how much more work dropping this
> > patch now creates on Eric's side ...
>
> I addressed one comment from Marc saying that ITTE was a wrong name and
> he had a patch ready to rename them.
>
> see https://patchwork.kernel.org/patch/9513491/
>
> Renaming back would largely impact the other patches I am afraid
>
Ok, it's fine, Marc likes the change, and Andre is happy that you are
now responsible for all ITS bugs, and I'm happy if you're all happy.
Let's keep this change :)
-Christoffer
^ permalink raw reply
* [PATCH] crypto: stm32 - fix building as a module
From: Arnd Bergmann @ 2017-04-27 11:07 UTC (permalink / raw)
To: linux-arm-kernel
The names in the MODULE_DEVICE_TABLE and the actual array don't match:
drivers/crypto/stm32/stm32_crc32.c:309:21: error: 'sti_dt_ids' undeclared here (not in a function); did you mean 'stm32_dt_ids'?
This changes the reference that was evidently copied incorrectly from
another driver.
Fixes: b51dbe90912a ("crypto: stm32 - Support for STM32 CRC32 crypto module")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/crypto/stm32/stm32_crc32.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/stm32/stm32_crc32.c b/drivers/crypto/stm32/stm32_crc32.c
index 765282262d44..ec83b1e6bfe8 100644
--- a/drivers/crypto/stm32/stm32_crc32.c
+++ b/drivers/crypto/stm32/stm32_crc32.c
@@ -306,7 +306,7 @@ static const struct of_device_id stm32_dt_ids[] = {
{ .compatible = "st,stm32f7-crc", },
{},
};
-MODULE_DEVICE_TABLE(of, sti_dt_ids);
+MODULE_DEVICE_TABLE(of, stm32_dt_ids);
static struct platform_driver stm32_crc_driver = {
.probe = stm32_crc_probe,
--
2.9.0
^ permalink raw reply related
* [PATCH v2] arm64: Add ASM modifier for xN register operands
From: Mark Rutland @ 2017-04-27 11:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170426214616.142580-1-mka@chromium.org>
Hi,
On Wed, Apr 26, 2017 at 02:46:16PM -0700, Matthias Kaehlcke wrote:
> Many inline assembly statements don't include the 'x' modifier when
> using xN registers as operands. This is perfectly valid, however it
> causes clang to raise warnings like this:
>
> warning: value size does not match register size specified by the
> constraint and modifier [-Wasm-operand-widths]
> ...
> arch/arm64/include/asm/barrier.h:62:23: note: expanded from macro
> '__smp_store_release'
> asm volatile ("stlr %1, %0"
>
> Add the modifiers to keep clang happy.
If we're going to make this consistent, it would make sense to similarly
annotate 'w' regs. That will make it easier going forward to enforce a
policy that registers are suitably annotated.
Also, there's a risk that we silently mask a bug here, for which clang's
warning is legitimate, so we need to review this very carefully...
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v2:
> - also add modifiers to multiline ASM statements in include/asm/
> {atomic_ll_sc.h,irqflags.h,pgtable.h,uaccess.h,word-at-a-time.h}
> that were missed on v1
>
> arch/arm64/include/asm/arch_gicv3.h | 2 +-
> arch/arm64/include/asm/atomic_ll_sc.h | 36 ++++++++++++++++-----------------
> arch/arm64/include/asm/barrier.h | 4 ++--
> arch/arm64/include/asm/io.h | 24 +++++++++++-----------
> arch/arm64/include/asm/irqflags.h | 10 ++++-----
> arch/arm64/include/asm/kvm_hyp.h | 10 ++++-----
> arch/arm64/include/asm/kvm_mmu.h | 12 +++++------
> arch/arm64/include/asm/percpu.h | 4 ++--
> arch/arm64/include/asm/pgtable.h | 20 +++++++++---------
> arch/arm64/include/asm/sysreg.h | 4 ++--
> arch/arm64/include/asm/uaccess.h | 14 ++++++-------
> arch/arm64/include/asm/word-at-a-time.h | 14 ++++++-------
> arch/arm64/kernel/armv8_deprecated.c | 4 ++--
> arch/arm64/kernel/probes/kprobes.c | 2 +-
> arch/arm64/kvm/hyp/switch.c | 4 ++--
> 15 files changed, 82 insertions(+), 82 deletions(-)
... to that end, could you split these into a few patches?
That way, knowledgeable people can focus their review on the code they
understand.
That doesn't need to be a patch per file; all the KVM bits can be
collated in one patch, for example. However, the atomics, kvm, and
uaccess+word-at-a-time bits should certainly be separate patches given
their (existing) complexity.
Otherwise, I have a couple of comments below.
> diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
> index f37e3a21f6e7..ba54e5bee885 100644
> --- a/arch/arm64/include/asm/arch_gicv3.h
> +++ b/arch/arm64/include/asm/arch_gicv3.h
> @@ -166,7 +166,7 @@ static inline void gic_write_sre(u32 val)
>
> static inline void gic_write_bpr1(u32 val)
> {
> - asm volatile("msr_s " __stringify(ICC_BPR1_EL1) ", %0" : : "r" (val));
> + asm volatile("msr_s " __stringify(ICC_BPR1_EL1) ", %x0" : : "r" (val));
> }
Please make this use write_sysreg_s() instead, i.e.
static inline void gic_write_bpr1(u32 val)
{
write_sysreg_s(var, ICC_BPR1_EL1);
}
... that uses the 'x' modifier internally, and it's what we do for the
other GIC sysreg accesors.
This accessor was missed by commit:
d44ffa5ae70a15a1 ("irqchip/gic-v3: Convert arm64 GIC accessors to {read,write}_sysreg_s")
... because it was added concurrently by commitL
91ef84428a86b75a ("irqchip/gic-v3: Reset BPR during initialization")
... i.e. it was not deliberately omitted.
[...]
> - asm volatile("strb %w0, [%1]" : : "rZ" (val), "r" (addr));
> + asm volatile("strb %w0, [%x1]" : : "rZ" (val), "r" (addr));
In general, the '[%xN]' pattern looks *very* suspicious to me. Any
address must be 64-bit, so this would mask a legitimate warning.
Given the prototype of this function the code if fine either way, but
were we to refactor things (e.g. making this a macro), that might not be
true.
... so I'm not sure it make sense to alter instances used for addresses.
Thanks,
Mark.
^ permalink raw reply
* [PATCH] clk: sunxi-ng: always select CCU_GATE
From: Arnd Bergmann @ 2017-04-27 11:02 UTC (permalink / raw)
To: linux-arm-kernel
When the base driver is enabled but all SoC specific drivers are turned
off, we now get a build error after code was added to always refer to the
clk gates:
drivers/clk/built-in.o: In function `ccu_pll_notifier_cb':
:(.text+0x154f8): undefined reference to `ccu_gate_helper_disable'
:(.text+0x15504): undefined reference to `ccu_gate_helper_enable'
This changes the Kconfig to always require the gate code to be built-in
when CONFIG_SUNXI_CCU is set.
Fixes: 02ae2bc6febd ("clk: sunxi-ng: Add clk notifier to gate then ungate PLL clocks")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/clk/sunxi-ng/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/sunxi-ng/Kconfig b/drivers/clk/sunxi-ng/Kconfig
index 64088e599404..b0d551a8efe4 100644
--- a/drivers/clk/sunxi-ng/Kconfig
+++ b/drivers/clk/sunxi-ng/Kconfig
@@ -16,7 +16,7 @@ config SUNXI_CCU_FRAC
bool
config SUNXI_CCU_GATE
- bool
+ def_bool y
config SUNXI_CCU_MUX
bool
--
2.9.0
^ permalink raw reply related
* [PATCH v5 01/22] KVM: arm/arm64: Add ITS save/restore API documentation
From: Christoffer Dall @ 2017-04-27 11:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <db5d0eb9-67eb-8b9a-50ca-816d217b3f53@redhat.com>
On Thu, Apr 27, 2017 at 11:33:39AM +0200, Auger Eric wrote:
> Hi Christoffer,
>
> On 27/04/2017 10:57, Christoffer Dall wrote:
> > On Wed, Apr 26, 2017 at 05:48:32PM +0200, Auger Eric wrote:
> >> Hi Christoffer,
> >>
> >> On 26/04/2017 14:31, Christoffer Dall wrote:
> >>> On Fri, Apr 14, 2017 at 12:15:13PM +0200, Eric Auger wrote:
> >>>> Add description for how to access ITS registers and how to save/restore
> >>>> ITS tables into/from memory.
> >>>>
> >>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >>>>
> >>>> ---
> >>>> v4 -> v5:
> >>>> - take into account Christoffer's comments
> >>>> - pending table save on GICV3 side now
> >>>>
> >>>> v3 -> v4:
> >>>> - take into account Peter's comments:
> >>>> - typos
> >>>> - KVM_DEV_ARM_VGIC_GRP_ITS_TABLES kvm_device_attr = 0
> >>>> - add a validity bit in DTE
> >>>> - document all fields in CTE and ITE
> >>>> - document ABI revision
> >>>> - take into account Andre's comments:
> >>>> - document restrictions about GITS_CREADR writing and GITS_IIDR
> >>>> - document -EBUSY error if one or more VCPUS are runnning
> >>>> - document 64b registers only can be accessed with 64b access
> >>>> - itt_addr field matches bits [51:8] of the itt_addr
> >>>>
> >>>> v1 -> v2:
> >>>> - DTE and ITE now are 8 bytes
> >>>> - DTE and ITE now indexed by deviceid/eventid
> >>>> - use ITE name instead of ITTE
> >>>> - mentions ITT_addr matches bits [51:8] of the actual address
> >>>> - mentions LE layout
> >>>> ---
> >>>> Documentation/virtual/kvm/devices/arm-vgic-its.txt | 99 ++++++++++++++++++++++
> >>>> 1 file changed, 99 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-its.txt b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> >>>> index 6081a5b..b5f010d 100644
> >>>> --- a/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> >>>> +++ b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> >>>> @@ -32,7 +32,106 @@ Groups:
> >>>> KVM_DEV_ARM_VGIC_CTRL_INIT
> >>>> request the initialization of the ITS, no additional parameter in
> >>>> kvm_device_attr.addr.
> >>>> +
> >>>> + KVM_DEV_ARM_ITS_SAVE_TABLES
> >>>> + save the ITS table data into guest RAM, at the location provisioned
> >>>> + by the guest in corresponding registers/table entries.
> >>>> +
> >>>> + The layout of the tables in guest memory defines an ABI. The entries
> >>>> + are laid out in little endian format as described in the last paragraph.
> >>>> +
> >>>> + KVM_DEV_ARM_ITS_RESTORE_TABLES
> >>>> + restore the ITS tables from guest RAM to ITS internal structures.
> >>>> +
> >>>> + The GICV3 must be restored before the ITS and all ITS registers but
> >>>> + the GITS_CTLR must be restored before restoring the ITS tables.
> >>>> +
> >>>> + The GITS_IIDR read-only register must also be restored before
> >>>> + the table restore as the IIDR revision field encodes the ABI revision.
> >>>> +
> >>>
> >>> what is the expected sequence of operations. For example, to restore
> >>> the ITS, do I call KVM_DEV_ARM_VGIC_CTRL_INIT first, then restore all
> >>> the memory and registers, and finally call KVM_DEV_ARM_ITS_RESTORE_TABLES?
> >> Yes KVM_DEV_ARM_VGIC_CTRL_INIT comes first, then restore all registers
> >> except GITS_CTLR, then table restore, then GITS_CTLR
> >>>
> >>> Is there any interaction between when you call KVM_DEV_ARM_ITS_RESTORE_TABLES
> >>> and restore GITS_CTLR (which enables the ITS)?
> >>
> >> Yep, when GITS_CTLR is set, LPIs may be enabled and this on that event
> >> that the pending table is read. But the whole pending table is not read
> >> as we only iterate on registered LPIs. So the ITT must have been
> >> restored previously.
> >>
> >> I became aware that the pending table sync is done twice, once in the
> >> pending table restore, and once in the GITS_CTLR restore. So if we
> >> leave this order specification, I should be able to remove the sync on
> >> table restore. This was the original reason why GITS_CTLR restore has
> >> been done at the very end.
> >
> > I'm sorry, I'm a bit confused. Do we not need
> > KVM_DEV_ARM_ITS_RESTORE_TABLES at all then?
>
> Yes you do. I was talking about the RDIST pending table sync. The save
> is explicit using GICV3 device KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES.
> However the sync is implicit on GITS_CTLR restore if LPIs are enabled.
> and today I do it also on ITS device KVM_DEV_ARM_ITS_RESTORE_TABLES
> which is not requested I think since GITS_CTLR restore does it already.
Shouldn't restoring the pending tables happen when restoring some
redeistributor state and not anything related to the ITS?
>
> KVM_DEV_ARM_ITS_RESTORE_TABLES restores all the ITS tables (device,
> collection, ITT)
Why do you need this if you anyway need to restore the CTLR as the last
thing? Just to make it absolutely clear when it happens, or is there
something which has to happen between the CTLR and the RESTORE?
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH v5 07/22] KVM: arm64: vgic-its: Implement vgic_its_has_attr_regs and attr_regs_access
From: Christoffer Dall @ 2017-04-27 11:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1492164934-988-8-git-send-email-eric.auger@redhat.com>
On Fri, Apr 14, 2017 at 12:15:19PM +0200, Eric Auger wrote:
> This patch implements vgic_its_has_attr_regs and vgic_its_attr_regs_access
> upon the MMIO framework. VGIC ITS KVM device KVM_DEV_ARM_VGIC_GRP_ITS_REGS
> group becomes functional.
>
> At least GITS_CREADR and GITS_IIDR require to differentiate a guest write
> action from a user access. As such let's introduce a new uaccess_its_write
> vgic_register_region callback.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
> v4 -> v5:
> - use GITS_TYPER instead of offset 0x8
> - uaccess_its_write now can return an error
>
> v3 -> v4:
> - remove changes to the REGISTER_ITS_DESC macro. This will be handled in
> subsequent patch with the introduction of a new REGISTER_ITS_DESC_UACCESS
> macro
> - fix IIDR access and add a comment wrt full length access
> - handle endianness
> - add kvm lock and vcpus lock
> ---
> virt/kvm/arm/vgic/vgic-its.c | 79 +++++++++++++++++++++++++++++++++++++++++--
> virt/kvm/arm/vgic/vgic-mmio.h | 9 +++--
> 2 files changed, 84 insertions(+), 4 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index f687e91..a9a2c12 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -1469,14 +1469,89 @@ static void vgic_its_destroy(struct kvm_device *kvm_dev)
> int vgic_its_has_attr_regs(struct kvm_device *dev,
> struct kvm_device_attr *attr)
> {
> - return -ENXIO;
> + const struct vgic_register_region *region;
> + struct vgic_io_device iodev = {
> + .regions = its_registers,
> + .nr_regions = ARRAY_SIZE(its_registers),
> + };
> + gpa_t offset = attr->attr;
> +
> + region = vgic_find_mmio_region(iodev.regions,
> + iodev.nr_regions,
> + offset);
why do you need to define the iodev here? Can't you just pass
its_registers and ARRAY_SIZE(its_registers) directly?
> + if (!region)
> + return -ENXIO;
> +
> + return 0;
> }
>
> int vgic_its_attr_regs_access(struct kvm_device *dev,
> struct kvm_device_attr *attr,
> u64 *reg, bool is_write)
> {
> - return -ENXIO;
> + const struct vgic_register_region *region;
> + struct vgic_io_device iodev = {
> + .regions = its_registers,
> + .nr_regions = ARRAY_SIZE(its_registers),
> + };
> + struct vgic_its *its = dev->private;
> + gpa_t addr, offset = attr->attr;
> + unsigned int len;
> + unsigned long data = 0;
> + int ret = 0;
nit: you could structure this whole declaration block more nicely by
having a separate line for the declaration and initialization of offset,
and by moving the iodev declaration/initialization to the end.
It might also be cleaner to do any non-zero initialization in a separate
block following the declarations.
> +
> + /*
> + * Among supported registers, only GITS_CTLR (0x0) and GITS_IIDR (0x4)
> + * are 32 bits. Others are 64 bits.
> + */
> + if ((offset < GITS_TYPER && offset & 0x3) ||
> + (offset >= GITS_TYPER && offset & 0x7))
> + return -EINVAL;
> +
> + mutex_lock(&dev->kvm->lock);
> +
> + if (IS_VGIC_ADDR_UNDEF(its->vgic_its_base)) {
> + ret = -ENXIO;
> + goto out;
> + }
> +
> + region = vgic_find_mmio_region(iodev.regions,
> + iodev.nr_regions,
> + offset);
> + if (!region) {
> + ret = -ENXIO;
> + goto out;
> + }
> +
> + if (!lock_all_vcpus(dev->kvm)) {
> + ret = -EBUSY;
> + goto out;
> + }
> +
> + addr = its->vgic_its_base + offset;
> +
> + /*
> + * Only full length register accesses are supported although
> + * the architecture spec theoretically allows upper/lower 32
does the spec allow 32-bit accesses, or only theoretically ? :)
> + * bits to be accessed independently
> + */
In any case, the comment is a bit confusing, because it seems to imply
that we only support 64-bit accesses, but we do set the length below to
4 or 8.
Did you mean:
/*
* Althought the spec supports upper/lower 32-bit accesses to
* 64-bit ITS registers, the userspace ABI requires 64-bit
* accesses to all 64-bit wide registers. We therefore only 32-bit
* accesses to the GITS_CTLR, GITS_IIDR registers.
*/
Also, I don't understand how this works with the ID registers? For
example, if userspace wants to read GITS_PIDR1 does it have to read
GITS_PIDR0 as a 64-bit register and split it afterwards? (that doesn't
work with this implementation)
> + len = region->access_flags & VGIC_ACCESS_64bit ? 8 : 4;
> +
> + if (is_write) {
> + data = vgic_data_mmio_bus_to_host(reg, len);
I don't think we need this anymore; we no longer share the guest
trapping MMIO path with uaccesses.
> + if (region->uaccess_its_write)
> + ret = region->uaccess_its_write(dev->kvm, its, addr,
> + len, data);
> + else
> + region->its_write(dev->kvm, its, addr, len, data);
> + } else {
> + data = region->its_read(dev->kvm, its, addr, len);
> + vgic_data_host_to_mmio_bus(reg, len, data);
same here.
> + }
> + unlock_all_vcpus(dev->kvm);
> +out:
> + mutex_unlock(&dev->kvm->lock);
> + return ret;
> }
>
> static int vgic_its_has_attr(struct kvm_device *dev,
> diff --git a/virt/kvm/arm/vgic/vgic-mmio.h b/virt/kvm/arm/vgic/vgic-mmio.h
> index 6eec91b..ea4171a 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio.h
> +++ b/virt/kvm/arm/vgic/vgic-mmio.h
> @@ -36,8 +36,13 @@ struct vgic_register_region {
> };
> unsigned long (*uaccess_read)(struct kvm_vcpu *vcpu, gpa_t addr,
> unsigned int len);
> - void (*uaccess_write)(struct kvm_vcpu *vcpu, gpa_t addr,
> - unsigned int len, unsigned long val);
> + union {
> + void (*uaccess_write)(struct kvm_vcpu *vcpu, gpa_t addr,
> + unsigned int len, unsigned long val);
> + int (*uaccess_its_write)(struct kvm *kvm, struct vgic_its *its,
> + gpa_t addr, unsigned int len,
> + unsigned long val);
> + };
> };
>
> extern struct kvm_io_device_ops kvm_io_gic_ops;
> --
> 2.5.5
>
Thanks,
-Christoffer
^ permalink raw reply
* [RFC/RFT PATCH 07/18] ARM: PCI: orion5x: Convert PCI scan API to pci_scan_root_bus_bridge()
From: Lorenzo Pieralisi @ 2017-04-27 10:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170426185348.GB14074@lunn.ch>
On Wed, Apr 26, 2017 at 08:53:48PM +0200, Andrew Lunn wrote:
> > > Note that IRQ 12 has disappeared from Flags: on 0000:00:00.0.
> >
> > Thank you for testing it, that's exactly what we need.
> >
> > To check if the plumbing is working (ie to check I have not messed
> > up the bus scan API rework), mind testing the patch below please
> > and report back (it applies on top of this series) ?
>
> 0000:00:00.0 Memory controller: Marvell Technology Group Ltd. 88f5182 [Orion-NAS] ARM SoC (rev 02)
> Subsystem: Marvell Technology Group Ltd. Device 11ab
> Flags: bus master, fast devsel, latency 0, IRQ 12
>
> The interrupt is back with this patch.
Ok the IRQ allocation works then, the difference is when the mapping
is called, original patch series moves the IRQ allocation to device
probe time (that's why IRQ does not show up, there is no matching
driver in the kernel for the PCI device above), with the additional
patch the IRQ allocation is done when the PCI device is added.
It is a user visible change, not sure though if that's a real issue,
that's exactly the reason why I posted this series, to find out
and converge.
Thanks !
Lorenzo
^ permalink raw reply
* stable/linux-3.18.y build: 204 builds: 5 failed, 199 passed, 35 errors, 212 warnings (v3.18.49)
From: gregkh @ 2017-04-27 10:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAK8P3a3QsSMtc7AWjVjtM+tW8ARt1Ygw53=CSjgbG6Pvpq0QvQ@mail.gmail.com>
On Fri, Apr 21, 2017 at 04:27:14PM +0200, Arnd Bergmann wrote:
> On Thu, Apr 20, 2017 at 7:04 PM, kernelci.org bot <bot@kernelci.org> wrote:
> > stable/linux-3.18.y build: 204 builds: 5 failed, 199 passed, 35 errors, 212 warnings (v3.18.49)
>
> I've gone through all these now and found a fix. In three cases, there is no
> fix yet since the respective drivers got removed before the warning was
> noticed. Do we have a policy for how to deal with those? Should I just
> send patches to address the warnings for 3.18?
I've wondered about this, and yeah, I would like to see the number drop
to 0 if at all possible (the scsi driver will not change), so i'll be
glad to take patches for the code that is no longer in upstream.
> > Errors summary:
> > 7 arch/mips/jz4740/irq.h:21:38: error: 'struct irq_data' declared inside
> > parameter list will not be visible outside of this definition or declaration
> > [-Werror]
> > 7 arch/mips/jz4740/irq.h:20:39: error: 'struct irq_data' declared inside
> > parameter list will not be visible outside of this definition or declaration
> > [-Werror]
>
>
> 5b235dc2647e4 MIPS: Fix the build on jz4740 after removing the custom gpio.h
now applied.
> > 2 arch/mips/mm/fault.c:321:1: error: the frame size of 1104 bytes is larger
> > than 1024 bytes [-Werror=frame-larger-than=]
>
> This is a result of a newer compiler version, combined with the -Werror
> flag that is applied to arch/mips/, and two of the mips defconfigs overriding
> CONFIG_FRAME_WARN to 1024 on a 64-bit architecture (probably by
> accident).
>
> I saw this also when I looked at the 3.16 warnings, but only now actually
> bisected it. The fix is
>
> 86038c5ea81b ("perf: Avoid horrible stack usage")
Now applied.
> > Detailed per-defconfig build reports:
> >
> > allmodconfig+CONFIG_OF=n (x86) ? PASS, 0 errors, 8 warnings, 0 section
> > mismatches
> >
> > Warnings:
> > fs/nfs/nfs4proc.c:3062:10: warning: switch condition has boolean value
> > [-Wswitch-bool]
>
> c7757074839f ("fs/nfs: fix new compiler warning about boolean in switch")
applied.
> > drivers/iommu/intel-iommu.c:1762:25: warning: unused variable 'drhd'
> > [-Wunused-variable]
>
> 509fca899d56 ("iommu/vt-d: Remove unused variable")
Ugh, I thought I had found this one in the past, sorry about that, now
applied.
> > drivers/message/i2o/i2o_config.c:893:19: warning: cast to pointer from
> > integer of different size [-Wint-to-pointer-cast]
> > drivers/message/i2o/i2o_config.c:953:10: warning: cast to pointer from
> > integer of different size [-Wint-to-pointer-cast]
>
> The driver is obsolete and was removed in v4.2. It looks like the warning
> should still be there in v4.1.y, but I don't remember having seen it. It
> would be trivial to fix this with an obvious patch adding a cast.
I'll take a fix :)
> > drivers/staging/bcm/CmHost.c:1503:3: warning: cast to pointer from integer
> > of different size [-Wint-to-pointer-cast]
> > drivers/staging/bcm/CmHost.c:1546:3: warning: cast to pointer from integer
> > of different size [-Wint-to-pointer-cast]
> > drivers/staging/bcm/CmHost.c:1564:3: warning: cast to pointer from integer
> > of different size [-Wint-to-pointer-cast]
>
> Similarly, the driver was removed in v3.19, but I could create a patch for
> the warning.
I'll take it! :)
> > drivers/scsi/advansys.c:71:2: warning: #warning this driver is still not
> > properly converted to the DMA API [-Wcpp]
>
> The driver was properly converted in v4.2 and the warning removed, but the
> conversion would be outside of stable-kernel-rules.
Yeah, this one is going to have to stay as-is :(
> > Section mismatches:
> > WARNING: arch/x86/kernel/built-in.o(.text.unlikely+0x157f): Section mismatch
> > in reference from the function cpumask_empty.constprop.3() to the variable
> > .init.data:nmi_ipi_mask
> > WARNING: arch/x86/built-in.o(.text.unlikely+0x189b): Section mismatch in
> > reference from the function cpumask_empty.constprop.3() to the variable
> > .init.data:nmi_ipi_mask
> > WARNING: vmlinux.o(.text.unlikely+0x1962): Section mismatch in reference
> > from the function cpumask_empty.constprop.3() to the variable
> > .init.data:nmi_ipi_mask
>
> f0ba662a6e06f2 x86: Properly _init-annotate NMI selftest code
That commit is from 3.4, so how can I add it to 3.18? :)
> > allnoconfig (mips) ? PASS, 0 errors, 1 warning, 0 section mismatches
> >
> > Warnings:
> > mm/page_alloc.c:5346:34: warning: array subscript is below array bounds
> > [-Warray-bounds]
>
> Also bisected this one now, this is also missing on 3.16:
>
> 90cae1fe1c35 ("mm/init: fix zone boundary creation")
Now applied to 3.1_8_ :)
> > ar7_defconfig (mips) ? PASS, 0 errors, 2 warnings, 0 section mismatches
> >
> > Warnings:
> > include/linux/kernel.h:713:17: warning: comparison of distinct pointer types
> > lacks a cast
>
> 2f5281ba2a8f ("net: ti: cpmac: Fix compiler warning due to type confusion")
Ugh, I don't know why this one wasn't in 3.18, that makes me wonder what
I missed that went into 4.4 that didn't make it to 3.18...
> > at91_dt_defconfig (arm) ? PASS, 0 errors, 2 warnings, 0 section mismatches
> >
> > Warnings:
> > drivers/clk/at91/clk-usb.c:155:20: warning: initialization from incompatible
> > pointer type [-Wincompatible-pointer-types]
> > drivers/clk/at91/clk-usb.c:193:20: warning: initialization from incompatible
> > pointer type [-Wincompatible-pointer-types]
>
> I've send this fix:
>
> 8<------
> clk: at91: usb: fix determine_rate prototype again
>
> We had an incorrect backport of
> 4591243102fa ("clk: at91: usb: propagate rate modification to the parent clk")
> that was fixed incorrectly in linux-3.18.y by
> 76723e7ed589 ("clk: at91: usb: fix determine_rate prototype")
>
> This should fix it properly.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/clk/at91/clk-usb.c b/drivers/clk/at91/clk-usb.c
> index 0283a5713d6c..930a424cc4a0 100644
> --- a/drivers/clk/at91/clk-usb.c
> +++ b/drivers/clk/at91/clk-usb.c
> @@ -59,7 +59,7 @@ static unsigned long
> at91sam9x5_clk_usb_recalc_rate(struct clk_hw *hw,
> static long at91sam9x5_clk_usb_determine_rate(struct clk_hw *hw,
> unsigned long rate,
> unsigned long *best_parent_rate,
> - struct clk_hw **best_parent_hw)
> + struct clk **best_parent_hw)
> {
> struct clk *parent = NULL;
> long best_rate = -EINVAL;
> @@ -91,7 +91,7 @@ static long at91sam9x5_clk_usb_determine_rate(struct
> clk_hw *hw,
> best_rate = tmp_rate;
> best_diff = tmp_diff;
> *best_parent_rate = tmp_parent_rate;
> - *best_parent_hw = __clk_get_hw(parent);
> + *best_parent_hw = parent;
> }
>
> if (!best_diff || tmp_rate < rate)
> ---------->8
Applied.
> > ath79_defconfig (mips) ? PASS, 0 errors, 2 warnings, 0 section mismatches
> >
> > Warnings:
> > arch/mips/kernel/entry.S:170: Warning: tried to set unrecognized symbol:
> > MIPS_ISA_LEVEL_RAW
>
> aebac99384f7 ("MIPS: kernel: entry.S: Set correct ISA level for mips_ihb")
That was in 3.18.14, what kernel are you looking at here???
> > cerfcube_defconfig (arm) ? PASS, 0 errors, 2 warnings, 0 section mismatches
> >
> > Warnings:
> > fs/nfsd/nfs4state.c:3781:3: warning: 'old_deny_bmap' may be used
> > uninitialized in this function [-Wmaybe-uninitialized]
>
> 5368e1a6 ("nfsd: work around a gcc-5.1 warning")
That commit id isn't in Linus's tree, where did you get it from?
> > corgi_defconfig (arm) ? PASS, 0 errors, 2 warnings, 0 section mismatches
> >
> > Warnings:
> > drivers/usb/gadget/legacy/inode.c:648:10: warning: 'value' may be used
> > uninitialized in this function [-Wmaybe-uninitialized]
>
> This is caused by the backport of f01d35a15fa0416 from 4.0 to 3.18:
> c81fc59be42c6e0 gadgetfs: use-after-free in ->aio_read()
>
> The backported patch was buggy, but the mainline code was
> rewritten in a larger patch directly following this one in a way that
> fixed the bug.
>
> For stable, we should need only a one-line change, which I sent now:
>
> diff --git a/drivers/usb/gadget/legacy/inode.c
> b/drivers/usb/gadget/legacy/inode.c
> index 54f964bbc79a..fe45311f243e 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c
> @@ -654,6 +654,7 @@ fail:
> GFP_KERNEL);
> if (!priv->iv) {
> kfree(priv);
> + value = -ENOMEM;
> goto fail;
> }
> }
Now applied.
> > defconfig+CONFIG_LKDTM=y (mips) ? PASS, 0 errors, 3 warnings, 0 section
> > mismatches
> >
> > Warnings:
> > include/asm-generic/div64.h:43:28: warning: comparison of distinct pointer
> > types lacks a cast
> > include/asm-generic/div64.h:43:28: warning: comparison of distinct pointer
> > types lacks a cast
>
> 2ae83bf93882 ("[CIFS] Fix setting time before epoch (negative time values)")
That was in 3.17, are you sure you are looking at 3.18 like the subject
says???
I'll do the rest after lunch...
thanks,
greg k-h
^ permalink raw reply
* [patch V2 00/24] cpu/hotplug: Convert get_online_cpus() to a percpu_rwsem
From: Thomas Gleixner @ 2017-04-27 10:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170427095744.GB31337@leverpostej>
On Thu, 27 Apr 2017, Mark Rutland wrote:
> On Thu, Apr 27, 2017 at 10:27:20AM +0200, Sebastian Siewior wrote:
> > On 2017-04-26 11:32:36 [+0100], Mark Rutland wrote:
> > > > So we could end up calling static_branch_enable_cpuslocked()
> > > > without actually holding the lock. Should we do a cpu_hotplug_begin/done in
> > > > setup_cpu_feature_capabilities ? I agree it doesn't look that nice. Thoughts ?
> > >
> > > I agree that's hideous, but it looks like the only choice given the
> > > hotplug rwsem cahnges. :/
> >
> > would work for you to provide a locked and unlocked version?
>
> Maybe. Today we have:
>
> // rwsem unlocked
> start_kernel()
> ->smp_prepare_boot_cpu()
> -->update_cpu_errata_workarounds()
> --->update_cpu_capabilities()
>
> // rwsem locked (by other CPU)
> secondary_start_kernel()
> ->check_local_cpu_capabilities()
> -->update_cpu_errata_workarounds()
> --->update_cpu_capabilities()
>
> With the common chain:
>
> update_cpu_capabilities()
> ->cpus_set_cap()
> -->static_branch_enable()
>
> ... so we could add a update_cpu_capabilities{,_cpuslocked}(), and say
> that cpus_set_cap() expects the hotplug rswem to be locked, as per the
> below diff.
You just can take the rwsen in smp_prepare_boot_cpu(), so you don't need
that conditional thingy at all. Hmm?
Thanks,
tglx
^ permalink raw reply
* [PATCH, 3.18-stable regression] ARM: psci: fix header file
From: Greg KH @ 2017-04-27 10:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170421135923.2735181-1-arnd@arndb.de>
On Fri, Apr 21, 2017 at 03:59:17PM +0200, Arnd Bergmann wrote:
> Commit be95485a0b828 was incorrectly backported to 3.18 and
> now causes a different build error when CONFIG_SMP is disabled:
>
> arch/arm/kernel/psci.c:287:12: error: redefinition of 'psci_init'
>
> This version is how it should be on 3.18, with the main psci
> code built for both SMP and UP, but the psci_smp_available()
> function only being available for SMP builds.
>
> Fixes: dbcfee724255 ("ARM: 8457/1: psci-smp is built only for SMP")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/arm/include/asm/psci.h | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
This should have already been fixed in 3.18.50 as dbcfee724255 was
reverted there. Because of that, this patch does not apply to the
3.18-stable tree.
thanks,
greg k-h
^ permalink raw reply
* [patch V2 00/24] cpu/hotplug: Convert get_online_cpus() to a percpu_rwsem
From: Mark Rutland @ 2017-04-27 9:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170427082719.3wyru4bk67kdmflb@linutronix.de>
On Thu, Apr 27, 2017 at 10:27:20AM +0200, Sebastian Siewior wrote:
> On 2017-04-26 11:32:36 [+0100], Mark Rutland wrote:
> > > So we could end up calling static_branch_enable_cpuslocked()
> > > without actually holding the lock. Should we do a cpu_hotplug_begin/done in
> > > setup_cpu_feature_capabilities ? I agree it doesn't look that nice. Thoughts ?
> >
> > I agree that's hideous, but it looks like the only choice given the
> > hotplug rwsem cahnges. :/
>
> would work for you to provide a locked and unlocked version?
Maybe. Today we have:
// rwsem unlocked
start_kernel()
->smp_prepare_boot_cpu()
-->update_cpu_errata_workarounds()
--->update_cpu_capabilities()
// rwsem locked (by other CPU)
secondary_start_kernel()
->check_local_cpu_capabilities()
-->update_cpu_errata_workarounds()
--->update_cpu_capabilities()
With the common chain:
update_cpu_capabilities()
->cpus_set_cap()
-->static_branch_enable()
... so we could add a update_cpu_capabilities{,_cpuslocked}(), and say
that cpus_set_cap() expects the hotplug rswem to be locked, as per the
below diff.
Thoughts?
Mark.
---->8----
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index f31c48d..7341579 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -145,7 +145,7 @@ static inline void cpus_set_cap(unsigned int num)
num, ARM64_NCAPS);
} else {
__set_bit(num, cpu_hwcaps);
- static_branch_enable(&cpu_hwcap_keys[num]);
+ static_branch_enable_cpuslocked(&cpu_hwcap_keys[num]);
}
}
@@ -217,8 +217,22 @@ static inline bool id_aa64pfr0_32bit_el0(u64 pfr0)
void __init setup_cpu_features(void);
-void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
- const char *info);
+void __update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
+ const char *info, bool cpuslocked);
+static inline void
+update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
+ const char *info)
+{
+ __update_cpu_capabilities(caps, info, false);
+}
+
+static inline void
+update_cpu_capabilities_cpuslocked(const struct arm64_cpu_capabilities *caps,
+ const char *info)
+{
+ __update_cpu_capabilities(caps, info, true);
+}
+
void enable_cpu_capabilities(const struct arm64_cpu_capabilities *caps);
void check_local_cpu_capabilities(void);
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index abda8e8..ae8ddc1 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -956,8 +956,8 @@ static void __init setup_elf_hwcaps(const struct arm64_cpu_capabilities *hwcaps)
cap_set_elf_hwcap(hwcaps);
}
-void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
- const char *info)
+void __update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
+ const char *info, bool cpuslocked)
{
for (; caps->matches; caps++) {
if (!caps->matches(caps, caps->def_scope))
@@ -965,7 +965,14 @@ void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
if (!cpus_have_cap(caps->capability) && caps->desc)
pr_info("%s %s\n", info, caps->desc);
- cpus_set_cap(caps->capability);
+
+ if (cpuslocked) {
+ cpus_set_cap(caps->capability);
+ } else {
+ get_online_cpus();
+ cpus_set_cap(caps->capability);
+ put_online_cpus();
+ }
}
}
^ permalink raw reply related
* stmmac still supporting spear600 ?
From: Thomas Petazzoni @ 2017-04-27 9:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0468e2fb-5a7d-97ec-c51c-2436a13dda69@st.com>
Hello Giuseppe,
On Mon, 3 Apr 2017 08:16:50 +0200, Giuseppe CAVALLARO wrote:
> I tested the SMSC on other platform (+ stmmac), not on SPEAr.
>
> ok for reset, keep the radar on clock. Hmm, can you attach a piece of
> log file to see the failure?
We finally identified the issue: in a MII configuration, the PS bit
need to be set for the DMA reset procedure to work, but setting the DMA
reset bit clears the PS bit. So you have to set the PS bit after
asserting the DMA reset, and before polling for the DMA reset bit to
clear.
I have sent a fix that works for us (tested GMII and MII platforms),
but not sure if the implementation is the most appropriate. Let me know
if you have better suggestions.
See: http://marc.info/?l=linux-netdev&m=149328635210461&w=2
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v5 03/22] KVM: arm/arm64: vgic-its: rename itte into ite
From: Auger Eric @ 2017-04-27 9:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c32b8369-711c-1a1a-b3c1-bba2408ff003@arm.com>
Hi Christoffer, Andre,
On 27/04/2017 11:20, Andre Przywara wrote:
> Hi,
>
> On 27/04/17 10:05, Christoffer Dall wrote:
>> On Fri, Apr 14, 2017 at 12:15:15PM +0200, Eric Auger wrote:
>>> The actual abbreviation for the interrupt translation table entry
>>> is ITE. Let's rename all itte instances by ite.
>>
>> Is there really any confusion or problems with using itte? This is a
>> lot of churn...
>
> I tend to agree (just didn't dare to mention this before).
> I see that the spec speaks of "ITE", but the spelled out term hints more
> at ITTE (because it's a "translation table").
> Besides three letters tend to be more ambiguous than a four letter
> identifier.
>
> Would adding a comment to the structure definition help?
>
> But speaking of churn I am not sure how much more work dropping this
> patch now creates on Eric's side ...
I addressed one comment from Marc saying that ITTE was a wrong name and
he had a patch ready to rename them.
see https://patchwork.kernel.org/patch/9513491/
Renaming back would largely impact the other patches I am afraid
Thanks
Eric
>
> Cheers,
> Andre.
>
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>>>
>>> ---
>>>
>>> v5: Add Marc's A-b
>>> ---
>>> virt/kvm/arm/vgic/vgic-its.c | 148 +++++++++++++++++++++----------------------
>>> 1 file changed, 74 insertions(+), 74 deletions(-)
>>>
>>> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
>>> index 8d1da1a..3ffcbbe 100644
>>> --- a/virt/kvm/arm/vgic/vgic-its.c
>>> +++ b/virt/kvm/arm/vgic/vgic-its.c
>>> @@ -114,8 +114,8 @@ struct its_collection {
>>> #define its_is_collection_mapped(coll) ((coll) && \
>>> ((coll)->target_addr != COLLECTION_NOT_MAPPED))
>>>
>>> -struct its_itte {
>>> - struct list_head itte_list;
>>> +struct its_ite {
>>> + struct list_head ite_list;
>>>
>>> struct vgic_irq *irq;
>>> struct its_collection *collection;
>>> @@ -143,27 +143,27 @@ static struct its_device *find_its_device(struct vgic_its *its, u32 device_id)
>>> * Device ID/Event ID pair on an ITS.
>>> * Must be called with the its_lock mutex held.
>>> */
>>> -static struct its_itte *find_itte(struct vgic_its *its, u32 device_id,
>>> +static struct its_ite *find_ite(struct vgic_its *its, u32 device_id,
>>> u32 event_id)
>>> {
>>> struct its_device *device;
>>> - struct its_itte *itte;
>>> + struct its_ite *ite;
>>>
>>> device = find_its_device(its, device_id);
>>> if (device == NULL)
>>> return NULL;
>>>
>>> - list_for_each_entry(itte, &device->itt_head, itte_list)
>>> - if (itte->event_id == event_id)
>>> - return itte;
>>> + list_for_each_entry(ite, &device->itt_head, ite_list)
>>> + if (ite->event_id == event_id)
>>> + return ite;
>>>
>>> return NULL;
>>> }
>>>
>>> /* To be used as an iterator this macro misses the enclosing parentheses */
>>> -#define for_each_lpi_its(dev, itte, its) \
>>> +#define for_each_lpi_its(dev, ite, its) \
>>> list_for_each_entry(dev, &(its)->device_list, dev_list) \
>>> - list_for_each_entry(itte, &(dev)->itt_head, itte_list)
>>> + list_for_each_entry(ite, &(dev)->itt_head, ite_list)
>>>
>>> /*
>>> * We only implement 48 bits of PA at the moment, although the ITS
>>> @@ -270,18 +270,18 @@ static int vgic_copy_lpi_list(struct kvm *kvm, u32 **intid_ptr)
>>> * Needs to be called whenever either the collection for a LPIs has
>>> * changed or the collection itself got retargeted.
>>> */
>>> -static void update_affinity_itte(struct kvm *kvm, struct its_itte *itte)
>>> +static void update_affinity_ite(struct kvm *kvm, struct its_ite *ite)
>>> {
>>> struct kvm_vcpu *vcpu;
>>>
>>> - if (!its_is_collection_mapped(itte->collection))
>>> + if (!its_is_collection_mapped(ite->collection))
>>> return;
>>>
>>> - vcpu = kvm_get_vcpu(kvm, itte->collection->target_addr);
>>> + vcpu = kvm_get_vcpu(kvm, ite->collection->target_addr);
>>>
>>> - spin_lock(&itte->irq->irq_lock);
>>> - itte->irq->target_vcpu = vcpu;
>>> - spin_unlock(&itte->irq->irq_lock);
>>> + spin_lock(&ite->irq->irq_lock);
>>> + ite->irq->target_vcpu = vcpu;
>>> + spin_unlock(&ite->irq->irq_lock);
>>> }
>>>
>>> /*
>>> @@ -292,13 +292,13 @@ static void update_affinity_collection(struct kvm *kvm, struct vgic_its *its,
>>> struct its_collection *coll)
>>> {
>>> struct its_device *device;
>>> - struct its_itte *itte;
>>> + struct its_ite *ite;
>>>
>>> - for_each_lpi_its(device, itte, its) {
>>> - if (!itte->collection || coll != itte->collection)
>>> + for_each_lpi_its(device, ite, its) {
>>> + if (!ite->collection || coll != ite->collection)
>>> continue;
>>>
>>> - update_affinity_itte(kvm, itte);
>>> + update_affinity_ite(kvm, ite);
>>> }
>>> }
>>>
>>> @@ -425,25 +425,25 @@ static int vgic_its_trigger_msi(struct kvm *kvm, struct vgic_its *its,
>>> u32 devid, u32 eventid)
>>> {
>>> struct kvm_vcpu *vcpu;
>>> - struct its_itte *itte;
>>> + struct its_ite *ite;
>>>
>>> if (!its->enabled)
>>> return -EBUSY;
>>>
>>> - itte = find_itte(its, devid, eventid);
>>> - if (!itte || !its_is_collection_mapped(itte->collection))
>>> + ite = find_ite(its, devid, eventid);
>>> + if (!ite || !its_is_collection_mapped(ite->collection))
>>> return E_ITS_INT_UNMAPPED_INTERRUPT;
>>>
>>> - vcpu = kvm_get_vcpu(kvm, itte->collection->target_addr);
>>> + vcpu = kvm_get_vcpu(kvm, ite->collection->target_addr);
>>> if (!vcpu)
>>> return E_ITS_INT_UNMAPPED_INTERRUPT;
>>>
>>> if (!vcpu->arch.vgic_cpu.lpis_enabled)
>>> return -EBUSY;
>>>
>>> - spin_lock(&itte->irq->irq_lock);
>>> - itte->irq->pending_latch = true;
>>> - vgic_queue_irq_unlock(kvm, itte->irq);
>>> + spin_lock(&ite->irq->irq_lock);
>>> + ite->irq->pending_latch = true;
>>> + vgic_queue_irq_unlock(kvm, ite->irq);
>>>
>>> return 0;
>>> }
>>> @@ -511,15 +511,15 @@ int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi)
>>> }
>>>
>>> /* Requires the its_lock to be held. */
>>> -static void its_free_itte(struct kvm *kvm, struct its_itte *itte)
>>> +static void its_free_ite(struct kvm *kvm, struct its_ite *ite)
>>> {
>>> - list_del(&itte->itte_list);
>>> + list_del(&ite->ite_list);
>>>
>>> /* This put matches the get in vgic_add_lpi. */
>>> - if (itte->irq)
>>> - vgic_put_irq(kvm, itte->irq);
>>> + if (ite->irq)
>>> + vgic_put_irq(kvm, ite->irq);
>>>
>>> - kfree(itte);
>>> + kfree(ite);
>>> }
>>>
>>> static u64 its_cmd_mask_field(u64 *its_cmd, int word, int shift, int size)
>>> @@ -544,17 +544,17 @@ static int vgic_its_cmd_handle_discard(struct kvm *kvm, struct vgic_its *its,
>>> {
>>> u32 device_id = its_cmd_get_deviceid(its_cmd);
>>> u32 event_id = its_cmd_get_id(its_cmd);
>>> - struct its_itte *itte;
>>> + struct its_ite *ite;
>>>
>>>
>>> - itte = find_itte(its, device_id, event_id);
>>> - if (itte && itte->collection) {
>>> + ite = find_ite(its, device_id, event_id);
>>> + if (ite && ite->collection) {
>>> /*
>>> * Though the spec talks about removing the pending state, we
>>> * don't bother here since we clear the ITTE anyway and the
>>> * pending state is a property of the ITTE struct.
>>> */
>>> - its_free_itte(kvm, itte);
>>> + its_free_ite(kvm, ite);
>>> return 0;
>>> }
>>>
>>> @@ -572,26 +572,26 @@ static int vgic_its_cmd_handle_movi(struct kvm *kvm, struct vgic_its *its,
>>> u32 event_id = its_cmd_get_id(its_cmd);
>>> u32 coll_id = its_cmd_get_collection(its_cmd);
>>> struct kvm_vcpu *vcpu;
>>> - struct its_itte *itte;
>>> + struct its_ite *ite;
>>> struct its_collection *collection;
>>>
>>> - itte = find_itte(its, device_id, event_id);
>>> - if (!itte)
>>> + ite = find_ite(its, device_id, event_id);
>>> + if (!ite)
>>> return E_ITS_MOVI_UNMAPPED_INTERRUPT;
>>>
>>> - if (!its_is_collection_mapped(itte->collection))
>>> + if (!its_is_collection_mapped(ite->collection))
>>> return E_ITS_MOVI_UNMAPPED_COLLECTION;
>>>
>>> collection = find_collection(its, coll_id);
>>> if (!its_is_collection_mapped(collection))
>>> return E_ITS_MOVI_UNMAPPED_COLLECTION;
>>>
>>> - itte->collection = collection;
>>> + ite->collection = collection;
>>> vcpu = kvm_get_vcpu(kvm, collection->target_addr);
>>>
>>> - spin_lock(&itte->irq->irq_lock);
>>> - itte->irq->target_vcpu = vcpu;
>>> - spin_unlock(&itte->irq->irq_lock);
>>> + spin_lock(&ite->irq->irq_lock);
>>> + ite->irq->target_vcpu = vcpu;
>>> + spin_unlock(&ite->irq->irq_lock);
>>>
>>> return 0;
>>> }
>>> @@ -679,7 +679,7 @@ static void vgic_its_free_collection(struct vgic_its *its, u32 coll_id)
>>> {
>>> struct its_collection *collection;
>>> struct its_device *device;
>>> - struct its_itte *itte;
>>> + struct its_ite *ite;
>>>
>>> /*
>>> * Clearing the mapping for that collection ID removes the
>>> @@ -690,10 +690,10 @@ static void vgic_its_free_collection(struct vgic_its *its, u32 coll_id)
>>> if (!collection)
>>> return;
>>>
>>> - for_each_lpi_its(device, itte, its)
>>> - if (itte->collection &&
>>> - itte->collection->collection_id == coll_id)
>>> - itte->collection = NULL;
>>> + for_each_lpi_its(device, ite, its)
>>> + if (ite->collection &&
>>> + ite->collection->collection_id == coll_id)
>>> + ite->collection = NULL;
>>>
>>> list_del(&collection->coll_list);
>>> kfree(collection);
>>> @@ -709,7 +709,7 @@ static int vgic_its_cmd_handle_mapi(struct kvm *kvm, struct vgic_its *its,
>>> u32 device_id = its_cmd_get_deviceid(its_cmd);
>>> u32 event_id = its_cmd_get_id(its_cmd);
>>> u32 coll_id = its_cmd_get_collection(its_cmd);
>>> - struct its_itte *itte;
>>> + struct its_ite *ite;
>>> struct its_device *device;
>>> struct its_collection *collection, *new_coll = NULL;
>>> int lpi_nr;
>>> @@ -728,7 +728,7 @@ static int vgic_its_cmd_handle_mapi(struct kvm *kvm, struct vgic_its *its,
>>> return E_ITS_MAPTI_PHYSICALID_OOR;
>>>
>>> /* If there is an existing mapping, behavior is UNPREDICTABLE. */
>>> - if (find_itte(its, device_id, event_id))
>>> + if (find_ite(its, device_id, event_id))
>>> return 0;
>>>
>>> collection = find_collection(its, coll_id);
>>> @@ -739,36 +739,36 @@ static int vgic_its_cmd_handle_mapi(struct kvm *kvm, struct vgic_its *its,
>>> new_coll = collection;
>>> }
>>>
>>> - itte = kzalloc(sizeof(struct its_itte), GFP_KERNEL);
>>> - if (!itte) {
>>> + ite = kzalloc(sizeof(struct its_ite), GFP_KERNEL);
>>> + if (!ite) {
>>> if (new_coll)
>>> vgic_its_free_collection(its, coll_id);
>>> return -ENOMEM;
>>> }
>>>
>>> - itte->event_id = event_id;
>>> - list_add_tail(&itte->itte_list, &device->itt_head);
>>> + ite->event_id = event_id;
>>> + list_add_tail(&ite->ite_list, &device->itt_head);
>>>
>>> - itte->collection = collection;
>>> - itte->lpi = lpi_nr;
>>> + ite->collection = collection;
>>> + ite->lpi = lpi_nr;
>>>
>>> irq = vgic_add_lpi(kvm, lpi_nr);
>>> if (IS_ERR(irq)) {
>>> if (new_coll)
>>> vgic_its_free_collection(its, coll_id);
>>> - its_free_itte(kvm, itte);
>>> + its_free_ite(kvm, ite);
>>> return PTR_ERR(irq);
>>> }
>>> - itte->irq = irq;
>>> + ite->irq = irq;
>>>
>>> - update_affinity_itte(kvm, itte);
>>> + update_affinity_ite(kvm, ite);
>>>
>>> /*
>>> * We "cache" the configuration table entries in out struct vgic_irq's.
>>> * However we only have those structs for mapped IRQs, so we read in
>>> * the respective config data from memory here upon mapping the LPI.
>>> */
>>> - update_lpi_config(kvm, itte->irq, NULL);
>>> + update_lpi_config(kvm, ite->irq, NULL);
>>>
>>> return 0;
>>> }
>>> @@ -776,15 +776,15 @@ static int vgic_its_cmd_handle_mapi(struct kvm *kvm, struct vgic_its *its,
>>> /* Requires the its_lock to be held. */
>>> static void vgic_its_unmap_device(struct kvm *kvm, struct its_device *device)
>>> {
>>> - struct its_itte *itte, *temp;
>>> + struct its_ite *ite, *temp;
>>>
>>> /*
>>> * The spec says that unmapping a device with still valid
>>> * ITTEs associated is UNPREDICTABLE. We remove all ITTEs,
>>> * since we cannot leave the memory unreferenced.
>>> */
>>> - list_for_each_entry_safe(itte, temp, &device->itt_head, itte_list)
>>> - its_free_itte(kvm, itte);
>>> + list_for_each_entry_safe(ite, temp, &device->itt_head, ite_list)
>>> + its_free_ite(kvm, ite);
>>>
>>> list_del(&device->dev_list);
>>> kfree(device);
>>> @@ -883,14 +883,14 @@ static int vgic_its_cmd_handle_clear(struct kvm *kvm, struct vgic_its *its,
>>> {
>>> u32 device_id = its_cmd_get_deviceid(its_cmd);
>>> u32 event_id = its_cmd_get_id(its_cmd);
>>> - struct its_itte *itte;
>>> + struct its_ite *ite;
>>>
>>>
>>> - itte = find_itte(its, device_id, event_id);
>>> - if (!itte)
>>> + ite = find_ite(its, device_id, event_id);
>>> + if (!ite)
>>> return E_ITS_CLEAR_UNMAPPED_INTERRUPT;
>>>
>>> - itte->irq->pending_latch = false;
>>> + ite->irq->pending_latch = false;
>>>
>>> return 0;
>>> }
>>> @@ -904,14 +904,14 @@ static int vgic_its_cmd_handle_inv(struct kvm *kvm, struct vgic_its *its,
>>> {
>>> u32 device_id = its_cmd_get_deviceid(its_cmd);
>>> u32 event_id = its_cmd_get_id(its_cmd);
>>> - struct its_itte *itte;
>>> + struct its_ite *ite;
>>>
>>>
>>> - itte = find_itte(its, device_id, event_id);
>>> - if (!itte)
>>> + ite = find_ite(its, device_id, event_id);
>>> + if (!ite)
>>> return E_ITS_INV_UNMAPPED_INTERRUPT;
>>>
>>> - return update_lpi_config(kvm, itte->irq, NULL);
>>> + return update_lpi_config(kvm, ite->irq, NULL);
>>> }
>>>
>>> /*
>>> @@ -1435,7 +1435,7 @@ static void vgic_its_destroy(struct kvm_device *kvm_dev)
>>> struct kvm *kvm = kvm_dev->kvm;
>>> struct vgic_its *its = kvm_dev->private;
>>> struct its_device *dev;
>>> - struct its_itte *itte;
>>> + struct its_ite *ite;
>>> struct list_head *dev_cur, *dev_temp;
>>> struct list_head *cur, *temp;
>>>
>>> @@ -1450,8 +1450,8 @@ static void vgic_its_destroy(struct kvm_device *kvm_dev)
>>> list_for_each_safe(dev_cur, dev_temp, &its->device_list) {
>>> dev = container_of(dev_cur, struct its_device, dev_list);
>>> list_for_each_safe(cur, temp, &dev->itt_head) {
>>> - itte = (container_of(cur, struct its_itte, itte_list));
>>> - its_free_itte(kvm, itte);
>>> + ite = (container_of(cur, struct its_ite, ite_list));
>>> + its_free_ite(kvm, ite);
>>> }
>>> list_del(dev_cur);
>>> kfree(dev);
>>> --
>>> 2.5.5
>>>
^ permalink raw reply
* [PATCH v5 01/22] KVM: arm/arm64: Add ITS save/restore API documentation
From: Auger Eric @ 2017-04-27 9:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170427085709.GB50776@lvm>
Hi Christoffer,
On 27/04/2017 10:57, Christoffer Dall wrote:
> On Wed, Apr 26, 2017 at 05:48:32PM +0200, Auger Eric wrote:
>> Hi Christoffer,
>>
>> On 26/04/2017 14:31, Christoffer Dall wrote:
>>> On Fri, Apr 14, 2017 at 12:15:13PM +0200, Eric Auger wrote:
>>>> Add description for how to access ITS registers and how to save/restore
>>>> ITS tables into/from memory.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>
>>>> ---
>>>> v4 -> v5:
>>>> - take into account Christoffer's comments
>>>> - pending table save on GICV3 side now
>>>>
>>>> v3 -> v4:
>>>> - take into account Peter's comments:
>>>> - typos
>>>> - KVM_DEV_ARM_VGIC_GRP_ITS_TABLES kvm_device_attr = 0
>>>> - add a validity bit in DTE
>>>> - document all fields in CTE and ITE
>>>> - document ABI revision
>>>> - take into account Andre's comments:
>>>> - document restrictions about GITS_CREADR writing and GITS_IIDR
>>>> - document -EBUSY error if one or more VCPUS are runnning
>>>> - document 64b registers only can be accessed with 64b access
>>>> - itt_addr field matches bits [51:8] of the itt_addr
>>>>
>>>> v1 -> v2:
>>>> - DTE and ITE now are 8 bytes
>>>> - DTE and ITE now indexed by deviceid/eventid
>>>> - use ITE name instead of ITTE
>>>> - mentions ITT_addr matches bits [51:8] of the actual address
>>>> - mentions LE layout
>>>> ---
>>>> Documentation/virtual/kvm/devices/arm-vgic-its.txt | 99 ++++++++++++++++++++++
>>>> 1 file changed, 99 insertions(+)
>>>>
>>>> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-its.txt b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
>>>> index 6081a5b..b5f010d 100644
>>>> --- a/Documentation/virtual/kvm/devices/arm-vgic-its.txt
>>>> +++ b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
>>>> @@ -32,7 +32,106 @@ Groups:
>>>> KVM_DEV_ARM_VGIC_CTRL_INIT
>>>> request the initialization of the ITS, no additional parameter in
>>>> kvm_device_attr.addr.
>>>> +
>>>> + KVM_DEV_ARM_ITS_SAVE_TABLES
>>>> + save the ITS table data into guest RAM, at the location provisioned
>>>> + by the guest in corresponding registers/table entries.
>>>> +
>>>> + The layout of the tables in guest memory defines an ABI. The entries
>>>> + are laid out in little endian format as described in the last paragraph.
>>>> +
>>>> + KVM_DEV_ARM_ITS_RESTORE_TABLES
>>>> + restore the ITS tables from guest RAM to ITS internal structures.
>>>> +
>>>> + The GICV3 must be restored before the ITS and all ITS registers but
>>>> + the GITS_CTLR must be restored before restoring the ITS tables.
>>>> +
>>>> + The GITS_IIDR read-only register must also be restored before
>>>> + the table restore as the IIDR revision field encodes the ABI revision.
>>>> +
>>>
>>> what is the expected sequence of operations. For example, to restore
>>> the ITS, do I call KVM_DEV_ARM_VGIC_CTRL_INIT first, then restore all
>>> the memory and registers, and finally call KVM_DEV_ARM_ITS_RESTORE_TABLES?
>> Yes KVM_DEV_ARM_VGIC_CTRL_INIT comes first, then restore all registers
>> except GITS_CTLR, then table restore, then GITS_CTLR
>>>
>>> Is there any interaction between when you call KVM_DEV_ARM_ITS_RESTORE_TABLES
>>> and restore GITS_CTLR (which enables the ITS)?
>>
>> Yep, when GITS_CTLR is set, LPIs may be enabled and this on that event
>> that the pending table is read. But the whole pending table is not read
>> as we only iterate on registered LPIs. So the ITT must have been
>> restored previously.
>>
>> I became aware that the pending table sync is done twice, once in the
>> pending table restore, and once in the GITS_CTLR restore. So if we
>> leave this order specification, I should be able to remove the sync on
>> table restore. This was the original reason why GITS_CTLR restore has
>> been done at the very end.
>
> I'm sorry, I'm a bit confused. Do we not need
> KVM_DEV_ARM_ITS_RESTORE_TABLES at all then?
Yes you do. I was talking about the RDIST pending table sync. The save
is explicit using GICV3 device KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES.
However the sync is implicit on GITS_CTLR restore if LPIs are enabled.
and today I do it also on ITS device KVM_DEV_ARM_ITS_RESTORE_TABLES
which is not requested I think since GITS_CTLR restore does it already.
KVM_DEV_ARM_ITS_RESTORE_TABLES restores all the ITS tables (device,
collection, ITT)
Hope it clarifies.
Thanks
Eric
>
>>>
>>>> Errors:
>>>> -ENXIO: ITS not properly configured as required prior to setting
>>>> this attribute
>>>> -ENOMEM: Memory shortage when allocating ITS internal data
>>>> + -EINVAL: Inconsistent restored data
>>>> + -EFAULT: Invalid guest ram access
>>>> + -EBUSY: One or more VCPUS are running
>>>> +
>>>> + KVM_DEV_ARM_VGIC_GRP_ITS_REGS
>>>> + Attributes:
>>>> + The attr field of kvm_device_attr encodes the offset of the
>>>> + ITS register, relative to the ITS control frame base address
>>>> + (ITS_base).
>>>> +
>>>> + kvm_device_attr.addr points to a __u64 value whatever the width
>>>> + of the addressed register (32/64 bits). 64 bit registers can only
>>>> + be accessed with full length.
>>>> +
>>>> + Writes to read-only registers are ignored by the kernel except for:
>>>> + - GITS_READR. It needs to be restored otherwise commands in the queue
>>>> + will be re-executed after restoring CWRITER. GITS_READR must be restored
>>>> + before restoring the GITS_CTLR which is likely to enable the ITS.
>>>> + Also it needs to be restored after GITS_CBASER since a write to
>>>> + GITS_CBASER resets GITS_CREADR.
>>>> + - GITS_IIDR. Its Revision field encodes the table layout ABI revision.
>>>> + In the future we might implement direct injection of virtual LPIS.
>>>> + This will require an upgrade of the table layout and an evolution of
>>>> + the ABI. GITS_IIDR must be restored before the table restoration.
>>>> +
>>>> + For other registers, getting or setting a register has the same
>>>> + effect as reading/writing the register on real hardware.
>>>> + Errors:
>>>> + -ENXIO: Offset does not correspond to any supported register
>>>> + -EFAULT: Invalid user pointer for attr->addr
>>>> + -EINVAL: Offset is not 64-bit aligned
>>>> + -EBUSY: one or more VCPUS are running
>>>
>>>
>>> It may be helpful to state the ordering requirements somewhere:
>>>
>>> Restoring the ITS:
>>> ------------------
>>> Restoring the ITS requires certain things to happen in order.
>>> Specifically:
>>> 1. Initialize the ITS (KVM_DEV_ARM_VGIC_CTRL_INIT)
>>> 2. Restore GITS_IIDR
>>> 3. Restore GITS_CBASER
>>> 4. Restore GITS_READR
>>> 5. Restore remainin registers except GITS_CTLR
>>> 6. Make sure all guest memory is restored
>>> 7. Load the ITS table data (KVM_DEV_ARM_ITS_RESTORE_TABLES)
>>
>> OK I will try to fit that description somewhere.
>>
>
> Thanks,
> -Christoffer
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* Duplicate .plt sections warning with CONFIG_ARM_MODULE_PLTS
From: Ard Biesheuvel @ 2017-04-27 9:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6bd5005b-61d1-d2f9-fb0f-5ce200bfffd9@gmail.com>
On 27 April 2017 at 00:28, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 04/26/2017 12:33 AM, Ard Biesheuvel wrote:
>> On 26 April 2017 at 00:35, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>> Hi Ard,
>>>
>>> While using CONFIG_ARM_MODULES_PLTS=y along with a large kernel module,
>>> I was able to have the kernel/module.c::add_sect_attrs() to complain
>>> about duplicate .plt sections found, each section gets its own sysfs
>>> attribute created under /sys/module/<modname>/sections/<sectname>
>>>
>>> And indeed the module has the following section headers and contents
>>> (see below). Is this something that looks legit or should we patch
>>> kernel/module.c to detect such duplicates and generate unique sysfs
>>> attribute section names instead? There does not appear to be any
>>> functional issue with that.
>>>
>>
>> Does it happen with a clean build?
>
> Yes it does, it was not quite the problem see below.
>
>> The .plt/.init.plt sections are
>> declared in the linker scripts, and so no module object should have
>> any such sections until the relocatable link occurs that combines them
>> all into a .ko, and it is at this point that these sections are
>> declared.
>
> It turned out that the module build I was debugging was built in a
> strange way with a first step producing foo.o_shipped, which (semi)
> manually pulling into the kernel's linker scripts, and then,
> foo.o_shipped was used to link foo.ko which then again got the linker
> scripts from the kernel. This is how we ended up with the two .plt and
> two .init.plt sections being present in foo.ko.
>
OK, so the fix is "don't do that" :-)
^ permalink raw reply
* [PATCH v5 03/22] KVM: arm/arm64: vgic-its: rename itte into ite
From: Andre Przywara @ 2017-04-27 9:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170427090504.GC50776@lvm>
Hi,
On 27/04/17 10:05, Christoffer Dall wrote:
> On Fri, Apr 14, 2017 at 12:15:15PM +0200, Eric Auger wrote:
>> The actual abbreviation for the interrupt translation table entry
>> is ITE. Let's rename all itte instances by ite.
>
> Is there really any confusion or problems with using itte? This is a
> lot of churn...
I tend to agree (just didn't dare to mention this before).
I see that the spec speaks of "ITE", but the spelled out term hints more
at ITTE (because it's a "translation table").
Besides three letters tend to be more ambiguous than a four letter
identifier.
Would adding a comment to the structure definition help?
But speaking of churn I am not sure how much more work dropping this
patch now creates on Eric's side ...
Cheers,
Andre.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>>
>> ---
>>
>> v5: Add Marc's A-b
>> ---
>> virt/kvm/arm/vgic/vgic-its.c | 148 +++++++++++++++++++++----------------------
>> 1 file changed, 74 insertions(+), 74 deletions(-)
>>
>> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
>> index 8d1da1a..3ffcbbe 100644
>> --- a/virt/kvm/arm/vgic/vgic-its.c
>> +++ b/virt/kvm/arm/vgic/vgic-its.c
>> @@ -114,8 +114,8 @@ struct its_collection {
>> #define its_is_collection_mapped(coll) ((coll) && \
>> ((coll)->target_addr != COLLECTION_NOT_MAPPED))
>>
>> -struct its_itte {
>> - struct list_head itte_list;
>> +struct its_ite {
>> + struct list_head ite_list;
>>
>> struct vgic_irq *irq;
>> struct its_collection *collection;
>> @@ -143,27 +143,27 @@ static struct its_device *find_its_device(struct vgic_its *its, u32 device_id)
>> * Device ID/Event ID pair on an ITS.
>> * Must be called with the its_lock mutex held.
>> */
>> -static struct its_itte *find_itte(struct vgic_its *its, u32 device_id,
>> +static struct its_ite *find_ite(struct vgic_its *its, u32 device_id,
>> u32 event_id)
>> {
>> struct its_device *device;
>> - struct its_itte *itte;
>> + struct its_ite *ite;
>>
>> device = find_its_device(its, device_id);
>> if (device == NULL)
>> return NULL;
>>
>> - list_for_each_entry(itte, &device->itt_head, itte_list)
>> - if (itte->event_id == event_id)
>> - return itte;
>> + list_for_each_entry(ite, &device->itt_head, ite_list)
>> + if (ite->event_id == event_id)
>> + return ite;
>>
>> return NULL;
>> }
>>
>> /* To be used as an iterator this macro misses the enclosing parentheses */
>> -#define for_each_lpi_its(dev, itte, its) \
>> +#define for_each_lpi_its(dev, ite, its) \
>> list_for_each_entry(dev, &(its)->device_list, dev_list) \
>> - list_for_each_entry(itte, &(dev)->itt_head, itte_list)
>> + list_for_each_entry(ite, &(dev)->itt_head, ite_list)
>>
>> /*
>> * We only implement 48 bits of PA at the moment, although the ITS
>> @@ -270,18 +270,18 @@ static int vgic_copy_lpi_list(struct kvm *kvm, u32 **intid_ptr)
>> * Needs to be called whenever either the collection for a LPIs has
>> * changed or the collection itself got retargeted.
>> */
>> -static void update_affinity_itte(struct kvm *kvm, struct its_itte *itte)
>> +static void update_affinity_ite(struct kvm *kvm, struct its_ite *ite)
>> {
>> struct kvm_vcpu *vcpu;
>>
>> - if (!its_is_collection_mapped(itte->collection))
>> + if (!its_is_collection_mapped(ite->collection))
>> return;
>>
>> - vcpu = kvm_get_vcpu(kvm, itte->collection->target_addr);
>> + vcpu = kvm_get_vcpu(kvm, ite->collection->target_addr);
>>
>> - spin_lock(&itte->irq->irq_lock);
>> - itte->irq->target_vcpu = vcpu;
>> - spin_unlock(&itte->irq->irq_lock);
>> + spin_lock(&ite->irq->irq_lock);
>> + ite->irq->target_vcpu = vcpu;
>> + spin_unlock(&ite->irq->irq_lock);
>> }
>>
>> /*
>> @@ -292,13 +292,13 @@ static void update_affinity_collection(struct kvm *kvm, struct vgic_its *its,
>> struct its_collection *coll)
>> {
>> struct its_device *device;
>> - struct its_itte *itte;
>> + struct its_ite *ite;
>>
>> - for_each_lpi_its(device, itte, its) {
>> - if (!itte->collection || coll != itte->collection)
>> + for_each_lpi_its(device, ite, its) {
>> + if (!ite->collection || coll != ite->collection)
>> continue;
>>
>> - update_affinity_itte(kvm, itte);
>> + update_affinity_ite(kvm, ite);
>> }
>> }
>>
>> @@ -425,25 +425,25 @@ static int vgic_its_trigger_msi(struct kvm *kvm, struct vgic_its *its,
>> u32 devid, u32 eventid)
>> {
>> struct kvm_vcpu *vcpu;
>> - struct its_itte *itte;
>> + struct its_ite *ite;
>>
>> if (!its->enabled)
>> return -EBUSY;
>>
>> - itte = find_itte(its, devid, eventid);
>> - if (!itte || !its_is_collection_mapped(itte->collection))
>> + ite = find_ite(its, devid, eventid);
>> + if (!ite || !its_is_collection_mapped(ite->collection))
>> return E_ITS_INT_UNMAPPED_INTERRUPT;
>>
>> - vcpu = kvm_get_vcpu(kvm, itte->collection->target_addr);
>> + vcpu = kvm_get_vcpu(kvm, ite->collection->target_addr);
>> if (!vcpu)
>> return E_ITS_INT_UNMAPPED_INTERRUPT;
>>
>> if (!vcpu->arch.vgic_cpu.lpis_enabled)
>> return -EBUSY;
>>
>> - spin_lock(&itte->irq->irq_lock);
>> - itte->irq->pending_latch = true;
>> - vgic_queue_irq_unlock(kvm, itte->irq);
>> + spin_lock(&ite->irq->irq_lock);
>> + ite->irq->pending_latch = true;
>> + vgic_queue_irq_unlock(kvm, ite->irq);
>>
>> return 0;
>> }
>> @@ -511,15 +511,15 @@ int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi)
>> }
>>
>> /* Requires the its_lock to be held. */
>> -static void its_free_itte(struct kvm *kvm, struct its_itte *itte)
>> +static void its_free_ite(struct kvm *kvm, struct its_ite *ite)
>> {
>> - list_del(&itte->itte_list);
>> + list_del(&ite->ite_list);
>>
>> /* This put matches the get in vgic_add_lpi. */
>> - if (itte->irq)
>> - vgic_put_irq(kvm, itte->irq);
>> + if (ite->irq)
>> + vgic_put_irq(kvm, ite->irq);
>>
>> - kfree(itte);
>> + kfree(ite);
>> }
>>
>> static u64 its_cmd_mask_field(u64 *its_cmd, int word, int shift, int size)
>> @@ -544,17 +544,17 @@ static int vgic_its_cmd_handle_discard(struct kvm *kvm, struct vgic_its *its,
>> {
>> u32 device_id = its_cmd_get_deviceid(its_cmd);
>> u32 event_id = its_cmd_get_id(its_cmd);
>> - struct its_itte *itte;
>> + struct its_ite *ite;
>>
>>
>> - itte = find_itte(its, device_id, event_id);
>> - if (itte && itte->collection) {
>> + ite = find_ite(its, device_id, event_id);
>> + if (ite && ite->collection) {
>> /*
>> * Though the spec talks about removing the pending state, we
>> * don't bother here since we clear the ITTE anyway and the
>> * pending state is a property of the ITTE struct.
>> */
>> - its_free_itte(kvm, itte);
>> + its_free_ite(kvm, ite);
>> return 0;
>> }
>>
>> @@ -572,26 +572,26 @@ static int vgic_its_cmd_handle_movi(struct kvm *kvm, struct vgic_its *its,
>> u32 event_id = its_cmd_get_id(its_cmd);
>> u32 coll_id = its_cmd_get_collection(its_cmd);
>> struct kvm_vcpu *vcpu;
>> - struct its_itte *itte;
>> + struct its_ite *ite;
>> struct its_collection *collection;
>>
>> - itte = find_itte(its, device_id, event_id);
>> - if (!itte)
>> + ite = find_ite(its, device_id, event_id);
>> + if (!ite)
>> return E_ITS_MOVI_UNMAPPED_INTERRUPT;
>>
>> - if (!its_is_collection_mapped(itte->collection))
>> + if (!its_is_collection_mapped(ite->collection))
>> return E_ITS_MOVI_UNMAPPED_COLLECTION;
>>
>> collection = find_collection(its, coll_id);
>> if (!its_is_collection_mapped(collection))
>> return E_ITS_MOVI_UNMAPPED_COLLECTION;
>>
>> - itte->collection = collection;
>> + ite->collection = collection;
>> vcpu = kvm_get_vcpu(kvm, collection->target_addr);
>>
>> - spin_lock(&itte->irq->irq_lock);
>> - itte->irq->target_vcpu = vcpu;
>> - spin_unlock(&itte->irq->irq_lock);
>> + spin_lock(&ite->irq->irq_lock);
>> + ite->irq->target_vcpu = vcpu;
>> + spin_unlock(&ite->irq->irq_lock);
>>
>> return 0;
>> }
>> @@ -679,7 +679,7 @@ static void vgic_its_free_collection(struct vgic_its *its, u32 coll_id)
>> {
>> struct its_collection *collection;
>> struct its_device *device;
>> - struct its_itte *itte;
>> + struct its_ite *ite;
>>
>> /*
>> * Clearing the mapping for that collection ID removes the
>> @@ -690,10 +690,10 @@ static void vgic_its_free_collection(struct vgic_its *its, u32 coll_id)
>> if (!collection)
>> return;
>>
>> - for_each_lpi_its(device, itte, its)
>> - if (itte->collection &&
>> - itte->collection->collection_id == coll_id)
>> - itte->collection = NULL;
>> + for_each_lpi_its(device, ite, its)
>> + if (ite->collection &&
>> + ite->collection->collection_id == coll_id)
>> + ite->collection = NULL;
>>
>> list_del(&collection->coll_list);
>> kfree(collection);
>> @@ -709,7 +709,7 @@ static int vgic_its_cmd_handle_mapi(struct kvm *kvm, struct vgic_its *its,
>> u32 device_id = its_cmd_get_deviceid(its_cmd);
>> u32 event_id = its_cmd_get_id(its_cmd);
>> u32 coll_id = its_cmd_get_collection(its_cmd);
>> - struct its_itte *itte;
>> + struct its_ite *ite;
>> struct its_device *device;
>> struct its_collection *collection, *new_coll = NULL;
>> int lpi_nr;
>> @@ -728,7 +728,7 @@ static int vgic_its_cmd_handle_mapi(struct kvm *kvm, struct vgic_its *its,
>> return E_ITS_MAPTI_PHYSICALID_OOR;
>>
>> /* If there is an existing mapping, behavior is UNPREDICTABLE. */
>> - if (find_itte(its, device_id, event_id))
>> + if (find_ite(its, device_id, event_id))
>> return 0;
>>
>> collection = find_collection(its, coll_id);
>> @@ -739,36 +739,36 @@ static int vgic_its_cmd_handle_mapi(struct kvm *kvm, struct vgic_its *its,
>> new_coll = collection;
>> }
>>
>> - itte = kzalloc(sizeof(struct its_itte), GFP_KERNEL);
>> - if (!itte) {
>> + ite = kzalloc(sizeof(struct its_ite), GFP_KERNEL);
>> + if (!ite) {
>> if (new_coll)
>> vgic_its_free_collection(its, coll_id);
>> return -ENOMEM;
>> }
>>
>> - itte->event_id = event_id;
>> - list_add_tail(&itte->itte_list, &device->itt_head);
>> + ite->event_id = event_id;
>> + list_add_tail(&ite->ite_list, &device->itt_head);
>>
>> - itte->collection = collection;
>> - itte->lpi = lpi_nr;
>> + ite->collection = collection;
>> + ite->lpi = lpi_nr;
>>
>> irq = vgic_add_lpi(kvm, lpi_nr);
>> if (IS_ERR(irq)) {
>> if (new_coll)
>> vgic_its_free_collection(its, coll_id);
>> - its_free_itte(kvm, itte);
>> + its_free_ite(kvm, ite);
>> return PTR_ERR(irq);
>> }
>> - itte->irq = irq;
>> + ite->irq = irq;
>>
>> - update_affinity_itte(kvm, itte);
>> + update_affinity_ite(kvm, ite);
>>
>> /*
>> * We "cache" the configuration table entries in out struct vgic_irq's.
>> * However we only have those structs for mapped IRQs, so we read in
>> * the respective config data from memory here upon mapping the LPI.
>> */
>> - update_lpi_config(kvm, itte->irq, NULL);
>> + update_lpi_config(kvm, ite->irq, NULL);
>>
>> return 0;
>> }
>> @@ -776,15 +776,15 @@ static int vgic_its_cmd_handle_mapi(struct kvm *kvm, struct vgic_its *its,
>> /* Requires the its_lock to be held. */
>> static void vgic_its_unmap_device(struct kvm *kvm, struct its_device *device)
>> {
>> - struct its_itte *itte, *temp;
>> + struct its_ite *ite, *temp;
>>
>> /*
>> * The spec says that unmapping a device with still valid
>> * ITTEs associated is UNPREDICTABLE. We remove all ITTEs,
>> * since we cannot leave the memory unreferenced.
>> */
>> - list_for_each_entry_safe(itte, temp, &device->itt_head, itte_list)
>> - its_free_itte(kvm, itte);
>> + list_for_each_entry_safe(ite, temp, &device->itt_head, ite_list)
>> + its_free_ite(kvm, ite);
>>
>> list_del(&device->dev_list);
>> kfree(device);
>> @@ -883,14 +883,14 @@ static int vgic_its_cmd_handle_clear(struct kvm *kvm, struct vgic_its *its,
>> {
>> u32 device_id = its_cmd_get_deviceid(its_cmd);
>> u32 event_id = its_cmd_get_id(its_cmd);
>> - struct its_itte *itte;
>> + struct its_ite *ite;
>>
>>
>> - itte = find_itte(its, device_id, event_id);
>> - if (!itte)
>> + ite = find_ite(its, device_id, event_id);
>> + if (!ite)
>> return E_ITS_CLEAR_UNMAPPED_INTERRUPT;
>>
>> - itte->irq->pending_latch = false;
>> + ite->irq->pending_latch = false;
>>
>> return 0;
>> }
>> @@ -904,14 +904,14 @@ static int vgic_its_cmd_handle_inv(struct kvm *kvm, struct vgic_its *its,
>> {
>> u32 device_id = its_cmd_get_deviceid(its_cmd);
>> u32 event_id = its_cmd_get_id(its_cmd);
>> - struct its_itte *itte;
>> + struct its_ite *ite;
>>
>>
>> - itte = find_itte(its, device_id, event_id);
>> - if (!itte)
>> + ite = find_ite(its, device_id, event_id);
>> + if (!ite)
>> return E_ITS_INV_UNMAPPED_INTERRUPT;
>>
>> - return update_lpi_config(kvm, itte->irq, NULL);
>> + return update_lpi_config(kvm, ite->irq, NULL);
>> }
>>
>> /*
>> @@ -1435,7 +1435,7 @@ static void vgic_its_destroy(struct kvm_device *kvm_dev)
>> struct kvm *kvm = kvm_dev->kvm;
>> struct vgic_its *its = kvm_dev->private;
>> struct its_device *dev;
>> - struct its_itte *itte;
>> + struct its_ite *ite;
>> struct list_head *dev_cur, *dev_temp;
>> struct list_head *cur, *temp;
>>
>> @@ -1450,8 +1450,8 @@ static void vgic_its_destroy(struct kvm_device *kvm_dev)
>> list_for_each_safe(dev_cur, dev_temp, &its->device_list) {
>> dev = container_of(dev_cur, struct its_device, dev_list);
>> list_for_each_safe(cur, temp, &dev->itt_head) {
>> - itte = (container_of(cur, struct its_itte, itte_list));
>> - its_free_itte(kvm, itte);
>> + ite = (container_of(cur, struct its_ite, ite_list));
>> + its_free_ite(kvm, ite);
>> }
>> list_del(dev_cur);
>> kfree(dev);
>> --
>> 2.5.5
>>
^ permalink raw reply
* [PATCH v5 06/22] KVM: arm/arm64: vgic: expose (un)lock_all_vcpus
From: Christoffer Dall @ 2017-04-27 9:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1492164934-988-7-git-send-email-eric.auger@redhat.com>
On Fri, Apr 14, 2017 at 12:15:18PM +0200, Eric Auger wrote:
> We need to use those helpers in vgic-its.c so let's
> expose them in the private vgic header.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Christoffer Dall <cdall@linaro.org>
>
> ---
>
> v4 -> v5:
> - Add Marc's A-b
> ---
> virt/kvm/arm/vgic/vgic-kvm-device.c | 4 ++--
> virt/kvm/arm/vgic/vgic.h | 3 +++
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
> index d181d2b..859bfa8 100644
> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
> @@ -259,13 +259,13 @@ static void unlock_vcpus(struct kvm *kvm, int vcpu_lock_idx)
> }
> }
>
> -static void unlock_all_vcpus(struct kvm *kvm)
> +void unlock_all_vcpus(struct kvm *kvm)
> {
> unlock_vcpus(kvm, atomic_read(&kvm->online_vcpus) - 1);
> }
>
> /* Returns true if all vcpus were locked, false otherwise */
> -static bool lock_all_vcpus(struct kvm *kvm)
> +bool lock_all_vcpus(struct kvm *kvm)
> {
> struct kvm_vcpu *tmp_vcpu;
> int c;
> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
> index 6cf557e..b87f1c6 100644
> --- a/virt/kvm/arm/vgic/vgic.h
> +++ b/virt/kvm/arm/vgic/vgic.h
> @@ -184,4 +184,7 @@ int vgic_init(struct kvm *kvm);
> int vgic_debug_init(struct kvm *kvm);
> int vgic_debug_destroy(struct kvm *kvm);
>
> +bool lock_all_vcpus(struct kvm *kvm);
> +void unlock_all_vcpus(struct kvm *kvm);
> +
> #endif
> --
> 2.5.5
>
^ 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