* [PATCH v3 09/15] drm/sun4i: Add A83T support
From: Maxime Ripard @ 2017-12-07 10:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1729482.Mgh5nx8BIs@jernej-laptop>
Hi,
On Wed, Dec 06, 2017 at 05:37:47PM +0100, Jernej ?krabec wrote:
> Hi,
>
> Dne torek, 05. december 2017 ob 16:42:55 CET je Jernej ?krabec napisal(a):
> > Hi Maxime,
> >
> > Dne torek, 05. december 2017 ob 16:10:21 CET je Maxime Ripard napisal(a):
> > > Add support for the A83T display pipeline.
> > >
> > > Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > ---
> > >
> > > drivers/gpu/drm/sun4i/sun4i_drv.c | 1 +
> > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 +++++
> > > drivers/gpu/drm/sun4i/sun8i_mixer.c | 9 +++++++++
> > > 3 files changed, 15 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c
> > > b/drivers/gpu/drm/sun4i/sun4i_drv.c index 49215d91c853..6f5e721b545e
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> > > @@ -347,6 +347,7 @@ static const struct of_device_id sun4i_drv_of_table[]
> > > =
> > > { { .compatible = "allwinner,sun6i-a31s-display-engine" },
> > >
> > > { .compatible = "allwinner,sun7i-a20-display-engine" },
> > > { .compatible = "allwinner,sun8i-a33-display-engine" },
> > >
> > > + { .compatible = "allwinner,sun8i-a83t-display-engine" },
> > >
> > > { .compatible = "allwinner,sun8i-v3s-display-engine" },
> > > { }
> > >
> > > };
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 92f4738101e6..9b757450555f
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > @@ -1132,6 +1132,10 @@ static const struct sun4i_tcon_quirks
> > > sun8i_a33_quirks = { .has_lvds_pll = true,
> > >
> > > };
> > >
> > > +static const struct sun4i_tcon_quirks sun8i_a83t_lcd_quirks = {
> > > + /* nothing is supported */
> > > +};
> > > +
> > >
> > > static const struct sun4i_tcon_quirks sun8i_v3s_quirks = {
> > >
> > > /* nothing is supported */
> > >
> > > };
> > >
> > > @@ -1144,6 +1148,7 @@ const struct of_device_id sun4i_tcon_of_table[] = {
> > >
> > > { .compatible = "allwinner,sun6i-a31s-tcon", .data =
> &sun6i_a31s_quirks
> > > },
> > >
> > > { .compatible = "allwinner,sun7i-a20-tcon", .data = &sun7i_a20_quirks }, {
> > > .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks }, +
> {
> > > .compatible = "allwinner,sun8i-a83t-tcon-lcd", .data =
> > > &sun8i_a83t_lcd_quirks }, { .compatible = "allwinner,sun8i-v3s-tcon",
> > > .data
> > > = &sun8i_v3s_quirks }, { }
> > >
> > > };
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index ff235e3228ce..6829bec4ba68
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > @@ -477,6 +477,11 @@ static int sun8i_mixer_remove(struct platform_device
> > > *pdev) return 0;
> > >
> > > }
> > >
> > > +static const struct sun8i_mixer_cfg sun8i_a83t_mixer_cfg = {
> > > + .vi_num = 1,
> > > + .ui_num = 3,
> > > +};
> > > +
> >
> > I think you should expand that structure with:
> > .ccsc = 0,
> > .scaler_mask = 0xf,
> > .mod_rate = 150000000,
>
> I guess you could set higher clock if CLK_SET_RATE_PARENT flag is set to de_clk
> in A83T CCU driver. BSP sets it to 500 MHz, which is a bit high...
Actually, I didn't have to change the rate used by default, so it's
not clear what it should be on the A83T.
And yeah, I'd like to avoid setting it at 500MHz without any
particular reason. The A83T draws way to much power already without
making it worse.
I'll add the CCSC and scaler_mask fields.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171207/c02352d1/attachment.sig>
^ permalink raw reply
* [kernel-hardening][PATCH v5 3/3] arm: mm: dump: add checking for writable and executable pages
From: Jinbum Park @ 2017-12-07 10:54 UTC (permalink / raw)
To: linux-arm-kernel
Page mappings with full RWX permissions are a security risk.
x86, arm64 has an option to walk the page tables
and dump any bad pages.
(1404d6f13e47
("arm64: dump: Add checking for writable and exectuable pages"))
Add a similar implementation for arm.
Tested-by: Laura Abbott <labbott@redhat.com>
Reviewed-by: Laura Abbott <labbott@redhat.com>
Signed-off-by: Jinbum Park <jinb.park7@gmail.com>
---
v5: No changes
(Just add Tested-by, Reviewed-by from Laura Abbott)
---
arch/arm/Kconfig.debug | 27 ++++++++++++++++++++
arch/arm/include/asm/ptdump.h | 8 ++++++
arch/arm/mm/dump.c | 58 ++++++++++++++++++++++++++++++++++++++++++-
arch/arm/mm/init.c | 2 ++
4 files changed, 94 insertions(+), 1 deletion(-)
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index e7b94db..78a6470 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -20,6 +20,33 @@ config ARM_PTDUMP_DEBUGFS
kernel.
If in doubt, say "N"
+config DEBUG_WX
+ bool "Warn on W+X mappings at boot"
+ select ARM_PTDUMP_CORE
+ ---help---
+ Generate a warning if any W+X mappings are found at boot.
+
+ This is useful for discovering cases where the kernel is leaving
+ W+X mappings after applying NX, as such mappings are a security risk.
+
+ Look for a message in dmesg output like this:
+
+ arm/mm: Checked W+X mappings: passed, no W+X pages found.
+
+ or like this, if the check failed:
+
+ arm/mm: Checked W+X mappings: FAILED, <N> W+X pages found.
+
+ Note that even if the check fails, your kernel is possibly
+ still fine, as W+X mappings are not a security hole in
+ themselves, what they do is that they make the exploitation
+ of other unfixed kernel bugs easier.
+
+ There is no runtime or memory usage effect of this option
+ once the kernel has booted up - it's a one time check.
+
+ If in doubt, say "Y".
+
# RMK wants arm kernels compiled with frame pointers or stack unwinding.
# If you know what you are doing and are willing to live without stack
# traces, you can get a slightly smaller kernel by setting this option to
diff --git a/arch/arm/include/asm/ptdump.h b/arch/arm/include/asm/ptdump.h
index 3a6c0b7..b6a0162 100644
--- a/arch/arm/include/asm/ptdump.h
+++ b/arch/arm/include/asm/ptdump.h
@@ -43,6 +43,14 @@ static inline int ptdump_debugfs_register(struct ptdump_info *info,
}
#endif /* CONFIG_ARM_PTDUMP_DEBUGFS */
+void ptdump_check_wx(void);
+
#endif /* CONFIG_ARM_PTDUMP_CORE */
+#ifdef CONFIG_DEBUG_WX
+#define debug_checkwx() ptdump_check_wx()
+#else
+#define debug_checkwx() do { } while (0)
+#endif
+
#endif /* __ASM_PTDUMP_H */
diff --git a/arch/arm/mm/dump.c b/arch/arm/mm/dump.c
index 43a2bee..084779c 100644
--- a/arch/arm/mm/dump.c
+++ b/arch/arm/mm/dump.c
@@ -52,6 +52,8 @@ struct pg_state {
unsigned long start_address;
unsigned level;
u64 current_prot;
+ bool check_wx;
+ unsigned long wx_pages;
const char *current_domain;
};
@@ -60,6 +62,8 @@ struct prot_bits {
u64 val;
const char *set;
const char *clear;
+ bool ro_bit;
+ bool nx_bit;
};
static const struct prot_bits pte_bits[] = {
@@ -73,11 +77,13 @@ struct prot_bits {
.val = L_PTE_RDONLY,
.set = "ro",
.clear = "RW",
+ .ro_bit = true,
}, {
.mask = L_PTE_XN,
.val = L_PTE_XN,
.set = "NX",
.clear = "x ",
+ .nx_bit = true,
}, {
.mask = L_PTE_SHARED,
.val = L_PTE_SHARED,
@@ -141,11 +147,13 @@ struct prot_bits {
.val = L_PMD_SECT_RDONLY | PMD_SECT_AP2,
.set = "ro",
.clear = "RW",
+ .ro_bit = true,
#elif __LINUX_ARM_ARCH__ >= 6
{
.mask = PMD_SECT_APX | PMD_SECT_AP_READ | PMD_SECT_AP_WRITE,
.val = PMD_SECT_APX | PMD_SECT_AP_WRITE,
.set = " ro",
+ .ro_bit = true,
}, {
.mask = PMD_SECT_APX | PMD_SECT_AP_READ | PMD_SECT_AP_WRITE,
.val = PMD_SECT_AP_WRITE,
@@ -164,6 +172,7 @@ struct prot_bits {
.mask = PMD_SECT_AP_READ | PMD_SECT_AP_WRITE,
.val = 0,
.set = " ro",
+ .ro_bit = true,
}, {
.mask = PMD_SECT_AP_READ | PMD_SECT_AP_WRITE,
.val = PMD_SECT_AP_WRITE,
@@ -182,6 +191,7 @@ struct prot_bits {
.val = PMD_SECT_XN,
.set = "NX",
.clear = "x ",
+ .nx_bit = true,
}, {
.mask = PMD_SECT_S,
.val = PMD_SECT_S,
@@ -194,6 +204,8 @@ struct pg_level {
const struct prot_bits *bits;
size_t num;
u64 mask;
+ const struct prot_bits *ro_bit;
+ const struct prot_bits *nx_bit;
};
static struct pg_level pg_level[] = {
@@ -226,6 +238,23 @@ static void dump_prot(struct pg_state *st, const struct prot_bits *bits, size_t
}
}
+static void note_prot_wx(struct pg_state *st, unsigned long addr)
+{
+ if (!st->check_wx)
+ return;
+ if ((st->current_prot & pg_level[st->level].ro_bit->mask) ==
+ pg_level[st->level].ro_bit->val)
+ return;
+ if ((st->current_prot & pg_level[st->level].nx_bit->mask) ==
+ pg_level[st->level].nx_bit->val)
+ return;
+
+ WARN_ONCE(1, "arm/mm: Found insecure W+X mapping at address %pS\n",
+ (void *)st->start_address);
+
+ st->wx_pages += (addr - st->start_address) / PAGE_SIZE;
+}
+
static void note_page(struct pg_state *st, unsigned long addr,
unsigned int level, u64 val, const char *domain)
{
@@ -244,6 +273,7 @@ static void note_page(struct pg_state *st, unsigned long addr,
unsigned long delta;
if (st->current_prot) {
+ note_prot_wx(st, addr);
pt_dump_seq_printf(st->seq, "0x%08lx-0x%08lx ",
st->start_address, addr);
@@ -367,6 +397,7 @@ void ptdump_walk_pgd(struct seq_file *m, struct ptdump_info *info)
struct pg_state st = {
.seq = m,
.marker = info->markers,
+ .check_wx = false,
};
walk_pgd(&st, info->mm, info->base_addr);
@@ -379,8 +410,13 @@ static void ptdump_initialize(void)
for (i = 0; i < ARRAY_SIZE(pg_level); i++)
if (pg_level[i].bits)
- for (j = 0; j < pg_level[i].num; j++)
+ for (j = 0; j < pg_level[i].num; j++) {
pg_level[i].mask |= pg_level[i].bits[j].mask;
+ if (pg_level[i].bits[j].ro_bit)
+ pg_level[i].ro_bit = &pg_level[i].bits[j];
+ if (pg_level[i].bits[j].nx_bit)
+ pg_level[i].nx_bit = &pg_level[i].bits[j];
+ }
address_markers[2].start_address = VMALLOC_START;
}
@@ -391,6 +427,26 @@ static void ptdump_initialize(void)
.base_addr = 0,
};
+void ptdump_check_wx(void)
+{
+ struct pg_state st = {
+ .seq = NULL,
+ .marker = (struct addr_marker[]) {
+ { 0, NULL},
+ { -1, NULL},
+ },
+ .check_wx = true,
+ };
+
+ walk_pgd(&st, &init_mm, 0);
+ note_page(&st, 0, 0, 0, NULL);
+ if (st.wx_pages)
+ pr_warn("Checked W+X mappings: FAILED, %lu W+X pages found\n",
+ st.wx_pages);
+ else
+ pr_info("Checked W+X mappings: passed, no W+X pages found\n");
+}
+
static int ptdump_init(void)
{
ptdump_initialize();
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index a1f11a7..bd6f451 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -36,6 +36,7 @@
#include <asm/system_info.h>
#include <asm/tlb.h>
#include <asm/fixmap.h>
+#include <asm/ptdump.h>
#include <asm/mach/arch.h>
#include <asm/mach/map.h>
@@ -738,6 +739,7 @@ static int __mark_rodata_ro(void *unused)
void mark_rodata_ro(void)
{
stop_machine(__mark_rodata_ro, NULL, NULL);
+ debug_checkwx();
}
void set_kernel_text_rw(void)
--
1.9.1
^ permalink raw reply related
* [PATCH v4 4/8] arm: OMAP: Move dmtimer driver out of plat-omap to drivers under clocksource
From: Daniel Lezcano @ 2017-12-07 10:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512105397-2544-5-git-send-email-j-keerthy@ti.com>
On 01/12/2017 06:16, Keerthy wrote:
> Move the dmtimer driver out of plat-omap to clocksource.
> So that non-omap devices also could use this.
>
> No Code changes done to the driver file.
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>
> Changes in v3:
>
> * Added Sebastian's Reviewed-by.
>
> Changes in v2:
>
> * No code changes in this v2 version. Only enhanced patch
> statistics for renames.
>
> arch/arm/plat-omap/Kconfig | 6 ------
> arch/arm/plat-omap/Makefile | 1 -
> drivers/clocksource/Kconfig | 6 ++++++
> drivers/clocksource/Makefile | 1 +
> {arch/arm/plat-omap => drivers/clocksource}/dmtimer.c | 0
> 5 files changed, 7 insertions(+), 7 deletions(-)
> rename {arch/arm/plat-omap => drivers/clocksource}/dmtimer.c (100%)
Take the opportunity to rename it timer-dm.c
> diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
> index 7276afe..afc1a1d 100644
> --- a/arch/arm/plat-omap/Kconfig
> +++ b/arch/arm/plat-omap/Kconfig
> @@ -106,12 +106,6 @@ config OMAP3_L2_AUX_SECURE_SERVICE_SET_ID
> help
> PPA routine service ID for setting L2 auxiliary control register.
>
> -config OMAP_DM_TIMER
> - bool "Use dual-mode timer"
> - depends on ARCH_OMAP16XX || ARCH_OMAP2PLUS
> - help
> - Select this option if you want to use OMAP Dual-Mode timers.
> -
> config OMAP_SERIAL_WAKE
> bool "Enable wake-up events for serial ports"
> depends on ARCH_OMAP1 && OMAP_MUX
> diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
> index 47e1867..7215ada 100644
> --- a/arch/arm/plat-omap/Makefile
> +++ b/arch/arm/plat-omap/Makefile
> @@ -9,5 +9,4 @@ obj-y := sram.o dma.o counter_32k.o
>
> # omap_device support (OMAP2+ only at the moment)
>
> -obj-$(CONFIG_OMAP_DM_TIMER) += dmtimer.o
> obj-$(CONFIG_OMAP_DEBUG_LEDS) += debug-leds.o
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index c729a88..4da66cf 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -46,6 +46,12 @@ config DIGICOLOR_TIMER
> help
> Enables the support for the digicolor timer driver.
>
> +config OMAP_DM_TIMER
> + bool "Use dual-mode timer"
> + depends on ARCH_OMAP16XX || ARCH_OMAP2PLUS
> + help
> + Select this option if you want to use Dual-Mode timers.
> +
So in the clocksource directory the rule is the following:
The config option is silent except if COMPILE_TEST is set.
It is the Kconfig's platform which select the driver so there is no deps
on the ARCH.
config OMAP_DM_TIMER
bool "Use dual-mode timer" if COMPILE_TEST
help
Select this option if you want to use Dual-Mode timers.
Here you will have to make sure the driver compiles on x86.
> config DW_APB_TIMER
> bool "DW APB timer driver" if COMPILE_TEST
> help
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index 72711f1..b077076 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_EM_TIMER_STI) += em_sti.o
> obj-$(CONFIG_CLKBLD_I8253) += i8253.o
> obj-$(CONFIG_CLKSRC_MMIO) += mmio.o
> obj-$(CONFIG_DIGICOLOR_TIMER) += timer-digicolor.o
> +obj-$(CONFIG_OMAP_DM_TIMER) += dmtimer.o
> obj-$(CONFIG_DW_APB_TIMER) += dw_apb_timer.o
> obj-$(CONFIG_DW_APB_TIMER_OF) += dw_apb_timer_of.o
> obj-$(CONFIG_FTTMR010_TIMER) += timer-fttmr010.o
> diff --git a/arch/arm/plat-omap/dmtimer.c b/drivers/clocksource/dmtimer.c
> similarity index 100%
> rename from arch/arm/plat-omap/dmtimer.c
> rename to drivers/clocksource/dmtimer.c
>
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* [PATCH v7 8/8] KVM: arm/arm64: Avoid work when userspace iqchips are not used
From: Christoffer Dall @ 2017-12-07 10:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171207105418.22428-1-christoffer.dall@linaro.org>
We currently check if the VM has a userspace irqchip on every exit from
the VCPU, and if so, we do some work to ensure correct timer behavior.
This is unfortunate, as we could avoid doing any work entirely, if we
didn't have to support irqchip in userspace.
Realizing the userspace irqchip on ARM is mostly a developer or hobby
feature, and is unlikely to be used in servers or other scenarios where
performance is a priority, we can use a refcounted static key to only
check the irqchip configuration when we have at least one VM that uses
an irqchip in userspace.
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
virt/kvm/arm/arch_timer.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 82d4963f63b8..df21451e7654 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -51,6 +51,8 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,
struct arch_timer_context *timer_ctx);
static bool kvm_timer_should_fire(struct arch_timer_context *timer_ctx);
+static DEFINE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
+
u64 kvm_phys_timer_read(void)
{
return timecounter->cc->read(timecounter->cc);
@@ -569,7 +571,8 @@ static void unmask_vtimer_irq_user(struct kvm_vcpu *vcpu)
*/
void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
{
- unmask_vtimer_irq_user(vcpu);
+ if (static_branch_unlikely(&userspace_irqchip_in_use))
+ unmask_vtimer_irq_user(vcpu);
}
int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu)
@@ -774,6 +777,8 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu)
soft_timer_cancel(&timer->bg_timer, &timer->expired);
soft_timer_cancel(&timer->phys_timer, NULL);
kvm_vgic_unmap_phys_irq(vcpu, vtimer->irq.irq);
+ if (timer->enabled && !irqchip_in_kernel(vcpu->kvm))
+ static_branch_dec(&userspace_irqchip_in_use);
}
static bool timer_irqs_are_valid(struct kvm_vcpu *vcpu)
@@ -826,8 +831,10 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
return 0;
/* Without a VGIC we do not map virtual IRQs to physical IRQs */
- if (!irqchip_in_kernel(vcpu->kvm))
+ if (!irqchip_in_kernel(vcpu->kvm)) {
+ static_branch_inc(&userspace_irqchip_in_use);
goto no_vgic;
+ }
if (!vgic_initialized(vcpu->kvm))
return -ENODEV;
--
2.14.2
^ permalink raw reply related
* [PATCH v7 7/8] KVM: arm/arm64: Provide a get_input_level for the arch timer
From: Christoffer Dall @ 2017-12-07 10:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171207105418.22428-1-christoffer.dall@linaro.org>
The VGIC can now support the life-cycle of mapped level-triggered
interrupts, and we no longer have to read back the timer state on every
exit from the VM if we had an asserted timer interrupt signal, because
the VGIC already knows if we hit the unlikely case where the guest
disables the timer without ACKing the virtual timer interrupt.
This means we rework a bit of the code to factor out the functionality
to snapshot the timer state from vtimer_save_state(), and we can reuse
this functionality in the sync path when we have an irqchip in
userspace, and also to support our implementation of the
get_input_level() function for the timer.
This change also means that we can no longer rely on the timer's view of
the interrupt line to set the active state, because we no longer
maintain this state for mapped interrupts when exiting from the guest.
Instead, we only set the active state if the virtual interrupt is
active, and otherwise we simply let the timer fire again and raise the
virtual interrupt from the ISR.
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
include/kvm/arm_arch_timer.h | 2 ++
virt/kvm/arm/arch_timer.c | 75 +++++++++++++++++++++-----------------------
2 files changed, 38 insertions(+), 39 deletions(-)
diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index 01ee473517e2..f57f795d704c 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -90,6 +90,8 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu);
void kvm_timer_init_vhe(void);
+bool kvm_arch_timer_get_input_level(int vintid);
+
#define vcpu_vtimer(v) (&(v)->arch.timer_cpu.vtimer)
#define vcpu_ptimer(v) (&(v)->arch.timer_cpu.ptimer)
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index e78ba5e20f74..82d4963f63b8 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -343,6 +343,12 @@ static void kvm_timer_update_state(struct kvm_vcpu *vcpu)
phys_timer_emulate(vcpu);
}
+static void __timer_snapshot_state(struct arch_timer_context *timer)
+{
+ timer->cnt_ctl = read_sysreg_el0(cntv_ctl);
+ timer->cnt_cval = read_sysreg_el0(cntv_cval);
+}
+
static void vtimer_save_state(struct kvm_vcpu *vcpu)
{
struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
@@ -354,10 +360,8 @@ static void vtimer_save_state(struct kvm_vcpu *vcpu)
if (!vtimer->loaded)
goto out;
- if (timer->enabled) {
- vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);
- vtimer->cnt_cval = read_sysreg_el0(cntv_cval);
- }
+ if (timer->enabled)
+ __timer_snapshot_state(vtimer);
/* Disable the virtual timer */
write_sysreg_el0(0, cntv_ctl);
@@ -454,8 +458,7 @@ static void kvm_timer_vcpu_load_vgic(struct kvm_vcpu *vcpu)
bool phys_active;
int ret;
- phys_active = vtimer->irq.level ||
- kvm_vgic_map_is_active(vcpu, vtimer->irq.irq);
+ phys_active = kvm_vgic_map_is_active(vcpu, vtimer->irq.irq);
ret = irq_set_irqchip_state(host_vtimer_irq,
IRQCHIP_STATE_ACTIVE,
@@ -541,27 +544,19 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)
set_cntvoff(0);
}
-static void unmask_vtimer_irq(struct kvm_vcpu *vcpu)
+/*
+ * With a userspace irqchip we have to check if the guest de-asserted the
+ * timer and if so, unmask the timer irq signal on the host interrupt
+ * controller to ensure that we see future timer signals.
+ */
+static void unmask_vtimer_irq_user(struct kvm_vcpu *vcpu)
{
struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
if (unlikely(!irqchip_in_kernel(vcpu->kvm))) {
- kvm_vtimer_update_mask_user(vcpu);
- return;
- }
-
- /*
- * If the guest disabled the timer without acking the interrupt, then
- * we must make sure the physical and virtual active states are in
- * sync by deactivating the physical interrupt, because otherwise we
- * wouldn't see the next timer interrupt in the host.
- */
- if (!kvm_vgic_map_is_active(vcpu, vtimer->irq.irq)) {
- int ret;
- ret = irq_set_irqchip_state(host_vtimer_irq,
- IRQCHIP_STATE_ACTIVE,
- false);
- WARN_ON(ret);
+ __timer_snapshot_state(vtimer);
+ if (!kvm_timer_should_fire(vtimer))
+ kvm_vtimer_update_mask_user(vcpu);
}
}
@@ -574,21 +569,7 @@ static void unmask_vtimer_irq(struct kvm_vcpu *vcpu)
*/
void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
{
- struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
-
- /*
- * If we entered the guest with the vtimer output asserted we have to
- * check if the guest has modified the timer so that we should lower
- * the line at this point.
- */
- if (vtimer->irq.level) {
- vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);
- vtimer->cnt_cval = read_sysreg_el0(cntv_cval);
- if (!kvm_timer_should_fire(vtimer)) {
- kvm_timer_update_irq(vcpu, false, vtimer);
- unmask_vtimer_irq(vcpu);
- }
- }
+ unmask_vtimer_irq_user(vcpu);
}
int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu)
@@ -819,6 +800,22 @@ static bool timer_irqs_are_valid(struct kvm_vcpu *vcpu)
return true;
}
+bool kvm_arch_timer_get_input_level(int vintid)
+{
+ struct kvm_vcpu *vcpu = kvm_arm_get_running_vcpu();
+ struct arch_timer_context *timer;
+
+ if (vintid == vcpu_vtimer(vcpu)->irq.irq)
+ timer = vcpu_vtimer(vcpu);
+ else
+ BUG(); /* We only map the vtimer so far */
+
+ if (timer->loaded)
+ __timer_snapshot_state(timer);
+
+ return kvm_timer_should_fire(timer);
+}
+
int kvm_timer_enable(struct kvm_vcpu *vcpu)
{
struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
@@ -841,7 +838,7 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
}
ret = kvm_vgic_map_phys_irq(vcpu, host_vtimer_irq, vtimer->irq.irq,
- NULL);
+ kvm_arch_timer_get_input_level);
if (ret)
return ret;
--
2.14.2
^ permalink raw reply related
* [PATCH v7 6/8] KVM: arm/arm64: Support VGIC dist pend/active changes for mapped IRQs
From: Christoffer Dall @ 2017-12-07 10:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171207105418.22428-1-christoffer.dall@linaro.org>
For mapped IRQs (with the HW bit set in the LR) we have to follow some
rules of the architecture. One of these rules is that VM must not be
allowed to deactivate a virtual interrupt with the HW bit set unless the
physical interrupt is also active.
This works fine when injecting mapped interrupts, because we leave it up
to the injector to either set EOImode==1 or manually set the active
state of the physical interrupt.
However, the guest can set virtual interrupt to be pending or active by
writing to the virtual distributor, which could lead to deactivating a
virtual interrupt with the HW bit set without the physical interrupt
being active.
We could set the physical interrupt to active whenever we are about to
enter the VM with a HW interrupt either pending or active, but that
would be really slow, especially on GICv2. So we take the long way
around and do the hard work when needed, which is expected to be
extremely rare.
When the VM sets the pending state for a HW interrupt on the virtual
distributor we set the active state on the physical distributor, because
the virtual interrupt can become active and then the guest can
deactivate it.
When the VM clears the pending state we also clear it on the physical
side, because the injector might otherwise raise the interrupt. We also
clear the physical active state when the virtual interrupt is not
active, since otherwise a SPEND/CPEND sequence from the guest would
prevent signaling of future interrupts.
Changing the state of mapped interrupts from userspace is not supported,
and it's expected that userspace unmaps devices from VFIO before
attempting to set the interrupt state, because the interrupt state is
driven by hardware.
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
virt/kvm/arm/vgic/vgic-mmio.c | 71 +++++++++++++++++++++++++++++++++++++++----
virt/kvm/arm/vgic/vgic.c | 7 +++++
virt/kvm/arm/vgic/vgic.h | 1 +
3 files changed, 73 insertions(+), 6 deletions(-)
diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
index fdad95f62fa3..83d82bd7dc4e 100644
--- a/virt/kvm/arm/vgic/vgic-mmio.c
+++ b/virt/kvm/arm/vgic/vgic-mmio.c
@@ -16,6 +16,7 @@
#include <linux/kvm.h>
#include <linux/kvm_host.h>
#include <kvm/iodev.h>
+#include <kvm/arm_arch_timer.h>
#include <kvm/arm_vgic.h>
#include "vgic.h"
@@ -143,10 +144,22 @@ static struct kvm_vcpu *vgic_get_mmio_requester_vcpu(void)
return vcpu;
}
+/* Must be called with irq->irq_lock held */
+static void vgic_hw_irq_spending(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
+ bool is_uaccess)
+{
+ if (is_uaccess)
+ return;
+
+ irq->pending_latch = true;
+ vgic_irq_set_phys_active(irq, true);
+}
+
void vgic_mmio_write_spending(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val)
{
+ bool is_uaccess = !vgic_get_mmio_requester_vcpu();
u32 intid = VGIC_ADDR_TO_INTID(addr, 1);
int i;
unsigned long flags;
@@ -155,17 +168,45 @@ void vgic_mmio_write_spending(struct kvm_vcpu *vcpu,
struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
spin_lock_irqsave(&irq->irq_lock, flags);
- irq->pending_latch = true;
-
+ if (irq->hw)
+ vgic_hw_irq_spending(vcpu, irq, is_uaccess);
+ else
+ irq->pending_latch = true;
vgic_queue_irq_unlock(vcpu->kvm, irq, flags);
vgic_put_irq(vcpu->kvm, irq);
}
}
+/* Must be called with irq->irq_lock held */
+static void vgic_hw_irq_cpending(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
+ bool is_uaccess)
+{
+ if (is_uaccess)
+ return;
+
+ irq->pending_latch = false;
+
+ /*
+ * We don't want the guest to effectively mask the physical
+ * interrupt by doing a write to SPENDR followed by a write to
+ * CPENDR for HW interrupts, so we clear the active state on
+ * the physical side if the virtual interrupt is not active.
+ * This may lead to taking an additional interrupt on the
+ * host, but that should not be a problem as the worst that
+ * can happen is an additional vgic injection. We also clear
+ * the pending state to maintain proper semantics for edge HW
+ * interrupts.
+ */
+ vgic_irq_set_phys_pending(irq, false);
+ if (!irq->active)
+ vgic_irq_set_phys_active(irq, false);
+}
+
void vgic_mmio_write_cpending(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val)
{
+ bool is_uaccess = !vgic_get_mmio_requester_vcpu();
u32 intid = VGIC_ADDR_TO_INTID(addr, 1);
int i;
unsigned long flags;
@@ -175,7 +216,10 @@ void vgic_mmio_write_cpending(struct kvm_vcpu *vcpu,
spin_lock_irqsave(&irq->irq_lock, flags);
- irq->pending_latch = false;
+ if (irq->hw)
+ vgic_hw_irq_cpending(vcpu, irq, is_uaccess);
+ else
+ irq->pending_latch = false;
spin_unlock_irqrestore(&irq->irq_lock, flags);
vgic_put_irq(vcpu->kvm, irq);
@@ -202,8 +246,19 @@ unsigned long vgic_mmio_read_active(struct kvm_vcpu *vcpu,
return value;
}
+/* Must be called with irq->irq_lock held */
+static void vgic_hw_irq_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
+ bool active, bool is_uaccess)
+{
+ if (is_uaccess)
+ return;
+
+ irq->active = active;
+ vgic_irq_set_phys_active(irq, active);
+}
+
static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
- bool new_active_state)
+ bool active)
{
unsigned long flags;
struct kvm_vcpu *requester_vcpu = vgic_get_mmio_requester_vcpu();
@@ -231,8 +286,12 @@ static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
irq->vcpu->cpu != -1) /* VCPU thread is running */
cond_resched_lock(&irq->irq_lock);
- irq->active = new_active_state;
- if (new_active_state)
+ if (irq->hw)
+ vgic_hw_irq_change_active(vcpu, irq, active, !requester_vcpu);
+ else
+ irq->active = active;
+
+ if (irq->active)
vgic_queue_irq_unlock(vcpu->kvm, irq, flags);
else
spin_unlock_irqrestore(&irq->irq_lock, flags);
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index eadabb249d2a..f4c92fae9cd3 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -144,6 +144,13 @@ void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)
kfree(irq);
}
+void vgic_irq_set_phys_pending(struct vgic_irq *irq, bool pending)
+{
+ WARN_ON(irq_set_irqchip_state(irq->host_irq,
+ IRQCHIP_STATE_PENDING,
+ pending));
+}
+
/* Get the input level of a mapped IRQ directly from the physical GIC */
bool vgic_get_phys_line_level(struct vgic_irq *irq)
{
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index d0787983a357..12c37b89f7a3 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -146,6 +146,7 @@ struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu,
u32 intid);
void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq);
bool vgic_get_phys_line_level(struct vgic_irq *irq);
+void vgic_irq_set_phys_pending(struct vgic_irq *irq, bool pending);
void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active);
bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq,
unsigned long flags);
--
2.14.2
^ permalink raw reply related
* [PATCH v7 5/8] KVM: arm/arm64: Support a vgic interrupt line level sample function
From: Christoffer Dall @ 2017-12-07 10:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171207105418.22428-1-christoffer.dall@linaro.org>
The GIC sometimes need to sample the physical line of a mapped
interrupt. As we know this to be notoriously slow, provide a callback
function for devices (such as the timer) which can do this much faster
than talking to the distributor, for example by comparing a few
in-memory values. Fall back to the good old method of poking the
physical GIC if no callback is provided.
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
include/kvm/arm_vgic.h | 13 ++++++++++++-
virt/kvm/arm/arch_timer.c | 3 ++-
virt/kvm/arm/vgic/vgic.c | 12 +++++++++---
3 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 8c896540a72c..cdbd142ca7f2 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -130,6 +130,17 @@ struct vgic_irq {
u8 priority;
enum vgic_irq_config config; /* Level or edge */
+ /*
+ * Callback function pointer to in-kernel devices that can tell us the
+ * state of the input level of mapped level-triggered IRQ faster than
+ * peaking into the physical GIC.
+ *
+ * Always called in non-preemptible section and the functions can use
+ * kvm_arm_get_running_vcpu() to get the vcpu pointer for private
+ * IRQs.
+ */
+ bool (*get_input_level)(int vintid);
+
void *owner; /* Opaque pointer to reserve an interrupt
for in-kernel devices. */
};
@@ -331,7 +342,7 @@ void kvm_vgic_init_cpu_hardware(void);
int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,
bool level, void *owner);
int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,
- u32 vintid);
+ u32 vintid, bool (*get_input_level)(int vindid));
int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int vintid);
bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, unsigned int vintid);
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index dd5aca05c500..e78ba5e20f74 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -840,7 +840,8 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
return -EINVAL;
}
- ret = kvm_vgic_map_phys_irq(vcpu, host_vtimer_irq, vtimer->irq.irq);
+ ret = kvm_vgic_map_phys_irq(vcpu, host_vtimer_irq, vtimer->irq.irq,
+ NULL);
if (ret)
return ret;
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index 607cbbc27a1c..eadabb249d2a 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -151,6 +151,9 @@ bool vgic_get_phys_line_level(struct vgic_irq *irq)
BUG_ON(!irq->hw);
+ if (irq->get_input_level)
+ return irq->get_input_level(irq->intid);
+
WARN_ON(irq_get_irqchip_state(irq->host_irq,
IRQCHIP_STATE_PENDING,
&line_level));
@@ -436,7 +439,8 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,
/* @irq->irq_lock must be held */
static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
- unsigned int host_irq)
+ unsigned int host_irq,
+ bool (*get_input_level)(int vindid))
{
struct irq_desc *desc;
struct irq_data *data;
@@ -456,6 +460,7 @@ static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
irq->hw = true;
irq->host_irq = host_irq;
irq->hwintid = data->hwirq;
+ irq->get_input_level = get_input_level;
return 0;
}
@@ -464,10 +469,11 @@ static inline void kvm_vgic_unmap_irq(struct vgic_irq *irq)
{
irq->hw = false;
irq->hwintid = 0;
+ irq->get_input_level = NULL;
}
int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,
- u32 vintid)
+ u32 vintid, bool (*get_input_level)(int vindid))
{
struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, vintid);
unsigned long flags;
@@ -476,7 +482,7 @@ int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,
BUG_ON(!irq);
spin_lock_irqsave(&irq->irq_lock, flags);
- ret = kvm_vgic_map_irq(vcpu, irq, host_irq);
+ ret = kvm_vgic_map_irq(vcpu, irq, host_irq, get_input_level);
spin_unlock_irqrestore(&irq->irq_lock, flags);
vgic_put_irq(vcpu->kvm, irq);
--
2.14.2
^ permalink raw reply related
* [PATCH v7 4/8] KVM: arm/arm64: vgic: Support level-triggered mapped interrupts
From: Christoffer Dall @ 2017-12-07 10:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171207105418.22428-1-christoffer.dall@linaro.org>
Level-triggered mapped IRQs are special because we only observe rising
edges as input to the VGIC, and we don't set the EOI flag and therefore
are not told when the level goes down, so that we can re-queue a new
interrupt when the level goes up.
One way to solve this problem is to side-step the logic of the VGIC and
special case the validation in the injection path, but it has the
unfortunate drawback of having to peak into the physical GIC state
whenever we want to know if the interrupt is pending on the virtual
distributor.
Instead, we can maintain the current semantics of a level triggered
interrupt by sort of treating it as an edge-triggered interrupt,
following from the fact that we only observe an asserting edge. This
requires us to be a bit careful when populating the LRs and when folding
the state back in though:
* We lower the line level when populating the LR, so that when
subsequently observing an asserting edge, the VGIC will do the right
thing.
* If the guest never acked the interrupt while running (for example if
it had masked interrupts at the CPU level while running), we have
to preserve the pending state of the LR and move it back to the
line_level field of the struct irq when folding LR state.
If the guest never acked the interrupt while running, but changed the
device state and lowered the line (again with interrupts masked) then
we need to observe this change in the line_level.
Both of the above situations are solved by sampling the physical line
and set the line level when folding the LR back.
* Finally, if the guest never acked the interrupt while running and
sampling the line reveals that the device state has changed and the
line has been lowered, we must clear the physical active state, since
we will otherwise never be told when the interrupt becomes asserted
again.
This has the added benefit of making the timer optimization patches
(https://lists.cs.columbia.edu/pipermail/kvmarm/2017-July/026343.html) a
bit simpler, because the timer code doesn't have to clear the active
state on the sync anymore. It also potentially improves the performance
of the timer implementation because the GIC knows the state or the LR
and only needs to clear the
active state when the pending bit in the LR is still set, where the
timer has to always clear it when returning from running the guest with
an injected timer interrupt.
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
virt/kvm/arm/vgic/vgic-v2.c | 29 +++++++++++++++++++++++++++++
virt/kvm/arm/vgic/vgic-v3.c | 29 +++++++++++++++++++++++++++++
virt/kvm/arm/vgic/vgic.c | 23 +++++++++++++++++++++++
virt/kvm/arm/vgic/vgic.h | 7 +++++++
4 files changed, 88 insertions(+)
diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c
index 80897102da26..c32d7b93ffd1 100644
--- a/virt/kvm/arm/vgic/vgic-v2.c
+++ b/virt/kvm/arm/vgic/vgic-v2.c
@@ -105,6 +105,26 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
irq->pending_latch = false;
}
+ /*
+ * Level-triggered mapped IRQs are special because we only
+ * observe rising edges as input to the VGIC.
+ *
+ * If the guest never acked the interrupt we have to sample
+ * the physical line and set the line level, because the
+ * device state could have changed or we simply need to
+ * process the still pending interrupt later.
+ *
+ * If this causes us to lower the level, we have to also clear
+ * the physical active state, since we will otherwise never be
+ * told when the interrupt becomes asserted again.
+ */
+ if (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT)) {
+ irq->line_level = vgic_get_phys_line_level(irq);
+
+ if (!irq->line_level)
+ vgic_irq_set_phys_active(irq, false);
+ }
+
spin_unlock_irqrestore(&irq->irq_lock, flags);
vgic_put_irq(vcpu->kvm, irq);
}
@@ -162,6 +182,15 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
val |= GICH_LR_EOI;
}
+ /*
+ * Level-triggered mapped IRQs are special because we only observe
+ * rising edges as input to the VGIC. We therefore lower the line
+ * level here, so that we can take new virtual IRQs. See
+ * vgic_v2_fold_lr_state for more info.
+ */
+ if (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT))
+ irq->line_level = false;
+
/* The GICv2 LR only holds five bits of priority. */
val |= (irq->priority >> 3) << GICH_LR_PRIORITY_SHIFT;
diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index 2f05f732d3fd..a14423a0d383 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -96,6 +96,26 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
irq->pending_latch = false;
}
+ /*
+ * Level-triggered mapped IRQs are special because we only
+ * observe rising edges as input to the VGIC.
+ *
+ * If the guest never acked the interrupt we have to sample
+ * the physical line and set the line level, because the
+ * device state could have changed or we simply need to
+ * process the still pending interrupt later.
+ *
+ * If this causes us to lower the level, we have to also clear
+ * the physical active state, since we will otherwise never be
+ * told when the interrupt becomes asserted again.
+ */
+ if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) {
+ irq->line_level = vgic_get_phys_line_level(irq);
+
+ if (!irq->line_level)
+ vgic_irq_set_phys_active(irq, false);
+ }
+
spin_unlock_irqrestore(&irq->irq_lock, flags);
vgic_put_irq(vcpu->kvm, irq);
}
@@ -145,6 +165,15 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
val |= ICH_LR_EOI;
}
+ /*
+ * Level-triggered mapped IRQs are special because we only observe
+ * rising edges as input to the VGIC. We therefore lower the line
+ * level here, so that we can take new virtual IRQs. See
+ * vgic_v3_fold_lr_state for more info.
+ */
+ if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT))
+ irq->line_level = false;
+
/*
* We currently only support Group1 interrupts, which is a
* known defect. This needs to be addressed at some point.
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index b168a328a9e0..607cbbc27a1c 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -144,6 +144,29 @@ void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)
kfree(irq);
}
+/* Get the input level of a mapped IRQ directly from the physical GIC */
+bool vgic_get_phys_line_level(struct vgic_irq *irq)
+{
+ bool line_level;
+
+ BUG_ON(!irq->hw);
+
+ WARN_ON(irq_get_irqchip_state(irq->host_irq,
+ IRQCHIP_STATE_PENDING,
+ &line_level));
+ return line_level;
+}
+
+/* Set/Clear the physical active state */
+void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active)
+{
+
+ BUG_ON(!irq->hw);
+ WARN_ON(irq_set_irqchip_state(irq->host_irq,
+ IRQCHIP_STATE_ACTIVE,
+ active));
+}
+
/**
* kvm_vgic_target_oracle - compute the target vcpu for an irq
*
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index efbcf8f96f9c..d0787983a357 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -104,6 +104,11 @@ static inline bool irq_is_pending(struct vgic_irq *irq)
return irq->pending_latch || irq->line_level;
}
+static inline bool vgic_irq_is_mapped_level(struct vgic_irq *irq)
+{
+ return irq->config == VGIC_CONFIG_LEVEL && irq->hw;
+}
+
/*
* This struct provides an intermediate representation of the fields contained
* in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC
@@ -140,6 +145,8 @@ vgic_get_mmio_region(struct kvm_vcpu *vcpu, struct vgic_io_device *iodev,
struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu,
u32 intid);
void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq);
+bool vgic_get_phys_line_level(struct vgic_irq *irq);
+void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active);
bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq,
unsigned long flags);
void vgic_kick_vcpus(struct kvm *kvm);
--
2.14.2
^ permalink raw reply related
* [PATCH v7 3/8] KVM: arm/arm64: Don't cache the timer IRQ level
From: Christoffer Dall @ 2017-12-07 10:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171207105418.22428-1-christoffer.dall@linaro.org>
The timer was modeled after a strict idea of modelling an interrupt line
level in software, meaning that only transitions in the level needed to
be reported to the VGIC. This works well for the timer, because the
arch timer code is in complete control of the device and can track the
transitions of the line.
However, as we are about to support using the HW bit in the VGIC not
just for the timer, but also for VFIO which cannot track transitions of
the interrupt line, we have to decide on an interface for level
triggered mapped interrupts to the GIC, which both the timer and VFIO
can use.
VFIO only sees an asserting transition of the physical interrupt line,
and tells the VGIC when that happens. That means that part of the
interrupt flow is offloaded to the hardware.
To use the same interface for VFIO devices and the timer, we therefore
have to change the timer (we cannot change VFIO because it doesn't know
the details of the device it is assigning to a VM).
Luckily, changing the timer is simple, we just need to stop 'caching'
the line level, but instead let the VGIC know the state of the timer
every time there is a potential change in the line level, and when the
line level should be asserted from the timer ISR. The VGIC can ignore
extra notifications using its validate mechanism.
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
virt/kvm/arm/arch_timer.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 4151250ce8da..dd5aca05c500 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -99,11 +99,9 @@ static irqreturn_t kvm_arch_timer_handler(int irq, void *dev_id)
}
vtimer = vcpu_vtimer(vcpu);
- if (!vtimer->irq.level) {
- vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);
- if (kvm_timer_irq_can_fire(vtimer))
- kvm_timer_update_irq(vcpu, true, vtimer);
- }
+ vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);
+ if (kvm_timer_irq_can_fire(vtimer))
+ kvm_timer_update_irq(vcpu, true, vtimer);
if (unlikely(!irqchip_in_kernel(vcpu->kvm)))
kvm_vtimer_update_mask_user(vcpu);
@@ -324,12 +322,20 @@ static void kvm_timer_update_state(struct kvm_vcpu *vcpu)
struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
+ bool level;
if (unlikely(!timer->enabled))
return;
- if (kvm_timer_should_fire(vtimer) != vtimer->irq.level)
- kvm_timer_update_irq(vcpu, !vtimer->irq.level, vtimer);
+ /*
+ * The vtimer virtual interrupt is a 'mapped' interrupt, meaning part
+ * of its lifecycle is offloaded to the hardware, and we therefore may
+ * not have lowered the irq.level value before having to signal a new
+ * interrupt, but have to signal an interrupt every time the level is
+ * asserted.
+ */
+ level = kvm_timer_should_fire(vtimer);
+ kvm_timer_update_irq(vcpu, level, vtimer);
if (kvm_timer_should_fire(ptimer) != ptimer->irq.level)
kvm_timer_update_irq(vcpu, !ptimer->irq.level, ptimer);
--
2.14.2
^ permalink raw reply related
* [PATCH v7 2/8] KVM: arm/arm64: Factor out functionality to get vgic mmio requester_vcpu
From: Christoffer Dall @ 2017-12-07 10:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171207105418.22428-1-christoffer.dall@linaro.org>
We are about to distinguish between userspace accesses and mmio traps
for a number of the mmio handlers. When the requester vcpu is NULL, it
means we are handling a userspace access.
Factor out the functionality to get the request vcpu into its own
function, mostly so we have a common place to document the semantics of
the return value.
Also take the chance to move the functionality outside of holding a
spinlock and instead explicitly disable and enable preemption. This
supports PREEMPT_RT kernels as well.
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
virt/kvm/arm/vgic/vgic-mmio.c | 44 +++++++++++++++++++++++++++----------------
1 file changed, 28 insertions(+), 16 deletions(-)
diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
index deb51ee16a3d..fdad95f62fa3 100644
--- a/virt/kvm/arm/vgic/vgic-mmio.c
+++ b/virt/kvm/arm/vgic/vgic-mmio.c
@@ -122,6 +122,27 @@ unsigned long vgic_mmio_read_pending(struct kvm_vcpu *vcpu,
return value;
}
+/*
+ * This function will return the VCPU that performed the MMIO access and
+ * trapped from within the VM, and will return NULL if this is a userspace
+ * access.
+ *
+ * We can disable preemption locally around accessing the per-CPU variable,
+ * and use the resolved vcpu pointer after enabling preemption again, because
+ * even if the current thread is migrated to another CPU, reading the per-CPU
+ * value later will give us the same value as we update the per-CPU variable
+ * in the preempt notifier handlers.
+ */
+static struct kvm_vcpu *vgic_get_mmio_requester_vcpu(void)
+{
+ struct kvm_vcpu *vcpu;
+
+ preempt_disable();
+ vcpu = kvm_arm_get_running_vcpu();
+ preempt_enable();
+ return vcpu;
+}
+
void vgic_mmio_write_spending(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val)
@@ -184,24 +205,10 @@ unsigned long vgic_mmio_read_active(struct kvm_vcpu *vcpu,
static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
bool new_active_state)
{
- struct kvm_vcpu *requester_vcpu;
unsigned long flags;
- spin_lock_irqsave(&irq->irq_lock, flags);
+ struct kvm_vcpu *requester_vcpu = vgic_get_mmio_requester_vcpu();
- /*
- * The vcpu parameter here can mean multiple things depending on how
- * this function is called; when handling a trap from the kernel it
- * depends on the GIC version, and these functions are also called as
- * part of save/restore from userspace.
- *
- * Therefore, we have to figure out the requester in a reliable way.
- *
- * When accessing VGIC state from user space, the requester_vcpu is
- * NULL, which is fine, because we guarantee that no VCPUs are running
- * when accessing VGIC state from user space so irq->vcpu->cpu is
- * always -1.
- */
- requester_vcpu = kvm_arm_get_running_vcpu();
+ spin_lock_irqsave(&irq->irq_lock, flags);
/*
* If this virtual IRQ was written into a list register, we
@@ -213,6 +220,11 @@ static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
* vgic_change_active_prepare) and still has to sync back this IRQ,
* so we release and re-acquire the spin_lock to let the other thread
* sync back the IRQ.
+ *
+ * When accessing VGIC state from user space, requester_vcpu is
+ * NULL, which is fine, because we guarantee that no VCPUs are running
+ * when accessing VGIC state from user space so irq->vcpu->cpu is
+ * always -1.
*/
while (irq->vcpu && /* IRQ may have state in an LR somewhere */
irq->vcpu != requester_vcpu && /* Current thread is not the VCPU thread */
--
2.14.2
^ permalink raw reply related
* [PATCH v7 1/8] KVM: arm/arm64: Remove redundant preemptible checks
From: Christoffer Dall @ 2017-12-07 10:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171207105418.22428-1-christoffer.dall@linaro.org>
The __this_cpu_read() and __this_cpu_write() functions already implement
checks for the required preemption levels when using
CONFIG_DEBUG_PREEMPT which gives you nice error messages and such.
Therefore there is no need to explicitly check this using a BUG_ON() in
the code (which we don't do for other uses of per cpu variables either).
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
virt/kvm/arm/arm.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index a6524ff27de4..859ff7e3a1eb 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -71,7 +71,6 @@ static DEFINE_PER_CPU(unsigned char, kvm_arm_hardware_enabled);
static void kvm_arm_set_running_vcpu(struct kvm_vcpu *vcpu)
{
- BUG_ON(preemptible());
__this_cpu_write(kvm_arm_running_vcpu, vcpu);
}
@@ -81,7 +80,6 @@ static void kvm_arm_set_running_vcpu(struct kvm_vcpu *vcpu)
*/
struct kvm_vcpu *kvm_arm_get_running_vcpu(void)
{
- BUG_ON(preemptible());
return __this_cpu_read(kvm_arm_running_vcpu);
}
--
2.14.2
^ permalink raw reply related
* [PATCH v7 0/8] Handle forwarded level-triggered interrupts
From: Christoffer Dall @ 2017-12-07 10:54 UTC (permalink / raw)
To: linux-arm-kernel
This series is an alternative approach to Eric Auger's direct EOI setup
patches [1] in terms of the KVM VGIC support.
The idea is to maintain existing semantics for the VGIC for mapped
level-triggered IRQs and also support the timer using mapped IRQs with
the same VGIC support as VFIO interrupts.
Based on v4.15-rc1.
Also available at:
git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git level-mapped-v7
Changes since v6:
- Removed double semi-colon
- Changed another confusing conditional in patch 6
- Fixed typos in commit message and comments
Changes since v5:
- Rebased on v4.15-rc1
- Changed comment on preemption code as suggested by Andre
- Fixed white space and confusing conditionals as suggested by Drew
Changes since v4:
- Rebased on the timer optimization series merged in the v4.15 merge
window, which caused a fair amount of modifications to patch 3.
- Added a static key to disable the sync operations when no VMs are
using userspace irqchips to further optimize the performance
- Fixed extra semicolon in vgic-mmio.c
- Added commentary as requested during review
- Dropped what was patch 4, because it was merged as part of GICv4
support.
- Factored out the VGIC input level function change as separate patch
(helps bisect and debugging), before providing a function for the
timer.
Changes since v3:
- Added a number of patches and moved patches around a bit.
- Check for uaccesses in the mmio handler functions
- Fixed bugs in the mmio handler functions
Changes since v2:
- Removed patch 5 from v2 and integrating the changes in what's now
patch 5 to make it easier to reuse code when adding VFIO integration.
- Changed the virtual distributor MMIO handling to use the
pending_latch and more closely match the semantics of SPENDR and
CPENDR for both level and edge mapped interrupts.
Changes since v1:
- Added necessary changes to the timer (Patch 1)
- Added handling of guest MMIO accesses to the virtual distributor
(Patch 4)
- Addressed Marc's comments from the initial RFC (mostly renames)
Thanks,
-Christoffer
[1]: https://lists.cs.columbia.edu/pipermail/kvmarm/2017-June/026072.html
Christoffer Dall (8):
KVM: arm/arm64: Remove redundant preemptible checks
KVM: arm/arm64: Factor out functionality to get vgic mmio
requester_vcpu
KVM: arm/arm64: Don't cache the timer IRQ level
KVM: arm/arm64: vgic: Support level-triggered mapped interrupts
KVM: arm/arm64: Support a vgic interrupt line level sample function
KVM: arm/arm64: Support VGIC dist pend/active changes for mapped IRQs
KVM: arm/arm64: Provide a get_input_level for the arch timer
KVM: arm/arm64: Avoid work when userspace iqchips are not used
include/kvm/arm_arch_timer.h | 2 +
include/kvm/arm_vgic.h | 13 ++++-
virt/kvm/arm/arch_timer.c | 105 +++++++++++++++++++++-----------------
virt/kvm/arm/arm.c | 2 -
virt/kvm/arm/vgic/vgic-mmio.c | 115 ++++++++++++++++++++++++++++++++++--------
virt/kvm/arm/vgic/vgic-v2.c | 29 +++++++++++
virt/kvm/arm/vgic/vgic-v3.c | 29 +++++++++++
virt/kvm/arm/vgic/vgic.c | 42 +++++++++++++--
virt/kvm/arm/vgic/vgic.h | 8 +++
9 files changed, 270 insertions(+), 75 deletions(-)
--
2.14.2
^ permalink raw reply
* [PATCH v5 08/16] arm64: Add vmap_stack header file
From: Mark Rutland @ 2017-12-07 10:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171206190142.9246-9-james.morse@arm.com>
Hi James,
On Wed, Dec 06, 2017 at 07:01:34PM +0000, James Morse wrote:
> Today the arm64 arch code allocates an extra IRQ stack per-cpu. If we
> also have SDEI and VMAP stacks we need two extra per-cpu VMAP stacks.
>
> Move the VMAP stack allocation out to a helper in a new header file.
> This avoids missing THREADINFO_GFP, or getting the all-important alignment
> wrong.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> Changes since v4:
> * Added gfp.h include
>
> Changes since v3:
> * Added BUILD_BUG() instead of a spooky link error
>
> arch/arm64/include/asm/vmap_stack.h | 26 ++++++++++++++++++++++++++
> arch/arm64/kernel/irq.c | 13 ++-----------
> 2 files changed, 28 insertions(+), 11 deletions(-)
> create mode 100644 arch/arm64/include/asm/vmap_stack.h
>
> diff --git a/arch/arm64/include/asm/vmap_stack.h b/arch/arm64/include/asm/vmap_stack.h
> new file mode 100644
> index 000000000000..5465e4e65987
> --- /dev/null
> +++ b/arch/arm64/include/asm/vmap_stack.h
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2017 Arm Ltd.
> +#ifndef __ASM_VMAP_STACK_H
> +#define __ASM_VMAP_STACK_H
> +
> +#include <linux/gfp.h>
> +#include <linux/vmalloc.h>
> +#include <asm/memory.h>
> +#include <asm/pgtable.h>
> +#include <asm/thread_info.h>
I think we also need:
#include <linux/bug.h> // for BUILD_BUG_ON()
#incldue <linux/kconfig.h> // for IS_ENABLED()
Otherwise, this looks good to me. With those includes folded in:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
> +
> +/*
> + * To ensure that VMAP'd stack overflow detection works correctly, all VMAP'd
> + * stacks need to have the same alignment.
> + */
> +static inline unsigned long *arch_alloc_vmap_stack(size_t stack_size, int node)
> +{
> + BUILD_BUG_ON(!IS_ENABLED(CONFIG_VMAP_STACK));
> +
> + return __vmalloc_node_range(stack_size, THREAD_ALIGN,
> + VMALLOC_START, VMALLOC_END,
> + THREADINFO_GFP, PAGE_KERNEL, 0, node,
> + __builtin_return_address(0));
> +}
> +
> +#endif /* __ASM_VMAP_STACK_H */
> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
> index 713561e5bcab..60e5fc661f74 100644
> --- a/arch/arm64/kernel/irq.c
> +++ b/arch/arm64/kernel/irq.c
> @@ -29,6 +29,7 @@
> #include <linux/irqchip.h>
> #include <linux/seq_file.h>
> #include <linux/vmalloc.h>
> +#include <asm/vmap_stack.h>
>
> unsigned long irq_err_count;
>
> @@ -58,17 +59,7 @@ static void init_irq_stacks(void)
> unsigned long *p;
>
> for_each_possible_cpu(cpu) {
> - /*
> - * To ensure that VMAP'd stack overflow detection works
> - * correctly, the IRQ stacks need to have the same
> - * alignment as other stacks.
> - */
> - p = __vmalloc_node_range(IRQ_STACK_SIZE, THREAD_ALIGN,
> - VMALLOC_START, VMALLOC_END,
> - THREADINFO_GFP, PAGE_KERNEL,
> - 0, cpu_to_node(cpu),
> - __builtin_return_address(0));
> -
> + p = arch_alloc_vmap_stack(IRQ_STACK_SIZE, cpu_to_node(cpu));
> per_cpu(irq_stack_ptr, cpu) = p;
> }
> }
> --
> 2.15.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [kernel-hardening][PATCH v5 2/3] arm: mm: dump: make the page table dumping seq_file optional
From: Jinbum Park @ 2017-12-07 10:53 UTC (permalink / raw)
To: linux-arm-kernel
This patch makes the page table dumping seq_file optional.
It makes the page table dumping code usable for other cases.
This patch refers below commit of arm64.
(ae5d1cf358a5
("arm64: dump: Make the page table dumping seq_file optional"))
Tested-by: Laura Abbott <labbott@redhat.com>
Reviewed-by: Laura Abbott <labbott@redhat.com>
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Jinbum Park <jinb.park7@gmail.com>
---
v5: No changes
(Just add Tested-by, Reviewed-by from Laura Abbott)
---
arch/arm/mm/dump.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mm/dump.c b/arch/arm/mm/dump.c
index 8dfe7c3..43a2bee 100644
--- a/arch/arm/mm/dump.c
+++ b/arch/arm/mm/dump.c
@@ -34,6 +34,18 @@
{ -1, NULL },
};
+#define pt_dump_seq_printf(m, fmt, args...) \
+({ \
+ if (m) \
+ seq_printf(m, fmt, ##args); \
+})
+
+#define pt_dump_seq_puts(m, fmt) \
+({ \
+ if (m) \
+ seq_printf(m, fmt); \
+})
+
struct pg_state {
struct seq_file *seq;
const struct addr_marker *marker;
@@ -210,7 +222,7 @@ static void dump_prot(struct pg_state *st, const struct prot_bits *bits, size_t
s = bits->clear;
if (s)
- seq_printf(st->seq, " %s", s);
+ pt_dump_seq_printf(st->seq, " %s", s);
}
}
@@ -224,7 +236,7 @@ static void note_page(struct pg_state *st, unsigned long addr,
st->level = level;
st->current_prot = prot;
st->current_domain = domain;
- seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
+ pt_dump_seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
} else if (prot != st->current_prot || level != st->level ||
domain != st->current_domain ||
addr >= st->marker[1].start_address) {
@@ -232,7 +244,7 @@ static void note_page(struct pg_state *st, unsigned long addr,
unsigned long delta;
if (st->current_prot) {
- seq_printf(st->seq, "0x%08lx-0x%08lx ",
+ pt_dump_seq_printf(st->seq, "0x%08lx-0x%08lx ",
st->start_address, addr);
delta = (addr - st->start_address) >> 10;
@@ -240,17 +252,19 @@ static void note_page(struct pg_state *st, unsigned long addr,
delta >>= 10;
unit++;
}
- seq_printf(st->seq, "%9lu%c", delta, *unit);
+ pt_dump_seq_printf(st->seq, "%9lu%c", delta, *unit);
if (st->current_domain)
- seq_printf(st->seq, " %s", st->current_domain);
+ pt_dump_seq_printf(st->seq, " %s",
+ st->current_domain);
if (pg_level[st->level].bits)
dump_prot(st, pg_level[st->level].bits, pg_level[st->level].num);
- seq_printf(st->seq, "\n");
+ pt_dump_seq_printf(st->seq, "\n");
}
if (addr >= st->marker[1].start_address) {
st->marker++;
- seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
+ pt_dump_seq_printf(st->seq, "---[ %s ]---\n",
+ st->marker->name);
}
st->start_address = addr;
st->current_prot = prot;
--
1.9.1
^ permalink raw reply related
* [kernel-hardening][PATCH v5 1/3] arm: mm: dump: make page table dumping reusable
From: Jinbum Park @ 2017-12-07 10:53 UTC (permalink / raw)
To: linux-arm-kernel
This patch refactors the arm page table dumping code,
so multiple tables may be registered with the framework.
This patch refers below commits of arm64.
(4674fdb9f149 ("arm64: mm: dump: make page table dumping reusable"))
(4ddb9bf83349 ("arm64: dump: Make ptdump debugfs a separate option"))
Tested-by: Laura Abbott <labbott@redhat.com>
Reviewed-by: Laura Abbott <labbott@redhat.com>
Signed-off-by: Jinbum Park <jinb.park7@gmail.com>
---
v5: No changes
(Just add Tested-by, Reviewed-by from Laura Abbott)
---
arch/arm/Kconfig.debug | 6 +++-
arch/arm/include/asm/ptdump.h | 48 ++++++++++++++++++++++++++++++++
arch/arm/mm/Makefile | 3 +-
arch/arm/mm/dump.c | 65 +++++++++++++++++++------------------------
arch/arm/mm/ptdump_debugfs.c | 34 ++++++++++++++++++++++
5 files changed, 117 insertions(+), 39 deletions(-)
create mode 100644 arch/arm/include/asm/ptdump.h
create mode 100644 arch/arm/mm/ptdump_debugfs.c
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 17685e1..e7b94db 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -3,10 +3,14 @@ menu "Kernel hacking"
source "lib/Kconfig.debug"
-config ARM_PTDUMP
+config ARM_PTDUMP_CORE
+ def_bool n
+
+config ARM_PTDUMP_DEBUGFS
bool "Export kernel pagetable layout to userspace via debugfs"
depends on DEBUG_KERNEL
depends on MMU
+ select ARM_PTDUMP_CORE
select DEBUG_FS
---help---
Say Y here if you want to show the kernel pagetable layout in a
diff --git a/arch/arm/include/asm/ptdump.h b/arch/arm/include/asm/ptdump.h
new file mode 100644
index 0000000..3a6c0b7
--- /dev/null
+++ b/arch/arm/include/asm/ptdump.h
@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2014 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __ASM_PTDUMP_H
+#define __ASM_PTDUMP_H
+
+#ifdef CONFIG_ARM_PTDUMP_CORE
+
+#include <linux/mm_types.h>
+#include <linux/seq_file.h>
+
+struct addr_marker {
+ unsigned long start_address;
+ char *name;
+};
+
+struct ptdump_info {
+ struct mm_struct *mm;
+ const struct addr_marker *markers;
+ unsigned long base_addr;
+};
+
+void ptdump_walk_pgd(struct seq_file *s, struct ptdump_info *info);
+#ifdef CONFIG_ARM_PTDUMP_DEBUGFS
+int ptdump_debugfs_register(struct ptdump_info *info, const char *name);
+#else
+static inline int ptdump_debugfs_register(struct ptdump_info *info,
+ const char *name)
+{
+ return 0;
+}
+#endif /* CONFIG_ARM_PTDUMP_DEBUGFS */
+
+#endif /* CONFIG_ARM_PTDUMP_CORE */
+
+#endif /* __ASM_PTDUMP_H */
diff --git a/arch/arm/mm/Makefile b/arch/arm/mm/Makefile
index 01bcc33..28be5f4 100644
--- a/arch/arm/mm/Makefile
+++ b/arch/arm/mm/Makefile
@@ -13,7 +13,8 @@ obj-y += nommu.o
obj-$(CONFIG_ARM_MPU) += pmsa-v7.o
endif
-obj-$(CONFIG_ARM_PTDUMP) += dump.o
+obj-$(CONFIG_ARM_PTDUMP_CORE) += dump.o
+obj-$(CONFIG_ARM_PTDUMP_DEBUGFS) += ptdump_debugfs.o
obj-$(CONFIG_MODULES) += proc-syms.o
obj-$(CONFIG_DEBUG_VIRTUAL) += physaddr.o
diff --git a/arch/arm/mm/dump.c b/arch/arm/mm/dump.c
index fc3b440..8dfe7c3 100644
--- a/arch/arm/mm/dump.c
+++ b/arch/arm/mm/dump.c
@@ -21,11 +21,7 @@
#include <asm/fixmap.h>
#include <asm/memory.h>
#include <asm/pgtable.h>
-
-struct addr_marker {
- unsigned long start_address;
- const char *name;
-};
+#include <asm/ptdump.h>
static struct addr_marker address_markers[] = {
{ MODULES_VADDR, "Modules" },
@@ -335,50 +331,36 @@ static void walk_pud(struct pg_state *st, pgd_t *pgd, unsigned long start)
}
}
-static void walk_pgd(struct seq_file *m)
+static void walk_pgd(struct pg_state *st, struct mm_struct *mm,
+ unsigned long start)
{
- pgd_t *pgd = swapper_pg_dir;
- struct pg_state st;
- unsigned long addr;
+ pgd_t *pgd = pgd_offset(mm, 0UL);
unsigned i;
-
- memset(&st, 0, sizeof(st));
- st.seq = m;
- st.marker = address_markers;
+ unsigned long addr;
for (i = 0; i < PTRS_PER_PGD; i++, pgd++) {
- addr = i * PGDIR_SIZE;
+ addr = start + i * PGDIR_SIZE;
if (!pgd_none(*pgd)) {
- walk_pud(&st, pgd, addr);
+ walk_pud(st, pgd, addr);
} else {
- note_page(&st, addr, 1, pgd_val(*pgd), NULL);
+ note_page(st, addr, 1, pgd_val(*pgd), NULL);
}
}
-
- note_page(&st, 0, 0, 0, NULL);
}
-static int ptdump_show(struct seq_file *m, void *v)
+void ptdump_walk_pgd(struct seq_file *m, struct ptdump_info *info)
{
- walk_pgd(m);
- return 0;
-}
+ struct pg_state st = {
+ .seq = m,
+ .marker = info->markers,
+ };
-static int ptdump_open(struct inode *inode, struct file *file)
-{
- return single_open(file, ptdump_show, NULL);
+ walk_pgd(&st, info->mm, info->base_addr);
+ note_page(&st, 0, 0, 0, NULL);
}
-static const struct file_operations ptdump_fops = {
- .open = ptdump_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
-
-static int ptdump_init(void)
+static void ptdump_initialize(void)
{
- struct dentry *pe;
unsigned i, j;
for (i = 0; i < ARRAY_SIZE(pg_level); i++)
@@ -387,9 +369,18 @@ static int ptdump_init(void)
pg_level[i].mask |= pg_level[i].bits[j].mask;
address_markers[2].start_address = VMALLOC_START;
+}
- pe = debugfs_create_file("kernel_page_tables", 0400, NULL, NULL,
- &ptdump_fops);
- return pe ? 0 : -ENOMEM;
+static struct ptdump_info kernel_ptdump_info = {
+ .mm = &init_mm,
+ .markers = address_markers,
+ .base_addr = 0,
+};
+
+static int ptdump_init(void)
+{
+ ptdump_initialize();
+ return ptdump_debugfs_register(&kernel_ptdump_info,
+ "kernel_page_tables");
}
__initcall(ptdump_init);
diff --git a/arch/arm/mm/ptdump_debugfs.c b/arch/arm/mm/ptdump_debugfs.c
new file mode 100644
index 0000000..be8d87b
--- /dev/null
+++ b/arch/arm/mm/ptdump_debugfs.c
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+
+#include <asm/ptdump.h>
+
+static int ptdump_show(struct seq_file *m, void *v)
+{
+ struct ptdump_info *info = m->private;
+
+ ptdump_walk_pgd(m, info);
+ return 0;
+}
+
+static int ptdump_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, ptdump_show, inode->i_private);
+}
+
+static const struct file_operations ptdump_fops = {
+ .open = ptdump_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+int ptdump_debugfs_register(struct ptdump_info *info, const char *name)
+{
+ struct dentry *pe;
+
+ pe = debugfs_create_file(name, 0400, NULL, info, &ptdump_fops);
+ return pe ? 0 : -ENOMEM;
+
+}
--
1.9.1
^ permalink raw reply related
* [kernel-hardening][PATCH v5 0/3] arm: Makes ptdump resuable and add WX page checking
From: Jinbum Park @ 2017-12-07 10:52 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Page table dumping code for arm64-x86 is reusable,
and they have function for WX page checking.
But arm doesn't have that.
This path series are to makes ptdump reusable,
and add WX page checking for arm.
This is heavily based on arm64 version.
v2 :
Fix a sender name of mail header, there was an mistake.
(from "jinb.park" to Jinbum Park)
Contents of patch-set are perfectly same.
v3 :
Take advantage of the existing pg_level and bits arrays
to check ro, nx prot.
v4 :
Add boolean for ro_bit, nx_bit into prot_bits
to point ro_bit, nx_bit in pg_level.
This change is suggested by Laura Abbott.
v5 :
No changes of code.
Just add Tested-by, Reviewed-by from Laura Abbott.
jinb.park (3):
arm: mm: dump: make page table dumping reusable
arm: mm: dump: make the page table dumping seq_file optional
arm: mm: dump: add checking for writable and executable pages
arch/arm/Kconfig.debug | 33 ++++++++-
arch/arm/include/asm/ptdump.h | 56 ++++++++++++++++
arch/arm/mm/Makefile | 3 +-
arch/arm/mm/dump.c | 151 +++++++++++++++++++++++++++++-------------
arch/arm/mm/init.c | 2 +
arch/arm/mm/ptdump_debugfs.c | 34 ++++++++++
6 files changed, 232 insertions(+), 47 deletions(-)
create mode 100644 arch/arm/include/asm/ptdump.h
create mode 100644 arch/arm/mm/ptdump_debugfs.c
--
1.9.1
^ permalink raw reply
* [PATCH] i2c: stm32: Fixes multibyte transfer for STM32F4 I2C controller
From: Wolfram Sang @ 2017-12-07 10:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFvLkMTsgxpjoGRgq7J0NFkT=YsOTYL6smkVhOPcfD=26Hzutg@mail.gmail.com>
On Tue, Oct 24, 2017 at 01:45:43PM +0200, Rados?aw Pietrzyk wrote:
> I'm afraid that didn't help and the problem still exists even with
> those patches applied.
So, my reading is: There is an issue which needs to be investigated?
Does applying the patch make sense until the issue is fully understood?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171207/c3624399/attachment.sig>
^ permalink raw reply
* [PATCH v3 08/15] drm/sun4i: Add LVDS support
From: Maxime Ripard @ 2017-12-07 10:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOFm3uEpnBFUDBHK+SqQ-pGUPcWbGXNrWum4HRJ34tMtoX-2Qw@mail.gmail.com>
Hi,
On Thu, Dec 07, 2017 at 11:14:27AM +0100, Philippe Ombredanne wrote:
> On Tue, Dec 5, 2017 at 4:10 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > The TCON supports the LVDS interface to output to a panel or a bridge.
> > Let's add support for it.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> []
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sun4i/sun4i_lvds.c
> > @@ -0,0 +1,183 @@
> > +/*
> > + * Copyright (C) 2015 NextThing Co
> > + * Copyright (C) 2015-2017 Free Electrons
> > + *
> > + * Maxime Ripard <maxime.ripard@free-electrons.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + */
>
> Would you consider using the new SPDX ids instead of this fine legalese?
> e.g. this as the top line:
>
> // SPDX-License-Identifier: GPL-2.0+
I did, and then forgot about it.
This will be in my next iteration, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171207/af641469/attachment.sig>
^ permalink raw reply
* [PATCH v5 15/30] arm64/sve: Signal handling support
From: Will Deacon @ 2017-12-07 10:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGXu5jJgsAg1VBMbx=mV3ep4hzs+1G46Sow4eeFqCK31b_sORA@mail.gmail.com>
Hi Kees,
On Wed, Dec 06, 2017 at 11:56:50AM -0800, Kees Cook wrote:
> On Tue, Oct 31, 2017 at 8:51 AM, Dave Martin <Dave.Martin@arm.com> wrote:
> > Miscellaneous:
> >
> > * Change inconsistent copy_to_user() calls to __copy_to_user() in
> > preserve_sve_context().
> >
> > There are already __put_user_error() calls here.
> >
> > The whole extended signal frame is already checked for
> > access_ok(VERIFY_WRITE) in get_sigframe().
>
> Verifying all these __copy_to/from_user() calls is rather non-trivial.
> For example, I had to understand that the access_ok() check actually
> spans memory that both user->sigframe and user->next_frame point into.
I don't think that's particularly difficult -- you just have to read the
four lines preceding the access_ok.
> And it isn't clear to me that all users of apply_user_offset() are
> within this range too, along with other manually calculated offsets in
> setup_sigframe().
The offsets passed into apply_user_offset are calculated by
setup_sigframe_layout as the stack is allocated, so they're correct by
construction. We could add a size check in apply_user_offset if you like?
> And it's not clear if parse_user_sigframe() is safe either. Are
> user->fpsimd and user->sve checked somewhere? It seems like it's
> safely contained by in sf->uc.uc_mcontext.__reserved, but it's hard to
> read, though I do see access_ok() checks against __reserved at the end
> of the while loop.
This one is certainly more difficult to follow, mainly because it's spread
about a bit and we have to check the extra context separately. However, the
main part of the frame is checked in sys_rt_sigreturn before calling
restore_sigframe, and the extra context is checked in parse_user_sigframe
if we find it.
Dave, any thoughts on making this easier to understand?
Will
^ permalink raw reply
* [PATCH] Fix Bug for cadence i2c interrupt overrunning buffer
From: Wolfram Sang @ 2017-12-07 10:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFcVECLWwo+OxSGAGYJfMSesjNT51X+Cwk+E_-RQk7gsEXy2YQ@mail.gmail.com>
> > I attach a patch of the changes to the i2c driver showing my debug in
> > this driver.
> > As I said I am happy to send more verbose bug output - I have about 36
> > odd runs with 9 occurences of the bug.
> > Once it happen twice in the one run
> >
> > I include the crashing line from the runs where the bug occured (I
> > added more diagnostics as the runs progressed):
>
> I'll check with out HW team as well and check your flow to
> see if we can reproduce it; will also check if the existing errata
> has larger scope.
> I'll let you know if we require further assistance with debug
> next week.
Any news here?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171207/bb2a5f6f/attachment.sig>
^ permalink raw reply
* [PATCH v2] mfd: stm32: Adopt SPDX identifier
From: Lee Jones @ 2017-12-07 10:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171205152418.28104-1-benjamin.gaignard@st.com>
On Tue, 05 Dec 2017, Benjamin Gaignard wrote:
> Add SPDX identifier
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
> drivers/mfd/stm32-lptimer.c | 6 +-----
> drivers/mfd/stm32-timers.c | 4 +---
> include/linux/mfd/stm32-lptimer.h | 6 +-----
> include/linux/mfd/stm32-timers.h | 4 +---
> 4 files changed, 4 insertions(+), 16 deletions(-)
Applied, thanks.
--
Lee Jones
Linaro Services Technical Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [RFC v3 PATCH 0/2] Introduce Security Version to EFI Stub
From: Ingo Molnar @ 2017-12-07 10:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171207102749.cv7ybllv2vi7o7hw@GaryWorkstation>
* Gary Lin <glin@suse.com> wrote:
> On Thu, Dec 07, 2017 at 09:18:16AM +0100, Ingo Molnar wrote:
> >
> > * Gary Lin <glin@suse.com> wrote:
> >
> > > On Thu, Dec 07, 2017 at 07:09:27AM +0100, Ingo Molnar wrote:
> > > >
> > > > * Gary Lin <glin@suse.com> wrote:
> > > >
> > > > > On Wed, Dec 06, 2017 at 07:37:34PM +0100, Ingo Molnar wrote:
> > > > > >
> > > > > > * Gary Lin <glin@suse.com> wrote:
> > > > > >
> > > > > > > On Tue, Dec 05, 2017 at 04:14:26PM -0500, Josh Boyer wrote:
> > > > > > > > On Tue, Dec 5, 2017 at 5:01 AM, Gary Lin <glin@suse.com> wrote:
> > > > > > > > > The series of patches introduce Security Version to EFI stub.
> > > > > > > > >
> > > > > > > > > Security Version is a monotonically increasing number and designed to
> > > > > > > > > prevent the user from loading an insecure kernel accidentally. The
> > > > > > > > > bootloader maintains a list of security versions corresponding to
> > > > > > > > > different distributions. After fixing a critical vulnerability, the
> > > > > > > > > distribution kernel maintainer bumps the "version", and the bootloader
> > > > > > > > > updates the list automatically. When the user tries to load a kernel
> > > > > > > > > with a lower security version, the bootloader shows a warning prompt
> > > > > > > > > to notify the user the potential risk.
> > > > > > > >
> > > > > > > > If a distribution releases a kernel with a higher security version and
> > > > > > > > that it automatically updated on boot, what happens if that kernel
> > > > > > > > contains a different bug that causes it to fail to boot or break
> > > > > > > > critical functionality? At that point, the user's machine would be in
> > > > > > > > a state where the higher security version is enforced but the only
> > > > > > > > kernel that provides that is broken. Wouldn't that make a bad
> > > > > > > > situation even worse by now requiring manual acceptance of the older
> > > > > > > > SV kernel boot physically at the machine?
> > > > > > > >
> > > > > > > > I feel like I'm missing a detail here or something.
> > > > > > > >
> > > > > > > If the new kernel fails to boot, then the user has to choose the kernel
> > > > > > > manually anyway, and there will be an option in the warning prompt to
> > > > > > > lower SV.
> > > > > >
> > > > > > And what if the firmware does not support a lowering of the SV?
> > > > > >
> > > > > The SV list is manipulated by the bootloader, and the firmware only
> > > > > provides the interface to the storage, i.e. non-volatile flash.
> > > >
> > > > What about systems where the bootloader is part of the system and users only have
> > > > the ability to provide kernel images, but no ability to change the boot loader?
> > >
> > > It depends on how the bootloader works. If the system uses my
> > > implementation of shim loader, it surely has the ability to lower SV,
> > > but it requires physical access on purpose.
> >
> > And that's my problem: if in practice the bootloader is 'part of the system', is
> > signed and is updated like the firmware, then putting a "Security Version" into
> > the kernel image and architecting a boot protocol for a monotonic method for the
> > bootloader to restrict the loading of kernel images is an obviously bad idea.
> >
> Even though the bootloader doesn't actually block the booting?
We don't know that for sure, in that scenario *how* the bootloader interprets the
SV is not under the user's control...
Thanks,
Ingo
^ permalink raw reply
* [PATCH v6 1/5] scsi: ufs: add Hisilicon ufs driver code
From: Philippe Ombredanne @ 2017-12-07 10:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171207102026.14977-2-liwei213@huawei.com>
Dear Li,
On Thu, Dec 7, 2017 at 11:20 AM, Li Wei <liwei213@huawei.com> wrote:
> add Hisilicon ufs driver code.
>
> Signed-off-by: Li Wei <liwei213@huawei.com>
> Signed-off-by: Geng Jianfeng <gengjianfeng@hisilicon.com>
> Signed-off-by: Zang Leigang <zangleigang@hisilicon.com>
> Signed-off-by: Yu Jianfeng <steven.yujianfeng@hisilicon.com>
[]
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-hisi.c
> @@ -0,0 +1,624 @@
> +/*
> + *
> + * HiSilicon Hixxxx UFS Driver
> + *
> + * Copyright (c) 2016-2017 Linaro Ltd.
> + * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
Would you consider using the new SPDX license ids instead?
Check Thomas doc patches for instructions.
This would be great and while you are it may be this could be adopted
for all HiSilicon and Huawei past, present and future contributions?
Thank you for your kind consideration!
--
Cordially
Philippe Ombredanne
^ permalink raw reply
* [PATCH 2/3] pinctrl: sunxi: Disable strict mode for old pinctrl drivers
From: Ludovic Desroches @ 2017-12-07 10:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b09cdfbf4c7546f299031c2923b9f8fcef881e3d.1507236823.git-series.maxime.ripard@free-electrons.com>
Hi,
On Thu, Oct 05, 2017 at 10:54:07PM +0200, Maxime Ripard wrote:
> Old pinctrl drivers will need to disable strict mode for various reasons,
> among which:
> - Some DT will still have a pinctrl group for each GPIO used, which will
> be rejected by pin_request. While we could remove those nodes, we still
> have to deal with old DTs.
> - Some GPIOs on these boards need to have their pin configuration changed
> (for bias or current), and there's no clear migration path
>
> Let's disable the strict mode on those SoCs so that there's no breakage.
>
I revive this thread since I am facing the same issue. Did you find a way to
deal with it? I have not seen a new version of this set of patches, I may have
missed it.
Why you didn't choose to add an optionnal property to enforce the use of
the strict mode? I mean, with your solution, if there is a new DT with
an old pin controller, it won't benefit from the strict mode.
Ludovic
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c | 1 +
> drivers/pinctrl/sunxi/pinctrl-sun5i.c | 1 +
> drivers/pinctrl/sunxi/pinctrl-sun6i-a31.c | 1 +
> drivers/pinctrl/sunxi/pinctrl-sun8i-a23.c | 1 +
> drivers/pinctrl/sunxi/pinctrl-sun8i-a33.c | 1 +
> drivers/pinctrl/sunxi/pinctrl-sun8i-h3.c | 3 ++-
> drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c | 1 +
> 7 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c b/drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c
> index f763d8d62d6e..295e48fc94bc 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c
> @@ -1289,6 +1289,7 @@ static const struct sunxi_pinctrl_desc sun4i_a10_pinctrl_data = {
> .npins = ARRAY_SIZE(sun4i_a10_pins),
> .irq_banks = 1,
> .irq_read_needs_mux = true,
> + .disable_strict_mode = true,
> };
>
> static int sun4i_a10_pinctrl_probe(struct platform_device *pdev)
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun5i.c b/drivers/pinctrl/sunxi/pinctrl-sun5i.c
> index 47afd558b114..27ec99e81c4c 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sun5i.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun5i.c
> @@ -713,6 +713,7 @@ static const struct sunxi_pinctrl_desc sun5i_pinctrl_data = {
> .pins = sun5i_pins,
> .npins = ARRAY_SIZE(sun5i_pins),
> .irq_banks = 1,
> + .disable_strict_mode = true,
> };
>
> static int sun5i_pinctrl_probe(struct platform_device *pdev)
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun6i-a31.c b/drivers/pinctrl/sunxi/pinctrl-sun6i-a31.c
> index 951a25c18815..82ffaf466892 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sun6i-a31.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun6i-a31.c
> @@ -965,6 +965,7 @@ static const struct sunxi_pinctrl_desc sun6i_a31_pinctrl_data = {
> .pins = sun6i_a31_pins,
> .npins = ARRAY_SIZE(sun6i_a31_pins),
> .irq_banks = 4,
> + .disable_strict_mode = true,
> };
>
> static int sun6i_a31_pinctrl_probe(struct platform_device *pdev)
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun8i-a23.c b/drivers/pinctrl/sunxi/pinctrl-sun8i-a23.c
> index 721b6935baf3..402fd7d21e7b 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sun8i-a23.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun8i-a23.c
> @@ -563,6 +563,7 @@ static const struct sunxi_pinctrl_desc sun8i_a23_pinctrl_data = {
> .pins = sun8i_a23_pins,
> .npins = ARRAY_SIZE(sun8i_a23_pins),
> .irq_banks = 3,
> + .disable_strict_mode = true,
> };
>
> static int sun8i_a23_pinctrl_probe(struct platform_device *pdev)
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun8i-a33.c b/drivers/pinctrl/sunxi/pinctrl-sun8i-a33.c
> index ef1e0bef4099..da387211a75e 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sun8i-a33.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun8i-a33.c
> @@ -486,6 +486,7 @@ static const struct sunxi_pinctrl_desc sun8i_a33_pinctrl_data = {
> .npins = ARRAY_SIZE(sun8i_a33_pins),
> .irq_banks = 2,
> .irq_bank_base = 1,
> + .disable_strict_mode = true,
> };
>
> static int sun8i_a33_pinctrl_probe(struct platform_device *pdev)
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun8i-h3.c b/drivers/pinctrl/sunxi/pinctrl-sun8i-h3.c
> index 518a92df4418..d1719a738c20 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sun8i-h3.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun8i-h3.c
> @@ -491,7 +491,8 @@ static const struct sunxi_pinctrl_desc sun8i_h3_pinctrl_data = {
> .pins = sun8i_h3_pins,
> .npins = ARRAY_SIZE(sun8i_h3_pins),
> .irq_banks = 2,
> - .irq_read_needs_mux = true
> + .irq_read_needs_mux = true,
> + .disable_strict_mode = true,
> };
>
> static int sun8i_h3_pinctrl_probe(struct platform_device *pdev)
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c b/drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c
> index bc14e954d7a2..472ef0d91b99 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c
> @@ -721,6 +721,7 @@ static const struct sunxi_pinctrl_desc sun9i_a80_pinctrl_data = {
> .pins = sun9i_a80_pins,
> .npins = ARRAY_SIZE(sun9i_a80_pins),
> .irq_banks = 5,
> + .disable_strict_mode = true,
> };
>
> static int sun9i_a80_pinctrl_probe(struct platform_device *pdev)
> --
> git-series 0.9.1
> --
> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [RFC v3 PATCH 0/2] Introduce Security Version to EFI Stub
From: Gary Lin @ 2017-12-07 10:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171207081816.jy2rw5y5iyxeqw6n@gmail.com>
On Thu, Dec 07, 2017 at 09:18:16AM +0100, Ingo Molnar wrote:
>
> * Gary Lin <glin@suse.com> wrote:
>
> > On Thu, Dec 07, 2017 at 07:09:27AM +0100, Ingo Molnar wrote:
> > >
> > > * Gary Lin <glin@suse.com> wrote:
> > >
> > > > On Wed, Dec 06, 2017 at 07:37:34PM +0100, Ingo Molnar wrote:
> > > > >
> > > > > * Gary Lin <glin@suse.com> wrote:
> > > > >
> > > > > > On Tue, Dec 05, 2017 at 04:14:26PM -0500, Josh Boyer wrote:
> > > > > > > On Tue, Dec 5, 2017 at 5:01 AM, Gary Lin <glin@suse.com> wrote:
> > > > > > > > The series of patches introduce Security Version to EFI stub.
> > > > > > > >
> > > > > > > > Security Version is a monotonically increasing number and designed to
> > > > > > > > prevent the user from loading an insecure kernel accidentally. The
> > > > > > > > bootloader maintains a list of security versions corresponding to
> > > > > > > > different distributions. After fixing a critical vulnerability, the
> > > > > > > > distribution kernel maintainer bumps the "version", and the bootloader
> > > > > > > > updates the list automatically. When the user tries to load a kernel
> > > > > > > > with a lower security version, the bootloader shows a warning prompt
> > > > > > > > to notify the user the potential risk.
> > > > > > >
> > > > > > > If a distribution releases a kernel with a higher security version and
> > > > > > > that it automatically updated on boot, what happens if that kernel
> > > > > > > contains a different bug that causes it to fail to boot or break
> > > > > > > critical functionality? At that point, the user's machine would be in
> > > > > > > a state where the higher security version is enforced but the only
> > > > > > > kernel that provides that is broken. Wouldn't that make a bad
> > > > > > > situation even worse by now requiring manual acceptance of the older
> > > > > > > SV kernel boot physically at the machine?
> > > > > > >
> > > > > > > I feel like I'm missing a detail here or something.
> > > > > > >
> > > > > > If the new kernel fails to boot, then the user has to choose the kernel
> > > > > > manually anyway, and there will be an option in the warning prompt to
> > > > > > lower SV.
> > > > >
> > > > > And what if the firmware does not support a lowering of the SV?
> > > > >
> > > > The SV list is manipulated by the bootloader, and the firmware only
> > > > provides the interface to the storage, i.e. non-volatile flash.
> > >
> > > What about systems where the bootloader is part of the system and users only have
> > > the ability to provide kernel images, but no ability to change the boot loader?
> >
> > It depends on how the bootloader works. If the system uses my
> > implementation of shim loader, it surely has the ability to lower SV,
> > but it requires physical access on purpose.
>
> And that's my problem: if in practice the bootloader is 'part of the system', is
> signed and is updated like the firmware, then putting a "Security Version" into
> the kernel image and architecting a boot protocol for a monotonic method for the
> bootloader to restrict the loading of kernel images is an obviously bad idea.
>
Even though the bootloader doesn't actually block the booting?
If the bootloader loads a kernel with lower SV, by default, the warning
prompt lasts for 30 seconds, and the system continues booting if the
user doesn't intervene. For the paranoid user, a variable can be set to
make the bootloader wait forever. Is it acceptable?
Thanks,
Gary Lin
^ 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