Xen PCIE hotplug VTD handling

Xen PCIE hotplug VTD handling

[Wang, Winston L]

[-- Attachment #1.1: Type: text/plain, Size: 319 bytes --]

Hi ,

Attached please review the  patch  handling  PCIE hotplug VTD support
by  adjusting VTD remapping entry after pcie hot add and removal.


Signed-off-by: Winston Wang winston.l.wang@intel.com<mailto:winston.l.wang@intel.com>

Signed-off-by: Allen Kay allen.m.kay@intel.com<mailto:allen.m.kay@intel.com>


[-- Attachment #1.2: Type: text/html, Size: 6271 bytes --]

[-- Attachment #2: xen_pcie_vtd.patch --]
[-- Type: application/octet-stream, Size: 2335 bytes --]

diff -p -r -u /usr/src/xen-unstable.hg/linux-2.6-pvops.git.org/drivers/pci/hotplug/pciehp_ctrl.c /usr/src/xen-unstable.hg/linux-2.6-pvops.git/drivers/pci/hotplug/pciehp_ctrl.c
--- /usr/src/xen-unstable.hg/linux-2.6-pvops.git.org/drivers/pci/hotplug/pciehp_ctrl.c	2009-12-14 06:19:20.000000000 -0500
+++ /usr/src/xen-unstable.hg/linux-2.6-pvops.git/drivers/pci/hotplug/pciehp_ctrl.c	2009-12-14 08:06:42.000000000 -0500
@@ -34,6 +34,7 @@
 #include <linux/workqueue.h>
 #include "../pci.h"
 #include "pciehp.h"
+#include <asm/xen/hypercall.h>
 
 static void interrupt_event_handler(struct work_struct *work);
 
@@ -207,9 +208,16 @@ static void set_slot_off(struct controll
 static int board_added(struct slot *p_slot)
 {
 	int retval = 0;
+        int r = 0;
+
 	struct controller *ctrl = p_slot->ctrl;
 	struct pci_bus *parent = ctrl->pci_dev->subordinate;
 
+        struct physdev_manage_pci manage_pci = {
+                 .bus   = p_slot->bus,
+                 .devfn = p_slot->device,
+         };
+
 	ctrl_dbg(ctrl, "%s: slot device, slot offset, hp slot = %d, %d, %d\n",
 		 __func__, p_slot->device, ctrl->slot_device_offset,
 		 p_slot->hp_slot);
@@ -254,7 +262,15 @@ static int board_added(struct slot *p_sl
 	if (PWR_LED(ctrl))
   		p_slot->hpc_ops->green_led_on(p_slot);
 
-	return 0;
+
+        /*
+         * Add xen VTD device
+         */
+
+        r = HYPERVISOR_physdev_op(PHYSDEVOP_manage_pci_add,&manage_pci);
+        ctrl_dbg(ctrl, "HYPERVISOR_physdev_op add return\n",r);
+        
+        return 0;
 
 err_exit:
 	set_slot_off(ctrl, p_slot);
@@ -268,7 +284,13 @@ err_exit:
 static int remove_board(struct slot *p_slot)
 {
 	int retval = 0;
-	struct controller *ctrl = p_slot->ctrl;
+        int r = 0;
+
+	struct physdev_manage_pci manage_pci = {
+                 .bus   = p_slot->bus,
+                 .devfn = p_slot->device,};
+       
+        struct controller *ctrl = p_slot->ctrl;
 
 	retval = pciehp_unconfigure_device(p_slot);
 	if (retval)
@@ -296,6 +318,13 @@ static int remove_board(struct slot *p_s
 		/* turn off Green LED */
 		p_slot->hpc_ops->green_led_off(p_slot);
 
+        /*
+         * Remove xen VTD device
+         */
+
+        r = HYPERVISOR_physdev_op(PHYSDEVOP_manage_pci_remove,&manage_pci);
+        ctrl_dbg(ctrl,"HYPERVISOR_physdev_op remove return\n",r);
+
 	return 0;
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: Xen PCIE hotplug VTD handling

[Jeremy Fitzhardinge]

On 12/15/2009 02:02 PM, Wang, Winston L wrote:
>
> Hi ,
>
> Attached please review the  patch  handling  PCIE hotplug VTD support
>
> by  adjusting VTD remapping entry after pcie hot add and removal.
>

This patch needs serious rethinking.  You can't just plonk Xen code into 
the middle of a generic driver.  You need to find some other way to get 
callbacks so you can do the appropriate Xen hypercalls, iff the kernel 
is actually running under Xen.

> Signed-off-by: Winston Wang winston.l.wang@intel.com 
> <mailto:winston.l.wang@intel.com>
>
> Signed-off-by: Allen Kay allen.m.kay@intel.com 
> <mailto:allen.m.kay@intel.com>
>
> diff -p -r -u 
> /usr/src/xen-unstable.hg/linux-2.6-pvops.git.org/drivers/pci/hotplug/pciehp_ctrl.c 
> /usr/src/xen-unstable.hg/linux-2.6-pvops.git/drivers/pci/hotplug/pciehp_ctrl.c 
> --- 
> /usr/src/xen-unstable.hg/linux-2.6-pvops.git.org/drivers/pci/hotplug/pciehp_ctrl.c 
> 2009-12-14 06:19:20.000000000 -0500 +++ 
> /usr/src/xen-unstable.hg/linux-2.6-pvops.git/drivers/pci/hotplug/pciehp_ctrl.c 
> 2009-12-14 08:06:42.000000000 -0500 

The standard way to generate patches is with just the kernel source 
directory starting the paths : "linux-base/..." and "linux-mychanges/...".

> @@ -34,6 +34,7 @@ #include <linux/workqueue.h> #include "../pci.h" 
> #include "pciehp.h" +#include <asm/xen/hypercall.h> 

Sticking Xen-specific changes directly into non-Xen files is not a good 
idea.   Particularly since this is not an architecture-specific file, so 
this will fail to compile on non-x86 systems.

> static void interrupt_event_handler(struct work_struct *work);
>
> @@ -207,9 +208,16 @@ static void set_slot_off(struct controll
> static int board_added(struct slot *p_slot)
> {
> int retval = 0;
> + int r = 0;
> +
> struct controller *ctrl = p_slot->ctrl;
> struct pci_bus *parent = ctrl->pci_dev->subordinate;
>
> + struct physdev_manage_pci manage_pci = {
> + .bus = p_slot->bus,
> + .devfn = p_slot->device,
> + };
> +
> ctrl_dbg(ctrl, "%s: slot device, slot offset, hp slot = %d, %d, %d\n",
> __func__, p_slot->device, ctrl->slot_device_offset,
> p_slot->hp_slot);
> @@ -254,7 +262,15 @@ static int board_added(struct slot *p_sl
> if (PWR_LED(ctrl))
> p_slot->hpc_ops->green_led_on(p_slot);
>
> - return 0;
> +
> + /*
> + * Add xen VTD device
> + */
> +
> + r = HYPERVISOR_physdev_op(PHYSDEVOP_manage_pci_add,&manage_pci);
> + ctrl_dbg(ctrl, "HYPERVISOR_physdev_op add return\n",r);
> +
> + return 0;

You can't stick a naked hypercall in here.  It will crash if you're not 
running under Xen.  A pvops kernel can choose at runtime whether it is 
running under Xen, KVM, bare hardware, etc.

Also, you haven't even made this depend on CONFIG_XEN, so it will 
attempt to do the hypercall completely unconditionally, even if building 
a non-Xen kernel.

>
> err_exit:
> set_slot_off(ctrl, p_slot);
> @@ -268,7 +284,13 @@ err_exit:
> static int remove_board(struct slot *p_slot)
> {
> int retval = 0;
> - struct controller *ctrl = p_slot->ctrl;
> + int r = 0;
> +
> + struct physdev_manage_pci manage_pci = {
> + .bus = p_slot->bus,
> + .devfn = p_slot->device,};
> +
> + struct controller *ctrl = p_slot->ctrl;
>
> retval = pciehp_unconfigure_device(p_slot);
> if (retval)
> @@ -296,6 +318,13 @@ static int remove_board(struct slot *p_s
> /* turn off Green LED */
> p_slot->hpc_ops->green_led_off(p_slot);
>
> + /*
> + * Remove xen VTD device
> + */
> +
> + r = HYPERVISOR_physdev_op(PHYSDEVOP_manage_pci_remove,&manage_pci);
> + ctrl_dbg(ctrl,"HYPERVISOR_physdev_op remove return\n",r);
> +

Same comments apply here.

     J