On 21-Jul-2020, at 9:24 AM, Paul Mackerras <paulus@ozlabs.org> = 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

diff --git = a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c

index = 3f90eee261fc..b10bb404f0d5 100644

--- a/arch/powerpc/kvm/book3s_hv.c

+++ = b/arch/powerpc/kvm/book3s_hv.c

@@ -1679,10 +1679,13 @@ static int = kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,

= case KVM_REG_PPC_UAMOR:

= *val =3D get_reg_val(id, vcpu->arch.uamor);

= break;

- = case KVM_REG_PPC_MMCR0 ... KVM_REG_PPC_MMCR2:

+ = case KVM_REG_PPC_MMCR0 ... KVM_REG_PPC_MMCR1:

= i =3D id - = KVM_REG_PPC_MMCR0;

*val = =3D get_reg_val(id, vcpu->arch.mmcr[i]);

= break;

+ case = KVM_REG_PPC_MMCR2:

+ *val =3D = get_reg_val(id, vcpu->arch.mmcr[2]);

+ = break;

case KVM_REG_PPC_MMCRA:

= *val =3D get_reg_val(id, = vcpu->arch.mmcra);

= break;

@@ -1906,10 +1909,13 @@ static int = kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,

= case KVM_REG_PPC_UAMOR:

= vcpu->arch.uamor =3D set_reg_val(id, *val);

= break;

- = case KVM_REG_PPC_MMCR0 ... KVM_REG_PPC_MMCR2:

+ = case KVM_REG_PPC_MMCR0 ... KVM_REG_PPC_MMCR1:

= i =3D id - = KVM_REG_PPC_MMCR0;

= vcpu->arch.mmcr[i] =3D set_reg_val(id, *val);

= break;

+ = case KVM_REG_PPC_MMCR2:

+ = vcpu->arch.mmcr[2] =3D set_reg_val(id, *val);

+ = break;

case = KVM_REG_PPC_MMCRA:

= vcpu->arch.mmcra =3D set_reg_val(id, *val);

= break;

=E2=80=94