RE: [PATCH v3 06/70] kvm: Introduce support for memory_attributes

Wang, Wei W
2024-01-09T14:53:21Z

On Tuesday, January 9, 2024 1:47 PM, Li, Xiaoyao wrote:
> 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.

What's the exact issue?
+#define KVM_MEMORY_ATTRIBUTE_READ (1ULL << 2)
+#define KVM_MEMORY_ATTRIBUTE_WRITE (1ULL << 1)
+#define KVM_MEMORY_ATTRIBUTE_EXE (1ULL << 0)

They are checked via
"if ((attr & kvm_supported_memory_attributes) != attr)" shared above in
kvm_set_memory_attributes.
In the case you described, kvm_supported_memory_attributes will be 0x7.
Anything unexpected?