None of these are regressions afaict, so considering how late we are
in the 4.15 process, I can see reasons to not take any of these. All
of them are backporting candidates though, imo.

1: correct off-by-1 in number-of-IOMMUs check
2: leave FECTL write to vtd_resume()
3: re-order register restoring in vtd_resume()
4: restore flush hooks when disabling qinval

Jan

> From: Jan Beulich <jbeulich@suse.com>
> Sent: Thursday, March 18, 2021 6:12 PM
> 
> None of these are regressions afaict, so considering how late we are
> in the 4.15 process, I can see reasons to not take any of these. All
> of them are backporting candidates though, imo.
> 
> 1: correct off-by-1 in number-of-IOMMUs check
> 2: leave FECTL write to vtd_resume()
> 3: re-order register restoring in vtd_resume()
> 4: restore flush hooks when disabling qinval
> 

For the series:

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

On 23.03.2021 09:12, Tian, Kevin wrote:
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Thursday, March 18, 2021 6:12 PM
>>
>> None of these are regressions afaict, so considering how late we are
>> in the 4.15 process, I can see reasons to not take any of these. All
>> of them are backporting candidates though, imo.
>>
>> 1: correct off-by-1 in number-of-IOMMUs check
>> 2: leave FECTL write to vtd_resume()
>> 3: re-order register restoring in vtd_resume()
>> 4: restore flush hooks when disabling qinval
>>
> 
> For the series:
> 
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>

Thanks Kevin. Ian - what are your thoughts here towards 4.15?

Jan

Jan Beulich writes ("Re: [PATCH 0/4][4.15?] VT-d: mostly S3 related corrections"):
> Thanks Kevin. Ian - what are your thoughts here towards 4.15?

I looked at these four patches.

In general I am not sure of the implications.  There are two important
sets of implications: (i) upside: what is the bug this fixes and how
severe is that bug *in its actual impact on users of Xen* (ii) what
possible problems might there be and how have we made sure that the
patch is right ?

I want look at this not from the point of view of technical details
but in terms of user impact.  User impact is harder to predict but it
is what we actually care about.

For one of the patches it seemed obvious to me that there was very
little downside risk and the upside is not corrupting something
(perhaps something important).

For the others, all I could see, besides the general statement that
these aren't regressions, there was a lot of intensive discussion in
the commit messages of the specific technical details.  Frankly, that
all went quite over my head.

I would be prepared to give a release ack for the others if I can be
convinced of satisfactory answers to my questions (i) and (ii).  For
an idea of what kind of answer I'm looking for, see the kind of thing
Roger has been putting in his 4.15-targeted patches.  The more complex
and to-me-impenetrable the underlying technical details the more
sceptical I will be :-).

I hope that makes sense.

Thanks,
Ian.

On 23.03.2021 14:37, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH 0/4][4.15?] VT-d: mostly S3 related corrections"):
>> Thanks Kevin. Ian - what are your thoughts here towards 4.15?
> 
> I looked at these four patches.
> 
> In general I am not sure of the implications.  There are two important
> sets of implications: (i) upside: what is the bug this fixes and how
> severe is that bug *in its actual impact on users of Xen* (ii) what
> possible problems might there be and how have we made sure that the
> patch is right ?
> 
> I want look at this not from the point of view of technical details
> but in terms of user impact.  User impact is harder to predict but it
> is what we actually care about.
> 
> For one of the patches it seemed obvious to me that there was very
> little downside risk and the upside is not corrupting something
> (perhaps something important).
> 
> For the others, all I could see, besides the general statement that
> these aren't regressions, there was a lot of intensive discussion in
> the commit messages of the specific technical details.  Frankly, that
> all went quite over my head.
> 
> I would be prepared to give a release ack for the others if I can be
> convinced of satisfactory answers to my questions (i) and (ii).  For
> an idea of what kind of answer I'm looking for, see the kind of thing
> Roger has been putting in his 4.15-targeted patches.  The more complex
> and to-me-impenetrable the underlying technical details the more
> sceptical I will be :-).
> 
> I hope that makes sense.

Of course it does, and I'm sorry that I get to replying only now. Since
I didn't think I could make points towards these patches being important
enough to take at this point, I didn't see a point in making this a high
priority. Nevertheless, to address your requests:

(i) Very hard to tell. Patch 4 at least very likely is no more than a
"just in case" fix. The other two patches address issues where after
resume IOMMU faults may not work correctly anymore or where, if there
were any faults to be delivered while resuming, those may not get
delivered / handled correctly ("unpredictable behavior" is probably the
best description I could come up with). Users (admins) therefore may
not become aware of there being issues on their system / with one or
more of the guests.

(ii) I've tested that things still work on a system where S3 halfway
works. Beyond that I'm afraid it really was staring at the code that
both made me notice the issues and makes me believe things at least
aren't worse with the patches in place.

I'm intending to commit these to staging now, but not to 4.15-staging.
I'll queue them for backporting, though, as indicated previously.

Jan