| Message ID | 1431700035-23479-4-git-send-email-alex.bennee@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 15, 2015 at 03:27:06PM +0100, Alex Bennée wrote: > This commit defines the API headers for guest debugging. There are two > architecture specific debug structures: > > - kvm_guest_debug_arch, allows us to pass in HW debug registers > - kvm_debug_exit_arch, signals exception and possible faulting address > > The type of debugging being used is controlled by the architecture > specific control bits of the kvm_guest_debug->control flags in the ioctl > structure. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com> > Reviewed-by: Andrew Jones <drjones@redhat.com> > Acked-by: Christoffer Dall <christoffer.dall@linaro.org> > > --- > v2 > - expose hsr and pc directly to user-space > v3 > - s/control/controlled/ in commit message > - add v8 to ARM ARM comment (ARM Architecture Reference Manual) > - add rb tag > - rm pc, add far > - re-word comments on alignment > - rename KVM_ARM_NDBG_REGS -> KVM_ARM_MAX_DBG_REGS > v4 > - now uses common HW/SW BP define > - add a-b-tag > - use u32 for control regs > > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > index d268320..8796610 100644 > --- a/arch/arm64/include/uapi/asm/kvm.h > +++ b/arch/arm64/include/uapi/asm/kvm.h > @@ -100,10 +100,28 @@ struct kvm_sregs { > struct kvm_fpu { > }; > > +/* > + * See v8 ARM ARM D7.3: Debug Registers > + * > + * The control registers are architecturally defined as 32 bits but are > + * stored as 64 bit values alongside the value registers. This is done Stale comment? They're stored as __u32 below. It's possible that the registers could grow in future as happened in the case of CLIDR_EL1, so it might be worth treating system registers generally as u64 values. Mark. > + * to keep the copying of these values into the vcpu context simple as > + * everything is 64 bit aligned (see DBGBCR0_EL1 onwards in kvm_asm.h). > + * > + * The architectural limit is 16 debug registers of each type although > + * in practice there are usually less (see ID_AA64DFR0_EL1). > + */ > +#define KVM_ARM_MAX_DBG_REGS 16 > struct kvm_guest_debug_arch { > + __u32 dbg_bcr[KVM_ARM_MAX_DBG_REGS]; > + __u64 dbg_bvr[KVM_ARM_MAX_DBG_REGS]; > + __u32 dbg_wcr[KVM_ARM_MAX_DBG_REGS]; > + __u64 dbg_wvr[KVM_ARM_MAX_DBG_REGS]; > }; > > struct kvm_debug_exit_arch { > + __u32 hsr; > + __u64 far; > }; > > struct kvm_sync_regs { > -- > 2.3.5 > > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Mark Rutland <mark.rutland@arm.com> writes: > On Fri, May 15, 2015 at 03:27:06PM +0100, Alex Bennée wrote: >> This commit defines the API headers for guest debugging. There are two >> architecture specific debug structures: >> >> - kvm_guest_debug_arch, allows us to pass in HW debug registers >> - kvm_debug_exit_arch, signals exception and possible faulting address >> >> The type of debugging being used is controlled by the architecture >> specific control bits of the kvm_guest_debug->control flags in the ioctl >> structure. >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com> >> Reviewed-by: Andrew Jones <drjones@redhat.com> >> Acked-by: Christoffer Dall <christoffer.dall@linaro.org> >> >> --- >> v2 >> - expose hsr and pc directly to user-space >> v3 >> - s/control/controlled/ in commit message >> - add v8 to ARM ARM comment (ARM Architecture Reference Manual) >> - add rb tag >> - rm pc, add far >> - re-word comments on alignment >> - rename KVM_ARM_NDBG_REGS -> KVM_ARM_MAX_DBG_REGS >> v4 >> - now uses common HW/SW BP define >> - add a-b-tag >> - use u32 for control regs >> >> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h >> index d268320..8796610 100644 >> --- a/arch/arm64/include/uapi/asm/kvm.h >> +++ b/arch/arm64/include/uapi/asm/kvm.h >> @@ -100,10 +100,28 @@ struct kvm_sregs { >> struct kvm_fpu { >> }; >> >> +/* >> + * See v8 ARM ARM D7.3: Debug Registers >> + * >> + * The control registers are architecturally defined as 32 bits but are >> + * stored as 64 bit values alongside the value registers. This is done > > Stale comment? They're stored as __u32 below. Gah yes it is. > It's possible that the registers could grow in future as happened in the > case of CLIDR_EL1, so it might be worth treating system registers > generally as u64 values. Really? I mean the existing debug *control* registers have reserved bits 24-31 so there is space for expansion. > > Mark. > >> + * to keep the copying of these values into the vcpu context simple as >> + * everything is 64 bit aligned (see DBGBCR0_EL1 onwards in kvm_asm.h). >> + * >> + * The architectural limit is 16 debug registers of each type although >> + * in practice there are usually less (see ID_AA64DFR0_EL1). >> + */ >> +#define KVM_ARM_MAX_DBG_REGS 16 >> struct kvm_guest_debug_arch { >> + __u32 dbg_bcr[KVM_ARM_MAX_DBG_REGS]; >> + __u64 dbg_bvr[KVM_ARM_MAX_DBG_REGS]; >> + __u32 dbg_wcr[KVM_ARM_MAX_DBG_REGS]; >> + __u64 dbg_wvr[KVM_ARM_MAX_DBG_REGS]; >> }; >> >> struct kvm_debug_exit_arch { >> + __u32 hsr; >> + __u64 far; >> }; >> >> struct kvm_sync_regs { >> -- >> 2.3.5 >> >> _______________________________________________ >> kvmarm mailing list >> kvmarm@lists.cs.columbia.edu >> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
On 15 May 2015 at 16:14, Alex Bennée <alex.bennee@linaro.org> wrote: > > Mark Rutland <mark.rutland@arm.com> writes: > >> On Fri, May 15, 2015 at 03:27:06PM +0100, Alex Bennée wrote: >>> +/* >>> + * See v8 ARM ARM D7.3: Debug Registers >>> + * >>> + * The control registers are architecturally defined as 32 bits but are >>> + * stored as 64 bit values alongside the value registers. This is done >> >> Stale comment? They're stored as __u32 below. > > Gah yes it is. > >> It's possible that the registers could grow in future as happened in the >> case of CLIDR_EL1, so it might be worth treating system registers >> generally as u64 values. > > Really? I mean the existing debug *control* registers have reserved bits > 24-31 so there is space for expansion. Other places in the userspace ABI which deal with sysregs (notably ONE_REG) consistently define them all as 64-bit (which makes sense anyway since the ISA only provides 64-bit accessors to them). "Architecturally 32 bits" only means "top 32 bits reserved". -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 15 May 2015 at 16:14, Alex Bennée <alex.bennee@linaro.org> wrote: >> >> Mark Rutland <mark.rutland@arm.com> writes: >> >>> On Fri, May 15, 2015 at 03:27:06PM +0100, Alex Bennée wrote: >>>> +/* >>>> + * See v8 ARM ARM D7.3: Debug Registers >>>> + * >>>> + * The control registers are architecturally defined as 32 bits but are >>>> + * stored as 64 bit values alongside the value registers. This is done >>> >>> Stale comment? They're stored as __u32 below. >> >> Gah yes it is. >> >>> It's possible that the registers could grow in future as happened in the >>> case of CLIDR_EL1, so it might be worth treating system registers >>> generally as u64 values. >> >> Really? I mean the existing debug *control* registers have reserved bits >> 24-31 so there is space for expansion. > > Other places in the userspace ABI which deal with sysregs (notably > ONE_REG) consistently define them all as 64-bit (which makes sense > anyway since the ISA only provides 64-bit accessors to them). > "Architecturally 32 bits" only means "top 32 bits reserved". Fair enough, I can switch it back. The main reason I had them as all 64 bit before was because of the mapping onto the sys_regs context. If everyone is happy bloating the ABI a little I'm OK with that. It will make the hyp.S macro a little less ugly for one. > > -- PMM
On Fri, May 15, 2015 at 04:17:46PM +0100, Peter Maydell wrote: > On 15 May 2015 at 16:14, Alex Bennée <alex.bennee@linaro.org> wrote: > > > > Mark Rutland <mark.rutland@arm.com> writes: > > > >> On Fri, May 15, 2015 at 03:27:06PM +0100, Alex Bennée wrote: > >>> +/* > >>> + * See v8 ARM ARM D7.3: Debug Registers > >>> + * > >>> + * The control registers are architecturally defined as 32 bits but are > >>> + * stored as 64 bit values alongside the value registers. This is done > >> > >> Stale comment? They're stored as __u32 below. > > > > Gah yes it is. > > > >> It's possible that the registers could grow in future as happened in the > >> case of CLIDR_EL1, so it might be worth treating system registers > >> generally as u64 values. > > > > Really? I mean the existing debug *control* registers have reserved bits > > 24-31 so there is space for expansion. > > Other places in the userspace ABI which deal with sysregs (notably > ONE_REG) consistently define them all as 64-bit Also for pt_regs.pstate. I just spotted that in user_hwdebug_state in ptrace.h we seem to expose the debug control regsiters as __u32 already, aalong with some other registers. So we're already inconsistent w.r.t. how we expose those registers, and I'm not sure what we'd do elsewhere if any registers got expanded. :/ Mark.
Mark Rutland <mark.rutland@arm.com> writes: > On Fri, May 15, 2015 at 04:17:46PM +0100, Peter Maydell wrote: >> On 15 May 2015 at 16:14, Alex Bennée <alex.bennee@linaro.org> wrote: >> > >> > Mark Rutland <mark.rutland@arm.com> writes: >> > >> >> On Fri, May 15, 2015 at 03:27:06PM +0100, Alex Bennée wrote: >> >>> +/* >> >>> + * See v8 ARM ARM D7.3: Debug Registers >> >>> + * >> >>> + * The control registers are architecturally defined as 32 bits but are >> >>> + * stored as 64 bit values alongside the value registers. This is done >> >> >> >> Stale comment? They're stored as __u32 below. >> > >> > Gah yes it is. >> > >> >> It's possible that the registers could grow in future as happened in the >> >> case of CLIDR_EL1, so it might be worth treating system registers >> >> generally as u64 values. >> > >> > Really? I mean the existing debug *control* registers have reserved bits >> > 24-31 so there is space for expansion. >> >> Other places in the userspace ABI which deal with sysregs (notably >> ONE_REG) consistently define them all as 64-bit > > Also for pt_regs.pstate. > > I just spotted that in user_hwdebug_state in ptrace.h we seem to expose > the debug control regsiters as __u32 already, aalong with some other > registers. I thought those where ptrace specific fields which then get munged into the final values inside the kernel? > > So we're already inconsistent w.r.t. how we expose those registers, and > I'm not sure what we'd do elsewhere if any registers got expanded. :/ > > Mark.
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index d268320..8796610 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -100,10 +100,28 @@ struct kvm_sregs { struct kvm_fpu { }; +/* + * See v8 ARM ARM D7.3: Debug Registers + * + * The control registers are architecturally defined as 32 bits but are + * stored as 64 bit values alongside the value registers. This is done + * to keep the copying of these values into the vcpu context simple as + * everything is 64 bit aligned (see DBGBCR0_EL1 onwards in kvm_asm.h). + * + * The architectural limit is 16 debug registers of each type although + * in practice there are usually less (see ID_AA64DFR0_EL1). + */ +#define KVM_ARM_MAX_DBG_REGS 16 struct kvm_guest_debug_arch { + __u32 dbg_bcr[KVM_ARM_MAX_DBG_REGS]; + __u64 dbg_bvr[KVM_ARM_MAX_DBG_REGS]; + __u32 dbg_wcr[KVM_ARM_MAX_DBG_REGS]; + __u64 dbg_wvr[KVM_ARM_MAX_DBG_REGS]; }; struct kvm_debug_exit_arch { + __u32 hsr; + __u64 far; }; struct kvm_sync_regs {