[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] Re: Xen PCIE hotplug VTD handling



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@xxxxxxxxx <mailto:winston.l.wang@xxxxxxxxx>

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

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

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.