WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH 1 of 5] ats: Move some ats functions to a new dir

To: "Wei Wang" <wei.wang2@xxxxxxx>
Subject: [Xen-devel] Re: [PATCH 1 of 5] ats: Move some ats functions to a new directory
From: "Jan Beulich" <JBeulich@xxxxxxxx>
Date: Thu, 03 Nov 2011 12:46:26 +0000
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Thu, 03 Nov 2011 05:47:18 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <d422e3cf7976c76c57fc.1320315181@xxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <patchbomb.1320315180@xxxxxxxxxxxx> <d422e3cf7976c76c57fc.1320315181@xxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
>>> On 03.11.11 at 11:13, Wei Wang <wei.wang2@xxxxxxx> wrote:
> # HG changeset patch
> # User Wei Wang <wei.wang2@xxxxxxx>
> # Date 1320314537 -3600
> # Node ID d422e3cf7976c76c57fc2d455b784d0fcc24d06b
> # Parent  119bccb1cc5eee1364bbbd3bd1a30f22e9db703a
> ats: Move some ats functions to a new directory.
> Remove VTD prefix from debug output.
> passhrough/x86 holds vendor neutral codes for x86 architecture.
> 
> Signed-off-by: Wei Wang <wei.wang2@xxxxxxx>
> 
> diff -r 119bccb1cc5e -r d422e3cf7976 xen/drivers/passthrough/Makefile
> --- a/xen/drivers/passthrough/Makefile        Wed Nov 02 13:53:05 2011 +0100
> +++ b/xen/drivers/passthrough/Makefile        Thu Nov 03 11:02:17 2011 +0100
> @@ -1,6 +1,7 @@
>  subdir-$(x86) += vtd
>  subdir-$(ia64) += vtd
>  subdir-$(x86) += amd
> +subdir-$(x86) += x86
>  
>  obj-y += iommu.o
>  obj-y += io.o
> diff -r 119bccb1cc5e -r d422e3cf7976 xen/drivers/passthrough/ats.h
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/xen/drivers/passthrough/ats.h   Thu Nov 03 11:02:17 2011 +0100
> @@ -0,0 +1,39 @@
> +/*
> + * 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 _ATS_H_
> +#define _ATS_H_
> +
> +#define ATS_REG_CAP    4
> +#define ATS_REG_CTL    6
> +#define ATS_QUEUE_DEPTH_MASK     0xF
> +#define ATS_ENABLE               (1<<15)
> +
> +struct pci_ats_dev {
> +    struct list_head list;
> +    u16 seg;
> +    u8 bus;
> +    u8 devfn;
> +    u16 ats_queue_depth;    /* ATS device invalidation queue depth */
> +};
> +
> +extern struct list_head ats_devices;
> +extern bool_t ats_enabled;
> +
> +int enable_ats_device(int seg, int bus, int devfn);
> +void disable_ats_device(int seg, int bus, int devfn);
> +
> +#endif /* _ATS_H_ */
> +
> diff -r 119bccb1cc5e -r d422e3cf7976 xen/drivers/passthrough/vtd/extern.h
> --- a/xen/drivers/passthrough/vtd/extern.h    Wed Nov 02 13:53:05 2011 +0100
> +++ b/xen/drivers/passthrough/vtd/extern.h    Thu Nov 03 11:02:17 2011 +0100
> @@ -57,13 +57,9 @@ struct acpi_drhd_unit * iommu_to_drhd(st
>  struct acpi_rhsa_unit * drhd_to_rhsa(struct acpi_drhd_unit *drhd);
>  
>  #ifdef CONFIG_X86_64

Did you overlook this conditional? It and the respective inline functions
and #define-s must be added/moved to the new header too.

> -extern bool_t ats_enabled;
> -
>  struct acpi_drhd_unit * find_ats_dev_drhd(struct iommu *iommu);
>  
>  int ats_device(const struct pci_dev *, const struct acpi_drhd_unit *);
> -int enable_ats_device(int seg, int bus, int devfn);
> -void disable_ats_device(int seg, int bus, int devfn);
>  
>  int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
>                           u64 addr, unsigned int size_order, u64 type);
> diff -r 119bccb1cc5e -r d422e3cf7976 xen/drivers/passthrough/vtd/iommu.c
> --- a/xen/drivers/passthrough/vtd/iommu.c     Wed Nov 02 13:53:05 2011 +0100
> +++ b/xen/drivers/passthrough/vtd/iommu.c     Thu Nov 03 11:02:17 2011 +0100
> @@ -40,6 +40,7 @@
>  #include "dmar.h"
>  #include "extern.h"
>  #include "vtd.h"
> +#include "../ats.h"
>  
>  #ifdef __ia64__
>  #define nr_ioapics              iosapic_get_nr_iosapics()
> diff -r 119bccb1cc5e -r d422e3cf7976 xen/drivers/passthrough/vtd/x86/ats.c
> --- a/xen/drivers/passthrough/vtd/x86/ats.c   Wed Nov 02 13:53:05 2011 +0100
> +++ b/xen/drivers/passthrough/vtd/x86/ats.c   Thu Nov 03 11:02:17 2011 +0100
> @@ -27,51 +27,10 @@
>  #include "../dmar.h"
>  #include "../vtd.h"
>  #include "../extern.h"
> +#include "../../ats.h"
>  
>  static LIST_HEAD(ats_dev_drhd_units);
>  
> -#define ATS_REG_CAP    4
> -#define ATS_REG_CTL    6
> -#define ATS_QUEUE_DEPTH_MASK     0xF
> -#define ATS_ENABLE               (1<<15)
> -
> -struct pci_ats_dev {
> -    struct list_head list;
> -    u16 seg;
> -    u8 bus;
> -    u8 devfn;
> -    u16 ats_queue_depth;    /* ATS device invalidation queue depth */
> -};
> -static LIST_HEAD(ats_devices);
> -
> -static void parse_ats_param(char *s);
> -custom_param("ats", parse_ats_param);
> -
> -bool_t __read_mostly ats_enabled = 1;
> -
> -static void __init parse_ats_param(char *s)
> -{
> -    char *ss;
> -
> -    do {
> -        ss = strchr(s, ',');
> -        if ( ss )
> -            *ss = '\0';
> -
> -        switch ( parse_bool(s) )
> -        {
> -        case 0:
> -            ats_enabled = 0;
> -            break;
> -        case 1:
> -            ats_enabled = 1;
> -            break;
> -        }
> -
> -        s = ss + 1;
> -    } while ( ss );
> -}
> -
>  struct acpi_drhd_unit * find_ats_dev_drhd(struct iommu *iommu)
>  {
>      struct acpi_drhd_unit *drhd;
> @@ -113,97 +72,6 @@ int ats_device(const struct pci_dev *pde
>      return pos;
>  }
>  
> -int enable_ats_device(int seg, int bus, int devfn)
> -{
> -    struct pci_ats_dev *pdev = NULL;
> -    u32 value;
> -    int pos;
> -
> -    pos = pci_find_ext_capability(seg, bus, devfn, PCI_EXT_CAP_ID_ATS);
> -    BUG_ON(!pos);
> -
> -    if ( iommu_verbose )
> -        dprintk(XENLOG_INFO VTDPREFIX,
> -                "%04x:%02x:%02x.%u: ATS capability found\n",
> -                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> -
> -    value = pci_conf_read16(seg, bus, PCI_SLOT(devfn),
> -                            PCI_FUNC(devfn), pos + ATS_REG_CTL);
> -    if ( value & ATS_ENABLE )
> -    {
> -        list_for_each_entry ( pdev, &ats_devices, list )
> -        {
> -            if ( pdev->seg == seg && pdev->bus == bus && pdev->devfn == 
> devfn )
> -            {
> -                pos = 0;
> -                break;
> -            }
> -        }
> -    }
> -    if ( pos )
> -        pdev = xmalloc(struct pci_ats_dev);
> -    if ( !pdev )
> -        return -ENOMEM;
> -
> -    if ( !(value & ATS_ENABLE) )
> -    {
> -        value |= ATS_ENABLE;
> -        pci_conf_write16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -                         pos + ATS_REG_CTL, value);
> -    }
> -
> -    if ( pos )
> -    {
> -        pdev->seg = seg;
> -        pdev->bus = bus;
> -        pdev->devfn = devfn;
> -        value = pci_conf_read16(seg, bus, PCI_SLOT(devfn),
> -                                PCI_FUNC(devfn), pos + ATS_REG_CAP);
> -        pdev->ats_queue_depth = value & ATS_QUEUE_DEPTH_MASK;
> -        list_add(&pdev->list, &ats_devices);
> -    }
> -
> -    if ( iommu_verbose )
> -        dprintk(XENLOG_INFO VTDPREFIX,
> -                "%04x:%02x:%02x.%u: ATS %s enabled\n",
> -                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -                pos ? "is" : "was");
> -
> -    return pos;
> -}
> -
> -void disable_ats_device(int seg, int bus, int devfn)
> -{
> -    struct pci_ats_dev *pdev;
> -    u32 value;
> -    int pos;
> -
> -    pos = pci_find_ext_capability(seg, bus, devfn, PCI_EXT_CAP_ID_ATS);
> -    BUG_ON(!pos);
> -
> -    value = pci_conf_read16(seg, bus, PCI_SLOT(devfn),
> -                            PCI_FUNC(devfn), pos + ATS_REG_CTL);
> -    value &= ~ATS_ENABLE;
> -    pci_conf_write16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> -                     pos + ATS_REG_CTL, value);
> -
> -    list_for_each_entry ( pdev, &ats_devices, list )
> -    {
> -        if ( pdev->seg == seg && pdev->bus == bus && pdev->devfn == devfn )
> -        {
> -            list_del(&pdev->list);
> -            xfree(pdev);
> -            break;
> -        }
> -    }
> -
> -    if ( iommu_verbose )
> -        dprintk(XENLOG_INFO VTDPREFIX,
> -                "%04x:%02x:%02x.%u: ATS is disabled\n",
> -                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> -}
> -
> -
>  static int device_in_domain(struct iommu *iommu, struct pci_ats_dev *pdev, 
> u16 did)
>  {
>      struct root_entry *root_entry = NULL;
> diff -r 119bccb1cc5e -r d422e3cf7976 xen/drivers/passthrough/x86/Makefile
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/xen/drivers/passthrough/x86/Makefile    Thu Nov 03 11:02:17 2011 +0100
> @@ -0,0 +1,1 @@
> +obj-y += ats.o

obj-$(CONFIG_X86_64) += ats.o

The original file got built for 64-bit only, too.

> \ No newline at end of file

This is self-explanatory, isn't it?

> diff -r 119bccb1cc5e -r d422e3cf7976 xen/drivers/passthrough/x86/ats.c
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/xen/drivers/passthrough/x86/ats.c       Thu Nov 03 11:02:17 2011 +0100
> @@ -0,0 +1,139 @@
> +/*
> + * 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.
> + */
> +
> +#include <xen/sched.h>
> +#include <xen/pci.h>
> +#include <xen/pci_regs.h>
> +#include "../ats.h"
> +
> +LIST_HEAD(ats_devices);
> +
> +static void parse_ats_param(char *s);
> +custom_param("ats", parse_ats_param);
> +
> +bool_t __read_mostly ats_enabled = 1;
> +
> +static void __init parse_ats_param(char *s)
> +{
> +    char *ss;
> +
> +    do {
> +        ss = strchr(s, ',');
> +        if ( ss )
> +            *ss = '\0';
> +
> +        switch ( parse_bool(s) )
> +        {
> +        case 0:
> +            ats_enabled = 0;
> +            break;
> +        case 1:
> +            ats_enabled = 1;
> +            break;
> +        }
> +
> +        s = ss + 1;
> +    } while ( ss );
> +}
> +
> +int enable_ats_device(int seg, int bus, int devfn)
> +{
> +    struct pci_ats_dev *pdev = NULL;
> +    u32 value;
> +    int pos;
> +
> +    pos = pci_find_ext_capability(seg, bus, devfn, PCI_EXT_CAP_ID_ATS);
> +    BUG_ON(!pos);
> +
> +    if ( iommu_verbose )
> +        dprintk(XENLOG_INFO,
> +                "%04x:%02x:%02x.%u: ATS capability found\n",

Here and below, the two lines can be folded (together they don't
exceed 80 chars per line) now that the VT-d prefix is gone.

Jan

> +                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +
> +    value = pci_conf_read16(seg, bus, PCI_SLOT(devfn),
> +                            PCI_FUNC(devfn), pos + ATS_REG_CTL);
> +    if ( value & ATS_ENABLE )
> +    {
> +        list_for_each_entry ( pdev, &ats_devices, list )
> +        {
> +            if ( pdev->seg == seg && pdev->bus == bus && pdev->devfn == 
> devfn )
> +            {
> +                pos = 0;
> +                break;
> +            }
> +        }
> +    }
> +    if ( pos )
> +        pdev = xmalloc(struct pci_ats_dev);
> +    if ( !pdev )
> +        return -ENOMEM;
> +
> +    if ( !(value & ATS_ENABLE) )
> +    {
> +        value |= ATS_ENABLE;
> +        pci_conf_write16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> +                         pos + ATS_REG_CTL, value);
> +    }
> +
> +    if ( pos )
> +    {
> +        pdev->seg = seg;
> +        pdev->bus = bus;
> +        pdev->devfn = devfn;
> +        value = pci_conf_read16(seg, bus, PCI_SLOT(devfn),
> +                                PCI_FUNC(devfn), pos + ATS_REG_CAP);
> +        pdev->ats_queue_depth = value & ATS_QUEUE_DEPTH_MASK;
> +        list_add(&pdev->list, &ats_devices);
> +    }
> +
> +    if ( iommu_verbose )
> +        dprintk(XENLOG_INFO,
> +                "%04x:%02x:%02x.%u: ATS %s enabled\n",
> +                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> +                pos ? "is" : "was");
> +
> +    return pos;
> +}
> +
> +void disable_ats_device(int seg, int bus, int devfn)
> +{
> +    struct pci_ats_dev *pdev;
> +    u32 value;
> +    int pos;
> +
> +    pos = pci_find_ext_capability(seg, bus, devfn, PCI_EXT_CAP_ID_ATS);
> +    BUG_ON(!pos);
> +
> +    value = pci_conf_read16(seg, bus, PCI_SLOT(devfn),
> +                            PCI_FUNC(devfn), pos + ATS_REG_CTL);
> +    value &= ~ATS_ENABLE;
> +    pci_conf_write16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> +                     pos + ATS_REG_CTL, value);
> +
> +    list_for_each_entry ( pdev, &ats_devices, list )
> +    {
> +        if ( pdev->seg == seg && pdev->bus == bus && pdev->devfn == devfn )
> +        {
> +            list_del(&pdev->list);
> +            xfree(pdev);
> +            break;
> +        }
> +    }
> +
> +    if ( iommu_verbose )
> +        dprintk(XENLOG_INFO,
> +                "%04x:%02x:%02x.%u: ATS is disabled\n",
> +                seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +}



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