linux-kernel - Re: [PATCH v3 00/13] KVM: xen: update shared_info and vcpu

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: <8527f707315812d9ac32201b37805256fab4a0a1.camel@infradead.org>
Date:   Mon, 18 Sep 2023 17:34:10 +0100
From:   David Woodhouse <dwmw2@...radead.org>
To:     Sean Christopherson <seanjc@...gle.com>,
        Paul Durrant <paul@....org>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Paul Durrant <pdurrant@...zon.com>,
        "H. Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Ingo Molnar <mingo@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>, x86@...nel.org
Subject: Re: [PATCH v3 00/13] KVM: xen: update shared_info and vcpu_info
 handling

On Mon, 2023-09-18 at 09:18 -0700, Sean Christopherson wrote:
> On Mon, Sep 18, 2023, Paul Durrant wrote:
> > From: Paul Durrant <pdurrant@...zon.com>
> > 
> > Currently we treat the shared_info page as guest memory and the VMM informs
> > KVM of its location using a GFN. However it is not guest memory as such;
> > it's an overlay page. So we pointlessly invalidate and re-cache a mapping
> > to the *same page* of memory every time the guest requests that shared_info
> > be mapped into its address space. Let's avoid doing that by modifying the
> > pfncache code to allow activation using a fixed userspace HVA as well as
> > a GPA.
> > 
> > Also, if the guest does not hypercall to explicitly set a pointer to a
> > vcpu_info in its own memory, the default vcpu_info embedded in the
> > shared_info page should be used. At the moment the VMM has to set up a
> > pointer to the structure explicitly (again treating it like it's in
> > guest memory, despite being in an overlay page). Let's also avoid the
> > need for that. We already have a cached mapping for the shared_info
> > page so just use that directly by default.
> 
> 1. Please Cc me on *all* patches if you Cc me on one patch.  I belive this is
>    the preference of the vast majority of maintainers/reviewers/contributors.
>    Having to go spelunking to find the rest of a series is annoying.
> 
> 2. Wait a reasonable amount of time between posting versions.  1 hour is not
>    reasonable.  At an *absolute minimum*, wait 1 business day.
> 
> 3. In the cover letter, summarize what's changed between versions.  Lack of a
>    summary exacerbates the problems from #1 and #2, e.g. I have a big pile of
>    mails scattered across my mailboxes, and I am effectively forced to find and
>    read them all if I want to have any clue as to why I have a 12 patch series
>    on version 3 in less than two business days.

I meant to mention that too.

> P.S. I very much appreciate that y'all are doing review publicly, thank you!

Well, to a certain extent you can't have both #2 and the P.S. Or at
least doesn't work very well if we try.

Paul and I were literally sitting in the same room last Friday talking
about this, and of course you saw the very first iteration of it on the
mailing list rather than it being done in private and then presented as
a fait accompli. We try to set that example for those around us.

(Just as you saw the very first attempt at the exit-on-hlt thing, and
the lore.kernel.org link was what I gave to my engineers to tell them
to see what happens if they try that.)

And there *are* going to be a couple of rounds of rapid review and
adjustment as we start from scratch in the open, as I firmly believe
that we should. I *want* to do it in public and I *want* you to be able
to see it, but I don't necessarily think it works for us to *wait* for
you. Maybe it makes more sense for you to dive deep into the details
only after the rapid fire round has finished?

Unless you don't *want* those first rounds to be in public? But I don't
think that's the right approach.

Suggestions welcome.

Maybe in this particular case given that I said something along the
lines of "knock something up and let's see how I feel about it when I
see it", it should be using an 'RFC' tag until I actually approve it?
Not sure how to extrapolate that to the general case, mind you.

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5965 bytes)