From: marc.zyngier@arm.com (Marc Zyngier)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: KVM: Cleanup exception injection
Date: Thu, 17 Dec 2015 08:59:38 +0000 [thread overview]
Message-ID: <5672797A.3050305@arm.com> (raw)
In-Reply-To: <20151216204018.GF24889@cbox>
On 16/12/15 20:40, Christoffer Dall wrote:
> On Mon, Dec 14, 2015 at 06:06:36PM +0000, Marc Zyngier wrote:
>> David Binderman reported that the exception injection code had a
>> couple of unused variables lingering around.
>>
>> Upon examination, it looked like this code could do with an
>> anticipated spring cleaning, which amounts to deduplicating
>> the CPSR/SPSR update.
>>
>> The spurious variables are removed in the process.
>>
>> Reported-by: David Binderman <dcb314@hotmail.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>> arch/arm/kvm/emulate.c | 59 ++++++++++++++++++++------------------------------
>> 1 file changed, 23 insertions(+), 36 deletions(-)
>>
>> diff --git a/arch/arm/kvm/emulate.c b/arch/arm/kvm/emulate.c
>> index d6c0052..245106e 100644
>> --- a/arch/arm/kvm/emulate.c
>> +++ b/arch/arm/kvm/emulate.c
>> @@ -275,6 +275,25 @@ static u32 exc_vector_base(struct kvm_vcpu *vcpu)
>> return vbar;
>> }
>>
>> +/* Switch to an exception mode, updating both CPSR and SPSR */
>> +static void kvm_update_psr(struct kvm_vcpu *vcpu, unsigned long mode)
>> +{
>> + unsigned long cpsr = *vcpu_cpsr(vcpu);
>> + u32 sctlr = vcpu->arch.cp15[c1_SCTLR];
>> +
>> + *vcpu_cpsr(vcpu) = (cpsr & ~MODE_MASK) | mode;
>> + *vcpu_cpsr(vcpu) |= PSR_I_BIT;
>> + *vcpu_cpsr(vcpu) &= ~(PSR_IT_MASK | PSR_J_BIT | PSR_E_BIT | PSR_T_BIT);
>> +
>> + if (sctlr & SCTLR_TE)
>> + *vcpu_cpsr(vcpu) |= PSR_T_BIT;
>> + if (sctlr & SCTLR_EE)
>> + *vcpu_cpsr(vcpu) |= PSR_E_BIT;
>> +
>> + /* Note: These now point to the mode banked copies */
>> + *vcpu_spsr(vcpu) = cpsr;
>> +}
>> +
>> /**
>> * kvm_inject_undefined - inject an undefined exception into the guest
>> * @vcpu: The VCPU to receive the undefined exception
>> @@ -286,29 +305,13 @@ static u32 exc_vector_base(struct kvm_vcpu *vcpu)
>> */
>> void kvm_inject_undefined(struct kvm_vcpu *vcpu)
>> {
>> - unsigned long new_lr_value;
>> - unsigned long new_spsr_value;
>> unsigned long cpsr = *vcpu_cpsr(vcpu);
>> - u32 sctlr = vcpu->arch.cp15[c1_SCTLR];
>> bool is_thumb = (cpsr & PSR_T_BIT);
>> u32 vect_offset = 4;
>> u32 return_offset = (is_thumb) ? 2 : 4;
>>
>> - new_spsr_value = cpsr;
>> - new_lr_value = *vcpu_pc(vcpu) - return_offset;
>> -
>> - *vcpu_cpsr(vcpu) = (cpsr & ~MODE_MASK) | UND_MODE;
>> - *vcpu_cpsr(vcpu) |= PSR_I_BIT;
>> - *vcpu_cpsr(vcpu) &= ~(PSR_IT_MASK | PSR_J_BIT | PSR_E_BIT | PSR_T_BIT);
>> -
>> - if (sctlr & SCTLR_TE)
>> - *vcpu_cpsr(vcpu) |= PSR_T_BIT;
>> - if (sctlr & SCTLR_EE)
>> - *vcpu_cpsr(vcpu) |= PSR_E_BIT;
>> -
>> - /* Note: These now point to UND banked copies */
>> - *vcpu_spsr(vcpu) = cpsr;
>> - *vcpu_reg(vcpu, 14) = new_lr_value;
>> + kvm_update_psr(vcpu, UND_MODE);
>> + *vcpu_reg(vcpu, 14) = *vcpu_pc(vcpu) - return_offset;
>>
>> /* Branch to exception vector */
>> *vcpu_pc(vcpu) = exc_vector_base(vcpu) + vect_offset;
>> @@ -320,30 +323,14 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
>> */
>> static void inject_abt(struct kvm_vcpu *vcpu, bool is_pabt, unsigned long addr)
>> {
>> - unsigned long new_lr_value;
>> - unsigned long new_spsr_value;
>> unsigned long cpsr = *vcpu_cpsr(vcpu);
>> - u32 sctlr = vcpu->arch.cp15[c1_SCTLR];
>> bool is_thumb = (cpsr & PSR_T_BIT);
>> u32 vect_offset;
>> u32 return_offset = (is_thumb) ? 4 : 0;
>> bool is_lpae;
>>
>> - new_spsr_value = cpsr;
>> - new_lr_value = *vcpu_pc(vcpu) + return_offset;
>> -
>> - *vcpu_cpsr(vcpu) = (cpsr & ~MODE_MASK) | ABT_MODE;
>> - *vcpu_cpsr(vcpu) |= PSR_I_BIT | PSR_A_BIT;
>
> seems like you're losing the PSR_A_BIT in kvm_update_psr() now?
Ah, nice catch. Indeed, CPSR.A gets set on abort entry, but not on
undef. I'll add a check in kvm_update_psr, with a reference to the
AArch32.EnterMode() pseudocode from the ARMv8 ARM.
>
>> - *vcpu_cpsr(vcpu) &= ~(PSR_IT_MASK | PSR_J_BIT | PSR_E_BIT | PSR_T_BIT);
>> -
>> - if (sctlr & SCTLR_TE)
>> - *vcpu_cpsr(vcpu) |= PSR_T_BIT;
>> - if (sctlr & SCTLR_EE)
>> - *vcpu_cpsr(vcpu) |= PSR_E_BIT;
>> -
>> - /* Note: These now point to ABT banked copies */
>> - *vcpu_spsr(vcpu) = cpsr;
>> - *vcpu_reg(vcpu, 14) = new_lr_value;
>> + kvm_update_psr(vcpu, ABT_MODE);
>> + *vcpu_reg(vcpu, 14) = *vcpu_pc(vcpu) + return_offset;
>>
>> if (is_pabt)
>> vect_offset = 12;
>> --
>> 2.1.4
>>
>
> Otherwise looks like a welcome cleanup!
>
> Thanks,
> -Christoffer
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
prev parent reply other threads:[~2015-12-17 8:59 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-14 18:06 [PATCH] ARM: KVM: Cleanup exception injection Marc Zyngier
2015-12-16 20:40 ` Christoffer Dall
2015-12-17 8:59 ` Marc Zyngier [this message]
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=5672797A.3050305@arm.com \
--to=marc.zyngier@arm.com \
--cc=linux-arm-kernel@lists.infradead.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;
as well as URLs for NNTP newsgroup(s).