Re: [PATCH RFC 1/5] KVM: introduce a set_bit function for bitmaps in user space

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



(2010/04/12 2:08), Avi Kivity wrote:
On 04/09/2010 12:30 PM, Takuya Yoshikawa wrote:
This work is initially suggested by Avi Kivity for moving the
dirty bitmaps used by KVM to user space: This makes it possible
to manipulate the bitmaps from qemu without copying from KVM.

Note: We are now brushing up this code before sending to x86's
maintainers.


The subject prefix will need to be x86:, not KVM:, since it isn't kvm
specific, and you will need to beef up the description since you will
undoubtedly be asked why this is needed.

OK, I'll do my best. I wish somebody other than KVM people also want to
use this.


Also, please add the generic implementation (in a separate patch). We
have dirty bitmaps for ppc as well.

Yes, I've already implemented asm-generic set bit user, but need to finish
other dirty bitmap related parts for ppc to explain why it is needed.


+/**
+ * set_bit_user: - Set a bit of a bitmap in user space.
+ * @nr: Bit offset to set.
+ * @addr: Base address, in user space.
+ *
+ * Context: User context only. This function may sleep.
+ *
+ * This macro sets a bit of a bitmap in user space. Note that this
+ * is same as __set_bit but not set_bit in the sense that setting
+ * the bit is not done atomically.
+ *
+ * Returns zero on success, -EFAULT on error.
+ */
+#define __set_bit_user_asm(nr, addr, err, errret) \
+ asm volatile("1: bts %1,%2\n" \
+ "2:\n" \
+ ".section .fixup,\"ax\"\n" \
+ "3: mov %3,%0\n" \
+ " jmp 2b\n" \
+ ".previous\n" \
+ _ASM_EXTABLE(1b, 3b) \
+ : "=r"(err) \
+ : "r" (nr), "m" (__m(addr)), "i" (errret), "0" (err))
+
+#define set_bit_user(nr, addr) \
+({ \
+ int __ret_sbu = 0; \
+ \
+ might_fault(); \
+ if (access_ok(VERIFY_WRITE, addr, nr/8 + 1)) \
+ __set_bit_user_asm(nr, addr, __ret_sbu, -EFAULT); \
+ else \
+ __ret_sbu = -EFAULT; \
+ \
+ __ret_sbu; \
+})
+

Should be called __set_bit_user() since it is non-atomic.

Actually I first named it like that and then noticed that in the uaccess'
convention, __ prefix means it is "with less checking" version.

I don't know which is better in this case, should be "set_bit_user_non_atomic"
though bit long? May be judged by x86 people.


An interesting wart is that this will use the kernel's word size instead
of userspace word size for access. So, a 32-bit process might allocate a
4-byte bitmap, and a 64-bit kernel will use a 64-bit access to touch it,
which might result in a fault. This might be resolved by documenting
that userspace bitmaps must be a multiple of 64-bits in size and
recommending that they be 64-bit aligned as well.

Oh, I didn't care about that. I'll explain in the next version.


Can you replace the macros with inline functions?


Yes, I can. I have both inline and macro versions and talking with Fernando
which will be preferred.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux