| Message ID | 1594996707-3727-3-git-send-email-atrajeev@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7e4a145e5b675d5a9182f756950f001eaa256795 |
| Headers | show |
| Series | powerpc/perf: Add support for power10 PMU Hardware | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (c27fe454aff795023d2f3f90f41eb1a3104e614f) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 115 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
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. Regards, Paul.
> 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 Thanks Athira — 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 = 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 = id - KVM_REG_PPC_MMCR0; *val = get_reg_val(id, vcpu->arch.mmcr[i]); break; + case KVM_REG_PPC_MMCR2: + *val = get_reg_val(id, vcpu->arch.mmcr[2]); + break; case KVM_REG_PPC_MMCRA: *val = 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 = 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 = id - KVM_REG_PPC_MMCR0; vcpu->arch.mmcr[i] = set_reg_val(id, *val); break; + case KVM_REG_PPC_MMCR2: + vcpu->arch.mmcr[2] = set_reg_val(id, *val); + break; case KVM_REG_PPC_MMCRA: vcpu->arch.mmcra = set_reg_val(id, *val); break; — > > Regards, > Paul.
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/daea78154eff1b7e2f36be05a8f95feb5a588912 Can you please check that matches what you sent. 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: > *val = 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 = id - KVM_REG_PPC_MMCR0; > *val = get_reg_val(id, vcpu->arch.mmcr[i]); > break; > + case KVM_REG_PPC_MMCR2: > + *val = get_reg_val(id, vcpu->arch.mmcr[2]); > + break; > case KVM_REG_PPC_MMCRA: > *val = 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 = 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 = id - KVM_REG_PPC_MMCR0; > vcpu->arch.mmcr[i] = set_reg_val(id, *val); > break; > + case KVM_REG_PPC_MMCR2: > + vcpu->arch.mmcr[2] = set_reg_val(id, *val); > + break; > case KVM_REG_PPC_MMCRA: > vcpu->arch.mmcra = set_reg_val(id, *val); > break; > — > > >> >> Regards, >> Paul.
Paul Mackerras <paulus@ozlabs.org> writes: > 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. I assume we don't have a KVM unit test that would have caught this? cheers
On Wed, Jul 22, 2020 at 07:39:26AM +0530, Athira Rajeev wrote: > > > > 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 Yes, that looks fine. By the way, is the new MMCRS register here at all related to the MMCRS that there used to be on POWER8, but which wasn't present (as far as I know) on POWER9? Paul.
> On 22-Jul-2020, at 10:07 AM, Michael Ellerman <mpe@ellerman.id.au> wrote: > > Athira Rajeev <atrajeev@linux.vnet.ibm.com <mailto: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/daea78154eff1b7e2f36be05a8f95feb5a588912 <https://github.com/linuxppc/linux/commit/daea78154eff1b7e2f36be05a8f95feb5a588912> > > > 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: >> *val = 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 = id - KVM_REG_PPC_MMCR0; >> *val = get_reg_val(id, vcpu->arch.mmcr[i]); >> break; >> + case KVM_REG_PPC_MMCR2: >> + *val = get_reg_val(id, vcpu->arch.mmcr[2]); >> + break; >> case KVM_REG_PPC_MMCRA: >> *val = 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 = 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 = id - KVM_REG_PPC_MMCR0; >> vcpu->arch.mmcr[i] = set_reg_val(id, *val); >> break; >> + case KVM_REG_PPC_MMCR2: >> + vcpu->arch.mmcr[2] = set_reg_val(id, *val); >> + break; >> case KVM_REG_PPC_MMCRA: >> vcpu->arch.mmcra = set_reg_val(id, *val); >> break; >> — >> >> >>> >>> Regards, >>> Paul.
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@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 > 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.
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 7e2d061..fc115e2 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -637,7 +637,9 @@ struct kvm_vcpu_arch { u32 ccr1; u32 dbsr; - u64 mmcr[5]; + u64 mmcr[3]; + u64 mmcra; + u64 mmcrs; u32 pmc[8]; u32 spmc[2]; u64 siar; 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) #define KVM_REG_PPC_MMCRS (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x14) #define KVM_REG_PPC_SIAR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x15) #define KVM_REG_PPC_SDAR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x16) diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c index 6657dc6..6fa4853 100644 --- a/arch/powerpc/kernel/asm-offsets.c +++ b/arch/powerpc/kernel/asm-offsets.c @@ -559,6 +559,8 @@ int main(void) OFFSET(VCPU_IRQ_PENDING, kvm_vcpu, arch.irq_pending); OFFSET(VCPU_DBELL_REQ, kvm_vcpu, arch.doorbell_request); OFFSET(VCPU_MMCR, kvm_vcpu, arch.mmcr); + OFFSET(VCPU_MMCRA, kvm_vcpu, arch.mmcra); + OFFSET(VCPU_MMCRS, kvm_vcpu, arch.mmcrs); OFFSET(VCPU_PMC, kvm_vcpu, arch.pmc); OFFSET(VCPU_SPMC, kvm_vcpu, arch.spmc); OFFSET(VCPU_SIAR, kvm_vcpu, arch.siar); diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index 6bf66649..3f90eee 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -1679,10 +1679,16 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id, case KVM_REG_PPC_UAMOR: *val = get_reg_val(id, vcpu->arch.uamor); break; - case KVM_REG_PPC_MMCR0 ... KVM_REG_PPC_MMCRS: + case KVM_REG_PPC_MMCR0 ... KVM_REG_PPC_MMCR2: i = id - KVM_REG_PPC_MMCR0; *val = get_reg_val(id, vcpu->arch.mmcr[i]); break; + case KVM_REG_PPC_MMCRA: + *val = get_reg_val(id, vcpu->arch.mmcra); + break; + case KVM_REG_PPC_MMCRS: + *val = get_reg_val(id, vcpu->arch.mmcrs); + break; case KVM_REG_PPC_PMC1 ... KVM_REG_PPC_PMC8: i = id - KVM_REG_PPC_PMC1; *val = get_reg_val(id, vcpu->arch.pmc[i]); @@ -1900,10 +1906,16 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu, u64 id, case KVM_REG_PPC_UAMOR: vcpu->arch.uamor = set_reg_val(id, *val); break; - case KVM_REG_PPC_MMCR0 ... KVM_REG_PPC_MMCRS: + case KVM_REG_PPC_MMCR0 ... KVM_REG_PPC_MMCR2: i = id - KVM_REG_PPC_MMCR0; vcpu->arch.mmcr[i] = set_reg_val(id, *val); break; + case KVM_REG_PPC_MMCRA: + vcpu->arch.mmcra = set_reg_val(id, *val); + break; + case KVM_REG_PPC_MMCRS: + vcpu->arch.mmcrs = set_reg_val(id, *val); + break; case KVM_REG_PPC_PMC1 ... KVM_REG_PPC_PMC8: i = id - KVM_REG_PPC_PMC1; vcpu->arch.pmc[i] = set_reg_val(id, *val); diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S index 7194389..702eaa2 100644 --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S @@ -3428,7 +3428,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_PMAO_BUG) mtspr SPRN_PMC6, r9 ld r3, VCPU_MMCR(r4) ld r5, VCPU_MMCR + 8(r4) - ld r6, VCPU_MMCR + 16(r4) + ld r6, VCPU_MMCRA(r4) ld r7, VCPU_SIAR(r4) ld r8, VCPU_SDAR(r4) mtspr SPRN_MMCR1, r5 @@ -3436,14 +3436,14 @@ END_FTR_SECTION_IFSET(CPU_FTR_PMAO_BUG) mtspr SPRN_SIAR, r7 mtspr SPRN_SDAR, r8 BEGIN_FTR_SECTION - ld r5, VCPU_MMCR + 24(r4) + ld r5, VCPU_MMCR + 16(r4) ld r6, VCPU_SIER(r4) mtspr SPRN_MMCR2, r5 mtspr SPRN_SIER, r6 BEGIN_FTR_SECTION_NESTED(96) lwz r7, VCPU_PMC + 24(r4) lwz r8, VCPU_PMC + 28(r4) - ld r9, VCPU_MMCR + 32(r4) + ld r9, VCPU_MMCRS(r4) mtspr SPRN_SPMC1, r7 mtspr SPRN_SPMC2, r8 mtspr SPRN_MMCRS, r9 @@ -3551,9 +3551,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) mfspr r8, SPRN_SDAR std r4, VCPU_MMCR(r9) std r5, VCPU_MMCR + 8(r9) - std r6, VCPU_MMCR + 16(r9) + std r6, VCPU_MMCRA(r9) BEGIN_FTR_SECTION - std r10, VCPU_MMCR + 24(r9) + std r10, VCPU_MMCR + 16(r9) END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) std r7, VCPU_SIAR(r9) std r8, VCPU_SDAR(r9) @@ -3578,7 +3578,7 @@ BEGIN_FTR_SECTION_NESTED(96) mfspr r8, SPRN_MMCRS stw r6, VCPU_PMC + 24(r9) stw r7, VCPU_PMC + 28(r9) - std r8, VCPU_MMCR + 32(r9) + std r8, VCPU_MMCRS(r9) lis r4, 0x8000 mtspr SPRN_MMCRS, r4 END_FTR_SECTION_NESTED(CPU_FTR_ARCH_300, 0, 96) diff --git a/tools/arch/powerpc/include/uapi/asm/kvm.h b/tools/arch/powerpc/include/uapi/asm/kvm.h index 264e266..e55d847 100644 --- a/tools/arch/powerpc/include/uapi/asm/kvm.h +++ b/tools/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) #define KVM_REG_PPC_MMCRS (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x14) #define KVM_REG_PPC_SIAR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x15) #define KVM_REG_PPC_SDAR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x16)
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. Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com> --- arch/powerpc/include/asm/kvm_host.h | 4 +++- arch/powerpc/include/uapi/asm/kvm.h | 4 ++-- arch/powerpc/kernel/asm-offsets.c | 2 ++ arch/powerpc/kvm/book3s_hv.c | 16 ++++++++++++++-- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 12 ++++++------ tools/arch/powerpc/include/uapi/asm/kvm.h | 4 ++-- 6 files changed, 29 insertions(+), 13 deletions(-)