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

[Chao Gao <chao.gao@xxxxxxxxx>]

On Thu, Oct 19, 2017 at 05:03:26PM +0100, Roger Pau Monné wrote:
>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?

gflags doesn't have enough bits to contain so much information.

>
>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.

Considering gtable and gflags are also needed for 'msi_ir', 
modifying the existing interface seems better than adding an new one.

Thanks
Chao

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