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 < [EMAIL PROTECTED] >
> 
> 
> Anthony

Hi.
Some comments below.


> add structure and helper function for interrupt sharing
> 
> Sign-off-by; Anthony Xu < [EMAIL PROTECTED] >
             ^             ^ 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?


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

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

> +
> +#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?


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


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


-- 
yamahata

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

Reply via email to