[PATCH] vgaarb: Add module param to allow for choosing the boot VGA device

[PATCH] vgaarb: Add module param to allow for choosing the boot VGA device

[Cal Peake]

My first attempt at solving this problem was based on a bit of a 
misunderstanding of the code and, while it worked, could be much improved.

This is a new attempt that creates the module parameter 'vgaarb.bootdev' 
that can be passed a PCI ID (e.g. 0a:00.0) that the VGA arbiter will 
attempt to use as the boot VGA device over any other eligible devices.

If the passed ID is invalid or is an ineligible device, the arbiter will 
fallback to its normal method of trying to find the preferred device for 
the job.

I've tested it thoroughly and it is working very well for me.

The patch is available below for review and, if found acceptable, can be 
pulled via:

  git pull https://github.com/peake/linux.git vgaarb-2

-- 
Cal Peake

Signed-off-by: Cal Peake <cp@absolutedigital.net>

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2522b11e593f..21ac87f4a8a9 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6518,6 +6518,13 @@
 			This is actually a boot loader parameter; the value is
 			passed to the kernel using a special protocol.
 
+	vgaarb.bootdev=	[PCI] Specify the PCI ID (e.g. 0e:00.0) of the
+			device to use as the boot VGA device, overriding
+			the heuristic used to normally determine which
+			of the eligible VGA devices to use. If the device
+			specified is not valid or not eligible, then we
+			fallback to the heuristic.
+
 	vm_debug[=options]	[KNL] Available with CONFIG_DEBUG_VM=y.
 			May slow down system boot speed, especially when
 			enabled on systems with a large amount of memory.
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index f80b6ec88dc3..d3689b7dc63d 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -35,6 +35,34 @@
 
 #include <linux/vgaarb.h>
 
+static char *bootdev __initdata;
+module_param(bootdev, charp, 0);
+MODULE_PARM_DESC(bootdev, "Force boot device to the specified PCI ID");
+
+/*
+ * Initialize to the last possible ID to have things work as normal
+ * when no 'bootdev' option is supplied. We especially do not want
+ * this to be zero (0) since that is a valid PCI ID (00:00.0).
+ */
+static u16 bootdev_id = 0xffff;
+
+static void __init parse_bootdev(char *input)
+{
+	unsigned int bus, dev, func;
+	int ret;
+
+	if (input == NULL)
+		return;
+
+	ret = sscanf(input, "%x:%x.%x", &bus, &dev, &func);
+	if (ret != 3) {
+		pr_warn("Improperly formatted PCI ID: %s\n", input);
+		return;
+	}
+
+	bootdev_id = PCI_DEVID(bus, PCI_DEVFN(dev, func));
+}
+
 static void vga_arbiter_notify_clients(void);
 /*
  * We keep a list of all vga devices in the system to speed
@@ -53,6 +81,7 @@ struct vga_device {
 	bool bridge_has_one_vga;
 	bool is_firmware_default;	/* device selected by firmware */
 	unsigned int (*set_decode)(struct pci_dev *pdev, bool decode);
+	bool is_chosen_one;		/* device specified on command line */
 };
 
 static LIST_HEAD(vga_list);
@@ -605,6 +634,7 @@ static bool vga_is_boot_device(struct vga_device *vgadev)
 
 	/*
 	 * We select the default VGA device in this order:
+	 *   User specified device (see module param bootdev=)
 	 *   Firmware framebuffer (see vga_arb_select_default_device())
 	 *   Legacy VGA device (owns VGA_RSRC_LEGACY_MASK)
 	 *   Non-legacy integrated device (see vga_arb_select_default_device())
@@ -612,6 +642,14 @@ static bool vga_is_boot_device(struct vga_device *vgadev)
 	 *   Other device (see vga_arb_select_default_device())
 	 */
 
+	if (boot_vga && boot_vga->is_chosen_one)
+		return false;
+
+	if (bootdev_id == PCI_DEVID(pdev->bus->number, pdev->devfn)) {
+		vgadev->is_chosen_one = true;
+		return true;
+	}
+
 	/*
 	 * We always prefer a firmware default device, so if we've already
 	 * found one, there's no need to consider vgadev.
@@ -1544,6 +1582,8 @@ static int __init vga_arb_device_init(void)
 	int rc;
 	struct pci_dev *pdev;
 
+	parse_bootdev(bootdev);
+
 	rc = misc_register(&vga_arb_device);
 	if (rc < 0)
 		pr_err("error %d registering device\n", rc);

Re: [PATCH] vgaarb: Add module param to allow for choosing the boot VGA device

[Bjorn Helgaas]

[+cc linux-pci]

On Mon, Jul 04, 2022 at 12:33:50AM -0400, Cal Peake wrote:
> My first attempt at solving this problem was based on a bit of a 
> misunderstanding of the code and, while it worked, could be much improved.

Can you outline the problem this solves in the commit log?

> This is a new attempt that creates the module parameter 'vgaarb.bootdev' 
> that can be passed a PCI ID (e.g. 0a:00.0) that the VGA arbiter will 
> attempt to use as the boot VGA device over any other eligible devices.
> 
> If the passed ID is invalid or is an ineligible device, the arbiter will 
> fallback to its normal method of trying to find the preferred device for 
> the job.
> 
> I've tested it thoroughly and it is working very well for me.
> 
> The patch is available below for review and, if found acceptable, can be 
> pulled via:
> 
>   git pull https://github.com/peake/linux.git vgaarb-2
> 
> -- 
> Cal Peake
> 
> Signed-off-by: Cal Peake <cp@absolutedigital.net>
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 2522b11e593f..21ac87f4a8a9 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6518,6 +6518,13 @@
>  			This is actually a boot loader parameter; the value is
>  			passed to the kernel using a special protocol.
>  
> +	vgaarb.bootdev=	[PCI] Specify the PCI ID (e.g. 0e:00.0) of the
> +			device to use as the boot VGA device, overriding
> +			the heuristic used to normally determine which
> +			of the eligible VGA devices to use. If the device
> +			specified is not valid or not eligible, then we
> +			fallback to the heuristic.
> +
>  	vm_debug[=options]	[KNL] Available with CONFIG_DEBUG_VM=y.
>  			May slow down system boot speed, especially when
>  			enabled on systems with a large amount of memory.
> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> index f80b6ec88dc3..d3689b7dc63d 100644
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -35,6 +35,34 @@
>  
>  #include <linux/vgaarb.h>
>  
> +static char *bootdev __initdata;
> +module_param(bootdev, charp, 0);
> +MODULE_PARM_DESC(bootdev, "Force boot device to the specified PCI ID");
> +
> +/*
> + * Initialize to the last possible ID to have things work as normal
> + * when no 'bootdev' option is supplied. We especially do not want
> + * this to be zero (0) since that is a valid PCI ID (00:00.0).
> + */
> +static u16 bootdev_id = 0xffff;
> +
> +static void __init parse_bootdev(char *input)
> +{
> +	unsigned int bus, dev, func;
> +	int ret;
> +
> +	if (input == NULL)
> +		return;
> +
> +	ret = sscanf(input, "%x:%x.%x", &bus, &dev, &func);
> +	if (ret != 3) {
> +		pr_warn("Improperly formatted PCI ID: %s\n", input);
> +		return;
> +	}
> +
> +	bootdev_id = PCI_DEVID(bus, PCI_DEVFN(dev, func));
> +}
> +
>  static void vga_arbiter_notify_clients(void);
>  /*
>   * We keep a list of all vga devices in the system to speed
> @@ -53,6 +81,7 @@ struct vga_device {
>  	bool bridge_has_one_vga;
>  	bool is_firmware_default;	/* device selected by firmware */
>  	unsigned int (*set_decode)(struct pci_dev *pdev, bool decode);
> +	bool is_chosen_one;		/* device specified on command line */
>  };
>  
>  static LIST_HEAD(vga_list);
> @@ -605,6 +634,7 @@ static bool vga_is_boot_device(struct vga_device *vgadev)
>  
>  	/*
>  	 * We select the default VGA device in this order:
> +	 *   User specified device (see module param bootdev=)
>  	 *   Firmware framebuffer (see vga_arb_select_default_device())
>  	 *   Legacy VGA device (owns VGA_RSRC_LEGACY_MASK)
>  	 *   Non-legacy integrated device (see vga_arb_select_default_device())
> @@ -612,6 +642,14 @@ static bool vga_is_boot_device(struct vga_device *vgadev)
>  	 *   Other device (see vga_arb_select_default_device())
>  	 */
>  
> +	if (boot_vga && boot_vga->is_chosen_one)
> +		return false;
> +
> +	if (bootdev_id == PCI_DEVID(pdev->bus->number, pdev->devfn)) {
> +		vgadev->is_chosen_one = true;
> +		return true;
> +	}
> +
>  	/*
>  	 * We always prefer a firmware default device, so if we've already
>  	 * found one, there's no need to consider vgadev.
> @@ -1544,6 +1582,8 @@ static int __init vga_arb_device_init(void)
>  	int rc;
>  	struct pci_dev *pdev;
>  
> +	parse_bootdev(bootdev);
> +
>  	rc = misc_register(&vga_arb_device);
>  	if (rc < 0)
>  		pr_err("error %d registering device\n", rc);