On 01/24/2013 07:01 PM, Marcelo Tosatti wrote:
On Thu, Jan 24, 2013 at 06:44:50PM -0200, Marcelo Tosatti wrote:
What 'subtle errors' are you thinking of?
It should be easy to convert as its greppable.
S/390 not synchronizing the env-> copy of the FULL register set is still
a bug, though (because the FULL set is what "cpu_synchronize_state" with
no parameter implies).
There seems to be a lot of good discussion going on regarding these
changes. Have we reached some kind of consensus on how we want to proceed?
If I understand the arguments being made... this patch set, as
submitted, is wrong because KVM_ARCH_GET_REGS should always retrieve
runtime regs only? Or must it sync everything? If there is not a clear
answer here we'll need to decide this before going anywhere with this
patch, yes?
Alex wrote:
"""
I think for now the best choice for get_regs() would be to ignore the
FULL/RESET bits and always keep the syncing as it happens today under
the RUNTIME umbrella only. So all of get_regs() only checks for RUNTIME.
Whenever get_xxx() happens, a bit gets set for set_xxx(). Up to this
point, only the RUNTIME bit is ever set, because that's what
cpu_synchronize_registers() sets.
Then s390 can add special separate bits for "sync GPRs" and "sync CRs".
SYNC_RUNTIME would include those bits. The kvm hypercall exit calls a
new synchronize_registers() function with a parameter telling it to only
sync GPRs. This marks GPRs dirty, but not RUNTIME. The set_registers()
function in s390 specific code could handle this particular case specially.
That way everything's solved and scalable, no?
"""
This is the most comprehensive suggestion I've been able to pick out of
the discussion. However, Marcelo has expressed concern over S390 not
updating everything in env:
"""
S/390 not synchronizing the env-> copy of the FULL register set is still
a bug, though (because the FULL set is what "cpu_synchronize_state" with
no parameter implies).
"""
Also Alex, what did you mean by get_xxx() and set_xxx()?
--
-- Jason J. Herne (jjhe...@linux.vnet.ibm.com)
