From: Janosch Frank <frankja@linux.ibm.com>
To: Janis Schoetterl-Glausch <scgl@linux.ibm.com>,
Christian Borntraeger <borntraeger@linux.ibm.com>,
Claudio Imbrenda <imbrenda@linux.ibm.com>
Cc: David Hildenbrand <david@redhat.com>,
Heiko Carstens <hca@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Alexander Gordeev <agordeev@linux.ibm.com>,
Sven Schnelle <svens@linux.ibm.com>,
Paolo Bonzini <pbonzini@redhat.com>,
kvm@vger.kernel.org, Shuah Khan <shuah@kernel.org>,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-s390@vger.kernel.org
Subject: Re: [PATCH v2 1/2] KVM: s390: Don't indicate suppression on dirtying, failing memop
Date: Tue, 26 Apr 2022 15:39:14 +0200 [thread overview]
Message-ID: <6aa94072-8bbe-38ae-e9ef-d666f86de217@linux.ibm.com> (raw)
In-Reply-To: <1c2caf30-da84-b4ce-d2ac-4edb5ef60a79@linux.ibm.com>
On 4/26/22 15:25, Janis Schoetterl-Glausch wrote:
> On 4/26/22 09:18, Janosch Frank wrote:
>> On 4/25/22 12:01, Janis Schoetterl-Glausch wrote:
>>> If user space uses a memop to emulate an instruction and that
>>> memop fails, the execution of the instruction ends.
>>> Instruction execution can end in different ways, one of which is
>>> suppression, which requires that the instruction execute like a no-op.
>>
>>
>>
>>> A writing memop that spans multiple pages and fails due to key
>>> protection can modified guest memory, as a result, the likely
>>> correct ending is termination. Therefore do not indicate a
>>> suppressing instruction ending in this case.
>>
>> Check grammar.
>>
>>>
>>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>>> ---
>>> arch/s390/kvm/gaccess.c | 47 ++++++++++++++++++++++++-----------------
>>> 1 file changed, 28 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
>>> index d53a183c2005..3b1fbef82288 100644
>>> --- a/arch/s390/kvm/gaccess.c
>>> +++ b/arch/s390/kvm/gaccess.c
>>> @@ -491,8 +491,8 @@ enum prot_type {
>>> PROT_TYPE_IEP = 4,
>>> };
>>> -static int trans_exc(struct kvm_vcpu *vcpu, int code, unsigned long gva,
>>> - u8 ar, enum gacc_mode mode, enum prot_type prot)
>>> +static int trans_exc_ending(struct kvm_vcpu *vcpu, int code, unsigned long gva, u8 ar,
>>> + enum gacc_mode mode, enum prot_type prot, bool suppress)
>>> {
>>> struct kvm_s390_pgm_info *pgm = &vcpu->arch.pgm;
>>> struct trans_exc_code_bits *tec;
>>> @@ -503,22 +503,24 @@ static int trans_exc(struct kvm_vcpu *vcpu, int code, unsigned long gva,
>>> switch (code) {
>>> case PGM_PROTECTION:
>>> - switch (prot) {
>>> - case PROT_TYPE_IEP:
>>> - tec->b61 = 1;
>>> - fallthrough;
>>> - case PROT_TYPE_LA:
>>> - tec->b56 = 1;
>>> - break;
>>> - case PROT_TYPE_KEYC:
>>> - tec->b60 = 1;
>>> - break;
>>> - case PROT_TYPE_ALC:
>>> - tec->b60 = 1;
>>> - fallthrough;
>>> - case PROT_TYPE_DAT:
>>> - tec->b61 = 1;
>>> - break;
>>> + if (suppress) {
>>> + switch (prot) {
>>> + case PROT_TYPE_IEP:
>>> + tec->b61 = 1;
>>> + fallthrough;
>>> + case PROT_TYPE_LA:
>>> + tec->b56 = 1;
>>> + break;
>>> + case PROT_TYPE_KEYC:
>>> + tec->b60 = 1;
>>> + break;
>>> + case PROT_TYPE_ALC:
>>> + tec->b60 = 1;
>>> + fallthrough;
>>> + case PROT_TYPE_DAT:
>>> + tec->b61 = 1;
>>> + break;
>>> + }
>>> }
>>
>> How about switching this around and masking those bits on termination.
>
> I did initially have if (!terminate) { ... }, but it seemed more straight forward
> to me without the negation. Or are you suggesting explicitly resetting the
> bits to zero when terminating?
Yes
>>
>>> fallthrough;
>>> case PGM_ASCE_TYPE:
>>> @@ -552,6 +554,12 @@ static int trans_exc(struct kvm_vcpu *vcpu, int code, unsigned long gva,
>>> return code;
>>> }
>>> +static int trans_exc(struct kvm_vcpu *vcpu, int code, unsigned long gva, u8 ar,
>>> + enum gacc_mode mode, enum prot_type prot)
>>> +{
>>> + return trans_exc_ending(vcpu, code, gva, ar, mode, prot, true);
>>> +}
>>> +
>>> static int get_vcpu_asce(struct kvm_vcpu *vcpu, union asce *asce,
>>> unsigned long ga, u8 ar, enum gacc_mode mode)
>>> {
>>> @@ -1110,7 +1118,8 @@ int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
>>> ga = kvm_s390_logical_to_effective(vcpu, ga + fragment_len);
>>> }
>>> if (rc > 0)
>>> - rc = trans_exc(vcpu, rc, ga, ar, mode, prot);
>>> + rc = trans_exc_ending(vcpu, rc, ga, ar, mode, prot,
>>> + (mode != GACC_STORE) || (idx == 0));
>>
>> Add a boolean variable named terminating, calculate the value before passing the boolean on.
>
> Ok. I'll scope it to the body of the if.
>>
>>> out_unlock:
>>> if (need_ipte_lock)
>>> ipte_unlock(vcpu);
>>
>>
>
next prev parent reply other threads:[~2022-04-26 13:39 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-25 10:01 [PATCH v2 0/2] Dirtying, failing memop: don't indicate suppression Janis Schoetterl-Glausch
2022-04-25 10:01 ` [PATCH v2 1/2] KVM: s390: Don't indicate suppression on dirtying, failing memop Janis Schoetterl-Glausch
2022-04-25 13:46 ` Claudio Imbrenda
2022-04-25 16:01 ` Christian Borntraeger
2022-04-26 7:18 ` Janosch Frank
2022-04-26 13:25 ` Janis Schoetterl-Glausch
2022-04-26 13:39 ` Janosch Frank [this message]
2022-04-25 10:01 ` [PATCH v2 2/2] KVM: s390: selftest: Test suppression indication on key prot exception Janis Schoetterl-Glausch
2022-04-25 13:47 ` Claudio Imbrenda
2022-04-28 16:48 ` Janis Schoetterl-Glausch
2022-04-25 16:30 ` [PATCH v2 0/2] Dirtying, failing memop: don't indicate suppression Christian Borntraeger
2022-04-25 17:29 ` Janis Schoetterl-Glausch
2022-04-26 6:19 ` Christian Borntraeger
2022-04-26 7:25 ` Janosch Frank
2022-04-26 11:56 ` Janis Schoetterl-Glausch
2022-04-26 12:34 ` Janosch Frank
2022-05-02 7:58 ` Christian Borntraeger
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=6aa94072-8bbe-38ae-e9ef-d666f86de217@linux.ibm.com \
--to=frankja@linux.ibm.com \
--cc=agordeev@linux.ibm.com \
--cc=borntraeger@linux.ibm.com \
--cc=david@redhat.com \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=imbrenda@linux.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=scgl@linux.ibm.com \
--cc=shuah@kernel.org \
--cc=svens@linux.ibm.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;
as well as URLs for NNTP newsgroup(s).