On Tue, Oct 12, 2021 at 10:33 AM Viresh Kumar viresh.kumar@linaro.org wrote:
On 12-10-21, 09:51, Arnd Bergmann wrote:
This should work fine, but it seems odd that this is asymmetric between enable/unmask and disable/mask. My feeling is that it would be more consistent if you also require the "enable" to happen before "unmask" to limit the number possible states that the irq line can be in.
If you look at a regular IP block (GPIO or GIC, etc) which exposes registers for enable/disable and mask/unmask, then normally they won't have any constraints on what needs to be done first enable or unmask. Both have different purposes and can be handled (and must be) handled separately. The same should apply here as well.
I don't see a problem with the flexibility, but I find the inconsistency slightly annoying: if disabling the interrupt line has the side-effect of masking it, it should not be possible to unmask it before enabling.
To have it more consistent, it would seem better to do one of two things:
a) require disabled interrupts to always be masked, only allowing the unmask to happen after enable, while forcing a mask during disable.
or
b) separate the 'mask' from the 'disable' operation, leaving the event descriptor queued if you disable it, but adding another operation for an explicit mask (i.e. return the event descriptor) that is separate from 'disable'.
I would prefer a) here since I think that makes a nicer virtio spec, but b) would make it more similar to hardware gpio controllers.
In the end, I don't think any of the combinations would cause problems, this is just a matter of personal taste.
Arnd