Patchwork [10/10] KVM: PPC: Enable the PAPR CAP for Book3S

login
register
mail settings
Submitter Alexander Graf
Date Aug. 9, 2011, 4:31 p.m.
Message ID <1312907508-14599-11-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/109253/
State Not Applicable
Headers show

Comments

Alexander Graf - Aug. 9, 2011, 4:31 p.m.
Now that Book3S PV mode can also run PAPR guests, we can add a PAPR cap and
enable it for all Book3S targets. Enabling that CAP switches KVM into PAPR
mode.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/kvm/powerpc.c |    5 +++++
 include/linux/kvm.h        |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)
Paul Mackerras - Aug. 10, 2011, 4:42 a.m.
On Tue, Aug 09, 2011 at 06:31:48PM +0200, Alexander Graf wrote:

> Now that Book3S PV mode can also run PAPR guests, we can add a PAPR cap and
> enable it for all Book3S targets. Enabling that CAP switches KVM into PAPR
> mode.

Don't we want to enable it only for 64-bit hosts?  Trying to run a
PAPR guest on a 32-bit Book 3S host won't work very well, unless I am
missing something...

Regards,
Paul.
Alexander Graf - Aug. 10, 2011, 7:59 a.m.
Am 10.08.2011 um 06:42 schrieb Paul Mackerras <paulus@samba.org>:

> On Tue, Aug 09, 2011 at 06:31:48PM +0200, Alexander Graf wrote:
> 
>> Now that Book3S PV mode can also run PAPR guests, we can add a PAPR cap and
>> enable it for all Book3S targets. Enabling that CAP switches KVM into PAPR
>> mode.
> 
> Don't we want to enable it only for 64-bit hosts?  Trying to run a
> PAPR guest on a 32-bit Book 3S host won't work very well, unless I am
> missing something...

I agree that it doesn't make sense, but if anything we should restrict it to 64-bit _guests_. you can also run 32-bit guests on 64-bit hosts.

And so far, we don't have a single interface setting PVR and PAPR mode at the same time, so you could still enable PAPR with a 64-bit guest CPU and then switch to a 32-bit CPU.

It'd be a nightmare to check all configurations on every setter function.

Unless...

We could introduce a sanity check function that gets executed every time we change PVR or enable PAPR. That could set a variable in the vcpu struct to indicate that the config is ok. We could then check that on vcpu_run.


Alex

>
Paul Mackerras - Aug. 10, 2011, 12:26 p.m.
On Wed, Aug 10, 2011 at 09:59:41AM +0200, Alexander Graf wrote:
> 
> Am 10.08.2011 um 06:42 schrieb Paul Mackerras <paulus@samba.org>:
> 
> > On Tue, Aug 09, 2011 at 06:31:48PM +0200, Alexander Graf wrote:
> > 
> >> Now that Book3S PV mode can also run PAPR guests, we can add a PAPR cap and
> >> enable it for all Book3S targets. Enabling that CAP switches KVM into PAPR
> >> mode.
> > 
> > Don't we want to enable it only for 64-bit hosts?  Trying to run a
> > PAPR guest on a 32-bit Book 3S host won't work very well, unless I am
> > missing something...
> 
> I agree that it doesn't make sense, but if anything we should
> restrict it to 64-bit _guests_. you can also run 32-bit guests on
> 64-bit hosts.

I had a look in PAPR and I didn't find anything that says the
processor has to be 64-bit, so I guess a 32-bit PAPR guest is possible
in theory.  However, I don't think there are currently any 32-bit PAPR
operating systems that would use hcalls.

> And so far, we don't have a single interface setting PVR and PAPR
> mode at the same time, so you could still enable PAPR with a 64-bit
> guest CPU and then switch to a 32-bit CPU.
> 
> It'd be a nightmare to check all configurations on every setter function.
> 
> Unless...
> 
> We could introduce a sanity check function that gets executed every
> time we change PVR or enable PAPR. That could set a variable in the
> vcpu struct to indicate that the config is ok. We could then check
> that on vcpu_run.

It's probably not worth worrying about it.

The rest of the series looks very nice.

Regards,
Paul.
Alexander Graf - Aug. 10, 2011, 12:29 p.m.
On 08/10/2011 02:26 PM, Paul Mackerras wrote:
> On Wed, Aug 10, 2011 at 09:59:41AM +0200, Alexander Graf wrote:
>> Am 10.08.2011 um 06:42 schrieb Paul Mackerras<paulus@samba.org>:
>>
>>> On Tue, Aug 09, 2011 at 06:31:48PM +0200, Alexander Graf wrote:
>>>
>>>> Now that Book3S PV mode can also run PAPR guests, we can add a PAPR cap and
>>>> enable it for all Book3S targets. Enabling that CAP switches KVM into PAPR
>>>> mode.
>>> Don't we want to enable it only for 64-bit hosts?  Trying to run a
>>> PAPR guest on a 32-bit Book 3S host won't work very well, unless I am
>>> missing something...
>> I agree that it doesn't make sense, but if anything we should
>> restrict it to 64-bit _guests_. you can also run 32-bit guests on
>> 64-bit hosts.
> I had a look in PAPR and I didn't find anything that says the
> processor has to be 64-bit, so I guess a 32-bit PAPR guest is possible
> in theory.  However, I don't think there are currently any 32-bit PAPR
> operating systems that would use hcalls.

That's what I figured :). The code flow we're affecting here is pretty 
much generic.

>> And so far, we don't have a single interface setting PVR and PAPR
>> mode at the same time, so you could still enable PAPR with a 64-bit
>> guest CPU and then switch to a 32-bit CPU.
>>
>> It'd be a nightmare to check all configurations on every setter function.
>>
>> Unless...
>>
>> We could introduce a sanity check function that gets executed every
>> time we change PVR or enable PAPR. That could set a variable in the
>> vcpu struct to indicate that the config is ok. We could then check
>> that on vcpu_run.
> It's probably not worth worrying about it.

Too late, already implemented it :). It really does make sense to have 
some sort of checking here - even if it only means that our hypercall 
implementation can't handle it yet or that we didn't test it. And we can 
put the "HV KVM can only run PAPR" check in there as well.


Alex

Patch

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 17a5c83..13bc798 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -189,6 +189,7 @@  int kvm_dev_ioctl_check_extension(long ext)
 #else
 	case KVM_CAP_PPC_SEGSTATE:
 	case KVM_CAP_PPC_HIOR:
+	case KVM_CAP_PPC_PAPR:
 #endif
 	case KVM_CAP_PPC_UNSET_IRQ:
 	case KVM_CAP_PPC_IRQ_LEVEL:
@@ -572,6 +573,10 @@  static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
 		r = 0;
 		vcpu->arch.osi_enabled = true;
 		break;
+	case KVM_CAP_PPC_PAPR:
+		r = 0;
+		vcpu->arch.papr_enabled = true;
+		break;
 	default:
 		r = -EINVAL;
 		break;
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 4d33f78..2d7161c 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -555,6 +555,7 @@  struct kvm_ppc_pvinfo {
 #define KVM_CAP_PPC_RMA	65
 #define KVM_CAP_MAX_VCPUS 66       /* returns max vcpus per vm */
 #define KVM_CAP_PPC_HIOR 67
+#define KVM_CAP_PPC_PAPR 68
 
 #ifdef KVM_CAP_IRQ_ROUTING