Re: [RFC v2 11/14] linux-headers/kvm.h: add capability to forward hypercall

From: Guoheyi <guoheyi@huawei.com>
To: Cornelia Huck <cohuck@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Marc Zyngier <marc.zyngier@arm.com>,
	qemu-devel@nongnu.org, qemu-arm@nongnu.org,
	James Morse <james.morse@arm.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	wanghaibin.wang@huawei.com, Dave Martin <Dave.Martin@arm.com>
Subject: Re: [RFC v2 11/14] linux-headers/kvm.h: add capability to forward hypercall
Date: Fri, 8 Nov 2019 09:54:05 +0800	[thread overview]
Message-ID: <3e0dc8b2-77dd-f37a-861b-fdd90134e880@huawei.com> (raw)
In-Reply-To: <20191107131210.26acf011.cohuck@redhat.com>

On 2019/11/7 20:12, Cornelia Huck wrote:
> On Thu, 7 Nov 2019 19:57:22 +0800
> Guoheyi <guoheyi@huawei.com> wrote:
>
>> On 2019/11/7 16:57, Michael S. Tsirkin wrote:
>>> On Thu, Nov 07, 2019 at 09:44:36AM +0800, Guoheyi wrote:
>>>> On 2019/11/7 1:55, Cornelia Huck wrote:
>>>>> On Tue, 5 Nov 2019 17:10:53 +0800
>>>>> Heyi Guo <guoheyi@huawei.com> wrote:
>>>>>   
>>>>>> To keep backward compatibility, we add new KVM capability
>>>>>> "KVM_CAP_FORWARD_HYPERCALL" to probe whether KVM supports forwarding
>>>>>> hypercall to userspace.
>>>>>>
>>>>>> The capability should be enabled explicitly, for we don't want user
>>>>>> space application to deal with unexpected hypercall exits. After
>>>>>> enabling this cap, all HVC calls unhandled by kvm will be forwarded to
>>>>>> user space.
>>>>>>
>>>>>> Signed-off-by: Heyi Guo <guoheyi@huawei.com>
>>>>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>>>>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>>>>>> Cc: Cornelia Huck <cohuck@redhat.com>
>>>>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>>>>> Cc: Dave Martin <Dave.Martin@arm.com>
>>>>>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>>>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>>>> Cc: James Morse <james.morse@arm.com>
>>>>>> ---
>>>>>>     linux-headers/linux/kvm.h |  1 +
>>>>>>     target/arm/sdei.c         | 16 ++++++++++++++++
>>>>>>     target/arm/sdei.h         |  2 ++
>>>>>>     3 files changed, 19 insertions(+)
>>>>>>
>>>>>> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
>>>>>> index 3d9b18f7f8..36c9b3859f 100644
>>>>>> --- a/linux-headers/linux/kvm.h
>>>>>> +++ b/linux-headers/linux/kvm.h
>>>>>> @@ -1000,6 +1000,7 @@ struct kvm_ppc_resize_hpt {
>>>>>>     #define KVM_CAP_PMU_EVENT_FILTER 173
>>>>>>     #define KVM_CAP_ARM_IRQ_LINE_LAYOUT_2 174
>>>>>>     #define KVM_CAP_HYPERV_DIRECT_TLBFLUSH 175
>>>>>> +#define KVM_CAP_FORWARD_HYPERCALL 176
>>>>>>     #ifdef KVM_CAP_IRQ_ROUTING
>>>>> Is this cap upstream already? I would have thought your header sync
>>>>> would have brought it in, then. (Saying this, that header sync looks
>>>>> awfully small.)
>>>>>
>>>>> If it is not upstream yet, please split off this hunk into a separate
>>>>> patch -- it's a bit annoying, but makes life easier for merging.
>>>> No, it is not upstream yet. The whole framework and interfaces between KVM
>>>> and qemu are still under discussion. I'll keep in mind of this when moving
>>>> forward to next steps...
>>>>
>>>> Thanks,
>>>> HG
>>> It's best to add it in some other place meanwhile.
>> Do you mean to split this patch from the whole patch set and send it
>> separately? Sorry I'm not clear about maintainers' work and may bring
>> you some trouble...
> My preferred approach:
>
> - add a commit entitled "placeholder for headers update" that contains
>    the not-yet-upstream changes in the header files you need
> - base the rest of your work on that
> ...
> <review happens, series looks good>
> ...
> - if kernel changes are upstream: replace the placeholder patch with a
>    real update (may include separate patches, if you need an additional
>    header); maintainer merges
> - if kernel changes are not yet upstream: maintainer merges with
>    placeholder to a feature branch, replaces with real update and merges
>    once kernel patches hit upstream
> (not every maintainer does the second approach; they may ask you
> instead to resend with a proper headers update once the kernel changes
> are upstream)

Thanks a lot. I'll do that in the next version.
HG

>
> .
>