linux-kernel - Re: [PATCH v3 0/4] Allow AVIC's IPI virtualization to be optional

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1d6044e0d71cd95c477e319d7e47819eee61a8fc.camel@redhat.com>
Date:   Wed, 04 Oct 2023 16:14:01 +0300
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     kvm@...r.kernel.org, Will Deacon <will@...nel.org>,
        linux-kernel@...r.kernel.org, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Joerg Roedel <joro@...tes.org>,
        Suravee Suthikulpanit <suravee.suthikulpanit@....com>,
        Robin Murphy <robin.murphy@....com>, iommu@...ts.linux.dev,
        Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH v3 0/4] Allow AVIC's IPI virtualization to be optional

У пн, 2023-10-02 у 12:21 -0700, Sean Christopherson пише:
> On Mon, Oct 02, 2023, Maxim Levitsky wrote:
> > Hi!
> > 
> > This patch allows AVIC's ICR emulation to be optional and thus allows
> > to workaround AVIC's errata #1235 by disabling this portion of the feature.
> > 
> > This is v3 of my patch series 'AVIC bugfixes and workarounds' including
> > review feedback.
> 
> Please respond to my idea[*] instead of sending more patches. 

Hi,

For the v2 of the patch I was already on the fence if to do it this way or to refactor
the code, and back when I posted it, I decided still to avoid the refactoring.

However, your idea of rewriting this patch, while it does change less lines of code,
is even less obvious and consequently required you to write even longer comment to 
justify it which is not a good sign.

In particular I don't want someone to find out later, and in the hard way that sometimes
real physid table is accessed, and sometimes a fake copy of it is.

So I decided to fix the root cause by not reading the physid table back,
which made the code cleaner, and even with the workaround the code 
IMHO is still simpler than it was before.

About the added 'vcpu->loaded' variable, I added it also because it is something that is 
long overdue to be added, I remember that in IPIv code there was also a need for this, 
and probalby more places in KVM can be refactored to take advantage of it,
instead of various hacks.

I did adopt your idea of using 'enable_ipiv', although I am still not 100% sure that this
is more readable than 'avic_zen2_workaround'.

Best regards,
	Maxim Levitsky

>  I'm not opposed to
> a different approach, but we need to have an actual discussion around the pros and
> cons, and hopefully come to an agreement.  This cover letter doesn't even acknowledge
> that there is an alternative proposal, let alone justify why the vcpu->loaded
> approach was taken.
> 
> [*] https://lore.kernel.org/all/ZRYxPNeq1rnp-M0f@google.com
>