Re: [Xen-devel] [RFC PATCH 5/9] x86/HVM/SVM: Add AVIC initialization code

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

. snip..

> diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
> new file mode 100644
> index 0000000..70bac69
> --- /dev/null
> +++ b/xen/arch/x86/hvm/svm/avic.c
> @@ -0,0 +1,217 @@
> +#include <xen/domain_page.h>
> +#include <xen/sched.h>
> +#include <xen/stdbool.h>
> +#include <asm/acpi.h>
> +#include <asm/apicdef.h>
> +#include <asm/event.h>
> +#include <asm/p2m.h>
> +#include <asm/page.h>
> +#include <asm/hvm/nestedhvm.h>
> +#include <asm/hvm/svm/avic.h>
> +#include <asm/hvm/vlapic.h>
> +#include <asm/hvm/emulate.h>
> +#include <asm/hvm/support.h>
> +
> +/* NOTE: Current max index allowed for physical APIC ID table is 255 */
> +#define AVIC_PHY_APIC_ID_MAX    0xFF
> +
> +#define AVIC_DOORBELL           0xc001011b
> +#define AVIC_HPA_MASK           ~((0xFFFULL << 52) || 0xFFF)
> +#define AVIC_APIC_BAR_MASK      0xFFFFFFFFFF000ULL
> +
> +bool_t svm_avic = 0;

static ? And also make it 'bool'

> +boolean_param("svm-avic", svm_avic);
> +
> +static struct svm_avic_phy_ait_entry *

This name resolves to 'SVM AVIC Physical APIC ID Table Entry'

That 'ait' keeps throwing me off as it sounds like 'eat' to me.

Perhaps  'avic_phy_apic_id_ent' ?

In other words s/ait/apic_id/ and s/svm_avic/avic/ followed by
s/_id_entry/id_ent/ ?


> +avic_get_phy_ait_entry(struct vcpu *v, int index)
> +{
> +    struct svm_avic_phy_ait_entry *avic_phy_ait;
> +    struct svm_domain *d = &v->domain->arch.hvm_domain.svm;
> +
> +    if ( !d->avic_phy_ait_mfn )
> +        return NULL;
> +
> +    /**
> +    * Note: APIC ID = 0xff is used for broadcast.
> +    *       APIC ID > 0xff is reserved.
> +    */
> +    if ( index >= 0xff )
> +        return NULL;
> +
> +    avic_phy_ait = mfn_to_virt(d->avic_phy_ait_mfn);
> +
> +    return &avic_phy_ait[index];
> +}
> +
> +/***************************************************************
> + * AVIC APIs
> + */
> +int svm_avic_dom_init(struct domain *d)
> +{
> +    int ret = 0;
> +    struct page_info *pg;
> +    unsigned long mfn;
> +
> +    if ( !svm_avic )
> +        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 here _mfn(0).

s/here/for APIC/


> +     */
> +    if ( !d->arch.hvm_domain.svm.avic_access_page_done )
> +    {
> +        set_mmio_p2m_entry(d, paddr_to_pfn(APIC_DEFAULT_PHYS_BASE),
> +                           _mfn(0), PAGE_ORDER_4K,
> +                           p2m_get_hostp2m(d)->default_access);
> +        d->arch.hvm_domain.svm.avic_access_page_done = true;
> +    }
> +
> +    /* Init AVIC log APIC ID table */

s/log/logical/
> +    pg = alloc_domheap_page(d, MEMF_no_owner);
> +    if ( !pg )
> +    {
> +        dprintk(XENLOG_ERR, "alloc AVIC logical APIC ID table error: %d\n",
> +                d->domain_id);
> +        ret = -ENOMEM;
> +        goto err_out;
> +    }
> +    mfn = page_to_mfn(pg);
> +    clear_domain_page(_mfn(mfn));
> +    d->arch.hvm_domain.svm.avic_log_ait_mfn = mfn;
> +
> +    /* Init AVIC phy APIC ID table */

physical ?

> +    pg = alloc_domheap_page(d, MEMF_no_owner);
> +    if ( !pg )
> +    {
> +        dprintk(XENLOG_ERR, "alloc AVIC physical APIC ID table error: %d\n",
> +                d->domain_id);
> +        ret = -ENOMEM;
> +        goto err_out;
> +    }
> +    mfn = page_to_mfn(pg);
> +    clear_domain_page(_mfn(mfn));
> +    d->arch.hvm_domain.svm.avic_phy_ait_mfn = mfn;
> +
> +    return ret;
> +err_out:
> +    svm_avic_dom_destroy(d);
> +    return ret;
> +}
> +
.. snip..
> +int svm_avic_init_vmcb(struct vcpu *v)
> +{
> +    paddr_t ma;
> +    u32 apic_id_reg;
> +    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;
> +    struct svm_avic_phy_ait_entry entry;
> +
> +    if ( !svm_avic )
> +        return 0;
> +
> +    vmcb->avic_bk_pg_pa = page_to_maddr(s->avic_bk_pg) & AVIC_HPA_MASK;
> +    ma = d->avic_log_ait_mfn;
> +    vmcb->avic_log_apic_id = (ma << PAGE_SHIFT) & AVIC_HPA_MASK;
> +    ma = d->avic_phy_ait_mfn;

s/ait/apic_id/ ?

> +    vmcb->avic_phy_apic_id = (ma << PAGE_SHIFT) & AVIC_HPA_MASK;
> +    vmcb->avic_phy_apic_id |= AVIC_PHY_APIC_ID_MAX;
> +

After reading the spec these tables make a bit more sense - one to
translate logical APIC -> physical APIC id, and the last one to
translate to host APIC.

It may be good to include just an simple ASCII flow of how say
IPI for a two vCPU guest would go?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel