[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 6/8] x86/iommu: call pi_update_irte through an hvm_function callback


  • To: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 12 Jan 2023 13:16:17 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=HSg0cm17QE9NumIyyQ1sqXZmMUnNT4vIsgXAUlpWqfs=; b=HirPbeh2vphmZ3wP/1C52yHTBHiYneFvxAV5ldTxJs5qWsRwoCS1mB5He72ZgBfn9v2NFxVZrWasI+DoMA3XDSzwMPwKxq/lZ8LU26r3uT78XhkwVBYcMOR5RqE8SGaDuT9GWMZJyByfyvFopvhpXE3z9z/ehME3ES8Pw2BRUfmuCrRSSNgXl3KjcLpyrGA333GBFeduYFBChEqjZ+y8sZ5jCXcRHq1ENz3zwg9/7cZbVb+tFKlk+N38KmK5I6zerVzRnlQGWizkLJ3q6PVkdBN1uDEqel6Y8UCEnniUO/JzoBkKxKYowsu+6xnPPYM2CrXRI6hFdefXyfB8fcQ9qA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=IkAkg+exqsZYredmu7r9MPmh5Agmrwzsp82aruLC1gTbp8r7spzqbK4XpDZdFWzsWV4Ped2caQmNK7BPJWzYgZv+3O4YgJcuoaoWpgo0/T1zU1OP9KE+Og0y+o4o5F+89heCd1vn3rkmnxI/HeU7x88jVrgWH1Ww0yLC8MmXsSsEbJHThmlN2ldWLG+HCzj2XOsDdVrfg5+JddEGsZW16eMaFuPjkjBt3UMO1JI3kxDa9014HP/4C2HmuWQE0w5GuWnkPlbGRAogWevVMJ3XYmvtxlRDDdU8vpYbEAzkFGY3WZDCPnbX5DAA9WKKnpggwhzw/S5/gueo9GoyEb/dIA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Jun Nakajima <jun.nakajima@xxxxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Paul Durrant <paul@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Thu, 12 Jan 2023 12:16:30 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 04.01.2023 09:45, Xenia Ragiadakou wrote:
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2143,6 +2143,14 @@ static bool cf_check vmx_test_pir(const struct vcpu 
> *v, uint8_t vec)
>      return pi_test_pir(vec, &v->arch.hvm.vmx.pi_desc);
>  }
>  
> +static int cf_check vmx_pi_update_irte(const struct vcpu *v,
> +                                       const struct pirq *pirq, uint8_t gvec)
> +{
> +    const struct pi_desc *pi_desc = v ? &v->arch.hvm.vmx.pi_desc : NULL;
> +
> +    return pi_update_irte(pi_desc, pirq, gvec);
> +}

This being the only caller of pi_update_irte(), I don't see the point in
having the extra wrapper. Adjust pi_update_irte() such that it can be
used as the intended hook directly. Plus perhaps prefix it with vtd_.

> @@ -2591,6 +2599,8 @@ static struct hvm_function_table __initdata_cf_clobber 
> vmx_function_table = {
>      .tsc_scaling = {
>          .max_ratio = VMX_TSC_MULTIPLIER_MAX,
>      },
> +
> +    .pi_update_irte = vmx_pi_update_irte,

You want to install this hook only when iommu_intpost (i.e. the only case
when it can actually be called, and only when INTEL_IOMMU=y (avoiding the
need for an inline stub of pi_update_irte() or whatever its final name is
going to be.

> @@ -250,6 +252,9 @@ struct hvm_function_table {
>          /* Architecture function to setup TSC scaling ratio */
>          void (*setup)(struct vcpu *v);
>      } tsc_scaling;
> +
> +    int (*pi_update_irte)(const struct vcpu *v,
> +                          const struct pirq *pirq, uint8_t gvec);
>  };

Please can this be moved higher up, e.g. next to .

> @@ -774,6 +779,16 @@ static inline void hvm_set_nonreg_state(struct vcpu *v,
>          alternative_vcall(hvm_funcs.set_nonreg_state, v, nrs);
>  }
>  
> +static inline int hvm_pi_update_irte(const struct vcpu *v,
> +                                     const struct pirq *pirq, uint8_t gvec)
> +{
> +    if ( hvm_funcs.pi_update_irte )
> +        return alternative_call(hvm_funcs.pi_update_irte, v, pirq, gvec);
> +
> +    return -EOPNOTSUPP;

I don't think the conditional is needed, at least not with the other
suggested adjustments. Plus the way alternative patching works, a NULL
hook will be converted to some equivalent of BUG() anyway, so
ASSERT_UNREACHABLE() should also be unnecessary.

> +}
> +
> +
>  #else  /* CONFIG_HVM */

Please don't add double blank lines.

> --- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h
> +++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
> @@ -146,6 +146,17 @@ static inline void pi_clear_sn(struct pi_desc *pi_desc)
>      clear_bit(POSTED_INTR_SN, &pi_desc->control);
>  }
>  
> +#ifdef CONFIG_INTEL_IOMMU
> +int pi_update_irte(const struct pi_desc *pi_desc,
> +                   const struct pirq *pirq, const uint8_t gvec);
> +#else
> +static inline int pi_update_irte(const struct pi_desc *pi_desc,
> +                                 const struct pirq *pirq, const uint8_t gvec)
> +{
> +    return -EOPNOTSUPP;
> +}
> +#endif

This still is a VT-d function, so I think its declaration would better
remain in asm/iommu.h.

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.