Re: [PATCH 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
Alex Williamson alex.william...@redhat.com writes:
On Mon, 2013-06-17 at 13:56 +1000, Benjamin Herrenschmidt wrote:
On Sun, 2013-06-16 at 21:13 -0600, Alex Williamson wrote:
IOMMU groups themselves don't provide security, they're accessed by
interfaces like VFIO, which provide the security. Given a brief look, I
agree, this looks like a possible backdoor. The typical VFIO way to
handle this would be to pass a VFIO file descriptor here to prove that
the process has access to the IOMMU group. This is how /dev/vfio/vfio
gains the ability to setup an IOMMU domain an do mappings with the
SET_CONTAINER ioctl using a group fd. Thanks,
How do you envision that in the kernel ? IE. I'm in KVM code, gets that
vfio fd, what do I do with it ?
Basically, KVM needs to know that the user is allowed to use that iommu
group. I don't think we want KVM however to call into VFIO directly
right ?
Right, we don't want to create dependencies across modules. I don't
have a vision for how this should work. This is effectively a complete
side-band to vfio, so we're really just dealing in the iommu group
space. Maybe there needs to be some kind of registration of ownership
for the group using some kind of token. It would need to include some
kind of notification when that ownership ends. That might also be a
convenient tag to toggle driver probing off for devices in the group.
Other ideas? Thanks,
It's actually not that bad.
eg.
struct vfio_container *vfio_container_from_file(struct file *filp)
{
if (filp-f_op != vfio_device_fops)
return ERR_PTR(-EINVAL);
/* OK it really is a vfio fd, return the data. */
}
EXPORT_SYMBOL_GPL(vfio_container_from_file);
...
inside KVM_CREATE_SPAPR_TCE_IOMMU:
struct file *vfio_filp;
struct vfio_container *(lookup)(struct file *filp);
vfio_filp = fget(create_tce_iommu.fd);
if (!vfio)
ret = -EBADF;
lookup = symbol_get(vfio_container_from_file);
if (!lookup)
ret = -EINVAL;
else {
container = lookup(vfio_filp);
if (IS_ERR(container))
ret = PTR_ERR(container);
else
...
symbol_put(vfio_container_from_file);
}
symbol_get() won't try to load a module; it'll just fail. This is what
you want, since they must have vfio in the kernel to get a valid fd...
Hope that helps,
Rusty.
--
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] BUILD_BUG_ON: make it handle more cases
On Thu, 5 Nov 2009 05:08:42 pm Stephen Rothwell wrote: Hi Rusty, On Thu, 5 Nov 2009 16:58:36 +1030 Rusty Russell ru...@rustcorp.com.au wrote: Huh? virtio_has_feature does: if (__builtin_constant_p(fbit)) BUILD_BUG_ON(fbit = 32); else BUG_ON(fbit = 32); In Linus' tree (and linux-next) it looks like this: Ah. My patch series fixes that as part of removing MAYBE_BUILD_BUG_ON. I've put both in for linux-next. Cheers, Rusty. -- 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: linux-next: tree build failure
On Tue, 20 Oct 2009 04:49:29 am Hollis Blanchard wrote: On Thu, 2009-10-15 at 08:27 +0100, Jan Beulich wrote: My perspective is that it just uncovered already existing brokenness. Sorry, I thought it was clear, but to be more explicit: I propose the following patch, which replaces the current BUILD_BUG_ON implementation with Rusty's version. OK, I switched my brain back on. Yeah, I agree: we may still want BUILD_OR_RUNTIME_BUG_ON one day, but I like this. It's just missing the giant comment that it needs :) /** * BUILD_BUG_ON - break compile if a condition is true. * @cond: the condition which the compiler should know is false. * * If you have some code which relies on certain constants being equal, or * other compile-time-evaluated condition, you should use BUILD_BUG_ON to * detect if someone changes it. * * The implementation uses gcc's reluctance to create a negative array, but * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments * to inline functions). So as a fallback we use the optimizer; if it can't * prove the condition is false, it will cause a link error on the undefined * __build_bug_on_failed. This error is less neat, and can be harder to * track down. */ Thanks! Rusty. -- 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] BUILD_BUG_ON: make it handle more cases
BUILD_BUG_ON used to use the optimizer to do code elimination or fail
at link time; it was changed to first the size of a negative array (a
nicer compile time error), then (in
8c87df457cb58fe75b9b893007917cf8095660a0) to a bitfield.
bitfields: needs a literal constant at parse time, and can't be put under
if (__builtin_constant_p(x)) for example.
negative array: can handle anything, but if the compiler can't tell it's
a constant, silently has no effect.
link time: breaks link if the compiler can't determine the value, but the
linker output is not usually as informative as a compiler error.
If we use the negative-array-size method *and* the link time trick,
we get the ability to use BUILD_BUG_ON() under __builtin_constant_p()
branches, and maximal ability for the compiler to detect errors at
build time.
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -683,12 +683,6 @@ struct sysinfo {
char _f[20-2*sizeof(long)-sizeof(int)]; /* Padding: libc5 uses this.. */
};
-/* Force a compilation error if condition is true */
-#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
-
-/* Force a compilation error if condition is constant and true */
-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
-
/* Force a compilation error if condition is true, but also produce a
result (of value 0 and type size_t), so the expression can be used
e.g. in a structure initializer (or where-ever else comma expressions
@@ -696,6 +690,33 @@ struct sysinfo {
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
#define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
+/**
+ * BUILD_BUG_ON - break compile if a condition is true.
+ * @cond: the condition which the compiler should know is false.
+ *
+ * If you have some code which relies on certain constants being equal, or
+ * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
+ * detect if someone changes it.
+ *
+ * The implementation uses gcc's reluctance to create a negative array, but
+ * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
+ * to inline functions). So as a fallback we use the optimizer; if it can't
+ * prove the condition is false, it will cause a link error on the undefined
+ * __build_bug_on_failed. This error message can be harder to track down
+ * though, hence the two different methods.
+ */
+#ifndef __OPTIMIZE__
+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#else
+extern int __build_bug_on_failed;
+#define BUILD_BUG_ON(condition)\
+ do {\
+ ((void)sizeof(char[1 - 2*!!(condition)])); \
+ if (condition) __build_bug_on_failed = 1; \
+ } while(0)
+#endif
+#define MAYBE_BUILD_BUG_ON(condition) BUILD_BUG_ON(condition)
+
/* Trap pasters of __FUNCTION__ at compile-time */
#define __FUNCTION__ (__func__)
--
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] [v2] linux: virtio: Standardize virtio's concept of page size
On Friday 14 November 2008 08:18:33 Hollis Blanchard wrote:
On Thu, 2008-11-13 at 08:44 +1030, Rusty Russell wrote:
Note that I still don't have a balloon patch: want to send me one?
linux: virtio-balloon: avoid implicit use of Linux page size in balloon
interface
Thanks, applied with following diff:
Use tabs to indent, and put BUILD_BUG_ON pagesize assumption.
Signed-off-by: Rusty Russell [EMAIL PROTECTED]
diff -r 50e970613233 drivers/virtio/virtio_balloon.c
--- a/drivers/virtio/virtio_balloon.c Fri Nov 14 11:40:38 2008 +1030
+++ b/drivers/virtio/virtio_balloon.c Fri Nov 14 11:41:19 2008 +1030
@@ -58,10 +58,11 @@
static u32 page_to_balloon_pfn(struct page *page)
{
-unsigned long pfn = page_to_pfn(page);
+ unsigned long pfn = page_to_pfn(page);
-/* Convert pfn from Linux page size to balloon page size. */
-return pfn (PAGE_SHIFT - VIRTIO_BALLOON_PFN_SHIFT);
+ BUILD_BUG_ON(PAGE_SHIFT VIRTIO_BALLOON_PFN_SHIFT);
+ /* Convert pfn from Linux page size to balloon page size. */
+ return pfn (PAGE_SHIFT - VIRTIO_BALLOON_PFN_SHIFT);
}
static void balloon_ack(struct virtqueue *vq)
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [v2] linux: virtio: Standardize virtio's concept of page size
On Thursday 13 November 2008 02:46:31 Hollis Blanchard wrote: On Wed, 2008-11-12 at 22:51 +1030, Rusty Russell wrote: On Tuesday 11 November 2008 10:07:09 Hollis Blanchard wrote: Both sides of the virtio interface must agree about how big a pfn really is. This is particularly an issue on architectures where the page size is configurable (e.g. PowerPC, IA64) -- the interface must be independent of PAGE_SHIFT. Currently there are three distinct problems: * The shift count used when passing the physical address of the ring to a PCI-based back end. * The ring layout itself is padded to span at least two pages. * The balloon driver operates in units of pages. Hi Hollis, The more I thought about this, the more I think we're not solving this as neatly as we could. The trigger was noting that we're breaking the userspace API (vring_size and vring_init are exposed to userspace): I know that qemu cut pastes, but that's no excuse. So instead, I've introduced separate constants for each use. Yes, all these constants are 12/4096. But just to be contrary, at the end is a patch to change lguest to 128. And there's no reason this couldn't change in future using some guest detection scheme. OK. I thought it was simpler to just say 4KB everywhere in all aspects of the virtio interface, but I'm happy as long as we solve the problem somehow. :) It is simpler, yes, but we can take this opportunity to deconflate them and make things clearer and better than the current code, not just fix it. Note that I still don't have a balloon patch: want to send me one? Thanks, Rusty. -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
