Re: [RFC PATCH v3 6/6] kvm/ppc/mpic: add KVM_CAP_IRQ_MPIC
Am 05.04.2013 um 00:35 schrieb Scott Wood scottw...@freescale.com: On 04/04/2013 05:30:05 PM, Alexander Graf wrote: Am 04.04.2013 um 20:41 schrieb Scott Wood scottw...@freescale.com: On 04/04/2013 07:54:20 AM, Alexander Graf wrote: On 03.04.2013, at 03:57, Scott Wood wrote: +if (opp-mpic_mode_mask == GCR_MODE_PROXY) Shouldn't this be an ? The way the mode field was originally documented was a two-bit field, where 0b11 was external proxy, and 0b10 was reserved. If we use it would have to be: if ((opp-mpic_mode_mask GCR_MODE_PROXY) == GCR_MODE_PROXY) ... Simply testing opp-mpic_mode_mask GCR_MODE_PROXY would return true in the case of GCR_MODE_MIXED. In MPIC 4.3 external proxy is defined as a separate bit (GCR[CI]) that is ignored if the mixed-mode bit (GCR[M]) is not set, which makes it a bit more legitimate to view it as a bitmap. Still, I doubt we'll see new mode bits. Ok, please add a comment about this here then :). What sort of comment would you like? Or do you want me to use the (x y) == y version? /* This might need to be changed if GCR gets extended */ @@ -460,6 +464,13 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu) tasklet_kill(vcpu-arch.tasklet); kvmppc_remove_vcpu_debugfs(vcpu); + +switch (vcpu-arch.irq_type) { +case KVMPPC_IRQ_MPIC: +kvmppc_mpic_put(vcpu-arch.mpic); This doesn't tell the MPIC that this exact CPU is getting killed. What if we hotplug remove just a single CPU? Don't we have to deregister the CPU with the MPIC? If we ever support hot vcpu removal, yes. We'd probably need some MPIC code changes to accommodate that, and we wouldn't currently have a way to test it, so I'd rather make it obviously not supported for now. Is there any way to break heavily if user space attempts this? Is there any way for userspace to request this currently? They can close the vcpu fd, but the vcpu won't actually be destroyed until the vm goes down. Are you sure? X86 does CPU hotplug today, so there has to be something :) Alex -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: [RFC PATCH v3 6/6] kvm/ppc/mpic: add KVM_CAP_IRQ_MPIC
On 04/05/2013 01:09:50 AM, Alexander Graf wrote: Am 05.04.2013 um 00:35 schrieb Scott Wood scottw...@freescale.com: On 04/04/2013 05:30:05 PM, Alexander Graf wrote: Am 04.04.2013 um 20:41 schrieb Scott Wood scottw...@freescale.com: On 04/04/2013 07:54:20 AM, Alexander Graf wrote: On 03.04.2013, at 03:57, Scott Wood wrote: @@ -460,6 +464,13 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu) tasklet_kill(vcpu-arch.tasklet); kvmppc_remove_vcpu_debugfs(vcpu); + +switch (vcpu-arch.irq_type) { +case KVMPPC_IRQ_MPIC: +kvmppc_mpic_put(vcpu-arch.mpic); This doesn't tell the MPIC that this exact CPU is getting killed. What if we hotplug remove just a single CPU? Don't we have to deregister the CPU with the MPIC? If we ever support hot vcpu removal, yes. We'd probably need some MPIC code changes to accommodate that, and we wouldn't currently have a way to test it, so I'd rather make it obviously not supported for now. Is there any way to break heavily if user space attempts this? Is there any way for userspace to request this currently? They can close the vcpu fd, but the vcpu won't actually be destroyed until the vm goes down. Are you sure? X86 does CPU hotplug today, so there has to be something :) Hot add, or hot remove? Can you give me any hint on where to look? kvm_cpu_hotplug() appears to deal with hotplug of *physical* cpus -- and is currently a no-op on powerpc. Other than that, grepping isn't turning up much. -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: [RFC PATCH v3 6/6] kvm/ppc/mpic: add KVM_CAP_IRQ_MPIC
On 03.04.2013, at 03:57, Scott Wood wrote: Enabling this capability connects the vcpu to the designated in-kernel MPIC. Using explicit connections between vcpus and irqchips allows for flexibility, but the main benefit at the moment is that it simplifies the code -- KVM doesn't need vm-global state to remember which MPIC object is associated with this vm, and it doesn't need to care about ordering between irqchip creation and vcpu creation. Signed-off-by: Scott Wood scottw...@freescale.com --- Documentation/virtual/kvm/api.txt |8 ++ arch/powerpc/include/asm/kvm_host.h |8 ++ arch/powerpc/include/asm/kvm_ppc.h |2 ++ arch/powerpc/kvm/booke.c|4 ++- arch/powerpc/kvm/mpic.c | 49 +++ arch/powerpc/kvm/powerpc.c | 26 +++ include/uapi/linux/kvm.h|1 + 7 files changed, 92 insertions(+), 6 deletions(-) diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index d52f3f9..4c326ae 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -2728,3 +2728,11 @@ to receive the topmost interrupt vector. When disabled (args[0] == 0), behavior is as if this facility is unsupported. When this capability is enabled, KVM_EXIT_EPR can occur. + +6.6 KVM_CAP_IRQ_MPIC + +Architectures: ppc +Parameters: args[0] is the MPIC device fd +args[1] is the MPIC CPU number for this vcpu + +This capability connects the vcpu to an in-kernel MPIC device. diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 7e7aef9..2a2e235 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -375,6 +375,11 @@ struct kvmppc_booke_debug_reg { u64 dac[KVMPPC_BOOKE_MAX_DAC]; }; +#define KVMPPC_IRQ_DEFAULT 0 +#define KVMPPC_IRQ_MPIC 1 + +struct openpic; + struct kvm_vcpu_arch { ulong host_stack; u32 host_pid; @@ -554,6 +559,9 @@ struct kvm_vcpu_arch { unsigned long magic_page_pa; /* phys addr to map the magic page to */ unsigned long magic_page_ea; /* effect. addr to map the magic page to */ + int irq_type; /* one of KVM_IRQ_* */ + struct openpic *mpic; /* KVM_IRQ_MPIC */ + #ifdef CONFIG_KVM_BOOK3S_64_HV struct kvm_vcpu_arch_shared shregs; diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h index 3b63b97..f54707f 100644 --- a/arch/powerpc/include/asm/kvm_ppc.h +++ b/arch/powerpc/include/asm/kvm_ppc.h @@ -276,6 +276,8 @@ static inline void kvmppc_set_epr(struct kvm_vcpu *vcpu, u32 epr) } void kvmppc_mpic_set_epr(struct kvm_vcpu *vcpu); +int kvmppc_mpic_connect_vcpu(struct file *mpic_filp, struct kvm_vcpu *vcpu, + u32 cpu); int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu, struct kvm_config_tlb *cfg); diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index cddc6b3..7d00222 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -430,8 +430,10 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, if (update_epr == true) { if (vcpu-arch.epr_flags KVMPPC_EPR_USER) kvm_make_request(KVM_REQ_EPR_EXIT, vcpu); - else if (vcpu-arch.epr_flags KVMPPC_EPR_KERNEL) + else if (vcpu-arch.epr_flags KVMPPC_EPR_KERNEL) { + BUG_ON(vcpu-arch.irq_type != KVMPPC_IRQ_MPIC); kvmppc_mpic_set_epr(vcpu); + } } new_msr = msr_mask; diff --git a/arch/powerpc/kvm/mpic.c b/arch/powerpc/kvm/mpic.c index 8cda2fa..caffe3b 100644 --- a/arch/powerpc/kvm/mpic.c +++ b/arch/powerpc/kvm/mpic.c @@ -1159,7 +1159,7 @@ static uint32_t openpic_iack(struct openpic *opp, struct irq_dest *dst, void kvmppc_mpic_set_epr(struct kvm_vcpu *vcpu) { - struct openpic *opp = vcpu-arch.irqchip_priv; + struct openpic *opp = vcpu-arch.mpic; int cpu = vcpu-vcpu_id; unsigned long flags; @@ -1442,10 +1442,10 @@ static void map_mmio(struct openpic *opp) static void unmap_mmio(struct openpic *opp) { - BUG_ON(opp-mmio_mapped); - opp-mmio_mapped = false; - - kvm_io_bus_unregister_dev(opp-kvm, KVM_MMIO_BUS, opp-mmio); + if (opp-mmio_mapped) { + opp-mmio_mapped = false; + kvm_io_bus_unregister_dev(opp-kvm, KVM_MMIO_BUS, opp-mmio); + } } static int set_base_addr(struct openpic *opp, struct kvm_device_attr *attr) @@ -1681,6 +1681,45 @@ static const struct file_operations kvm_mpic_fops = { .release = kvm_mpic_release, }; +int kvmppc_mpic_connect_vcpu(struct file *mpic_filp, struct kvm_vcpu *vcpu, + u32 cpu) +{ +
Re: [RFC PATCH v3 6/6] kvm/ppc/mpic: add KVM_CAP_IRQ_MPIC
On 04/04/2013 07:54:20 AM, Alexander Graf wrote: On 03.04.2013, at 03:57, Scott Wood wrote: + if (opp-mpic_mode_mask == GCR_MODE_PROXY) Shouldn't this be an ? The way the mode field was originally documented was a two-bit field, where 0b11 was external proxy, and 0b10 was reserved. If we use it would have to be: if ((opp-mpic_mode_mask GCR_MODE_PROXY) == GCR_MODE_PROXY) ... Simply testing opp-mpic_mode_mask GCR_MODE_PROXY would return true in the case of GCR_MODE_MIXED. In MPIC 4.3 external proxy is defined as a separate bit (GCR[CI]) that is ignored if the mixed-mode bit (GCR[M]) is not set, which makes it a bit more legitimate to view it as a bitmap. Still, I doubt we'll see new mode bits. @@ -460,6 +464,13 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu) tasklet_kill(vcpu-arch.tasklet); kvmppc_remove_vcpu_debugfs(vcpu); + + switch (vcpu-arch.irq_type) { + case KVMPPC_IRQ_MPIC: + kvmppc_mpic_put(vcpu-arch.mpic); This doesn't tell the MPIC that this exact CPU is getting killed. What if we hotplug remove just a single CPU? Don't we have to deregister the CPU with the MPIC? If we ever support hot vcpu removal, yes. We'd probably need some MPIC code changes to accommodate that, and we wouldn't currently have a way to test it, so I'd rather make it obviously not supported for now. -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: [RFC PATCH v3 6/6] kvm/ppc/mpic: add KVM_CAP_IRQ_MPIC
Am 04.04.2013 um 20:41 schrieb Scott Wood scottw...@freescale.com: On 04/04/2013 07:54:20 AM, Alexander Graf wrote: On 03.04.2013, at 03:57, Scott Wood wrote: +if (opp-mpic_mode_mask == GCR_MODE_PROXY) Shouldn't this be an ? The way the mode field was originally documented was a two-bit field, where 0b11 was external proxy, and 0b10 was reserved. If we use it would have to be: if ((opp-mpic_mode_mask GCR_MODE_PROXY) == GCR_MODE_PROXY) ... Simply testing opp-mpic_mode_mask GCR_MODE_PROXY would return true in the case of GCR_MODE_MIXED. In MPIC 4.3 external proxy is defined as a separate bit (GCR[CI]) that is ignored if the mixed-mode bit (GCR[M]) is not set, which makes it a bit more legitimate to view it as a bitmap. Still, I doubt we'll see new mode bits. Ok, please add a comment about this here then :). @@ -460,6 +464,13 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu) tasklet_kill(vcpu-arch.tasklet); kvmppc_remove_vcpu_debugfs(vcpu); + +switch (vcpu-arch.irq_type) { +case KVMPPC_IRQ_MPIC: +kvmppc_mpic_put(vcpu-arch.mpic); This doesn't tell the MPIC that this exact CPU is getting killed. What if we hotplug remove just a single CPU? Don't we have to deregister the CPU with the MPIC? If we ever support hot vcpu removal, yes. We'd probably need some MPIC code changes to accommodate that, and we wouldn't currently have a way to test it, so I'd rather make it obviously not supported for now. Is there any way to break heavily if user space attempts this? Alex -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: [RFC PATCH v3 6/6] kvm/ppc/mpic: add KVM_CAP_IRQ_MPIC
On 04/04/2013 05:30:05 PM, Alexander Graf wrote: Am 04.04.2013 um 20:41 schrieb Scott Wood scottw...@freescale.com: On 04/04/2013 07:54:20 AM, Alexander Graf wrote: On 03.04.2013, at 03:57, Scott Wood wrote: +if (opp-mpic_mode_mask == GCR_MODE_PROXY) Shouldn't this be an ? The way the mode field was originally documented was a two-bit field, where 0b11 was external proxy, and 0b10 was reserved. If we use it would have to be: if ((opp-mpic_mode_mask GCR_MODE_PROXY) == GCR_MODE_PROXY) ... Simply testing opp-mpic_mode_mask GCR_MODE_PROXY would return true in the case of GCR_MODE_MIXED. In MPIC 4.3 external proxy is defined as a separate bit (GCR[CI]) that is ignored if the mixed-mode bit (GCR[M]) is not set, which makes it a bit more legitimate to view it as a bitmap. Still, I doubt we'll see new mode bits. Ok, please add a comment about this here then :). What sort of comment would you like? Or do you want me to use the (x y) == y version? @@ -460,6 +464,13 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu) tasklet_kill(vcpu-arch.tasklet); kvmppc_remove_vcpu_debugfs(vcpu); + +switch (vcpu-arch.irq_type) { +case KVMPPC_IRQ_MPIC: +kvmppc_mpic_put(vcpu-arch.mpic); This doesn't tell the MPIC that this exact CPU is getting killed. What if we hotplug remove just a single CPU? Don't we have to deregister the CPU with the MPIC? If we ever support hot vcpu removal, yes. We'd probably need some MPIC code changes to accommodate that, and we wouldn't currently have a way to test it, so I'd rather make it obviously not supported for now. Is there any way to break heavily if user space attempts this? Is there any way for userspace to request this currently? They can close the vcpu fd, but the vcpu won't actually be destroyed until the vm goes down. -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
[RFC PATCH v3 6/6] kvm/ppc/mpic: add KVM_CAP_IRQ_MPIC
Enabling this capability connects the vcpu to the designated in-kernel MPIC. Using explicit connections between vcpus and irqchips allows for flexibility, but the main benefit at the moment is that it simplifies the code -- KVM doesn't need vm-global state to remember which MPIC object is associated with this vm, and it doesn't need to care about ordering between irqchip creation and vcpu creation. Signed-off-by: Scott Wood scottw...@freescale.com --- Documentation/virtual/kvm/api.txt |8 ++ arch/powerpc/include/asm/kvm_host.h |8 ++ arch/powerpc/include/asm/kvm_ppc.h |2 ++ arch/powerpc/kvm/booke.c|4 ++- arch/powerpc/kvm/mpic.c | 49 +++ arch/powerpc/kvm/powerpc.c | 26 +++ include/uapi/linux/kvm.h|1 + 7 files changed, 92 insertions(+), 6 deletions(-) diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index d52f3f9..4c326ae 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -2728,3 +2728,11 @@ to receive the topmost interrupt vector. When disabled (args[0] == 0), behavior is as if this facility is unsupported. When this capability is enabled, KVM_EXIT_EPR can occur. + +6.6 KVM_CAP_IRQ_MPIC + +Architectures: ppc +Parameters: args[0] is the MPIC device fd +args[1] is the MPIC CPU number for this vcpu + +This capability connects the vcpu to an in-kernel MPIC device. diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 7e7aef9..2a2e235 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -375,6 +375,11 @@ struct kvmppc_booke_debug_reg { u64 dac[KVMPPC_BOOKE_MAX_DAC]; }; +#define KVMPPC_IRQ_DEFAULT 0 +#define KVMPPC_IRQ_MPIC1 + +struct openpic; + struct kvm_vcpu_arch { ulong host_stack; u32 host_pid; @@ -554,6 +559,9 @@ struct kvm_vcpu_arch { unsigned long magic_page_pa; /* phys addr to map the magic page to */ unsigned long magic_page_ea; /* effect. addr to map the magic page to */ + int irq_type; /* one of KVM_IRQ_* */ + struct openpic *mpic; /* KVM_IRQ_MPIC */ + #ifdef CONFIG_KVM_BOOK3S_64_HV struct kvm_vcpu_arch_shared shregs; diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h index 3b63b97..f54707f 100644 --- a/arch/powerpc/include/asm/kvm_ppc.h +++ b/arch/powerpc/include/asm/kvm_ppc.h @@ -276,6 +276,8 @@ static inline void kvmppc_set_epr(struct kvm_vcpu *vcpu, u32 epr) } void kvmppc_mpic_set_epr(struct kvm_vcpu *vcpu); +int kvmppc_mpic_connect_vcpu(struct file *mpic_filp, struct kvm_vcpu *vcpu, +u32 cpu); int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu, struct kvm_config_tlb *cfg); diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index cddc6b3..7d00222 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -430,8 +430,10 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, if (update_epr == true) { if (vcpu-arch.epr_flags KVMPPC_EPR_USER) kvm_make_request(KVM_REQ_EPR_EXIT, vcpu); - else if (vcpu-arch.epr_flags KVMPPC_EPR_KERNEL) + else if (vcpu-arch.epr_flags KVMPPC_EPR_KERNEL) { + BUG_ON(vcpu-arch.irq_type != KVMPPC_IRQ_MPIC); kvmppc_mpic_set_epr(vcpu); + } } new_msr = msr_mask; diff --git a/arch/powerpc/kvm/mpic.c b/arch/powerpc/kvm/mpic.c index 8cda2fa..caffe3b 100644 --- a/arch/powerpc/kvm/mpic.c +++ b/arch/powerpc/kvm/mpic.c @@ -1159,7 +1159,7 @@ static uint32_t openpic_iack(struct openpic *opp, struct irq_dest *dst, void kvmppc_mpic_set_epr(struct kvm_vcpu *vcpu) { - struct openpic *opp = vcpu-arch.irqchip_priv; + struct openpic *opp = vcpu-arch.mpic; int cpu = vcpu-vcpu_id; unsigned long flags; @@ -1442,10 +1442,10 @@ static void map_mmio(struct openpic *opp) static void unmap_mmio(struct openpic *opp) { - BUG_ON(opp-mmio_mapped); - opp-mmio_mapped = false; - - kvm_io_bus_unregister_dev(opp-kvm, KVM_MMIO_BUS, opp-mmio); + if (opp-mmio_mapped) { + opp-mmio_mapped = false; + kvm_io_bus_unregister_dev(opp-kvm, KVM_MMIO_BUS, opp-mmio); + } } static int set_base_addr(struct openpic *opp, struct kvm_device_attr *attr) @@ -1681,6 +1681,45 @@ static const struct file_operations kvm_mpic_fops = { .release = kvm_mpic_release, }; +int kvmppc_mpic_connect_vcpu(struct file *mpic_filp, struct kvm_vcpu *vcpu, +u32 cpu) +{ + struct openpic *opp = mpic_filp-private_data; + int ret = 0;