* Re: [PATCH v2 08/91] clk: bcm: rpi: Remove global pllb_arm clock pointer
From: Stephen Boyd @ 2020-05-27 6:48 UTC (permalink / raw)
To: Eric Anholt, Maxime Ripard, Nicolas Saenz Julienne
Cc: Tim Gover, Dave Stevenson, Michael Turquette, linux-kernel,
dri-devel, linux-clk, bcm-kernel-feedback-list, linux-rpi-kernel,
Phil Elwell, linux-arm-kernel, Maxime Ripard
In-Reply-To: <cf81ef7d0235e7f7fdc70e1628180bebe692e3a5.1587742492.git-series.maxime@cerno.tech>
Quoting Maxime Ripard (2020-04-24 08:33:49)
> The pllb_arm clk_hw pointer in the raspberry_clk structure isn't used
> anywhere but in the raspberrypi_register_pllb_arm.
>
> Let's remove it, this will make our lives easier in future patches.
>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: linux-clk@vger.kernel.org
> Acked-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH RFCv2 9/9] arm64: Support async page fault
From: Paolo Bonzini @ 2020-05-27 6:48 UTC (permalink / raw)
To: Gavin Shan, kvmarm
Cc: maz, linux-kernel, shan.gavin, catalin.marinas, will,
linux-arm-kernel
In-Reply-To: <20200508032919.52147-10-gshan@redhat.com>
Hi Gavin,
I definitely appreciate the work, but this is repeating most of the
mistakes done in the x86 implementation. In particular:
- the page ready signal can be done as an interrupt, rather than an
exception. This is because "page ready" can be handled asynchronously,
in contrast to "page not present" which must be done on the same
instruction that triggers it. You can refer to the recent series from
Vitaly Kuznetsov that switched "page ready" to an interrupt.
- the page not present is reusing the memory abort exception, and
there's really no reason to do so. I think it would be best if ARM
could reserve one ESR exception code for the hypervisor. Mark, any
ideas how to proceed here?
- for x86 we're also thinking of initiating the page fault from the
exception handler, rather than doing so from the hypervisor before
injecting the exception. If ARM leads the way here, we would do our
best to share code when x86 does the same.
- do not bother with using KVM_ASYNC_PF_SEND_ALWAYS, it's a fringe case
that adds a lot of complexity.
Also, please include me on further iterations of the series.
Thanks!
Paolo
On 08/05/20 05:29, Gavin Shan wrote:
> This supports asynchronous page fault for the guest. The design is
> similar to what x86 has: on receiving a PAGE_NOT_PRESENT signal from
> the host, the current task is either rescheduled or put into power
> saving mode. The task will be waken up when PAGE_READY signal is
> received. The PAGE_READY signal might be received in the context
> of the suspended process, to be waken up. That means the suspended
> process has to wake up itself, but it's not safe and prone to cause
> dead-lock on CPU runqueue lock. So the wakeup is delayed on returning
> from kernel space to user space or idle process is picked for running.
>
> The signals are conveyed through the async page fault control block,
> which was passed to host on enabling the functionality. On each page
> fault, the control block is checked and switch to the async page fault
> handling flow if any signals exist.
>
> The feature is put into the CONFIG_KVM_GUEST umbrella, which is added
> by this patch. So we have inline functions implemented in kvm_para.h,
> like other architectures do, to check if async page fault (one of the
> KVM para-virtualized features) is available. Also, the kernel boot
> parameter "no-kvmapf" can be specified to disable the feature.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
> arch/arm64/Kconfig | 11 +
> arch/arm64/include/asm/exception.h | 3 +
> arch/arm64/include/asm/kvm_para.h | 27 +-
> arch/arm64/kernel/entry.S | 33 +++
> arch/arm64/kernel/process.c | 4 +
> arch/arm64/mm/fault.c | 434 +++++++++++++++++++++++++++++
> 6 files changed, 505 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 40fb05d96c60..2d5e5ee62d6d 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1045,6 +1045,17 @@ config PARAVIRT
> under a hypervisor, potentially improving performance significantly
> over full virtualization.
>
> +config KVM_GUEST
> + bool "KVM Guest Support"
> + depends on PARAVIRT
> + default y
> + help
> + This option enables various optimizations for running under the KVM
> + hypervisor. Overhead for the kernel when not running inside KVM should
> + be minimal.
> +
> + In case of doubt, say Y
> +
> config PARAVIRT_TIME_ACCOUNTING
> bool "Paravirtual steal time accounting"
> select PARAVIRT
> diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
> index 7a6e81ca23a8..d878afa42746 100644
> --- a/arch/arm64/include/asm/exception.h
> +++ b/arch/arm64/include/asm/exception.h
> @@ -46,4 +46,7 @@ void bad_el0_sync(struct pt_regs *regs, int reason, unsigned int esr);
> void do_cp15instr(unsigned int esr, struct pt_regs *regs);
> void do_el0_svc(struct pt_regs *regs);
> void do_el0_svc_compat(struct pt_regs *regs);
> +#ifdef CONFIG_KVM_GUEST
> +void kvm_async_pf_delayed_wake(void);
> +#endif
> #endif /* __ASM_EXCEPTION_H */
> diff --git a/arch/arm64/include/asm/kvm_para.h b/arch/arm64/include/asm/kvm_para.h
> index 0ea481dd1c7a..b2f8ef243df7 100644
> --- a/arch/arm64/include/asm/kvm_para.h
> +++ b/arch/arm64/include/asm/kvm_para.h
> @@ -3,6 +3,20 @@
> #define _ASM_ARM_KVM_PARA_H
>
> #include <uapi/asm/kvm_para.h>
> +#include <linux/of.h>
> +#include <asm/hypervisor.h>
> +
> +#ifdef CONFIG_KVM_GUEST
> +static inline int kvm_para_available(void)
> +{
> + return 1;
> +}
> +#else
> +static inline int kvm_para_available(void)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_KVM_GUEST */
>
> static inline bool kvm_check_and_clear_guest_paused(void)
> {
> @@ -11,17 +25,16 @@ static inline bool kvm_check_and_clear_guest_paused(void)
>
> static inline unsigned int kvm_arch_para_features(void)
> {
> - return 0;
> + unsigned int features = 0;
> +
> + if (kvm_arm_hyp_service_available(ARM_SMCCC_KVM_FUNC_APF))
> + features |= (1 << KVM_FEATURE_ASYNC_PF);
> +
> + return features;
> }
>
> static inline unsigned int kvm_arch_para_hints(void)
> {
> return 0;
> }
> -
> -static inline bool kvm_para_available(void)
> -{
> - return false;
> -}
> -
> #endif /* _ASM_ARM_KVM_PARA_H */
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index ddcde093c433..15efd57129ff 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -751,12 +751,45 @@ finish_ret_to_user:
> enable_step_tsk x1, x2
> #ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> bl stackleak_erase
> +#endif
> +#ifdef CONFIG_KVM_GUEST
> + bl kvm_async_pf_delayed_wake
> #endif
> kernel_exit 0
> ENDPROC(ret_to_user)
>
> .popsection // .entry.text
>
> +#ifdef CONFIG_KVM_GUEST
> + .pushsection ".rodata", "a"
> +SYM_DATA_START(__exception_handlers_offset)
> + .quad 0
> + .quad 0
> + .quad 0
> + .quad 0
> + .quad el1_sync - vectors
> + .quad el1_irq - vectors
> + .quad 0
> + .quad el1_error - vectors
> + .quad el0_sync - vectors
> + .quad el0_irq - vectors
> + .quad 0
> + .quad el0_error - vectors
> +#ifdef CONFIG_COMPAT
> + .quad el0_sync_compat - vectors
> + .quad el0_irq_compat - vectors
> + .quad 0
> + .quad el0_error_compat - vectors
> +#else
> + .quad 0
> + .quad 0
> + .quad 0
> + .quad 0
> +#endif
> +SYM_DATA_END(__exception_handlers_offset)
> + .popsection // .rodata
> +#endif /* CONFIG_KVM_GUEST */
> +
> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> /*
> * Exception vectors trampoline.
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 56be4cbf771f..5e7ee553566d 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -53,6 +53,7 @@
> #include <asm/processor.h>
> #include <asm/pointer_auth.h>
> #include <asm/stacktrace.h>
> +#include <asm/exception.h>
>
> #if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR_PER_TASK)
> #include <linux/stackprotector.h>
> @@ -70,6 +71,9 @@ void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
>
> static void __cpu_do_idle(void)
> {
> +#ifdef CONFIG_KVM_GUEST
> + kvm_async_pf_delayed_wake();
> +#endif
> dsb(sy);
> wfi();
> }
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index c9cedc0432d2..cbf8b52135c9 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -19,10 +19,12 @@
> #include <linux/page-flags.h>
> #include <linux/sched/signal.h>
> #include <linux/sched/debug.h>
> +#include <linux/swait.h>
> #include <linux/highmem.h>
> #include <linux/perf_event.h>
> #include <linux/preempt.h>
> #include <linux/hugetlb.h>
> +#include <linux/kvm_para.h>
>
> #include <asm/acpi.h>
> #include <asm/bug.h>
> @@ -48,8 +50,31 @@ struct fault_info {
> const char *name;
> };
>
> +#ifdef CONFIG_KVM_GUEST
> +struct kvm_task_sleep_node {
> + struct hlist_node link;
> + struct swait_queue_head wq;
> + u32 token;
> + struct task_struct *task;
> + int cpu;
> + bool halted;
> + bool delayed;
> +};
> +
> +struct kvm_task_sleep_head {
> + raw_spinlock_t lock;
> + struct hlist_head list;
> +};
> +#endif /* CONFIG_KVM_GUEST */
> +
> static const struct fault_info fault_info[];
> static struct fault_info debug_fault_info[];
> +#ifdef CONFIG_KVM_GUEST
> +extern char __exception_handlers_offset[];
> +static bool async_pf_available = true;
> +static DEFINE_PER_CPU(struct kvm_vcpu_pv_apf_data, apf_data) __aligned(64);
> +static DEFINE_PER_CPU(struct kvm_task_sleep_head, apf_head);
> +#endif
>
> static inline const struct fault_info *esr_to_fault_info(unsigned int esr)
> {
> @@ -717,10 +742,281 @@ static const struct fault_info fault_info[] = {
> { do_bad, SIGKILL, SI_KERNEL, "unknown 63" },
> };
>
> +#ifdef CONFIG_KVM_GUEST
> +static inline unsigned int kvm_async_pf_read_enabled(void)
> +{
> + return __this_cpu_read(apf_data.enabled);
> +}
> +
> +static inline void kvm_async_pf_write_enabled(unsigned int val)
> +{
> + __this_cpu_write(apf_data.enabled, val);
> +}
> +
> +static inline unsigned int kvm_async_pf_read_reason(void)
> +{
> + return __this_cpu_read(apf_data.reason);
> +}
> +
> +static inline void kvm_async_pf_write_reason(unsigned int val)
> +{
> + __this_cpu_write(apf_data.reason, val);
> +}
> +
> +#define kvm_async_pf_lock(b, flags) \
> + raw_spin_lock_irqsave(&(b)->lock, (flags))
> +#define kvm_async_pf_trylock(b, flags) \
> + raw_spin_trylock_irqsave(&(b)->lock, (flags))
> +#define kvm_async_pf_unlock(b, flags) \
> + raw_spin_unlock_irqrestore(&(b)->lock, (flags))
> +#define kvm_async_pf_unlock_and_clear(b, flags) \
> + do { \
> + raw_spin_unlock_irqrestore(&(b)->lock, (flags)); \
> + kvm_async_pf_write_reason(0); \
> + } while (0)
> +
> +static struct kvm_task_sleep_node *kvm_async_pf_find(
> + struct kvm_task_sleep_head *b, u32 token)
> +{
> + struct kvm_task_sleep_node *n;
> + struct hlist_node *p;
> +
> + hlist_for_each(p, &b->list) {
> + n = hlist_entry(p, typeof(*n), link);
> + if (n->token == token)
> + return n;
> + }
> +
> + return NULL;
> +}
> +
> +static void kvm_async_pf_wait(u32 token, int in_kernel)
> +{
> + struct kvm_task_sleep_head *b = this_cpu_ptr(&apf_head);
> + struct kvm_task_sleep_node n, *e;
> + DECLARE_SWAITQUEUE(wait);
> + unsigned long flags;
> +
> + kvm_async_pf_lock(b, flags);
> + e = kvm_async_pf_find(b, token);
> + if (e) {
> + /* dummy entry exist -> wake up was delivered ahead of PF */
> + hlist_del(&e->link);
> + kfree(e);
> + kvm_async_pf_unlock_and_clear(b, flags);
> +
> + return;
> + }
> +
> + n.token = token;
> + n.task = current;
> + n.cpu = smp_processor_id();
> + n.halted = is_idle_task(current) ||
> + (IS_ENABLED(CONFIG_PREEMPT_COUNT) ?
> + preempt_count() > 1 || rcu_preempt_depth() : in_kernel);
> + n.delayed = false;
> + init_swait_queue_head(&n.wq);
> + hlist_add_head(&n.link, &b->list);
> + kvm_async_pf_unlock_and_clear(b, flags);
> +
> + for (;;) {
> + if (!n.halted) {
> + prepare_to_swait_exclusive(&n.wq, &wait,
> + TASK_UNINTERRUPTIBLE);
> + }
> +
> + if (hlist_unhashed(&n.link))
> + break;
> +
> + if (!n.halted) {
> + schedule();
> + } else {
> + dsb(sy);
> + wfi();
> + }
> + }
> +
> + if (!n.halted)
> + finish_swait(&n.wq, &wait);
> +}
> +
> +/*
> + * There are two cases the suspended processed can't be waken up
> + * immediately: The waker is exactly the suspended process, or
> + * the current CPU runqueue has been locked. Otherwise, we might
> + * run into dead-lock.
> + */
> +static inline void kvm_async_pf_wake_one(struct kvm_task_sleep_node *n)
> +{
> + if (n->task == current ||
> + cpu_rq_is_locked(smp_processor_id())) {
> + n->delayed = true;
> + return;
> + }
> +
> + hlist_del_init(&n->link);
> + if (n->halted)
> + smp_send_reschedule(n->cpu);
> + else
> + swake_up_one(&n->wq);
> +}
> +
> +void kvm_async_pf_delayed_wake(void)
> +{
> + struct kvm_task_sleep_head *b;
> + struct kvm_task_sleep_node *n;
> + struct hlist_node *p, *next;
> + unsigned int reason;
> + unsigned long flags;
> +
> + if (!kvm_async_pf_read_enabled())
> + return;
> +
> + /*
> + * We're running in the edging context, we need to complete
> + * the work as quick as possible. So we have a preliminary
> + * check without holding the lock.
> + */
> + b = this_cpu_ptr(&apf_head);
> + if (hlist_empty(&b->list))
> + return;
> +
> + /*
> + * Set the async page fault reason to something to avoid
> + * receiving the signals, which might cause lock contention
> + * and possibly dead-lock. As we're in guest context, it's
> + * safe to set the reason here.
> + *
> + * There might be pending signals. For that case, we needn't
> + * do anything. Otherwise, the pending signal will be lost.
> + */
> + reason = kvm_async_pf_read_reason();
> + if (!reason) {
> + kvm_async_pf_write_reason(KVM_PV_REASON_PAGE_NOT_PRESENT +
> + KVM_PV_REASON_PAGE_READY);
> + }
> +
> + if (!kvm_async_pf_trylock(b, flags))
> + goto done;
> +
> + hlist_for_each_safe(p, next, &b->list) {
> + n = hlist_entry(p, typeof(*n), link);
> + if (n->cpu != smp_processor_id())
> + continue;
> + if (!n->delayed)
> + continue;
> +
> + kvm_async_pf_wake_one(n);
> + }
> +
> + kvm_async_pf_unlock(b, flags);
> +
> +done:
> + if (!reason)
> + kvm_async_pf_write_reason(0);
> +}
> +NOKPROBE_SYMBOL(kvm_async_pf_delayed_wake);
> +
> +static void kvm_async_pf_wake_all(void)
> +{
> + struct kvm_task_sleep_head *b;
> + struct kvm_task_sleep_node *n;
> + struct hlist_node *p, *next;
> + unsigned long flags;
> +
> + b = this_cpu_ptr(&apf_head);
> + kvm_async_pf_lock(b, flags);
> +
> + hlist_for_each_safe(p, next, &b->list) {
> + n = hlist_entry(p, typeof(*n), link);
> + kvm_async_pf_wake_one(n);
> + }
> +
> + kvm_async_pf_unlock(b, flags);
> +
> + kvm_async_pf_write_reason(0);
> +}
> +
> +static void kvm_async_pf_wake(u32 token)
> +{
> + struct kvm_task_sleep_head *b = this_cpu_ptr(&apf_head);
> + struct kvm_task_sleep_node *n;
> + unsigned long flags;
> +
> + if (token == ~0) {
> + kvm_async_pf_wake_all();
> + return;
> + }
> +
> +again:
> + kvm_async_pf_lock(b, flags);
> +
> + n = kvm_async_pf_find(b, token);
> + if (!n) {
> + /*
> + * Async PF was not yet handled. Add dummy entry
> + * for the token. Busy wait until other CPU handles
> + * the async PF on allocation failure.
> + */
> + n = kzalloc(sizeof(*n), GFP_ATOMIC);
> + if (!n) {
> + kvm_async_pf_unlock(b, flags);
> + cpu_relax();
> + goto again;
> + }
> + n->token = token;
> + n->task = current;
> + n->cpu = smp_processor_id();
> + n->halted = false;
> + n->delayed = false;
> + init_swait_queue_head(&n->wq);
> + hlist_add_head(&n->link, &b->list);
> + } else {
> + kvm_async_pf_wake_one(n);
> + }
> +
> + kvm_async_pf_unlock_and_clear(b, flags);
> +}
> +
> +static bool do_async_pf(unsigned long addr, unsigned int esr,
> + struct pt_regs *regs)
> +{
> + u32 reason;
> +
> + if (!kvm_async_pf_read_enabled())
> + return false;
> +
> + reason = kvm_async_pf_read_reason();
> + if (!reason)
> + return false;
> +
> + switch (reason) {
> + case KVM_PV_REASON_PAGE_NOT_PRESENT:
> + kvm_async_pf_wait((u32)addr, !user_mode(regs));
> + break;
> + case KVM_PV_REASON_PAGE_READY:
> + kvm_async_pf_wake((u32)addr);
> + break;
> + default:
> + if (reason) {
> + pr_warn("%s: Illegal reason %d\n", __func__, reason);
> + kvm_async_pf_write_reason(0);
> + }
> + }
> +
> + return true;
> +}
> +#endif /* CONFIG_KVM_GUEST */
> +
> void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs)
> {
> const struct fault_info *inf = esr_to_fault_info(esr);
>
> +#ifdef CONFIG_KVM_GUEST
> + if (do_async_pf(addr, esr, regs))
> + return;
> +#endif
> +
> if (!inf->fn(addr, esr, regs))
> return;
>
> @@ -878,3 +1174,141 @@ void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr,
> debug_exception_exit(regs);
> }
> NOKPROBE_SYMBOL(do_debug_exception);
> +
> +#ifdef CONFIG_KVM_GUEST
> +static int __init kvm_async_pf_available(char *arg)
> +{
> + async_pf_available = false;
> + return 0;
> +}
> +early_param("no-kvmapf", kvm_async_pf_available);
> +
> +static void kvm_async_pf_enable(bool enable)
> +{
> + struct arm_smccc_res res;
> + unsigned long *offsets = (unsigned long *)__exception_handlers_offset;
> + u32 enabled = kvm_async_pf_read_enabled();
> + u64 val;
> + int i;
> +
> + if (enable == enabled)
> + return;
> +
> + if (enable) {
> + /*
> + * Asychonous page faults will be prohibited when CPU runs
> + * instructions between the vector base and the maximal
> + * offset, plus 4096. The 4096 is the assumped maximal
> + * length for individual handler. The hardware registers
> + * should be saved to stack at the beginning of the handlers,
> + * so 4096 shuld be safe enough.
> + */
> + val = 0;
> + for (i = 0; i < 16; i++) {
> + if (offsets[i] > val)
> + val = offsets[i];
> + }
> +
> + val += 4096;
> + val |= BIT(62);
> +
> + arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_APF_FUNC_ID,
> + (u32)val, (u32)(val >> 32), &res);
> + if (res.a0 != SMCCC_RET_SUCCESS) {
> + pr_warn("Async PF configuration error %ld on CPU %d\n",
> + res.a0, smp_processor_id());
> + return;
> + }
> +
> + /* FIXME: Enable KVM_ASYNC_PF_SEND_ALWAYS */
> + val = BIT(63);
> + val |= virt_to_phys(this_cpu_ptr(&apf_data));
> + val |= KVM_ASYNC_PF_ENABLED;
> +
> + kvm_async_pf_write_enabled(1);
> + arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_APF_FUNC_ID,
> + (u32)val, (u32)(val >> 32), &res);
> + if (res.a0 != SMCCC_RET_SUCCESS) {
> + pr_warn("Async PF enable error %ld on CPU %d\n",
> + res.a0, smp_processor_id());
> + kvm_async_pf_write_enabled(0);
> + return;
> + }
> + } else {
> + val = BIT(63);
> + arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_APF_FUNC_ID,
> + (u32)val, (u32)(val >> 32), &res);
> + if (res.a0 != SMCCC_RET_SUCCESS) {
> + pr_warn("Async PF disable error %ld on CPU %d\n",
> + res.a0, smp_processor_id());
> + return;
> + }
> +
> + kvm_async_pf_write_enabled(0);
> + }
> +
> + pr_info("Async PF %s on CPU %d\n",
> + enable ? "enabled" : "disabled", smp_processor_id());
> +}
> +
> +static void kvm_async_pf_cpu_reboot(void *unused)
> +{
> + kvm_async_pf_enable(false);
> +}
> +
> +static int kvm_async_pf_cpu_reboot_notify(struct notifier_block *nb,
> + unsigned long code, void *unused)
> +{
> + if (code == SYS_RESTART)
> + on_each_cpu(kvm_async_pf_cpu_reboot, NULL, 1);
> +
> + return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block kvm_async_pf_cpu_reboot_nb = {
> + .notifier_call = kvm_async_pf_cpu_reboot_notify,
> +};
> +
> +static int kvm_async_pf_cpu_online(unsigned int cpu)
> +{
> + struct kvm_task_sleep_head *b;
> +
> + b = this_cpu_ptr(&apf_head);
> + raw_spin_lock_init(&b->lock);
> + kvm_async_pf_enable(true);
> + return 0;
> +}
> +
> +static int kvm_async_pf_cpu_offline(unsigned int cpu)
> +{
> + kvm_async_pf_enable(false);
> + return 0;
> +}
> +
> +static int __init kvm_async_pf_cpu_init(void)
> +{
> + struct kvm_task_sleep_head *b;
> + int ret;
> +
> + if (!kvm_para_has_feature(KVM_FEATURE_ASYNC_PF) ||
> + !async_pf_available)
> + return -EPERM;
> +
> + register_reboot_notifier(&kvm_async_pf_cpu_reboot_nb);
> + ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
> + "arm/kvm:online", kvm_async_pf_cpu_online,
> + kvm_async_pf_cpu_offline);
> + if (ret < 0) {
> + pr_warn("%s: Error %d to install cpu hotplug callbacks\n",
> + __func__, ret);
> + return ret;
> + }
> +
> + b = this_cpu_ptr(&apf_head);
> + raw_spin_lock_init(&b->lock);
> + kvm_async_pf_enable(true);
> +
> + return 0;
> +}
> +early_initcall(kvm_async_pf_cpu_init);
> +#endif /* CONFIG_KVM_GUEST */
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 06/91] clk: bcm: rpi: Statically init clk_init_data
From: Stephen Boyd @ 2020-05-27 6:47 UTC (permalink / raw)
To: Eric Anholt, Maxime Ripard, Nicolas Saenz Julienne
Cc: Tim Gover, Dave Stevenson, Michael Turquette, linux-kernel,
dri-devel, linux-clk, bcm-kernel-feedback-list, linux-rpi-kernel,
Phil Elwell, linux-arm-kernel, Maxime Ripard
In-Reply-To: <eeae1a92da7812f04a034498d3a1cb60e282fec7.1587742492.git-series.maxime@cerno.tech>
Quoting Maxime Ripard (2020-04-24 08:33:47)
> Instead of declaring the clk_init_data and then calling memset on it, just
> initialise properly.
>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: linux-clk@vger.kernel.org
> Acked-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [RFC] Use SMMU HTTU for DMA dirty page tracking
From: Zengtao (B) @ 2020-05-27 6:46 UTC (permalink / raw)
To: zhengxiang (A), Jean-Philippe Brucker
Cc: alex.williamson@redhat.com, Yan Zhao, Suzuki K Poulose,
maz@kernel.org, iommu@lists.linux-foundation.org, Kirti Wankhede,
Wangzhou (B), James Morse, julien.thierry.kdev@gmail.com,
Wanghaibin (D), Will Deacon, kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <e68c1158-8573-a477-42ce-48cee510c3ce@huawei.com>
> -----Original Message-----
> From: zhengxiang (A)
> Sent: Monday, May 25, 2020 7:34 PM
> To: Jean-Philippe Brucker
> Cc: linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> Will Deacon; Wanghaibin (D); Zengtao (B); maz@kernel.org; James Morse;
> julien.thierry.kdev@gmail.com; Suzuki K Poulose; Wangzhou (B);
> iommu@lists.linux-foundation.org; Kirti Wankhede; Yan Zhao;
> alex.williamson@redhat.com
> Subject: Re: [RFC] Use SMMU HTTU for DMA dirty page tracking
>
> [+cc Kirti, Yan, Alex]
>
> On 2020/5/23 1:14, Jean-Philippe Brucker wrote:
> > Hi,
> >
> > On Tue, May 19, 2020 at 05:42:55PM +0800, Xiang Zheng wrote:
> >> Hi all,
> >>
> >> Is there any plan for enabling SMMU HTTU?
> >
> > Not outside of SVA, as far as I know.
> >
>
> >> I have seen the patch locates in the SVA series patch, which adds
> >> support for HTTU:
> >> https://www.spinics.net/lists/arm-kernel/msg798694.html
> >>
> >> HTTU reduces the number of access faults on SMMU fault queue
> >> (permission faults also benifit from it).
> >>
> >> Besides reducing the faults, HTTU also helps to track dirty pages for
> >> device DMA. Is it feasible to utilize HTTU to get dirty pages on device
> >> DMA during VFIO live migration?
> >
> > As you know there is a VFIO interface for this under discussion:
> >
> https://lore.kernel.org/kvm/1589781397-28368-1-git-send-email-kwankhe
> de@nvidia.com/
> > It doesn't implement an internal API to communicate with the IOMMU
> driver
> > about dirty pages.
>
> >
> >> If SMMU can track dirty pages, devices are not required to implement
> >> additional dirty pages tracking to support VFIO live migration.
> >
> > It seems feasible, though tracking it in the device might be more
> > efficient. I might have misunderstood but I think for live migration of
> > the Intel NIC they trap guest accesses to the device and introspect its
> > state to figure out which pages it is accessing.
> >
> > With HTTU I suppose (without much knowledge about live migration)
> that
> > you'd need several new interfaces to the IOMMU drivers:
> >
> > * A way for VFIO to query HTTU support in the SMMU. There are some
> > discussions about communicating more IOMMU capabilities through
> VFIO but
> > no implementation yet. When HTTU isn't supported the DIRTY_PAGES
> bitmap
> > would report all pages as they do now.
> >
> > * VFIO_IOMMU_DIRTY_PAGES_FLAG_START/STOP would clear the dirty
> bit
> > for all VFIO mappings (which is going to take some time). There is a
> > walker in io-pgtable for iova_to_phys() which could be extended. I
> > suppose it's also possible to atomically switch the HA and HD bits in
> > context descriptors.
>
> Maybe we need not switch HA and HD bits, just turn on them all the time?
I think we need START/STOP, start is called when migration is started and STOP
called when migration finished.
I think you mean that during the migration(between START and STOP), HA and HD
functionality is always turned on.
>
> >
> > * VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP would query the
> dirty bit for all
> > VFIO mappings.
> >
We need a clear during the query which mean we have to reset the dirty status
after a query.
>
> I think we need to consider the case of IOMMU dirty pages logging. We
> want
> to test Kirti's VFIO migration patches combined with SMMU HTTU, any
> suggestions?
@kirti @yan @alex
As we know, you have worked on this feature for a long time, and it seems that
everything is going very well now, thanks very much for your VFIO migration
framework, it really helps a lot, and we want to start with SMMU HTTU enabled
based on your jobs, it's kind of hardware dirty page tracking, and expected to bring us
better performance, any suggestions?
Thank you.
Zengtao
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC] Use SMMU HTTU for DMA dirty page tracking
From: Xiang Zheng @ 2020-05-27 6:45 UTC (permalink / raw)
To: Tian, Kevin, Jean-Philippe Brucker
Cc: Zhao, Yan Y, Suzuki K Poulose, maz@kernel.org,
iommu@lists.linux-foundation.org, Kirti Wankhede,
alex.williamson@redhat.com, James Morse,
linux-arm-kernel@lists.infradead.org, prime.zeng@hisilicon.com,
Wang Haibin, Will Deacon, kvmarm@lists.cs.columbia.edu,
julien.thierry.kdev@gmail.com
In-Reply-To: <MWHPR11MB16454475DA1FF417CEF5A32B8CB10@MWHPR11MB1645.namprd11.prod.outlook.com>
On 2020/5/27 11:27, Tian, Kevin wrote:
>> From: Xiang Zheng
>> Sent: Monday, May 25, 2020 7:34 PM
>>
>> [+cc Kirti, Yan, Alex]
>>
>> On 2020/5/23 1:14, Jean-Philippe Brucker wrote:
>>> Hi,
>>>
>>> On Tue, May 19, 2020 at 05:42:55PM +0800, Xiang Zheng wrote:
>>>> Hi all,
>>>>
>>>> Is there any plan for enabling SMMU HTTU?
>>>
>>> Not outside of SVA, as far as I know.
>>>
>>
>>>> I have seen the patch locates in the SVA series patch, which adds
>>>> support for HTTU:
>>>> https://www.spinics.net/lists/arm-kernel/msg798694.html
>>>>
>>>> HTTU reduces the number of access faults on SMMU fault queue
>>>> (permission faults also benifit from it).
>>>>
>>>> Besides reducing the faults, HTTU also helps to track dirty pages for
>>>> device DMA. Is it feasible to utilize HTTU to get dirty pages on device
>>>> DMA during VFIO live migration?
>>>
>>> As you know there is a VFIO interface for this under discussion:
>>> https://lore.kernel.org/kvm/1589781397-28368-1-git-send-email-
>> kwankhede@nvidia.com/
>>> It doesn't implement an internal API to communicate with the IOMMU
>> driver
>>> about dirty pages.
>
> We plan to add such API later, e.g. to utilize A/D bit in VT-d 2nd-level
> page tables (Rev 3.0).
>
Thank you, Kevin.
When will you send this series patches? Maybe(Hope) we can also support
hardware-based dirty pages tracking via common APIs based on your patches. :)
>>
>>>
>>>> If SMMU can track dirty pages, devices are not required to implement
>>>> additional dirty pages tracking to support VFIO live migration.
>>>
>>> It seems feasible, though tracking it in the device might be more
>>> efficient. I might have misunderstood but I think for live migration of
>>> the Intel NIC they trap guest accesses to the device and introspect its
>>> state to figure out which pages it is accessing.
>
> Does HTTU implement A/D-like mechanism in SMMU page tables, or just
> report dirty pages in a log buffer? Either way tracking dirty pages in IOMMU
> side is generic thus doesn't require device-specific tweak like in Intel NIC.
>
Currently HTTU just implement A/D-like mechanism in SMMU page tables. We certainly
expect SMMU can also implement PML-like feature so that we can avoid walking the
whole page table to get the dirty pages.
By the way, I'm not sure whether HTTU or SLAD can help for mediated deivce.
--
Thanks,
Xiang
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] clk: versatile: remove redundant assignment to pointer clk
From: Stephen Boyd @ 2020-05-27 6:40 UTC (permalink / raw)
To: Colin King, Linus Walleij, Michael Turquette, linux-arm-kernel,
linux-clk
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20200526224116.63549-1-colin.king@canonical.com>
Quoting Colin King (2020-05-26 15:41:16)
> From: Colin Ian King <colin.king@canonical.com>
>
> The pointer clk is being initialized with a value that is never read
> and is being updated with a new value later on. The initialization
> is redundant and can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
Applied to clk-next
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] Revert "clk: rockchip: fix wrong mmc sample phase shift for rk3328"
From: Stephen Boyd @ 2020-05-27 6:36 UTC (permalink / raw)
To: Robin Murphy, heiko, mturquette
Cc: linux-rockchip, linux-clk, linux-arm-kernel
In-Reply-To: <e563eea358181446dc42c99e842c33f7ce911936.1589460539.git.robin.murphy@arm.com>
Quoting Robin Murphy (2020-05-14 05:58:14)
> This reverts commit 82f4b67f018c88a7cc9337f0067ed3d6ec352648.
>
> According to a subsequent revert in the vendor kernel, the original
> change was based on unclear documentation and was in fact incorrect.
>
> Emprically, my board's SD card at 50MHz and eMMC at 200MHZ seem to get
> lucky with a phase where it had no impact, but limiting the eMMC clock
> to 150MHz to match the nominal limit for the I/O pins made it virtually
> unusable, constantly throwing errors and retuning. With this revert, it
> starts behaving perfectly at 150MHz too.
>
> Fixes: 82f4b67f018c ("clk: rockchip: fix wrong mmc sample phase shift for rk3328")
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
Heiko?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCHv2 0/7] Support inhibiting input devices
From: Dmitry Torokhov @ 2020-05-27 6:34 UTC (permalink / raw)
To: Hans de Goede
Cc: Nick Dyer, linux-iio, Benjamin Tissoires, platform-driver-x86,
ibm-acpi-devel, Laxman Dewangan, Peter Meerwald-Stadler, kernel,
Fabio Estevam, linux-samsung-soc, Krzysztof Kozlowski,
Jonathan Hunter, linux-acpi, Kukjin Kim, NXP Linux Team,
linux-input, Len Brown, Peter Hutterer, Michael Hennerich,
Sascha Hauer, Sylvain Lemieux, Henrique de Moraes Holschuh,
Vladimir Zapolskiy, Lars-Peter Clausen, linux-tegra,
linux-arm-kernel, Barry Song, Ferruh Yigit, patches,
Rafael J . Wysocki, Andrzej Pietrasiewicz, Thierry Reding,
Sangwon Jee, Pengutronix Kernel Team, Hartmut Knaack, Shawn Guo,
Jonathan Cameron
In-Reply-To: <aa2ce2ab-e5bc-9cb4-8b53-c1ef9348b646@redhat.com>
On Tue, May 19, 2020 at 11:36:34AM +0200, Hans de Goede wrote:
> Hi,
>
> On 5/19/20 11:02 AM, Andrzej Pietrasiewicz wrote:
> > Hi Hans, Hi Dmitry,
> >
> > W dniu 18.05.2020 o 16:23, Hans de Goede pisze:
> > > Hi,
> >
> > <snip>
> >
> > > > > > >
> > > > > > > So I wonder what this series actually adds for functionality for
> > > > > > > userspace which can not already be achieved this way?
> > > > > > >
> > > > > > > I also noticed that you keep the device open (do not call the
> > > > > > > input_device's close callback) when inhibited and just throw away
> > > > > >
> > > > > > I'm not sure if I understand you correctly, it is called:
> > > > > >
> > > > > > +static inline void input_stop(struct input_dev *dev)
> > > > > > +{
> > > > > > + if (dev->poller)
> > > > > > + input_dev_poller_stop(dev->poller);
> > > > > > + if (dev->close)
> > > > > > + dev->close(dev);
> > > > > > ^^^^^^^^^^^^^^^^
> > > > > > +static int input_inhibit(struct input_dev *dev)
> > > > > > +{
> > > > > > + int ret = 0;
> > > > > > +
> > > > > > + mutex_lock(&dev->mutex);
> > > > > > +
> > > > > > + if (dev->inhibited)
> > > > > > + goto out;
> > > > > > +
> > > > > > + if (dev->users) {
> > > > > > + if (dev->inhibit) {
> > > > > > + ret = dev->inhibit(dev);
> > > > > > + if (ret)
> > > > > > + goto out;
> > > > > > + }
> > > > > > + input_stop(dev);
> > > > > > ^^^^^^^^^^^^^^^^
> > > > > >
> > > > > > It will not be called when dev->users is zero, but if it is zero,
> > > > > > then nobody has opened the device yet so there is nothing to close.
> > > > >
> > > > > Ah, I missed that.
> > > > >
> > > > > So if the device implements the inhibit call back then on
> > > > > inhibit it will get both the inhibit and close callback called?
> > > > >
> > > >
> > > > That's right. And conversely, upon uninhibit open() and uninhibit()
> > > > callbacks will be invoked. Please note that just as with open()/close(),
> > > > providing inhibit()/uninhibit() is optional.
> > >
> > > Ack.
> > >
> > > > > And what happens if the last user goes away and the device
> > > > > is not inhibited?
> > > >
> > > > close() is called as usually.
> > >
> > > But not inhibit, hmm, see below.
> > >
> > > > > I'm trying to understand here what the difference between the 2
> > > > > is / what the goal of having a separate inhibit callback ?
> > > > >
> > > >
> > > > Drivers have very different ideas about what it means to suspend/resume
> > > > and open/close. The optional inhibit/uninhibit callbacks are meant for
> > > > the drivers to know that it is this particular action going on.
> > >
> > > So the inhibit() callback triggers the "suspend" behavior ?
> > > But shouldn't drivers which are capable of suspending the device
> > > always do so on close() ?
> > >
> > > Since your current proposal also calls close() on inhibit() I
> > > really see little difference between an inhibit() and the last
> > > user of the device closing it and IMHO unless there is a good
> > > reason to actually differentiate the 2 it would be better
> > > to only stick with the existing close() and in cases where
> > > that does not put the device in a low-power mode yet, fix
> > > the existing close() callback to do the low-power mode
> > > setting instead of adding a new callback.
> > >
> > > > For inhibit() there's one more argument: close() does not return a value,
> > > > so its meaning is "do some last cleanup" and as such it is not allowed
> > > > to fail - whatever its effect is, we must deem it successful. inhibit()
> > > > does return a value and so it is allowed to fail.
> > >
> > > Well, we could make close() return an error and at least in the inhibit()
> > > case propagate that to userspace. I wonder if userspace is going to
> > > do anything useful with that error though...
It really can't do anything. Have you ever seen userspace handling
errors from close()? And what can be done? A program is terminating, but
the kernel says "no, you closing input device failed, you have to
continue running indefinitely..."
> > >
> > > In my experience errors during cleanup/shutdown are best logged
> > > (using dev_err) and otherwise ignored, so that we try to clean up
> > > as much possible. Unless the very first step of the shutdown process
> > > fails the device is going to be in some twilight zone state anyways
> > > at this point we might as well try to cleanup as much as possible.
> >
> > What you say makes sense to me.
> > @Dmitry?
I will note here, that inhibit is closer to suspend() than to close(),
and we do report errors for suspend(). Therefore we could conceivably
try to handle errors if driver really wants to be fancy. But I think
majority of cases will be quite happy with using close() and simply
logging errors, as Hans said.
That said, I think the way we should handle inhibit/uninhibit, is that
if we have the callback defined, then we call it, and only call open and
close if uninhibit or inhibit are _not_ defined.
> >
> > >
> > > > All in all, it is up to the drivers to decide which callback they
> > > > provide. Based on my work so far I would say that there are tens
> > > > of simple cases where open() and close() are sufficient, out of total
> > > > ~400 users of input_allocate_device():
> > > >
> > > > $ git grep "input_allocate_device(" | grep -v ^Documentation | \
> > > > cut -f1 -d: | sort | uniq | wc
> > > > 390 390 13496
> > >
> > > So can you explain a bit more about the cases where only having
> > > open/close is not sufficient? So far I have the feeling that
> > > those are all we need and that we really do not need separate
> > > [un]inhibit callbacks.
> >
> > My primary concern was not being able to propagate inhibit() error
> > to userspace, and then if we have inhibit(), uninhibit() should be
> > there for completeness. If propagating the error to userspace can
> > be neglected then yes, it seems open/close should be sufficient,
> > even more because the real meaning of "open" is "prepare the device
> > for generating input events".
> >
> > To validate the idea of not introducing inhibit()/uninhibit() callbacks
> > to implement device inhibiting/uninhibiting let's look at
> > drivers/input/mouse/elan_i2c_core.c (PATCH 7/7):
> >
> > static int elan_inhibit(struct input_dev *input)
> > {
> > [...]
> >
> > ret = mutex_lock_interruptible(&data->sysfs_mutex);
> > if (ret)
> > return ret;
> >
> > disable_irq(client->irq);
> >
> > ret = elan_disable_power(data);
> > if (ret)
> > enable_irq(client->irq);
> > [...]
> > }
> >
> > First, close() does not exist in this driver. Of course this can be
> > fixed. Then it doesn't return a value. Then, if either taking the
> > mutex or disabling the power fails, the close() is still deemed
> > successful. Is it ok?
>
> Note I also mentioned another solution for the error propagation,
> which would require a big "flag day" commit adding "return 0"
> to all existing close callbacks, but otherwise should work for your
> purposes:
No, please, no flag days and no changing close() to return error, it
makes no sense for close().
>
> > Well, we could make close() return an error and at least in the inhibit()
> > case propagate that to userspace. I wonder if userspace is going to
> > do anything useful with that error though...
>
> And I guess we could log an error that close failed in the old close() path
> where we cannot propagate the error.
>
> Also why the mutex_lock_interruptible() ? If you change that to
> a normal mutex_lock() you loose one of the possible 2 error cases and
> I doubt anyone is going to do a CTRL-C of the process doing the
> inhibiting (or that that process starts a timer using a signal
> to ensure the inhibit does not take to long or some such).
Well, we have the dedicated callbacks in Chrome OS, so when I did the
patch I could even handle Ctrl-C, so why not? But it indeed can easily
be dropped in favor of straight mutex_lock().
Thanks.
--
Dmitry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2] clk: mediatek: assign the initial value to clk_init_data of mtk_mux
From: Weiyi Lu @ 2020-05-27 6:25 UTC (permalink / raw)
To: Nicolas Boichat, Stephen Boyd, Matthias Brugger
Cc: James Liao, Weiyi Lu, srv_heupstream, linux-kernel, stable,
Fan Chen, linux-mediatek, Owen Chen, linux-clk, linux-arm-kernel
When some new clock supports are introduced, e.g. [1]
it might lead to an error although it should be NULL because
clk_init_data is on the stack and it might have random values
if using without initialization.
Add the missing initial value to clk_init_data.
[1] https://android-review.googlesource.com/c/kernel/common/+/1278046
Fixes: a3ae549917f1 ("clk: mediatek: Add new clkmux register API")
Cc: <stable@vger.kernel.org>
Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
---
drivers/clk/mediatek/clk-mux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
index 76f9cd0..14e127e 100644
--- a/drivers/clk/mediatek/clk-mux.c
+++ b/drivers/clk/mediatek/clk-mux.c
@@ -160,7 +160,7 @@ struct clk *mtk_clk_register_mux(const struct mtk_mux *mux,
spinlock_t *lock)
{
struct mtk_clk_mux *clk_mux;
- struct clk_init_data init;
+ struct clk_init_data init = {};
struct clk *clk;
clk_mux = kzalloc(sizeof(*clk_mux), GFP_KERNEL);
--
1.8.1.1.dirty
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v4 6/7] KVM: MIPS: clean up redundant 'kvm_run' parameters
From: Tianjia Zhang @ 2020-05-27 6:24 UTC (permalink / raw)
To: Huacai Chen
Cc: wanpengli, kvm, david, Benjamin Herrenschmidt, heiko.carstens,
Peter Xu, open list:MIPS, paulus, hpa, kvmarm, linux-s390,
frankja, Marc Zyngier, joro, x86, borntraeger, mingo,
julien.thierry.kdev, thuth, gor, suzuki.poulose, kvm-ppc,
Borislav Petkov, Thomas Gleixner, linux-arm-kernel, jmattson,
Thomas Bogendoerfer, cohuck, christoffer.dall,
sean.j.christopherson, LKML, james.morse, mpe, Paolo Bonzini,
vkuznets, linuxppc-dev
In-Reply-To: <CAAhV-H7kpKUfQoWid6GSNL5+4hTTroGyL83EaW6yZwS2+Ti9kA@mail.gmail.com>
On 2020/4/27 13:40, Huacai Chen wrote:
> Reviewed-by: Huacai Chen <chenhc@lemote.com>
>
> On Mon, Apr 27, 2020 at 12:35 PM Tianjia Zhang
> <tianjia.zhang@linux.alibaba.com> wrote:
>>
>> In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu'
>> structure. For historical reasons, many kvm-related function parameters
>> retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time. This
>> patch does a unified cleanup of these remaining redundant parameters.
>>
>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>> ---
>> arch/mips/include/asm/kvm_host.h | 28 +-------
>> arch/mips/kvm/emulate.c | 59 ++++++----------
>> arch/mips/kvm/mips.c | 11 ++-
>> arch/mips/kvm/trap_emul.c | 114 ++++++++++++++-----------------
>> arch/mips/kvm/vz.c | 26 +++----
>> 5 files changed, 87 insertions(+), 151 deletions(-)
>>
Hi Huacai,
These two patches(6/7 and 7/7) should be merged into the tree of the
mips architecture separately. At present, there seems to be no good way
to merge the whole architecture patchs.
For this series of patches, some architectures have been merged, some
need to update the patch.
Thanks and best,
Tianjia
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCHv2 0/7] Support inhibiting input devices
From: Peter Hutterer @ 2020-05-27 6:13 UTC (permalink / raw)
To: Andrzej Pietrasiewicz
Cc: Nick Dyer, linux-iio, Benjamin Tissoires, platform-driver-x86,
ibm-acpi-devel, Laxman Dewangan, Peter Meerwald-Stadler, kernel,
Fabio Estevam, linux-samsung-soc, Krzysztof Kozlowski,
Jonathan Hunter, linux-acpi, Kukjin Kim, NXP Linux Team,
linux-input, Len Brown, Peter Hutterer, Michael Hennerich,
Sascha Hauer, Sylvain Lemieux, Henrique de Moraes Holschuh,
Vladimir Zapolskiy, Hans de Goede, Lars-Peter Clausen,
linux-tegra, linux-arm-kernel, Barry Song, Ferruh Yigit, patches,
Dmitry Torokhov, Rafael J . Wysocki, Thierry Reding, Sangwon Jee,
Pengutronix Kernel Team, Hartmut Knaack, Shawn Guo,
Jonathan Cameron
In-Reply-To: <513f25c0-7125-c564-0090-052d626fe508@collabora.com>
Hi Andrzej,
On Fri, May 22, 2020 at 05:35:56PM +0200, Andrzej Pietrasiewicz wrote:
> Hi Hans, hi Dmitry,
>
> W dniu 18.05.2020 o 04:40, Dmitry Torokhov pisze:
> > Hi Hans, Peter,
> >
> > On Mon, May 18, 2020 at 08:55:10AM +1000, Peter Hutterer wrote:
> > > On Fri, May 15, 2020 at 08:19:10PM +0200, Hans de Goede wrote:
> > > > Hi Andrezj,
> > > >
>
> <snip>
>
> > >
> > > > I also noticed that you keep the device open (do not call the
> > > > input_device's close callback) when inhibited and just throw away
> > > > any events generated. This seems inefficient and may lead to
> > > > the internal state getting out of sync. What if a key is pressed
> > > > while inhibited and then the device is uninhibited while the key
> > > > is still pressed? Now the press event is lost and userspace
> > > > querying the current state will see the pressed key as being
> > > > released.
> >
> > This is a good point. We should look into signalling that some events
> > have been dropped (via EV_SYN/SYN_DROPPED) so that clients are aware of
> > it.
> >
>
> It seems to me that the situation Hans envisions is not possible,
> or will not be possible with a simple change. Let me explain.
>
> For a start, let's recall that the input core prevents consecutive
> events of the same kind (type _and_ code _and_ value) from being
> delivered to handlers. The decision is made in input_get_disposition().
> For EV_KEY it is:
>
> if (is_event_supported(code, dev->keybit, KEY_MAX)) {
>
> /* auto-repeat bypasses state updates */
> if (value == 2) {
> disposition = INPUT_PASS_TO_HANDLERS;
> break;
> }
>
> if (!!test_bit(code, dev->key) != !!value) {
>
> __change_bit(code, dev->key);
> disposition = INPUT_PASS_TO_HANDLERS;
> }
> }
note that this isn't per-process state, userspace can get release events
after open() for keys it never got the press event for. Simple test:
type evtest<enter> and KEY_ENTER up is the first event you'll get.
But otherwise I agree with you that press/release should always be balanced
if input_dev_release_keys() is called on inhibit and with that autorepeat
snippet below. At least I couldn't come up with any combination of multiple
clients opening/closing/inhibiting that resulted in an unwanted release
event after uninhibit.
Cheers,
Peter
> Let's now focus on value != 2 (events other than auto-repeat).
> The disposition changes from the default INPUT_IGNORE_EVENT to
> INPUT_PASS_TO_HANDLERS only when the event in question changes
> the current state: either by releasing a pressed key, or by
> pressing a released key. Subsequent releases of a released key
> or subsequent presses of a pressed key will be ignored.
>
> What Hans points out is the possibility of uninhibiting a device
> while its key is pressed and then releasing the key. First of all,
> during inhibiting input_dev_release_keys() is called, so input_dev's
> internal state will be cleared of all pressed keys. Then the device
> - after being uninhibited - all of a sudden produces a key release
> event. It will be ignored as per the "subsequent releases of a
> released key" case, so the handlers will not be passed an unmatched
> key release event. Assuming that passing an unmatched key release
> event was Hans's concern, in this case it seems impossible.
>
> Now, the value of 2 (auto-repeat) needs some attention. There are two
> cases to consider: the device uses input core's software repeat or it
> uses its own (hardware) repeat.
>
> Let's consider the first case. The timer which generates auto-repeat
> is only started on a key press event and only stopped on a key release
> event. As such, if any auto-repeat was in progress when inhibiting
> happened, it must have been stopped as per input_dev_release_keys().
> Then the key is pressed and held after the device has been inhibited,
> and the device is being uninhibited. Since it uses software auto-repeat,
> no events will be reported by the device until the key is released,
> and, as explained above, the release event will be ignored.
>
> Let's consider the second case. The key is pressed and held after the
> device has been inhibited and the device is being uninhibited. The worst
> thing that can happen is unmatched key repeat events will start coming
> from the device. We must prevent them from reaching the handlers and
> ignore them instead. So I suggest something on the lines of:
>
> if (is_event_supported(code, dev->keybit, KEY_MAX)) {
>
> /* auto-repeat bypasses state updates */
> - if (value == 2) {
> + if (value == 2 && test_bit(code, dev->key)) {
> disposition = INPUT_PASS_TO_HANDLERS;
> break;
> }
>
> The intended meaning is "ignore key repeat events if the key is not
> pressed".
>
> With this small change I believe it is not possible to have neither
> unmatched release nor unmatched repeat being delivered to handlers.
>
> Regards,
>
> Andrzej
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [RFC PATCH v12 07/11] psci: Add hypercall service for kvm ptp.
From: Jianyong Wu @ 2020-05-27 6:06 UTC (permalink / raw)
To: Steven Price, netdev@vger.kernel.org, yangbo.lu@nxp.com,
john.stultz@linaro.org, tglx@linutronix.de, pbonzini@redhat.com,
sean.j.christopherson@intel.com, maz@kernel.org,
richardcochran@gmail.com, Mark Rutland, will@kernel.org,
Suzuki Poulose
Cc: Justin He, Wei Chen, kvm@vger.kernel.org, Steve Capper,
linux-kernel@vger.kernel.org, Kaly Xin, nd,
kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <d7ec2534-95e4-ae79-fc53-4d48a4ea628c@arm.com>
Hi Steven,
> -----Original Message-----
> From: Steven Price <steven.price@arm.com>
> Sent: Tuesday, May 26, 2020 7:02 PM
> To: Jianyong Wu <Jianyong.Wu@arm.com>; netdev@vger.kernel.org;
> yangbo.lu@nxp.com; john.stultz@linaro.org; tglx@linutronix.de;
> pbonzini@redhat.com; sean.j.christopherson@intel.com; maz@kernel.org;
> richardcochran@gmail.com; Mark Rutland <Mark.Rutland@arm.com>;
> will@kernel.org; Suzuki Poulose <Suzuki.Poulose@arm.com>
> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org; Steve Capper
> <Steve.Capper@arm.com>; Kaly Xin <Kaly.Xin@arm.com>; Justin He
> <Justin.He@arm.com>; Wei Chen <Wei.Chen@arm.com>; nd <nd@arm.com>
> Subject: Re: [RFC PATCH v12 07/11] psci: Add hypercall service for kvm ptp.
>
> On 25/05/2020 03:11, Jianyong Wu wrote:
> > Hi Steven,
>
> Hi Jianyong,
>
> [...]>>> diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
> >>> index db6dce3d0e23..c964122f8dae 100644
> >>> --- a/virt/kvm/arm/hypercalls.c
> >>> +++ b/virt/kvm/arm/hypercalls.c
> >>> @@ -3,6 +3,7 @@
> >>>
> >>> #include <linux/arm-smccc.h>
> >>> #include <linux/kvm_host.h>
> >>> +#include <linux/clocksource_ids.h>
> >>>
> >>> #include <asm/kvm_emulate.h>
> >>>
> >>> @@ -11,6 +12,10 @@
> >>>
> >>> int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> >>> {
> >>> +#ifdef CONFIG_ARM64_KVM_PTP_HOST
> >>> + struct system_time_snapshot systime_snapshot;
> >>> + u64 cycles;
> >>> +#endif
> >>> u32 func_id = smccc_get_function(vcpu);
> >>> u32 val[4] = {SMCCC_RET_NOT_SUPPORTED};
> >>> u32 feature;
> >>> @@ -70,7 +75,49 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> >>> break;
> >>> case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
> >>> val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
> >>> +
> >>> +#ifdef CONFIG_ARM64_KVM_PTP_HOST
> >>> + val[0] |= BIT(ARM_SMCCC_KVM_FUNC_KVM_PTP); #endif
> >>> break;
> >>> +
> >>> +#ifdef CONFIG_ARM64_KVM_PTP_HOST
> >>> + /*
> >>> + * This serves virtual kvm_ptp.
> >>> + * Four values will be passed back.
> >>> + * reg0 stores high 32-bit host ktime;
> >>> + * reg1 stores low 32-bit host ktime;
> >>> + * reg2 stores high 32-bit difference of host cycles and cntvoff;
> >>> + * reg3 stores low 32-bit difference of host cycles and cntvoff.
> >>> + */
> >>> + case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> >>> + /*
> >>> + * system time and counter value must captured in the same
> >>> + * time to keep consistency and precision.
> >>> + */
> >>> + ktime_get_snapshot(&systime_snapshot);
> >>> + if (systime_snapshot.cs_id != CSID_ARM_ARCH_COUNTER)
> >>> + break;
> >>> + val[0] = upper_32_bits(systime_snapshot.real);
> >>> + val[1] = lower_32_bits(systime_snapshot.real);
> >>> + /*
> >>> + * which of virtual counter or physical counter being
> >>> + * asked for is decided by the first argument.
> >>> + */
> >>> + feature = smccc_get_arg1(vcpu);
> >>> + switch (feature) {
> >>> + case ARM_SMCCC_VENDOR_HYP_KVM_PTP_PHY_FUNC_ID:
> >>> + cycles = systime_snapshot.cycles;
> >>> + break;
> >>> + default:
> >>
> >> There's something a bit odd here.
> >>
> >> ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID and
> >> ARM_SMCCC_VENDOR_HYP_KVM_PTP_PHY_FUNC_ID look like they
> should be
> >> names of separate (top-level) functions, but actually the _PHY_ one
> >> is a parameter for the first. If the intention is to have a parameter
> >> then it would be better to pick a better name for the _PHY_ define
> >> and not define it using ARM_SMCCC_CALL_VAL.
> >>
> > Yeah, _PHY_ is not the same meaning with _PTP_FUNC_ID, so I think it
> should be a different name.
> > What about ARM_SMCCC_VENDOR_HYP_KVM_PTP_PHY_COUNTER?
>
> Personally I'd go with something much shorter, e.g.
> ARM_PTP_PHY_COUNTER.
> This is just an argument to an SMCCC call so there's no need for most of the
> prefix, indeed if (for whatever reason) there was a non-SMCCC mechanism
> added to do the same thing it would be reasonable to reuse the same values.
>
Ok , this shorter name is better.
> >> Second the use of "default:" means that there's no possibility to
> >> later extend this interface for more clocks if needed in the future.
> >>
> > I think we can add more clocks by adding more cases, this "default" means
> we can use no first arg to determine the default clock.
>
> The problem with the 'default' is it means it's not possible to probe whether
> the kernel supports any more clocks. If we used a different value (that the
> kernel doesn't support) then we end up in the default case and have no idea
> whether the clock value is the one we requested or not.
>
Yeah, it's more meaningful. Should return the exact value back to user.
> It's generally better when defining an ABI to explicitly return an error for
> unknown parameters, that way a future user of the ABI can discover
> whether the call did what was expected or not.
>
ok. I will fix it.
> >> Alternatively you could indeed implement as two top-level functions
> >> and change this to a...
> >>
> >> switch (func_id)
> >>
> >> ... along with multiple case labels as the functions would obviously
> >> be mostly the same.
> >>
> >> Also a minor style issue - you might want to consider splitting this
> >> into it's own function.
> >>
> > I think "switch (feature)" maybe better as this _PHY_ is not like a function
> id. Just like:
> > "
> > case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
> > feature = smccc_get_arg1(vcpu);
> > switch (feature) {
> > case ARM_SMCCC_ARCH_WORKAROUND_1:
> > ...
> > "
>
> I'm happy either way - it's purely that the definition/naming of
> ARM_SMCCC_VENDOR_HYP_KVM_PTP_PHY_FUNC_ID made it look like that
> was the intention. My preference would be to stick with the 'feature'
> approach as above because there's no need to "use up" the top-level SMCCC
> calls (but equally there's a large space so we'd have to work very hard to run
> out... ;) )
>
We can change the name of "_PHY_COUNTER", but it will remain in the same level with "_FUNC_ID" as
It will still occupy a place reserved for VENDOR SMCCC call.
Just like ARM_SMCCC_ARCH_WORKAROUND_1,
#define ARM_SMCCC_ARCH_WORKAROUND_1 \
ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
ARM_SMCCC_SMC_32, \
0, 0x8000)
It will be a ARCH SMCCC call id from the view of its definition.
> >> Finally I do think it would be useful to add some documentation of
> >> the new SMC calls. It would be easier to review the interface based
> >> on that documentation rather than trying to reverse-engineer the
> >> interface from the code.
> >>
> > Yeah, more doc needed here.
>
> Thanks, I think it's a good idea to get the ABI nailed down before worrying
> too much about the code, and it's easier to discuss based on documentation
> rather than code.
>
Yeah, a document here is in favor of code review.
Thanks
Jianyong
> Thanks,
>
> Steve
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] clk: versatile: remove redundant assignment to pointer clk
From: Linus Walleij @ 2020-05-27 5:49 UTC (permalink / raw)
To: Colin King
Cc: Stephen Boyd, Michael Turquette, kernel-janitors,
linux-kernel@vger.kernel.org, linux-clk, Linux ARM
In-Reply-To: <20200526224116.63549-1-colin.king@canonical.com>
On Wed, May 27, 2020 at 12:41 AM Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The pointer clk is being initialized with a value that is never read
> and is being updated with a new value later on. The initialization
> is redundant and can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 3/7] KVM: PPC: Remove redundant kvm_run from vcpu_arch
From: Tianjia Zhang @ 2020-05-27 5:23 UTC (permalink / raw)
To: Paul Mackerras
Cc: wanpengli, kvm, david, benh, heiko.carstens, peterx, linux-mips,
hpa, kvmarm, linux-s390, frankja, chenhuacai, maz, joro, x86,
borntraeger, mingo, julien.thierry.kdev, thuth, gor,
suzuki.poulose, kvm-ppc, bp, tglx, linux-arm-kernel, jmattson,
tsbogend, cohuck, christoffer.dall, sean.j.christopherson,
linux-kernel, james.morse, mpe, pbonzini, vkuznets, linuxppc-dev
In-Reply-To: <20200527042055.GG293451@thinks.paulus.ozlabs.org>
On 2020/5/27 12:20, Paul Mackerras wrote:
> On Mon, Apr 27, 2020 at 12:35:10PM +0800, Tianjia Zhang wrote:
>> The 'kvm_run' field already exists in the 'vcpu' structure, which
>> is the same structure as the 'kvm_run' in the 'vcpu_arch' and
>> should be deleted.
>>
>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>
> Thanks, patches 3 and 4 of this series applied to my kvm-ppc-next branch.
>
> Paul.
>
Thanks for your suggestion, for 5/7, I will submit a new version patch.
Thanks,
Tianjia
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] media: omap3isp: Shuffle cacheflush.h and include mm.h
From: Christoph Hellwig @ 2020-05-27 5:10 UTC (permalink / raw)
To: Nathan Chancellor
Cc: linux-ia64, linux-sh, zippel, linux-mips, linux-mm, sparclinux,
linux-riscv, hch, linux-arch, linux-c6x-dev, linux-hexagon, x86,
linux-xtensa, arnd, linux-alpha, linux-um, linux-m68k, openrisc,
linux-arm-kernel, monstr, linux-kernel, jeyu, linux-fsdevel, akpm,
linuxppc-dev
In-Reply-To: <20200527043426.3242439-1-natechancellor@gmail.com>
On Tue, May 26, 2020 at 09:34:27PM -0700, Nathan Chancellor wrote:
> After mm.h was removed from the asm-generic version of cacheflush.h,
> s390 allyesconfig shows several warnings of the following nature:
Hmm, I'm pretty sure I sent the same fix a few days ago in response to
a build bot report. But if that didn't get picked up I'm fine with
your version of it as well of course:
Acked-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 00/13] add ecspi ERR009165 for i.mx6/7 soc family
From: Sascha Hauer @ 2020-05-27 5:03 UTC (permalink / raw)
To: Robin Gong
Cc: mark.rutland, devicetree, martin.fuzzey, catalin.marinas,
will.deacon, linux-kernel, robh+dt, linux-spi, vkoul, broonie,
linux-imx, festevam, u.kleine-koenig, dan.j.williams, shawnguo,
kernel, linux-arm-kernel
In-Reply-To: <1590006865-20900-1-git-send-email-yibin.gong@nxp.com>
On Thu, May 21, 2020 at 04:34:12AM +0800, Robin Gong wrote:
> There is ecspi ERR009165 on i.mx6/7 soc family, which cause FIFO
> transfer to be send twice in DMA mode. Please get more information from:
> https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf. The workaround is adding
> new sdma ram script which works in XCH mode as PIO inside sdma instead
> of SMC mode, meanwhile, 'TX_THRESHOLD' should be 0. The issue should be
> exist on all legacy i.mx6/7 soc family before i.mx6ul.
> NXP fix this design issue from i.mx6ul, so newer chips including i.mx6ul/
> 6ull/6sll do not need this workaroud anymore. All other i.mx6/7/8 chips
> still need this workaroud. This patch set add new 'fsl,imx6ul-ecspi'
> for ecspi driver and 'ecspi_fixed' in sdma driver to choose if need errata
> or not.
> The first two reverted patches should be the same issue, though, it
> seems 'fixed' by changing to other shp script. Hope Sean or Sascha could
> have the chance to test this patch set if could fix their issues.
> Besides, enable sdma support for i.mx8mm/8mq and fix ecspi1 not work
> on i.mx8mm because the event id is zero.
For the series:
Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] media: omap3isp: Shuffle cacheflush.h and include mm.h
From: Nathan Chancellor @ 2020-05-27 4:34 UTC (permalink / raw)
To: hch
Cc: linux-ia64, linux-sh, zippel, linux-mips, linux-mm, sparclinux,
linux-riscv, linux-arch, linux-c6x-dev, linux-hexagon, x86,
linux-xtensa, arnd, linux-alpha, linux-um, linux-m68k, openrisc,
Nathan Chancellor, linux-arm-kernel, monstr, linux-kernel, jeyu,
linux-fsdevel, akpm, linuxppc-dev
In-Reply-To: <20200515143646.3857579-7-hch@lst.de>
After mm.h was removed from the asm-generic version of cacheflush.h,
s390 allyesconfig shows several warnings of the following nature:
In file included from ./arch/s390/include/generated/asm/cacheflush.h:1,
from drivers/media/platform/omap3isp/isp.c:42:
./include/asm-generic/cacheflush.h:16:42: warning: 'struct mm_struct'
declared inside parameter list will not be visible outside of this
definition or declaration
cacheflush.h does not include mm.h nor does it include any forward
declaration of these structures hence the warning. To avoid this,
include mm.h explicitly in this file and shuffle cacheflush.h below it.
Fixes: 19c0054597a0 ("asm-generic: don't include <linux/mm.h> in cacheflush.h")
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
I am aware the fixes tag is kind of irrelevant because that SHA will
change in the next linux-next revision and this will probably get folded
into the original patch anyways but still.
The other solution would be to add forward declarations of these structs
to the top of cacheflush.h, I just chose to do what Christoph did in the
original patch. I am happy to do that instead if you all feel that is
better.
drivers/media/platform/omap3isp/isp.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index a4ee6b86663e..54106a768e54 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -39,8 +39,6 @@
* Troy Laramy <t-laramy@ti.com>
*/
-#include <asm/cacheflush.h>
-
#include <linux/clk.h>
#include <linux/clkdev.h>
#include <linux/delay.h>
@@ -49,6 +47,7 @@
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/mfd/syscon.h>
+#include <linux/mm.h>
#include <linux/module.h>
#include <linux/omap-iommu.h>
#include <linux/platform_device.h>
@@ -58,6 +57,8 @@
#include <linux/sched.h>
#include <linux/vmalloc.h>
+#include <asm/cacheflush.h>
+
#ifdef CONFIG_ARM_DMA_USE_IOMMU
#include <asm/dma-iommu.h>
#endif
--
2.27.0.rc0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v4 3/7] KVM: PPC: Remove redundant kvm_run from vcpu_arch
From: Paul Mackerras @ 2020-05-27 4:20 UTC (permalink / raw)
To: Tianjia Zhang
Cc: wanpengli, kvm, david, benh, heiko.carstens, peterx, linux-mips,
hpa, kvmarm, linux-s390, frankja, chenhuacai, maz, joro, x86,
borntraeger, mingo, julien.thierry.kdev, thuth, gor,
suzuki.poulose, kvm-ppc, bp, tglx, linux-arm-kernel, jmattson,
tsbogend, cohuck, christoffer.dall, sean.j.christopherson,
linux-kernel, james.morse, mpe, pbonzini, vkuznets, linuxppc-dev
In-Reply-To: <20200427043514.16144-4-tianjia.zhang@linux.alibaba.com>
On Mon, Apr 27, 2020 at 12:35:10PM +0800, Tianjia Zhang wrote:
> The 'kvm_run' field already exists in the 'vcpu' structure, which
> is the same structure as the 'kvm_run' in the 'vcpu_arch' and
> should be deleted.
>
> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Thanks, patches 3 and 4 of this series applied to my kvm-ppc-next branch.
Paul.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH RFCv2 7/9] kvm/arm64: Support async page fault
From: Gavin Shan @ 2020-05-27 4:05 UTC (permalink / raw)
To: Mark Rutland
Cc: aarcange, drjones, suzuki.poulose, catalin.marinas, linux-kernel,
eric.auger, james.morse, shan.gavin, maz, will, kvmarm,
linux-arm-kernel
In-Reply-To: <20200526123424.GF1363@C02TD0UTHF1T.local>
Hi Mark,
On 5/26/20 10:34 PM, Mark Rutland wrote:
> On Fri, May 08, 2020 at 01:29:17PM +1000, Gavin Shan wrote:
>> There are two stages of fault pages and the stage one page fault is
>> handled by guest itself. The guest is trapped to host when the page
>> fault is caused by stage 2 page table, for example missing. The guest
>> is suspended until the requested page is populated. To populate the
>> requested page can be related to IO activities if the page was swapped
>> out previously. In this case, the guest has to suspend for a few of
>> milliseconds at least, regardless of the overall system load. There
>> is no useful work done during the suspended period from guest's view.
>
> This is a bit difficult to read. How about:
>
> | When a vCPU triggers a Stage-2 fault (e.g. when accessing a page that
> | is not mapped at Stage-2), the vCPU is suspended until the host has
> | handled the fault. It can take the host milliseconds or longer to
> | handle the fault as this may require IO, and when the system load is
> | low neither the host nor guest perform useful work during such
> | periods.
>
Yes, It's much better.
>>
>> This adds asychornous page fault to improve the situation. A signal
>
> Nit: typo for `asynchronous` here, and there are a few other typos in
> the patch itself. It would be nice if you could run a spellcheck over
> that.
>
Sure.
>> (PAGE_NOT_PRESENT) is sent to guest if the requested page needs some time
>> to be populated. Guest might reschedule to another running process if
>> possible. Otherwise, the vCPU is put into power-saving mode, which is
>> actually to cause vCPU reschedule from host's view. A followup signal
>> (PAGE_READY) is sent to guest once the requested page is populated.
>> The suspended task is waken up or scheduled when guest receives the
>> signal. With this mechanism, the vCPU won't be stuck when the requested
>> page is being populated by host.
>
> It would probably be best to say 'notification' rather than 'signal'
> here, and say 'the guest is notified', etc. As above, it seems that this
> is per-vCPU, so it's probably better to say 'vCPU' rather than guest, to
> make it clear which context this applies to.
>
Ok.
>>
>> There are more details highlighted as below. Note the implementation is
>> similar to what x86 has to some extent:
>>
>> * A dedicated SMCCC ID is reserved to enable, disable or configure
>> the functionality. The only 64-bits parameter is conveyed by two
>> registers (w2/w1). Bits[63:56] is the bitmap used to specify the
>> operated functionality like enabling/disabling/configuration. The
>> bits[55:6] is the physical address of control block or external
>> data abort injection disallowed region. Bit[5:0] are used to pass
>> control flags.
>>
>> * Signal (PAGE_NOT_PRESENT) is sent to guest if the requested page
>> isn't ready. In the mean while, a work is started to populate the
>> page asynchronously in background. The stage 2 page table entry is
>> updated accordingly and another signal (PAGE_READY) is fired after
>> the request page is populted. The signals is notified by injected
>> data abort fault.
>>
>> * The signals are fired and consumed in sequential fashion. It means
>> no more signals will be fired if there is pending one, awaiting the
>> guest to consume. It's because the injected data abort faults have
>> to be done in sequential fashion.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>> arch/arm64/include/asm/kvm_host.h | 43 ++++
>> arch/arm64/include/asm/kvm_para.h | 27 ++
>> arch/arm64/include/uapi/asm/Kbuild | 2 -
>> arch/arm64/include/uapi/asm/kvm_para.h | 22 ++
>> arch/arm64/kvm/Kconfig | 1 +
>> arch/arm64/kvm/Makefile | 2 +
>> include/linux/arm-smccc.h | 6 +
>> virt/kvm/arm/arm.c | 36 ++-
>> virt/kvm/arm/async_pf.c | 335 +++++++++++++++++++++++++
>> virt/kvm/arm/hypercalls.c | 8 +
>> virt/kvm/arm/mmu.c | 29 ++-
>> 11 files changed, 506 insertions(+), 5 deletions(-)
>> create mode 100644 arch/arm64/include/asm/kvm_para.h
>> create mode 100644 arch/arm64/include/uapi/asm/kvm_para.h
>> create mode 100644 virt/kvm/arm/async_pf.c
>>
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index f77c706777ec..a207728d6f3f 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -250,6 +250,23 @@ struct vcpu_reset_state {
>> bool reset;
>> };
>>
>> +#ifdef CONFIG_KVM_ASYNC_PF
>> +
>> +/* Should be a power of two number */
>> +#define ASYNC_PF_PER_VCPU 64
>
> What exactly is this number?
>
It's maximal number of async page faults pending on the specific vCPU.
>> +
>> +/*
>> + * The association of gfn and token. The token will be sent to guest as
>> + * page fault address. Also, the guest could be in aarch32 mode. So its
>> + * length should be 32-bits.
>> + */
>
> The length of what should be 32-bit? The token?
>
> The guest sees the token as the fault address? How exactly is that
> exposed to the guest, is that via a synthetic S1 fault?
>
Yes, the token is 32-bits in length and it's exposed to guest via fault
address. I'll improve the comments in next revision.
>> +struct kvm_arch_async_pf {
>> + u32 token;
>> + gfn_t gfn;
>> + u32 esr;
>> +};
>> +#endif /* CONFIG_KVM_ASYNC_PF */
>> +
>> struct kvm_vcpu_arch {
>> struct kvm_cpu_context ctxt;
>> void *sve_state;
>> @@ -351,6 +368,17 @@ struct kvm_vcpu_arch {
>> u64 last_steal;
>> gpa_t base;
>> } steal;
>> +
>> +#ifdef CONFIG_KVM_ASYNC_PF
>> + struct {
>> + struct gfn_to_hva_cache cache;
>> + gfn_t gfns[ASYNC_PF_PER_VCPU];
>> + u64 control_block;
>> + u16 id;
>> + bool send_user_only;
>> + u64 no_fault_inst_range;
>
> What are all of these fields? This implies functionality not covered
> in the commit message, and it's not at all clear what these are.
>
> For example, what exactly is `no_fault_inst_range`? If it's a range,
> surely that needs a start/end or base/size pair rather than a single
> value?
>
Ok. I will add more words about how the data struct is designed and
how it's used etc in next revision.
>> + } apf;
>> +#endif
>> };
>>
>> /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
>> @@ -604,6 +632,21 @@ int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu,
>>
>> static inline void __cpu_init_stage2(void) {}
>>
>> +#ifdef CONFIG_KVM_ASYNC_PF
>> +bool kvm_async_pf_hash_find(struct kvm_vcpu *vcpu, gfn_t gfn);
>> +bool kvm_arch_can_inject_async_page_not_present(struct kvm_vcpu *vcpu);
>> +bool kvm_arch_can_inject_async_page_present(struct kvm_vcpu *vcpu);
>> +int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu, u32 esr,
>> + gpa_t gpa, gfn_t gfn);
>> +void kvm_arch_async_page_not_present(struct kvm_vcpu *vcpu,
>> + struct kvm_async_pf *work);
>> +void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
>> + struct kvm_async_pf *work);
>> +void kvm_arch_async_page_ready(struct kvm_vcpu *vcpu,
>> + struct kvm_async_pf *work);
>> +long kvm_async_pf_hypercall(struct kvm_vcpu *vcpu);
>> +#endif /* CONFIG_KVM_ASYNC_PF */
>> +
>> /* Guest/host FPSIMD coordination helpers */
>> int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu);
>> void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu);
>> diff --git a/arch/arm64/include/asm/kvm_para.h b/arch/arm64/include/asm/kvm_para.h
>> new file mode 100644
>> index 000000000000..0ea481dd1c7a
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/kvm_para.h
>> @@ -0,0 +1,27 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _ASM_ARM_KVM_PARA_H
>> +#define _ASM_ARM_KVM_PARA_H
>> +
>> +#include <uapi/asm/kvm_para.h>
>> +
>> +static inline bool kvm_check_and_clear_guest_paused(void)
>> +{
>> + return false;
>> +}
>> +
>> +static inline unsigned int kvm_arch_para_features(void)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline unsigned int kvm_arch_para_hints(void)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline bool kvm_para_available(void)
>> +{
>> + return false;
>> +}
>> +
>> +#endif /* _ASM_ARM_KVM_PARA_H */
>> diff --git a/arch/arm64/include/uapi/asm/Kbuild b/arch/arm64/include/uapi/asm/Kbuild
>> index 602d137932dc..f66554cd5c45 100644
>> --- a/arch/arm64/include/uapi/asm/Kbuild
>> +++ b/arch/arm64/include/uapi/asm/Kbuild
>> @@ -1,3 +1 @@
>> # SPDX-License-Identifier: GPL-2.0
>> -
>> -generic-y += kvm_para.h
>> diff --git a/arch/arm64/include/uapi/asm/kvm_para.h b/arch/arm64/include/uapi/asm/kvm_para.h
>> new file mode 100644
>> index 000000000000..e0bd0e579b9a
>> --- /dev/null
>> +++ b/arch/arm64/include/uapi/asm/kvm_para.h
>> @@ -0,0 +1,22 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>> +#ifndef _UAPI_ASM_ARM_KVM_PARA_H
>> +#define _UAPI_ASM_ARM_KVM_PARA_H
>> +
>> +#include <linux/types.h>
>> +
>> +#define KVM_FEATURE_ASYNC_PF 0
>> +
>> +/* Async PF */
>> +#define KVM_ASYNC_PF_ENABLED (1 << 0)
>> +#define KVM_ASYNC_PF_SEND_ALWAYS (1 << 1)
>> +
>> +#define KVM_PV_REASON_PAGE_NOT_PRESENT 1
>> +#define KVM_PV_REASON_PAGE_READY 2
>> +
>> +struct kvm_vcpu_pv_apf_data {
>> + __u32 reason;
>> + __u8 pad[60];
>> + __u32 enabled;
>> +};
>
> What's all the padding for?
>
The padding is ensure the @reason and @enabled in different cache
line. @reason is shared by host/guest, while @enabled is almostly
owned by guest.
>> +
>> +#endif /* _UAPI_ASM_ARM_KVM_PARA_H */
>> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
>> index 449386d76441..1053e16b1739 100644
>> --- a/arch/arm64/kvm/Kconfig
>> +++ b/arch/arm64/kvm/Kconfig
>> @@ -34,6 +34,7 @@ config KVM
>> select KVM_VFIO
>> select HAVE_KVM_EVENTFD
>> select HAVE_KVM_IRQFD
>> + select KVM_ASYNC_PF
>> select KVM_ARM_PMU if HW_PERF_EVENTS
>> select HAVE_KVM_MSI
>> select HAVE_KVM_IRQCHIP
>> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
>> index 5ffbdc39e780..3be24c1e401f 100644
>> --- a/arch/arm64/kvm/Makefile
>> +++ b/arch/arm64/kvm/Makefile
>> @@ -37,3 +37,5 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-debug.o
>> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o
>> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o
>> kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o
>> +kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/async_pf.o
>> +kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/arm/async_pf.o
>> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
>> index bdc0124a064a..22007dd3b9f0 100644
>> --- a/include/linux/arm-smccc.h
>> +++ b/include/linux/arm-smccc.h
>> @@ -94,6 +94,7 @@
>>
>> /* KVM "vendor specific" services */
>> #define ARM_SMCCC_KVM_FUNC_FEATURES 0
>> +#define ARM_SMCCC_KVM_FUNC_APF 1
>> #define ARM_SMCCC_KVM_FUNC_FEATURES_2 127
>> #define ARM_SMCCC_KVM_NUM_FUNCS 128
>>
>> @@ -102,6 +103,11 @@
>> ARM_SMCCC_SMC_32, \
>> ARM_SMCCC_OWNER_VENDOR_HYP, \
>> ARM_SMCCC_KVM_FUNC_FEATURES)
>> +#define ARM_SMCCC_VENDOR_HYP_KVM_APF_FUNC_ID \
>> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
>> + ARM_SMCCC_SMC_32, \
>> + ARM_SMCCC_OWNER_VENDOR_HYP, \
>> + ARM_SMCCC_KVM_FUNC_APF)
>>
>> #ifndef __ASSEMBLY__
>>
>> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
>> index 2cbb57485760..3f62899cef13 100644
>> --- a/virt/kvm/arm/arm.c
>> +++ b/virt/kvm/arm/arm.c
>> @@ -222,6 +222,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>> */
>> r = 1;
>> break;
>> +#ifdef CONFIG_KVM_ASYNC_PF
>> + case KVM_CAP_ASYNC_PF:
>> + r = 1;
>> + break;
>> +#endif
>> default:
>> r = kvm_arch_vm_ioctl_check_extension(kvm, ext);
>> break;
>> @@ -269,6 +274,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
>> /* Force users to call KVM_ARM_VCPU_INIT */
>> vcpu->arch.target = -1;
>> bitmap_zero(vcpu->arch.features, KVM_VCPU_MAX_FEATURES);
>> +#ifdef CONFIG_KVM_ASYNC_PF
>> + vcpu->arch.apf.control_block = 0UL;
>> + vcpu->arch.apf.no_fault_inst_range = 0x800;
>
> Where has this magic number come from?
>
It's the total length (size) of exception vectors. When the vCPU's
PC in the range, the DABT can't be injected.
>> +#endif
>>
>> /* Set up the timer */
>> kvm_timer_vcpu_init(vcpu);
>> @@ -426,8 +435,27 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
>> int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
>> {
>> bool irq_lines = *vcpu_hcr(v) & (HCR_VI | HCR_VF);
>> - return ((irq_lines || kvm_vgic_vcpu_pending_irq(v))
>> - && !v->arch.power_off && !v->arch.pause);
>> +
>> + if ((irq_lines || kvm_vgic_vcpu_pending_irq(v)) &&
>> + !v->arch.power_off && !v->arch.pause)
>> + return true;
>> +
>> +#ifdef CONFIG_KVM_ASYNC_PF
>> + if (v->arch.apf.control_block & KVM_ASYNC_PF_ENABLED) {
>> + u32 val;
>> + int ret;
>> +
>> + if (!list_empty_careful(&v->async_pf.done))
>> + return true;
>> +
>> + ret = kvm_read_guest_cached(v->kvm, &v->arch.apf.cache,
>> + &val, sizeof(val));
>> + if (ret || val)
>> + return true;
>> + }
>> +#endif
>> +
>> + return false;
>> }
>>
>> bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
>> @@ -683,6 +711,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>
>> check_vcpu_requests(vcpu);
>>
>> +#ifdef CONFIG_KVM_ASYNC_PF
>> + kvm_check_async_pf_completion(vcpu);
>> +#endif
>
> Rather than adding ifdeffery like this, please add an empty stub for
> when CONFIG_KVM_ASYNC_PF isn't selected, so that this can be used
> unconditionally.
>
Ok.
>> +
>> /*
>> * Preparing the interrupts to be injected also
>> * involves poking the GIC, which must be done in a
>> diff --git a/virt/kvm/arm/async_pf.c b/virt/kvm/arm/async_pf.c
>> new file mode 100644
>> index 000000000000..5be49d684de3
>> --- /dev/null
>> +++ b/virt/kvm/arm/async_pf.c
>> @@ -0,0 +1,335 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Asynchronous Page Fault Support
>> + *
>> + * Copyright (C) 2020 Red Hat, Inc., Gavin Shan
>> + *
>> + * Based on arch/x86/kernel/kvm.c
>> + */
>> +
>> +#include <linux/arm-smccc.h>
>> +#include <linux/kvm_host.h>
>> +#include <asm/kvm_emulate.h>
>> +#include <kvm/arm_hypercalls.h>
>> +
>> +static inline u32 kvm_async_pf_hash_fn(gfn_t gfn)
>> +{
>> + return hash_32(gfn & 0xffffffff, order_base_2(ASYNC_PF_PER_VCPU));
>> +}
>> +
>> +static inline u32 kvm_async_pf_hash_next(u32 key)
>> +{
>> + return (key + 1) & (ASYNC_PF_PER_VCPU - 1);
>> +}
>> +
>> +static inline void kvm_async_pf_hash_reset(struct kvm_vcpu *vcpu)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ASYNC_PF_PER_VCPU; i++)
>> + vcpu->arch.apf.gfns[i] = ~0;
>> +}
>> +
>> +/*
>> + * Add gfn to the hash table. It's ensured there is a free entry
>> + * when this function is called.
>> + */
>> +static void kvm_async_pf_hash_add(struct kvm_vcpu *vcpu, gfn_t gfn)
>> +{
>> + u32 key = kvm_async_pf_hash_fn(gfn);
>> +
>> + while (vcpu->arch.apf.gfns[key] != ~0)
>> + key = kvm_async_pf_hash_next(key);
>> +
>> + vcpu->arch.apf.gfns[key] = gfn;
>> +}
>> +
>> +static u32 kvm_async_pf_hash_slot(struct kvm_vcpu *vcpu, gfn_t gfn)
>> +{
>> + u32 key = kvm_async_pf_hash_fn(gfn);
>> + int i;
>> +
>> + for (i = 0; i < ASYNC_PF_PER_VCPU; i++) {
>> + if (vcpu->arch.apf.gfns[key] == gfn ||
>> + vcpu->arch.apf.gfns[key] == ~0)
>> + break;
>> +
>> + key = kvm_async_pf_hash_next(key);
>> + }
>> +
>> + return key;
>> +}
>> +
>> +static void kvm_async_pf_hash_remove(struct kvm_vcpu *vcpu, gfn_t gfn)
>> +{
>> + u32 i, j, k;
>> +
>> + i = j = kvm_async_pf_hash_slot(vcpu, gfn);
>> + while (true) {
>> + vcpu->arch.apf.gfns[i] = ~0;
>> +
>> + do {
>> + j = kvm_async_pf_hash_next(j);
>> + if (vcpu->arch.apf.gfns[j] == ~0)
>> + return;
>> +
>> + k = kvm_async_pf_hash_fn(vcpu->arch.apf.gfns[j]);
>> + /*
>> + * k lies cyclically in ]i,j]
>> + * | i.k.j |
>> + * |....j i.k.| or |.k..j i...|
>> + */
>> + } while ((i <= j) ? (i < k && k <= j) : (i < k || k <= j));
>> +
>> + vcpu->arch.apf.gfns[i] = vcpu->arch.apf.gfns[j];
>> + i = j;
>> + }
>> +}
>
> This looks like a copy-paste of code under arch/x86.
>
> This looks like something that should be factored into common code
> rather than duplicated. Do we not have an existing common hash table
> implementation that we can use rather than building one specific to KVM
> async page faults?
>
Yeah, It's copied from arch/x86. I will explore the possiblity to
make the code is sharable among multiple architectures.
>> +
>> +bool kvm_async_pf_hash_find(struct kvm_vcpu *vcpu, gfn_t gfn)
>> +{
>> + u32 key = kvm_async_pf_hash_slot(vcpu, gfn);
>> +
>> + return vcpu->arch.apf.gfns[key] == gfn;
>> +}
>> +
>> +static inline int kvm_async_pf_read_cache(struct kvm_vcpu *vcpu, u32 *val)
>> +{
>> + return kvm_read_guest_cached(vcpu->kvm, &vcpu->arch.apf.cache,
>> + val, sizeof(*val));
>> +}
>> +
>> +static inline int kvm_async_pf_write_cache(struct kvm_vcpu *vcpu, u32 val)
>> +{
>> + return kvm_write_guest_cached(vcpu->kvm, &vcpu->arch.apf.cache,
>> + &val, sizeof(val));
>> +}
>> +
>> +bool kvm_arch_can_inject_async_page_not_present(struct kvm_vcpu *vcpu)
>> +{
>> + u64 vbar, pc;
>> + u32 val;
>> + int ret;
>> +
>> + if (!(vcpu->arch.apf.control_block & KVM_ASYNC_PF_ENABLED))
>> + return false;
>> +
>> + if (vcpu->arch.apf.send_user_only && vcpu_mode_priv(vcpu))
>> + return false;
>> +
>> + /* Pending page fault, which ins't acknowledged by guest */
>> + ret = kvm_async_pf_read_cache(vcpu, &val);
>> + if (ret || val)
>> + return false;
>> +
>> + /*
>> + * Events can't be injected through data abort because it's
>> + * going to clobber ELR_EL1, which might not consued (or saved)
>> + * by guest yet.
>> + */
>> + vbar = vcpu_read_sys_reg(vcpu, VBAR_EL1);
>> + pc = *vcpu_pc(vcpu);
>> + if (pc >= vbar && pc < (vbar + vcpu->arch.apf.no_fault_inst_range))
>> + return false;
>
> Ah, so that's when this `no_fault_inst_range` is for.
>
> As-is this is not sufficient, and we'll need t be extremely careful
> here.
>
> The vectors themselves typically only have a small amount of stub code,
> and the bulk of the non-reentrant exception entry work happens
> elsewhere, in a mixture of assembly and C code that isn't even virtually
> contiguous with either the vectors or itself.
>
> It's possible in theory that code in modules (or perhaps in eBPF JIT'd
> code) that isn't safe to take a fault from, so even having a contiguous
> range controlled by the kernel isn't ideal.
>
> How does this work on x86?
>
Yeah, here we just provide a mechanism to forbid injecting data abort. The
range is fed by guest through HVC call. So I think it's guest related issue.
You had more comments about this in PATCH[9]. I will explain a bit more there.
x86 basically relies on EFLAGS[IF] flag. The async page fault can be injected
if it's on. Otherwise, it's forbidden. It's workable because exception is
special interrupt to x86 if I'm correct.
return (vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
!(vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) &
(GUEST_INTR_STATE_STI | GUEST_INTR_STATE_MOV_SS));
>> +
>> + return true;
>> +}
>> +
>> +/*
>> + * We need deliver the page present signal as quick as possible because
>> + * it's performance critical. So the signal is delivered no matter which
>> + * privilege level the guest has. It's possible the signal can't be handled
>> + * by the guest immediately. However, host doesn't contribute the delay
>> + * anyway.
>> + */
>> +bool kvm_arch_can_inject_async_page_present(struct kvm_vcpu *vcpu)
>> +{
>> + u64 vbar, pc;
>> + u32 val;
>> + int ret;
>> +
>> + if (!(vcpu->arch.apf.control_block & KVM_ASYNC_PF_ENABLED))
>> + return true;
>> +
>> + /* Pending page fault, which ins't acknowledged by guest */
>> + ret = kvm_async_pf_read_cache(vcpu, &val);
>> + if (ret || val)
>> + return false;
>> +
>> + /*
>> + * Events can't be injected through data abort because it's
>> + * going to clobber ELR_EL1, which might not consued (or saved)
>> + * by guest yet.
>> + */
>> + vbar = vcpu_read_sys_reg(vcpu, VBAR_EL1);
>> + pc = *vcpu_pc(vcpu);
>> + if (pc >= vbar && pc < (vbar + vcpu->arch.apf.no_fault_inst_range))
>> + return false;
>> +
>> + return true;
>> +}
>
> Much of this is identical to the not_present case, so the same comments
> apply. The common bits should probably be factored out into a helper.
>
Yep, I will take look to introduce a helper function if needed.
>> +
>> +int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu, u32 esr,
>> + gpa_t gpa, gfn_t gfn)
>> +{
>> + struct kvm_arch_async_pf arch;
>> + unsigned long hva = kvm_vcpu_gfn_to_hva(vcpu, gfn);
>> +
>> + arch.token = (vcpu->arch.apf.id++ << 16) | vcpu->vcpu_id;
>> + arch.gfn = gfn;
>> + arch.esr = esr;
>> +
>> + return kvm_setup_async_pf(vcpu, gpa, hva, &arch);
>> +}
>> +
>> +/*
>> + * It's garanteed that no pending asynchronous page fault when this is
>> + * called. It means all previous issued asynchronous page faults have
>> + * been acknoledged.
>> + */
>> +void kvm_arch_async_page_not_present(struct kvm_vcpu *vcpu,
>> + struct kvm_async_pf *work)
>> +{
>> + int ret;
>> +
>> + kvm_async_pf_hash_add(vcpu, work->arch.gfn);
>> + ret = kvm_async_pf_write_cache(vcpu, KVM_PV_REASON_PAGE_NOT_PRESENT);
>> + if (ret) {
>> + kvm_err("%s: Error %d writing cache\n", __func__, ret);
>> + kvm_async_pf_hash_remove(vcpu, work->arch.gfn);
>> + return;
>> + }
>> +
>> + kvm_inject_dabt(vcpu, work->arch.token);
>> +}
>> +
>> +/*
>> + * It's garanteed that no pending asynchronous page fault when this is
>> + * called. It means all previous issued asynchronous page faults have
>> + * been acknoledged.
>> + */
>> +void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
>> + struct kvm_async_pf *work)
>> +{
>> + int ret;
>> +
>> + /* Broadcast wakeup */
>> + if (work->wakeup_all)
>> + work->arch.token = ~0;
>> + else
>> + kvm_async_pf_hash_remove(vcpu, work->arch.gfn);
>> +
>> + ret = kvm_async_pf_write_cache(vcpu, KVM_PV_REASON_PAGE_READY);
>> + if (ret) {
>> + kvm_err("%s: Error %d writing cache\n", __func__, ret);
>> + return;
>> + }
>> +
>> + kvm_inject_dabt(vcpu, work->arch.token);
>
> So the guest sees a fake S1 abort with a fake address?
>
> How is the guest expected to distinguish this from a real S1 fault?
>
KVM_PV_REASON_PAGE_READY is stored in the per-vCPU status block, which
is represented by the following data struct. When the @reason is zero,
it's normal fault. Otherwise, it's a async page fault. So we shouldn't
have async page fault and normal one cross over.
struct kvm_vcpu_pv_apf_data {
__u32 reason;
__u8 pad[60];
__u32 enabled;
};
>> +}
>> +
>> +void kvm_arch_async_page_ready(struct kvm_vcpu *vcpu,
>> + struct kvm_async_pf *work)
>> +{
>> + struct kvm_memory_slot *memslot;
>> + unsigned int esr = work->arch.esr;
>> + phys_addr_t gpa = work->cr2_or_gpa;
>> + gfn_t gfn = gpa >> PAGE_SHIFT;
>
> Perhaps:
>
> gfn_t gfn = gpa_to_gfn(gpa);
>
Sure.
>> + unsigned long hva;
>> + bool write_fault, writable;
>> + int idx;
>> +
>> + /*
>> + * We shouldn't issue prefault for special work to wake up
>> + * all pending tasks because the associated token (address)
>> + * is invalid.
>> + */
>
> I'm not sure what this comment is trying to say.
>
There are two types of workers. One of them is to wakeup all sleepers
in guest, preparing to shutdown the machine. For this type of wakeup,
the token (faulting address) is set to ~0, which is invalid. So there
isn't a prefault issued for this particular wakeup. I'll improve the
comments in next reivion :)
>> + if (work->wakeup_all)
>> + return;
>> +
>> + /*
>> + * The gpa was validated before the work is started. However, the
>> + * memory slots might be changed since then. So we need to redo the
>> + * validatation here.
>> + */
>> + idx = srcu_read_lock(&vcpu->kvm->srcu);
>> +
>> + write_fault = kvm_is_write_fault(esr);
>> + memslot = gfn_to_memslot(vcpu->kvm, gfn);
>> + hva = gfn_to_hva_memslot_prot(memslot, gfn, &writable);
>> + if (kvm_is_error_hva(hva) || (write_fault && !writable))
>> + goto out;
>> +
>> + kvm_handle_user_mem_abort(vcpu, esr, memslot, gpa, hva, true);
>> +
>> +out:
>> + srcu_read_unlock(&vcpu->kvm->srcu, idx);
>> +}
>> +
>> +static long kvm_async_pf_update_enable_reg(struct kvm_vcpu *vcpu, u64 data)
>> +{
>> + bool enabled, enable;
>> + gpa_t gpa = (data & ~0x3F);
>
> What exactly is going on here? Why are the low 7 bits of data not valid?
>
> This will also truncate the value to 32 bits; did you mean to do that?
>
Yes, because the control block (@data) is 64-bytes aligned.
>> + int ret;
>> +
>> + enabled = !!(vcpu->arch.apf.control_block & KVM_ASYNC_PF_ENABLED);
>> + enable = !!(data & KVM_ASYNC_PF_ENABLED);
>> + if (enable == enabled) {
>> + kvm_debug("%s: Async PF has been %s (0x%llx -> 0x%llx)\n",
>> + __func__, enabled ? "enabled" : "disabled",
>> + vcpu->arch.apf.control_block, data);
>> + return SMCCC_RET_NOT_REQUIRED;
>> + }
>> +
>> + if (enable) {
>> + ret = kvm_gfn_to_hva_cache_init(
>> + vcpu->kvm, &vcpu->arch.apf.cache,
>> + gpa + offsetof(struct kvm_vcpu_pv_apf_data, reason),
>> + sizeof(u32));
>> + if (ret) {
>> + kvm_err("%s: Error %d initializing cache on 0x%llx\n",
>> + __func__, ret, data);
>> + return SMCCC_RET_NOT_SUPPORTED;
>> + }
>> +
>> + kvm_async_pf_hash_reset(vcpu);
>> + vcpu->arch.apf.send_user_only =
>> + !(data & KVM_ASYNC_PF_SEND_ALWAYS);
>> + kvm_async_pf_wakeup_all(vcpu);
>> + vcpu->arch.apf.control_block = data;
>> + } else {
>> + kvm_clear_async_pf_completion_queue(vcpu);
>> + vcpu->arch.apf.control_block = data;
>> + }
>> +
>> + return SMCCC_RET_SUCCESS;
>> +}
>> +
>> +long kvm_async_pf_hypercall(struct kvm_vcpu *vcpu)
>> +{
>> + u64 data, func, val, range;
>> + long ret = SMCCC_RET_SUCCESS;
>> +
>> + data = (smccc_get_arg2(vcpu) << 32) | smccc_get_arg1(vcpu);
>
> What prevents the high bits being set in arg1?
>
The function shuld work for both 32-bit and 64-bit guest.
>> + func = data & (0xfful << 56);
>> + val = data & ~(0xfful << 56);
>> + switch (func) {
>> + case BIT(63):
>> + ret = kvm_async_pf_update_enable_reg(vcpu, val);
>
> Please give BIT(63) a mnemonic.
>
>> + break;
>> + case BIT(62):
>
> Likewise.
>
Yes.
>> + if (vcpu->arch.apf.control_block & KVM_ASYNC_PF_ENABLED) {
>> + ret = SMCCC_RET_NOT_SUPPORTED;
>> + break;
>> + }
>> +
>> + range = vcpu->arch.apf.no_fault_inst_range;
>> + vcpu->arch.apf.no_fault_inst_range = max(range, val);
>
> Huh? How is the `no_fault_inst_range` set by he guest?
>
I'll explain to you when replying your comments on PATCH[9] :)
> Thanks,
> Mark.
>
Thanks,
Gavin
>> + break;
>> + default:
>> + kvm_err("%s: Unrecognized function 0x%llx\n", __func__, func);
>> + ret = SMCCC_RET_NOT_SUPPORTED;
>> + }
>> +
>> + return ret;
>> +}
>> diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
>> index db6dce3d0e23..a7e0fe17e2f1 100644
>> --- a/virt/kvm/arm/hypercalls.c
>> +++ b/virt/kvm/arm/hypercalls.c
>> @@ -70,7 +70,15 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
>> break;
>> case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
>> val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
>> +#ifdef CONFIG_KVM_ASYNC_PF
>> + val[0] |= BIT(ARM_SMCCC_KVM_FUNC_APF);
>> +#endif
>> break;
>> +#ifdef CONFIG_KVM_ASYNC_PF
>> + case ARM_SMCCC_VENDOR_HYP_KVM_APF_FUNC_ID:
>> + val[0] = kvm_async_pf_hypercall(vcpu);
>> + break;
>> +#endif
>> default:
>> return kvm_psci_call(vcpu);
>> }
>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>> index 95aaabb2b1fc..a303815845a2 100644
>> --- a/virt/kvm/arm/mmu.c
>> +++ b/virt/kvm/arm/mmu.c
>> @@ -1656,6 +1656,30 @@ static bool fault_supports_stage2_huge_mapping(struct kvm_memory_slot *memslot,
>> (hva & ~(map_size - 1)) + map_size <= uaddr_end;
>> }
>>
>> +static bool try_async_pf(struct kvm_vcpu *vcpu, u32 esr, gpa_t gpa,
>> + gfn_t gfn, kvm_pfn_t *pfn, bool write,
>> + bool *writable, bool prefault)
>> +{
>> + struct kvm_memory_slot *slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
>> +#ifdef CONFIG_KVM_ASYNC_PF
>> + bool async = false;
>> +
>> + /* Bail if *pfn has correct page */
>> + *pfn = __gfn_to_pfn_memslot(slot, gfn, false, &async, write, writable);
>> + if (!async)
>> + return false;
>> +
>> + if (!prefault && kvm_arch_can_inject_async_page_not_present(vcpu)) {
>> + if (kvm_async_pf_hash_find(vcpu, gfn) ||
>> + kvm_arch_setup_async_pf(vcpu, esr, gpa, gfn))
>> + return true;
>> + }
>> +#endif
>> +
>> + *pfn = __gfn_to_pfn_memslot(slot, gfn, false, NULL, write, writable);
>> + return false;
>> +}
>> +
>> int kvm_handle_user_mem_abort(struct kvm_vcpu *vcpu, unsigned int esr,
>> struct kvm_memory_slot *memslot,
>> phys_addr_t fault_ipa, unsigned long hva,
>> @@ -1737,7 +1761,10 @@ int kvm_handle_user_mem_abort(struct kvm_vcpu *vcpu, unsigned int esr,
>> */
>> smp_rmb();
>>
>> - pfn = gfn_to_pfn_prot(kvm, gfn, write_fault, &writable);
>> + if (try_async_pf(vcpu, esr, fault_ipa, gfn, &pfn,
>> + write_fault, &writable, prefault))
>> + return 1;
>> +
>> if (pfn == KVM_PFN_ERR_HWPOISON) {
>> kvm_send_hwpoison_signal(hva, vma_shift);
>> return 0;
>> --
>> 2.23.0
>>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 00/91] drm/vc4: Support BCM2711 Display Pipelin
From: Daniel Drake @ 2020-05-27 3:49 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-arm-kernel, devicetree, Linux Kernel, dri-devel,
Eric Anholt, bcm-kernel-feedback-list, linux-rpi-kernel,
Jian-Hong Pan, Linux Upstreaming Team, linux-clk,
Nicolas Saenz Julienne, linux-i2c
In-Reply-To: <20200526102018.kznh6aglpkqlp6en@gilmour.lan>
Hi Maxime,
On Tue, May 26, 2020 at 6:20 PM Maxime Ripard <maxime@cerno.tech> wrote:
> I gave it a try with U-Boot with my latest work and couldn't reproduce it, so it
> seems that I fixed it along the way
Is your latest work available in a git branch anywhere that we could
test directly?
Thanks
Daniel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 5/8] clk: stm32: Fix stm32f429's ltdc driver hang in set clock rate, fix duplicated ltdc clock register to 'clk_core' case ltdc's clock turn off by clk_disable_unused()
From: dillon min @ 2020-05-27 3:30 UTC (permalink / raw)
To: Stephen Boyd
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linus Walleij, linux-clk, Linux Kernel Mailing List,
open list:DRM PANEL DRIVERS, linux-spi, Mark Brown, linux-stm32,
Linux ARM
In-Reply-To: <159054389592.88029.12389551390229328953@swboyd.mtv.corp.google.com>
Hi Stephen,
Thanks for reviewing.
On Wed, May 27, 2020 at 9:44 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting dillon.minfei@gmail.com (2020-05-24 20:45:45)
> > From: dillon min <dillon.minfei@gmail.com>
> >
> > ltdc set clock rate crashed
> > 'post_div_data[]''s pll_num is PLL_I2S, PLL_SAI (number is 1,2). but,
>
> Please write "post_div_data[]'s" if it is possessive. "But" doesn't
> start a sentence. This is one sentence, not two.
Ok.
>
> > as pll_num is offset of 'clks[]' input to clk_register_pll_div(), which
> > is FCLK, CLK_LSI, defined in 'include/dt-bindings/clock/stm32fx-clock.h'
> > so, this is a null object at the register time.
> > then, in ltdc's clock is_enabled(), enable(), will call to_clk_gate().
> > will return a null object, cause kernel crashed.
> > need change pll_num to PLL_VCO_I2S, PLL_VCO_SAI for 'post_div_data[]'
> >
> > duplicated ltdc clock
> > 'stm32f429_gates[]' has a member 'ltdc' register to 'clk_core', but no
> > upper driver use it, ltdc driver use the lcd-tft defined in
> > 'stm32f429_aux_clk[]'. after system startup, as stm32f429_gates[]'s ltdc
> > enable_count is zero, so turn off by clk_disable_unused()
>
> I sort of follow this. Is this another patch? Seems like two things are
> going on here.
This patch fix two bugs about stm32's clock.
bug1: ltdc driver loading hang in clk_set_rate(), this is due to
misuse ‘PLL_VCO_SAI' and
'PLL_SAI'.
speak in short, from the below code,
’PLL_SAI' is 2, 'PLL_VCO_SAI' is 7.
'post_div' point to 'post_div_data[]', 'post_div->pll_num' is
PLL_I2S, PLL_SAI.
'clks[PLL_VCOM_SAI' has vaild 'struct clk_hw* ' return from
stm32f4_rcc_register_pll()
but, at line 1776, use the 'clks[post_div->pll_num]', equal to
'clks[PLL_SAI]', this is invaild
at that time.
include/dt-bindings/clock/stm32fx-clock.h
29 #define PLL_VCO_SAI 7
drivers/clk/clk-stm32f4.c
494 enum {
495 PLL,
496 PLL_I2S,
497 PLL_SAI,
498 };
558 static const struct stm32f4_pll_post_div_data
post_div_data[MAX_POST_DIV] = {
559 { CLK_I2SQ_PDIV, PLL_I2S, "plli2s-q-div", "plli2s-q",
560 CLK_SET_RATE_PARENT, STM32F4_RCC_DCKCFGR, 0, 5, 0, NULL},
561
562 { CLK_SAIQ_PDIV, PLL_SAI, "pllsai-q-div", "pllsai-q",
563 CLK_SET_RATE_PARENT, STM32F4_RCC_DCKCFGR, 8, 5, 0, NULL },
564
565 { NO_IDX, PLL_SAI, "pllsai-r-div", "pllsai-r", CLK_SET_RATE_PARENT,
566 STM32F4_RCC_DCKCFGR, 16, 2, 0, post_divr_table },
567 };
1759 clks[PLL_VCO_SAI] = stm32f4_rcc_register_pll("vco_in",
1760 &data->pll_data[2], &stm32f4_clk_lock);
1761
1762 for (n = 0; n < MAX_POST_DIV; n++) {
1763 const struct stm32f4_pll_post_div_data *post_div;
1764 struct clk_hw *hw;
1765
1766 post_div = &post_div_data[n];
1767
1768 hw = clk_register_pll_div(post_div->name,
1769 post_div->parent,
1770 post_div->flag,
1771 base + post_div->offset,
1772 post_div->shift,
1773 post_div->width,
1774 post_div->flag_div,
1775 post_div->div_table,
1776 clks[post_div->pll_num],
1777 &stm32f4_clk_lock);
1778
1779 if (post_div->idx != NO_IDX)
1780 clks[post_div->idx] = hw;
1781 }
bug2: ltdc's clock turn off by clk_disable_unused()
from your comments at '[PATCH v3 4/5] clk: stm32: Fix stm32f429 ltdc
driver loading hang
in clk set rate. keep ltdc clk running after kernel startup' , i go
deep into the code, found
stm32's clk driver register two gate clk to clk core by
clk_hw_register_gate() and
clk_hw_register_composite()
first: 'stm32f429_gates[]', clk name is 'ltdc', this is no user used.
second: 'stm32f429_aux_clk[]', clk name is 'lcd-tft', this is used by
ltdc driver
both of them point to the same offset of stm32's RCC register. after
kernel enter console,
clk core turn off ltdc's clk as 'stm32f429_gates[]' is unused. but,
actually 'stm32f429_aux_clk[]'
is in use.
i can separate this patch to two, each bug a patch if necessary
>
> >
> > Changes since V3:
> > 1 drop last wrong changes about 'CLK_IGNORE_UNUSED' patch
> > 2 fix PLL_SAI mismatch with PLL_VCO_SAI
>
> This change log goes under the --- below.
ok
>
> >
> > Signed-off-by: dillon min <dillon.minfei@gmail.com>
>
> Any Fixes tag?
ok, will add --fixup in git commit next time, this patch fix two bugs,
i should make two commit, each one has a
fixes tag, right?
first point to '517633e clk: stm32f4: Add post divisor for I2S & SAI PLLs'
second point to 'daf2d11 clk: stm32f4: Add lcd-tft clock'
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [RFC] Use SMMU HTTU for DMA dirty page tracking
From: Tian, Kevin @ 2020-05-27 3:27 UTC (permalink / raw)
To: Xiang Zheng, Jean-Philippe Brucker
Cc: Zhao, Yan Y, Suzuki K Poulose, maz@kernel.org,
iommu@lists.linux-foundation.org, Kirti Wankhede,
alex.williamson@redhat.com, James Morse,
linux-arm-kernel@lists.infradead.org, prime.zeng@hisilicon.com,
Wang Haibin, Will Deacon, kvmarm@lists.cs.columbia.edu,
julien.thierry.kdev@gmail.com
In-Reply-To: <e68c1158-8573-a477-42ce-48cee510c3ce@huawei.com>
> From: Xiang Zheng
> Sent: Monday, May 25, 2020 7:34 PM
>
> [+cc Kirti, Yan, Alex]
>
> On 2020/5/23 1:14, Jean-Philippe Brucker wrote:
> > Hi,
> >
> > On Tue, May 19, 2020 at 05:42:55PM +0800, Xiang Zheng wrote:
> >> Hi all,
> >>
> >> Is there any plan for enabling SMMU HTTU?
> >
> > Not outside of SVA, as far as I know.
> >
>
> >> I have seen the patch locates in the SVA series patch, which adds
> >> support for HTTU:
> >> https://www.spinics.net/lists/arm-kernel/msg798694.html
> >>
> >> HTTU reduces the number of access faults on SMMU fault queue
> >> (permission faults also benifit from it).
> >>
> >> Besides reducing the faults, HTTU also helps to track dirty pages for
> >> device DMA. Is it feasible to utilize HTTU to get dirty pages on device
> >> DMA during VFIO live migration?
> >
> > As you know there is a VFIO interface for this under discussion:
> > https://lore.kernel.org/kvm/1589781397-28368-1-git-send-email-
> kwankhede@nvidia.com/
> > It doesn't implement an internal API to communicate with the IOMMU
> driver
> > about dirty pages.
We plan to add such API later, e.g. to utilize A/D bit in VT-d 2nd-level
page tables (Rev 3.0).
>
> >
> >> If SMMU can track dirty pages, devices are not required to implement
> >> additional dirty pages tracking to support VFIO live migration.
> >
> > It seems feasible, though tracking it in the device might be more
> > efficient. I might have misunderstood but I think for live migration of
> > the Intel NIC they trap guest accesses to the device and introspect its
> > state to figure out which pages it is accessing.
Does HTTU implement A/D-like mechanism in SMMU page tables, or just
report dirty pages in a log buffer? Either way tracking dirty pages in IOMMU
side is generic thus doesn't require device-specific tweak like in Intel NIC.
Thanks
kevin
> >
> > With HTTU I suppose (without much knowledge about live migration) that
> > you'd need several new interfaces to the IOMMU drivers:
> >
> > * A way for VFIO to query HTTU support in the SMMU. There are some
> > discussions about communicating more IOMMU capabilities through VFIO
> but
> > no implementation yet. When HTTU isn't supported the DIRTY_PAGES
> bitmap
> > would report all pages as they do now.
> >
> > * VFIO_IOMMU_DIRTY_PAGES_FLAG_START/STOP would clear the dirty bit
> > for all VFIO mappings (which is going to take some time). There is a
> > walker in io-pgtable for iova_to_phys() which could be extended. I
> > suppose it's also possible to atomically switch the HA and HD bits in
> > context descriptors.
>
> Maybe we need not switch HA and HD bits, just turn on them all the time?
>
> >
> > * VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP would query the dirty
> bit for all
> > VFIO mappings.
> >
>
> I think we need to consider the case of IOMMU dirty pages logging. We want
> to test Kirti's VFIO migration patches combined with SMMU HTTU, any
> suggestions?
>
> --
> Thanks,
> Xiang
>
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v7 3/3] clk: at91: allow setting all PMC clock parents via DT
From: Stephen Boyd @ 2020-05-27 3:23 UTC (permalink / raw)
To: Alexandre Belloni, Ludovic Desroches, Michael Turquette,
Michał Mirosław, Nicolas Ferre, Rob Herring
Cc: devicetree, linux-clk, linux-arm-kernel, linux-kernel
In-Reply-To: <fa39cc10dab8341ea4bc2b7152be9217b2cd34a5.1588630999.git.mirq-linux@rere.qmqm.pl>
Quoting Michał Mirosław (2020-05-04 15:37:57)
> We need to have clocks accessible via phandle to select them
> as peripheral clock parent using assigned-clock-parents in DT.
> Add support for PLLACK/PLLBCK/AUDIOPLLCK clocks where available.
>
> Signed-off-by: Micha\u0142 Miros\u0142aw <mirq-linux@rere.qmqm.pl>
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
Applied to clk-next
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v7 1/3] clk: at91: optimize pmc data allocation
From: Stephen Boyd @ 2020-05-27 3:23 UTC (permalink / raw)
To: Alexandre Belloni, Ludovic Desroches, Michael Turquette,
Michał Mirosław, Nicolas Ferre, Rob Herring
Cc: devicetree, linux-clk, linux-arm-kernel, linux-kernel
In-Reply-To: <fc6f6d67b8cee0beace4a9d9cca7431e5efa769d.1588630999.git.mirq-linux@rere.qmqm.pl>
Quoting Michał Mirosław (2020-05-04 15:37:56)
> Alloc whole data structure in one block. This makes the code shorter,
> more efficient and easier to extend in following patch.
>
> Signed-off-by: Micha\u0142 Miros\u0142aw <mirq-linux@rere.qmqm.pl>
> ---
Applied to clk-next
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v7 2/3] clk: at91: allow setting PCKx parent via DT
From: Stephen Boyd @ 2020-05-27 3:23 UTC (permalink / raw)
To: Alexandre Belloni, Ludovic Desroches, Michael Turquette,
Michał Mirosław, Nicolas Ferre, Rob Herring
Cc: devicetree, linux-clk, linux-arm-kernel, linux-kernel
In-Reply-To: <0054532c00163ddf405dad658b32f0d7d97fcc8e.1588630999.git.mirq-linux@rere.qmqm.pl>
Quoting Michał Mirosław (2020-05-04 15:37:56)
> This exposes PROGx clocks for use in assigned-clocks DeviceTree property
> for selecting PCKx parent clock.
>
> Signed-off-by: Micha\u0142 Miros\u0142aw <mirq-linux@rere.qmqm.pl>
> ---
Applied to clk-next
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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