>=20 >=20 > Can you please check that matches what you sent. Hi Michael, Yes, it matches. Thanks for making this change. >=20 > cheers >=20 >> 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 >>=20 >>=20 >>>=20 >>> Regards, >>> Paul. --Apple-Mail=_373E7DF6-25B7-4763-98BF-D1995093FADF Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8

On 22-Jul-2020, at 10:07 AM, Michael Ellerman <mpe@ellerman.id.au> = wrote:

Athira Rajeev <atrajeev@linux.vnet.ibm.com> writes:
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

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

I did the same patch I think in my testing branch, it's = here:

https://github.com/linuxppc/linux/commit/daea78154eff1b7e2f36be= 05a8f95feb5a588912

Can you please check that matches what you sent.

Hi = Michael,

Yes, it matches. Thanks for = making this change.

cheers

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:
          &nb= sp;    *val =3D get_reg_val(id, = vcpu->arch.uamor);
          &nb= sp;    break;
- =       case KVM_REG_PPC_MMCR0 ... = KVM_REG_PPC_MMCR2:
+ =       case KVM_REG_PPC_MMCR0 ... = KVM_REG_PPC_MMCR1:
          &nb= sp;    i =3D id - KVM_REG_PPC_MMCR0;
          &nb= sp;    *val =3D get_reg_val(id, = vcpu->arch.mmcr[i]);
          &nb= sp;    break;
+ =       case KVM_REG_PPC_MMCR2:
+ =             &n= bsp; *val =3D get_reg_val(id, vcpu->arch.mmcr[2]);
+= =             &n= bsp; break;
       case = KVM_REG_PPC_MMCRA:
          &nb= sp;    *val =3D get_reg_val(id, = vcpu->arch.mmcra);
          &nb= sp;    break;
@@ -1906,10 +1909,13 @@ = static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
       case = KVM_REG_PPC_UAMOR:
          &nb= sp;    vcpu->arch.uamor =3D set_reg_val(id, = *val);
          &nb= sp;    break;
- =       case KVM_REG_PPC_MMCR0 ... = KVM_REG_PPC_MMCR2:
+ =       case KVM_REG_PPC_MMCR0 ... = KVM_REG_PPC_MMCR1:
          &nb= sp;    i =3D id - KVM_REG_PPC_MMCR0;
          &nb= sp;    vcpu->arch.mmcr[i] =3D set_reg_val(id, = *val);
          &nb= sp;    break;
+ =       case KVM_REG_PPC_MMCR2:
+ =             &n= bsp; vcpu->arch.mmcr[2] =3D set_reg_val(id, *val);
+= =             &n= bsp; break;
       case = KVM_REG_PPC_MMCRA:
          &nb= sp;    vcpu->arch.mmcra =3D set_reg_val(id, = *val);
          &nb= sp;    break;
=E2=80=94

Regards,
Paul.

= --Apple-Mail=_373E7DF6-25B7-4763-98BF-D1995093FADF--