Re: [PATCH 04/14] KVM: PPC: e500: MMU API

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

 



On 11/01/2011 05:16 PM, Scott Wood wrote:
On 11/01/2011 03:58 AM, Avi Kivity wrote:
On 10/31/2011 10:12 PM, Scott Wood wrote:
+4.59 KVM_DIRTY_TLB
+
+Capability: KVM_CAP_SW_TLB
+Architectures: ppc
+Type: vcpu ioctl
+Parameters: struct kvm_dirty_tlb (in)
+Returns: 0 on success, -1 on error
+
+struct kvm_dirty_tlb {
+	__u64 bitmap;
+	__u32 num_dirty;
+};
This is not 32/64 bit safe.  e500 is 32-bit only, yes?
e5500 is 64-bit -- we don't support it with KVM yet, but it's planned.

but what if someone wants to emulate an e500 on a ppc64?  maybe it's better to add
padding here.
What is unsafe about it?  Are you picturing TLBs with more than 4
billion entries?
sizeof(struct kvm_tlb_dirty) == 12 for 32-bit userspace, but ==  16 for
64-bit userspace and the kernel.  ABI structures must have the same
alignment and size for 32/64 bit userspace, or they need compat handling.
The size is 16 on 32-bit ppc -- the alignment of __u64 forces this.  It
looks like this is different in the 32x86 ABI.

We can pad explicitly if you prefer.

I would prefer if we keep this stable :). There's no good reason to pad it - ppc64 creates the same struct definition.

There shouldn't be any alignment issues.

Another alternative is to drop the num_dirty field (and let the kernel
compute it instead, shouldn't take long?), and have the third argument
to ioctl() reference the bitmap directly.
The idea was to make it possible for the kernel to apply a threshold
above which it would be better to ignore the bitmap entirely and flush
everything:

http://www.spinics.net/lists/kvm/msg50079.html

Currently we always just flush everything, and QEMU always says
everything is dirty when it makes a change, but the API is there if needed.
Right, but you don't need num_dirty for it.  There are typically only a
few dozen entries, yes?  It should take a trivial amount of time to
calculate its weight.
There are over 500 entries currently, and QEMU could make it much larger
if it wants to decrease guest-visible faults on certain workloads.

It's not the most important feature, indeed we currently ignore the
bitmap entirely.  But it could be useful depending on how the API is
used in the future, and I don't think we gain much by dropping it at
this point.  Alex, any thoughts?

The kernel can always opt in to ignore the field if it chooses to, so I don't see the point in dropping it. There shouldn't be an alignment problem in the first place :).


Alex

--
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