* [PATCH 1/3] arm: hisi: add ARCH_MULTI_V5 support
From: Marty Plummer @ 2016-12-09 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2fcf5a5f-8935-c4f0-a034-c8a72dc2be0f@hisilicon.com>
On 12/04/2016 08:03 PM, Jiancheng Xue wrote:
> Hi Arnd,
>
> On 2016/10/17 21:48, Arnd Bergmann wrote:
>> On Monday, October 17, 2016 8:07:03 PM CEST Pan Wen wrote:
>>> Add support for some HiSilicon SoCs which depend on ARCH_MULTI_V5.
>>>
>>> Signed-off-by: Pan Wen <wenpan@hisilicon.com>
>>>
>>
>> Looks ok. I've added Marty Plummer to Cc, he was recently proposing
>> patches for Hi3520, which I think is closely related to this one.
>> Please try to work together so the patches don't conflict. It should
>> be fairly straightforward since you are basically doing the same
>> change here.
>>
> Marty hasn't give any replies about this thread until now. I reviewed
> the patch for Hi3520. And I think this patch won't conflict with Hi3520.
> Could you help us to ack this patch?
>
> Thanks,
> Jiancheng
>
>
Hello all
Sorry for my lack of activity, I've just been very busy lately with real
world considerations (well, real world but related to this; I have
another board based on hi3521a I've been tinkering with, trying to get
the manuf. to release gpl source via the sfconfservancy). I've not given
up on the project, however, since devices like this really need updates
in light of the recent botnets targeting devices of this sort as
manpower.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161209/978fa980/attachment.sig>
^ permalink raw reply
* [PATCH] nommu: allow mmap when !CONFIG_MMU
From: Benjamin Gaignard @ 2016-12-09 15:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480600080-30196-1-git-send-email-benjamin.gaignard@linaro.org>
+ Vladimir
2016-12-01 14:48 GMT+01:00 Benjamin Gaignard <benjamin.gaignard@linaro.org>:
> commit ab6494f0c96f ("nommu: Add noMMU support to the DMA API") have
> add CONFIG_MMU compilation flag but that prohibit to use dma_mmap_wc()
> when the platform doesn't have MMU.
>
> This patch call vm_iomap_memory() in noMMU case to test if addresses
> are correct and set wma->vm_flags rather than all return an error.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: arnd at arndb.de
> ---
> arch/arm/mm/dma-mapping.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index ab4f745..230875e 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -868,6 +868,9 @@ static int __arm_dma_mmap(struct device *dev, struct vm_area_struct *vma,
> vma->vm_end - vma->vm_start,
> vma->vm_page_prot);
> }
> +#else
> + ret = vm_iomap_memory(vma, vma->vm_start,
> + (vma->vm_end - vma->vm_start));
> #endif /* CONFIG_MMU */
>
> return ret;
> --
> 1.9.1
>
^ permalink raw reply
* [PATCH] nommu: allow mmap when !CONFIG_MMU
From: Vladimir Murzin @ 2016-12-09 15:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+M3ks67a9H2Ro87mPZ1_3=ucnyWNMMKnr_EW=dLS1kBxuU3Lw@mail.gmail.com>
On 09/12/16 15:27, Benjamin Gaignard wrote:
> + Vladimir
I'm not in DMA, but I can see that with your patch it deviates from
dma_common_mmap(), can you give more context, please?
Cheers
Vladimir
>
> 2016-12-01 14:48 GMT+01:00 Benjamin Gaignard <benjamin.gaignard@linaro.org>:
>> commit ab6494f0c96f ("nommu: Add noMMU support to the DMA API") have
>> add CONFIG_MMU compilation flag but that prohibit to use dma_mmap_wc()
>> when the platform doesn't have MMU.
>>
>> This patch call vm_iomap_memory() in noMMU case to test if addresses
>> are correct and set wma->vm_flags rather than all return an error.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: arnd at arndb.de
>> ---
>> arch/arm/mm/dma-mapping.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>> index ab4f745..230875e 100644
>> --- a/arch/arm/mm/dma-mapping.c
>> +++ b/arch/arm/mm/dma-mapping.c
>> @@ -868,6 +868,9 @@ static int __arm_dma_mmap(struct device *dev, struct vm_area_struct *vma,
>> vma->vm_end - vma->vm_start,
>> vma->vm_page_prot);
>> }
>> +#else
>> + ret = vm_iomap_memory(vma, vma->vm_start,
>> + (vma->vm_end - vma->vm_start));
>> #endif /* CONFIG_MMU */
>>
>> return ret;
>> --
>> 1.9.1
>>
>
^ permalink raw reply
* [PATCH] nommu: allow mmap when !CONFIG_MMU
From: Benjamin Gaignard @ 2016-12-09 15:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <584ACF82.5090609@arm.com>
2016-12-09 16:36 GMT+01:00 Vladimir Murzin <vladimir.murzin@arm.com>:
> On 09/12/16 15:27, Benjamin Gaignard wrote:
>> + Vladimir
>
> I'm not in DMA, but I can see that with your patch it deviates from
> dma_common_mmap(), can you give more context, please?
I'm working on ARM platform with MMU (stm32f4) to enable display driver.
Framebuffer is allocated with dma_alloc_wc() and when userland try to mmap
drm/kms fraemwork calls dma_mmap_wc().
All this is in drivers/gpu/drm/drm_gem_cma_helper.c
dma_mmap_wc() call failed because __arm_dma_mmap() always return an
error if CONFIG_MMU
isn't defined.
That what I try to solve with this patch.
> Cheers
> Vladimir
>
>>
>> 2016-12-01 14:48 GMT+01:00 Benjamin Gaignard <benjamin.gaignard@linaro.org>:
>>> commit ab6494f0c96f ("nommu: Add noMMU support to the DMA API") have
>>> add CONFIG_MMU compilation flag but that prohibit to use dma_mmap_wc()
>>> when the platform doesn't have MMU.
>>>
>>> This patch call vm_iomap_memory() in noMMU case to test if addresses
>>> are correct and set wma->vm_flags rather than all return an error.
>>>
>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: arnd at arndb.de
>>> ---
>>> arch/arm/mm/dma-mapping.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>>> index ab4f745..230875e 100644
>>> --- a/arch/arm/mm/dma-mapping.c
>>> +++ b/arch/arm/mm/dma-mapping.c
>>> @@ -868,6 +868,9 @@ static int __arm_dma_mmap(struct device *dev, struct vm_area_struct *vma,
>>> vma->vm_end - vma->vm_start,
>>> vma->vm_page_prot);
>>> }
>>> +#else
>>> + ret = vm_iomap_memory(vma, vma->vm_start,
>>> + (vma->vm_end - vma->vm_start));
>>> #endif /* CONFIG_MMU */
>>>
>>> return ret;
>>> --
>>> 1.9.1
>>>
>>
>
--
Benjamin Gaignard
Graphic Study Group
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PULL] KVM/ARM updates for 4.10
From: Marc Zyngier @ 2016-12-09 15:53 UTC (permalink / raw)
To: linux-arm-kernel
[Now with the various lists on Cc... sorry for the duplicates]
Paolo, Radim,
Here's the KVM/ARM updates for 4.10. Only a handful of patches this
time, as what we had planned for this cycle (GICv3 save/restore,
mainly) is taking much more time than expected.
The only feature worth mentionning is the support of the GICv3 ITS on
32bit ARMv8 platforms. The rest is only a bunch of fixes.
Thanks,
M.
The following changes since commit a25f0944ba9b1d8a6813fd6f1a86f1bd59ac25a6:
Linux 4.9-rc5 (2016-11-13 10:32:32 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git tags/kvm-arm-for-4.10
for you to fetch changes up to 21cbe3cc8a48ff17059912e019fbde28ed54745a:
arm64: KVM: pmu: Reset PMSELR_EL0.SEL to a sane value before entering the guest (2016-12-09 15:47:00 +0000)
----------------------------------------------------------------
KVM/ARM updates for 4.10:
- Support for the GICv3 ITS on 32bit platforms
- A handful of timer and GIC emulation fixes
- A PMU architecture fix
----------------------------------------------------------------
Andre Przywara (1):
KVM: arm/arm64: vgic-v2: Limit ITARGETSR bits to number of VCPUs
Christoffer Dall (1):
KVM: arm/arm64: timer: Check for properly initialized timer on init
Longpeng(Mike) (1):
arm/arm64: KVM: Clean up useless code in kvm_timer_enable
Marc Zyngier (1):
arm64: KVM: pmu: Reset PMSELR_EL0.SEL to a sane value before entering the guest
Vladimir Murzin (2):
KVM: arm64: vgic-its: Fix compatibility with 32-bit
ARM: KVM: Support vGICv3 ITS
Documentation/virtual/kvm/api.txt | 2 +-
arch/arm/include/uapi/asm/kvm.h | 2 ++
arch/arm/kvm/Kconfig | 1 +
arch/arm/kvm/Makefile | 1 +
arch/arm/kvm/arm.c | 6 ++++++
arch/arm64/kvm/Kconfig | 4 ----
arch/arm64/kvm/hyp/switch.c | 8 +++++++-
arch/arm64/kvm/reset.c | 6 ------
include/linux/irqchip/arm-gic-v3.h | 8 ++++----
virt/kvm/arm/arch_timer.c | 17 ++++++-----------
virt/kvm/arm/vgic/vgic-its.c | 11 ++++++-----
virt/kvm/arm/vgic/vgic-kvm-device.c | 2 --
virt/kvm/arm/vgic/vgic-mmio-v2.c | 3 ++-
virt/kvm/arm/vgic/vgic-mmio-v3.c | 2 --
virt/kvm/arm/vgic/vgic.h | 26 --------------------------
15 files changed, 36 insertions(+), 63 deletions(-)
^ permalink raw reply
* [PATCH 1/6] KVM: arm64: vgic-its: Fix compatibility with 32-bit
From: Marc Zyngier @ 2016-12-09 15:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481298811-30798-1-git-send-email-marc.zyngier@arm.com>
From: Vladimir Murzin <vladimir.murzin@arm.com>
Evaluate GITS_BASER_ENTRY_SIZE once as an int data (GITS_BASER<n>'s
Entry Size is 5-bit wide only), so when used as divider no reference
to __aeabi_uldivmod is generated when build for AArch32.
Use unsigned long long for GITS_BASER_PAGE_SIZE_* since they are
used in conjunction with 64-bit data.
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
include/linux/irqchip/arm-gic-v3.h | 8 ++++----
virt/kvm/arm/vgic/vgic-its.c | 11 ++++++-----
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index b7e3431..0deea34 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -295,10 +295,10 @@
#define GITS_BASER_InnerShareable \
GIC_BASER_SHAREABILITY(GITS_BASER, InnerShareable)
#define GITS_BASER_PAGE_SIZE_SHIFT (8)
-#define GITS_BASER_PAGE_SIZE_4K (0UL << GITS_BASER_PAGE_SIZE_SHIFT)
-#define GITS_BASER_PAGE_SIZE_16K (1UL << GITS_BASER_PAGE_SIZE_SHIFT)
-#define GITS_BASER_PAGE_SIZE_64K (2UL << GITS_BASER_PAGE_SIZE_SHIFT)
-#define GITS_BASER_PAGE_SIZE_MASK (3UL << GITS_BASER_PAGE_SIZE_SHIFT)
+#define GITS_BASER_PAGE_SIZE_4K (0ULL << GITS_BASER_PAGE_SIZE_SHIFT)
+#define GITS_BASER_PAGE_SIZE_16K (1ULL << GITS_BASER_PAGE_SIZE_SHIFT)
+#define GITS_BASER_PAGE_SIZE_64K (2ULL << GITS_BASER_PAGE_SIZE_SHIFT)
+#define GITS_BASER_PAGE_SIZE_MASK (3ULL << GITS_BASER_PAGE_SIZE_SHIFT)
#define GITS_BASER_PAGES_MAX 256
#define GITS_BASER_PAGES_SHIFT (0)
#define GITS_BASER_NR_PAGES(r) (((r) & 0xff) + 1)
diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
index 4660a7d..8c2b3cd 100644
--- a/virt/kvm/arm/vgic/vgic-its.c
+++ b/virt/kvm/arm/vgic/vgic-its.c
@@ -632,21 +632,22 @@ static bool vgic_its_check_id(struct vgic_its *its, u64 baser, int id)
int index;
u64 indirect_ptr;
gfn_t gfn;
+ int esz = GITS_BASER_ENTRY_SIZE(baser);
if (!(baser & GITS_BASER_INDIRECT)) {
phys_addr_t addr;
- if (id >= (l1_tbl_size / GITS_BASER_ENTRY_SIZE(baser)))
+ if (id >= (l1_tbl_size / esz))
return false;
- addr = BASER_ADDRESS(baser) + id * GITS_BASER_ENTRY_SIZE(baser);
+ addr = BASER_ADDRESS(baser) + id * esz;
gfn = addr >> PAGE_SHIFT;
return kvm_is_visible_gfn(its->dev->kvm, gfn);
}
/* calculate and check the index into the 1st level */
- index = id / (SZ_64K / GITS_BASER_ENTRY_SIZE(baser));
+ index = id / (SZ_64K / esz);
if (index >= (l1_tbl_size / sizeof(u64)))
return false;
@@ -670,8 +671,8 @@ static bool vgic_its_check_id(struct vgic_its *its, u64 baser, int id)
indirect_ptr &= GENMASK_ULL(51, 16);
/* Find the address of the actual entry */
- index = id % (SZ_64K / GITS_BASER_ENTRY_SIZE(baser));
- indirect_ptr += index * GITS_BASER_ENTRY_SIZE(baser);
+ index = id % (SZ_64K / esz);
+ indirect_ptr += index * esz;
gfn = indirect_ptr >> PAGE_SHIFT;
return kvm_is_visible_gfn(its->dev->kvm, gfn);
--
2.1.4
^ permalink raw reply related
* [PATCH 2/6] ARM: KVM: Support vGICv3 ITS
From: Marc Zyngier @ 2016-12-09 15:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481298811-30798-1-git-send-email-marc.zyngier@arm.com>
From: Vladimir Murzin <vladimir.murzin@arm.com>
This patch allows to build and use vGICv3 ITS in 32-bit mode.
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
Documentation/virtual/kvm/api.txt | 2 +-
arch/arm/include/uapi/asm/kvm.h | 2 ++
arch/arm/kvm/Kconfig | 1 +
arch/arm/kvm/Makefile | 1 +
arch/arm/kvm/arm.c | 6 ++++++
arch/arm64/kvm/Kconfig | 4 ----
arch/arm64/kvm/reset.c | 6 ------
virt/kvm/arm/vgic/vgic-kvm-device.c | 2 --
virt/kvm/arm/vgic/vgic-mmio-v3.c | 2 --
virt/kvm/arm/vgic/vgic.h | 26 --------------------------
10 files changed, 11 insertions(+), 41 deletions(-)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 739db9a..2feeae6 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2198,7 +2198,7 @@ after pausing the vcpu, but before it is resumed.
4.71 KVM_SIGNAL_MSI
Capability: KVM_CAP_SIGNAL_MSI
-Architectures: x86 arm64
+Architectures: x86 arm arm64
Type: vm ioctl
Parameters: struct kvm_msi (in)
Returns: >0 on delivery, 0 if guest blocked the MSI, and -1 on error
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index b38c10c..af05f8e 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -87,9 +87,11 @@ struct kvm_regs {
/* Supported VGICv3 address types */
#define KVM_VGIC_V3_ADDR_TYPE_DIST 2
#define KVM_VGIC_V3_ADDR_TYPE_REDIST 3
+#define KVM_VGIC_ITS_ADDR_TYPE 4
#define KVM_VGIC_V3_DIST_SIZE SZ_64K
#define KVM_VGIC_V3_REDIST_SIZE (2 * SZ_64K)
+#define KVM_VGIC_V3_ITS_SIZE (2 * SZ_64K)
#define KVM_ARM_VCPU_POWER_OFF 0 /* CPU is started in OFF state */
#define KVM_ARM_VCPU_PSCI_0_2 1 /* CPU uses PSCI v0.2 */
diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
index 3e1cd04..90d0176 100644
--- a/arch/arm/kvm/Kconfig
+++ b/arch/arm/kvm/Kconfig
@@ -34,6 +34,7 @@ config KVM
select HAVE_KVM_IRQFD
select HAVE_KVM_IRQCHIP
select HAVE_KVM_IRQ_ROUTING
+ select HAVE_KVM_MSI
depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
---help---
Support hosting virtualized guest machines.
diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile
index f19842e..d571243 100644
--- a/arch/arm/kvm/Makefile
+++ b/arch/arm/kvm/Makefile
@@ -32,5 +32,6 @@ obj-y += $(KVM)/arm/vgic/vgic-mmio.o
obj-y += $(KVM)/arm/vgic/vgic-mmio-v2.o
obj-y += $(KVM)/arm/vgic/vgic-mmio-v3.o
obj-y += $(KVM)/arm/vgic/vgic-kvm-device.o
+obj-y += $(KVM)/arm/vgic/vgic-its.o
obj-y += $(KVM)/irqchip.o
obj-y += $(KVM)/arm/arch_timer.o
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 19b5f5c..8f92efa 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -221,6 +221,12 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_MAX_VCPUS:
r = KVM_MAX_VCPUS;
break;
+ case KVM_CAP_MSI_DEVID:
+ if (!kvm)
+ r = -EINVAL;
+ else
+ r = kvm->arch.vgic.msis_require_devid;
+ break;
default:
r = kvm_arch_dev_ioctl_check_extension(kvm, ext);
break;
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index 6eaf12c..52cb7ad 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -16,9 +16,6 @@ menuconfig VIRTUALIZATION
if VIRTUALIZATION
-config KVM_ARM_VGIC_V3_ITS
- bool
-
config KVM
bool "Kernel-based Virtual Machine (KVM) support"
depends on OF
@@ -34,7 +31,6 @@ config KVM
select KVM_VFIO
select HAVE_KVM_EVENTFD
select HAVE_KVM_IRQFD
- select KVM_ARM_VGIC_V3_ITS
select KVM_ARM_PMU if HW_PERF_EVENTS
select HAVE_KVM_MSI
select HAVE_KVM_IRQCHIP
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 5bc4608..e95d4f6 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -86,12 +86,6 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_VCPU_ATTRIBUTES:
r = 1;
break;
- case KVM_CAP_MSI_DEVID:
- if (!kvm)
- r = -EINVAL;
- else
- r = kvm->arch.vgic.msis_require_devid;
- break;
default:
r = 0;
}
diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
index ce1f4ed..fbe87a6 100644
--- a/virt/kvm/arm/vgic/vgic-kvm-device.c
+++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
@@ -221,11 +221,9 @@ int kvm_register_vgic_device(unsigned long type)
ret = kvm_register_device_ops(&kvm_arm_vgic_v3_ops,
KVM_DEV_TYPE_ARM_VGIC_V3);
-#ifdef CONFIG_KVM_ARM_VGIC_V3_ITS
if (ret)
break;
ret = kvm_vgic_register_its_device();
-#endif
break;
}
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index 0d3c76a..50f42f0 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -42,7 +42,6 @@ u64 update_64bit_reg(u64 reg, unsigned int offset, unsigned int len,
return reg | ((u64)val << lower);
}
-#ifdef CONFIG_KVM_ARM_VGIC_V3_ITS
bool vgic_has_its(struct kvm *kvm)
{
struct vgic_dist *dist = &kvm->arch.vgic;
@@ -52,7 +51,6 @@ bool vgic_has_its(struct kvm *kvm)
return dist->has_its;
}
-#endif
static unsigned long vgic_mmio_read_v3_misc(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len)
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index 9d9e014..859f65c 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -84,37 +84,11 @@ int vgic_v3_probe(const struct gic_kvm_info *info);
int vgic_v3_map_resources(struct kvm *kvm);
int vgic_register_redist_iodevs(struct kvm *kvm, gpa_t dist_base_address);
-#ifdef CONFIG_KVM_ARM_VGIC_V3_ITS
int vgic_register_its_iodevs(struct kvm *kvm);
bool vgic_has_its(struct kvm *kvm);
int kvm_vgic_register_its_device(void);
void vgic_enable_lpis(struct kvm_vcpu *vcpu);
int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi);
-#else
-static inline int vgic_register_its_iodevs(struct kvm *kvm)
-{
- return -ENODEV;
-}
-
-static inline bool vgic_has_its(struct kvm *kvm)
-{
- return false;
-}
-
-static inline int kvm_vgic_register_its_device(void)
-{
- return -ENODEV;
-}
-
-static inline void vgic_enable_lpis(struct kvm_vcpu *vcpu)
-{
-}
-
-static inline int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi)
-{
- return -ENODEV;
-}
-#endif
int kvm_register_vgic_device(unsigned long type);
int vgic_lazy_init(struct kvm *kvm);
--
2.1.4
^ permalink raw reply related
* [PATCH 3/6] arm/arm64: KVM: Clean up useless code in kvm_timer_enable
From: Marc Zyngier @ 2016-12-09 15:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481298811-30798-1-git-send-email-marc.zyngier@arm.com>
From: "Longpeng(Mike)" <longpeng2@huawei.com>
1) Since commit:41a54482 changed timer enabled variable to per-vcpu,
the correlative comment in kvm_timer_enable is useless now.
2) After the kvm module init successfully, the timecounter is always
non-null, so we can remove the checking of timercounter.
Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
virt/kvm/arm/arch_timer.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 27a1f63..17b8fa5 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -498,17 +498,7 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
if (ret)
return ret;
-
- /*
- * There is a potential race here between VCPUs starting for the first
- * time, which may be enabling the timer multiple times. That doesn't
- * hurt though, because we're just setting a variable to the same
- * variable that it already was. The important thing is that all
- * VCPUs have the enabled variable set, before entering the guest, if
- * the arch timers are enabled.
- */
- if (timecounter)
- timer->enabled = 1;
+ timer->enabled = 1;
return 0;
}
--
2.1.4
^ permalink raw reply related
* [PATCH 4/6] KVM: arm/arm64: vgic-v2: Limit ITARGETSR bits to number of VCPUs
From: Marc Zyngier @ 2016-12-09 15:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481298811-30798-1-git-send-email-marc.zyngier@arm.com>
From: Andre Przywara <andre.przywara@arm.com>
The GICv2 spec says in section 4.3.12 that a "CPU targets field bit that
corresponds to an unimplemented CPU interface is RAZ/WI."
Currently we allow the guest to write any value in there and it can
read that back.
Mask the written value with the proper CPU mask to be spec compliant.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
virt/kvm/arm/vgic/vgic-mmio-v2.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c
index b44b359..78e34bc 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v2.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c
@@ -129,6 +129,7 @@ static void vgic_mmio_write_target(struct kvm_vcpu *vcpu,
unsigned long val)
{
u32 intid = VGIC_ADDR_TO_INTID(addr, 8);
+ u8 cpu_mask = GENMASK(atomic_read(&vcpu->kvm->online_vcpus) - 1, 0);
int i;
/* GICD_ITARGETSR[0-7] are read-only */
@@ -141,7 +142,7 @@ static void vgic_mmio_write_target(struct kvm_vcpu *vcpu,
spin_lock(&irq->irq_lock);
- irq->targets = (val >> (i * 8)) & 0xff;
+ irq->targets = (val >> (i * 8)) & cpu_mask;
target = irq->targets ? __ffs(irq->targets) : 0;
irq->target_vcpu = kvm_get_vcpu(vcpu->kvm, target);
--
2.1.4
^ permalink raw reply related
* [PATCH 5/6] KVM: arm/arm64: timer: Check for properly initialized timer on init
From: Marc Zyngier @ 2016-12-09 15:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481298811-30798-1-git-send-email-marc.zyngier@arm.com>
From: Christoffer Dall <christoffer.dall@linaro.org>
When the arch timer code fails to initialize (for example because the
memory mapped timer doesn't work, which is currently seen with the AEM
model), then KVM just continues happily with a final result that KVM
eventually does a NULL pointer dereference of the uninitialized cycle
counter.
Check directly for this in the init path and give the user a reasonable
error in this case.
Cc: Shih-Wei Li <shihwei@cs.columbia.edu>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
virt/kvm/arm/arch_timer.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 17b8fa5..ae95fc0 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -425,6 +425,11 @@ int kvm_timer_hyp_init(void)
info = arch_timer_get_kvm_info();
timecounter = &info->timecounter;
+ if (!timecounter->cc) {
+ kvm_err("kvm_arch_timer: uninitialized timecounter\n");
+ return -ENODEV;
+ }
+
if (info->virtual_irq <= 0) {
kvm_err("kvm_arch_timer: invalid virtual timer IRQ: %d\n",
info->virtual_irq);
--
2.1.4
^ permalink raw reply related
* [PATCH 6/6] arm64: KVM: pmu: Reset PMSELR_EL0.SEL to a sane value before entering the guest
From: Marc Zyngier @ 2016-12-09 15:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481298811-30798-1-git-send-email-marc.zyngier@arm.com>
The ARMv8 architecture allows the cycle counter to be configured
by setting PMSELR_EL0.SEL==0x1f and then accessing PMXEVTYPER_EL0,
hence accessing PMCCFILTR_EL0. But it disallows the use of
PMSELR_EL0.SEL==0x1f to access the cycle counter itself through
PMXEVCNTR_EL0.
Linux itself doesn't violate this rule, but we may end up with
PMSELR_EL0.SEL being set to 0x1f when we enter a guest. If that
guest accesses PMXEVCNTR_EL0, the access may UNDEF at EL1,
despite the guest not having done anything wrong.
In order to avoid this unfortunate course of events (haha!), let's
sanitize PMSELR_EL0 on guest entry. This ensures that the guest
won't explode unexpectedly.
Cc: stable at vger.kernel.org #4.6+
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/kvm/hyp/switch.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 83037cd..0c848c1 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -85,7 +85,13 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
write_sysreg(val, hcr_el2);
/* Trap on AArch32 cp15 c15 accesses (EL1 or EL0) */
write_sysreg(1 << 15, hstr_el2);
- /* Make sure we trap PMU access from EL0 to EL2 */
+ /*
+ * Make sure we trap PMU access from EL0 to EL2. Also sanitize
+ * PMSELR_EL0 to make sure it never contains the cycle
+ * counter, which could make a PMXEVCNTR_EL0 access UNDEF at
+ * EL1 instead of being trapped to EL2.
+ */
+ write_sysreg(0, pmselr_el0);
write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0);
write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
__activate_traps_arch()();
--
2.1.4
^ permalink raw reply related
* [PATCH 1/3] rtc: armada38x: improve RTC errata implementation
From: Gregory CLEMENT @ 2016-12-09 16:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161208172923.GQ26852@lunn.ch>
Hi Andrew,
On jeu., d?c. 08 2016, Andrew Lunn <andrew@lunn.ch> wrote:
>> +struct str_value_to_freq {
>> + unsigned long value;
>> + u8 freq;
>> +} __packed;
>> +
>> +static unsigned long read_rtc_register_wa(struct armada38x_rtc *rtc, u8 rtc_reg)
>> +{
>> + unsigned long value_array[SAMPLE_NR], i, j, value;
>> + unsigned long max = 0, index_max = SAMPLE_NR - 1;
>> + struct str_value_to_freq value_to_freq[SAMPLE_NR];
>
> Hi Gregory
>
> This appears to be putting over 900 bytes on the stack. Is there any
Actually the structure being packed it is 500 bytes.
> danger of overflowing the stack? Would it be safer to make these
> arrays part of armada38x_rtc?
We could do this if you fear a stack overflow.
Gregory
>
> Andrew
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH] staging: vc04_services: Fix NULL ptr sparse warnings
From: Mike Kofron @ 2016-12-09 16:21 UTC (permalink / raw)
To: linux-arm-kernel
In calls to queue_message() in vchiq_core.c, the "void *context"
parameter is set as 0 rather than NULL. This patch amends each call to
use the proper NULL pointer. The following sparse warnings are fixed:
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:1623:23: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:1976:47: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:2075:47: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:2095:47: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:2907:39: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:2929:39: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:3059:72: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:3860:31: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:3870:31: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:3880:31: warning: Using plain integer as NULL pointer
Signed-off-by: Mike Kofron <mpkofron@gmail.com>
---
.../vc04_services/interface/vchiq_arm/vchiq_core.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 028e90b..32b00f8 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -1620,7 +1620,7 @@ parse_open(VCHIQ_STATE_T *state, VCHIQ_HEADER_T *header)
/* No available service, or an invalid request - send a CLOSE */
if (queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_CLOSE, 0, VCHIQ_MSG_SRCPORT(msgid)),
- NULL, 0, 0, 0) == VCHIQ_RETRY)
+ NULL, NULL, 0, 0) == VCHIQ_RETRY)
goto bail_not_ready;
return 1;
@@ -1973,7 +1973,7 @@ parse_rx_slots(VCHIQ_STATE_T *state)
/* Send a PAUSE in response */
if (queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_PAUSE, 0, 0),
- NULL, 0, 0, QMFLAGS_NO_MUTEX_UNLOCK)
+ NULL, NULL, 0, QMFLAGS_NO_MUTEX_UNLOCK)
== VCHIQ_RETRY)
goto bail_not_ready;
if (state->is_master)
@@ -2072,7 +2072,7 @@ slot_handler_func(void *v)
pause_bulks(state);
if (queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_PAUSE, 0, 0),
- NULL, 0, 0,
+ NULL, NULL, 0,
QMFLAGS_NO_MUTEX_UNLOCK)
!= VCHIQ_RETRY) {
vchiq_set_conn_state(state,
@@ -2092,7 +2092,7 @@ slot_handler_func(void *v)
case VCHIQ_CONNSTATE_RESUMING:
if (queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_RESUME, 0, 0),
- NULL, 0, 0, QMFLAGS_NO_MUTEX_LOCK)
+ NULL, NULL, 0, QMFLAGS_NO_MUTEX_LOCK)
!= VCHIQ_RETRY) {
if (state->is_master)
resume_bulks(state);
@@ -2904,7 +2904,7 @@ vchiq_close_service_internal(VCHIQ_SERVICE_T *service, int close_recvd)
(VCHIQ_MSG_CLOSE,
service->localport,
VCHIQ_MSG_DSTPORT(service->remoteport)),
- NULL, 0, 0, 0);
+ NULL, NULL, 0, 0);
}
break;
@@ -2926,7 +2926,7 @@ vchiq_close_service_internal(VCHIQ_SERVICE_T *service, int close_recvd)
(VCHIQ_MSG_CLOSE,
service->localport,
VCHIQ_MSG_DSTPORT(service->remoteport)),
- NULL, 0, 0, QMFLAGS_NO_MUTEX_UNLOCK);
+ NULL, NULL, 0, QMFLAGS_NO_MUTEX_UNLOCK);
if (status == VCHIQ_SUCCESS) {
if (!close_recvd) {
@@ -3056,7 +3056,7 @@ vchiq_connect_internal(VCHIQ_STATE_T *state, VCHIQ_INSTANCE_T instance)
if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED) {
if (queue_message(state, NULL,
- VCHIQ_MAKE_MSG(VCHIQ_MSG_CONNECT, 0, 0), NULL, 0,
+ VCHIQ_MAKE_MSG(VCHIQ_MSG_CONNECT, 0, 0), NULL, NULL,
0, QMFLAGS_IS_BLOCKING) == VCHIQ_RETRY)
return VCHIQ_RETRY;
@@ -3857,7 +3857,7 @@ VCHIQ_STATUS_T vchiq_send_remote_use(VCHIQ_STATE_T *state)
if (state->conn_state != VCHIQ_CONNSTATE_DISCONNECTED)
status = queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_REMOTE_USE, 0, 0),
- NULL, 0, 0, 0);
+ NULL, NULL, 0, 0);
return status;
}
@@ -3867,7 +3867,7 @@ VCHIQ_STATUS_T vchiq_send_remote_release(VCHIQ_STATE_T *state)
if (state->conn_state != VCHIQ_CONNSTATE_DISCONNECTED)
status = queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_REMOTE_RELEASE, 0, 0),
- NULL, 0, 0, 0);
+ NULL, NULL, 0, 0);
return status;
}
@@ -3877,7 +3877,7 @@ VCHIQ_STATUS_T vchiq_send_remote_use_active(VCHIQ_STATE_T *state)
if (state->conn_state != VCHIQ_CONNSTATE_DISCONNECTED)
status = queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_REMOTE_USE_ACTIVE, 0, 0),
- NULL, 0, 0, 0);
+ NULL, NULL, 0, 0);
return status;
}
--
2.9.3
^ permalink raw reply related
* [PATCH 1/3] rtc: armada38x: improve RTC errata implementation
From: Andrew Lunn @ 2016-12-09 16:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8760mtf538.fsf@free-electrons.com>
On Fri, Dec 09, 2016 at 05:19:07PM +0100, Gregory CLEMENT wrote:
> Hi Andrew,
>
> On jeu., d?c. 08 2016, Andrew Lunn <andrew@lunn.ch> wrote:
>
> >> +struct str_value_to_freq {
> >> + unsigned long value;
> >> + u8 freq;
> >> +} __packed;
> >> +
> >> +static unsigned long read_rtc_register_wa(struct armada38x_rtc *rtc, u8 rtc_reg)
> >> +{
> >> + unsigned long value_array[SAMPLE_NR], i, j, value;
> >> + unsigned long max = 0, index_max = SAMPLE_NR - 1;
> >> + struct str_value_to_freq value_to_freq[SAMPLE_NR];
> >
> > Hi Gregory
> >
> > This appears to be putting over 900 bytes on the stack. Is there any
>
> Actually the structure being packed it is 500 bytes.
Did you verify this? I never remember where the __packed needs to go.
You clearly have a packed structure, but is the array of structures
packed?
And the long value_array[SAMPLE_NR] is another 400 bytes, totalling
900. And as Russell pointed out, this is on 32 bit systems. Until your
third patch, 64 bit systems probably have double that. I would also
suggest squashing patch #3 into #1.
> > danger of overflowing the stack? Would it be safer to make these
> > arrays part of armada38x_rtc?
>
> We could do this if you fear a stack overflow.
It is generally consider not a good idea to put > $BIG structures on
the stack, but the value of $BIG is not clearly defined. Stack
overflow seems to be an issue with lots of layering going on, swap on
NFS etc. But it seems unlikely to me reading the RTC will happen with
an already deep stack. So this is probably O.K.
Andrew
^ permalink raw reply
* [PATCH 1/3] rtc: armada38x: improve RTC errata implementation
From: Gregory CLEMENT @ 2016-12-09 16:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161208173717.GJ14217@n2100.armlinux.org.uk>
Hi Russell King,
On jeu., d?c. 08 2016, Russell King - ARM Linux <linux@armlinux.org.uk> wrote:
> On Thu, Dec 08, 2016 at 06:10:08PM +0100, Gregory CLEMENT wrote:
>> From: Shaker Daibes <shaker@marvell.com>
>>
>> According to FE-3124064:
>> The device supports CPU write and read access to the RTC time register.
>> However, due to this errata, read from RTC TIME register may fail.
>>
>> Workaround:
>> General configuration:
>> 1. Configure the RTC Mbus Bridge Timing Control register (offset 0x184A0)
>> to value 0xFD4D4FFF
>> Write RTC WRCLK Period to its maximum value (0x3FF)
>> Write RTC WRCLK setup to 0x53 (default value )
>> Write RTC WRCLK High Time to 0x53 (default value )
>> Write RTC Read Output Delay to its maximum value (0x1F)
>> Mbus - Read All Byte Enable to 0x1 (default value )
>> 2. Configure the RTC Test Configuration Register (offset 0xA381C) bit3
>> to '1' (Reserved, Marvell internal)
>>
>> For any RTC register read operation:
>> 1. Read the requested register 100 times.
>> 2. Find the result that appears most frequently and use this result
>> as the correct value.
>>
>> For any RTC register write operation:
>> 1. Issue two dummy writes of 0x0 to the RTC Status register (offset
>> 0xA3800).
>> 2. Write the time to the RTC Time register (offset 0xA380C).
>>
>> [gregory.clement at free-electrons.com: cosmetic changes and fix issues for
>> interrupt in original patch]
>
> A particularly interesting question here is whether the above description
> came first or whether the code below came first, and which was derived
> from which.
Well, it is difficult to say. the above description comes from the
errata datasheet (since rev B). But the errata sheet itself mentioned
the software implementation in one of the Marvell release.
>
> Given that both readl() writel() involves a barrier operation, which is
> not mentioned in the above workaround text, is it appropriate to use the
> barriered operations?
Do you suggest to use the the relaxed version of readl() and writel()?
>
>>
>> Reviewed-by: Lior Amsalem <alior@marvell.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>> drivers/rtc/rtc-armada38x.c | 109 ++++++++++++++++++++++++++++++++++----------
>> 1 file changed, 85 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/rtc/rtc-armada38x.c b/drivers/rtc/rtc-armada38x.c
>> index 9a3f2a6f512e..a0859286a4c4 100644
>> --- a/drivers/rtc/rtc-armada38x.c
>> +++ b/drivers/rtc/rtc-armada38x.c
>> @@ -29,12 +29,21 @@
>> #define RTC_TIME 0xC
>> #define RTC_ALARM1 0x10
>>
>> +#define SOC_RTC_BRIDGE_TIMING_CTL 0x0
>> +#define SOC_RTC_PERIOD_OFFS 0
>> +#define SOC_RTC_PERIOD_MASK (0x3FF << SOC_RTC_PERIOD_OFFS)
>> +#define SOC_RTC_READ_DELAY_OFFS 26
>> +#define SOC_RTC_READ_DELAY_MASK (0x1F << SOC_RTC_READ_DELAY_OFFS)
>> +
>> #define SOC_RTC_INTERRUPT 0x8
>> #define SOC_RTC_ALARM1 BIT(0)
>> #define SOC_RTC_ALARM2 BIT(1)
>> #define SOC_RTC_ALARM1_MASK BIT(2)
>> #define SOC_RTC_ALARM2_MASK BIT(3)
>>
>> +
>> +#define SAMPLE_NR 100
>> +
>> struct armada38x_rtc {
>> struct rtc_device *rtc_dev;
>> void __iomem *regs;
>> @@ -47,32 +56,85 @@ struct armada38x_rtc {
>> * According to the datasheet, the OS should wait 5us after every
>> * register write to the RTC hard macro so that the required update
>> * can occur without holding off the system bus
>> + * According to errata FE-3124064, Write to any RTC register
>> + * may fail. As a workaround, before writing to RTC
>> + * register, issue a dummy write of 0x0 twice to RTC Status
>> + * register.
>> */
>> +
>> static void rtc_delayed_write(u32 val, struct armada38x_rtc *rtc, int offset)
>> {
>> + writel(0, rtc->regs + RTC_STATUS);
>> + writel(0, rtc->regs + RTC_STATUS);
>> writel(val, rtc->regs + offset);
>> udelay(5);
>> }
>>
>> +/* Update RTC-MBUS bridge timing parameters */
>> +static void rtc_update_mbus_timing_params(struct armada38x_rtc *rtc)
>> +{
>> + uint32_t reg;
>> +
>> + reg = readl(rtc->regs_soc + SOC_RTC_BRIDGE_TIMING_CTL);
>> + reg &= ~SOC_RTC_PERIOD_MASK;
>> + reg |= 0x3FF << SOC_RTC_PERIOD_OFFS; /* Maximum value */
>> + reg &= ~SOC_RTC_READ_DELAY_MASK;
>> + reg |= 0x1F << SOC_RTC_READ_DELAY_OFFS; /* Maximum value */
>> + writel(reg, rtc->regs_soc + SOC_RTC_BRIDGE_TIMING_CTL);
>> +}
>> +
>> +struct str_value_to_freq {
>> + unsigned long value;
>> + u8 freq;
>> +} __packed;
>
> Why packed? This makes accesses to this structure very inefficient -
> the compiler for some CPUs will load "value" by bytes.
As pointed by Andrew, I think the intent was to reduce the amount of
memory used from the stack.
Thanks,
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH v4] arm64: fpsimd: improve stacking logic in non-interruptible context
From: Ard Biesheuvel @ 2016-12-09 16:46 UTC (permalink / raw)
To: linux-arm-kernel
Currently, we allow kernel mode NEON in softirq or hardirq context by
stacking and unstacking a slice of the NEON register file for each call
to kernel_neon_begin() and kernel_neon_end(), respectively.
Given that
a) a CPU typically spends most of its time in userland, during which time
no kernel mode NEON in process context is in progress,
b) a CPU spends most of its time in the kernel doing other things than
kernel mode NEON when it gets interrupted to perform kernel mode NEON
in softirq context
the stacking and subsequent unstacking is only necessary if we are
interrupting a thread while it is performing kernel mode NEON in process
context, which means that in all other cases, we can simply preserve the
userland FPSIMD state once, and only restore it upon return to userland,
even if we are being invoked from softirq or hardirq context.
So instead of checking whether we are running in interrupt context, keep
track of the level of nested kernel mode NEON calls in progress, and only
perform the eager stack/unstack if the level exceeds 1.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v4:
- use this_cpu_inc/dec, which give sufficient guarantees regarding
concurrency, but do not imply SMP barriers, which are not needed here
v3:
- avoid corruption by concurrent invocations of kernel_neon_begin()/_end()
v2:
- BUG() on unexpected values of the nesting level
- relax the BUG() on num_regs>32 to a WARN, given that nothing actually
breaks in that case
arch/arm64/kernel/fpsimd.c | 47 ++++++++++++++------
1 file changed, 33 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 394c61db5566..37d6dfc9059b 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -220,20 +220,35 @@ void fpsimd_flush_task_state(struct task_struct *t)
#ifdef CONFIG_KERNEL_MODE_NEON
-static DEFINE_PER_CPU(struct fpsimd_partial_state, hardirq_fpsimdstate);
-static DEFINE_PER_CPU(struct fpsimd_partial_state, softirq_fpsimdstate);
+/*
+ * Although unlikely, it is possible for three kernel mode NEON contexts to
+ * be live at the same time: process context, softirq context and hardirq
+ * context. So while the userland context is stashed in the thread's fpsimd
+ * state structure, we need two additional levels of storage.
+ */
+static DEFINE_PER_CPU(struct fpsimd_partial_state, nested_fpsimdstate[2]);
+static DEFINE_PER_CPU(int, kernel_neon_nesting_level);
/*
* Kernel-side NEON support functions
*/
void kernel_neon_begin_partial(u32 num_regs)
{
- if (in_interrupt()) {
- struct fpsimd_partial_state *s = this_cpu_ptr(
- in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
+ struct fpsimd_partial_state *s;
+ int level;
+
+ preempt_disable();
+
+ level = this_cpu_inc_return(kernel_neon_nesting_level);
+ BUG_ON(level > 3);
+
+ if (level > 1) {
+ s = this_cpu_ptr(nested_fpsimdstate);
- BUG_ON(num_regs > 32);
- fpsimd_save_partial_state(s, roundup(num_regs, 2));
+ WARN_ON_ONCE(num_regs > 32);
+ num_regs = min(roundup(num_regs, 2), 32U);
+
+ fpsimd_save_partial_state(&s[level - 2], num_regs);
} else {
/*
* Save the userland FPSIMD state if we have one and if we
@@ -241,7 +256,6 @@ void kernel_neon_begin_partial(u32 num_regs)
* that there is no longer userland FPSIMD state in the
* registers.
*/
- preempt_disable();
if (current->mm &&
!test_and_set_thread_flag(TIF_FOREIGN_FPSTATE))
fpsimd_save_state(¤t->thread.fpsimd_state);
@@ -252,13 +266,18 @@ EXPORT_SYMBOL(kernel_neon_begin_partial);
void kernel_neon_end(void)
{
- if (in_interrupt()) {
- struct fpsimd_partial_state *s = this_cpu_ptr(
- in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
- fpsimd_load_partial_state(s);
- } else {
- preempt_enable();
+ struct fpsimd_partial_state *s;
+ int level;
+
+ level = this_cpu_read(kernel_neon_nesting_level);
+ BUG_ON(level < 1);
+
+ if (level > 1) {
+ s = this_cpu_ptr(nested_fpsimdstate);
+ fpsimd_load_partial_state(&s[level - 2]);
}
+ this_cpu_dec(kernel_neon_nesting_level);
+ preempt_enable();
}
EXPORT_SYMBOL(kernel_neon_end);
--
2.7.4
^ permalink raw reply related
* [PATCH v7 1/2] provide lock-less versions of clk_{enable|disable}
From: Alexandre Bailon @ 2016-12-09 16:59 UTC (permalink / raw)
To: linux-arm-kernel
Rename __clk_{enable|disble} in davinci_clk_{enable|disable}.
davinci_clk_{enable|disable} is a lock-less version of
clk_{enable|disable}.
This is useful to recursively enable clock without doing recursive call
to clk_enable(), which would cause a recursive locking issue.
The lock-less version could be used by example by the usb20 phy clock,
that need to enable the usb20 clock before to start.
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Suggested-by: David Lechner <david@lechnology.com>
---
arch/arm/mach-davinci/clock.c | 12 ++++++------
arch/arm/mach-davinci/clock.h | 2 ++
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c
index df42c93..f5dce9b 100644
--- a/arch/arm/mach-davinci/clock.c
+++ b/arch/arm/mach-davinci/clock.c
@@ -31,10 +31,10 @@ static LIST_HEAD(clocks);
static DEFINE_MUTEX(clocks_mutex);
static DEFINE_SPINLOCK(clockfw_lock);
-static void __clk_enable(struct clk *clk)
+void davinci_clk_enable(struct clk *clk)
{
if (clk->parent)
- __clk_enable(clk->parent);
+ davinci_clk_enable(clk->parent);
if (clk->usecount++ == 0) {
if (clk->flags & CLK_PSC)
davinci_psc_config(clk->domain, clk->gpsc, clk->lpsc,
@@ -44,7 +44,7 @@ static void __clk_enable(struct clk *clk)
}
}
-static void __clk_disable(struct clk *clk)
+void davinci_clk_disable(struct clk *clk)
{
if (WARN_ON(clk->usecount == 0))
return;
@@ -56,7 +56,7 @@ static void __clk_disable(struct clk *clk)
clk->clk_disable(clk);
}
if (clk->parent)
- __clk_disable(clk->parent);
+ davinci_clk_disable(clk->parent);
}
int davinci_clk_reset(struct clk *clk, bool reset)
@@ -103,7 +103,7 @@ int clk_enable(struct clk *clk)
return -EINVAL;
spin_lock_irqsave(&clockfw_lock, flags);
- __clk_enable(clk);
+ davinci_clk_enable(clk);
spin_unlock_irqrestore(&clockfw_lock, flags);
return 0;
@@ -118,7 +118,7 @@ void clk_disable(struct clk *clk)
return;
spin_lock_irqsave(&clockfw_lock, flags);
- __clk_disable(clk);
+ davinci_clk_disable(clk);
spin_unlock_irqrestore(&clockfw_lock, flags);
}
EXPORT_SYMBOL(clk_disable);
diff --git a/arch/arm/mach-davinci/clock.h b/arch/arm/mach-davinci/clock.h
index e2a5437..fa2b837 100644
--- a/arch/arm/mach-davinci/clock.h
+++ b/arch/arm/mach-davinci/clock.h
@@ -132,6 +132,8 @@ int davinci_set_sysclk_rate(struct clk *clk, unsigned long rate);
int davinci_set_refclk_rate(unsigned long rate);
int davinci_simple_set_rate(struct clk *clk, unsigned long rate);
int davinci_clk_reset(struct clk *clk, bool reset);
+void davinci_clk_enable(struct clk *clk);
+void davinci_clk_disable(struct clk *clk);
extern struct platform_device davinci_wdt_device;
extern void davinci_watchdog_reset(struct platform_device *);
--
2.7.3
^ permalink raw reply related
* [PATCH v7 2/2] ARM: davinci: da8xx: Fix sleeping function called from invalid context
From: Alexandre Bailon @ 2016-12-09 16:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481302773-14181-1-git-send-email-abailon@baylibre.com>
Everytime the usb20 phy is enabled, there is a
"sleeping function called from invalid context" BUG.
In addition, there is a recursive locking happening
because of the recurse call to clk_enable().
clk_enable() from arch/arm/mach-davinci/clock.c uses spin_lock_irqsave()
before to invoke the callback usb20_phy_clk_enable().
usb20_phy_clk_enable() uses clk_get() and clk_enable_prepapre()
which may sleep.
replace clk_prepare_enable() by davinci_clk_enable().
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Suggested-by: David Lechner <david@lechnology.com>
---
arch/arm/mach-davinci/usb-da8xx.c | 34 +++++++++++++++-------------------
1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
index b010e5f..71f1e81 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -22,6 +22,8 @@
#define DA8XX_USB0_BASE 0x01e00000
#define DA8XX_USB1_BASE 0x01e25000
+static struct clk *usb20_clk;
+
static struct platform_device da8xx_usb_phy = {
.name = "da8xx-usb-phy",
.id = -1,
@@ -158,26 +160,13 @@ int __init da8xx_register_usb_refclkin(int rate)
static void usb20_phy_clk_enable(struct clk *clk)
{
- struct clk *usb20_clk;
- int err;
u32 val;
u32 timeout = 500000; /* 500 msec */
val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
- usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
- if (IS_ERR(usb20_clk)) {
- pr_err("could not get usb20 clk: %ld\n", PTR_ERR(usb20_clk));
- return;
- }
-
/* The USB 2.O PLL requires that the USB 2.O PSC is enabled as well. */
- err = clk_prepare_enable(usb20_clk);
- if (err) {
- pr_err("failed to enable usb20 clk: %d\n", err);
- clk_put(usb20_clk);
- return;
- }
+ davinci_clk_enable(usb20_clk);
/*
* Turn on the USB 2.0 PHY, but just the PLL, and not OTG. The USB 1.1
@@ -197,8 +186,7 @@ static void usb20_phy_clk_enable(struct clk *clk)
pr_err("Timeout waiting for USB 2.0 PHY clock good\n");
done:
- clk_disable_unprepare(usb20_clk);
- clk_put(usb20_clk);
+ davinci_clk_disable(usb20_clk);
}
static void usb20_phy_clk_disable(struct clk *clk)
@@ -285,11 +273,19 @@ static struct clk_lookup usb20_phy_clk_lookup =
int __init da8xx_register_usb20_phy_clk(bool use_usb_refclkin)
{
struct clk *parent;
- int ret = 0;
+ int ret;
+
+ usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
+ ret = PTR_ERR_OR_ZERO(usb20_clk);
+ if (ret)
+ return ret;
parent = clk_get(NULL, use_usb_refclkin ? "usb_refclkin" : "pll0_aux");
- if (IS_ERR(parent))
- return PTR_ERR(parent);
+ ret = PTR_ERR_OR_ZERO(parent);
+ if (ret) {
+ clk_put(usb20_clk);
+ return ret;
+ }
usb20_phy_clk.parent = parent;
ret = clk_register(&usb20_phy_clk);
--
2.7.3
^ permalink raw reply related
* [PATCH v8 04/16] MSI-X: update GSI routing after changed MSI-X configuration
From: Marc Zyngier @ 2016-12-09 17:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161104173203.21168-5-andre.przywara@arm.com>
On 04/11/16 17:31, Andre Przywara wrote:
> When we set up GSI routing to map MSIs to KVM's GSI numbers, we
> write the current device's MSI setup into the kernel routing table.
> However the device driver in the guest can use PCI configuration space
> accesses to change the MSI configuration (address and/or payload data).
> Whenever this happens after we have setup the routing table already,
> we must amend the previously sent data.
> So when MSI-X PCI config space accesses write address or payload,
> find the associated GSI number and the matching routing table entry
> and update the kernel routing table (only if the data has changed).
>
> This fixes vhost-net, where the queue's IRQFD was setup before the
> MSI vectors.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> include/kvm/irq.h | 1 +
> irq.c | 31 +++++++++++++++++++++++++++++++
> virtio/pci.c | 36 +++++++++++++++++++++++++++++++++---
> 3 files changed, 65 insertions(+), 3 deletions(-)
>
> diff --git a/include/kvm/irq.h b/include/kvm/irq.h
> index bb71521..f35eb7e 100644
> --- a/include/kvm/irq.h
> +++ b/include/kvm/irq.h
> @@ -21,5 +21,6 @@ int irq__exit(struct kvm *kvm);
>
> int irq__allocate_routing_entry(void);
> int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg);
> +void irq__update_msix_route(struct kvm *kvm, u32 gsi, struct msi_msg *msg);
>
> #endif
> diff --git a/irq.c b/irq.c
> index a742aa2..895e5eb 100644
> --- a/irq.c
> +++ b/irq.c
> @@ -93,6 +93,37 @@ int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
> return next_gsi++;
> }
>
> +static bool update_data(u32 *ptr, u32 newdata)
> +{
> + if (*ptr == newdata)
> + return false;
> +
> + *ptr = newdata;
> + return true;
> +}
> +
> +void irq__update_msix_route(struct kvm *kvm, u32 gsi, struct msi_msg *msg)
> +{
> + struct kvm_irq_routing_msi *entry;
> + unsigned int i;
> + bool changed;
> +
> + for (i = 0; i < irq_routing->nr; i++)
> + if (gsi == irq_routing->entries[i].gsi)
> + break;
> + if (i == irq_routing->nr)
> + return;
> +
> + entry = &irq_routing->entries[i].u.msi;
> +
> + changed = update_data(&entry->address_hi, msg->address_hi);
> + changed |= update_data(&entry->address_lo, msg->address_lo);
> + changed |= update_data(&entry->data, msg->data);
> +
> + if (changed)
> + ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, irq_routing);
Check the return value and let the caller know if something has failed?
> +}
> +
> int __attribute__((weak)) irq__exit(struct kvm *kvm)
> {
> free(irq_routing);
> diff --git a/virtio/pci.c b/virtio/pci.c
> index 072e5b7..b3b4aac 100644
> --- a/virtio/pci.c
> +++ b/virtio/pci.c
> @@ -152,6 +152,30 @@ static bool virtio_pci__io_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 p
> return ret;
> }
>
> +static void update_msix_map(struct virtio_pci *vpci,
> + struct msix_table *msix_entry, u32 vecnum)
> +{
> + u32 gsi, i;
> +
> + /* Find the GSI number used for that vector */
> + if (vecnum == vpci->config_vector) {
> + gsi = vpci->config_gsi;
> + } else {
> + for (i = 0; i < VIRTIO_PCI_MAX_VQ; i++)
> + if (vpci->vq_vector[i] == vecnum)
> + break;
> + if (i == VIRTIO_PCI_MAX_VQ)
> + return;
> + gsi = vpci->gsis[i];
> + }
> +
> + if (gsi == 0)
> + return;
> +
> + msix_entry = &msix_entry[vecnum];
> + irq__update_msix_route(vpci->kvm, gsi, &msix_entry->msg);
> +}
> +
> static bool virtio_pci__specific_io_out(struct kvm *kvm, struct virtio_device *vdev, u16 port,
> void *data, int size, int offset)
> {
> @@ -270,10 +294,16 @@ static void virtio_pci__msix_mmio_callback(struct kvm_cpu *vcpu,
> offset = vpci->msix_io_block;
> }
>
> - if (is_write)
> - memcpy(table + addr - offset, data, len);
> - else
> + if (!is_write) {
> memcpy(data, table + addr - offset, len);
> + return;
> + }
> +
> + memcpy(table + addr - offset, data, len);
> +
> + /* Did we just update the address or payload? */
> + if (addr % 0x10 < 0xc)
> + update_msix_map(vpci, table, (addr - offset) / 16);
Where are these constants coming from? Please stick to either decimal or
hex...
> }
>
> static void virtio_pci__signal_msi(struct kvm *kvm, struct virtio_pci *vpci, int vec)
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [BUG/RFC] efi/libstub: Preserve the memory map pointed to by the FDT
From: James Morse @ 2016-12-09 17:15 UTC (permalink / raw)
To: linux-arm-kernel
allocate_new_fdt_and_exit_boot() bakes a pointer to the UEFI memory map
into the FDT for later use. The corresponding memory is then free()d
and (hopefully) allocated again via efi_exit_boot_services() calling
efi_get_memory_map() as part of the exit_boot_services() loop. The
final copy is annotated with virtual mappings and used to create the
runtime map.
Linux expects the FDT to contain a pointer to the UEFI memory map
with the virtual mapping annotations, which will only happen if
AllocatePool() returns memory to the stub that was just FreePool()d
by the stub. This behaviour doesn't appear to guaranteed by the spec.
Platforms that don't do this will use the stale memory map (assuming
no later owner of the freed() memory changed it) and fail to initialise
runtime services.
Instead, keep the memory map we baked info the FDT, and create a new
'exit_map' pointer for use in the exit_boot_services() loop. (This was
already different, it just had the same name and we hoped it would be
in the same place).
Annotate the memory map pointed to by the FDT with virtual mappings.
This assumes the final memory map (which may be different), has
not moved any of the regions with the runtime attribute.
(Take the opportunity to remove some comments that are stale since
commit ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT"))
Signed-off-by: James Morse <james.morse@arm.com>
Fixes: ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT")
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
---
I haven't seen this causing problems on any real platforms, only kvmtool
which I'm busily hacking up to have primitive UEFI support. Needless to
say my AllocatePool() doesn't re-use memory.
drivers/firmware/efi/libstub/fdt.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index a6a93116a8f0..f623894c633c 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -181,10 +181,6 @@ static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
* which are fixed at 4K bytes, so in most cases the first
* allocation should succeed.
* EFI boot services are exited at the end of this function.
- * There must be no allocations between the get_memory_map()
- * call and the exit_boot_services() call, so the exiting of
- * boot services is very tightly tied to the creation of the FDT
- * with the final memory map in it.
*/
efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
@@ -199,7 +195,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
unsigned long map_size, desc_size, buff_size;
u32 desc_ver;
unsigned long mmap_key;
- efi_memory_desc_t *memory_map, *runtime_map;
+ efi_memory_desc_t *memory_map, *runtime_map, *exit_map;
unsigned long new_fdt_size;
efi_status_t status;
int runtime_entry_count = 0;
@@ -243,11 +239,6 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
goto fail;
}
- /*
- * Now that we have done our final memory allocation (and free)
- * we can get the memory map key needed for
- * exit_boot_services().
- */
status = efi_get_memory_map(sys_table, &map);
if (status != EFI_SUCCESS)
goto fail_free_new_fdt;
@@ -259,8 +250,16 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
memory_map, map_size, desc_size, desc_ver);
/* Succeeding the first time is the expected case. */
- if (status == EFI_SUCCESS)
+ if (status == EFI_SUCCESS) {
+ /*
+ * Annotate the memory_map in the FDT with virtual
+ * addresses. These shouldn't change as the placement
+ * of EFI_MEMORY_RUNTIME regions should be fixed.
+ */
+ efi_get_virtmap(memory_map, map_size, desc_size,
+ runtime_map, &runtime_entry_count);
break;
+ }
if (status == EFI_BUFFER_TOO_SMALL) {
/*
@@ -279,7 +278,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
}
}
- sys_table->boottime->free_pool(memory_map);
+ map.map = &exit_map;
priv.runtime_map = runtime_map;
priv.runtime_entry_count = &runtime_entry_count;
status = efi_exit_boot_services(sys_table, handle, &map, &priv,
--
2.10.1
^ permalink raw reply related
* Tearing down DMA transfer setup after DMA client has finished
From: Vinod Koul @ 2016-12-09 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6ce1ea97-1d68-2203-c7b4-73315e801655@laposte.net>
On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
>
> What concrete solution do you propose?
I have already proposed two solutions.
A) Request a channel only when you need it. Obviously we can't do virtual
channels with this (though we should still use virt-channels framework).
The sbox setup and teardown can be done as part of channel request and
freeup. PL08x already does this.
Downside is that we can only have as many consumers at a time as channels.
I have not heard any technical reason for not doing this apart from drivers
grab the channel at probe, which is incorrect and needs to be fixed
irrespective of the problem at hand.
This is my preferred option.
B) Create a custom driver specific API. This API for example:
sbox_setup(bool enable, ...)
can be called by client to explicitly setup and clear up the sbox setting.
This way we can have transactions muxed.
I have not heard any arguments on why we shouldn't do this except Russell's
comment that A) solves this.
> Alternatively, one can think of the current issue (i.e.: the fact that the IRQ
> arrives "too soon") in a different way.
> Instead of thinking the IRQ indicates "transfer complete", it is indicating "ready
> to accept another command", which in practice (and with proper API support) can
> translate into efficient queuing of DMA operations.
That IMO is a better understanding of this issue. But based on discussion, I
think the issue is that submitting next transaction cannot be done until
sbox is setup (as a consequence torn down for previous). Tear down can be
down only when client knows that transfer is done, from dma controller data
has been pushed and in-flight.
--
~Vinod
^ permalink raw reply
* [PATCH v4] arm64: fpsimd: improve stacking logic in non-interruptible context
From: Dave Martin @ 2016-12-09 17:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481301992-2344-1-git-send-email-ard.biesheuvel@linaro.org>
On Fri, Dec 09, 2016 at 04:46:32PM +0000, Ard Biesheuvel wrote:
> Currently, we allow kernel mode NEON in softirq or hardirq context by
> stacking and unstacking a slice of the NEON register file for each call
> to kernel_neon_begin() and kernel_neon_end(), respectively.
>
> Given that
> a) a CPU typically spends most of its time in userland, during which time
> no kernel mode NEON in process context is in progress,
> b) a CPU spends most of its time in the kernel doing other things than
> kernel mode NEON when it gets interrupted to perform kernel mode NEON
> in softirq context
>
> the stacking and subsequent unstacking is only necessary if we are
> interrupting a thread while it is performing kernel mode NEON in process
> context, which means that in all other cases, we can simply preserve the
> userland FPSIMD state once, and only restore it upon return to userland,
> even if we are being invoked from softirq or hardirq context.
>
> So instead of checking whether we are running in interrupt context, keep
> track of the level of nested kernel mode NEON calls in progress, and only
> perform the eager stack/unstack if the level exceeds 1.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
This looks good to me.
This should also make the SVE case trivial now: there can only be live
SVE state when in process context with !TIF_FOREIGN_FPSTATE, and the
SVE save/restore is then handled by fpsimd_{save,load}_state()
directly. For deeper nesting levels, there is already no live SVE
state, so kernel_neon_{save,load}_partial_state() are enough in that
case.
As and when KERNEL_MODE_SVE comes along this will need another look,
but this patch looks like a step forward for now.
Reviewed-by: Dave Martin <Dave.Martin@arm.com>
> ---
> v4:
> - use this_cpu_inc/dec, which give sufficient guarantees regarding
> concurrency, but do not imply SMP barriers, which are not needed here
>
> v3:
> - avoid corruption by concurrent invocations of kernel_neon_begin()/_end()
>
> v2:
> - BUG() on unexpected values of the nesting level
> - relax the BUG() on num_regs>32 to a WARN, given that nothing actually
> breaks in that case
>
> arch/arm64/kernel/fpsimd.c | 47 ++++++++++++++------
> 1 file changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 394c61db5566..37d6dfc9059b 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -220,20 +220,35 @@ void fpsimd_flush_task_state(struct task_struct *t)
>
> #ifdef CONFIG_KERNEL_MODE_NEON
>
> -static DEFINE_PER_CPU(struct fpsimd_partial_state, hardirq_fpsimdstate);
> -static DEFINE_PER_CPU(struct fpsimd_partial_state, softirq_fpsimdstate);
> +/*
> + * Although unlikely, it is possible for three kernel mode NEON contexts to
> + * be live at the same time: process context, softirq context and hardirq
> + * context. So while the userland context is stashed in the thread's fpsimd
> + * state structure, we need two additional levels of storage.
> + */
> +static DEFINE_PER_CPU(struct fpsimd_partial_state, nested_fpsimdstate[2]);
> +static DEFINE_PER_CPU(int, kernel_neon_nesting_level);
>
> /*
> * Kernel-side NEON support functions
> */
> void kernel_neon_begin_partial(u32 num_regs)
> {
> - if (in_interrupt()) {
> - struct fpsimd_partial_state *s = this_cpu_ptr(
> - in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
> + struct fpsimd_partial_state *s;
> + int level;
> +
> + preempt_disable();
> +
> + level = this_cpu_inc_return(kernel_neon_nesting_level);
> + BUG_ON(level > 3);
> +
> + if (level > 1) {
> + s = this_cpu_ptr(nested_fpsimdstate);
>
> - BUG_ON(num_regs > 32);
> - fpsimd_save_partial_state(s, roundup(num_regs, 2));
> + WARN_ON_ONCE(num_regs > 32);
> + num_regs = min(roundup(num_regs, 2), 32U);
> +
> + fpsimd_save_partial_state(&s[level - 2], num_regs);
> } else {
> /*
> * Save the userland FPSIMD state if we have one and if we
> @@ -241,7 +256,6 @@ void kernel_neon_begin_partial(u32 num_regs)
> * that there is no longer userland FPSIMD state in the
> * registers.
> */
> - preempt_disable();
> if (current->mm &&
> !test_and_set_thread_flag(TIF_FOREIGN_FPSTATE))
> fpsimd_save_state(¤t->thread.fpsimd_state);
> @@ -252,13 +266,18 @@ EXPORT_SYMBOL(kernel_neon_begin_partial);
>
> void kernel_neon_end(void)
> {
> - if (in_interrupt()) {
> - struct fpsimd_partial_state *s = this_cpu_ptr(
> - in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
> - fpsimd_load_partial_state(s);
> - } else {
> - preempt_enable();
> + struct fpsimd_partial_state *s;
> + int level;
> +
> + level = this_cpu_read(kernel_neon_nesting_level);
> + BUG_ON(level < 1);
> +
> + if (level > 1) {
> + s = this_cpu_ptr(nested_fpsimdstate);
> + fpsimd_load_partial_state(&s[level - 2]);
> }
> + this_cpu_dec(kernel_neon_nesting_level);
> + preempt_enable();
> }
> EXPORT_SYMBOL(kernel_neon_end);
>
> --
> 2.7.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Måns Rullgård @ 2016-12-09 17:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161209171727.GK6408@localhost>
Vinod Koul <vinod.koul@intel.com> writes:
> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
>>
>> What concrete solution do you propose?
>
> I have already proposed two solutions.
>
> A) Request a channel only when you need it. Obviously we can't do virtual
> channels with this (though we should still use virt-channels framework).
> The sbox setup and teardown can be done as part of channel request and
> freeup. PL08x already does this.
>
> Downside is that we can only have as many consumers at a time as channels.
>
> I have not heard any technical reason for not doing this apart from drivers
> grab the channel at probe, which is incorrect and needs to be fixed
> irrespective of the problem at hand.
>
> This is my preferred option.
Sorry, but this is not acceptable.
> B) Create a custom driver specific API. This API for example:
> sbox_setup(bool enable, ...)
> can be called by client to explicitly setup and clear up the sbox setting.
>
> This way we can have transactions muxed.
>
> I have not heard any arguments on why we shouldn't do this except Russell's
> comment that A) solves this.
Driver-specific interfaces are not the solution. That way lies chaos
and madness.
This would all be so much easier if you all would just shut up for a
moment and let me fix it properly.
--
M?ns Rullg?rd
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Mason @ 2016-12-09 17:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161209171727.GK6408@localhost>
On 09/12/2016 18:17, Vinod Koul wrote:
> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
>>
>> What concrete solution do you propose?
>
> I have already proposed two solutions.
>
> A) Request a channel only when you need it. Obviously we can't do virtual
> channels with this (though we should still use virt-channels framework).
> The sbox setup and teardown can be done as part of channel request and
> freeup. PL08x already does this.
>
> Downside is that we can only have as many consumers at a time as channels.
>
> I have not heard any technical reason for not doing this apart from drivers
> grab the channel at probe, which is incorrect and needs to be fixed
> irrespective of the problem at hand.
>
> This is my preferred option.
There is one important drawback with this solution. If a driver calls
dma_request_chan() when no channels are currently available, it will
get -EBUSY. If there were a flag in dma_request_chan to be put to
sleep (with timeout) until a channel is available, then it would
work. But busy waiting in the client driver is a waste of power.
Regards.
^ permalink raw reply
* [BUG/RFC] efi/libstub: Preserve the memory map pointed to by the FDT
From: Ard Biesheuvel @ 2016-12-09 17:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161209171501.21269-1-james.morse@arm.com>
Hi James,
On 9 December 2016 at 17:15, James Morse <james.morse@arm.com> wrote:
> allocate_new_fdt_and_exit_boot() bakes a pointer to the UEFI memory map
> into the FDT for later use. The corresponding memory is then free()d
> and (hopefully) allocated again via efi_exit_boot_services() calling
> efi_get_memory_map() as part of the exit_boot_services() loop. The
> final copy is annotated with virtual mappings and used to create the
> runtime map.
>
> Linux expects the FDT to contain a pointer to the UEFI memory map
> with the virtual mapping annotations, which will only happen if
> AllocatePool() returns memory to the stub that was just FreePool()d
> by the stub. This behaviour doesn't appear to guaranteed by the spec.
>
> Platforms that don't do this will use the stale memory map (assuming
> no later owner of the freed() memory changed it) and fail to initialise
> runtime services.
>
> Instead, keep the memory map we baked info the FDT, and create a new
> 'exit_map' pointer for use in the exit_boot_services() loop. (This was
> already different, it just had the same name and we hoped it would be
> in the same place).
>
> Annotate the memory map pointed to by the FDT with virtual mappings.
> This assumes the final memory map (which may be different), has
> not moved any of the regions with the runtime attribute.
>
> (Take the opportunity to remove some comments that are stale since
> commit ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT"))
>
> Signed-off-by: James Morse <james.morse@arm.com>
> Fixes: ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT")
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
---
> I haven't seen this causing problems on any real platforms, only kvmtool
> which I'm busily hacking up to have primitive UEFI support. Needless to
> say my AllocatePool() doesn't re-use memory.
>
Thanks for the report. This is obviously a serious bug, and should be
fixed asap.
However, the approach is not correct. The whole point of the memory
map dance is that *only* the final version is correct, and the code
Jeffrey added is to ensure that even if ExitBootServices() fails the
first time (which can occur when a timer event fires between
GetMemoryMap() and ExitBootServices()), the memory map is retrieved
again.
The only correct approach is to annotate the final version with
virtual addresses, and pass /that/ address to the kernel. So the bug
is arguably that we pass the wrong version of the memory map to the
OS.
I think we need to fix this by using fdt_setprop_inplace() to poke the
correct value into the FDT after ExitBootServices() returns.
--
Ard.
> drivers/firmware/efi/libstub/fdt.c | 23 +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> index a6a93116a8f0..f623894c633c 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -181,10 +181,6 @@ static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
> * which are fixed at 4K bytes, so in most cases the first
> * allocation should succeed.
> * EFI boot services are exited at the end of this function.
> - * There must be no allocations between the get_memory_map()
> - * call and the exit_boot_services() call, so the exiting of
> - * boot services is very tightly tied to the creation of the FDT
> - * with the final memory map in it.
> */
>
> efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
> @@ -199,7 +195,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
> unsigned long map_size, desc_size, buff_size;
> u32 desc_ver;
> unsigned long mmap_key;
> - efi_memory_desc_t *memory_map, *runtime_map;
> + efi_memory_desc_t *memory_map, *runtime_map, *exit_map;
> unsigned long new_fdt_size;
> efi_status_t status;
> int runtime_entry_count = 0;
> @@ -243,11 +239,6 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
> goto fail;
> }
>
> - /*
> - * Now that we have done our final memory allocation (and free)
> - * we can get the memory map key needed for
> - * exit_boot_services().
> - */
> status = efi_get_memory_map(sys_table, &map);
> if (status != EFI_SUCCESS)
> goto fail_free_new_fdt;
> @@ -259,8 +250,16 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
> memory_map, map_size, desc_size, desc_ver);
>
> /* Succeeding the first time is the expected case. */
> - if (status == EFI_SUCCESS)
> + if (status == EFI_SUCCESS) {
> + /*
> + * Annotate the memory_map in the FDT with virtual
> + * addresses. These shouldn't change as the placement
> + * of EFI_MEMORY_RUNTIME regions should be fixed.
> + */
> + efi_get_virtmap(memory_map, map_size, desc_size,
> + runtime_map, &runtime_entry_count);
> break;
> + }
>
> if (status == EFI_BUFFER_TOO_SMALL) {
> /*
> @@ -279,7 +278,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
> }
> }
>
> - sys_table->boottime->free_pool(memory_map);
> + map.map = &exit_map;
> priv.runtime_map = runtime_map;
> priv.runtime_entry_count = &runtime_entry_count;
> status = efi_exit_boot_services(sys_table, handle, &map, &priv,
> --
> 2.10.1
>
^ 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