Re: [PATCH v2 4/4] x86/dpci: remove the dpci EOI timer

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.01.2021 15:28, Roger Pau Monne wrote:
> Current interrupt pass though code will setup a timer for each
> interrupt injected to the guest that requires an EOI from the guest.
> Such timer would perform two actions if the guest doesn't EOI the
> interrupt before a given period of time. The first one is deasserting
> the virtual line, the second is perform an EOI of the physical
> interrupt source if it requires such.
> 
> The deasserting of the guest virtual line is wrong, since it messes
> with the interrupt status of the guest. This seems to have been done
> in order to compensate for missing deasserts when certain interrupt
> controller actions are performed. The original motivation of the
> introduction of the timer was to fix issues when a GSI was shared
> between different guests. We believe that other changes in the
> interrupt handling code (ie: proper propagation of EOI related actions
> to dpci) will have fixed such errors now.
> 
> Performing an EOI of the physical interrupt source is redundant, since
> there's already a timer that takes care of this for all interrupts,
> not just the HVM dpci ones, see irq_guest_action_t struct eoi_timer
> field.
> 
> Since both of the actions performed by the dpci timer are not
> required, remove it altogether.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with ...

> Changes since v1:
>  - Add parentheses.

... this, as you've already clarified, indeed having happened.
I understand this change is independent of the earlier ones in
this series. The minor adjustment could be taken care of while
committing, and I'm inclined to not wait ...

>  xen/drivers/passthrough/vtd/x86/hvm.c |  3 -

... for Kevin's ack on this obvious part of the change,
considering his prior input on the discussion we've had (where
he did signal agreement with the removal).

However, despite this series having been posted before the
deadline, it feels to me like a change that doesn't come
without risk (and hence would have been better to have earlier
in the release cycle). Hence, Ian, I'd like to gather your
release manager opinion on taking this now vs postponing for
4.16 (and then still being a likely backporting candidate). My
vote is "pro", in case it matters.

Jan