KVM Archive mirror
 help / color / mirror / Atom feed
From: "Li, Xin3" <xin3.li@intel.com>
To: "Gao, Chao" <chao.gao@intel.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>,
	"seanjc@google.com" <seanjc@google.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"corbet@lwn.net" <corbet@lwn.net>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"bp@alien8.de" <bp@alien8.de>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	"x86@kernel.org" <x86@kernel.org>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"shuah@kernel.org" <shuah@kernel.org>,
	"vkuznets@redhat.com" <vkuznets@redhat.com>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"Shankar, Ravi V" <ravi.v.shankar@intel.com>,
	"xin@zytor.com" <xin@zytor.com>
Subject: RE: [PATCH v2 12/25] KVM: VMX: Handle FRED event data
Date: Fri, 10 May 2024 09:36:03 +0000	[thread overview]
Message-ID: <SA1PR11MB6734740F9B6085E0997A4179A8E72@SA1PR11MB6734.namprd11.prod.outlook.com> (raw)
In-Reply-To: <ZjBiLDJ4SdQ0p5xm@chao-email>

> >+               if (kvm_is_fred_enabled(vcpu)) {
> >+                       u64 event_data = 0;
> >+
> >+                       if (is_debug(intr_info))
> >+                               /*
> >+                                * Compared to DR6, FRED #DB event data saved on
> >+                                * the stack frame have bits 4 ~ 11 and 16 ~ 31
> >+                                * inverted, i.e.,
> >+                                *   fred_db_event_data = dr6 ^ 0xFFFF0FF0UL
> >+                                */
> >+                               event_data = vcpu->arch.dr6 ^ DR6_RESERVED;
> >+                       else if (is_page_fault(intr_info))
> >+                               event_data = vcpu->arch.cr2;
> >+                       else if (is_nm_fault(intr_info))
> >+                               event_data =
> >+ to_vmx(vcpu)->fred_xfd_event_data;
> >+
> 
> IMO, deriving an event_data from CR2/DR6 is a little short-sighted because the
> event_data and CR2/DR6 __can__ be different, e.g., L1 VMM __can__ set CR2 to A
> and event_data field to B (!=A) when injecting #PF.

VMM should guarantee a FRED guest _sees_ consistent values in CR6/DR6
and event data. If not it's just a VMM bug that we need to fix.

> 
> And this approach cannot be extended to handle a (future) exception whose
> event_data isn't tied to a dedicated register like CR2/DR6.

See below.

> Adding a new field fred_xfd_event_data in struct vcpu has problems too:
> fred_xfd_event_data gets lost during migration;

I'm not bothered, because this is not hard to fix, right?

> strickly speaking, event_data is tied
> to an exception rather than a CPU. e.g., the CPU may detect a nested exception when
> delivering one and both have their own event_data.

No, don't get me wrong. An event data has to be _regenerated_ after
a nested exception is handled and the original instruction flow is
restarted. 
sometimes the original event could be gone.

We don't say an event data is tied to an exception or a CPU, which
is just confusing, or misleading.

> I think we can make event_data a property of exceptions. i.e., add a payload2 to
> struct kvm_queued_exception. and add new APIs to kvm_queue_exception* family to
> accept a payload2 and in VMX code, just program payload2 to the VMCS event_data
> field if FRED is enabled. KVM ABI should be extended as well to pass
> payload2 to userspace like how the payload is handled in
> kvm_vcpu_ioctl_x86_get/put_vcpu_events.

Yes, it's very likely that we will need to add a payload2 in future,
but NOT now. 2 reasons:

1) The first-generation FRED is designed to NOT go too far from what
   IDT can do. And FRED event data is conceptually an alias of CR2/DR6
   in the latest FRED spec (not considering xfd event data for now).
   And the existing payload is a nice match for now;

2) FRED is an extendable CPU architecture, which allows the structure
   of event data to become way bigger and complicated. Let's not assume
   anything and add a payload2 too early.

  reply	other threads:[~2024-05-10  9:36 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-07 17:26 [PATCH v2 00/25] Enable FRED with KVM VMX Xin Li
2024-02-07 17:26 ` [PATCH v2 01/25] KVM: VMX: Cleanup VMX basic information defines and usages Xin Li
2024-02-07 17:26 ` [PATCH v2 02/25] KVM: VMX: Cleanup VMX misc " Xin Li
2024-02-07 17:26 ` [PATCH v2 03/25] KVM: VMX: Add support for the secondary VM exit controls Xin Li
2024-04-19 10:21   ` Chao Gao
2024-02-07 17:26 ` [PATCH v2 04/25] KVM: x86: Mark CR4.FRED as not reserved Xin Li
2024-04-19 10:22   ` Chao Gao
2024-02-07 17:26 ` [PATCH v2 05/25] KVM: VMX: Initialize FRED VM entry/exit controls in vmcs_config Xin Li
2024-04-19 10:24   ` Chao Gao
2024-02-07 17:26 ` [PATCH v2 06/25] KVM: VMX: Defer enabling FRED MSRs save/load until after set CPUID Xin Li
2024-04-19 11:02   ` Chao Gao
2024-02-07 17:26 ` [PATCH v2 07/25] KVM: VMX: Set intercept for FRED MSRs Xin Li
2024-04-19 13:35   ` Chao Gao
2024-04-19 17:06     ` Li, Xin3
2024-02-07 17:26 ` [PATCH v2 08/25] KVM: VMX: Initialize VMCS FRED fields Xin Li
2024-04-19 14:01   ` Chao Gao
2024-04-19 17:02     ` Li, Xin3
2024-02-07 17:26 ` [PATCH v2 09/25] KVM: VMX: Switch FRED RSP0 between host and guest Xin Li
2024-04-19 14:23   ` Chao Gao
2024-04-19 16:37     ` Li, Xin3
2024-02-07 17:26 ` [PATCH v2 10/25] KVM: VMX: Add support for FRED context save/restore Xin Li
2024-04-29  6:31   ` Chao Gao
2024-02-07 17:26 ` [PATCH v2 11/25] KVM: x86: Add kvm_is_fred_enabled() Xin Li
2024-04-29  8:24   ` Chao Gao
2024-05-11  1:24     ` Li, Xin3
2024-05-11  1:53       ` Chao Gao
2024-02-07 17:26 ` [PATCH v2 12/25] KVM: VMX: Handle FRED event data Xin Li
2024-04-30  3:14   ` Chao Gao
2024-05-10  9:36     ` Li, Xin3 [this message]
2024-05-11  3:03       ` Chao Gao
2024-02-07 17:26 ` [PATCH v2 13/25] KVM: VMX: Handle VMX nested exception for FRED Xin Li
2024-04-30  7:34   ` Chao Gao
2024-02-07 17:26 ` [PATCH v2 14/25] KVM: VMX: Disable FRED if FRED consistency checks fail Xin Li
2024-04-30  8:21   ` Chao Gao
2024-02-07 17:26 ` [PATCH v2 15/25] KVM: VMX: Dump FRED context in dump_vmcs() Xin Li
2024-04-30  9:09   ` Chao Gao
2024-02-07 17:26 ` [PATCH v2 16/25] KVM: VMX: Invoke vmx_set_cpu_caps() before nested setup Xin Li
2024-02-07 17:26 ` [PATCH v2 17/25] KVM: nVMX: Add support for the secondary VM exit controls Xin Li
2024-02-07 17:26 ` [PATCH v2 18/25] KVM: nVMX: Add a prerequisite to SHADOW_FIELD_R[OW] macros Xin Li
2024-02-07 17:26 ` [PATCH v2 19/25] KVM: nVMX: Add FRED VMCS fields Xin Li
2024-02-07 17:26 ` [PATCH v2 20/25] KVM: nVMX: Add support for VMX FRED controls Xin Li
2024-02-07 17:26 ` [PATCH v2 21/25] KVM: nVMX: Add VMCS FRED states checking Xin Li
2024-02-07 17:26 ` [PATCH v2 22/25] KVM: x86: Allow FRED/LKGS/WRMSRNS to be exposed to guests Xin Li
2024-02-07 17:26 ` [PATCH v2 23/25] KVM: selftests: Run debug_regs test with FRED enabled Xin Li
2024-02-07 17:26 ` [PATCH v2 24/25] KVM: selftests: Add a new VM guest mode to run user level code Xin Li
2024-02-07 17:26 ` [PATCH v2 25/25] KVM: selftests: Add fred exception tests Xin Li
2024-03-29 20:18   ` Muhammad Usama Anjum
2024-03-29 20:18     ` Muhammad Usama Anjum
2024-04-24 16:08       ` Sean Christopherson
2024-03-27  8:08 ` [PATCH v2 00/25] Enable FRED with KVM VMX Kang, Shan
2024-04-15 17:58 ` Li, Xin3

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=SA1PR11MB6734740F9B6085E0997A4179A8E72@SA1PR11MB6734.namprd11.prod.outlook.com \
    --to=xin3.li@intel.com \
    --cc=bp@alien8.de \
    --cc=chao.gao@intel.com \
    --cc=corbet@lwn.net \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=ravi.v.shankar@intel.com \
    --cc=seanjc@google.com \
    --cc=shuah@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=vkuznets@redhat.com \
    --cc=x86@kernel.org \
    --cc=xin@zytor.com \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).