Hi Paul, On Tue, Jan 23, 2018 at 04:38:32PM +1100, Paul Mackerras wrote: > 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.) Thanks for point it out. I didn't think about it before and will investigate. I plan to work out this PR KVM HTM kvmppc_get/set_one_reg() (and the KVM_SET_REGS you mentioned in another mail) with seperate patch/patch set, so that the reworked V2 of current patches can be sent out in parallel. In case it is not appropriate for you, please let me know. > > 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.) Good idea. I will look into kvmppc_set_msr_pr() / kvmppc_giveup_ext() to simplify the solution. Thanks for your review and time. BR, - Simon
