Re: [Xen-devel] [PATCH V3 24/29] tools/libxc: Add a new interface to bind remapping format msi with pirq

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

On Thu, Sep 21, 2017 at 11:02:05PM -0400, Lan Tianyu wrote:
> From: Chao Gao <chao.gao@xxxxxxxxx>

The title for this patch it's wrong, it modifies both the hypervisor
and libxc. Please fix it.

> When exposing vIOMMU (vvtd) to guest, guest can configure the msi to
> remapping format. For pass-through device, the physical interrupt now
> can be bound with remapping format msi. This patch introduce a flag,
> HVM_IRQ_DPCI_GUEST_REMAPPED, which indicate a physical interrupt is
> bound with remapping format guest interrupt. Thus, we can use
> (HVM_IRQ_DPCI_GUEST_REMAPPED | HVM_IRQ_DPCI_GUEST_MSI) to show the new
> binding type. Also provide an new interface to manage the new binding.
> 
> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> 
> ---
> diff --git a/xen/include/asm-x86/hvm/irq.h b/xen/include/asm-x86/hvm/irq.h
> index bd8a918..4f5d37b 100644
> --- a/xen/include/asm-x86/hvm/irq.h
> +++ b/xen/include/asm-x86/hvm/irq.h
> @@ -121,6 +121,7 @@ struct dev_intx_gsi_link {
>  #define _HVM_IRQ_DPCI_GUEST_PCI_SHIFT           4
>  #define _HVM_IRQ_DPCI_GUEST_MSI_SHIFT           5
>  #define _HVM_IRQ_DPCI_IDENTITY_GSI_SHIFT        6
> +#define _HVM_IRQ_DPCI_GUEST_REMAPPED_SHIFT      7
>  #define _HVM_IRQ_DPCI_TRANSLATE_SHIFT          15
>  #define HVM_IRQ_DPCI_MACH_PCI        (1u << _HVM_IRQ_DPCI_MACH_PCI_SHIFT)
>  #define HVM_IRQ_DPCI_MACH_MSI        (1u << _HVM_IRQ_DPCI_MACH_MSI_SHIFT)
> @@ -128,6 +129,7 @@ struct dev_intx_gsi_link {
>  #define HVM_IRQ_DPCI_EOI_LATCH       (1u << _HVM_IRQ_DPCI_EOI_LATCH_SHIFT)
>  #define HVM_IRQ_DPCI_GUEST_PCI       (1u << _HVM_IRQ_DPCI_GUEST_PCI_SHIFT)
>  #define HVM_IRQ_DPCI_GUEST_MSI       (1u << _HVM_IRQ_DPCI_GUEST_MSI_SHIFT)
> +#define HVM_IRQ_DPCI_GUEST_REMAPPED  (1u << 
> _HVM_IRQ_DPCI_GUEST_REMAPPED_SHIFT)
>  #define HVM_IRQ_DPCI_IDENTITY_GSI    (1u << _HVM_IRQ_DPCI_IDENTITY_GSI_SHIFT)
>  #define HVM_IRQ_DPCI_TRANSLATE       (1u << _HVM_IRQ_DPCI_TRANSLATE_SHIFT)

Please keep this sorted. It should go after the _GSI one.

>  
> @@ -137,6 +139,11 @@ struct hvm_gmsi_info {
>              uint32_t gvec;
>              uint32_t gflags;
>          } legacy;
> +        struct {
> +            uint32_t source_id;
> +            uint32_t data;
> +            uint64_t addr;
> +        } intremap;
>      };
>      int dest_vcpu_id; /* -1 :multi-dest, non-negative: dest_vcpu_id */
>      bool posted; /* directly deliver to guest via VT-d PI? */
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 68854b6..8c59cfc 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -559,6 +559,7 @@ typedef enum pt_irq_type_e {
>      PT_IRQ_TYPE_MSI,
>      PT_IRQ_TYPE_MSI_TRANSLATE,
>      PT_IRQ_TYPE_SPI,    /* ARM: valid range 32-1019 */
> +    PT_IRQ_TYPE_MSI_IR,

Introducing a new irq type seems dubious, at the end this is still a
MSI interrupt.

>  } pt_irq_type_t;
>  struct xen_domctl_bind_pt_irq {
>      uint32_t machine_irq;
> @@ -586,6 +587,12 @@ struct xen_domctl_bind_pt_irq {
>              uint64_aligned_t gtable;
>          } msi;
>          struct {
> +            uint32_t source_id;
> +            uint32_t data;
> +            uint64_t addr;
> +            uint64_t gtable;
> +        } msi_ir;

Have you tried to expand gflags somehow so that you don't need a new
type together with a new structure?

It seems quite cumbersome and also involves adding more handlers to
libxc.

At the end this is a domctl interface, so you should be able to modify
it at will.

Roger.

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