On Fri, Aug 20, 2021 at 2:29 AM Marc Zyngier <[email protected]> wrote: > > On Thu, 19 Aug 2021 23:34:06 +0100, > Raghavendra Rao Ananta <[email protected]> wrote: > > > > Potentially, the guests could trigger a debug exception that's > > outside the exception class range. > > How? All the exception classes that lead to this functions are already > handled in the switch/case statement. > I guess I didn't think this through. Landing into kvm_handle_guest_debug() itself is not possible :)
> > This could lead to an excessive syslog flooding. Hence, ratelimit > > the error message. > > > > Signed-off-by: Raghavendra Rao Ananta <[email protected]> > > --- > > arch/arm64/kvm/handle_exit.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c > > index 04ebab299aa4..c7cec7ffe93c 100644 > > --- a/arch/arm64/kvm/handle_exit.c > > +++ b/arch/arm64/kvm/handle_exit.c > > @@ -134,7 +134,7 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu) > > case ESR_ELx_EC_BRK64: > > break; > > default: > > - kvm_err("%s: un-handled case esr: %#08x\n", > > + kvm_pr_unimpl("%s: un-handled case esr: %#08x\n", > > __func__, (unsigned int) esr); > > ret = -1; > > break; > > My take on this is that this code isn't reachable, and that it could > be better rewritten as: > > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c > index 6f48336b1d86..ae7ec086827b 100644 > --- a/arch/arm64/kvm/handle_exit.c > +++ b/arch/arm64/kvm/handle_exit.c > @@ -119,28 +119,14 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu) > { > struct kvm_run *run = vcpu->run; > u32 esr = kvm_vcpu_get_esr(vcpu); > - int ret = 0; > > run->exit_reason = KVM_EXIT_DEBUG; > run->debug.arch.hsr = esr; > > - switch (ESR_ELx_EC(esr)) { > - case ESR_ELx_EC_WATCHPT_LOW: > + if (ESR_ELx_EC(esr) == ESR_ELx_EC_WATCHPT_LOW) > run->debug.arch.far = vcpu->arch.fault.far_el2; > - fallthrough; > - case ESR_ELx_EC_SOFTSTP_LOW: > - case ESR_ELx_EC_BREAKPT_LOW: > - case ESR_ELx_EC_BKPT32: > - case ESR_ELx_EC_BRK64: > - break; > - default: > - kvm_err("%s: un-handled case esr: %#08x\n", > - __func__, (unsigned int) esr); > - ret = -1; > - break; > - } > > - return ret; > + return 0; > } > This looks better, but do you think we would be compromising on readability? Regards, Raghavendra > static int kvm_handle_unknown_ec(struct kvm_vcpu *vcpu) > > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible.
_______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
