[Qemu-devel] Re: [PATCH 0/9] S390x KVM support

[Alexander Graf]

On 22.10.2009, at 12:03, Avi Kivity wrote:

> On 10/22/2009 11:55 AM, Alexander Graf wrote:
> > On 22.10.2009, at 11:53, Avi Kivity wrote:
> >
> > > On 10/22/2009 11:11 AM, Alexander Graf wrote:
> > > > > Doesn't this break backward compatibility by changing the  
> > > > > structure?
> > > > >
> > > > > Best to put it after the union (and as a copy, so userspace that  
> > > > > expects the previous location still works).  If you're reading  
> > > > > it from the kernel, also need a way to tell the kernel which  
> > > > > copy to read from.
> > > > >
> > > > > Also advertise with a KVM_CAP.
> > > >
> > > >
> > > > I don't think we need to go through the hassle here. There is  
> > > > effectively no user of that code for now and the ABI is  
> > > > considered unstable.
> > >
> > > At the very least we need a KVM_CAP so qemu knows to fail on older  
> > > kernels.
> >
> > Hm. Oh well :-). It can't hurt to have yet another CAP, right?
> >
> > > > > I'd also appreciate an explanation of what this is all about.
> > > >
> > > > Explanation in the code or explanation in an email reply?
> > >
> > >
> > > email.  I assume s390 hackers would understand why the psw needs  
> > > to be exposed to qemu on every exit.  This is mostly for my  
> > > personal interest.
> >
> > PSW = (eflags << 32) | pc;
> >
> > :-)
> >
> > Before that patch it was only synced with the "vmcb" on special  
> > userspace handled intercepts, now it's synced on every exit to  
> > userspace.
>
> Right, but why?  x86 qemu doesn't care about either pc or eflags  
> (with in-kernel irqchip, which s390 essentially is).

The old code would only sync the psw for us on "intercepts that the  
kernel knows require userspace interaction". We'd "x /i $pc" somewhere  
along the way and would get an old psw, but GET_REGS would give us  
recent register states, not including the psw. That's a total nightmare.

To solve the psw nightmare there were basically 2 roads possible to  
take here:

1) Sync the PSW in the GET_REGS ioctl
2) Sync the PSW in kvm_run on all exits to userspace

Otherwise we can't debug guests.

The same goes for setting the PSW. Setting the PSW was only  
implemented when kvm_run->exit_reason == "the kernel thinks userspace  
needs to do something". So If you wanted to change the pc along the  
way, you couldn't.

Furthermore, there was a nasty bug in this whole optimization that  
lead me to think it's better to just get rid of the whole "kernel  
tries to be clever" implementation and just sync the psw at all times.  
Most exits to userspace are of the "userspace needs to do something"  
type anyways, so we're not really losing anything here. Implementing  
2) was just really easy.

So I guess Carsten thought the same way and improved my original patch.


I hope that clarifies things.

Alex