Re: [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support

From: "Alex Bennée" <alex.bennee@linaro.org>
To: Will Deacon <will.deacon@arm.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
	"christoffer.dall@linaro.org" <christoffer.dall@linaro.org>,
	Marc Zyngier <Marc.Zyngier@arm.com>,
	"peter.maydell@linaro.org" <peter.maydell@linaro.org>,
	"agraf@suse.de" <agraf@suse.de>,
	"drjones@redhat.com" <drjones@redhat.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"zhichao.huang@linaro.org" <zhichao.huang@linaro.org>,
	"jan.kiszka@siemens.com" <jan.kiszka@siemens.com>,
	"dahi@linux.vnet.ibm.com" <dahi@linux.vnet.ibm.com>,
	"r65777@freescale.com" <r65777@freescale.com>,
	"bp@suse.de" <bp@suse.de>, Gleb Natapov <gleb@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Catalin Marinas <Catalin.Marinas@arm.com>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>,
	Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	"open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	"open list:ABI/API" <linux-api@vger.kernel.org>
Subject: Re: [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support
Date: Fri, 03 Jul 2015 17:07:41 +0100	[thread overview]
Message-ID: <87h9plrzqq.fsf@linaro.org> (raw)
In-Reply-To: <20150703092323.GA1588@arm.com>

Will Deacon <will.deacon@arm.com> writes:

> Hi Alex,
>
> On Thu, Jul 02, 2015 at 02:50:33PM +0100, Alex Bennée wrote:
>> Are you happy with this?:
>
> [...]
>
>> +/**
>> + * kvm_arch_dev_ioctl_check_extension
>> + *
>> + * We currently assume that the number of HW registers is uniform
>> + * across all CPUs (see cpuinfo_sanity_check).
>> + */
>>  int kvm_arch_dev_ioctl_check_extension(long ext)
>>  {
>>         int r;
>> @@ -64,6 +71,12 @@ int kvm_arch_dev_ioctl_check_extension(long ext)
>>         case KVM_CAP_ARM_EL1_32BIT:
>>                 r = cpu_has_32bit_el1();
>>                 break;
>> +       case KVM_CAP_GUEST_DEBUG_HW_BPS:
>> +               r = hw_breakpoint_slots(TYPE_INST);
>> +               break;
>> +       case KVM_CAP_GUEST_DEBUG_HW_WPS:
>> +               r  = hw_breakpoint_slots(TYPE_DATA);
>> +               break;
>
> Whilst I much prefer this code, it actually adds an unwanted dependency
> on PERF_EVENTS that I didn't think about to start with. Sorry to keep
> messing you about -- I guess your original patch is the best thing after
> all.

Everything looks to be in hw_breakpoint.[ch] which does depend on
CONFIG_HAVE_HW_BREAKPOINT which depends on PERF_EVENTS to be built.
However the previous code depended on this behaviour as well.

It would seem weird to enable guest debug using HW debug registers to
debug the guest yet not allowing the host kernel to use them? Of course
this is the only code they would share as all the magic of guest
debugging is already mostly there for dirty guest handling.

I'm not familiar with Kconfig but it looks like this is all part of
arm64 defconfig. Are people really going to want to disable PERF_EVENTS
but still debug their guests with HW support?

>
> Will

-- 
Alex Bennée