Re: [PATCH] KVM: selftests: Rename vm_handle_exception in evmcs test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux