Re: [PATCH v2 2/2] VT-d: avoid infinite recursion on domain_context_mapping_one() error path

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.04.2022 09:51, Roger Pau Monné wrote:
> On Thu, Apr 07, 2022 at 08:11:45AM +0200, Jan Beulich wrote:
>> Despite the comment there infinite recursion was still possible, by
>> flip-flopping between two domains. This is because prev_dom is derived
>> from the DID found in the context entry, which was already updated by
>> the time error recovery is invoked. Simply introduce yet another mode
>> flag to prevent rolling back an in-progress roll-back of a prior
>> mapping attempt.
>>
>> Also drop the existing recursion prevention for having been dead anyway:
>> Earlier in the function we already bail when prev_dom == domain.
> 
> I wonder whether it would be cleaner to stash the previous context
> entry if present and try to (re)set that one instead of recurring into
> ourselves.

I'm not sure this would be cleaner (it might be easier): The entry
may have had modifications which we want to undo by a clean
establishing of the "new" (really original) mapping. Otoh roll-back
can certainly mean simply going back to what was there. But that
would likely want to be a separate change, for likely coming with
a lot of code churn: I'd see the function gaining a two-iteration
loop then, which would mean re-indenting fair parts of it. But
maybe it could also be dealt with by other means, while still not
introducing a fake loop via adding a "goto" back to the top of the
function ...

>> Fixes: 8f41e481b485 ("VT-d: re-assign devices directly")
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.

Jan