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

RE: [Xen-ia64-devel][PATCH][VTD] add structure and helper function for interrupt sharing


  • To: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
  • From: "Xu, Anthony" <anthony.xu@xxxxxxxxx>
  • Date: Thu, 16 Oct 2008 18:59:33 +0800
  • Accept-language: en-US
  • Acceptlanguage: en-US
  • Cc: xen-ia64-devel <xen-ia64-devel@xxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 16 Oct 2008 03:59:41 -0700
  • List-id: Discussion of the ia64 port of Xen <xen-ia64-devel.lists.xensource.com>
  • Thread-index: AckvZlX9JqYZt3LfT3m/zzPJ007F3gAFiX+g
  • Thread-topic: [Xen-ia64-devel][PATCH][VTD] add structure and helper function for interrupt sharing

Isaku Yamahata wrote:
> On Wed, Oct 15, 2008 at 05:44:41PM +0800, Xu, Anthony wrote:
>> add structure and helper function for interrupt sharing
>>
>> Sign-off-by; Anthony Xu < anthony.xu@xxxxxxxxx >
>>
>>
>> Anthony
>
> Hi.
> Some comments below.
>
>
>> add structure and helper function for interrupt sharing
>>
>> Sign-off-by; Anthony Xu < anthony.xu@xxxxxxxxx >
>              ^             ^ no space around email address.
>              not semicolon, but colon.
>
>
:-)

>>
>>
>> diff -r 9b227eb09263 xen/arch/ia64/linux-xen/irq_ia64.c
>> --- a/xen/arch/ia64/linux-xen/irq_ia64.c      Tue Oct 14 19:19:48
>> 2008 +0100 +++ b/xen/arch/ia64/linux-xen/irq_ia64.c      Wed Oct 15
>> 17:18:25 2008 +0800 @@ -334,3 +334,62 @@
>>
>>       writeq(ipi_data, ipi_addr);
>>  }
>> +
>> +
>> +static void __hvm_pci_intx_assert(
>> +             struct domain *d, unsigned int device, unsigned int
>> intx) +{ +     struct hvm_irq *hvm_irq = &d->arch.hvm_domain.irq; +
>> unsigned int gsi; +
>> +     ASSERT((device <= 31) && (intx <= 3));
>> +
>> +     if ( __test_and_set_bit(device*4 + intx, &hvm_irq->pci_intx.i)
>> ) +             return; +     gsi = hvm_pci_intx_gsi(device, intx);
>> +     if ( ++hvm_irq->gsi_assert_count[gsi] == 1 )
>> +        viosapic_set_irq(d, gsi, 1);
>> +}
>> +
>> +
>> +void hvm_pci_intx_assert(
>> +             struct domain *d, unsigned int device, unsigned int
>> intx) +{ +     __hvm_pci_intx_assert(d, device, intx);
>> +}
>> +
>
> Why are the two version (with and without __) defined?
> Looking x86 version, a lock will be added in future. doesn't it?
>
In x86 side, vpic and vioapci use a big lock, so it is needed to get the lock 
first before call __hvm_pci_intx_assert.
In ia65 side, there is a dedicated lock for viosapic, there is no need to get a 
lock here.
I'll merge the two version


>
>> +static void __hvm_pci_intx_deassert(
>> +             struct domain *d, unsigned int device, unsigned int
>> intx) +{ +     struct hvm_irq *hvm_irq = &d->arch.hvm_domain.irq; +
>> unsigned int gsi; +
>> +     ASSERT((device <= 31) && (intx <= 3));
>> +
>> +     if ( !__test_and_clear_bit(device*4 + intx,
>> &hvm_irq->pci_intx.i) ) +             return; +
>> +     gsi = hvm_pci_intx_gsi(device, intx);
>> +
>> +     if (--hvm_irq->gsi_assert_count[gsi] == 0)
>> +             viosapic_set_irq(d, gsi, 0);
>> +}
>> +
>> +void hvm_pci_intx_deassert(
>> +             struct domain *d, unsigned int device, unsigned int
>> intx) +{ +         __hvm_pci_intx_deassert(d, device, intx); +}
>> +
>> +void hvm_isa_irq_assert(
>> +                     struct domain *d, unsigned int isa_irq) +{
>> +}
>> +
>> +
>> +void hvm_isa_irq_deassert(
>> +                     struct domain *d, unsigned int isa_irq) +{
>> +}
>> +
>> +
>
> Those are for HVM domain, so please move them under
> the directory, xen/arch/ia64/vmx.

Agree
>
>> diff -r 9b227eb09263 xen/arch/ia64/vmx/viosapic.c
>> --- a/xen/arch/ia64/vmx/viosapic.c    Tue Oct 14 19:19:48 2008 +0100
>> +++ b/xen/arch/ia64/vmx/viosapic.c    Wed Oct 15 17:18:25 2008 +0800
>>  @@ -314,10 +314,6 @@ out:
>>      spin_unlock(&viosapic->lock);
>>  }
>> -
>> -#define hvm_pci_intx_gsi(dev, intx)  \
>> -    (((((dev) << 2) + ((dev) >> 3) + (intx)) & 31) + 16) -
>>
>>  void viosapic_set_pci_irq(struct domain *d, int device, int intx,
>> int level)  {
>> diff -r 9b227eb09263 xen/arch/ia64/xen/domain.c
>> --- a/xen/arch/ia64/xen/domain.c      Tue Oct 14 19:19:48 2008 +0100
>> +++ b/xen/arch/ia64/xen/domain.c      Wed Oct 15 17:18:25 2008 +0800
>> @@ -568,6 +568,8 @@
>>
>>       if (is_idle_domain(d))
>>           return 0;
>> +
>> +     INIT_LIST_HEAD(&d->arch.pdev_list);
>>
>>       foreign_p2m_init(d);
>>  #ifdef CONFIG_XEN_IA64_PERVCPU_VHPT
>> diff -r 9b227eb09263 xen/include/asm-ia64/domain.h
>> --- a/xen/include/asm-ia64/domain.h   Tue Oct 14 19:19:48 2008 +0100
>> +++ b/xen/include/asm-ia64/domain.h   Wed Oct 15 17:18:25 2008 +0800
>>  @@ -129,6 +129,8 @@ /* Set an optimization feature in the struct
>>  arch_domain. */ extern int domain_opt_feature(struct domain *,
>> struct xen_ia64_opt_feature*);
>>
>> +#define has_arch_pdevs(d)   (!list_empty(&(d)->arch.pdev_list)) +
>>  struct arch_domain {
>>      struct mm_struct mm;
>>
>> @@ -166,6 +168,7 @@
>>      unsigned char rid_bits;          /* number of virtual rid bits
>>      (default: 18) */ int breakimm;               /* The imm value
>> for hypercalls.  */
>>
>> +    struct list_head pdev_list;
>>      struct virtual_platform_def     vmx_platform;
>>  #define      hvm_domain vmx_platform /* platform defs are not vmx
>> specific */
>>
>> diff -r 9b227eb09263 xen/include/asm-ia64/hvm/irq.h
>> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>> +++ b/xen/include/asm-ia64/hvm/irq.h  Wed Oct 15 17:18:25 2008 +0800
>> @@ -0,0 +1,178 @@
>> +/******************************************************************************
>> + * irq.h + * + * Interrupt distribution and delivery logic.
>> + *
>> + * Copyright (c) 2006, K A Fraser, XenSource Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> modify it + * under the terms and conditions of the GNU General
>> Public License, + * version 2, as published by the Free Software
>> Foundation. + * + * This program is distributed in the hope it will
>> be useful, but WITHOUT + * ANY WARRANTY; without even the implied
>> warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE.
>> See the GNU General Public License for + * more details. + *
>> + * You should have received a copy of the GNU General Public
>> License along with + * this program; if not, write to the Free
>> Software Foundation, Inc., 59 Temple + * Place - Suite 330, Boston,
>> MA 02111-1307 USA. + */
>> +
>> +#ifndef __ASM_IA64_HVM_IRQ_H__
>> +#define __ASM_IA64_HVM_IRQ_H__
>> +
>> +#include <xen/types.h>
>> +#include <xen/spinlock.h>
>> +#include <asm/irq.h>
>> +#include <public/hvm/save.h>
>> +#include <xen/irq.h>
>> +
>> +#define NR_VECTORS 256
>> +#define VIOAPIC_NUM_PINS  48
>> +
>> +
>> +
>> +struct dev_intx_gsi_link {
>> +    struct list_head list;
>> +    uint8_t device;
>> +    uint8_t intx;
>> +    uint8_t gsi;
>> +    uint8_t link;
>> +};
>> +
>> +struct hvm_hw_pci_irqs {
>> +    /*
>> +     * Virtual interrupt wires for a single PCI bus.
>> +     * Indexed by: device*4 + INTx#.
>> +     */
>> +    union {
>> +        DECLARE_BITMAP(i, 32*4);
>> +        uint64_t pad[2];
>> +    };
>> +};
>
> In the x86 case, they are defined in public/arch-x86/hvm/save.h.
> Although I'm not sure, aren't they needed for save/restore?

I am not sure, I guess they are not needed for save/restore.
VTD save/restore is not supported directly
It is supported by using teaming driver.

For example
There are two NICs, one is a NIC assigned through VTD
The other is a qemu e1000,
In normal time,  VTD NIC is used by guest through teaming driver
When you want to save/restore guest, hotunplug VTD NIC, then only qemu e1000
works, then save.







>
>> +
>> +#define HVM_IRQ_DPCI_VALID 0x1
>> +#define HVM_IRQ_DPCI_MSI   0x2
>> +#define _HVM_IRQ_DPCI_MSI  0x1
>> +
>> +
>> +struct hvm_gmsi_info {
>> +    uint32_t gvec;
>> +    uint32_t gflags;
>> +};
>> +
>> +struct hvm_mirq_dpci_mapping {
>> +    uint32_t flags;
>> +    int pending;
>> +    struct list_head digl_list;
>> +    struct domain *dom;
>> +    struct hvm_gmsi_info gmsi;
>> +};
>> +
>> +struct hvm_girq_dpci_mapping {
>> +    uint8_t valid;
>> +    uint8_t device;
>> +    uint8_t intx;
>> +    uint8_t machine_gsi;
>> +};
>> +
>> +#define NR_ISAIRQS  16
>> +#define NR_LINK     4
>> +struct hvm_irq_dpci {
>> +    spinlock_t dirq_lock;
>> +    /* Machine IRQ to guest device/intx mapping. */
>> +    struct hvm_mirq_dpci_mapping mirq[NR_IRQS];
>> +    /* Guest IRQ to guest device/intx mapping. */
>> +    struct hvm_girq_dpci_mapping girq[NR_IRQS];
>> +    uint8_t msi_gvec_pirq[NR_VECTORS];
>> +    DECLARE_BITMAP(dirq_mask, NR_IRQS);
>> +    /* Record of mapped ISA IRQs */
>> +    DECLARE_BITMAP(isairq_map, NR_ISAIRQS);
>> +    /* Record of mapped Links */
>> +    uint8_t link_cnt[NR_LINK];
>> +    struct timer hvm_timer[NR_IRQS];
>> +};
>> +
>> +struct hvm_irq {
>> +    /*
>> +     * Virtual interrupt wires for a single PCI bus.
>> +     * Indexed by: device*4 + INTx#.
>> +     */
>> +    struct hvm_hw_pci_irqs pci_intx;
>> +
>> +    /* Virtual interrupt and via-link for paravirtual platform
>> driver. */ +    uint32_t callback_via_asserted;
>> +    union {
>> +        enum {
>> +            HVMIRQ_callback_none,
>> +            HVMIRQ_callback_gsi,
>> +            HVMIRQ_callback_pci_intx
>> +        } callback_via_type;
>> +    };
>> +    union {
>> +        uint32_t gsi;
>> +        struct { uint8_t dev, intx; } pci;
>> +    } callback_via;
>> +
>> +    /*
>> +     * Number of wires asserting each GSI.
>> +     *
>> +     * GSIs 0-15 are the ISA IRQs. ISA devices map directly into
>> this space +     * except ISA IRQ 0, which is connected to GSI 2.
>> +     * PCI links map into this space via the PCI-ISA bridge. +     *
>> +     * GSIs 16+ are used only be PCI devices. The mapping from PCI
>> device to +     * GSI is as follows: ((device*4 + device/8 + INTx#)
>> & 31) + 16 +     */ +    u8 gsi_assert_count[VIOAPIC_NUM_PINS];
>> +
>> +    /*
>> +     * GSIs map onto PIC/IO-APIC in the usual way:
>> +     *  0-7:  Master 8259 PIC, IO-APIC pins 0-7
>> +     *  8-15: Slave  8259 PIC, IO-APIC pins 8-15
>> +     *  16+ : IO-APIC pins 16+
>> +     */
>> +
>> +    /* Last VCPU that was delivered a LowestPrio interrupt. */
>> +    u8 round_robin_prev_vcpu;
>> +
>> +    struct hvm_irq_dpci *dpci;
>> +};
>> +
>> +#define hvm_pci_intx_gsi(dev, intx)  \
>> +    (((((dev)<<2) + ((dev)>>3) + (intx)) & 31) + 16)
>> +#define hvm_pci_intx_link(dev, intx) \
>> +    (((dev) + (intx)) & 3)
>> +
>> +
>> +extern u8 irq_vector[NR_IRQ_VECTORS];
>> +extern int vector_irq[NR_VECTORS];
>> +
>> +/* Extract the IA-64 vector that corresponds to IRQ.  */ +static
>> inline int +irq_to_vector (int irq)
>> +{
>> +        return irq;
>> +}
>> +
>> +/* Modify state of a PCI INTx wire. */
>> +void hvm_pci_intx_assert(
>> +    struct domain *d, unsigned int device, unsigned int intx);
>> +void hvm_pci_intx_deassert( +    struct domain *d, unsigned int
>> device, unsigned int intx); + +/* Modify state of an ISA device's
>> IRQ wire. */ +void hvm_isa_irq_assert(
>> +    struct domain *d, unsigned int isa_irq);
>> +void hvm_isa_irq_deassert(
>> +    struct domain *d, unsigned int isa_irq);
>> +
>> +void hvm_set_pci_link_route(struct domain *d, u8 link, u8 isa_irq);
>> + +void hvm_maybe_deassert_evtchn_irq(void);
>> +void hvm_assert_evtchn_irq(struct vcpu *v);
>> +void hvm_set_callback_via(struct domain *d, uint64_t via); +
>> +
>> +#endif /* __ASM_IA64_HVM_IRQ_H__ */
>
> This file looks like almost same to xen/include/asm-x86/hvm/irq.h
> I suppose those are arch generic because they are pci stuff.
> So can those definitions be shared by x86 and ia64 with
> the file. E.g. xen/include/xen/hvm/irq.h or something like that?
> Or are you expecting future divergence?

I'll extract common code out to common head file.



>
>
>> diff -r 9b227eb09263 xen/include/asm-ia64/linux/asm/hw_irq.h
>> --- a/xen/include/asm-ia64/linux/asm/hw_irq.h Tue Oct 14 19:19:48
>> 2008 +0100 +++ b/xen/include/asm-ia64/linux/asm/hw_irq.h Wed Oct 15
>>  17:18:25 2008 +0800 @@ -14,6 +14,7 @@ #include <asm/machvec.h>
>>  #include <asm/ptrace.h>
>>  #include <asm/smp.h>
>> +#include <asm/hvm/irq.h>
>>
>>  typedef u8 ia64_vector;
>>
>> @@ -124,13 +125,6 @@
>>       return irq_desc + irq;
>>  }
>>
>> -/* Extract the IA-64 vector that corresponds to IRQ.  */
>> -static inline ia64_vector
>> -irq_to_vector (int irq)
>> -{
>> -     return (ia64_vector) irq;
>> -}
>> -
>>  /*
>>   * Convert the local IA-64 vector to the corresponding irq number.
>> This translation is
>>   * done in the context of the interrupt domain that the currently
>> executing CPU belongs
>
> Please move the file under xen/include/asm-ia64/linux-xen/asm/,
> and don't forget update README.origin. Then comment out by #ifndef
> XEN.
>
Okay



Thanks,
Anthony


>
>> diff -r 9b227eb09263 xen/include/asm-ia64/vmx_platform.h
>> --- a/xen/include/asm-ia64/vmx_platform.h     Tue Oct 14 19:19:48
>> 2008 +0100 +++ b/xen/include/asm-ia64/vmx_platform.h     Wed Oct 15
>> 17:18:25 2008 +0800 @@ -21,8 +21,10 @@
>>
>>  #include <public/xen.h>
>>  #include <public/hvm/params.h>
>> +#include <asm/hvm/irq.h>
>>  #include <asm/viosapic.h>
>>  #include <asm/hvm/vacpi.h>
>> +#include <xen/hvm/iommu.h>
>>
>>  struct vmx_ioreq_page {
>>      spinlock_t          lock;
>> @@ -41,6 +43,9 @@
>>      /* One IOSAPIC now... */
>>      struct viosapic             viosapic;
>>      struct vacpi                vacpi;
>> +    /* Pass-throgh VT-d */
>> +    struct hvm_irq              irq;
>> +    struct hvm_iommu            hvm_iommu;
>>  } vir_plat_t;
>>
>>  static inline int __fls(uint32_t word)

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel


 


Rackspace

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