[PATCH 1/2] arm: KVM: Do not update PC if the trap handler has updated it

[Shannon Zhao]

On 2016/1/7 16:50, Marc Zyngier wrote:
> On 22/12/15 14:50, Peter Maydell wrote:
>> On 22 December 2015 at 14:39, Christoffer Dall
>> <christoffer.dall at linaro.org> wrote:
>>> On Tue, Dec 22, 2015 at 11:08:10AM +0000, Peter Maydell wrote:
>>>> Won't this result in our incorrectly skipping the first insn
>>>> in the fault handler if the original offending instruction
>>>> was itself the first insn in the fault handler?
>>>>
>>> Wouldn't that then loop with the exception forever?
>>
>> Yes, but so would real hardware...
> 
> Indeed. As it is, this patch is not doing what it should. On the other
> hand, I came to the conclusion that we do not need to fix this just yet,
> as long as we only let KVM inject an UNDEF, and that's what the PMU code
> requires.
> 
> I'll comment on the PMU thread, but the gist of it is:
> 1) fix the arm64 UNDEF/PABRT/DABRT code to properly account for the the
> source EL (Table D1-7 of the ARMv8 ARM).
This looks like something we add in the PMU patch set.

+		switch (cpsr & (PSR_MODE_MASK | PSR_MODE32_BIT)) {
+		case PSR_MODE_EL0t:
+			exc_offset = EL0_EXCEPT_SYNC_OFFSET_64;
+			break;
+		case PSR_MODE_EL1t:
+			exc_offset = EL1_EXCEPT_BAD_SYNC_OFFSET;
+			break;
+		case PSR_MODE_EL1h:
+			exc_offset = EL1_EXCEPT_SYNC_OFFSET;
+			break;
+		default:
+			exc_offset = EL0_EXCEPT_SYNC_OFFSET_32;
+		}
+

> 2) instead of crafting an exception that modifies the PC, fail the
> sysreg access and let KVM inject an UNDEF.
> 
> I'll post another patch today to address 1), and I'll finish reviewing
> the PMU thread (I have a separate patch addressing 2)).
> 
Thanks!

-- 
Shannon