From: Maxim Levitsky <mlevitsk@redhat.com>
To: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Cc: pbonzini@redhat.com, seanjc@google.com, joro@8bytes.org,
jon.grimm@amd.com, wei.huang2@amd.com, terry.bowman@amd.com
Subject: Re: [RFC PATCH 11/13] KVM: SVM: Add logic to switch between APIC and x2APIC virtualization mode
Date: Thu, 24 Feb 2022 22:03:28 +0200 [thread overview]
Message-ID: <5f3b7d10e63126073fa4c17ba4e095b0fa0795e8.camel@redhat.com> (raw)
In-Reply-To: <20220221021922.733373-12-suravee.suthikulpanit@amd.com>
On Sun, 2022-02-20 at 20:19 -0600, Suravee Suthikulpanit wrote:
> When an AVIC-enabled guest switch from APIC to x2APIC mode during runtime,
> the SVM driver needs to
>
> 1. Set the x2APIC mode bit for AVIC in VMCB along with the maximum
> APIC ID support for each mode accodingly.
>
> 2. Disable x2APIC MSRs interception in order to allow the hardware
> to virtualize x2APIC MSRs accesses.
>
> This is currently handled in the svm_refresh_apicv_exec_ctrl().
>
> Note that guest kerenel does not need to disable APIC before swtiching
> to x2APIC. Therefore the WARN_ON in vcpu_load() to check if the vCPU is
> currently running is no longer appropriate.
>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
> arch/x86/kvm/svm/avic.c | 61 +++++++++++++++++++++++++++++++++++++----
> 1 file changed, 55 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index 3543b7a4514a..3306b74f1d8b 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -79,6 +79,50 @@ static inline enum avic_modes avic_get_vcpu_apic_mode(struct vcpu_svm *svm)
> return AVIC_MODE_NONE;
> }
>
> +static inline void avic_set_x2apic_msr_interception(struct vcpu_svm *svm, bool disable)
> +{
> + int i;
> +
> + for (i = 0x800; i <= 0x8ff; i++)
> + set_msr_interception(&svm->vcpu, svm->msrpm, i,
> + !disable, !disable);
> +}
> +
> +void avic_activate_vmcb(struct vcpu_svm *svm)
> +{
> + struct vmcb *vmcb = svm->vmcb01.ptr;
> +
> + vmcb->control.int_ctl |= AVIC_ENABLE_MASK;
> +
> + if (svm->x2apic_enabled) {
Use apic_x2apic_mode here as well
> + vmcb->control.int_ctl |= X2APIC_MODE_MASK;
> + vmcb->control.avic_physical_id &= ~X2AVIC_MAX_PHYSICAL_ID;
> + vmcb->control.avic_physical_id |= X2AVIC_MAX_PHYSICAL_ID;
Why not just use
phys_addr_t ppa = __sme_set(page_to_phys(kvm_svm->avic_physical_id_table_page));;
vmcb->control.avic_physical_id = ppa | X2AVIC_MAX_PHYSICAL_ID;
> + /* Disabling MSR intercept for x2APIC registers */
> + avic_set_x2apic_msr_interception(svm, false);
> + } else {
> + vmcb->control.avic_physical_id &= ~AVIC_MAX_PHYSICAL_ID;
> + vmcb->control.avic_physical_id |= AVIC_MAX_PHYSICAL_ID;
Same here....
> + /* Enabling MSR intercept for x2APIC registers */
> + avic_set_x2apic_msr_interception(svm, true);
> + }
> +}
> +
> +void avic_deactivate_vmcb(struct vcpu_svm *svm)
> +{
> + struct vmcb *vmcb = svm->vmcb01.ptr;
> +
> + vmcb->control.int_ctl &= ~(AVIC_ENABLE_MASK | X2APIC_MODE_MASK);
> +
> + if (svm->x2apic_enabled)
> + vmcb->control.avic_physical_id &= ~X2AVIC_MAX_PHYSICAL_ID;
> + else
> + vmcb->control.avic_physical_id &= ~AVIC_MAX_PHYSICAL_ID;
> +
> + /* Enabling MSR intercept for x2APIC registers */
> + avic_set_x2apic_msr_interception(svm, true);
> +}
> +
> /* Note:
> * This function is called from IOMMU driver to notify
> * SVM to schedule in a particular vCPU of a particular VM.
> @@ -195,13 +239,12 @@ void avic_init_vmcb(struct vcpu_svm *svm)
> vmcb->control.avic_backing_page = bpa & AVIC_HPA_MASK;
> vmcb->control.avic_logical_id = lpa & AVIC_HPA_MASK;
> vmcb->control.avic_physical_id = ppa & AVIC_HPA_MASK;
> - vmcb->control.avic_physical_id |= AVIC_MAX_PHYSICAL_ID;
> vmcb->control.avic_vapic_bar = APIC_DEFAULT_PHYS_BASE & VMCB_AVIC_APIC_BAR_MASK;
>
> if (kvm_apicv_activated(svm->vcpu.kvm))
> - vmcb->control.int_ctl |= AVIC_ENABLE_MASK;
> + avic_activate_vmcb(svm);
Why not set AVIC_ENABLE_MASK in avic_activate_vmcb ?
> else
> - vmcb->control.int_ctl &= ~AVIC_ENABLE_MASK;
> + avic_deactivate_vmcb(svm);
> }
>
> static u64 *avic_get_physical_id_entry(struct kvm_vcpu *vcpu,
> @@ -657,6 +700,13 @@ void avic_update_vapic_bar(struct vcpu_svm *svm, u64 data)
> svm->x2apic_enabled ? "x2APIC" : "xAPIC");
> vmcb_mark_dirty(svm->vmcb, VMCB_AVIC);
> kvm_vcpu_update_apicv(&svm->vcpu);
> +
> + /*
> + * The VM could be running w/ AVIC activated switching from APIC
> + * to x2APIC mode. We need to all refresh to make sure that all
> + * x2AVIC configuration are being done.
> + */
> + svm_refresh_apicv_exec_ctrl(&svm->vcpu);
That also should be done in avic_set_virtual_apic_mode instead, but otherwise should be fine.
Also it seems that .avic_set_virtual_apic_mode will cover you on the case when x2apic is disabled
in the guest cpuid - kvm_set_apic_base checks if the guest cpuid has x2apic support and refuses
to enable it if it is not set.
But still a WARN_ON_ONCE won't hurt to see that you are not enabling x2avic when not supported.
> }
>
> void svm_set_virtual_apic_mode(struct kvm_vcpu *vcpu)
> @@ -722,9 +772,9 @@ void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
> * accordingly before re-activating.
> */
> avic_post_state_restore(vcpu);
> - vmcb->control.int_ctl |= AVIC_ENABLE_MASK;
> + avic_activate_vmcb(svm);
> } else {
> - vmcb->control.int_ctl &= ~AVIC_ENABLE_MASK;
> + avic_deactivate_vmcb(svm);
> }
> vmcb_mark_dirty(vmcb, VMCB_AVIC);
>
> @@ -1019,7 +1069,6 @@ void avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> return;
>
> entry = READ_ONCE(*(svm->avic_physical_id_cache));
> - WARN_ON(entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK);
Why?
>
> entry &= ~AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK;
> entry |= (h_physical_id & AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK);
Best regards,
Maxim Levitsky
next prev parent reply other threads:[~2022-02-24 20:03 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-21 2:19 [RFC PATCH 00/13] Introducing AMD x2APIC Virtualization (x2AVIC) support Suravee Suthikulpanit
2022-02-21 2:19 ` [RFC PATCH 01/13] KVM: SVM: Add warning when encounter invalid APIC ID Suravee Suthikulpanit
2022-02-24 16:30 ` Maxim Levitsky
2022-02-21 2:19 ` [RFC PATCH 02/13] x86/cpufeatures: Introduce x2AVIC CPUID bit Suravee Suthikulpanit
2022-02-24 16:32 ` Maxim Levitsky
2022-02-21 2:19 ` [RFC PATCH 03/13] KVM: SVM: Detect X2APIC virtualization (x2AVIC) support Suravee Suthikulpanit
2022-02-24 16:52 ` Maxim Levitsky
2022-03-01 9:45 ` Suravee Suthikulpanit
2022-02-21 2:19 ` [RFC PATCH 04/13] KVM: SVM: Only call vcpu_(un)blocking when AVIC is enabled Suravee Suthikulpanit
2022-02-24 16:54 ` Maxim Levitsky
2022-03-01 9:59 ` Suravee Suthikulpanit
2022-02-21 2:19 ` [RFC PATCH 05/13] KVM: SVM: Update max number of vCPUs supported for x2AVIC mode Suravee Suthikulpanit
2022-02-24 17:18 ` Maxim Levitsky
2022-03-01 10:47 ` Suravee Suthikulpanit
2022-03-01 11:31 ` Maxim Levitsky
2022-02-21 2:19 ` [RFC PATCH 06/13] KVM: SVM: Add logic to determine x2APIC mode Suravee Suthikulpanit
2022-02-24 17:29 ` Maxim Levitsky
2022-03-03 2:12 ` Suthikulpanit, Suravee
2022-03-03 13:12 ` Suravee Suthikulpanit
2022-02-21 2:19 ` [RFC PATCH 07/13] KVM: SVM: Update avic_kick_target_vcpus to support 32-bit APIC ID Suravee Suthikulpanit
2022-02-24 17:35 ` Maxim Levitsky
2022-03-03 14:41 ` Suravee Suthikulpanit
2022-02-21 2:19 ` [RFC PATCH 08/13] KVM: SVM: Do not update logical APIC ID table when in x2APIC mode Suravee Suthikulpanit
2022-02-24 17:41 ` Maxim Levitsky
2022-03-08 5:24 ` Suthikulpanit, Suravee
2022-02-21 2:19 ` [RFC PATCH 09/13] KVM: SVM: Introduce helper function avic_get_apic_id Suravee Suthikulpanit
2022-02-24 19:46 ` Maxim Levitsky
2022-02-21 2:19 ` [RFC PATCH 10/13] KVM: SVM: Adding support for configuring x2APIC MSRs interception Suravee Suthikulpanit
2022-02-24 19:51 ` Maxim Levitsky
2022-03-07 10:14 ` Suthikulpanit, Suravee
2022-02-21 2:19 ` [RFC PATCH 11/13] KVM: SVM: Add logic to switch between APIC and x2APIC virtualization mode Suravee Suthikulpanit
2022-02-22 5:39 ` Suthikulpanit, Suravee
2022-02-24 20:03 ` Maxim Levitsky [this message]
2022-03-04 11:22 ` Suravee Suthikulpanit
2022-03-04 11:51 ` Maxim Levitsky
2022-02-21 2:19 ` [RFC PATCH 12/13] KVM: SVM: Remove APICv inhibit reasone due to x2APIC Suravee Suthikulpanit
2022-02-24 20:06 ` Maxim Levitsky
2022-03-01 14:02 ` Suravee Suthikulpanit
2022-02-21 2:19 ` [RFC PATCH 13/13] KVM: SVM: Use fastpath x2apic IPI emulation when #vmexit with x2AVIC Suravee Suthikulpanit
2022-02-24 20:12 ` Maxim Levitsky
2022-03-07 6:24 ` Suthikulpanit, Suravee
2022-02-22 5:37 ` [RFC PATCH 00/13] Introducing AMD x2APIC Virtualization (x2AVIC) support Suthikulpanit, Suravee
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=5f3b7d10e63126073fa4c17ba4e095b0fa0795e8.camel@redhat.com \
--to=mlevitsk@redhat.com \
--cc=jon.grimm@amd.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=suravee.suthikulpanit@amd.com \
--cc=terry.bowman@amd.com \
--cc=wei.huang2@amd.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 NNTP newsgroup(s).