On 7/22/20 10:24 AM, Paul Mackerras wrote:
On Wed, Jul 22, 2020 at 07:39:26AM +0530, Athira Rajeev wrote:
On 21-Jul-2020, at 9:24 AM, Paul Mackerras <paulus@xxxxxxxxxx> wrote:
On Fri, Jul 17, 2020 at 10:38:14AM -0400, Athira Rajeev wrote:
Currently `kvm_vcpu_arch` stores all Monitor Mode Control registers
in a flat array in order: mmcr0, mmcr1, mmcra, mmcr2, mmcrs
Split this to give mmcra and mmcrs its own entries in vcpu and
use a flat array for mmcr0 to mmcr2. This patch implements this
cleanup to make code easier to read.
Changing the way KVM stores these values internally is fine, but
changing the user ABI is not. This part:
diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
index 264e266..e55d847 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -510,8 +510,8 @@ struct kvm_ppc_cpu_char {
#define KVM_REG_PPC_MMCR0 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x10)
#define KVM_REG_PPC_MMCR1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x11)
-#define KVM_REG_PPC_MMCRA (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x12)
-#define KVM_REG_PPC_MMCR2 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x13)
+#define KVM_REG_PPC_MMCR2 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x12)
+#define KVM_REG_PPC_MMCRA (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x13)
means that existing userspace programs that used to work would now be
broken. That is not acceptable (breaking the user ABI is only ever
acceptable with a very compelling reason). So NAK to this part of the
patch.
Hi Paul
Thanks for checking the patch. I understood your point on user ABI breakage that this particular change can cause.
I will retain original KVM_REG_PPC_MMCRA and KVM_REG_PPC_MMCR2 order in `kvm.h`
And with that, additionally I will need below change ( on top of current patch ) for my clean up updates for kvm cpu MMCR to work,
Because now mmcra and mmcrs will have its own entries in vcpu and is not part of the mmcr[] array
Please suggest if this looks good
Yes, that looks fine.
By the way, is the new MMCRS register here at all related to the MMCRS
Hi Paul,
We have only split the current array (mmcr[]) and separated it to mmcra
and mmcrs.
Only new spr that is added is mmcr3 (for Power10).
Maddy
that there used to be on POWER8, but which wasn't present (as far as I
know) on POWER9?
Paul.
