public inbox for linux-s390@vger.kernel.org
 help / color / mirror / Atom feed
From: Eric Farman <farman@linux.ibm.com>
To: Christian Borntraeger <borntraeger@linux.ibm.com>,
	Janosch Frank <frankja@linux.ibm.com>,
	Claudio Imbrenda <imbrenda@linux.ibm.com>,
	David Hildenbrand <david@redhat.com>
Cc: kvm@vger.kernel.org, linux-s390@vger.kernel.org,
	Eric Farman <farman@linux.ibm.com>
Subject: [RFC PATCH] KVM: s390: remove extra copy of access registers into KVM_RUN
Date: Wed, 31 Jan 2024 21:58:32 +0100	[thread overview]
Message-ID: <20240131205832.2179029-1-farman@linux.ibm.com> (raw)

The routine ar_translation() is called by get_vcpu_asce(), which is
called from a handful of places, such as an interception that is
being handled during KVM_RUN processing. In that case, the access
registers of the vcpu had been saved to a host_acrs struct and then
the guest access registers loaded from the KVM_RUN struct prior to
entering SIE. Saving them back to KVM_RUN at this point doesn't do
any harm, since it will be done again at the end of the KVM_RUN
loop when the host access registers are restored.

But that's not the only path into this code. The MEM_OP ioctl can
be used while specifying an access register, and will arrive here.

Linux itself doesn't use the access registers for much, but it does
squirrel the thread local storage variable into ACRs 0 and 1 in
copy_thread() [1]. This means that the MEM_OP ioctl may copy
non-zero access registers (the upper- and lower-halves of the TLS
pointer) to the KVM_RUN struct, which will end up getting propogated
to the guest once KVM_RUN ioctls occur. Since these are almost
certainly invalid as far as an ALET goes, an ALET Specification
Exception would be triggered if it were attempted to be used.

[1] arch/s390/kernel/process.c:169

Signed-off-by: Eric Farman <farman@linux.ibm.com>
---

Notes:
    I've gone back and forth about whether the correct fix is
    to simply remove the save_access_regs() call and inspect
    the contents from the most recent KVM_RUN directly, versus
    storing the contents locally. Both work for me but I've
    opted for the latter, as it continues to behave the same
    as it does today but without the implicit use of the
    KVM_RUN space. As it is, this is (was) the only reference
    to vcpu->run in this file, which stands out since the
    routines are used by other callers.
    
    Curious about others' thoughts.

 arch/s390/kvm/gaccess.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index 5bfcc50c1a68..9205496195a4 100644
--- a/arch/s390/kvm/gaccess.c
+++ b/arch/s390/kvm/gaccess.c
@@ -380,6 +380,7 @@ void ipte_unlock(struct kvm *kvm)
 static int ar_translation(struct kvm_vcpu *vcpu, union asce *asce, u8 ar,
 			  enum gacc_mode mode)
 {
+	int acrs[NUM_ACRS];
 	union alet alet;
 	struct ale ale;
 	struct aste aste;
@@ -391,8 +392,8 @@ static int ar_translation(struct kvm_vcpu *vcpu, union asce *asce, u8 ar,
 	if (ar >= NUM_ACRS)
 		return -EINVAL;
 
-	save_access_regs(vcpu->run->s.regs.acrs);
-	alet.val = vcpu->run->s.regs.acrs[ar];
+	save_access_regs(acrs);
+	alet.val = acrs[ar];
 
 	if (ar == 0 || alet.val == 0) {
 		asce->val = vcpu->arch.sie_block->gcr[1];
-- 
2.40.1


             reply	other threads:[~2024-01-31 20:58 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-31 20:58 Eric Farman [this message]
2024-02-01 15:14 ` [RFC PATCH] KVM: s390: remove extra copy of access registers into KVM_RUN Heiko Carstens
2024-02-01 16:56   ` Eric Farman
2024-02-06 15:47     ` Heiko Carstens
2024-02-06 17:07       ` Eric Farman
2024-02-08 11:50 ` Christian Borntraeger
2024-02-08 12:37   ` Janosch Frank
2024-02-08 13:51     ` Christian Borntraeger
2024-02-08 19:15       ` Eric Farman
2024-02-08 12:39 ` Janosch Frank
2024-02-08 19:13   ` Eric Farman

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=20240131205832.2179029-1-farman@linux.ibm.com \
    --to=farman@linux.ibm.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=david@redhat.com \
    --cc=frankja@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    /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