[PATCH v3 0/6] x86/intr: HVM guest interrupt handling fixes/cleanup

Roger Pau Monne posted 6 patches 3 years, 2 months ago
Test gitlab-ci passed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20210126134521.25784-1-roger.pau@citrix.com
xen/arch/x86/hvm/vioapic.c            | 19 ++++++
xen/arch/x86/hvm/vpic.c               | 36 ++++++++--
xen/drivers/passthrough/vtd/x86/hvm.c |  3 -
xen/drivers/passthrough/x86/hvm.c     | 95 +--------------------------
xen/include/asm-x86/hvm/irq.h         |  3 -
xen/include/xen/iommu.h               |  5 --
6 files changed, 52 insertions(+), 109 deletions(-)
[PATCH v3 0/6] x86/intr: HVM guest interrupt handling fixes/cleanup
Posted by Roger Pau Monne 3 years, 2 months ago
Hello,

The following series aims to fix some shortcomings of guest interrupt
handling when using passthrough devices. The first 5 patches are
bugfixes or cleanups, which I think should solve the issue(s) that
caused the dpci EOI timer to be introduced. However neither me nor
others seem to be able to reproduce the original issue, so it's hard to
tell.

It's my opinion that we should remove the timer and see what explodes
(if anything). That's the only way we will be able to figure out what
the original issue was, and how to fix it without introducing yet
another per-guest-irq related timer.

Thanks, Roger.

Roger Pau Monne (6):
  x86/vioapic: top word redir entry writes don't trigger interrupts
  x86/vioapic: issue EOI to dpci when switching pin to edge trigger mode
  x86/vpic: force int output to low when in init mode
  x86/vpic: don't trigger unmask event until end of init
  x86/vpic: issue dpci EOI for cleared pins at ICW1
  x86/dpci: remove the dpci EOI timer

 xen/arch/x86/hvm/vioapic.c            | 19 ++++++
 xen/arch/x86/hvm/vpic.c               | 36 ++++++++--
 xen/drivers/passthrough/vtd/x86/hvm.c |  3 -
 xen/drivers/passthrough/x86/hvm.c     | 95 +--------------------------
 xen/include/asm-x86/hvm/irq.h         |  3 -
 xen/include/xen/iommu.h               |  5 --
 6 files changed, 52 insertions(+), 109 deletions(-)

-- 
2.29.2


Re: [PATCH v3 0/6] x86/intr: HVM guest interrupt handling fixes/cleanup
Posted by Jan Beulich 3 years, 2 months ago
Ian,

On 26.01.2021 14:45, Roger Pau Monne wrote:
> The following series aims to fix some shortcomings of guest interrupt
> handling when using passthrough devices. The first 5 patches are
> bugfixes or cleanups, which I think should solve the issue(s) that
> caused the dpci EOI timer to be introduced. However neither me nor
> others seem to be able to reproduce the original issue, so it's hard to
> tell.
> 
> It's my opinion that we should remove the timer and see what explodes
> (if anything). That's the only way we will be able to figure out what
> the original issue was, and how to fix it without introducing yet
> another per-guest-irq related timer.
> 
> Thanks, Roger.
> 
> Roger Pau Monne (6):
>   x86/vioapic: top word redir entry writes don't trigger interrupts
>   x86/vioapic: issue EOI to dpci when switching pin to edge trigger mode
>   x86/vpic: force int output to low when in init mode
>   x86/vpic: don't trigger unmask event until end of init
>   x86/vpic: issue dpci EOI for cleared pins at ICW1
>   x86/dpci: remove the dpci EOI timer

while half of this series was still submitted in time, I'd still
like to raise the question of including part or all of it in
4.15. In particular the last change is one which I would prefer
to see happen early in a release cycle. Risk assessment is
pretty difficult, I'm afraid (Roger can correct me here), as at
least some of what gets adjusted are cases we don't normally
expect to be exercised. (FAOD patch 5 is still pending a R-b
tag.)

Roger, if you could give your own judgement on which of the
changes you would view as more or less clear 4.15 candidates,
this may help Ian take a decision.

Jan

Re: [PATCH v3 0/6] x86/intr: HVM guest interrupt handling fixes/cleanup
Posted by Roger Pau Monné 3 years, 2 months ago
On Tue, Jan 26, 2021 at 06:07:23PM +0100, Jan Beulich wrote:
> Ian,
> 
> On 26.01.2021 14:45, Roger Pau Monne wrote:
> > The following series aims to fix some shortcomings of guest interrupt
> > handling when using passthrough devices. The first 5 patches are
> > bugfixes or cleanups, which I think should solve the issue(s) that
> > caused the dpci EOI timer to be introduced. However neither me nor
> > others seem to be able to reproduce the original issue, so it's hard to
> > tell.
> > 
> > It's my opinion that we should remove the timer and see what explodes
> > (if anything). That's the only way we will be able to figure out what
> > the original issue was, and how to fix it without introducing yet
> > another per-guest-irq related timer.
> > 
> > Thanks, Roger.
> > 
> > Roger Pau Monne (6):
> >   x86/vioapic: top word redir entry writes don't trigger interrupts
> >   x86/vioapic: issue EOI to dpci when switching pin to edge trigger mode
> >   x86/vpic: force int output to low when in init mode
> >   x86/vpic: don't trigger unmask event until end of init
> >   x86/vpic: issue dpci EOI for cleared pins at ICW1
> >   x86/dpci: remove the dpci EOI timer
> 
> while half of this series was still submitted in time, I'd still
> like to raise the question of including part or all of it in
> 4.15. In particular the last change is one which I would prefer
> to see happen early in a release cycle. Risk assessment is
> pretty difficult, I'm afraid (Roger can correct me here), as at
> least some of what gets adjusted are cases we don't normally
> expect to be exercised. (FAOD patch 5 is still pending a R-b
> tag.)
> 
> Roger, if you could give your own judgement on which of the
> changes you would view as more or less clear 4.15 candidates,
> this may help Ian take a decision.

I agree, the only risky patch is the last one, mainly because we have
no way to reproduce the issue that code was fixing. It's my assumption
that all the fixes prior to the last patch should address the same
issues the timer was trying to address.

So my recommendation would be to not commit the last patch unless all
the prior ones have been committed, and that would include 5/6 which
is still missing a R-b.

Thanks, Roger.