Re: [kvm-devel] [PATCH 3/3] KVM: Expose MSRs to userspace

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

 



Arnd Bergmann wrote:
On Thursday 16 November 2006 19:04, Avi Kivity wrote:
+struct kvm_msr_entry {
+       __u32 index;
+       __u32 reserved;
+       __u64 data;
+};
+
+/* for KVM_GET_MSRS and KVM_SET_MSRS */
+struct kvm_msrs {
+       __u32 vcpu;
+       __u32 nmsrs; /* number of msrs in entries */
+
+       union {
+               struct kvm_msr_entry __user *entries;
+               __u64 padding;
+       };
+};

ioctl interfaces with pointers in them are generally a bad idea,
though you handle most of the points against them fine here
(endianess doesn't matter, padding is correct).

Still, it might be better not to set a bad example. Is accessing
the MSRs actually performance critical? If not, you could
define the ioctl to take only a single entry argument.


But then you can't dynamically determine which MSRs are available.

And no, reading/setting MSRs isn't performance critical for the current use cases.

A possible alternative could also be to have a variable length
argument like below, but that creates other problems:

+struct kvm_msrs {
+       __u32 vcpu;
+       __u32 nmsrs; /* number of msrs in entries */
+       struct kvm_msr_entry entries[0]; /* followed by actual msrs */
+};

This would mean that you can't tell the transfer size from the
ioctl number, but you can't do that in your code either, because
you do two separate transfers.


Heh. That was the original implementation by Uri. I felt that was wrong because _IOW() encodes the size in the ioctl number, bit the actual size is different.


	Arnd <><


--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux