mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>,
	Nathan Chancellor <nathan@kernel.org>,
	kvm@vger.kernel.org, Jim Mattson <jmattson@google.com>,
	Ben Serebrin <serebrin@google.com>,
	Peter Shier <pshier@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Maxim Levitsky <mlevitsk@redhat.com>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] kvm: vmx: Add IA32_FLUSH_CMD guest support
Date: Mon, 20 Mar 2023 17:48:49 +0100	[thread overview]
Message-ID: <c83cb646-ce29-5397-aa1b-4a26f92a6102@redhat.com> (raw)
In-Reply-To: <ZBiIt2LBoogxQ2jP@google.com>



Am 20/03/2023 um 17:24 schrieb Sean Christopherson:
> On Mon, Mar 20, 2023, Emanuele Giuseppe Esposito wrote:
>>
>> Am 20/03/2023 um 15:53 schrieb Sean Christopherson:
>>> The patches obviously weren't tested,
>> Well... no. They were tested. Call it wrongly tested, badly tested,
>> whatever you want but don't say "obviously weren't tested".
> 
> Heh, depends on how you define "tested".  I was defining tested as "tested to
> work as expected on systems with and without support for IA32_FLUSH_CMD".
> 
> But yeah, I should have said "properly tested".
> 
>> I even asked you in a private email why the cpu flag was visible in Linux and
>> not in rhel when using the same machine.
>>
>> So again, my bad with these patches, I sincerely apologize but I would
>> prefer that you think I don't know how to test this stuff rather than
>> say that I carelessly sent something without checking :)
> 
> I didn't intend to imply that you didn't try to do the right thing, nor am I
> unhappy with you personally.  My apologies if my response came off that way.
> 
> What I am most grumpy about is that this series was queued without tests.  E.g.
> unless there's a subtlety I'm missing, a very basic KVM-Unit-Test to verify that
> the guest can write MSR_IA32_FLUSH_CMD with L1D_FLUSH when the MSR is supported
> would have caught this bug.  One of the reasons for requiring actual testcases is
> that dedicated testcases reduce the probability of "testing gone wrong", e.g. a
> TEST_SKIPPED would have alerted you that the KVM code wasn't actually being exercised.
> 
Yeah, I should have added a test. I see what you mean.

Anyways, as the cover letter said patches 1-2 are both unnecessary and
taken from an old past serie that was left unanswered (that's why I
thought it was lost).
What mainly interested me was patch 3, ie advertising FLUSH_L1D to user
space. As far as I understand, that looks good to you, right?

I'll be happy to do the exercise and resend all three patches plus an
unit test to verify it works if you want. But if you think they are
useless, just drop the first two and take only the third.

As always, I appreciate you&Paolo&others feedback :)

Let me know what you think.

Thank you,
Emanuele


  reply	other threads:[~2023-03-20 17:04 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-01 13:29 [PATCH 0/3] KVM: support the cpu feature FLUSH_L1D Emanuele Giuseppe Esposito
2023-02-01 13:29 ` [PATCH 1/3] kvm: vmx: Add IA32_FLUSH_CMD guest support Emanuele Giuseppe Esposito
2023-03-17 19:04   ` Nathan Chancellor
2023-03-17 22:53     ` Pawan Gupta
2023-03-17 23:14       ` Nathan Chancellor
2023-03-17 23:59         ` Pawan Gupta
2023-03-20 14:53           ` Sean Christopherson
2023-03-20 15:40             ` Emanuele Giuseppe Esposito
2023-03-20 16:24               ` Sean Christopherson
2023-03-20 16:48                 ` Emanuele Giuseppe Esposito [this message]
2023-03-21 23:59             ` Sean Christopherson
2023-02-01 13:29 ` [PATCH 2/3] kvm: svm: " Emanuele Giuseppe Esposito
2023-02-01 13:29 ` [PATCH 3/3] kvm: x86: Advertise FLUSH_L1D to user space Emanuele Giuseppe Esposito
2023-03-14 13:29 ` [PATCH 0/3] KVM: support the cpu feature FLUSH_L1D Paolo Bonzini
2023-03-20 16:52 ` Jim Mattson
2023-03-21  8:40   ` Emanuele Giuseppe Esposito
2023-03-21  9:43   ` Paolo Bonzini
2023-03-21 18:30     ` Jim Mattson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c83cb646-ce29-5397-aa1b-4a26f92a6102@redhat.com \
    --to=eesposit@redhat.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=mlevitsk@redhat.com \
    --cc=nathan@kernel.org \
    --cc=pawan.kumar.gupta@linux.intel.com \
    --cc=pbonzini@redhat.com \
    --cc=pshier@google.com \
    --cc=seanjc@google.com \
    --cc=serebrin@google.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®