On Thu, Jan 11, 2018 at 06:11:13PM +0800, wei.guo.simon@xxxxxxxxx wrote: > From: Simon Guo <wei.guo.simon@xxxxxxxxx> > > In current days, many OS distributions have utilized transaction > memory functionality. In PowerPC, HV KVM supports TM. But PR KVM > does not. > > The drive for the transaction memory support of PR KVM is the > openstack Continuous Integration testing - They runs a HV(hypervisor) > KVM(as level 1) and then run PR KVM(as level 2) on top of that. > > This patch set add transaction memory support on PR KVM. Thanks for the patch set. It mostly looks good, though I have some comments on the individual patches. I don't see where you are implementing support for userspace accessing the TM checkpointed register values using the GET_ONE_REG/SET_ONE_REG API. This would mean that you couldn't migrate a guest that was in the middle of a transaction. We will need to have the one_reg API access to the TM checkpoint implemented, though there will be a difficulty in that kvmppc_get_one_reg() and kvmppc_set_one_reg() are called with the vcpu context loaded. With your scheme of having the TM checkpoint stored in the CPU while the vcpu context is loaded, the values you want to access in kvmppc_get/set_one_reg are inaccessible since they're stored in the CPU. You would have to arrange for kvmppc_get/set_one_reg to be called without the vcpu context loaded (recent patches in the kvm next branch probably make that easier) or else explicitly unload and reload the vcpu context in those functions. (This is easier in HV KVM since the checkpoint is not in the CPU at the point of doing kvmppc_get/set_one_reg.) There is also complexity added because it's possible for the guest to have TM, FP, VEC and VSX all enabled from its point of view but to have FP/VEC/VSX not actually enabled in the hardware when the guest is running. As you note in your patch descriptions, this means that the guest can do tbegin and create a checkpoint with bogus values for the FP/VEC/VSX registers. Rather than trying to detect and fix up this situation after the fact, I would suggest that if the guest has TM enabled then we make sure that the real FP/VEC/VSX bits in the MSR match what the guest thinks it has. That way we would avoid the bogus checkpoint problem. (There is still the possibility of getting bogus checkpointed FP/VEC/VSX registers if the guest does tbegin with the FP/VEC/VSX bits clear in the MSR, but that is the guest's problem to deal with.) Paul.
