| Message ID | 1312907508-14599-11-git-send-email-agraf@suse.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
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.
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 >
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.
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
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
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(-)