Re: [PATCH v2] xen: introduce privcmd device

  • From: Wei Liu <wei.liu2@xxxxxxxxxx>
  • To: Antti Kantee <pooka@xxxxxx>
  • Date: Wed, 24 Jun 2015 10:03:49 +0100

On Tue, Jun 23, 2015 at 11:20:07AM +0000, Antti Kantee wrote:

On 23/06/15 10:59, Wei Liu wrote:
The semantics of /kern/xen/privcmd is the same as NetBSD's privcmd.

Oh, one more thing I wanted to mention. We had a mini-discussion on irc
with Wei Liu about which Xen interface semantics to follow. The general
consensus was that we should follow NetBSD as closely as possible because:

a) applications don't need special "this is a rump kernel" flags and
will(/should mostly) just work
b) if we some day, for whatever yet unseen reason, change from minios to
NetBSD drivers, we don't have to introduce compat layers to keep existing
applications working.

That's just in case someone was wondering why the patch hooks to kernfs
instead of just hooking to the existing xendev infra.

+ KERNFS_ALLOCENTRY(dkt, M_TEMP, M_WAITOK);
+ /* XXX: Take VREG value from sys/vnode.h. Including vnode.h
+ * opens a can of worms. Remember to update that value if
+ * vnode.h changes.
+ */

remove that comment now?

Sure.


diff --git a/platform/xen/xen/include/mini-os/x86/os.h
b/platform/xen/xen/include/mini-os/x86/os.h
index 9aa4275..22750d8 100644
--- a/platform/xen/xen/include/mini-os/x86/os.h
+++ b/platform/xen/xen/include/mini-os/x86/os.h
@@ -434,12 +434,15 @@ static __inline__ unsigned long __ffs(unsigned long
word)
(val) = ((unsigned long)__a) | (((unsigned long)__d)<<32); \
} while(0)

-#define wrmsr(msr,val1,val2) \
+#define _wrmsr(msr,val1,val2) \
__asm__ __volatile__("wrmsr" \
: /* no outputs */ \
: "c" (msr), "a" (val1), "d" (val2))

-#define wrmsrl(msr,val)
wrmsr(msr,(uint32_t)((uint64_t)(val)),((uint64_t)(val))>>32)
+static inline void wrmsr(uint32_t msr, uint64_t val)
+{
+ _wrmsr(msr,(uint32_t)((uint64_t)(val)),((uint64_t)(val))>>32);
+}

Hmm, doesn't #ifndef __RUMP_KERNEL__ in the minios headers work here? In
fact, per code reading, this case should already be covered by existing
#ifndefs. Am I missing something?

Unfortunately it doesn't work. I'm not sure why wrmsr gets leaked.

It's easy to reproduce, just include sys/vnode.h in any xen device
(say evtdev.c) and you will see that error.

Wei.

Other related posts: