On 2024/3/20 3:26, Peter Maydell wrote:
> On Tue, 19 Mar 2024 at 18:51, Richard Henderson
> <richard.hender...@linaro.org> wrote:
>>
>> On 3/19/24 07:28, Peter Maydell wrote:
>>>> switch (excp_idx) {
>>>> + case EXCP_NMI:
>>>> + pstate_unmasked = !allIntMask;
>>>> + break;
>>>> +
>>>> + case EXCP_VNMI:
>>>> + if ((!(hcr_el2 & HCR_IMO) && !(hcr_el2 & HCR_FMO)) ||
>>>> + (hcr_el2 & HCR_TGE)) {
>>>> + /* VNMIs(VIRQs or VFIQs) are only taken when hypervized. */
>>>> + return false;
>>>> + }
>>>
>>> VINMI and VFNMI aren't the same thing: do we definitely want to
>>> merge them into one EXCP_VNMI ?
>>
>> We do not, which is why VFNMI is going through EXCP_VFIQ. A previous
>> version did, and I
>> see the comment did not change to match the new implementation.
>
> Why do we put VFNMI through VFIQ, though? They're not
> the same thing either... I think I would find this less
> confusing if we implemented what the spec says and distinguished
> all of (IRQ, FIQ, IRQ-with-superpriority and FIQ-with-superpriority).
Should there be separate VNMI lines and exceptions for the
IRQ-with-superpriority and FIQ-with-superpriority ?
>
>>> The use of the _eff() versions of the functions here is
>>> correct but it introduces a new case where we need to
>>> reevaluate the status of the VNMI etc interrupt status:
>>> when we change from Secure to NonSecure or when we change
>>> SCR_EL3.EEL2 or SCR_EL3.HXEN. We either need to make sure
>>> we reevaluate when we drop from EL3 to EL2 (which would be
>>> OK since VINMI and VFNMI can't be taken at EL3 and none of
>>> these bits can change except at EL3) or else make the calls
>>> to reevaluate them when we write to SCR_EL3. At least, I don't
>>> think we currently reevaluate these bits on an EL change.
>>
>> We re-evaluate these bits on EL change via gicv3_cpuif_el_change_hook.
>
> Only if the GIC is connected to the VIRQ and VFIQ interrupt lines,
> which it doesn't in theory have to be (though in practice it
> usually will be). Plus it feels a bit fragile against
> somebody deciding to put in a "this line hasn't changed state
> so don't bother calling the handler again" optimization in the
> future.
>
> thanks
> -- PMM
