Re: [PATCH 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
On 05/24/2013 09:45:24 PM, David Gibson wrote: On Wed, May 22, 2013 at 04:06:57PM -0500, Scott Wood wrote: On 05/20/2013 10:06:46 PM, Alexey Kardashevskiy wrote: diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 8465c2a..da6bf61 100644 --- a/arch/powerpc/kvm/powerpc.c @@ -396,6 +396,7 @@ int kvm_dev_ioctl_check_extension(long ext) +++ b/arch/powerpc/kvm/powerpc.c break; #endif case KVM_CAP_SPAPR_MULTITCE: + case KVM_CAP_SPAPR_TCE_IOMMU: r = 1; break; default: Don't advertise SPAPR capabilities if it's not book3s -- and probably there's some additional limitation that would be appropriate. So, in the case of MULTITCE, that's not quite right. PR KVM can emulate a PAPR system on a BookE machine, and there's no reason not to allow TCE acceleration as well. That might (or might not; consider things like Altivec versus SPE opcode conflict, whether unimplemented SPRs trap, behavior of unprivileged SPRs/instructions, etc) be true in theory, but it's not currently a supported configuration. BookE KVM does not support emulating a different CPU than the host. In the unlikely case that ever changes to the point of allowing PAPR guests on a BookE host, then we can revisit how to properly determine whether the capability is supported, but for now the capability will never be valid in the CONFIG_BOOKE case (though I'd rather see it depend on an appropriate book3s symbol than depend on !BOOKE). Or we could just leave it as is, and let it indicate whether the host kernel supports the feature in general, with the user needing to understand when it's applicable... I'm a bit confused by the documentation, however -- the MULTITCE capability was documented in the capabilities that can be enabled section, but I don't see where it can be enabled. -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 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
On 05/26/2013 09:44:24 PM, Alexey Kardashevskiy wrote:
On 05/25/2013 12:45 PM, David Gibson wrote:
On Wed, May 22, 2013 at 04:06:57PM -0500, Scott Wood wrote:
On 05/20/2013 10:06:46 PM, Alexey Kardashevskiy wrote:
diff --git a/arch/powerpc/kvm/powerpc.c
b/arch/powerpc/kvm/powerpc.c
index 8465c2a..da6bf61 100644
--- a/arch/powerpc/kvm/powerpc.c
@@ -396,6 +396,7 @@ int kvm_dev_ioctl_check_extension(long ext)
+++ b/arch/powerpc/kvm/powerpc.c
break;
#endif
case KVM_CAP_SPAPR_MULTITCE:
+ case KVM_CAP_SPAPR_TCE_IOMMU:
r = 1;
break;
default:
Don't advertise SPAPR capabilities if it's not book3s -- and
probably there's some additional limitation that would be
appropriate.
So, in the case of MULTITCE, that's not quite right. PR KVM can
emulate a PAPR system on a BookE machine, and there's no reason not
to
allow TCE acceleration as well. We can't make it dependent on PAPR
mode being selected, because that's enabled per-vcpu, whereas these
capabilities are queried on the VM before the vcpus are created.
CAP_SPAPR_TCE_IOMMU should be dependent on the presence of suitable
host side hardware (i.e. a PAPR style IOMMU), though.
The capability says that the ioctl is supported. If there is no IOMMU
group
registered, than it will fail with a reasonable error and nobody gets
hurt.
What is the problem?
You could say that about a lot of the capabilities that just advertise
the existence of new ioctls. :-)
Sometimes it's nice to know in advance whether it's supported, before
actually requesting that something happen.
@@ -939,6 +940,9 @@ struct kvm_s390_ucas_mapping {
#define KVM_GET_DEVICE_ATTR _IOW(KVMIO, 0xe2, struct
kvm_device_attr)
#define KVM_HAS_DEVICE_ATTR _IOW(KVMIO, 0xe3, struct
kvm_device_attr)
+/* ioctl for SPAPR TCE IOMMU */
+#define KVM_CREATE_SPAPR_TCE_IOMMU _IOW(KVMIO, 0xe4, struct
kvm_create_spapr_tce_iommu)
Shouldn't this go under the vm ioctl section?
The KVM_CREATE_SPAPR_TCE_IOMMU ioctl (the version for emulated
devices) is
in this section so I decided to keep them together. Wrong?
You decided to keep KVM_CREATE_SPAPR_TCE_IOMMU together with
KVM_CREATE_SPAPR_TCE_IOMMU?
-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 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
On 05/29/2013 03:45 AM, Scott Wood wrote:
On 05/26/2013 09:44:24 PM, Alexey Kardashevskiy wrote:
On 05/25/2013 12:45 PM, David Gibson wrote:
On Wed, May 22, 2013 at 04:06:57PM -0500, Scott Wood wrote:
On 05/20/2013 10:06:46 PM, Alexey Kardashevskiy wrote:
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 8465c2a..da6bf61 100644
--- a/arch/powerpc/kvm/powerpc.c
@@ -396,6 +396,7 @@ int kvm_dev_ioctl_check_extension(long ext)
+++ b/arch/powerpc/kvm/powerpc.c
break;
#endif
case KVM_CAP_SPAPR_MULTITCE:
+case KVM_CAP_SPAPR_TCE_IOMMU:
r = 1;
break;
default:
Don't advertise SPAPR capabilities if it's not book3s -- and
probably there's some additional limitation that would be
appropriate.
So, in the case of MULTITCE, that's not quite right. PR KVM can
emulate a PAPR system on a BookE machine, and there's no reason not to
allow TCE acceleration as well. We can't make it dependent on PAPR
mode being selected, because that's enabled per-vcpu, whereas these
capabilities are queried on the VM before the vcpus are created.
CAP_SPAPR_TCE_IOMMU should be dependent on the presence of suitable
host side hardware (i.e. a PAPR style IOMMU), though.
The capability says that the ioctl is supported. If there is no IOMMU group
registered, than it will fail with a reasonable error and nobody gets hurt.
What is the problem?
You could say that about a lot of the capabilities that just advertise the
existence of new ioctls. :-)
Sometimes it's nice to know in advance whether it's supported, before
actually requesting that something happen.
Yes, would be nice. There is just no quick way to know if this real system
supports IOMMU groups. I could add another helper to generic IOMMU code
which would return the number of registered IOMMU groups but it is a bit
too much :)
@@ -939,6 +940,9 @@ struct kvm_s390_ucas_mapping {
#define KVM_GET_DEVICE_ATTR _IOW(KVMIO, 0xe2, struct
kvm_device_attr)
#define KVM_HAS_DEVICE_ATTR _IOW(KVMIO, 0xe3, struct
kvm_device_attr)
+/* ioctl for SPAPR TCE IOMMU */
+#define KVM_CREATE_SPAPR_TCE_IOMMU _IOW(KVMIO, 0xe4, struct
kvm_create_spapr_tce_iommu)
Shouldn't this go under the vm ioctl section?
The KVM_CREATE_SPAPR_TCE_IOMMU ioctl (the version for emulated devices) is
in this section so I decided to keep them together. Wrong?
You decided to keep KVM_CREATE_SPAPR_TCE_IOMMU together with
KVM_CREATE_SPAPR_TCE_IOMMU?
Yes.
--
Alexey
--
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 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
On 05/29/2013 09:35 AM, Scott Wood wrote:
On 05/28/2013 06:30:40 PM, Alexey Kardashevskiy wrote:
@@ -939,6 +940,9 @@ struct kvm_s390_ucas_mapping {
#define KVM_GET_DEVICE_ATTR _IOW(KVMIO, 0xe2, struct
kvm_device_attr)
#define KVM_HAS_DEVICE_ATTR _IOW(KVMIO, 0xe3, struct
kvm_device_attr)
+/* ioctl for SPAPR TCE IOMMU */
+#define KVM_CREATE_SPAPR_TCE_IOMMU _IOW(KVMIO, 0xe4, struct
kvm_create_spapr_tce_iommu)
Shouldn't this go under the vm ioctl section?
The KVM_CREATE_SPAPR_TCE_IOMMU ioctl (the version for emulated
devices) is
in this section so I decided to keep them together. Wrong?
You decided to keep KVM_CREATE_SPAPR_TCE_IOMMU together with
KVM_CREATE_SPAPR_TCE_IOMMU?
Yes.
Sigh. That's the same thing repeated. There's only one IOCTL. Nothing is
being kept together.
Sorry, I meant this ioctl - KVM_CREATE_SPAPR_TCE.
--
Alexey
--
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 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
On 05/29/2013 02:32 AM, Scott Wood wrote: On 05/24/2013 09:45:24 PM, David Gibson wrote: On Wed, May 22, 2013 at 04:06:57PM -0500, Scott Wood wrote: On 05/20/2013 10:06:46 PM, Alexey Kardashevskiy wrote: diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 8465c2a..da6bf61 100644 --- a/arch/powerpc/kvm/powerpc.c @@ -396,6 +396,7 @@ int kvm_dev_ioctl_check_extension(long ext) +++ b/arch/powerpc/kvm/powerpc.c break; #endif case KVM_CAP_SPAPR_MULTITCE: +case KVM_CAP_SPAPR_TCE_IOMMU: r = 1; break; default: Don't advertise SPAPR capabilities if it's not book3s -- and probably there's some additional limitation that would be appropriate. So, in the case of MULTITCE, that's not quite right. PR KVM can emulate a PAPR system on a BookE machine, and there's no reason not to allow TCE acceleration as well. That might (or might not; consider things like Altivec versus SPE opcode conflict, whether unimplemented SPRs trap, behavior of unprivileged SPRs/instructions, etc) be true in theory, but it's not currently a supported configuration. BookE KVM does not support emulating a different CPU than the host. In the unlikely case that ever changes to the point of allowing PAPR guests on a BookE host, then we can revisit how to properly determine whether the capability is supported, but for now the capability will never be valid in the CONFIG_BOOKE case (though I'd rather see it depend on an appropriate book3s symbol than depend on !BOOKE). Or we could just leave it as is, and let it indicate whether the host kernel supports the feature in general, with the user needing to understand when it's applicable... I'm a bit confused by the documentation, however -- the MULTITCE capability was documented in the capabilities that can be enabled section, but I don't see where it can be enabled. True, it cannot be enabled (but it could be enabled a long time ago), it is either supported or not, I'll fix the documentation. Thanks! -- Alexey -- 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
