[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 2/8] x86/HVM/SVM: Add AVIC initialization code
- To: Jan Beulich <JBeulich@xxxxxxxx>, Janakarajan Natarajan <Janakarajan.Natarajan@xxxxxxx>
- From: "Natarajan, Janakarajan" <jnataraj@xxxxxxx>
- Date: Thu, 26 Apr 2018 10:32:05 -0500
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=Janakarajan.Natarajan@xxxxxxx;
- Cc: Tim Deegan <tim@xxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wei.liu2@xxxxxxxxxx>, Jun Nakajima <jun.nakajima@xxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxx, Julien Grall <julien.grall@xxxxxxx>, Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
- Delivery-date: Thu, 26 Apr 2018 15:32:47 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
On 4/16/2018 10:55 AM, Jan Beulich wrote:
On 04.04.18 at 01:01, <Janakarajan.Natarajan@xxxxxxx> wrote:
From: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
Introduce AVIC base initialization code. This includes:
* Setting up per-VM data structures.
* Setting up per-vCPU data structure.
* Initializing AVIC-related VMCB bit fields.
This patch also introduces a new Xen parameter (svm-avic),
which can be used to enable/disable AVIC support.
This sentence looks stale/misplaced now.
--- /dev/null
+++ b/xen/arch/x86/hvm/svm/avic.c
@@ -0,0 +1,191 @@
+/*
+ * avic.c: implements AMD Advanced Virtual Interrupt Controller (AVIC) support
+ * Copyright (c) 2016, Advanced Micro Devices, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/domain_page.h>
+#include <xen/sched.h>
+#include <xen/stdbool.h>
+#include <asm/acpi.h>
+#include <asm/apicdef.h>
+#include <asm/atomic.h>
+#include <asm/event.h>
+#include <asm/hvm/emulate.h>
+#include <asm/hvm/nestedhvm.h>
+#include <asm/hvm/support.h>
+#include <asm/hvm/svm/avic.h>
+#include <asm/hvm/vlapic.h>
+#include <asm/p2m.h>
+#include <asm/page.h>
+
+/*
+ * Note: Current max index allowed for physical APIC ID table is 255.
+ */
This is a single line comment.
+#define AVIC_PHY_APIC_ID_MAX 0xFF
Is this really an AVIC-specific constant, rather than e.g.
GET_xAPIC_ID(APIC_ID_MASK)?
+#define AVIC_VAPIC_BAR_MASK (((1ULL << 40) - 1) << PAGE_SHIFT)
+
+/*
+ * Note:
+ * Currently, svm-avic mode is not supported with nested virtualization.
+ * Therefore, it is not yet currently enabled by default. Once the support
+ * is in-place, this should be enabled by default.
+ */
+bool svm_avic = 0;
false (or simply omit the intializer)
+int svm_avic_dom_init(struct domain *d)
+{
+ int ret = 0;
+ struct page_info *pg;
+
+ if ( !svm_avic || !has_vlapic(d) )
+ return 0;
+
+ /*
+ * Note:
+ * AVIC hardware walks the nested page table to check permissions,
+ * but does not use the SPA address specified in the leaf page
+ * table entry since it uses address in the AVIC_BACKING_PAGE pointer
+ * field of the VMCB. Therefore, we set up a dummy page for APIC _mfn(0).
+ */
+ set_mmio_p2m_entry(d, paddr_to_pfn(APIC_DEFAULT_PHYS_BASE),
+ _mfn(0), PAGE_ORDER_4K,
+ p2m_get_hostp2m(d)->default_access);
The use of MFN 0 here looks risky to me: How do you guarantee nothing else
might ever use that P2M entry?
Do you have any suggestions for an alternative to MFN 0 that is
guaranteed to never be used?
Thanks,
Janak
+ /* Init AVIC logical APIC ID table */
+ pg = alloc_domheap_page(d, MEMF_no_owner);
+ if ( !pg )
+ {
+ ret = -ENOMEM;
+ goto err_out;
+ }
+ clear_domain_page(_mfn(page_to_mfn(pg)));
+ d->arch.hvm_domain.svm.avic_logical_id_table_pg = pg;
+ d->arch.hvm_domain.svm.avic_logical_id_table =
__map_domain_page_global(pg);
Both here and ...
+ /* Init AVIC physical APIC ID table */
+ pg = alloc_domheap_page(d, MEMF_no_owner);
+ if ( !pg )
+ {
+ ret = -ENOMEM;
+ goto err_out;
+ }
+ clear_domain_page(_mfn(page_to_mfn(pg)));
+ d->arch.hvm_domain.svm.avic_physical_id_table_pg = pg;
+ d->arch.hvm_domain.svm.avic_physical_id_table =
__map_domain_page_global(pg);
... here: Do you really need to store both page pointer and map address?
+void svm_avic_dom_destroy(struct domain *d)
+{
+ if ( !svm_avic || !has_vlapic(d) )
+ return;
+
+ if ( d->arch.hvm_domain.svm.avic_physical_id_table )
+ {
+
unmap_domain_page_global(d->arch.hvm_domain.svm.avic_physical_id_table);
+ free_domheap_page(d->arch.hvm_domain.svm.avic_physical_id_table_pg);
+ d->arch.hvm_domain.svm.avic_physical_id_table_pg = 0;
+ d->arch.hvm_domain.svm.avic_physical_id_table = 0;
DYM NULL?
+int svm_avic_init_vmcb(struct vcpu *v)
+{
+ u32 apic_id;
+ unsigned long tmp;
+ struct arch_svm_struct *s = &v->arch.hvm_svm;
+ struct vmcb_struct *vmcb = s->vmcb;
+ struct svm_domain *d = &v->domain->arch.hvm_domain.svm;
+ const struct vlapic *vlapic = vcpu_vlapic(v);
+ struct avic_physical_id_entry *entry = (struct avic_physical_id_entry
*)&tmp;
+
+ if ( !svm_avic || !has_vlapic(v->domain) )
+ return 0;
+
+ if ( !vlapic || !vlapic->regs_page )
+ return -EINVAL;
+
+ apic_id = vlapic_read_aligned(vcpu_vlapic(v), APIC_ID);
+ s->avic_last_phy_id = avic_get_physical_id_entry(d, GET_xAPIC_ID(apic_id));
+ if ( !s->avic_last_phy_id )
+ return -EINVAL;
+
+ vmcb->avic_bk_pg_pa = page_to_maddr(vlapic->regs_page);
+ vmcb->avic_logical_id_table_pa =
domain_page_map_to_mfn(d->avic_logical_id_table) << PAGE_SHIFT;
+ vmcb->avic_physical_id_table_pa =
domain_page_map_to_mfn(d->avic_physical_id_table) << PAGE_SHIFT;
DYM something like mfn_to_maddr()? Please don't open-code things.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
