Re: [PATCH v1 07/28] x86/hvm: avic: add VMCB state initialization

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

On 11/5/26 1:33 AM, dmukhin@xxxxxxxx wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx>
>
> Update SVM VMCB with AVIC-related data types as per [1].
>
> Install AVIC backing page and logical/physical APIC ID tables mappings
> into VMCB.
>
> Set VMCB 'avic_enable' bit to 1 in case of successful initialization.
>
> [1] AMD APM Volumes 1-5 "15.29 Advanced Virtual Interrupt Controller".
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> ---
>   xen/arch/x86/hvm/svm/avic.c | 110 ++++++++++++++++++++++++++++++++++++
>   xen/arch/x86/hvm/svm/avic.h |   3 +
>   xen/arch/x86/hvm/svm/vmcb.c |   7 +++
>   xen/arch/x86/hvm/svm/vmcb.h |  19 +++++--
>   4 files changed, 135 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
> index 2c0e113ae34e..bc98661a7057 100644
> --- a/xen/arch/x86/hvm/svm/avic.c
> +++ b/xen/arch/x86/hvm/svm/avic.c
> @@ -9,6 +9,16 @@
>   #include <xen/param.h>
>   
>   #include "avic.h"
> +#include "vmcb.h"
> +
> +#define AVIC_PHYSICAL_ID_ENTRY_BACKING_PAGE_MASK        GENMASK_ULL(51, 12U)
> +#define AVIC_PHYSICAL_ID_TABLE_SIZE_MASK                (0xFFULL)
> +
> +/*
> + * For AVIC, the max index allowed for physical APIC ID table is 0xfe (254), as
> + * 0xff is a broadcast to all CPUs, i.e. can't be targeted individually.
> + */
> +#define AVIC_MAX_PHYSICAL_ID                            (0xFEU)
>   
>   bool __read_mostly svm_avic_enable;
>   
> @@ -125,6 +135,106 @@ void svm_avic_domain_relinquish_resources(struct domain *d)
>       }
>   }
>   
> +static bool avic_vcpu_supported(struct vcpu *v)
> +{
> +    struct domain *d = v->domain;
> +    unsigned int id;
> +
> +    if ( !svm_avic_enable || !has_vlapic(d) )
> +        return false;
> +
> +    if ( d->max_vcpus * 2 >= AVIC_PHYSICAL_ID_TABLE_SIZE_MASK )
> +    {
> +        gprintk(XENLOG_WARNING,
> +                "%pv: AVIC: unsupported domain max_vcpus %u\n",
> +                v, d->max_vcpus);
> +        return false;
> +    }
> +
> +    id = GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID));
> +    if ( id > AVIC_MAX_PHYSICAL_ID )
> +    {
> +        gprintk(XENLOG_WARNING,
> +                "%pv: AVIC: unsupported APIC ID %u\n",
> +                v, id);
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +static void avic_vcpu_enable(const struct vcpu *v, bool enable)
> +{
> +    const struct svm_vcpu *sv = &v->arch.hvm.svm;
> +    struct vmcb_struct *vmcb = sv->vmcb;
> +
> +    vmcb->_vintr.fields.x2avic_enable = 0;
> +    vmcb->_vintr.fields.avic_enable = enable;
> +    vmcb->cleanbits.avic = 0;
> +
> +    printk(XENLOG_INFO
> +           "%pv: AVIC: %s: apic_id=%#x avic_physical_id=%#"PRIx64" 
> max_vcpus=%u\n",
> +           v,
> +           enable ? "enable" : "disable",
> +           GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID)),
> +           sv->avic_physical_id ? ACCESS_ONCE(*sv->avic_physical_id) :
> +                                  0xDEADDEADUL,
> +           v->domain->max_vcpus);
> +}
This is called from svm_avic_vmcb_destroy() which may be called after
domain_relinquish_resources(). Dereferencing avic_physical_id hits a page
fault since the page is already unmapped by then.

Ross