On Wed, May 08, 2024 at 01:39:21PM +0100, Alejandro Vallejo wrote:
> Otherwise it's not possible to call functions described in hvm/vlapic.h from
> the
> inline functions of hvm/hvm.h.
>
> This is because a static inline in vlapic.h depends on hvm.h, and pulls it
> transitively through vpt.h. The ultimate cause is having hvm.h included in any
> of the "v*.h" headers, so break the cycle moving the guilty inline into hvm.h.
>
> No functional change.
>
> Signed-off-by: Alejandro Vallejo <alejandro.vall...@cloud.com>
Acked-by: Roger Pau Monné <roger....@citrix.com>
One cosmetic comment below.
> ---
> v2:
> * New patch. Prereq to moving vlapic_cpu_policy_changed() onto hvm.h
> ---
> xen/arch/x86/hvm/irq.c | 6 +++---
> xen/arch/x86/hvm/vlapic.c | 4 ++--
> xen/arch/x86/include/asm/hvm/hvm.h | 6 ++++++
> xen/arch/x86/include/asm/hvm/vlapic.h | 6 ------
> xen/arch/x86/include/asm/hvm/vpt.h | 1 -
> 5 files changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
> index 4a9fe82cbd8d..4f5479b12c98 100644
> --- a/xen/arch/x86/hvm/irq.c
> +++ b/xen/arch/x86/hvm/irq.c
> @@ -512,13 +512,13 @@ struct hvm_intack hvm_vcpu_has_pending_irq(struct vcpu
> *v)
> int vector;
>
> /*
> - * Always call vlapic_sync_pir_to_irr so that PIR is synced into IRR when
> - * using posted interrupts. Note this is also done by
> + * Always call hvm_vlapic_sync_pir_to_irr so that PIR is synced into IRR
> + * when using posted interrupts. Note this is also done by
> * vlapic_has_pending_irq but depending on which interrupts are pending
> * hvm_vcpu_has_pending_irq will return early without calling
> * vlapic_has_pending_irq.
> */
> - vlapic_sync_pir_to_irr(v);
> + hvm_vlapic_sync_pir_to_irr(v);
>
> if ( unlikely(v->arch.nmi_pending) )
> return hvm_intack_nmi;
> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
> index 61a96474006b..8a244100009c 100644
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -98,7 +98,7 @@ static void vlapic_clear_irr(int vector, struct vlapic
> *vlapic)
>
> static int vlapic_find_highest_irr(struct vlapic *vlapic)
> {
> - vlapic_sync_pir_to_irr(vlapic_vcpu(vlapic));
> + hvm_vlapic_sync_pir_to_irr(vlapic_vcpu(vlapic));
>
> return vlapic_find_highest_vector(&vlapic->regs->data[APIC_IRR]);
> }
> @@ -1516,7 +1516,7 @@ static int cf_check lapic_save_regs(struct vcpu *v,
> hvm_domain_context_t *h)
> if ( !has_vlapic(v->domain) )
> return 0;
>
> - vlapic_sync_pir_to_irr(v);
> + hvm_vlapic_sync_pir_to_irr(v);
>
> return hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, vcpu_vlapic(v)->regs);
> }
> diff --git a/xen/arch/x86/include/asm/hvm/hvm.h
> b/xen/arch/x86/include/asm/hvm/hvm.h
> index e1f0585d75a9..84911f3ebcb4 100644
> --- a/xen/arch/x86/include/asm/hvm/hvm.h
> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
> @@ -798,6 +798,12 @@ static inline void hvm_update_vlapic_mode(struct vcpu *v)
> alternative_vcall(hvm_funcs.update_vlapic_mode, v);
> }
>
> +static inline void hvm_vlapic_sync_pir_to_irr(struct vcpu *v)
> +{
> + if ( hvm_funcs.sync_pir_to_irr )
> + alternative_vcall(hvm_funcs.sync_pir_to_irr, v);
Nit: for consistency the wrappers are usually named hvm_<hook_name>,
so in this case it would be hvm_sync_pir_to_irr(), or the hvm_funcs
field should be renamed to vlapic_sync_pir_to_irr.
Thanks, Roger.
