On 12/21/2023 6:36 PM, Wang, Wei W wrote:
On Thursday, December 21, 2023 2:11 PM, Li, Xiaoyao wrote:
On 12/12/2023 9:56 PM, Wang, Wei W wrote:
On Wednesday, November 15, 2023 3:14 PM, Xiaoyao Li wrote:
Introduce the helper functions to set the attributes of a range of
memory to private or shared.
This is necessary to notify KVM the private/shared attribute of each gpa
range.
KVM needs the information to decide the GPA needs to be mapped at
hva- based shared memory or guest_memfd based private memory.
Signed-off-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxx>
---
accel/kvm/kvm-all.c | 42
++++++++++++++++++++++++++++++++++++++++++
include/sysemu/kvm.h | 3 +++
2 files changed, 45 insertions(+)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index
69afeb47c9c0..76e2404d54d2 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -102,6 +102,7 @@ bool kvm_has_guest_debug; static int
kvm_sstep_flags; static bool kvm_immediate_exit; static bool
kvm_guest_memfd_supported;
+static uint64_t kvm_supported_memory_attributes;
static hwaddr kvm_max_slot_size = ~0;
static const KVMCapabilityInfo kvm_required_capabilites[] = { @@
-1305,6
+1306,44 @@ void kvm_set_max_memslot_size(hwaddr max_slot_size)
kvm_max_slot_size = max_slot_size;
}
+static int kvm_set_memory_attributes(hwaddr start, hwaddr size,
+uint64_t attr) {
+ struct kvm_memory_attributes attrs;
+ int r;
+
+ attrs.attributes = attr;
+ attrs.address = start;
+ attrs.size = size;
+ attrs.flags = 0;
+
+ r = kvm_vm_ioctl(kvm_state, KVM_SET_MEMORY_ATTRIBUTES, &attrs);
+ if (r) {
+ warn_report("%s: failed to set memory (0x%lx+%#zx) with attr
+ 0x%lx
error '%s'",
+ __func__, start, size, attr, strerror(errno));
+ }
+ return r;
+}
+
+int kvm_set_memory_attributes_private(hwaddr start, hwaddr size) {
+ if (!(kvm_supported_memory_attributes &
KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
+ error_report("KVM doesn't support PRIVATE memory attribute\n");
+ return -EINVAL;
+ }
+
+ return kvm_set_memory_attributes(start, size,
+KVM_MEMORY_ATTRIBUTE_PRIVATE); }
+
+int kvm_set_memory_attributes_shared(hwaddr start, hwaddr size) {
+ if (!(kvm_supported_memory_attributes &
KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
+ error_report("KVM doesn't support PRIVATE memory attribute\n");
+ return -EINVAL;
+ }
Duplicate code in kvm_set_memory_attributes_shared/private.
Why not move the check into kvm_set_memory_attributes?
Because it's not easy to put the check into there.
Both setting and clearing one bit require the capability check. If moving the
check into kvm_set_memory_attributes(), the check of
KVM_MEMORY_ATTRIBUTE_PRIVATE will have to become unconditionally,
which is not aligned to the function name because the name is not restricted to
shared/private attribute only.
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.
attrs.attributes = attr;
attrs.address = start;
attrs.size = size;
@@ -1390,21 +1395,11 @@ static int kvm_set_memory_attributes(hwaddr start, hwaddr size, uint64_t attr)
int kvm_set_memory_attributes_private(hwaddr start, hwaddr size)
{
- if (!(kvm_supported_memory_attributes & KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
- error_report("KVM doesn't support PRIVATE memory attribute\n");
- return -EINVAL;
- }
-
return kvm_set_memory_attributes(start, size, KVM_MEMORY_ATTRIBUTE_PRIVATE);
}
int kvm_set_memory_attributes_shared(hwaddr start, hwaddr size)
{
- if (!(kvm_supported_memory_attributes & KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
- error_report("KVM doesn't support PRIVATE memory attribute\n");
- return -EINVAL;
- }
-
return kvm_set_memory_attributes(start, size, 0);
}
Maybe you don't even need the kvm_set_memory_attributes_shared/private wrappers.
