Re: [Xen-devel] [PATCH V2 22/25] x86/vmsi: Hook delivering remapping format msi to guest

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Aug 09, 2017 at 04:34:23PM -0400, Lan Tianyu wrote:
> From: Chao Gao <chao.gao@xxxxxxxxx>
> 
> In two situations, hypervisor delivers a msi to a hvm guest. One is
> when qemu sends a request to hypervisor through XEN_DMOP_inject_msi.
> The other is when a physical interrupt arrives and it has been bound
> to a guest msi.
> 
> For the former, the msi is routed to common vIOMMU layer if it is in
> remapping format. For the latter, if the pt irq is bound to a guest
> remapping msi, a new remapping msi is constructed based on the binding
> information and routed to common vIOMMU layer.

After looking at the code below, I'm wondering whether it would make
sense to add a new flag that's HVM_IRQ_DPCI_GUEST_REMAPPED or similar,
so that you would use:

HVM_IRQ_DPCI_GUEST_MSI | HVM_IRQ_DPCI_GUEST_REMAPPED

In order to designate a remapped MSI. It seems like it would avoid
some of the changes below (where you are just adding
HVM_IRQ_DPCI_GUEST_MSI_IR to code paths already used by
HVM_IRQ_DPCI_GUEST_MSI). More of a suggestion rather than a request
for you to change the code.

> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> ---
>  xen/arch/x86/hvm/irq.c       | 11 ++++++++++
>  xen/arch/x86/hvm/vmsi.c      | 14 ++++++++++--
>  xen/drivers/passthrough/io.c | 51 
> +++++++++++++++++++++++++++++++++-----------
>  xen/include/asm-x86/msi.h    |  3 +++
>  4 files changed, 65 insertions(+), 14 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
> index e425df9..12d83b3 100644
> --- a/xen/arch/x86/hvm/irq.c
> +++ b/xen/arch/x86/hvm/irq.c
> @@ -26,6 +26,7 @@
>  #include <asm/hvm/domain.h>
>  #include <asm/hvm/support.h>
>  #include <asm/msi.h>
> +#include <asm/viommu.h>
>  
>  /* Must be called with hvm_domain->irq_lock hold */
>  static void assert_gsi(struct domain *d, unsigned ioapic_gsi)
> @@ -340,6 +341,16 @@ int hvm_inject_msi(struct domain *d, uint64_t addr, 
> uint32_t data)
>          >> MSI_DATA_TRIGGER_SHIFT;
>      uint8_t vector = data & MSI_DATA_VECTOR_MASK;
>  
> +    if ( addr & MSI_ADDR_INTEFORMAT_MASK )
> +    {
> +        struct irq_remapping_request request;
> +
> +        irq_request_msi_fill(&request, 0, addr, data);
> +        /* Currently, only viommu 0 is supported */
> +        viommu_handle_irq_request(d, 0, &request);
> +        return 0;
> +    }
> +
>      if ( !vector )
>      {
>          int pirq = ((addr >> 32) & 0xffffff00) | dest;
> diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
> index c4ec0ad..75ceb19 100644
> --- a/xen/arch/x86/hvm/vmsi.c
> +++ b/xen/arch/x86/hvm/vmsi.c
> @@ -114,9 +114,19 @@ void vmsi_deliver_pirq(struct domain *d, const struct 
> hvm_pirq_dpci *pirq_dpci)
>                  "vector=%x trig_mode=%x\n",
>                  dest, dest_mode, delivery_mode, vector, trig_mode);
>  
> -    ASSERT(pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI);
> +    ASSERT(pirq_dpci->flags & (HVM_IRQ_DPCI_GUEST_MSI | 
> HVM_IRQ_DPCI_GUEST_MSI_IR));

Line break.

Roger.

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