On 12/21/2023 9:47 PM, Wang, Wei W wrote:
On Thursday, December 21, 2023 7:54 PM, Li, Xiaoyao wrote:
On 12/21/2023 6:36 PM, Wang, Wei W wrote:
No need to specifically check for KVM_MEMORY_ATTRIBUTE_PRIVATE there.
I'm suggesting below:
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index
2d9a2455de..63ba74b221 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1375,6 +1375,11 @@ static int kvm_set_memory_attributes(hwaddr
start, hwaddr size, uint64_t attr)
struct kvm_memory_attributes attrs;
int r;
+ if ((attr & kvm_supported_memory_attributes) != attr) {
+ error_report("KVM doesn't support memory attr %lx\n", attr);
+ return -EINVAL;
+ }
In the case of setting a range of memory to shared while KVM doesn't support
private memory. Above check doesn't work. and following IOCTL fails.
SHARED attribute uses the value 0, which indicates it's always supported, no?
For the implementation, can you find in the KVM side where the ioctl
would get failed in that case?
I'm worrying about the future case, that KVM supports other memory
attribute than shared/private. For example, KVM supports RWX bits (bit 0
- 2) but not shared/private bit.
This patch designs kvm_set_memory_attributes() to be common for all the
bits (and for future bits), thus it leaves the support check to each
caller function separately.
If you think it's unnecessary, I can change the name of
kvm_set_memory_attributes() to kvm_set_memory_shared_private() to make
it only for shared/private bit, then the check can be moved to it.
static int kvm_vm_ioctl_set_mem_attributes(struct kvm *kvm,
struct kvm_memory_attributes *attrs)
{
gfn_t start, end;
/* flags is currently not used. */
if (attrs->flags)
return -EINVAL;
if (attrs->attributes & ~kvm_supported_mem_attributes(kvm)) ==> 0 here
return -EINVAL;
if (attrs->size == 0 || attrs->address + attrs->size < attrs->address)
return -EINVAL;
if (!PAGE_ALIGNED(attrs->address) || !PAGE_ALIGNED(attrs->size))
return -EINVAL;
