Re: [RFC PATCH v1 3/4] KVM: arm64: GICv4.1: Restore VLPI's pending state to physical side

From: Auger Eric <eric.auger@redhat.com>
To: Shenming Lu <lushenming@huawei.com>, Marc Zyngier <maz@kernel.org>
Cc: James Morse <james.morse@arm.com>,
	Julien Thierry <julien.thierry.kdev@gmail.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Christoffer Dall <christoffer.dall@arm.com>,
	Alex Williamson <alex.williamson@redhat.com>,
	Kirti Wankhede <kwankhede@nvidia.com>,
	Cornelia Huck <cohuck@redhat.com>, Neo Jia <cjia@nvidia.com>,
	wanghaibin.wang@huawei.com, yuzenghui@huawei.com
Subject: Re: [RFC PATCH v1 3/4] KVM: arm64: GICv4.1: Restore VLPI's pending state to physical side
Date: Wed, 16 Dec 2020 11:35:54 +0100	[thread overview]
Message-ID: <f8b398df-9945-9ce6-18e6-970637a1bb51@redhat.com> (raw)
In-Reply-To: <59ec07e5-c017-8644-b96f-e87fe600c490@huawei.com>

Hi Shenming,

On 12/1/20 1:15 PM, Shenming Lu wrote:
> On 2020/12/1 19:50, Marc Zyngier wrote:
>> On 2020-12-01 11:40, Shenming Lu wrote:
>>> On 2020/12/1 18:55, Marc Zyngier wrote:
>>>> On 2020-11-30 07:23, Shenming Lu wrote:
>>>>
>>>> Hi Shenming,
>>>>
>>>>> We are pondering over this problem these days, but still don't get a
>>>>> good solution...
>>>>> Could you give us some advice on this?
>>>>>
>>>>> Or could we move the restoring of the pending states (include the sync
>>>>> from guest RAM and the transfer to HW) to the GIC VM state change handler,
>>>>> which is completely corresponding to save_pending_tables (more symmetric?)
>>>>> and don't expose GICv4...
>>>>
>>>> What is "the GIC VM state change handler"? Is that a QEMU thing?
>>>
>>> Yeah, it is a a QEMU thing...
>>>
>>>> We don't really have that concept in KVM, so I'd appreciate if you could
>>>> be a bit more explicit on this.
>>>
>>> My thought is to add a new interface (to QEMU) for the restoring of
>>> the pending states, which is completely corresponding to
>>> KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES...
>>> And it is called from the GIC VM state change handler in QEMU, which
>>> is happening after the restoring (call kvm_vgic_v4_set_forwarding())
>>> but before the starting (running) of the VFIO device.
>>
>> Right, that makes sense. I still wonder how much the GIC save/restore
>> stuff differs from other architectures that implement similar features,
>> such as x86 with VT-D.
> 
> I am not familiar with it...
> 
>>
>> It is obviously too late to change the userspace interface, but I wonder
>> whether we missed something at the time.
> 
> The interface seems to be really asymmetrical?...

in qemu d5aa0c229a ("hw/intc/arm_gicv3_kvm: Implement pending table
save") commit message, it is traced:

"There is no explicit restore as the tables are implicitly sync'ed
on ITS table restore and on LPI enable at redistributor level."

At that time there was no real justification behind adding the RESTORE
fellow attr.

Maybe a stupid question but isn't it possible to unset the forwarding
when saving and rely on VFIO to automatically restore it when resuming
on destination?

Thanks

Eric

> 
> Or is there a possibility that we could know which irq is hw before the VFIO
> device calls kvm_vgic_v4_set_forwarding()?
> 
> Thanks,
> Shenming
> 
>>
>> Thanks,
>>
>>         M.
> 