[PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with paravirt.

[PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with paravirt.

[Bharat Bhushan]

From: Bhushan Bharat-R65777 <R65777@freescale.com>

If there is pending critical or machine check interrupt then guest
would like to capture it when guest enable MSR.CE and MSR_ME respectively.
Also as mostly MSR_CE and MSR_ME are updated with rfi/rfci/rfmii
which anyway traps so removing the the paravirt optimization for MSR.CE
and MSR.ME.

Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
 Documentation/virtual/kvm/ppc-pv.txt |    2 --
 arch/powerpc/kernel/kvm_emul.S       |    2 +-
 2 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/Documentation/virtual/kvm/ppc-pv.txt b/Documentation/virtual/kvm/ppc-pv.txt
index 6e7c370..4911cf9 100644
--- a/Documentation/virtual/kvm/ppc-pv.txt
+++ b/Documentation/virtual/kvm/ppc-pv.txt
@@ -109,8 +109,6 @@ The following bits are safe to be set inside the guest:
 
   MSR_EE
   MSR_RI
-  MSR_CR
-  MSR_ME
 
 If any other bit changes in the MSR, please still use mtmsr(d).
 
diff --git a/arch/powerpc/kernel/kvm_emul.S b/arch/powerpc/kernel/kvm_emul.S
index e291cf3..3dfa890 100644
--- a/arch/powerpc/kernel/kvm_emul.S
+++ b/arch/powerpc/kernel/kvm_emul.S
@@ -132,7 +132,7 @@ kvm_emulate_mtmsrd_len:
 	.long (kvm_emulate_mtmsrd_end - kvm_emulate_mtmsrd) / 4
 
 
-#define MSR_SAFE_BITS (MSR_EE | MSR_CE | MSR_ME | MSR_RI)
+#define MSR_SAFE_BITS (MSR_EE | MSR_RI)
 #define MSR_CRITICAL_BITS ~MSR_SAFE_BITS
 
 .global kvm_emulate_mtmsr
-- 
1.7.0.4

Re: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with paravirt.

[Alexander Graf]

On 05/15/2012 09:37 AM, Bharat Bhushan wrote:
> From: Bhushan Bharat-R65777<R65777@freescale.com>
>
> If there is pending critical or machine check interrupt then guest
> would like to capture it when guest enable MSR.CE and MSR_ME respectively.
> Also as mostly MSR_CE and MSR_ME are updated with rfi/rfci/rfmii
> which anyway traps so removing the the paravirt optimization for MSR.CE
> and MSR.ME.

It's only not safe for e500mc and above, right? E500v2 and book3s should 
be fine.


Alex

RE: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with paravirt.

[Bhushan Bharat-R65777]

> -----Original Message-----
> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-owner@vger.kernel.org] On
> Behalf Of Alexander Graf
> Sent: Tuesday, May 15, 2012 8:01 PM
> To: Bhushan Bharat-R65777
> Cc: kvm-ppc@vger.kernel.org; Bhushan Bharat-R65777
> Subject: Re: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with paravirt.
> 
> On 05/15/2012 09:37 AM, Bharat Bhushan wrote:
> > From: Bhushan Bharat-R65777<R65777@freescale.com>
> >
> > If there is pending critical or machine check interrupt then guest
> > would like to capture it when guest enable MSR.CE and MSR_ME respectively.
> > Also as mostly MSR_CE and MSR_ME are updated with rfi/rfci/rfmii which
> > anyway traps so removing the the paravirt optimization for MSR.CE and
> > MSR.ME.
> 
> It's only not safe for e500mc and above, right?

For e500mc and above the paravirt emulation code will not come into picture, And critical and machine check interrupt will happen only if MSR have corresponding bits set. So they are already safe. Is not it?

> E500v2 and book3s should be
> fine.

And with this patch e500v2 will be fine? Not sure of book3s :).

Thanks
-Bharat

> 
> 
> Alex
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with paravirt.

[Alexander Graf]

On 05/16/2012 10:42 AM, Bhushan Bharat-R65777 wrote:
>
>> -----Original Message-----
>> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-owner@vger.kernel.org] On
>> Behalf Of Alexander Graf
>> Sent: Tuesday, May 15, 2012 8:01 PM
>> To: Bhushan Bharat-R65777
>> Cc: kvm-ppc@vger.kernel.org; Bhushan Bharat-R65777
>> Subject: Re: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with paravirt.
>>
>> On 05/15/2012 09:37 AM, Bharat Bhushan wrote:
>>> From: Bhushan Bharat-R65777<R65777@freescale.com>
>>>
>>> If there is pending critical or machine check interrupt then guest
>>> would like to capture it when guest enable MSR.CE and MSR_ME respectively.
>>> Also as mostly MSR_CE and MSR_ME are updated with rfi/rfci/rfmii which
>>> anyway traps so removing the the paravirt optimization for MSR.CE and
>>> MSR.ME.
>> It's only not safe for e500mc and above, right?
> For e500mc and above the paravirt emulation code will not come into picture, And critical and machine check interrupt will happen only if MSR have corresponding bits set. So they are already safe. Is not it?

Yup, though it might be worth documenting the fact with a few #ifdef's, 
in case anyone wants to run PR KVM on e500mc ever.

>
>> E500v2 and book3s should be
>> fine.
> And with this patch e500v2 will be fine? Not sure of book3s :).

Well, e500v2 has edge triggered MCs, no? MSR.CE is unsafe however, as 
criticals are basically the same as externals.


Alex

RE: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with paravirt.

[Sethi Varun-B16395]

> -----Original Message-----
> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
> owner@vger.kernel.org] On Behalf Of Alexander Graf
> Sent: Wednesday, May 16, 2012 2:42 PM
> To: Bhushan Bharat-R65777
> Cc: kvm-ppc@vger.kernel.org
> Subject: Re: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with
> paravirt.
> 
> On 05/16/2012 10:42 AM, Bhushan Bharat-R65777 wrote:
> >
> >> -----Original Message-----
> >> From: kvm-ppc-owner@vger.kernel.org
> >> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf
> >> Sent: Tuesday, May 15, 2012 8:01 PM
> >> To: Bhushan Bharat-R65777
> >> Cc: kvm-ppc@vger.kernel.org; Bhushan Bharat-R65777
> >> Subject: Re: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with
> paravirt.
> >>
> >> On 05/15/2012 09:37 AM, Bharat Bhushan wrote:
> >>> From: Bhushan Bharat-R65777<R65777@freescale.com>
> >>>
> >>> If there is pending critical or machine check interrupt then guest
> >>> would like to capture it when guest enable MSR.CE and MSR_ME
> respectively.
> >>> Also as mostly MSR_CE and MSR_ME are updated with rfi/rfci/rfmii
> >>> which anyway traps so removing the the paravirt optimization for
> >>> MSR.CE and MSR.ME.
> >> It's only not safe for e500mc and above, right?
> > For e500mc and above the paravirt emulation code will not come into
> picture, And critical and machine check interrupt will happen only if MSR
> have corresponding bits set. So they are already safe. Is not it?
> 
> Yup, though it might be worth documenting the fact with a few #ifdef's,
> in case anyone wants to run PR KVM on e500mc ever.
> 
> >
Synchronous error report machine checks don't depend on MSR[ME] in case of e500mc.

> >> E500v2 and book3s should be
> >> fine.
> > And with this patch e500v2 will be fine? Not sure of book3s :).
> 
> Well, e500v2 has edge triggered MCs, no? MSR.CE is unsafe however, as
> criticals are basically the same as externals.
> 

In case of e500v2 machine check would be reported if MSR[ME], but if MSR[ME] is not set
the core would enter a check stop state. Yes, machine checks on e500v2 are edge triggered.
Why is MSR[CE] unsafe?

-Varun

Re: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with paravirt.

[Alexander Graf]

On 05/16/2012 02:27 PM, Sethi Varun-B16395 wrote:
>
>> -----Original Message-----
>> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
>> owner@vger.kernel.org] On Behalf Of Alexander Graf
>> Sent: Wednesday, May 16, 2012 2:42 PM
>> To: Bhushan Bharat-R65777
>> Cc: kvm-ppc@vger.kernel.org
>> Subject: Re: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with
>> paravirt.
>>
>> On 05/16/2012 10:42 AM, Bhushan Bharat-R65777 wrote:
>>>> -----Original Message-----
>>>> From: kvm-ppc-owner@vger.kernel.org
>>>> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf
>>>> Sent: Tuesday, May 15, 2012 8:01 PM
>>>> To: Bhushan Bharat-R65777
>>>> Cc: kvm-ppc@vger.kernel.org; Bhushan Bharat-R65777
>>>> Subject: Re: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with
>> paravirt.
>>>> On 05/15/2012 09:37 AM, Bharat Bhushan wrote:
>>>>> From: Bhushan Bharat-R65777<R65777@freescale.com>
>>>>>
>>>>> If there is pending critical or machine check interrupt then guest
>>>>> would like to capture it when guest enable MSR.CE and MSR_ME
>> respectively.
>>>>> Also as mostly MSR_CE and MSR_ME are updated with rfi/rfci/rfmii
>>>>> which anyway traps so removing the the paravirt optimization for
>>>>> MSR.CE and MSR.ME.
>>>> It's only not safe for e500mc and above, right?
>>> For e500mc and above the paravirt emulation code will not come into
>> picture, And critical and machine check interrupt will happen only if MSR
>> have corresponding bits set. So they are already safe. Is not it?
>>
>> Yup, though it might be worth documenting the fact with a few #ifdef's,
>> in case anyone wants to run PR KVM on e500mc ever.
>>
> Synchronous error report machine checks don't depend on MSR[ME] in case of e500mc.
>
>>>> E500v2 and book3s should be
>>>> fine.
>>> And with this patch e500v2 will be fine? Not sure of book3s :).
>> Well, e500v2 has edge triggered MCs, no? MSR.CE is unsafe however, as
>> criticals are basically the same as externals.
>>
> In case of e500v2 machine check would be reported if MSR[ME], but if MSR[ME] is not set
> the core would enter a check stop state. Yes, machine checks on e500v2 are edge triggered.
> Why is MSR[CE] unsafe?

MSR=0
* critical interrupt comes in *
mtmsr(MSR_CE)
--> MSR = MSR_CE
* interrupt should be delivered, but host doesn't get notified that 
MSR_CE is changing *

However, we completely ignore critical interrupts in KVM these days, no?


Alex

RE: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with paravirt.

[Sethi Varun-B16395]

> -----Original Message-----
> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
> owner@vger.kernel.org] On Behalf Of Alexander Graf
> Sent: Wednesday, May 16, 2012 6:18 PM
> To: Sethi Varun-B16395
> Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org
> Subject: Re: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with
> paravirt.
> 
> On 05/16/2012 02:27 PM, Sethi Varun-B16395 wrote:
> >
> >> -----Original Message-----
> >> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
> >> owner@vger.kernel.org] On Behalf Of Alexander Graf
> >> Sent: Wednesday, May 16, 2012 2:42 PM
> >> To: Bhushan Bharat-R65777
> >> Cc: kvm-ppc@vger.kernel.org
> >> Subject: Re: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with
> >> paravirt.
> >>
> >> On 05/16/2012 10:42 AM, Bhushan Bharat-R65777 wrote:
> >>>> -----Original Message-----
> >>>> From: kvm-ppc-owner@vger.kernel.org
> >>>> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf
> >>>> Sent: Tuesday, May 15, 2012 8:01 PM
> >>>> To: Bhushan Bharat-R65777
> >>>> Cc: kvm-ppc@vger.kernel.org; Bhushan Bharat-R65777
> >>>> Subject: Re: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE
> >>>> with
> >> paravirt.
> >>>> On 05/15/2012 09:37 AM, Bharat Bhushan wrote:
> >>>>> From: Bhushan Bharat-R65777<R65777@freescale.com>
> >>>>>
> >>>>> If there is pending critical or machine check interrupt then guest
> >>>>> would like to capture it when guest enable MSR.CE and MSR_ME
> >> respectively.
> >>>>> Also as mostly MSR_CE and MSR_ME are updated with rfi/rfci/rfmii
> >>>>> which anyway traps so removing the the paravirt optimization for
> >>>>> MSR.CE and MSR.ME.
> >>>> It's only not safe for e500mc and above, right?
> >>> For e500mc and above the paravirt emulation code will not come into
> >> picture, And critical and machine check interrupt will happen only if
> >> MSR have corresponding bits set. So they are already safe. Is not it?
> >>
> >> Yup, though it might be worth documenting the fact with a few
> >> #ifdef's, in case anyone wants to run PR KVM on e500mc ever.
> >>
> > Synchronous error report machine checks don't depend on MSR[ME] in case
> of e500mc.
> >
> >>>> E500v2 and book3s should be
> >>>> fine.
> >>> And with this patch e500v2 will be fine? Not sure of book3s :).
> >> Well, e500v2 has edge triggered MCs, no? MSR.CE is unsafe however, as
> >> criticals are basically the same as externals.
> >>
> > In case of e500v2 machine check would be reported if MSR[ME], but if
> > MSR[ME] is not set the core would enter a check stop state. Yes,
> machine checks on e500v2 are edge triggered.
> > Why is MSR[CE] unsafe?
> 
> MSR=0
> * critical interrupt comes in *
> mtmsr(MSR_CE)
> --> MSR = MSR_CE
> * interrupt should be delivered, but host doesn't get notified that
> MSR_CE is changing *
> 
> However, we completely ignore critical interrupts in KVM these days, no?
> 
Well, it's least likely that MSR_CE/MSR_ME would get changed directly. They would
mostly get changed via rfci/rfmci, which would trap.

-Varun

Re: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with paravirt.

[Scott Wood]

On 05/16/2012 10:13 AM, Sethi Varun-B16395 wrote:
>>> Why is MSR[CE] unsafe?
>>
>> MSR=0
>> * critical interrupt comes in *
>> mtmsr(MSR_CE)
>> --> MSR = MSR_CE
>> * interrupt should be delivered, but host doesn't get notified that
>> MSR_CE is changing *
>>
>> However, we completely ignore critical interrupts in KVM these days, no?

Watchdog will use them.

> Well, it's least likely that MSR_CE/MSR_ME would get changed directly. They would
> mostly get changed via rfci/rfmci, which would trap.

It being unlikely means that there's little performance downside to this
patch -- it doesn't mean that this patch is unnecessary.

FWIW, Topaz directly manipulates MSR[CE] extensively, since it uses
critical interrupts as its main interrupts (to distinguish from EE
interrupts which are delivered directly to the guest).

-Scott

RE: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with paravirt.

[Sethi Varun-B16395]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, May 17, 2012 3:37 AM
> To: Sethi Varun-B16395
> Cc: Alexander Graf; Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org
> Subject: Re: [PATCH] KVM: PPC: Not optimizing MSR_CE and MSR_DE with
> paravirt.
> 
> On 05/16/2012 10:13 AM, Sethi Varun-B16395 wrote:
> >>> Why is MSR[CE] unsafe?
> >>
> >> MSR=0
> >> * critical interrupt comes in *
> >> mtmsr(MSR_CE)
> >> --> MSR = MSR_CE
> >> * interrupt should be delivered, but host doesn't get notified that
> >> MSR_CE is changing *
> >>
> >> However, we completely ignore critical interrupts in KVM these days,
> no?
> 
> Watchdog will use them.
> 
> > Well, it's least likely that MSR_CE/MSR_ME would get changed directly.
> > They would mostly get changed via rfci/rfmci, which would trap.
> 
> It being unlikely means that there's little performance downside to this
> patch -- it doesn't mean that this patch is unnecessary.
> 
Agreed.

> FWIW, Topaz directly manipulates MSR[CE] extensively, since it uses
> critical interrupts as its main interrupts (to distinguish from EE
> interrupts which are delivered directly to the guest).
> 
Yes, but at least we won't see that happening with linux.

-Varun