Re: [PATCH v4 02/12] KVM: define common KVM_GUESTDBG_USE_SW/HW_BP bits

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2015-05-15 19:33, David Hildenbrand wrote:
>> Am 15.05.2015 um 16:27 schrieb Alex Bennée:
>>> +++ b/arch/s390/include/uapi/asm/kvm.h
>>> @@ -114,8 +114,6 @@ struct kvm_fpu {
>>>  	__u64 fprs[16];
>>>  };
>>>
>>> -#define KVM_GUESTDBG_USE_HW_BP		0x00010000
>> [...]
>>> +++ b/include/uapi/linux/kvm.h
>> [...]
>>> +#define KVM_GUESTDBG_USE_SW_BP		(1 << 16)
>>> +#define KVM_GUESTDBG_USE_HW_BP		(1 << 17)
>>
>> This is an ABI break for s390, no?
>>
>> David, do you remember why we do not use KVM_GUESTDBG_USE_SW_BP?
>>
> 
> We never had to tell the kernel about software breakpoints as this is all
> handled via 4 byte DIAG instructions until now. We don't have to turn this
> mechanism on. QEMU can directly insert the desired DIAG instructions and gets
> notified when they are about to get executed.
> 
> (But we still have 2 byte breakpoint support todo - still tbd how exactly this
> will be realized - could be turned on via such a mechanism)
> 
> The problem is, that these bits are arch specific, now Alex wants to unify
> them for all archs.
> 
> So yes, this is an ABI break for us and breaks hardware breakpoints.(I think
> the first version of this patch didn't contain this ABI break when I had a look)
> 
> I wonder if it wouldn't make more sense to
> 
> - introduce new bits in the arch-unspecific section
> - rework the existing implementers to accept both bits
> 
> Or to simply leave stuff as it is and handle it via arch specific bits.

With one arch proving the "all need this" theory wrong, just drop this
patch. Even quicker when it breaks an ABI.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux