On 2021-06-05 00:11, Ricardo Koller wrote:
On Fri, Jun 04, 2021 at 09:26:54PM +0000, Sean Christopherson wrote:
On Fri, Jun 04, 2021, Ricardo Koller wrote:
> Kernel test robot reports this:
>
> > /usr/bin/ld: tools/testing/selftests/kvm/x86_64/evmcs_test.c:157: undefined reference to `vm_handle_exception'
> > /usr/bin/ld: tools/testing/selftests/kvm/x86_64/evmcs_test.c:158: undefined reference to `vm_handle_exception'
> > collect2: error: ld returned 1 exit status
>
> Fix it by renaming vm_handle_exception to vm_install_vector_handler in
> evmcs_test.c.
>
> Fixes: a2bad6a990a4 ("KVM: selftests: Rename vm_handle_exception")
Belated code review...
Thanks for the review.
Can we rename the helper to vm_install_exception_handler()?
In x86, "vector" is the number of the exception and "vectoring" is the
process
of determining the resulting vector that gets delivered to software
(e.g. when
dealing with contributory faults like #GP->#PF->#DF), but the thing
that's being
handled is an exception.
Got it. What about this renaming:
vm_handle_exception(vec) -> vm_install_exception_handler(vec)
vm_install_exception_handler(vec, ec) -> vm_install_sync_handler(vec,
ec)
arm appears to have similar terminology. And looking at the arm code,
it's very
confusing to have a helper vm_install_vector_handler() install into
exception_handlers, _not_ into vector_handlers. Calling the
vector_handlers
"default" handlers is also confusing, as "default" usually implies the
thing can
be overwritten. But in this case, the "default" handler is just
another layer
in the routing.
The multiple layers of routing is also confusing and a bit hard to
wade through
for the uninitiated. The whole thing can be made more straightfoward
by doing
away with the intermediate routing, whacking ~50 lines of code in the
process.
E.g. (definitely not functional code):
I think that works and it does remove a bunch of code. Just need to
play
with the idea and check that it can cover all cases.
For now, given that the build is broken, what about this series of
patches:
1. keep this patch to fix x86 kvm selftests
2. rename both arm and x86 to vm_install_exception_handler and
vm_install_sync_handler
3. restructure the internals of exception handling in arm
Alternatively, I can send 1+2 together and then 3. What do you think?
This is becoming a bit messy. I'd rather drop the whole series from
-next, and get something that doesn't break in the middle. Please
resend the series tested on top of -rc4.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
