mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Sean Christopherson <seanjc@google.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: allow compiling out SMM support
Date: Wed, 28 Sep 2022 13:41:47 +0200	[thread overview]
Message-ID: <f708d769-5d93-351f-ea24-8fa7deb9f689@redhat.com> (raw)
In-Reply-To: <YzM55hqavzENQq7I@google.com>

On 9/27/22 19:59, Sean Christopherson wrote:
>> 	The patch isn't pretty.  I could skip all the changes to add WARNs
>> 	to called functions, but the point of adding the config symbol is
>> 	to make sure that those functions, and all the baggage they bring,
>> 	are dead.
> 
> I would much rather we go even further and completely kill off those functions
> at compile time.

Ok, but then we should go all the way and move as much as possible to a 
separate file.  This also means moving the out-of-SMM flow away from the 
emulator, which in turn enables using ctxt only for the GPRs and not for 
ctxt->ops.

I have already done all that and it's quite a bit nicer; I'll send it 
once I've tested it with more than just smm_test.  I left a couple stubs 
behind where the balance seemed to be better that way (mostly for use in 
kvm_vcpu_ioctl_x86_set_vcpu_events), but most of the code is compiled out.

> There are side effects that should also be eliminated, e.g. x86 should not define
> __KVM_VCPU_MULTIPLE_ADDRESS_SPACE so that usersepace can't create memslots for
> SMM.  Dropping the functions entirely wrapping those #defines in #ifdef as well,
> and so makes it all but impossible for KVM to do anything SMM related.
> 
> Eliminating those at compile time requires a bit more #ifdeffery, but it's not
> awful, and IMO it's better than sprinkling WARNs in a bunch of paths.  KVM_REQ_SMI
> in particular might be going too far, but even for that one I vote to kill it.

Sounds good, though of course some of the various cleanups are best done 
in separate patches.

>>  static int kvm_vcpu_ioctl_smi(struct kvm_vcpu *vcpu)
>>  {
>> -	kvm_make_request(KVM_REQ_SMI, vcpu);
>> -
>> +	if (IS_ENABLED(CONFIG_KVM_SMM))
>> +		kvm_make_request(KVM_REQ_SMI, vcpu);
>>  	return 0;
> 
> This should return -EINVAL, not 0.

I'm a bit wary of changing this in case userspace is relying on it not 
failing, because the paths that lead to the failing ioctl are most 
likely controlled by the guest.

Paolo


  reply	other threads:[~2022-09-28 11:42 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-27 15:22 Paolo Bonzini
2022-09-27 17:59 ` Sean Christopherson
2022-09-28 11:41   ` Paolo Bonzini [this message]
2022-09-28 14:59     ` Sean Christopherson
2022-09-28 16:05       ` Paolo Bonzini
2022-09-29 15:49 ` Maciej S. Szmigiero
2022-09-29 16:47   ` Paolo Bonzini
2022-09-29 20:38     ` Sean Christopherson

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=f708d769-5d93-351f-ea24-8fa7deb9f689@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=seanjc@google.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®