Date: Thu, 31 May 2012 17:07:42 -0600 From: Zbigniew Bodek <zbb@semihalf.com> To: Marcel Moolenaar <marcel@xcllnt.net> Cc: powerpc@freebsd.org, =?UTF-8?Q?Piotr_Zi=C4=99cik?= <kosmo@semihalf.com> Subject: Re: RFC: OpenPIC IPI patch Message-ID: <fcdca283083e354eb4d9733a572a1041@smtp.semihalf.com> In-Reply-To: <251AF144-587C-4854-88B2-0CD7D26E1DF1@xcllnt.net> References: <0362C399-CB54-451E-A879-E836EF13CE72@semihalf.com> <251AF144-587C-4854-88B2-0CD7D26E1DF1@xcllnt.net>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --] On Wed, 30 May 2012 13:36:46 -0700, Marcel Moolenaar <marcel@xcllnt.net> wrote: > On May 30, 2012, at 1:13 PM, Rafal Jaworowski wrote: > >> Can you please have a look at this patch and let us know about any >> comments / objections? We identified a problem with IPI on the recent FSL >> eOpenPIC, description in the patch: >> >> http://people.freebsd.org/~raj/patches/powerpc/openpic.diff > > Looks good. Please consider adding some checks to openpic_ipi() where > we peek into the cpuset_t type and access the "bits". An assert would > be nice if the set contains cpus number 32 or up. This to make it > painfully obvious that it's time to extend openpic_ipi() to handle more > than 32 CPUs if and when the need arises. > > FYI, Indeed, there should be a sanity check in here. I suggest the following assert: diff --git a/sys/powerpc/powerpc/openpic.c b/sys/powerpc/powerpc/openpic.c index 95af605..45858b6 100644 --- a/sys/powerpc/powerpc/openpic.c +++ b/sys/powerpc/powerpc/openpic.c @@ -29,6 +29,7 @@ #include <sys/systm.h> #include <sys/bus.h> #include <sys/conf.h> +#include <sys/cpuset.h> #include <sys/kernel.h> #include <sys/proc.h> #include <sys/rman.h> @@ -344,8 +345,14 @@ void openpic_ipi(device_t dev, cpuset_t cpumask) { struct openpic_softc *sc; + cpuset_t ns_cpus; /* Mask of not supported CPUs */ + + CPU_FILL(&ns_cpus); + ns_cpus.__bits[0] = 0; KASSERT(dev == root_pic, ("Cannot send IPIs from non-root OpenPIC")); + KASSERT(CPU_OVERLAP(&ns_cpus, &cpumask) == 0, + ("Cannot send an IPI to a CPU which number exceeds #31")); sc = device_get_softc(dev); sched_pin(); The patch is also available in the email's attachment. Please send any comments and/or objections. Best regards Zbyszek Bodek [-- Attachment #2 --] diff --git a/sys/powerpc/powerpc/openpic.c b/sys/powerpc/powerpc/openpic.c index 95af605..45858b6 100644 --- a/sys/powerpc/powerpc/openpic.c +++ b/sys/powerpc/powerpc/openpic.c @@ -29,6 +29,7 @@ #include <sys/systm.h> #include <sys/bus.h> #include <sys/conf.h> +#include <sys/cpuset.h> #include <sys/kernel.h> #include <sys/proc.h> #include <sys/rman.h> @@ -344,8 +345,14 @@ void openpic_ipi(device_t dev, cpuset_t cpumask) { struct openpic_softc *sc; + cpuset_t ns_cpus; /* Mask of not supported CPUs */ + + CPU_FILL(&ns_cpus); + ns_cpus.__bits[0] = 0; KASSERT(dev == root_pic, ("Cannot send IPIs from non-root OpenPIC")); + KASSERT(CPU_OVERLAP(&ns_cpus, &cpumask) == 0, + ("Cannot send an IPI to a CPU which number exceeds #31")); sc = device_get_softc(dev); sched_pin();help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?fcdca283083e354eb4d9733a572a1041>