Messages in this thread | | | Subject | Re: [PATCH v4 16/20] KVM: arm64: GICv4.1: Allow SGIs to switch between HW and SW interrupts | From | Zenghui Yu <> | Date | Mon, 2 Mar 2020 10:40:30 +0800 |
| |
Hi Marc,
On 2020/2/29 3:16, Marc Zyngier wrote: > Hi Zenghui, > > On 2020-02-20 03:55, Zenghui Yu wrote: >> Hi Marc, >> >> On 2020/2/14 22:57, Marc Zyngier wrote: >>> In order to let a guest buy in the new, active-less SGIs, we >>> need to be able to switch between the two modes. >>> >>> Handle this by stopping all guest activity, transfer the state >>> from one mode to the other, and resume the guest. >>> >>> Signed-off-by: Marc Zyngier <maz@kernel.org> >> >> [...] >> >>> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c >>> index 1bc09b523486..2c9fc13e2c59 100644 >>> --- a/virt/kvm/arm/vgic/vgic-v3.c >>> +++ b/virt/kvm/arm/vgic/vgic-v3.c >>> @@ -540,6 +540,8 @@ int vgic_v3_map_resources(struct kvm *kvm) >>> goto out; >>> } >>> + if (kvm_vgic_global_state.has_gicv4_1) >>> + vgic_v4_configure_vsgis(kvm); >>> dist->ready = true; >>> out: >> >> Is there any reason to invoke vgic_v4_configure_vsgis() here? >> This is called on the first VCPU run, through kvm_vgic_map_resources(). >> Shouldn't the vSGI configuration only driven by a GICD_CTLR.nASSGIreq >> writing (from guest, or from userspace maybe)? > > What I'm trying to catch here is the guest that has been restored with > nASSGIreq set. At the moment, we don't do anything on the userspace > side, because the vmm could decide to write that particular bit > multiple times, and switching between the two modes is expensive (not > to mention that all the vcpus may not have been created yet). > > Moving it to the first run makes all these pitfalls go away (we have the > final nASSSGIreq value, and all the vcpus are accounted for).
So what will happen on restoration is (roughly):
- for GICR_ISPENR0: We will restore the pending status of vSGIs into software pending_latch, just like what we've done for normal SGIs. - for GICD_CTLR.nASSGIreq: We will only record the written value. (Note to myself: No invocation of configure_vsgis() in uaccess_write callback, I previously mixed it up with the guest write callback.) - Finally, you choose the first vcpu run as the appropriate point to potentially flush the pending status to HW according to the final nASSGIreq value.
> > Does this make sense to you?
Yeah, it sounds like a good idea! And please ignore what I've replied to patch #15, I obviously missed your intention at that time, sorry...
But can we move this hunk to some places more appropriate, for example, put it together with the GICD_CTLR's uaccess_write change? It might make things a bit clearer for other reviewers. :-)
Thanks, Zenghui
| |