> From: Roger Pau Monné <roger....@citrix.com> > Sent: Tuesday, March 24, 2020 6:47 PM > > On Tue, Mar 24, 2020 at 08:49:27AM +0000, Tian, Kevin wrote: > > > From: Jan Beulich <jbeul...@suse.com> > > > Sent: Tuesday, March 24, 2020 4:10 PM > > > > > > On 24.03.2020 06:41, Tian, Kevin wrote: > > > >> From: Roger Pau Monné <roger....@citrix.com> > > > >> Sent: Monday, March 23, 2020 10:49 PM > > > >> > > > >> On Mon, Mar 23, 2020 at 09:09:59AM +0100, Jan Beulich wrote: > > > >>> On 20.03.2020 20:07, Roger Pau Monne wrote: > > > >>>> This reverts commit f96e1469ad06b61796c60193daaeb9f8a96d7458. > > > >>>> > > > >>>> The commit is wrong, as the whole point of nvmx_update_apicv is > to > > > >>>> update the guest interrupt status field when the Ack on exit VMEXIT > > > >>>> control feature is enabled. > > > >>>> > > > >>>> Signed-off-by: Roger Pau Monné <roger....@citrix.com> > > > >>> > > > >>> Before anyone gets to look at the other two patches, should this > > > >>> be thrown in right away? > > > >> > > > >> I would like if possible get a confirmation from Kevin (or anyone > > > >> else) that my understanding is correct. I find the nested code very > > > >> confusing, and I've already made a mistake while trying to fix it. > > > >> That being said, this was spotted by osstest as introducing a > > > >> regression, so I guess it's safe to just toss it in now. > > > >> > > > >> FWIW patch 2/3 attempts to provide a description of my > understanding > > > >> of how nvmx_update_apicv works. > > > >> > > > > > > > > I feel it is not good to take this patch alone, as it was introduced to > > > > fix > > > > another problem. W/o understanding whether the whole series can > > > > fix both old and new problems, we may risk putting nested interrupt > > > > logic in an even worse state... > > > > > > Well, okay, I'll wait then, but it would seem to me that reverting > > > wouldn't put us in a worse state than we were in before that change > > > was put in. > > > > Roger needs to make the call, i.e. which problem is more severe, old or > > new one. > > AFAICT those problems affect different kind of hardware, so it doesn't > make much difference. IMO f96e1469ad06b6 should be reverted because > it's plain wrong, and albeit it seemed to fix the issue in one of my > boxes it was just luck. > > I don't thinks it's going to make matters worse, as osstest is already > blocked, but reverting it alone without the rest of the series is not > going to unblock osstest either. If you agree it's wrong I think it > could go in now, even if it's just to avoid having to repost it with > newer versions of the series. >
yes, I agree it's wrong. Reviewed-by: Kevin Tian <kevin.t...@intel.com>
