qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH RESEND v15 08/10] target-arm: kvm64: inject sync


From: gengdongjiu
Subject: Re: [Qemu-devel] [PATCH RESEND v15 08/10] target-arm: kvm64: inject synchronous External Abort
Date: Mon, 26 Nov 2018 17:25:09 +0000

> >>
> >> Hi Peter,
> >>   Thanks for the review and comments.
> >>
> >> >
> >> > On 8 November 2018 at 10:29, Dongjiu Geng <address@hidden> wrote:
> >> > > +bool write_part_cpustate_to_list(ARMCPU *cpu, ptrdiff_t
> >> > > +fieldoffset)
> >> >
> >> > What is this about? Nothing else in QEMU needs to mess with the
> >> > cpustate synchronization. My first assumption is that you should not 
> >> > need to do so either.
> >>
> >> We should change the guest CP15 ESR_EL1's value, the only method is
> >> to change the cpu->cpreg_values[] in QEMU, then QEMU call 
> >> write_list_to_kvmstate() to set the cpu->cpreg_values[] to KVM which
> include the specified ESR_EL1 value, KVM do world switch, and then set the 
> specified ESR_EL1's value to guest kernel.
> >
> > Ah, I see. This is a bug in our current handling of the register
> > state, where we implicitly assume that nothing in QEMU will ever want
> > to change any system register values. This assumption is now false --
> > kvm_arm_handle_debug() broke it -- so we need to fix the code that
> > does kvm_arch_put_registers(). There is a comment in the kvm32.c
> > version of that function about this. (The kvm64.c version has the same
> > assumption but doesn't comment on it.)
> >
> > We should (ideally) fix this bug in the code that does register
> > syncing, without requiring places in QEMU that update system registers
> > to have to manually indicate which registers they have changed. I'll
> > have a think about how best to do this.
> >
> >> About the detailed explanation, as shown in [2].
> >>
> >> kvm_arm_handle_debug() does not need to do this because QEMU does not need 
> >> to change CP15 registers, such as ESR_EL1.
> >
> > kvm_arm_handle_debug does change ESR_EL1: it is injecting an exception
> > and so should set the exception register. This happens when it calls
> > the do_interrupt() hook, because arm_cpu_do_interrupt_aarch64() writes
> > to env->cp15.esr_el[new_el].
> >
> > I'm not entirely sure why this is working today, in fact.
> > Alex, did you test whether our debug-exception-injection reports the
> > correct ESR_EL1 to the guest ?
> <snip>
> 
> I did not - I was mostly focusing in the host-debugging-the-guest test case. 
> I'll get a test rig up and check.

Thanks, please test it in the KVM mode, not in the TCG mode. Waiting for your 
test result.

> 
> --
> Alex Bennée

reply via email to

[Prev in Thread] Current Thread [Next in Thread]