Mark Rutland <mark.rutland@xxxxxxx> writes: > On Thu, Nov 08, 2018 at 02:38:43PM +0000, Peter Maydell wrote: >> On 8 November 2018 at 14:28, Alex Bennée <alex.bennee@xxxxxxxxxx> wrote: >> > >> > Mark Rutland <mark.rutland@xxxxxxx> writes: >> >> One problem is that I couldn't spot when we advance the PC for an MMIO >> >> trap. I presume we do that in the kernel, *after* the MMIO trap, but I >> >> can't see where that happens. >> > >> > Nope it gets done before during decode_hsr in mmio.c: >> > >> > /* >> > * The MMIO instruction is emulated and should not be re-executed >> > * in the guest. >> > */ >> > kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu)); >> >> I think that this attempt to do the PC-advance early is >> probably an underlying problem that is not helping the >> code structure here. >> >> An enhancement that's been floated previously is that the >> MMIO emulation in userspace should be able to report back >> to KVM "nope, that access should generate a guest synchronous >> external abort (with ESR_EL1.EA = 0/1)". >> If we have that, then we definitely need to not advance the >> PC until after userspace has done the emulation and told >> us whether the memory access succeeded or not... > > Yup. > > I think that we absolutely want to do all the CPU state advancement (PC, > SS bit, etc) at the point we apply the effects of the instruction. Not > before we emulate the instruction, and not higher/lower in the call > stack. There is certainly an argument to abstract some of the advancement logic so we can make debug related decisions in one place. I don't know how much churn we would need to get there. Currently most of the guest debug decisions are made in one place as we head into the guest run. Generally I don't think the emulation code want to know or care about the SS bit or what debug is currently happening although I guess the presence of the SS bit could be used to decide on exactly what exit type you are going to do - back to guest or out to user space. Currently kvm_arm_handle_step_debug on cares about host directed debug but we could expand it to raise the appropriate guest exception if required. > We have a big problem in that guest-directed singlestep and > host-directed singlestep don't compose, and given that host-directed > singlestep doesn't work reliably today I'd be tempted to rip that out > until we've fixed guest-directed singlestep. Getting host and guest debug to run at the same time is tricky given we end up subverting guest state when the host debug is in control. It did make my head spin when I worked on it originally which led to the acceptance that guest debug couldn't be expected to work transparently while host directed debug was in effect. If we can support it without adding complexity then that would be great but it's a pretty niche use case. I'd be loathed to rip out the current single step support as it does actually work pretty well - it's just corner cases with emulated MMIO and first step that are failing. Last I looked these were cases x86 didn't even get right and no one has called to remove it's single step support AFAIK. > We should have a story for how host-directed debug is handled > transparently from the PoV of a guest using guest-directed debug. What's the use case for this apart from having a cleaner abstraction? Do users really spend time running multiple gdbs at different levels in the stack? > > Thanks, > Mark. -- Alex Bennée
