| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20130208013947.GA3364@amt.cnet>
Date: Thu, 7 Feb 2013 23:39:47 -0200
From: Marcelo Tosatti <mtosatti@...hat.com>
To: Hu Tao <hutao@...fujitsu.com>
Cc: kvm list <kvm@...r.kernel.org>, qemu-devel <qemu-devel@...gnu.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Daniel P. Berrange" <berrange@...hat.com>,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
Jan Kiszka <jan.kiszka@...mens.com>,
Gleb Natapov <gleb@...hat.com>,
Blue Swirl <blauwirbel@...il.com>,
Eric Blake <eblake@...hat.com>,
Andrew Jones <drjones@...hat.com>,
Sasha Levin <levinsasha928@...il.com>,
Luiz Capitulino <lcapitulino@...hat.com>,
Wen Congyang <wency@...fujitsu.com>
Subject: Re: [PATCH v12 rebased] kvm: notify host when the guest is panicked
Hi,
On Wed, Jan 23, 2013 at 03:19:21PM +0800, Hu Tao wrote:
> We can know the guest is panicked when the guest runs on xen.
> But we do not have such feature on kvm.
>
> Another purpose of this feature is: management app(for example:
> libvirt) can do auto dump when the guest is panicked. If management
> app does not do auto dump, the guest's user can do dump by hand if
> he sees the guest is panicked.
>
> We have three solutions to implement this feature:
> 1. use vmcall
> 2. use I/O port
> 3. use virtio-serial.
>
> We have decided to avoid touching hypervisor. The reason why I choose
> choose the I/O port is:
> 1. it is easier to implememt
> 2. it does not depend any virtual device
> 3. it can work when starting the kernel
>
> Signed-off-by: Wen Congyang <wency@...fujitsu.com>
> Signed-off-by: Hu Tao <hutao@...fujitsu.com>
> ---
> arch/ia64/kvm/irq.h | 19 +++++++++++++
> arch/powerpc/include/asm/kvm_para.h | 18 ++++++++++++
> arch/s390/include/asm/kvm_para.h | 19 +++++++++++++
> arch/x86/include/asm/kvm_para.h | 20 ++++++++++++++
> arch/x86/include/uapi/asm/kvm_para.h | 2 ++
> arch/x86/kernel/kvm.c | 53 ++++++++++++++++++++++++++++++++++++
> include/linux/kvm_para.h | 18 ++++++++++++
> include/uapi/linux/kvm_para.h | 6 ++++
> kernel/panic.c | 4 +++
> 9 files changed, 159 insertions(+)
>
> diff --git a/arch/ia64/kvm/irq.h b/arch/ia64/kvm/irq.h
> index c0785a7..b3870f8 100644
> --- a/arch/ia64/kvm/irq.h
> +++ b/arch/ia64/kvm/irq.h
> @@ -30,4 +30,23 @@ static inline int irqchip_in_kernel(struct kvm *kvm)
> return 1;
> }
>
> +static inline int kvm_arch_pv_event_init(void)
> +{
> + return 0;
> +}
> +
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> + return 0;
> +}
> +
> +static inline void kvm_arch_pv_eject_event(unsigned int event)
> +{
> +}
> +
> +static inline bool kvm_arch_pv_event_enabled(void)
> +{
> + return false;
> +}
> +
The interface is x86 only, no need to touch other architectures.
> #endif
> diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h
> index 2b11965..17dd013 100644
> --- a/arch/powerpc/include/asm/kvm_para.h
> +++ b/arch/powerpc/include/asm/kvm_para.h
> @@ -144,4 +144,22 @@ static inline bool kvm_check_and_clear_guest_paused(void)
> return false;
> }
>
> +static inline int kvm_arch_pv_event_init(void)
> +{
> + return 0;
> +}
> +
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> + return 0;
> +}
> +
> +static inline void kvm_arch_pv_eject_event(unsigned int event)
> +{
> +}
> +
> +static inline bool kvm_arch_pv_event_enabled(void)
> +{
> + return false;
> +}
> #endif /* __POWERPC_KVM_PARA_H__ */
> diff --git a/arch/s390/include/asm/kvm_para.h b/arch/s390/include/asm/kvm_para.h
> index e0f8423..81d87ec 100644
> --- a/arch/s390/include/asm/kvm_para.h
> +++ b/arch/s390/include/asm/kvm_para.h
> @@ -154,4 +154,23 @@ static inline bool kvm_check_and_clear_guest_paused(void)
> return false;
> }
>
> +static inline int kvm_arch_pv_event_init(void)
> +{
> + return 0;
> +}
> +
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> + return 0;
> +}
> +
> +static inline void kvm_arch_pv_eject_event(unsigned int event)
> +{
> +}
> +
> +static inline bool kvm_arch_pv_event_enabled(void)
> +{
> + return false;
> +}
> +
> #endif /* __S390_KVM_PARA_H */
> --- a/arch/x86/include/asm/kvm_para.h
> +++ b/arch/x86/include/asm/kvm_para.h
> @@ -133,4 +133,24 @@ static inline void kvm_disable_steal_time(void)
> }
> #endif
>
> +static inline int kvm_arch_pv_event_init(void)
> +{
> + if (!request_region(KVM_PV_EVENT_PORT, 4, "KVM_PV_EVENT"))
> + return -1;
> +
> + return 0;
> +}
This should be in a driver in arch/x86/kernel/kvm-panic.c, or so.
> +
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> + return inl(KVM_PV_EVENT_PORT);
> +}
> +
> +static inline void kvm_arch_pv_eject_event(unsigned int event)
> +{
> + outl(event, KVM_PV_EVENT_PORT);
> +}
> +
> +bool kvm_arch_pv_event_enabled(void);
> +
> #endif /* _ASM_X86_KVM_PARA_H */
> diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
> index 06fdbd9..c15ef33 100644
> --- a/arch/x86/include/uapi/asm/kvm_para.h
> +++ b/arch/x86/include/uapi/asm/kvm_para.h
> @@ -96,5 +96,7 @@ struct kvm_vcpu_pv_apf_data {
> #define KVM_PV_EOI_ENABLED KVM_PV_EOI_MASK
> #define KVM_PV_EOI_DISABLED 0x0
>
> +#define KVM_PV_EVENT_PORT (0x505UL)
> +
No need for the ioport to be hard coded. What are the options to
communicate an address to the guest? An MSR, via ACPI?
>
> #endif /* _UAPI_ASM_X86_KVM_PARA_H */
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 9c2bd8b..0aa7b3e 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -73,6 +73,20 @@ static int parse_no_kvmclock_vsyscall(char *arg)
>
> early_param("no-kvmclock-vsyscall", parse_no_kvmclock_vsyscall);
>
> +static int pv_event = 1;
> +static int parse_no_pv_event(char *arg)
> +{
> + pv_event = 0;
> + return 0;
> +}
> +
> +bool kvm_arch_pv_event_enabled(void)
> +{
> + return !!pv_event;
> +}
> +
> +early_param("no-pv-event", parse_no_pv_event);
> +
"pv-event" is a bad name for an interface which is specific to notify
panic events. Please use pv-panic everywhere.
> static DEFINE_PER_CPU(struct kvm_vcpu_pv_apf_data, apf_reason) __aligned(64);
> static DEFINE_PER_CPU(struct kvm_steal_time, steal_time) __aligned(64);
> static int has_steal_clock = 0;
> @@ -385,6 +399,17 @@ static struct notifier_block kvm_pv_reboot_nb = {
> .notifier_call = kvm_pv_reboot_notify,
> };
>
> +static int
> +kvm_pv_panic_notify(struct notifier_block *nb, unsigned long code, void *unused)
> +{
> + kvm_pv_eject_event(KVM_PV_EVENT_PANICKED);
> + return NOTIFY_DONE;
> +}
Why 'eject' ?
> +
> +static struct notifier_block kvm_pv_panic_nb = {
> + .notifier_call = kvm_pv_panic_notify,
> +};
> +
> static u64 kvm_steal_clock(int cpu)
> {
> u64 steal;
> @@ -462,6 +487,34 @@ static void __init kvm_apf_trap_init(void)
> set_intr_gate(14, &async_page_fault);
> }
>
> +static void __init kvm_pv_panicked_event_init(void)
> +{
> + if (!kvm_para_available())
> + return;
> +
> + if (kvm_pv_has_feature(KVM_PV_FEATURE_PANICKED))
> + atomic_notifier_chain_register(&panic_notifier_list,
> + &kvm_pv_panic_nb);
> +}
> +
> +static inline int kvm_pv_event_init(void)
> +{
> + return kvm_arch_pv_event_init();
> +}
> +
> +static int __init enable_pv_event(void)
> +{
> + if (pv_event) {
> + if (kvm_pv_event_init())
> + return 0;
> +
> + kvm_pv_panicked_event_init();
> + }
> +
> + return 0;
> +}
> +arch_initcall(enable_pv_event);
Call the initialization code from kvm_guest_init, only one function is
necessary.
> +
> void __init kvm_guest_init(void)
> {
> int i;
> diff --git a/include/linux/kvm_para.h b/include/linux/kvm_para.h
> index 00a97bb..6fb6198 100644
> --- a/include/linux/kvm_para.h
> +++ b/include/linux/kvm_para.h
> @@ -10,4 +10,22 @@ static inline int kvm_para_has_feature(unsigned int feature)
> return 1;
> return 0;
> }
> +
> +static inline int kvm_pv_has_feature(unsigned int feature)
> +{
> + if (kvm_arch_pv_features() & (1UL << feature))
> + return 1;
> + return 0;
> +}
> +
> +static inline void kvm_pv_eject_event(unsigned int event)
> +{
> + kvm_arch_pv_eject_event(event);
> +}
> +
> +static inline bool kvm_pv_event_enabled(void)
> +{
> + return kvm_arch_pv_event_enabled();
> +}
No need for this helpers, as noted.
> #endif /* __LINUX_KVM_PARA_H */
> diff --git a/include/uapi/linux/kvm_para.h b/include/uapi/linux/kvm_para.h
> index cea2c5c..c41ddce 100644
> --- a/include/uapi/linux/kvm_para.h
> +++ b/include/uapi/linux/kvm_para.h
> @@ -20,6 +20,12 @@
> #define KVM_HC_FEATURES 3
> #define KVM_HC_PPC_MAP_MAGIC_PAGE 4
>
> +/* The bit of supported pv event */
> +#define KVM_PV_FEATURE_PANICKED 0
> +
> +/* The pv event value */
> +#define KVM_PV_EVENT_PANICKED 1
> +
This is a hypercall header. You want
arch/x86/include/asm/kvm_para.h
> /*
> * hypercalls use architecture specific
> */
> diff --git a/kernel/panic.c b/kernel/panic.c
> index e1b2822..a764d2e 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -23,6 +23,7 @@
> #include <linux/init.h>
> #include <linux/nmi.h>
> #include <linux/dmi.h>
> +#include <linux/kvm_para.h>
>
> #define PANIC_TIMER_STEP 100
> #define PANIC_BLINK_SPD 18
> @@ -132,6 +133,9 @@ void panic(const char *fmt, ...)
> if (!panic_blink)
> panic_blink = no_blink;
>
> + if (kvm_pv_event_enabled())
> + panic_timeout = 0;
> +
What is the rationale behind this?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists