Hi Marc,
On 31/05/2017 08:46, Marc Zyngier wrote:
> On Wed, 31 May 2017 08:33:05 +0200
> Auger Eric <eric.auger@xxxxxxxxxx> wrote:
>
>> Hi Marc,
>>
>> On 30/05/2017 16:24, Marc Zyngier wrote:
>>> On 30/05/17 08:48, Auger Eric wrote:
>>>> Hi Marc
>>>>
>>>> On 03/05/2017 12:45, Marc Zyngier wrote:
>>>>> Add a handler for writing the guest's view of the ICC_EOIR1_EL1
>>>>> register. This involves dropping the priority of the interrupt,
>>>>> and deactivating it if required (EOImode == 0).
>>>>>
>>>>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>>>>> ---
>>>>> include/linux/irqchip/arm-gic-v3.h | 2 +
>>>>> virt/kvm/arm/hyp/vgic-v3-sr.c | 119 +++++++++++++++++++++++++++++++++++++
>>>>> 2 files changed, 121 insertions(+)
>>>>>
>>>>> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
>>>>> index 7610ea4e8337..c56d9bc2c904 100644
>>>>> --- a/include/linux/irqchip/arm-gic-v3.h
>>>>> +++ b/include/linux/irqchip/arm-gic-v3.h
>>>>> @@ -403,6 +403,8 @@
>>>>>
>>>>> #define ICH_HCR_EN (1 << 0)
>>>>> #define ICH_HCR_UIE (1 << 1)
>>>>> +#define ICH_HCR_EOIcount_SHIFT 27
>>>>> +#define ICH_HCR_EOIcount_MASK (0x1f << ICH_HCR_EOIcount_SHIFT)
>>>>>
>>>>> #define ICH_VMCR_CBPR_SHIFT 4
>>>>> #define ICH_VMCR_CBPR_MASK (1 << ICH_VMCR_CBPR_SHIFT)
>>>>> diff --git a/virt/kvm/arm/hyp/vgic-v3-sr.c b/virt/kvm/arm/hyp/vgic-v3-sr.c
>>>>> index 49aad1de3ac8..a76351b3ad66 100644
>>>>> --- a/virt/kvm/arm/hyp/vgic-v3-sr.c
>>>>> +++ b/virt/kvm/arm/hyp/vgic-v3-sr.c
>>>>> @@ -425,6 +425,26 @@ static int __hyp_text __vgic_v3_highest_priority_lr(struct kvm_vcpu *vcpu,
>>>>> return lr;
>>>>> }
>>>>>
>>>>> +static int __hyp_text __vgic_v3_find_active_lr(struct kvm_vcpu *vcpu,
>>>>> + int intid, u64 *lr_val)
>>>>> +{
>>>>> + unsigned int used_lrs = vcpu->arch.vgic_cpu.used_lrs;
>>>>> + int i;
>>>>> +
>>>>> + for (i = 0; i < used_lrs; i++) {
>>>>> + u64 val = __gic_v3_get_lr(i);
>>>>> +
>>>>> + if ((val & ICH_LR_VIRTUAL_ID_MASK) == intid &&
>>>>> + (val & ICH_LR_ACTIVE_BIT)) {
>>>> I guess it is safe because we don't have yet virtual interrupts directly
>>>> mapped to phys IRQs, besides timer one?
>>>
>>> What would that change? I don't see how having a HW interrupt here would
>>> be unsafe... Am I missing something?
>> I was thinking about the case of an HW mapped interrupt whose active
>> state must be observed at distributor level - as I understood the spec -
>> and not at LR level.
>
> What part of the spec are you referring to?
>
> A virtual interrupt, even backed by a HW interrupt, does have an active
> state (unless it is an LPI). The only state we cannot observe in the LR
> when the HW bit is set is the Active+Pending state because the pending
> bit is then at the physical distributor level.
>
> For all intent and purposes, the active state in the LR behaves the
> same, irrespective of the HW state, until the guest issues a
> deactivation (either using EOI or DIR, depending on EOImode).
Hum OK. I was referring to the note in table 5-9 of IHI0048B2 but
effectively I added a "s" where there is none :-(
"
For hardware interrupts, the pending and active state is held in the
physical Distributor rather than the virtual
CPU interface. A hypervisor must only use the pending and active state
for software originated interrupts
associated with virtual devices, or SGIs.
"
Sorry for the noise.
Eric
>
> Thanks,
>
> M.
>
