On Sun, Sep 24, 2023 at 11:09 PM Mingwei Zhang <mizhang@xxxxxxxxxx> wrote:
>
> Hi Sean,
>
> On Fri, Sep 22, 2023 at 4:00 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> >
> > On Fri, Sep 22, 2023, Mingwei Zhang wrote:
> > > So yes, they could be put together and they could be put separately.
> > > But I don't see why they _cannot_ be together or cause confusion.
> >
> > Because they don't need to be put together. Roman's patch kinda sorta overlaps
> > with the prev_counter mess, but Jim's fixes are entirely orthogonal.
> >
> > If one person initially posted such a series with everything together I probably
> > wouldn't care *too* much, but combining patches and/or series that aren't tightly
> > coupled or dependent in some way usually does more harm than good. E.g. if a
> > maintainer has complaints against only one or two patches in series of unrelated
> > patches, then grabbing the "good" patches is unnecessarily difficult. It's not
> > truly hard on the maintainer's end, but little bits of avoidable friction in the
> > process adds up across hundreds and thousands of patches.
> >
> > FWIW, my plan is to apply Roman's patch pretty much as-is, grab v2 from Jim, and
> > post my cleanups as a separate series on top (maybe two series, really haven't
> > thought about it yet). The only reason I have them all in a single branch is
> > because there are code conflicts and I know I will apply the patches from Roman
> > and Jim first, i.e. I didn't want to develop on a base that I knew would become
> > stale.
> >
> > > So, I would like to put them together in the same context with a cover letter
> > > fully describing the details.
> >
> > I certainly won't object to a thorough bug report/analysis, but I'd prefer that
> > Jim's series be posted separately (though I don't care if it's you or Jim that
> > posts it).
>
> Thanks for agreeing to put things together. In fact, everything
> together means all relevant fix patches for the same bug need to be
> together. But I will put my patch explicitly as _optional_ mentioned
> in the cover letter.
>
> If the series causes inconvenience, please accept my apology. For the
> sense of responsibility, I think I could just use this opportunity to
> send my updated version with your comment fixed. I will also use this
> chance to update your fix to Jim's patches.
>
> One last thing, breaking the kvm-unit-test/pmu still surprises me.
> Please test it again when you have a chance. Maybe adding more fixes
> on top. With the series sent, I will hand it over to you.
>
Never, this is a test failure that we already solved internally.
Applying the following fix to kvm-unit-tests/pmu remove the failures:
diff --git a/x86/pmu.c b/x86/pmu.c
index 0def2869..667e6233 100644
--- a/x86/pmu.c
+++ b/x86/pmu.c
@@ -68,6 +68,7 @@ volatile uint64_t irq_received;
static void cnt_overflow(isr_regs_t *regs)
{
irq_received++;
+ apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);
apic_write(APIC_EOI, 0);
}
Since KVM vPMU adds a mask when injecting the PMI, it is the
responsibility of the guest PMI handler to remove the mask and allow
subsequent PMIs delivered.
We should upstream the above fix some time.
Thanks.
-Mingwei
