Re: [PATCH 6/6] KVM: PPC: BOOKE: Emulate debug registers and exception

[Alexander Graf <agraf@suse.de>]

On 11.07.14 10:39, Bharat Bhushan wrote:
> This patch emulates debug registers and debug exception
> to support guest using debug resource. This enables running
> gdb/kgdb etc in guest.
>
> On BOOKE architecture we cannot share debug resources between QEMU and
> guest because:
>      When QEMU is using debug resources then debug exception must
>      be always enabled. To achieve this we set MSR_DE and also set
>      MSRP_DEP so guest cannot change MSR_DE.
>
>      When emulating debug resource for guest we want guest
>      to control MSR_DE (enable/disable debug interrupt on need).
>
>      So above mentioned two configuration cannot be supported
>      at the same time. So the result is that we cannot share
>      debug resources between QEMU and Guest on BOOKE architecture.
>
> In the current design QEMU gets priority over guest, this means that if
> QEMU is using debug resources then guest cannot use them and if guest is
> using debug resource then QEMU can overwrite them.
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> ---
> Hi Alex,
>
> I thought of having some print in register emulation if QEMU
> is using debug resource, Also when QEMU overwrites guest written
> values but that looks excessive. If I uses some variable which
> get set when guest starts using debug registers and check in
> debug set ioctl then that look ugly. Looking for suggestions

Whatever you do, have QEMU do the print, not the kernel.

>
>   arch/powerpc/include/asm/kvm_ppc.h |   3 +
>   arch/powerpc/kvm/booke.c           |  27 +++++++
>   arch/powerpc/kvm/booke_emulate.c   | 157 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 187 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index e2fd5a1..f3f7611 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -173,6 +173,9 @@ extern int kvmppc_xics_get_xive(struct kvm *kvm, u32 irq, u32 *server,
>   extern int kvmppc_xics_int_on(struct kvm *kvm, u32 irq);
>   extern int kvmppc_xics_int_off(struct kvm *kvm, u32 irq);
>   
> +void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu);
> +void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu);
> +
>   union kvmppc_one_reg {
>   	u32	wval;
>   	u64	dval;
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index fadfe76..c2471ed 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -264,6 +264,16 @@ static void kvmppc_core_dequeue_watchdog(struct kvm_vcpu *vcpu)
>   	clear_bit(BOOKE_IRQPRIO_WATCHDOG, &vcpu->arch.pending_exceptions);
>   }
>   
> +void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu)
> +{
> +	kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG);
> +}
> +
> +void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu)
> +{
> +	clear_bit(BOOKE_IRQPRIO_DEBUG, &vcpu->arch.pending_exceptions);
> +}
> +
>   static void set_guest_srr(struct kvm_vcpu *vcpu, unsigned long srr0, u32 srr1)
>   {
>   #ifdef CONFIG_KVM_BOOKE_HV
> @@ -783,6 +793,23 @@ 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;
>   
> +	if (vcpu->guest_debug == 0) {
> +		/* Debug resources belong to Guest */
> +		if (dbsr && (vcpu->arch.shared->msr & MSR_DE))
> +			kvmppc_core_queue_debug(vcpu);
> +
> +		/* Inject a program interrupt if trap debug is not allowed */
> +		if ((dbsr & DBSR_TIE) && !(vcpu->arch.shared->msr & MSR_DE))
> +			kvmppc_core_queue_program(vcpu, ESR_PTR);

In that case we would've received a program interrupt and never entered 
this code path, no?


Alex