All of lore.kernel.org
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Andrew Morton <akpm@osdl.org>,
	Eduardo Habkost <ehabkost@redhat.com>,
	kvm@vger.kernel.org, kexec@lists.infradead.org,
	Haren Myneni <hbabu@us.ibm.com>,
	Simon Horman <horms@verge.net.au>,
	Vivek Goyal <vgoyal@redhat.com>
Subject: Re: [PATCH 0/2] kvm: disable virtualization on kdump
Date: Mon, 27 Oct 2008 11:13:41 +0200	[thread overview]
Message-ID: <49058645.9010005@redhat.com> (raw)
In-Reply-To: <m1bpx6bxhm.fsf@frodo.ebiederm.org>

Eric W. Biederman wrote:
>> NMI IPIs are already used on x86 native_machine_crash_shutdown(), so
>> it wouldn't get more messy that it is currently. We just need to add
>> another bit of code to the code that already runs on an NMI handler.
>>     
>
> Yes.  And handling of those NMIs is best effort.  Nothing fails if
> they don't actually run.
>
>   

Unless someone can come up with another way to disable vmx remotely, 
that's going to change if you have vmx enabled.

> Well we could fairly easily have a non-modular function that does.
> if (vmx_present && vmx_enabled) {
>    turn_off_vmx();
> }
>
> Which at first skim looks like it is all of about 10-20 machine
> instructions.
>
>   

There's no way to query whether vmx is enabled or disabled, AFAICT.  So 
we have to execute vmxoff and ignore possible #UDs.

If we trust the exception handlers, there's no problem.  Otherwise we 
need to replace the current #UD handler with an iret (perhaps switching 
temporarily to another IDT).

> There are a few real places where we need code on the kdump
> path because there it is not possible to do the work any
> other way.  However we need to think long and hard about
> that because placing the code anywhere besides in a broken
> and failing kernel is going to be easier to maintain and
> more reliable.
>   

vmx blocking INITs makes it impossible to leave this to the new kernel.

> I oppose an atomic notifier because it makes the review
> essentially impossible.  If any module can come in and register
> a notifier we can't know what code is running on that code
> path and we can't be certain the code is safe in an abnormal
> case to run on that code path.
>   

What if it's a specialized notifier for kexec?  Or even kexec_crash?

That said, I have no issue with static code at the call site.

> Right now we only need to support vmx on the kdump path because
> of what appears to be a hardware design bug.  Enabling vmx
> apparently disables standard functions like an INIT IPI.  Things
> like this do happen but they should be rare.
>   

The general kexec path also wants this fixed.

-- 
error compiling committee.c: too many arguments to function


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: Avi Kivity <avi-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Cc: Andrew Morton <akpm-3NddpPZAyC0@public.gmane.org>,
	Eduardo Habkost
	<ehabkost-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Haren Myneni <hbabu-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>,
	Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>,
	Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Subject: Re: [PATCH 0/2] kvm: disable virtualization on kdump
Date: Mon, 27 Oct 2008 11:13:41 +0200	[thread overview]
Message-ID: <49058645.9010005@redhat.com> (raw)
In-Reply-To: <m1bpx6bxhm.fsf-B27657KtZYmhTnVgQlOflh2eb7JE58TQ@public.gmane.org>

Eric W. Biederman wrote:
>> NMI IPIs are already used on x86 native_machine_crash_shutdown(), so
>> it wouldn't get more messy that it is currently. We just need to add
>> another bit of code to the code that already runs on an NMI handler.
>>     
>
> Yes.  And handling of those NMIs is best effort.  Nothing fails if
> they don't actually run.
>
>   

Unless someone can come up with another way to disable vmx remotely, 
that's going to change if you have vmx enabled.

> Well we could fairly easily have a non-modular function that does.
> if (vmx_present && vmx_enabled) {
>    turn_off_vmx();
> }
>
> Which at first skim looks like it is all of about 10-20 machine
> instructions.
>
>   

There's no way to query whether vmx is enabled or disabled, AFAICT.  So 
we have to execute vmxoff and ignore possible #UDs.

If we trust the exception handlers, there's no problem.  Otherwise we 
need to replace the current #UD handler with an iret (perhaps switching 
temporarily to another IDT).

> There are a few real places where we need code on the kdump
> path because there it is not possible to do the work any
> other way.  However we need to think long and hard about
> that because placing the code anywhere besides in a broken
> and failing kernel is going to be easier to maintain and
> more reliable.
>   

vmx blocking INITs makes it impossible to leave this to the new kernel.

> I oppose an atomic notifier because it makes the review
> essentially impossible.  If any module can come in and register
> a notifier we can't know what code is running on that code
> path and we can't be certain the code is safe in an abnormal
> case to run on that code path.
>   

What if it's a specialized notifier for kexec?  Or even kexec_crash?

That said, I have no issue with static code at the call site.

> Right now we only need to support vmx on the kdump path because
> of what appears to be a hardware design bug.  Enabling vmx
> apparently disables standard functions like an INIT IPI.  Things
> like this do happen but they should be rare.
>   

The general kexec path also wants this fixed.

-- 
error compiling committee.c: too many arguments to function

  reply	other threads:[~2008-10-27  9:13 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-20 15:01 [PATCH 0/2] kvm: disable virtualization on kdump Eduardo Habkost
2008-10-20 15:01 ` Eduardo Habkost
2008-10-20 15:01 ` [PATCH 1/2] kdump: crash-time CPU halt notifier interface Eduardo Habkost
2008-10-20 15:01   ` Eduardo Habkost
2008-10-20 15:01 ` [PATCH 2/2] kvm: disable virtualization when halting CPUs on crash Eduardo Habkost
2008-10-20 15:01   ` Eduardo Habkost
2008-10-22 23:28 ` [PATCH 0/2] kvm: disable virtualization on kdump Simon Horman
2008-10-22 23:28   ` Simon Horman
2008-10-23 19:41   ` Eduardo Habkost
2008-10-23 19:41     ` Eduardo Habkost
2008-10-23 22:29     ` Simon Horman
2008-10-23 22:29       ` Simon Horman
2008-10-24  1:00       ` Eric W. Biederman
2008-10-24  1:00         ` Eric W. Biederman
2008-10-26 12:49         ` Avi Kivity
2008-10-26 12:49           ` Avi Kivity
2008-10-26 14:46           ` Eric W. Biederman
2008-10-26 14:46             ` Eric W. Biederman
2008-10-26 15:07             ` Avi Kivity
2008-10-26 15:07               ` Avi Kivity
2008-10-26 21:39               ` Eduardo Habkost
2008-10-26 21:39                 ` Eduardo Habkost
2008-10-27  2:08                 ` Eric W. Biederman
2008-10-27  2:08                   ` Eric W. Biederman
2008-10-27  9:13                   ` Avi Kivity [this message]
2008-10-27  9:13                     ` Avi Kivity
2008-10-27 12:28                     ` Eduardo Habkost
2008-10-27 12:28                       ` Eduardo Habkost
2008-10-27 14:02                       ` Avi Kivity
2008-10-27 14:02                         ` Avi Kivity
2008-10-27 17:32                         ` Eric W. Biederman
2008-10-27 17:32                           ` Eric W. Biederman
2008-10-28 19:45                           ` Eduardo Habkost
2008-10-28 19:45                             ` Eduardo Habkost
2008-10-28 20:13                             ` Eric W. Biederman
2008-10-28 20:13                               ` Eric W. Biederman
2008-10-29  9:41                               ` Avi Kivity
2008-10-29  9:41                                 ` Avi Kivity
2008-10-29 14:54                                 ` Eric W. Biederman
2008-10-29 14:54                                   ` Eric W. Biederman
2008-10-29 17:03                                   ` Avi Kivity
2008-10-29 17:03                                     ` Avi Kivity
2008-10-30  1:33                                     ` Eric W. Biederman
2008-10-30  1:33                                       ` Eric W. Biederman
2008-10-30  7:35                                       ` Chris Lalancette
2008-10-30  7:35                                         ` Chris Lalancette
2008-10-30  7:43                                         ` Avi Kivity
2008-10-30  7:43                                           ` Avi Kivity
2008-10-30  7:52                                       ` Avi Kivity
2008-10-30  7:52                                         ` Avi Kivity
2008-10-29  9:31                             ` Avi Kivity
2008-10-29  9:31                               ` Avi Kivity
2008-10-27 15:05                     ` Eric W. Biederman
2008-10-27 15:05                       ` Eric W. Biederman
2008-10-27 15:50                       ` Eduardo Habkost
2008-10-27 15:50                         ` Eduardo Habkost
2008-10-27  8:54                 ` Avi Kivity
2008-10-27  8:54                   ` Avi Kivity
2008-10-27 13:09                   ` Vivek Goyal
2008-10-27 13:09                     ` Vivek Goyal
2008-10-27 14:04                     ` Avi Kivity
2008-10-27 14:04                       ` Avi Kivity
2008-10-29 20:10                     ` Eduardo Habkost
2008-10-29 20:10                       ` Eduardo Habkost
2008-10-29 20:29                       ` Avi Kivity
2008-10-29 20:29                         ` Avi Kivity
2008-10-29 21:05                       ` Vivek Goyal
2008-10-29 21:05                         ` Vivek Goyal
2008-10-30  0:58                         ` Eric W. Biederman
2008-10-30  0:58                           ` Eric W. Biederman
2008-10-26 21:47               ` Eric W. Biederman
2008-10-26 21:47                 ` Eric W. Biederman
2008-10-27  8:59                 ` Avi Kivity
2008-10-27  8:59                   ` Avi Kivity
2008-10-27 15:02                   ` Eric W. Biederman
2008-10-27 15:02                     ` Eric W. Biederman
2008-10-27 15:38                     ` Eduardo Habkost
2008-10-27 15:38                       ` Eduardo Habkost
2008-10-26 12:46     ` Avi Kivity
2008-10-26 12:46       ` Avi Kivity

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49058645.9010005@redhat.com \
    --to=avi@redhat.com \
    --cc=akpm@osdl.org \
    --cc=ebiederm@xmission.com \
    --cc=ehabkost@redhat.com \
    --cc=hbabu@us.ibm.com \
    --cc=horms@verge.net.au \
    --cc=kexec@lists.infradead.org \
    --cc=kvm@vger.kernel.org \
    --cc=vgoyal@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.