Re:
Did You Recieve Our Last Notification!! -- 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
[PATCH v3 41/45] powerpc: Use get/put_online_cpus_atomic() to prevent CPU offline
Once stop_machine() is gone from the CPU offline path, we won't be able
to depend on disabling preemption to prevent CPUs from going offline
from under us.
Use the get/put_online_cpus_atomic() APIs to prevent CPUs from going
offline, while invoking from atomic context.
Cc: Benjamin Herrenschmidt
Cc: Gleb Natapov
Cc: Alexander Graf
Cc: Rob Herring
Cc: Grant Likely
Cc: Kumar Gala
Cc: Zhao Chenhui
Cc: linuxppc-...@lists.ozlabs.org
Cc: k...@vger.kernel.org
Cc: kvm-ppc@vger.kernel.org
Cc: oprofile-l...@lists.sf.net
Cc: cbe-oss-...@lists.ozlabs.org
Signed-off-by: Srivatsa S. Bhat
---
arch/powerpc/kernel/irq.c |7 ++-
arch/powerpc/kernel/machine_kexec_64.c |4 ++--
arch/powerpc/kernel/smp.c |2 ++
arch/powerpc/kvm/book3s_hv.c |5 +++--
arch/powerpc/mm/mmu_context_nohash.c |3 +++
arch/powerpc/oprofile/cell/spu_profiler.c |3 +++
arch/powerpc/oprofile/cell/spu_task_sync.c |4
arch/powerpc/oprofile/op_model_cell.c |6 ++
8 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index ca39bac..41e9961 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -45,6 +45,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -410,7 +411,10 @@ void migrate_irqs(void)
unsigned int irq;
static int warned;
cpumask_var_t mask;
- const struct cpumask *map = cpu_online_mask;
+ const struct cpumask *map;
+
+ get_online_cpus_atomic();
+ map = cpu_online_mask;
alloc_cpumask_var(&mask, GFP_ATOMIC);
@@ -436,6 +440,7 @@ void migrate_irqs(void)
}
free_cpumask_var(mask);
+ put_online_cpus_atomic();
local_irq_enable();
mdelay(1);
diff --git a/arch/powerpc/kernel/machine_kexec_64.c
b/arch/powerpc/kernel/machine_kexec_64.c
index 611acdf..38f6d75 100644
--- a/arch/powerpc/kernel/machine_kexec_64.c
+++ b/arch/powerpc/kernel/machine_kexec_64.c
@@ -187,7 +187,7 @@ static void kexec_prepare_cpus_wait(int wait_state)
int my_cpu, i, notified=-1;
hw_breakpoint_disable();
- my_cpu = get_cpu();
+ my_cpu = get_online_cpus_atomic();
/* Make sure each CPU has at least made it to the state we need.
*
* FIXME: There is a (slim) chance of a problem if not all of the CPUs
@@ -266,7 +266,7 @@ static void kexec_prepare_cpus(void)
*/
kexec_prepare_cpus_wait(KEXEC_STATE_REAL_MODE);
- put_cpu();
+ put_online_cpus_atomic();
}
#else /* ! SMP */
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index ee7ac5e..2123bec 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -277,9 +277,11 @@ void smp_send_debugger_break(void)
if (unlikely(!smp_ops))
return;
+ get_online_cpus_atomic();
for_each_online_cpu(cpu)
if (cpu != me)
do_message_pass(cpu, PPC_MSG_DEBUGGER_BREAK);
+ put_online_cpus_atomic();
}
#endif
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 2efa9dd..9d8a973 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -78,7 +79,7 @@ void kvmppc_fast_vcpu_kick(struct kvm_vcpu *vcpu)
++vcpu->stat.halt_wakeup;
}
- me = get_cpu();
+ me = get_online_cpus_atomic();
/* CPU points to the first thread of the core */
if (cpu != me && cpu >= 0 && cpu < nr_cpu_ids) {
@@ -88,7 +89,7 @@ void kvmppc_fast_vcpu_kick(struct kvm_vcpu *vcpu)
else if (cpu_online(cpu))
smp_send_reschedule(cpu);
}
- put_cpu();
+ put_online_cpus_atomic();
}
/*
diff --git a/arch/powerpc/mm/mmu_context_nohash.c
b/arch/powerpc/mm/mmu_context_nohash.c
index e779642..c7bdcb4 100644
--- a/arch/powerpc/mm/mmu_context_nohash.c
+++ b/arch/powerpc/mm/mmu_context_nohash.c
@@ -194,6 +194,8 @@ void switch_mmu_context(struct mm_struct *prev, struct
mm_struct *next)
unsigned int i, id, cpu = smp_processor_id();
unsigned long *map;
+ get_online_cpus_atomic();
+
/* No lockless fast path .. yet */
raw_spin_lock(&context_lock);
@@ -280,6 +282,7 @@ void switch_mmu_context(struct mm_struct *prev, struct
mm_struct *next)
pr_hardcont(" -> %d\n", id);
set_context(id, next->pgd);
raw_spin_unlock(&context_lock);
+ put_online_cpus_atomic();
}
/*
diff --git a/arch/powerpc/oprofile/cell/spu_profiler.c
b/arch/powerpc/oprofile/cell/spu_profiler.c
index b129d00..ab6e6c1 100644
--- a/arch/powerpc/oprofile/cell/spu_profiler.c
+++ b/arch/powerpc/oprofile/cell/spu_profiler.c
@@ -14,6 +14,7 @@
#include
#include
+#include
#include
#include
#inc
Re: [PATCH 8/8] KVM: PPC: Add hugepage support for IOMMU in-kernel handling
On 06/27/2013 12:02:36 AM, Alexey Kardashevskiy wrote:
+/*
+ * The KVM guest can be backed with 16MB pages.
+ * In this case, we cannot do page counting from the real mode
+ * as the compound pages are used - they are linked in a list
+ * with pointers as virtual addresses which are inaccessible
+ * in real mode.
+ *
+ * The code below keeps a 16MB pages list and uses page struct
+ * in real mode if it is already locked in RAM and inserted into
+ * the list or switches to the virtual mode where it can be
+ * handled in a usual manner.
+ */
+#define KVMPPC_HUGEPAGE_HASH(gpa) hash_32(gpa >> 24, 32)
+
+struct kvmppc_iommu_hugepage {
+ struct hlist_node hash_node;
+ unsigned long gpa; /* Guest physical address */
+ unsigned long hpa; /* Host physical address */
+ struct page *page; /* page struct of the very first
subpage */
+ unsigned long size; /* Huge page size (always 16MB at the
moment) */
+};
Shouldn't this be namespaced to something like "book3s" or "spapr"?
-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 Sun, Jun 23, 2013 at 10:41:24PM -0600, Alex Williamson wrote: > On Mon, 2013-06-24 at 13:52 +1000, David Gibson wrote: > > On Sat, Jun 22, 2013 at 08:28:06AM -0600, Alex Williamson wrote: > > > On Sat, 2013-06-22 at 22:03 +1000, David Gibson wrote: > > > > On Thu, Jun 20, 2013 at 08:55:13AM -0600, Alex Williamson wrote: > > > > > On Thu, 2013-06-20 at 18:48 +1000, Alexey Kardashevskiy wrote: > > > > > > On 06/20/2013 05:47 PM, Benjamin Herrenschmidt wrote: > > > > > > > On Thu, 2013-06-20 at 15:28 +1000, David Gibson wrote: > > > > > > >>> Just out of curiosity - would not get_file() and fput_atomic() > > > > > > >>> on a > > > > > > >> group's > > > > > > >>> file* do the right job instead of > > > > > > >>> vfio_group_add_external_user() and > > > > > > >>> vfio_group_del_external_user()? > > > > > > >> > > > > > > >> I was thinking that too. Grabbing a file reference would > > > > > > >> certainly be > > > > > > >> the usual way of handling this sort of thing. > > > > > > > > > > > > > > But that wouldn't prevent the group ownership to be returned to > > > > > > > the kernel or another user would it ? > > > > > > > > > > > > > > > > > > Holding the file pointer does not let the group->container_users > > > > > > counter go > > > > > > to zero > > > > > > > > > > How so? Holding the file pointer means the file won't go away, which > > > > > means the group release function won't be called. That means the > > > > > group > > > > > won't go away, but that doesn't mean it's attached to an IOMMU. A > > > > > user > > > > > could call UNSET_CONTAINER. > > > > > > > > Uhh... *thinks*. Ah, I see. > > > > > > > > I think the interface should not take the group fd, but the container > > > > fd. Holding a reference to *that* would keep the necessary things > > > > around. But more to the point, it's the right thing semantically: > > > > > > > > The container is essentially the handle on a host iommu address space, > > > > and so that's what should be bound by the KVM call to a particular > > > > guest iommu address space. e.g. it would make no sense to bind two > > > > different groups to different guest iommu address spaces, if they were > > > > in the same container - the guest thinks they are different spaces, > > > > but if they're in the same container they must be the same space. > > > > > > While the container is the gateway to the iommu, what empowers the > > > container to maintain an iommu is the group. What happens to a > > > container when all the groups are disconnected or closed? Groups are > > > the unit that indicates hardware access, not containers. Thanks, > > > > Uh... huh? I'm really not sure what you're getting at. > > > > The operation we're doing for KVM here is binding a guest iommu > > address space to a particular host iommu address space. Why would we > > not want to use the obvious handle on the host iommu address space, > > which is the container fd? > > AIUI, the request isn't for an interface through which to do iommu > mappings. The request is for an interface to show that the user has > sufficient privileges to do mappings. Groups are what gives the user > that ability. The iommu is also possibly associated with multiple iommu > groups and I believe what is being asked for here is a way to hold and > lock a single iommu group with iommu protection. > > >From a practical point of view, the iommu interface is de-privileged > once the groups are disconnected or closed. Holding a reference count > on the iommu fd won't prevent that. That means we'd have to use a > notifier to have KVM stop the side-channel iommu access. Meanwhile > holding the file descriptor for the group and adding an interface that > bumps use counter allows KVM to lock itself in, just as if it had a > device opened itself. Thanks, Ah, good point. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson pgpAjDK6J9rb1.pgp Description: PGP signature
Re: [PATCH 3/8] vfio: add external user support
On 06/27/2013 07:42 PM, Benjamin Herrenschmidt wrote:
> On Thu, 2013-06-27 at 16:59 +1000, Stephen Rothwell wrote:
>>> +/* Allows an external user (for example, KVM) to unlock an IOMMU
>> group */
>>> +static void vfio_group_del_external_user(struct file *filep)
>>> +{
>>> + struct vfio_group *group = filep->private_data;
>>> +
>>> + BUG_ON(filep->f_op != &vfio_group_fops);
>>
>> We usually reserve BUG_ON for situations where there is no way to
>> continue running or continuing will corrupt the running kernel. Maybe
>> WARN_ON() and return?
>
> Not even that. This is a user space provided "fd", we shouldn't oops the
> kernel because we passed a wrong argument, just return -EINVAL or
> something like that (add a return code).
I'll change to WARN_ON but...
This is going to be called on KVM exit on a file pointer previously
verified for correctness. If it is a wrong file*, then something went
terribly wrong.
--
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/8] vfio: add external user support
On Thu, 2013-06-27 at 16:59 +1000, Stephen Rothwell wrote:
> > +/* Allows an external user (for example, KVM) to unlock an IOMMU
> group */
> > +static void vfio_group_del_external_user(struct file *filep)
> > +{
> > + struct vfio_group *group = filep->private_data;
> > +
> > + BUG_ON(filep->f_op != &vfio_group_fops);
>
> We usually reserve BUG_ON for situations where there is no way to
> continue running or continuing will corrupt the running kernel. Maybe
> WARN_ON() and return?
Not even that. This is a user space provided "fd", we shouldn't oops the
kernel because we passed a wrong argument, just return -EINVAL or
something like that (add a return code).
Ben.
--
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/8] vfio: add external user support
Hi Alexy,
On Thu, 27 Jun 2013 15:02:31 +1000 Alexey Kardashevskiy wrote:
>
> +/* Allows an external user (for example, KVM) to unlock an IOMMU group */
> +static void vfio_group_del_external_user(struct file *filep)
> +{
> + struct vfio_group *group = filep->private_data;
> +
> + BUG_ON(filep->f_op != &vfio_group_fops);
We usually reserve BUG_ON for situations where there is no way to
continue running or continuing will corrupt the running kernel. Maybe
WARN_ON() and return?
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
pgpiQGEYh_pRZ.pgp
Description: PGP signature
