On Wed, Aug 03, 2022, Chao Peng wrote:
On Fri, Jul 29, 2022 at 07:51:29PM +0000, Sean Christopherson wrote:
On Wed, Jul 06, 2022, Chao Peng wrote:
@@ -1332,9 +1332,18 @@ yet and must be cleared on entry. __u64 userspace_addr; /* start of the userspace allocated memory */ };
- struct kvm_userspace_memory_region_ext {
- struct kvm_userspace_memory_region region;
- __u64 private_offset;
- __u32 private_fd;
- __u32 pad1;
- __u64 pad2[14];
+};
- /* for kvm_memory_region::flags */ #define KVM_MEM_LOG_DIRTY_PAGES (1UL << 0) #define KVM_MEM_READONLY (1UL << 1)
- #define KVM_MEM_PRIVATE (1UL << 2)
Very belatedly following up on prior feedback...
| I think a flag is still needed, the problem is private_fd can be safely | accessed only when this flag is set, e.g. without this flag, we can't | copy_from_user these new fields since they don't exist for previous | kvm_userspace_memory_region callers.
I forgot about that aspect of things. We don't technically need a dedicated PRIVATE flag to handle that, but it does seem to be the least awful soltuion. We could either add a generic KVM_MEM_EXTENDED_REGION or an entirely new ioctl(), e.g. KVM_SET_USER_MEMORY_REGION2, but in both approaches there's a decent chance that we'll end up needed individual "this field is valid" flags anways.
E.g. if KVM requires pad1 and pad2 to be zero to carve out future extensions, then we're right back here if some future extension needs to treat '0' as a legal input.
I had such practice (always rejecting none-zero 'pad' value when introducing new user APIs) in other project previously, but I rarely see that in KVM.
Ya, KVM often uses flags to indicate the validity of a field specifically so that KVM doesn't misinterpret a '0' from an older userspace as an intended value.