| Message ID | 20210929001012.2539461-1-ricarkol@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: vgic: Missing checks for REDIST/CPU and ITS regions above the VM IPA size | expand |
Hi Ricardo, On 9/29/21 2:10 AM, Ricardo Koller wrote: > Add some ITS device tests: general KVM device tests (address not defined > already, address aligned) and tests for the ITS region being within the > addressable IPA range. > > Signed-off-by: Ricardo Koller <ricarkol@google.com> > --- > .../testing/selftests/kvm/aarch64/vgic_init.c | 42 +++++++++++++++++++ > 1 file changed, 42 insertions(+) > > diff --git a/tools/testing/selftests/kvm/aarch64/vgic_init.c b/tools/testing/selftests/kvm/aarch64/vgic_init.c > index 417a9a515cad..180221ec325d 100644 > --- a/tools/testing/selftests/kvm/aarch64/vgic_init.c > +++ b/tools/testing/selftests/kvm/aarch64/vgic_init.c > @@ -603,6 +603,47 @@ static void test_v3_redist_ipa_range_check_at_vcpu_run(void) > vm_gic_destroy(&v); > } > > +static void test_v3_its_region(void) > +{ > + struct vm_gic v; > + uint64_t addr; > + int its_fd, ret; > + > + v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS); > + its_fd = kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_ITS, false); this may fail if the ITS device has not been registered by KVM (host GICv2) Maybe refine the patch title mentionning this is an ITS device "init" test. as per Documentation/virt/kvm/devices/arm-vgic-its.rst we could also try instantiating the ITS before the GIC and try instantiating several ITSs with overlapping addresses. But I would totally understand if you consider this out of the scope of your fixes + tests. Thanks! Eric > + > + addr = 0x401000; > + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > + TEST_ASSERT(ret && errno == EINVAL, > + "ITS region with misaligned address"); > + > + addr = max_phys_size; > + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > + TEST_ASSERT(ret && errno == E2BIG, > + "register ITS region with base address beyond IPA range"); > + > + addr = max_phys_size - 0x10000; > + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > + TEST_ASSERT(ret && errno == E2BIG, > + "Half of ITS region is beyond IPA range"); > + > + /* This one succeeds setting the ITS base */ > + addr = 0x400000; > + kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > + > + addr = 0x300000; > + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > + TEST_ASSERT(ret && errno == EEXIST, "ITS base set again"); > + > + close(its_fd); > + vm_gic_destroy(&v); > +} > + > /* > * Returns 0 if it's possible to create GIC device of a given type (V2 or V3). > */ > @@ -655,6 +696,7 @@ void run_tests(uint32_t gic_dev_type) > test_v3_last_bit_redist_regions(); > test_v3_last_bit_single_rdist(); > test_v3_redist_ipa_range_check_at_vcpu_run(); > + test_v3_its_region(); > } > } >
Hi Eric, On Thu, Sep 30, 2021 at 11:14:02AM +0200, Eric Auger wrote: > Hi Ricardo, > > On 9/29/21 2:10 AM, Ricardo Koller wrote: > > Add some ITS device tests: general KVM device tests (address not defined > > already, address aligned) and tests for the ITS region being within the > > addressable IPA range. > > > > Signed-off-by: Ricardo Koller <ricarkol@google.com> > > --- > > .../testing/selftests/kvm/aarch64/vgic_init.c | 42 +++++++++++++++++++ > > 1 file changed, 42 insertions(+) > > > > diff --git a/tools/testing/selftests/kvm/aarch64/vgic_init.c b/tools/testing/selftests/kvm/aarch64/vgic_init.c > > index 417a9a515cad..180221ec325d 100644 > > --- a/tools/testing/selftests/kvm/aarch64/vgic_init.c > > +++ b/tools/testing/selftests/kvm/aarch64/vgic_init.c > > @@ -603,6 +603,47 @@ static void test_v3_redist_ipa_range_check_at_vcpu_run(void) > > vm_gic_destroy(&v); > > } > > > > +static void test_v3_its_region(void) > > +{ > > + struct vm_gic v; > > + uint64_t addr; > > + int its_fd, ret; > > + > > + v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS); > > + its_fd = kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_ITS, false); > this may fail if the ITS device has not been registered by KVM (host GICv2) At the moment it's just called in the GICv3 case. It seems that registering a GICv3 device results in having an ITS registered as well (from kvm_register_vgic_device()). I'm assuming this won't change; we might as well check that assumption. What do you think? Thanks, Ricardo > > Maybe refine the patch title mentionning this is an ITS device "init" test. > as per Documentation/virt/kvm/devices/arm-vgic-its.rst we could also try > instantiating the ITS before the GIC and try instantiating several ITSs > with overlapping addresses. > But I would totally understand if you consider this out of the scope of > your fixes + tests. > > Thanks! > > Eric > > + > > + addr = 0x401000; > > + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > > + TEST_ASSERT(ret && errno == EINVAL, > > + "ITS region with misaligned address"); > > + > > + addr = max_phys_size; > > + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > > + TEST_ASSERT(ret && errno == E2BIG, > > + "register ITS region with base address beyond IPA range"); > > + > > + addr = max_phys_size - 0x10000; > > + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > > + TEST_ASSERT(ret && errno == E2BIG, > > + "Half of ITS region is beyond IPA range"); > > + > > + /* This one succeeds setting the ITS base */ > > + addr = 0x400000; > > + kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > > + > > + addr = 0x300000; > > + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > > + TEST_ASSERT(ret && errno == EEXIST, "ITS base set again"); > > + > > + close(its_fd); > > + vm_gic_destroy(&v); > > +} > > + > > /* > > * Returns 0 if it's possible to create GIC device of a given type (V2 or V3). > > */ > > @@ -655,6 +696,7 @@ void run_tests(uint32_t gic_dev_type) > > test_v3_last_bit_redist_regions(); > > test_v3_last_bit_single_rdist(); > > test_v3_redist_ipa_range_check_at_vcpu_run(); > > + test_v3_its_region(); > > } > > } > > >
On Thu, Sep 30, 2021 at 11:14:02AM +0200, Eric Auger wrote: > Hi Ricardo, > > On 9/29/21 2:10 AM, Ricardo Koller wrote: > > Add some ITS device tests: general KVM device tests (address not defined > > already, address aligned) and tests for the ITS region being within the > > addressable IPA range. > > > > Signed-off-by: Ricardo Koller <ricarkol@google.com> > > --- > > .../testing/selftests/kvm/aarch64/vgic_init.c | 42 +++++++++++++++++++ > > 1 file changed, 42 insertions(+) > > > > diff --git a/tools/testing/selftests/kvm/aarch64/vgic_init.c b/tools/testing/selftests/kvm/aarch64/vgic_init.c > > index 417a9a515cad..180221ec325d 100644 > > --- a/tools/testing/selftests/kvm/aarch64/vgic_init.c > > +++ b/tools/testing/selftests/kvm/aarch64/vgic_init.c > > @@ -603,6 +603,47 @@ static void test_v3_redist_ipa_range_check_at_vcpu_run(void) > > vm_gic_destroy(&v); > > } > > > > +static void test_v3_its_region(void) > > +{ > > + struct vm_gic v; > > + uint64_t addr; > > + int its_fd, ret; > > + > > + v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS); > > + its_fd = kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_ITS, false); > this may fail if the ITS device has not been registered by KVM (host GICv2) > > Maybe refine the patch title mentionning this is an ITS device "init" test. > as per Documentation/virt/kvm/devices/arm-vgic-its.rst we could also try > instantiating the ITS before the GIC and try instantiating several ITSs > with overlapping addresses. > But I would totally understand if you consider this out of the scope of > your fixes + tests. Will just send a v4 with init tests for now. ACK on changing the patch title. Thanks, Ricardo > > Thanks! > > Eric > > + > > + addr = 0x401000; > > + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > > + TEST_ASSERT(ret && errno == EINVAL, > > + "ITS region with misaligned address"); > > + > > + addr = max_phys_size; > > + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > > + TEST_ASSERT(ret && errno == E2BIG, > > + "register ITS region with base address beyond IPA range"); > > + > > + addr = max_phys_size - 0x10000; > > + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > > + TEST_ASSERT(ret && errno == E2BIG, > > + "Half of ITS region is beyond IPA range"); > > + > > + /* This one succeeds setting the ITS base */ > > + addr = 0x400000; > > + kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > > + > > + addr = 0x300000; > > + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > > + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); > > + TEST_ASSERT(ret && errno == EEXIST, "ITS base set again"); > > + > > + close(its_fd); > > + vm_gic_destroy(&v); > > +} > > + > > /* > > * Returns 0 if it's possible to create GIC device of a given type (V2 or V3). > > */ > > @@ -655,6 +696,7 @@ void run_tests(uint32_t gic_dev_type) > > test_v3_last_bit_redist_regions(); > > test_v3_last_bit_single_rdist(); > > test_v3_redist_ipa_range_check_at_vcpu_run(); > > + test_v3_its_region(); > > } > > } > > >
Hi Ricardo, On 9/30/21 10:10 PM, Ricardo Koller wrote: > Hi Eric, > > On Thu, Sep 30, 2021 at 11:14:02AM +0200, Eric Auger wrote: >> Hi Ricardo, >> >> On 9/29/21 2:10 AM, Ricardo Koller wrote: >>> Add some ITS device tests: general KVM device tests (address not defined >>> already, address aligned) and tests for the ITS region being within the >>> addressable IPA range. >>> >>> Signed-off-by: Ricardo Koller <ricarkol@google.com> >>> --- >>> .../testing/selftests/kvm/aarch64/vgic_init.c | 42 +++++++++++++++++++ >>> 1 file changed, 42 insertions(+) >>> >>> diff --git a/tools/testing/selftests/kvm/aarch64/vgic_init.c b/tools/testing/selftests/kvm/aarch64/vgic_init.c >>> index 417a9a515cad..180221ec325d 100644 >>> --- a/tools/testing/selftests/kvm/aarch64/vgic_init.c >>> +++ b/tools/testing/selftests/kvm/aarch64/vgic_init.c >>> @@ -603,6 +603,47 @@ static void test_v3_redist_ipa_range_check_at_vcpu_run(void) >>> vm_gic_destroy(&v); >>> } >>> >>> +static void test_v3_its_region(void) >>> +{ >>> + struct vm_gic v; >>> + uint64_t addr; >>> + int its_fd, ret; >>> + >>> + v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS); >>> + its_fd = kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_ITS, false); >> this may fail if the ITS device has not been registered by KVM (host GICv2) > At the moment it's just called in the GICv3 case. It seems that OK I missed that. in that case that's fine. Thanks Eric > registering a GICv3 device results in having an ITS registered as well > (from kvm_register_vgic_device()). I'm assuming this won't change; > we might as well check that assumption. What do you think? > > Thanks, > Ricardo > >> Maybe refine the patch title mentionning this is an ITS device "init" test. >> as per Documentation/virt/kvm/devices/arm-vgic-its.rst we could also try >> instantiating the ITS before the GIC and try instantiating several ITSs >> with overlapping addresses. >> But I would totally understand if you consider this out of the scope of >> your fixes + tests. >> >> Thanks! >> >> Eric >>> + >>> + addr = 0x401000; >>> + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, >>> + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); >>> + TEST_ASSERT(ret && errno == EINVAL, >>> + "ITS region with misaligned address"); >>> + >>> + addr = max_phys_size; >>> + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, >>> + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); >>> + TEST_ASSERT(ret && errno == E2BIG, >>> + "register ITS region with base address beyond IPA range"); >>> + >>> + addr = max_phys_size - 0x10000; >>> + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, >>> + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); >>> + TEST_ASSERT(ret && errno == E2BIG, >>> + "Half of ITS region is beyond IPA range"); >>> + >>> + /* This one succeeds setting the ITS base */ >>> + addr = 0x400000; >>> + kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, >>> + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); >>> + >>> + addr = 0x300000; >>> + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, >>> + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); >>> + TEST_ASSERT(ret && errno == EEXIST, "ITS base set again"); >>> + >>> + close(its_fd); >>> + vm_gic_destroy(&v); >>> +} >>> + >>> /* >>> * Returns 0 if it's possible to create GIC device of a given type (V2 or V3). >>> */ >>> @@ -655,6 +696,7 @@ void run_tests(uint32_t gic_dev_type) >>> test_v3_last_bit_redist_regions(); >>> test_v3_last_bit_single_rdist(); >>> test_v3_redist_ipa_range_check_at_vcpu_run(); >>> + test_v3_its_region(); >>> } >>> } >>>
diff --git a/tools/testing/selftests/kvm/aarch64/vgic_init.c b/tools/testing/selftests/kvm/aarch64/vgic_init.c index 417a9a515cad..180221ec325d 100644 --- a/tools/testing/selftests/kvm/aarch64/vgic_init.c +++ b/tools/testing/selftests/kvm/aarch64/vgic_init.c @@ -603,6 +603,47 @@ static void test_v3_redist_ipa_range_check_at_vcpu_run(void) vm_gic_destroy(&v); } +static void test_v3_its_region(void) +{ + struct vm_gic v; + uint64_t addr; + int its_fd, ret; + + v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS); + its_fd = kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_ITS, false); + + addr = 0x401000; + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); + TEST_ASSERT(ret && errno == EINVAL, + "ITS region with misaligned address"); + + addr = max_phys_size; + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); + TEST_ASSERT(ret && errno == E2BIG, + "register ITS region with base address beyond IPA range"); + + addr = max_phys_size - 0x10000; + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); + TEST_ASSERT(ret && errno == E2BIG, + "Half of ITS region is beyond IPA range"); + + /* This one succeeds setting the ITS base */ + addr = 0x400000; + kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); + + addr = 0x300000; + ret = _kvm_device_access(its_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, + KVM_VGIC_ITS_ADDR_TYPE, &addr, true); + TEST_ASSERT(ret && errno == EEXIST, "ITS base set again"); + + close(its_fd); + vm_gic_destroy(&v); +} + /* * Returns 0 if it's possible to create GIC device of a given type (V2 or V3). */ @@ -655,6 +696,7 @@ void run_tests(uint32_t gic_dev_type) test_v3_last_bit_redist_regions(); test_v3_last_bit_single_rdist(); test_v3_redist_ipa_range_check_at_vcpu_run(); + test_v3_its_region(); } }
Add some ITS device tests: general KVM device tests (address not defined already, address aligned) and tests for the ITS region being within the addressable IPA range. Signed-off-by: Ricardo Koller <ricarkol@google.com> --- .../testing/selftests/kvm/aarch64/vgic_init.c | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+)