Message ID | 1447345251-22625-2-git-send-email-alex.bennee@linaro.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 12 November 2015 at 16:20, Alex Bennée <alex.bennee@linaro.org> wrote: > As we haven't always had guest debug support we need to probe for it. > Additionally we don't do this in the start-up capability code so we > don't fall over on old kernels. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > target-arm/kvm64.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c > index ceebfeb..d087794 100644 > --- a/target-arm/kvm64.c > +++ b/target-arm/kvm64.c > @@ -25,6 +25,22 @@ > #include "internals.h" > #include "hw/arm/arm.h" > > +static bool have_guest_debug; > + > +/** > + * kvm_arm_init_debug() > + * @cs: CPUState > + * > + * Check for guest debug capabilities. > + * > + */ > +static void kvm_arm_init_debug(CPUState *cs) > +{ > + have_guest_debug = kvm_check_extension(cs->kvm_state, > + KVM_CAP_SET_GUEST_DEBUG); > + return; > +} > + > static inline void set_feature(uint64_t *features, int feature) > { > *features |= 1ULL << feature; > @@ -121,6 +137,8 @@ int kvm_arch_init_vcpu(CPUState *cs) > } > cpu->mp_affinity = mpidr & ARM64_AFFINITY_MASK; > > + kvm_arm_init_debug(cs); > + > return kvm_arm_init_cpreg_list(cpu); > } I assume in practice the kernel guarantees that either all CPUs have the SET_GUEST_DEBUG cap, or none do :-) Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 20 November 2015 at 15:05, Peter Maydell <peter.maydell@linaro.org> wrote: > On 12 November 2015 at 16:20, Alex Bennée <alex.bennee@linaro.org> wrote: >> As we haven't always had guest debug support we need to probe for it. >> Additionally we don't do this in the start-up capability code so we >> don't fall over on old kernels. >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> target-arm/kvm64.c | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c >> index ceebfeb..d087794 100644 >> --- a/target-arm/kvm64.c >> +++ b/target-arm/kvm64.c >> @@ -25,6 +25,22 @@ >> #include "internals.h" >> #include "hw/arm/arm.h" >> >> +static bool have_guest_debug; >> + >> +/** >> + * kvm_arm_init_debug() >> + * @cs: CPUState >> + * >> + * Check for guest debug capabilities. >> + * >> + */ >> +static void kvm_arm_init_debug(CPUState *cs) >> +{ >> + have_guest_debug = kvm_check_extension(cs->kvm_state, >> + KVM_CAP_SET_GUEST_DEBUG); >> + return; >> +} >> + >> static inline void set_feature(uint64_t *features, int feature) >> { >> *features |= 1ULL << feature; >> @@ -121,6 +137,8 @@ int kvm_arch_init_vcpu(CPUState *cs) >> } >> cpu->mp_affinity = mpidr & ARM64_AFFINITY_MASK; >> >> + kvm_arm_init_debug(cs); >> + >> return kvm_arm_init_cpreg_list(cpu); >> } > > I assume in practice the kernel guarantees that either all > CPUs have the SET_GUEST_DEBUG cap, or none do :-) > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> ...except I've just noticed that nothing else in this patchset ever reads the have_guest_debug bool we just set, so what is the purpose of this patch? thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 20 November 2015 at 15:05, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 12 November 2015 at 16:20, Alex Bennée <alex.bennee@linaro.org> wrote: >>> As we haven't always had guest debug support we need to probe for it. >>> Additionally we don't do this in the start-up capability code so we >>> don't fall over on old kernels. >>> >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> --- >>> target-arm/kvm64.c | 18 ++++++++++++++++++ >>> 1 file changed, 18 insertions(+) >>> >>> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c >>> index ceebfeb..d087794 100644 >>> --- a/target-arm/kvm64.c >>> +++ b/target-arm/kvm64.c >>> @@ -25,6 +25,22 @@ >>> #include "internals.h" >>> #include "hw/arm/arm.h" >>> >>> +static bool have_guest_debug; >>> + >>> +/** >>> + * kvm_arm_init_debug() >>> + * @cs: CPUState >>> + * >>> + * Check for guest debug capabilities. >>> + * >>> + */ >>> +static void kvm_arm_init_debug(CPUState *cs) >>> +{ >>> + have_guest_debug = kvm_check_extension(cs->kvm_state, >>> + KVM_CAP_SET_GUEST_DEBUG); >>> + return; >>> +} >>> + >>> static inline void set_feature(uint64_t *features, int feature) >>> { >>> *features |= 1ULL << feature; >>> @@ -121,6 +137,8 @@ int kvm_arch_init_vcpu(CPUState *cs) >>> } >>> cpu->mp_affinity = mpidr & ARM64_AFFINITY_MASK; >>> >>> + kvm_arm_init_debug(cs); >>> + >>> return kvm_arm_init_cpreg_list(cpu); >>> } >> >> I assume in practice the kernel guarantees that either all >> CPUs have the SET_GUEST_DEBUG cap, or none do :-) >> >> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > ...except I've just noticed that nothing else in this patchset > ever reads the have_guest_debug bool we just set, so what is > the purpose of this patch? Oops, maybe to point out my stupidity ;-) But yes the SET_GUEST_DEBUG cap is kernel wide. > > thanks > -- PMM
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c index ceebfeb..d087794 100644 --- a/target-arm/kvm64.c +++ b/target-arm/kvm64.c @@ -25,6 +25,22 @@ #include "internals.h" #include "hw/arm/arm.h" +static bool have_guest_debug; + +/** + * kvm_arm_init_debug() + * @cs: CPUState + * + * Check for guest debug capabilities. + * + */ +static void kvm_arm_init_debug(CPUState *cs) +{ + have_guest_debug = kvm_check_extension(cs->kvm_state, + KVM_CAP_SET_GUEST_DEBUG); + return; +} + static inline void set_feature(uint64_t *features, int feature) { *features |= 1ULL << feature; @@ -121,6 +137,8 @@ int kvm_arch_init_vcpu(CPUState *cs) } cpu->mp_affinity = mpidr & ARM64_AFFINITY_MASK; + kvm_arm_init_debug(cs); + return kvm_arm_init_cpreg_list(cpu); }
As we haven't always had guest debug support we need to probe for it. Additionally we don't do this in the start-up capability code so we don't fall over on old kernels. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- target-arm/kvm64.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)