| Message ID | 1407138762-25531-4-git-send-email-Bharat.Bhushan@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 2014-08-04 at 13:22 +0530, Bharat Bhushan wrote: > Dbsr is not visible to userspace and we do not think any need to > expose this to userspace because: > Userspace cannot inject debug interrupt to guest (as this > does not know guest ability to handle debug interrupt), so > userspace will always clear DBSR. > Now if userspace has to always clear DBSR in KVM_EXIT_DEBUG > handling then clearing dbsr in kernel looks simple as this > avoid doing SET_SREGS/set_one_reg() to clear DBSR > > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com> > --- > arch/powerpc/kvm/booke.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index 322da7d..5c2e26a 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -735,6 +735,17 @@ static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu *vcpu) > struct debug_reg *dbg_reg = &(vcpu->arch.shadow_dbg_reg); > u32 dbsr = vcpu->arch.dbsr; > > + /* Clear guest dbsr (vcpu->arch.dbsr). > + * dbsr is not visible to userspace and we do not think any > + * need to expose this to userspace because: > + * Userspace cannot inject debug interrupt to guest (as this does > + * not know guest ability to handle debug interrupt), so userspace > + * will always clear DBSR. > + * Now if userspace has to always clear DBSR in KVM_EXIT_DEBUG > + * handling then clearing here looks simple as this > + * avoid doing SET_SREGS/set_one_reg() to clear DBSR > + */ > + vcpu->arch.dbsr = 0; > run->debug.arch.status = 0; > run->debug.arch.address = vcpu->arch.pc; > I think the changelog is adequate -- I don't think we need to be so verbose in the code itself. The question was just whether this was a userspace-visible change, and it isn't. FWIW, I think dbsr should be visible to userspace in general (regardless of whether it's cleared here), because all guest registers should be visible to userspace. I may be debugging a guest through means that don't require owning debug resources, such as stopping and inspecting a guest that has hung or crashed. -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Wood Scott-B07421 > Sent: Tuesday, August 05, 2014 4:17 AM > To: Bhushan Bharat-R65777 > Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Yoder Stuart- > B08248 > Subject: Re: [PATCH 4/5] KVM: PPC: BOOKE: Clear guest dbsr in userspace exit > KVM_EXIT_DEBUG > > On Mon, 2014-08-04 at 13:22 +0530, Bharat Bhushan wrote: > > Dbsr is not visible to userspace and we do not think any need to > > expose this to userspace because: > > Userspace cannot inject debug interrupt to guest (as this > > does not know guest ability to handle debug interrupt), so > > userspace will always clear DBSR. > > Now if userspace has to always clear DBSR in KVM_EXIT_DEBUG > > handling then clearing dbsr in kernel looks simple as this > > avoid doing SET_SREGS/set_one_reg() to clear DBSR > > > > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com> > > --- > > arch/powerpc/kvm/booke.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index > > 322da7d..5c2e26a 100644 > > --- a/arch/powerpc/kvm/booke.c > > +++ b/arch/powerpc/kvm/booke.c > > @@ -735,6 +735,17 @@ static int kvmppc_handle_debug(struct kvm_run *run, > struct kvm_vcpu *vcpu) > > struct debug_reg *dbg_reg = &(vcpu->arch.shadow_dbg_reg); > > u32 dbsr = vcpu->arch.dbsr; > > > > + /* Clear guest dbsr (vcpu->arch.dbsr). > > + * dbsr is not visible to userspace and we do not think any > > + * need to expose this to userspace because: > > + * Userspace cannot inject debug interrupt to guest (as this does > > + * not know guest ability to handle debug interrupt), so userspace > > + * will always clear DBSR. > > + * Now if userspace has to always clear DBSR in KVM_EXIT_DEBUG > > + * handling then clearing here looks simple as this > > + * avoid doing SET_SREGS/set_one_reg() to clear DBSR > > + */ > > + vcpu->arch.dbsr = 0; > > run->debug.arch.status = 0; > > run->debug.arch.address = vcpu->arch.pc; > > > > I think the changelog is adequate -- I don't think we need to be so verbose in > the code itself. The question was just whether this was a userspace-visible > change, and it isn't. Ok, will make a small comment. > > FWIW, I think dbsr should be visible to userspace in general (regardless of > whether it's cleared here), because all guest registers should be visible to > userspace. I may be debugging a guest through means that don't require owning > debug resources, such as stopping and inspecting a guest that has hung or > crashed. Do you mean that we should also add a one-reg interface for DBSR ? Thanks -Bharat > > -Scott >
On Mon, 2014-08-04 at 22:33 -0500, Bhushan Bharat-R65777 wrote: > > > -----Original Message----- > > From: Wood Scott-B07421 > > Sent: Tuesday, August 05, 2014 4:17 AM > > To: Bhushan Bharat-R65777 > > Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Yoder Stuart- > > B08248 > > Subject: Re: [PATCH 4/5] KVM: PPC: BOOKE: Clear guest dbsr in userspace exit > > KVM_EXIT_DEBUG > > > > FWIW, I think dbsr should be visible to userspace in general (regardless of > > whether it's cleared here), because all guest registers should be visible to > > userspace. I may be debugging a guest through means that don't require owning > > debug resources, such as stopping and inspecting a guest that has hung or > > crashed. > > Do you mean that we should also add a one-reg interface for DBSR ? Yes. -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 322da7d..5c2e26a 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -735,6 +735,17 @@ static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu *vcpu) struct debug_reg *dbg_reg = &(vcpu->arch.shadow_dbg_reg); u32 dbsr = vcpu->arch.dbsr; + /* Clear guest dbsr (vcpu->arch.dbsr). + * dbsr is not visible to userspace and we do not think any + * need to expose this to userspace because: + * Userspace cannot inject debug interrupt to guest (as this does + * not know guest ability to handle debug interrupt), so userspace + * will always clear DBSR. + * Now if userspace has to always clear DBSR in KVM_EXIT_DEBUG + * handling then clearing here looks simple as this + * avoid doing SET_SREGS/set_one_reg() to clear DBSR + */ + vcpu->arch.dbsr = 0; run->debug.arch.status = 0; run->debug.arch.address = vcpu->arch.pc;
Dbsr is not visible to userspace and we do not think any need to expose this to userspace because: Userspace cannot inject debug interrupt to guest (as this does not know guest ability to handle debug interrupt), so userspace will always clear DBSR. Now if userspace has to always clear DBSR in KVM_EXIT_DEBUG handling then clearing dbsr in kernel looks simple as this avoid doing SET_SREGS/set_one_reg() to clear DBSR Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com> --- arch/powerpc/kvm/booke.c | 11 +++++++++++ 1 file changed, 11 insertions(+)