RE: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
-Original Message- From: Wood Scott-B07421 Sent: Friday, July 18, 2014 6:19 AM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-...@vger.kernel.org; kvm@vger.kernel.org; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On Fri, 2014-07-18 at 02:37 +0200, Alexander Graf wrote: On 18.07.14 02:36, Scott Wood wrote: On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. The same problem applies to e500mc. We have two SPRG3 registers 1) SPRN_SPRG3 -- All read/write access to this register in guest will trap and emulate, So no need to save/restore. 2) SPRN_SPRG3R -- This is guest read only and We do not see any user of this register, so can leave this for now There we have SPRN_GSPRG3, no? Oh, right. Since it's only a problem for PR-mode, it can be fixed without needing to avoid SPRG3 entirely, since PR-mode doesn't use DO_KVM. We'd only need to avoid using SPRG_THREAD in __KVM_HANDLER (i.e. revert commit ffe129ecd79779221fdb03305049ec8b5a8beb0f). Did not get why using SPRG_THREAD here is a problem? Is not this will always access host SPRG3 and guest read write will always trap (maintained in vcpu-arch-shared-reg-sprg3) Yes we are not consistent with KVM, the host SPRG1 is used to point to the current VCPU data structure , which need to be corrected Thanks -Bharat And if we decide it's not worthwhile and don't revert that commit, we should at least remove the comment that Under KVM, the host SPRG1 is used to point to the current VCPU data structure... -Scott
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On 18.07.14 11:57, bharat.bhus...@freescale.com wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, July 18, 2014 6:19 AM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-...@vger.kernel.org; kvm@vger.kernel.org; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On Fri, 2014-07-18 at 02:37 +0200, Alexander Graf wrote: On 18.07.14 02:36, Scott Wood wrote: On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. The same problem applies to e500mc. We have two SPRG3 registers 1) SPRN_SPRG3 -- All read/write access to this register in guest will trap and emulate, So no need to save/restore. 2) SPRN_SPRG3R -- This is guest read only and We do not see any user of this register, so can leave this for now There we have SPRN_GSPRG3, no? Oh, right. Since it's only a problem for PR-mode, it can be fixed without needing to avoid SPRG3 entirely, since PR-mode doesn't use DO_KVM. We'd only need to avoid using SPRG_THREAD in __KVM_HANDLER (i.e. revert commit ffe129ecd79779221fdb03305049ec8b5a8beb0f). Did not get why using SPRG_THREAD here is a problem? Is not this will always access host SPRG3 and guest read write will always trap (maintained in vcpu-arch-shared-reg-sprg3) Guest reads via SPRG3R will access the real SPRG3 register. Guest reads via SPRG3 will trap :). Alex -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On 18.07.14 02:49, Scott Wood wrote: On Fri, 2014-07-18 at 02:37 +0200, Alexander Graf wrote: On 18.07.14 02:36, Scott Wood wrote: On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. The same problem applies to e500mc. There we have SPRN_GSPRG3, no? Oh, right. Since it's only a problem for PR-mode, it can be fixed without needing to avoid SPRG3 entirely, since PR-mode doesn't use DO_KVM. We'd only need to avoid using SPRG_THREAD in __KVM_HANDLER (i.e. revert commit ffe129ecd79779221fdb03305049ec8b5a8beb0f). And if we decide it's not worthwhile and don't revert that commit, we should at least remove the comment that Under KVM, the host SPRG1 is used to point to the current VCPU data structure... Agreed, please send a patch :). Alex -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, July 18, 2014 4:25 PM To: Bhushan Bharat-R65777; Wood Scott-B07421 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 18.07.14 11:57, bharat.bhus...@freescale.com wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, July 18, 2014 6:19 AM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-...@vger.kernel.org; kvm@vger.kernel.org; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On Fri, 2014-07-18 at 02:37 +0200, Alexander Graf wrote: On 18.07.14 02:36, Scott Wood wrote: On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. The same problem applies to e500mc. We have two SPRG3 registers 1) SPRN_SPRG3 -- All read/write access to this register in guest will trap and emulate, So no need to save/restore. 2) SPRN_SPRG3R -- This is guest read only and We do not see any user of this register, so can leave this for now There we have SPRN_GSPRG3, no? Oh, right. Since it's only a problem for PR-mode, it can be fixed without needing to avoid SPRG3 entirely, since PR-mode doesn't use DO_KVM. We'd only need to avoid using SPRG_THREAD in __KVM_HANDLER (i.e. revert commit ffe129ecd79779221fdb03305049ec8b5a8beb0f). Did not get why using SPRG_THREAD here is a problem? Is not this will always access host SPRG3 and guest read write will always trap (maintained in vcpu-arch-shared-reg-sprg3) Guest reads via SPRG3R will access the real SPRG3 register. Guest reads via SPRG3 will trap :). Agree, Also we do not see Linux as guest is accessing SPRG3R. But what I did not get what the mentioned patch have to do with that? Alex
RE: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On 18.07.14 11:57, bharat.bhus...@freescale.com wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, July 18, 2014 6:19 AM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-...@vger.kernel.org; kvm@vger.kernel.org; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On Fri, 2014-07-18 at 02:37 +0200, Alexander Graf wrote: On 18.07.14 02:36, Scott Wood wrote: On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. The same problem applies to e500mc. We have two SPRG3 registers 1) SPRN_SPRG3 -- All read/write access to this register in guest will trap and emulate, So no need to save/restore. 2) SPRN_SPRG3R -- This is guest read only and We do not see any user of this register, so can leave this for now There we have SPRN_GSPRG3, no? Oh, right. Since it's only a problem for PR-mode, it can be fixed without needing to avoid SPRG3 entirely, since PR-mode doesn't use DO_KVM. We'd only need to avoid using SPRG_THREAD in __KVM_HANDLER (i.e. revert commit ffe129ecd79779221fdb03305049ec8b5a8beb0f). Did not get why using SPRG_THREAD here is a problem? Is not this will always access host SPRG3 and guest read write will always trap (maintained in vcpu-arch-shared-reg-sprg3) Guest reads via SPRG3R will access the real SPRG3 register. Guest reads via SPRG3 will trap :). Agree, Also we do not see Linux as guest is accessing SPRG3R. But what I did not get what the mentioned patch have to do with that? Got it after discussion on IRC, If we need to care about SPRNG3R emulation then we should not use SPRG_THREAD . Thanks -Bharat Alex
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On 18.07.14 02:49, Scott Wood wrote: On Fri, 2014-07-18 at 02:37 +0200, Alexander Graf wrote: On 18.07.14 02:36, Scott Wood wrote: On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org Cc: k...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. The same problem applies to e500mc. There we have SPRN_GSPRG3, no? Oh, right. Since it's only a problem for PR-mode, it can be fixed without needing to avoid SPRG3 entirely, since PR-mode doesn't use DO_KVM. We'd only need to avoid using SPRG_THREAD in __KVM_HANDLER (i.e. revert commit ffe129ecd79779221fdb03305049ec8b5a8beb0f). And if we decide it's not worthwhile and don't revert that commit, we should at least remove the comment that Under KVM, the host SPRG1 is used to point to the current VCPU data structure... Agreed, please send a patch :). Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, July 18, 2014 4:25 PM To: Bhushan Bharat-R65777; Wood Scott-B07421 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 18.07.14 11:57, bharat.bhus...@freescale.com wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, July 18, 2014 6:19 AM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On Fri, 2014-07-18 at 02:37 +0200, Alexander Graf wrote: On 18.07.14 02:36, Scott Wood wrote: On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org Cc: k...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. The same problem applies to e500mc. We have two SPRG3 registers 1) SPRN_SPRG3 -- All read/write access to this register in guest will trap and emulate, So no need to save/restore. 2) SPRN_SPRG3R -- This is guest read only and We do not see any user of this register, so can leave this for now There we have SPRN_GSPRG3, no? Oh, right. Since it's only a problem for PR-mode, it can be fixed without needing to avoid SPRG3 entirely, since PR-mode doesn't use DO_KVM. We'd only need to avoid using SPRG_THREAD in __KVM_HANDLER (i.e. revert commit ffe129ecd79779221fdb03305049ec8b5a8beb0f). Did not get why using SPRG_THREAD here is a problem? Is not this will always access host SPRG3 and guest read write will always trap (maintained in vcpu-arch-shared-reg-sprg3) Guest reads via SPRG3R will access the real SPRG3 register. Guest reads via SPRG3 will trap :). Agree, Also we do not see Linux as guest is accessing SPRG3R. But what I did not get what the mentioned patch have to do with that? Alex
RE: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On 18.07.14 11:57, bharat.bhus...@freescale.com wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, July 18, 2014 6:19 AM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On Fri, 2014-07-18 at 02:37 +0200, Alexander Graf wrote: On 18.07.14 02:36, Scott Wood wrote: On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org Cc: k...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. The same problem applies to e500mc. We have two SPRG3 registers 1) SPRN_SPRG3 -- All read/write access to this register in guest will trap and emulate, So no need to save/restore. 2) SPRN_SPRG3R -- This is guest read only and We do not see any user of this register, so can leave this for now There we have SPRN_GSPRG3, no? Oh, right. Since it's only a problem for PR-mode, it can be fixed without needing to avoid SPRG3 entirely, since PR-mode doesn't use DO_KVM. We'd only need to avoid using SPRG_THREAD in __KVM_HANDLER (i.e. revert commit ffe129ecd79779221fdb03305049ec8b5a8beb0f). Did not get why using SPRG_THREAD here is a problem? Is not this will always access host SPRG3 and guest read write will always trap (maintained in vcpu-arch-shared-reg-sprg3) Guest reads via SPRG3R will access the real SPRG3 register. Guest reads via SPRG3 will trap :). Agree, Also we do not see Linux as guest is accessing SPRG3R. But what I did not get what the mentioned patch have to do with that? Got it after discussion on IRC, If we need to care about SPRNG3R emulation then we should not use SPRG_THREAD . Thanks -Bharat Alex
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ + PPC_LD(r3, VCPU_SHARED_SPRG3, r5) + mtspr SPRN_SPRG3, r3 We also need to restore it when resuming the host, no? Alex PPC_LD(r3, VCPU_SHARED_SPRG4, r5) mtspr SPRN_SPRG4W, r3 PPC_LD(r3, VCPU_SHARED_SPRG5, r5) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ + PPC_LD(r3, VCPU_SHARED_SPRG3, r5) + mtspr SPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Thanks -Bharat Alex PPC_LD(r3, VCPU_SHARED_SPRG4, r5) mtspr SPRN_SPRG4W, r3 PPC_LD(r3, VCPU_SHARED_SPRG5, r5) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ + PPC_LD(r3, VCPU_SHARED_SPRG3, r5) + mtspr SPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Alex -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). Alex -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:58 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ + PPC_LD(r3, VCPU_SHARED_SPRG3, r5) + mtspr SPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. I will add a comment in code as well. Thanks -Bharat Alex -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. Alex -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. The same problem applies to e500mc. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On 18.07.14 02:36, Scott Wood wrote: On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. The same problem applies to e500mc. There we have SPRN_GSPRG3, no? Alex -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On Fri, 2014-07-18 at 02:37 +0200, Alexander Graf wrote: On 18.07.14 02:36, Scott Wood wrote: On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-...@vger.kernel.org Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. The same problem applies to e500mc. There we have SPRN_GSPRG3, no? Oh, right. Since it's only a problem for PR-mode, it can be fixed without needing to avoid SPRG3 entirely, since PR-mode doesn't use DO_KVM. We'd only need to avoid using SPRG_THREAD in __KVM_HANDLER (i.e. revert commit ffe129ecd79779221fdb03305049ec8b5a8beb0f). And if we decide it's not worthwhile and don't revert that commit, we should at least remove the comment that Under KVM, the host SPRG1 is used to point to the current VCPU data structure... -Scott -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ + PPC_LD(r3, VCPU_SHARED_SPRG3, r5) + mtspr SPRN_SPRG3, r3 We also need to restore it when resuming the host, no? Alex PPC_LD(r3, VCPU_SHARED_SPRG4, r5) mtspr SPRN_SPRG4W, r3 PPC_LD(r3, VCPU_SHARED_SPRG5, r5) -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org Cc: k...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ + PPC_LD(r3, VCPU_SHARED_SPRG3, r5) + mtspr SPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Thanks -Bharat Alex PPC_LD(r3, VCPU_SHARED_SPRG4, r5) mtspr SPRN_SPRG4W, r3 PPC_LD(r3, VCPU_SHARED_SPRG5, r5) -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org Cc: k...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ + PPC_LD(r3, VCPU_SHARED_SPRG3, r5) + mtspr SPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org Cc: k...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:58 PM To: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org Cc: k...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org Cc: k...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ + PPC_LD(r3, VCPU_SHARED_SPRG3, r5) + mtspr SPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. I will add a comment in code as well. Thanks -Bharat Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org Cc: k...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. The same problem applies to e500mc. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On 18.07.14 02:36, Scott Wood wrote: On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org Cc: k...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. The same problem applies to e500mc. There we have SPRN_GSPRG3, no? Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
On Fri, 2014-07-18 at 02:37 +0200, Alexander Graf wrote: On 18.07.14 02:36, Scott Wood wrote: On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: On 18.07.14 02:28, Scott Wood wrote: On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: On 17.07.14 18:27, Alexander Graf wrote: On 17.07.14 18:24, bharat.bhus...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 17, 2014 9:41 PM To: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org Cc: k...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest On 16.07.14 08:02, Bharat Bhushan wrote: SPRG3 is guest accessible and SPRG3 can be clobbered by host or another guest, So this need to be restored when loading guest state. SPRG3 is not guest writeable. We should be doing this so that guest reads of SPRG3 through the alternative read-only SPR work, not because SPRG3 can be clobbered by host or another guest. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/kvm/booke_interrupts.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5ef..0d3403f 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -459,6 +459,8 @@ lightweight_exit: * written directly to the shared area, so we * need to reload them here with the guest's values. */ +PPC_LD(r3, VCPU_SHARED_SPRG3, r5) +mtsprSPRN_SPRG3, r3 We also need to restore it when resuming the host, no? I do not think host expect some meaningful value when returning from guest, same true for SPRG4-7. So there seems no reason to save host values and restore them. Linux no longer uses SPRG4-7 for itself. That is not true of SPRG3, as Alex points out. Hmm - arch/powerpc/include/asm/reg.h says: * All 32-bit: * - SPRG3 current thread_info pointer *(virtual on BookE, physical on others) but I can indeed find no trace of usage anywhere. This at least needs to go into the patch description. Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so incredibly important that I have no idea how we could possibly run without switching the host value back in very early. And even then our interrupt handlers wouldn't work anymore. This is more complicated :). To make this work we need to avoid SPRG3 as well, or at least avoid using it for something needed prior to DO_KVM. We also need to update the documentation in reg.h to reflect the fact that we don't use SPRG4-7 anymore on e500. I would personally prefer if we claim SPRG3R as unsupported on e500v2 until we find someone who actually uses it. There's a good chance we'd start jumping through a lot of hoops and reduce overall performance for no real-world gain today. The same problem applies to e500mc. There we have SPRN_GSPRG3, no? Oh, right. Since it's only a problem for PR-mode, it can be fixed without needing to avoid SPRG3 entirely, since PR-mode doesn't use DO_KVM. We'd only need to avoid using SPRG_THREAD in __KVM_HANDLER (i.e. revert commit ffe129ecd79779221fdb03305049ec8b5a8beb0f). And if we decide it's not worthwhile and don't revert that commit, we should at least remove the comment that Under KVM, the host SPRG1 is used to point to the current VCPU data structure... -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
