lkml.org 
[lkml]   [2020]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4 16/20] KVM: arm64: GICv4.1: Allow SGIs to switch between HW and SW interrupts
From
Date
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

\
 
 \ /
  Last update: 2020-03-02 03:41    [W:0.072 / U:3.392 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site