| Message ID | 1398788262-3307-2-git-send-email-agraf@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 29, 2014 at 06:17:37PM +0200, Alexander Graf wrote: > When we expose a POWER8 CPU into the guest, it will start accessing PMU SPRs > that we don't emulate. Just ignore accesses to them. > > Signed-off-by: Alexander Graf <agraf@suse.de> This patch is OK as it stands, but in fact the architecture says that kernel accesses to unimplemented SPRs are mostly supposed to be no-ops rather than causing a trap (mostly == excluding mtspr to 0 or mfspr from 0, 4, 5 or 6). I have a patch to implement that, which I'll post. Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/01/2014 12:12 AM, Paul Mackerras wrote: > On Tue, Apr 29, 2014 at 06:17:37PM +0200, Alexander Graf wrote: >> When we expose a POWER8 CPU into the guest, it will start accessing PMU SPRs >> that we don't emulate. Just ignore accesses to them. >> >> Signed-off-by: Alexander Graf <agraf@suse.de> > This patch is OK as it stands, but in fact the architecture says that > kernel accesses to unimplemented SPRs are mostly supposed to be no-ops > rather than causing a trap (mostly == excluding mtspr to 0 or mfspr > from 0, 4, 5 or 6). I have a patch to implement that, which I'll > post. I think what we want is a flag similar to x86 where we can force ignore unknown SPRs, but leave it at triggering an interrupt as default. We usually have to be at least aware of unknown SPRs and check that not implementing them is ok for the guest. Debugging a program interrupt because of an unknown SPR is usually a lot easier than debugging a breaking guest because it was using the SPR as storage and we didn't back it by anything. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 02, 2014 at 10:35:09AM +0200, Alexander Graf wrote: > On 05/01/2014 12:12 AM, Paul Mackerras wrote: > >On Tue, Apr 29, 2014 at 06:17:37PM +0200, Alexander Graf wrote: > >>When we expose a POWER8 CPU into the guest, it will start accessing PMU SPRs > >>that we don't emulate. Just ignore accesses to them. > >> > >>Signed-off-by: Alexander Graf <agraf@suse.de> > >This patch is OK as it stands, but in fact the architecture says that > >kernel accesses to unimplemented SPRs are mostly supposed to be no-ops > >rather than causing a trap (mostly == excluding mtspr to 0 or mfspr > >from 0, 4, 5 or 6). I have a patch to implement that, which I'll > >post. > > I think what we want is a flag similar to x86 where we can force > ignore unknown SPRs, but leave it at triggering an interrupt as > default. We usually have to be at least aware of unknown SPRs and > check that not implementing them is ok for the guest. > > Debugging a program interrupt because of an unknown SPR is usually a > lot easier than debugging a breaking guest because it was using the > SPR as storage and we didn't back it by anything. That has not been my experience, for accesses by the Linux kernel early in the boot process; usually we end up in a loop of ISI interrupts because the HPT isn't set up yet, with the original interrupt cause (and PC) lost long ago. The Power ISA was changed in version 2.05 (POWER6) to specify that accesses to unimplemented SPRs by privileged code must be no-ops on server processors. Before that the architecture allowed either an illegal instruction interrupt or "boundedly undefined" behaviour (which would include a no-op). So, if we're emulating POWERx for x >= 6, to be correct we need to do the no-op behaviour, even if we retain the option of making them trap for debugging purposes. Of course at the moment we basically never look at what specific CPU we're emulating, but maybe now we have to. Regards, Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/07/2014 09:09 AM, Paul Mackerras wrote: > On Fri, May 02, 2014 at 10:35:09AM +0200, Alexander Graf wrote: >> On 05/01/2014 12:12 AM, Paul Mackerras wrote: >>> On Tue, Apr 29, 2014 at 06:17:37PM +0200, Alexander Graf wrote: >>>> When we expose a POWER8 CPU into the guest, it will start accessing PMU SPRs >>>> that we don't emulate. Just ignore accesses to them. >>>> >>>> Signed-off-by: Alexander Graf <agraf@suse.de> >>> This patch is OK as it stands, but in fact the architecture says that >>> kernel accesses to unimplemented SPRs are mostly supposed to be no-ops >>> rather than causing a trap (mostly == excluding mtspr to 0 or mfspr >> >from 0, 4, 5 or 6). I have a patch to implement that, which I'll >>> post. >> I think what we want is a flag similar to x86 where we can force >> ignore unknown SPRs, but leave it at triggering an interrupt as >> default. We usually have to be at least aware of unknown SPRs and >> check that not implementing them is ok for the guest. >> >> Debugging a program interrupt because of an unknown SPR is usually a >> lot easier than debugging a breaking guest because it was using the >> SPR as storage and we didn't back it by anything. > That has not been my experience, for accesses by the Linux kernel > early in the boot process; usually we end up in a loop of ISI > interrupts because the HPT isn't set up yet, with the original > interrupt cause (and PC) lost long ago. > > The Power ISA was changed in version 2.05 (POWER6) to specify that > accesses to unimplemented SPRs by privileged code must be no-ops on > server processors. Before that the architecture allowed either an > illegal instruction interrupt or "boundedly undefined" behaviour > (which would include a no-op). > > So, if we're emulating POWERx for x >= 6, to be correct we need to do > the no-op behaviour, even if we retain the option of making them trap > for debugging purposes. Of course at the moment we basically never > look at what specific CPU we're emulating, but maybe now we have to. I think it makes sense to have some more detailed knowledge of the target CPU we're modeling for other reasons, but for this we should just settle on either trapping or not trapping by default. Finding the root cause of an ISI storm is easy when you run traces. Finding the root cause of a null pointer exception because Linux was trying to read from somewhere it mfspr()'d before requires quite a bit more knowledge of what's going on. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c index 45d0a80..914beb2 100644 --- a/arch/powerpc/kvm/book3s_emulate.c +++ b/arch/powerpc/kvm/book3s_emulate.c @@ -455,6 +455,11 @@ int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val) case SPRN_WPAR_GEKKO: case SPRN_MSSSR0: case SPRN_DABR: + case SPRN_MMCRS: + case SPRN_MMCRA: + case SPRN_MMCR0: + case SPRN_MMCR1: + case SPRN_MMCR2: break; unprivileged: default: @@ -553,6 +558,11 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val case SPRN_WPAR_GEKKO: case SPRN_MSSSR0: case SPRN_DABR: + case SPRN_MMCRS: + case SPRN_MMCRA: + case SPRN_MMCR0: + case SPRN_MMCR1: + case SPRN_MMCR2: *spr_val = 0; break; default:
When we expose a POWER8 CPU into the guest, it will start accessing PMU SPRs that we don't emulate. Just ignore accesses to them. Signed-off-by: Alexander Graf <agraf@suse.de> --- arch/powerpc/kvm/book3s_emulate.c | 10 ++++++++++ 1 file changed, 10 insertions(+)