[PATCH 1/4] pci: Update pci_set_vga_state to call arch functions

[PATCH 1/4] pci: Update pci_set_vga_state to call arch functions

[Mike Travis]

[-- Attachment #1: update-pci --]
[-- Type: text/plain, Size: 3137 bytes --]

Update pci_set_vga_state to call arch dependent functions to enable
Legacy VGA I/O transactions to be redirected to correct target.

Signed-off-by: Mike Travis <travis@sgi.com>

Cc: Dave Airlie <airlied@redhat.com>
Cc: Alex Chiang <achiang@hp.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Robin Holt <holt@sgi.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Adam Jackson <ajax@nwnk.net>
Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Gaetan Nadon <memsize@videotron.ca>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Aaron Plattner <aplattner@nvidia.com>
Cc: Jack Steiner <steiner@sgi.com>
Cc: Tiago Vignatti <tiago.vignatti@nokia.com>
Cc: Cliff Wickman <cpw@sgi.com>
Cc: Matthew Wilcox <willy@linux.intel.com>
Cc: Christian Zander <chzander@nvidia.com>
Cc: Yu Zhao <yu.zhao@intel.com>

Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: x86@kernel.org
---
 drivers/pci/pci.c   |   25 ++++++++++++++++++++++++-
 include/linux/pci.h |    5 +++++
 2 files changed, 29 insertions(+), 1 deletion(-)

--- linux.orig/drivers/pci/pci.c
+++ linux/drivers/pci/pci.c
@@ -2544,6 +2544,23 @@
 	return 0;
 }
 
+/* Some architectures require additional programming to enable VGA */
+static arch_set_vga_state_t arch_set_vga_state;
+
+void pci_register_set_vga_state(arch_set_vga_state_t func)
+{
+	arch_set_vga_state = func;	/* NULL disables */
+}
+
+static int pci_set_vga_state_arch(struct pci_dev *dev, bool decode,
+		      unsigned int command_bits, bool change_bridge)
+{
+	if (arch_set_vga_state)
+		return arch_set_vga_state(dev, decode, command_bits,
+						change_bridge);
+	return 0;
+}
+
 /**
  * pci_set_vga_state - set VGA decode state on device and parents if requested
  * @dev: the PCI device
@@ -2557,9 +2574,15 @@
 	struct pci_bus *bus;
 	struct pci_dev *bridge;
 	u16 cmd;
+	int rc;
 
 	WARN_ON(command_bits & ~(PCI_COMMAND_IO|PCI_COMMAND_MEMORY));
 
+	/* ARCH specific VGA enables */
+	rc = pci_set_vga_state_arch(dev, decode, command_bits, change_bridge);
+	if (rc)
+		return rc;
+
 	pci_read_config_word(dev, PCI_COMMAND, &cmd);
 	if (decode == true)
 		cmd |= command_bits;
@@ -2769,6 +2792,7 @@
 EXPORT_SYMBOL(pci_disable_device);
 EXPORT_SYMBOL(pci_find_capability);
 EXPORT_SYMBOL(pci_bus_find_capability);
+EXPORT_SYMBOL(pci_register_set_vga_state);
 EXPORT_SYMBOL(pci_release_regions);
 EXPORT_SYMBOL(pci_request_regions);
 EXPORT_SYMBOL(pci_request_regions_exclusive);
@@ -2801,4 +2825,3 @@
 EXPORT_SYMBOL(pci_prepare_to_sleep);
 EXPORT_SYMBOL(pci_back_from_sleep);
 EXPORT_SYMBOL_GPL(pci_set_pcie_reset_state);
-
--- linux.orig/include/linux/pci.h
+++ linux/include/linux/pci.h
@@ -942,6 +942,11 @@
 }
 #endif /* CONFIG_PCI_DOMAINS */
 
+/* some architectures require additional setup to direct VGA traffic */
+typedef int (*arch_set_vga_state_t)(struct pci_dev *pdev, bool decode,
+		      unsigned int command_bits, bool change_bridge);
+extern void pci_register_set_vga_state(arch_set_vga_state_t func);
+
 #else /* CONFIG_PCI is not enabled */
 
 /*

-- 

Re: [PATCH 1/4] pci: Update pci_set_vga_state to call arch functions

[Andrew Morton]

On Mon, 18 Jan 2010 12:34:13 -0600
Mike Travis <travis@sgi.com> wrote:

> Update pci_set_vga_state to call arch dependent functions to enable
> Legacy VGA I/O transactions to be redirected to correct target.
> 

Changelog doesn't explain the reason for doing this, but it looks like
that becomes clearer in later patches.

>  
> +/* Some architectures require additional programming to enable VGA */
> +static arch_set_vga_state_t arch_set_vga_state;
> +
> +void pci_register_set_vga_state(arch_set_vga_state_t func)
> +{
> +	arch_set_vga_state = func;	/* NULL disables */
> +}
> +
> +static int pci_set_vga_state_arch(struct pci_dev *dev, bool decode,
> +		      unsigned int command_bits, bool change_bridge)
> +{
> +	if (arch_set_vga_state)
> +		return arch_set_vga_state(dev, decode, command_bits,
> +						change_bridge);
> +	return 0;
> +}

hm, that's not terribly elegant.  It's racy too, although it seems
unlikely that an arch will call pci_set_vga_state_arch() more than
once.

Still, a neater solution might be

int arch_pci_set_vga_state(...) __weak
{
	return 0;
}

and then resolve it at linkage time?

Re: [PATCH 1/4] pci: Update pci_set_vga_state to call arch functions

[Mike Travis]

Andrew Morton wrote:
> On Mon, 18 Jan 2010 12:34:13 -0600
> Mike Travis <travis@sgi.com> wrote:
> 
>> Update pci_set_vga_state to call arch dependent functions to enable
>> Legacy VGA I/O transactions to be redirected to correct target.
>>
> 
> Changelog doesn't explain the reason for doing this, but it looks like
> that becomes clearer in later patches.

You're right, I should have carried more from the intro page into the
actual patch.
> 
>>  
>> +/* Some architectures require additional programming to enable VGA */
>> +static arch_set_vga_state_t arch_set_vga_state;
>> +
>> +void pci_register_set_vga_state(arch_set_vga_state_t func)
>> +{
>> +	arch_set_vga_state = func;	/* NULL disables */
>> +}
>> +
>> +static int pci_set_vga_state_arch(struct pci_dev *dev, bool decode,
>> +		      unsigned int command_bits, bool change_bridge)
>> +{
>> +	if (arch_set_vga_state)
>> +		return arch_set_vga_state(dev, decode, command_bits,
>> +						change_bridge);
>> +	return 0;
>> +}
> 
> hm, that's not terribly elegant.  It's racy too, although it seems
> unlikely that an arch will call pci_set_vga_state_arch() more than
> once.

Yes, it's only set (once) if it is a UV system.

> 
> Still, a neater solution might be
> 
> int arch_pci_set_vga_state(...) __weak
> {
> 	return 0;
> }
> 
> and then resolve it at linkage time?
> 
> 

Wouldn't the linked in stronger function then need to surround
the UV register updating with "if (uv_system)"?

Thanks for the comments Andrew.

Re: [PATCH 1/4] pci: Update pci_set_vga_state to call arch functions

[Andrew Morton]

On Mon, 25 Jan 2010 16:09:52 -0800
Mike Travis <travis@sgi.com> wrote:

> > 
> > Still, a neater solution might be
> > 
> > int arch_pci_set_vga_state(...) __weak
> > {
> > 	return 0;
> > }
> > 
> > and then resolve it at linkage time?
> > 
> > 
> 
> Wouldn't the linked in stronger function then need to surround
> the UV register updating with "if (uv_system)"?

If the file which implements arch_pci_set_vga_state() is present in
vmlinux on non-uv systems then yes.

Presumably that file is arch/x86/kernel/apic/x2apic_uv_x.c, although
that might later become a problem if other sub-architectures a) want to
get at this hook and b) can be linked into vmlinux along with
arch/x86/kernel/apic/x2apic_uv_x.o.  But such a kernel wouldn't
actually link, so we'll need to address it at that stage.


If this is a UV-specific hack which other architectures and
sub-architectures are unlikely to need (seems that way?) then I'd be
going for something whcih is simple and minimal.

[PATCH 2/4] x86_64 UV: Update UV arch to target Legacy VGA I/O correctly.

[Mike Travis]

[-- Attachment #1: update-uv-using-bios --]
[-- Type: text/plain, Size: 4084 bytes --]

Add function to direct Legacy VGA I/O traffic to correct I/O Hub.

Signed-off-by: Mike Travis <travis@sgi.com>

Cc: Dave Airlie <airlied@redhat.com>
Cc: Alex Chiang <achiang@hp.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Robin Holt <holt@sgi.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Adam Jackson <ajax@nwnk.net>
Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Gaetan Nadon <memsize@videotron.ca>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Aaron Plattner <aplattner@nvidia.com>
Cc: Jack Steiner <steiner@sgi.com>
Cc: Tiago Vignatti <tiago.vignatti@nokia.com>
Cc: Cliff Wickman <cpw@sgi.com>
Cc: Matthew Wilcox <willy@linux.intel.com>
Cc: Christian Zander <chzander@nvidia.com>
Cc: Yu Zhao <yu.zhao@intel.com>

Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: x86@kernel.org
---
 arch/x86/include/asm/uv/bios.h     |    4 +++-
 arch/x86/kernel/apic/x2apic_uv_x.c |   30 ++++++++++++++++++++++++++++++
 arch/x86/kernel/bios_uv.c          |   19 +++++++++++++++++++
 3 files changed, 52 insertions(+), 1 deletion(-)

--- linux.orig/arch/x86/include/asm/uv/bios.h
+++ linux/arch/x86/include/asm/uv/bios.h
@@ -36,7 +36,8 @@
 	UV_BIOS_WATCHLIST_ALLOC,
 	UV_BIOS_WATCHLIST_FREE,
 	UV_BIOS_MEMPROTECT,
-	UV_BIOS_GET_PARTITION_ADDR
+	UV_BIOS_GET_PARTITION_ADDR,
+	UV_BIOS_SET_LEGACY_VGA_TARGET
 };
 
 /*
@@ -96,6 +97,7 @@
 extern int uv_bios_mq_watchlist_free(int, int);
 extern s64 uv_bios_change_memprotect(u64, u64, enum uv_memprotect);
 extern s64 uv_bios_reserved_page_pa(u64, u64 *, u64 *, u64 *);
+extern int uv_bios_set_legacy_vga_target(bool decode, int domain, int bus);
 
 extern void uv_bios_init(void);
 
--- linux.orig/arch/x86/kernel/apic/x2apic_uv_x.c
+++ linux/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -20,6 +20,7 @@
 #include <linux/cpu.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/pci.h>
 
 #include <asm/uv/uv_mmrs.h>
 #include <asm/uv/uv_hub.h>
@@ -34,6 +35,8 @@
 
 DEFINE_PER_CPU(int, x2apic_extra_bits);
 
+#define PR_DEVEL(fmt, args...)	pr_devel("%s: " fmt, __func__, args)
+
 static enum uv_system_type uv_system_type;
 static u64 gru_start_paddr, gru_end_paddr;
 int uv_min_hub_revision_id;
@@ -557,6 +560,30 @@
 
 #endif /* !CONFIG_HOTPLUG_CPU */
 
+/* Direct Legacy VGA I/O traffic to designated IOH */
+int uv_set_vga_state(struct pci_dev *pdev, bool decode,
+		      unsigned int command_bits, bool change_bridge)
+{
+	int domain, bus, rc;
+
+	PR_DEVEL("devfn %x decode %d cmd %x chg_brdg %d\n",
+			pdev->devfn, decode, command_bits, change_bridge);
+
+	if (!change_bridge)
+		return 0;
+
+	if ((command_bits & PCI_COMMAND_IO) == 0)
+		return 0;
+
+	domain = pci_domain_nr(pdev->bus);
+	bus = pdev->bus->number;
+
+	rc = uv_bios_set_legacy_vga_target(decode, domain, bus);
+	PR_DEVEL("vga decode %d %x:%x, rc: %d\n", decode, domain, bus, rc);
+
+	return rc;
+}
+
 /*
  * Called on each cpu to initialize the per_cpu UV data area.
  * FIXME: hotplug not supported yet
@@ -717,4 +744,7 @@
 	uv_cpu_init();
 	uv_scir_register_cpu_notifier();
 	proc_mkdir("sgi_uv", NULL);
+
+	/* register Legacy VGA I/O redirection handler */
+	pci_register_set_vga_state(uv_set_vga_state);
 }
--- linux.orig/arch/x86/kernel/bios_uv.c
+++ linux/arch/x86/kernel/bios_uv.c
@@ -163,6 +163,25 @@
 }
 EXPORT_SYMBOL_GPL(uv_bios_freq_base);
 
+/*
+ * uv_bios_set_legacy_vga_target - Set Legacy VGA I/O Target
+ * @decode: true to enable target, false to disable target
+ * @domain: PCI domain number
+ * @bus: PCI bus number
+ *
+ * Returns:
+ *    0: Success
+ *    -EINVAL: Invalid domain or bus number
+ *    -ENOSYS: Capability not available
+ *    -EBUSY: Legacy VGA I/O cannot be retargeted at this time
+ */
+int uv_bios_set_legacy_vga_target(bool decode, int domain, int bus)
+{
+	return uv_bios_call(UV_BIOS_SET_LEGACY_VGA_TARGET,
+				(u64)decode, (u64)domain, (u64)bus, 0, 0);
+}
+EXPORT_SYMBOL_GPL(uv_bios_set_legacy_vga_target);
+
 
 #ifdef CONFIG_EFI
 void uv_bios_init(void)

-- 

Re: [PATCH 2/4] x86_64 UV: Update UV arch to target Legacy VGA I/O correctly.

[Andrew Morton]

On Mon, 18 Jan 2010 12:34:14 -0600
Mike Travis <travis@sgi.com> wrote:

> @@ -717,4 +744,7 @@

Please use `diff -p'.

This is within `void __init uv_system_init(void)'

>  	uv_cpu_init();
>  	uv_scir_register_cpu_notifier();
>  	proc_mkdir("sgi_uv", NULL);
> +
> +	/* register Legacy VGA I/O redirection handler */
> +	pci_register_set_vga_state(uv_set_vga_state);
>  }

So pci_register_set_vga_state() could/should have been __init?

Re: [PATCH 2/4] x86_64 UV: Update UV arch to target Legacy VGA I/O correctly.

[Mike Travis]

Andrew Morton wrote:
> On Mon, 18 Jan 2010 12:34:14 -0600
> Mike Travis <travis@sgi.com> wrote:
> 
>> @@ -717,4 +744,7 @@
> 
> Please use `diff -p'.
> 
> This is within `void __init uv_system_init(void)'
> 
>>  	uv_cpu_init();
>>  	uv_scir_register_cpu_notifier();
>>  	proc_mkdir("sgi_uv", NULL);
>> +
>> +	/* register Legacy VGA I/O redirection handler */
>> +	pci_register_set_vga_state(uv_set_vga_state);
>>  }
> 
> So pci_register_set_vga_state() could/should have been __init?

Good catch.  Do you want me to update and resubmit?

Re: [PATCH 2/4] x86_64 UV: Update UV arch to target Legacy VGA I/O correctly.

[H. Peter Anvin]

On 01/25/2010 04:05 PM, Mike Travis wrote:
>>
>> So pci_register_set_vga_state() could/should have been __init?
> 
> Good catch.  Do you want me to update and resubmit?

Please.  This patchset is still on my review queue, FWIW.

	-hpa

Re: [PATCH 2/4] x86_64 UV: Update UV arch to target Legacy VGA I/O correctly.

[Mike Travis]

H. Peter Anvin wrote:
> On 01/25/2010 04:05 PM, Mike Travis wrote:
>>> So pci_register_set_vga_state() could/should have been __init?
>> Good catch.  Do you want me to update and resubmit?
> 
> Please.  This patchset is still on my review queue, FWIW.
> 
> 	-hpa

Looks like Andrew added a fix-it patch to the stream.

[PATCH 3/4] vgaarb: Fix VGA Arbiter to accept PCI Domains other than 0

[Mike Travis]

[-- Attachment #1: update-vgaarb --]
[-- Type: text/plain, Size: 2611 bytes --]

Update the VGA Arbiter to accept PCI Domains other than 0.

Signed-off-by: Mike Travis <travis@sgi.com>

Cc: Dave Airlie <airlied@redhat.com>
Cc: Alex Chiang <achiang@hp.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Robin Holt <holt@sgi.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Adam Jackson <ajax@nwnk.net>
Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Gaetan Nadon <memsize@videotron.ca>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Aaron Plattner <aplattner@nvidia.com>
Cc: Jack Steiner <steiner@sgi.com>
Cc: Tiago Vignatti <tiago.vignatti@nokia.com>
Cc: Cliff Wickman <cpw@sgi.com>
Cc: Matthew Wilcox <willy@linux.intel.com>
Cc: Christian Zander <chzander@nvidia.com>
Cc: Yu Zhao <yu.zhao@intel.com>

Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: x86@kernel.org
---
 drivers/gpu/vga/vgaarb.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

--- linux.orig/drivers/gpu/vga/vgaarb.c
+++ linux/drivers/gpu/vga/vgaarb.c
@@ -954,6 +954,7 @@
 		}
 
 	} else if (strncmp(curr_pos, "target ", 7) == 0) {
+		struct pci_bus *pbus;
 		unsigned int domain, bus, devfn;
 		struct vga_device *vgadev;
 
@@ -969,18 +970,31 @@
 				ret_val = -EPROTO;
 				goto done;
 			}
+			pr_devel("vgaarb: %s ==> %x:%x:%x.%x\n", curr_pos,
+				domain, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
 
-			pdev = pci_get_bus_and_slot(bus, devfn);
+			pbus = pci_find_bus(domain, bus);
+			pr_devel("vgaarb: pbus %p\n", pbus);
+			if (pbus == NULL) {
+				pr_err("vgaarb: invalid PCI domain and/or bus address %x:%x\n",
+					domain, bus);
+				ret_val = -ENODEV;
+				goto done;
+			}
+			pdev = pci_get_slot(pbus, devfn);
+			pr_devel("vgaarb: pdev %p\n", pdev);
 			if (!pdev) {
-				pr_info("vgaarb: invalid PCI address!\n");
+				pr_err("vgaarb: invalid PCI address %x:%x\n",
+					bus, devfn);
 				ret_val = -ENODEV;
 				goto done;
 			}
 		}
 
 		vgadev = vgadev_find(pdev);
+		pr_devel("vgaarb: vgadev %p\n", vgadev);
 		if (vgadev == NULL) {
-			pr_info("vgaarb: this pci device is not a vga device\n");
+			pr_err("vgaarb: this pci device is not a vga device\n");
 			pci_dev_put(pdev);
 			ret_val = -ENODEV;
 			goto done;
@@ -998,7 +1012,8 @@
 			}
 		}
 		if (i == MAX_USER_CARDS) {
-			pr_err("vgaarb: maximum user cards number reached!\n");
+			pr_err("vgaarb: maximum user cards (%d) number reached!\n",
+				MAX_USER_CARDS);
 			pci_dev_put(pdev);
 			/* XXX: which value to return? */
 			ret_val =  -ENOMEM;

-- 

[PATCH 4/4] vgaarb: Add user selectability of the number of GPUS in a system

[Mike Travis]

[-- Attachment #1: update-vga-max-cards --]
[-- Type: text/plain, Size: 1906 bytes --]

Update the VGA Arbiter to allow the user to select the number
of GPU's supported in a system.

Signed-off-by: Mike Travis <travis@sgi.com>

Cc: Dave Airlie <airlied@redhat.com>
Cc: Alex Chiang <achiang@hp.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Robin Holt <holt@sgi.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Adam Jackson <ajax@nwnk.net>
Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Gaetan Nadon <memsize@videotron.ca>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Aaron Plattner <aplattner@nvidia.com>
Cc: Jack Steiner <steiner@sgi.com>
Cc: Tiago Vignatti <tiago.vignatti@nokia.com>
Cc: Cliff Wickman <cpw@sgi.com>
Cc: Matthew Wilcox <willy@linux.intel.com>
Cc: Christian Zander <chzander@nvidia.com>
Cc: Yu Zhao <yu.zhao@intel.com>

Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: x86@kernel.org
---
 drivers/gpu/vga/Kconfig  |    8 ++++++++
 drivers/gpu/vga/vgaarb.c |    4 ++++
 2 files changed, 12 insertions(+)

--- linux.orig/drivers/gpu/vga/Kconfig
+++ linux/drivers/gpu/vga/Kconfig
@@ -8,3 +8,11 @@
 	  are accessed at same time they need some kind of coordination. Please
 	  see Documentation/vgaarbiter.txt for more details. Select this to
 	  enable VGA arbiter.
+
+config VGA_ARB_MAX_GPUS
+	int "Maximum number of GPUs"
+	default 64
+	depends on VGA_ARB
+	help
+	  Reserves space in the kernel to maintain resource locking for
+	  multiple GPUS.  The overhead for each GPU is very small.
--- linux.orig/drivers/gpu/vga/vgaarb.c
+++ linux/drivers/gpu/vga/vgaarb.c
@@ -688,7 +688,11 @@
  * the arbiter.
  */
 
+#ifdef CONFIG_VGA_ARB_MAX_GPUS
+#define MAX_USER_CARDS         CONFIG_VGA_ARB_MAX_GPUS
+#else
 #define MAX_USER_CARDS         16
+#endif
 #define PCI_INVALID_CARD       ((struct pci_dev *)-1UL)
 
 /*

--