Hi Yuji,
I have two comments.
1. Is the "if" statement necessary?
I think that the "if" statement is not necessary.
Because, if existing_dev_info is None, pci_state is "Initialising"
certainly. Please see the line: 790-791 in XendDomainInfo.py.
pci_device_configure@xxxxxxxxxxxxxxxxx
@@ -829,15 +838,22 @@
# If pci platform does not exist, create and exit.
if existing_dev_info is None:
self.device_create(dev_sxp)
+ if self.info.is_hvm():
+ if pci_state == 'Initialising': <--- here
+ # HVM PCI device attachment
+ self.pci_device_attach(dev_sxp)
return True
pci_device_configure@xxxxxxxxxxxxxxxxx
line:790-791
if existing_dev_info is None and pci_state != 'Initialising':
raise XendError("Cannot detach when pci platform does not exist")
2. Tab indents are included.
+def xenbusStatusCallback(statusPath, ev, result):
+ log.debug("xenbusStatusCallback %s.", statusPath)
+
+ status = xstransact.Read(statusPath)
+
+ if status == str(xenbusState['Connected']):
+ result['status'] = Connected
+ elif status == str(xenbusState['Initialised']):
+ result['status'] = Initialised <---- here
+ elif status == str(xenbusState['Reconfigured']):
+ result['status'] = Reconfigured <---- here
+ else:
+ return 1
Best regards,
Kan
Wed, 22 Apr 2009 11:20:21 +0900, Yuji Shimada wrote:
>This patch removes Xend method which saves/restores PCI configuration
>space.
>And this patch modifies the timing of saving/restoring configuration
>space like below.
>
> When pciback is bound to devices.
> - Pciback saves configuration space.
>
> When pciback is unbound to devices.
> - Pciback restores configuration space.
>
> When guest OS boots or a device is hotadded.
> - Pciback restores configuration space.
> - Pciback changes state of backend device to Initialised/Reconfigured.
> - Xend waits for the transition to Initialised/Reconfigured.
>
> When guest OS shutdowns or a device is hotremoved.
> - Pciback restores configuration space.
> - Xend resets devices.
> * If D-state of the device is not D0, the state is changed to D0
> before resetting the device.
> - Xend deassigns devices.
>
> * Essentially, devices should be reset before configuration space is
> restored. But it needs big modifications. Applying these patches,
> configuration space is restored when guest OS boots, a device is
> hotadded or pciback is unbound. So it has no matter.
>
>Thanks,
>--
>Yuji Shimada
>
>
>Signed-off-by: Yuji Shimada <shimada-yxb@xxxxxxxxxxxxxxx>
>
>diff -r 94ffd85005c5 tools/python/xen/util/pci.py
>--- a/tools/python/xen/util/pci.py Tue Apr 14 15:23:53 2009 +0100
>+++ b/tools/python/xen/util/pci.py Tue Apr 21 16:51:20 2009 +0900
>@@ -70,6 +70,7 @@
> PCI_PM_CTRL_NO_SOFT_RESET = 0x0008
> PCI_PM_CTRL_STATE_MASK = 0x0003
> PCI_D3hot = 3
>+PCI_D2 = 2
> PCI_D0hot = 0
>
> VENDOR_INTEL = 0x8086
>@@ -209,33 +210,6 @@
> finally:
> lspci_info_lock.release()
>
>-def save_pci_conf_space(devs_string):
>- pci_list = []
>- cfg_list = []
>- sysfs_mnt = find_sysfs_mnt()
>- for pci_str in devs_string:
>- pci_path = sysfs_mnt + SYSFS_PCI_DEVS_PATH + '/' + pci_str + \
>- SYSFS_PCI_DEV_CONFIG_PATH
>- fd = os.open(pci_path, os.O_RDONLY)
>- configs = []
>- for i in range(0, 256, 4):
>- configs = configs + [os.read(fd,4)]
>- os.close(fd)
>- pci_list = pci_list + [pci_path]
>- cfg_list = cfg_list + [configs]
>- return (pci_list, cfg_list)
>-
>-def restore_pci_conf_space(pci_cfg_list):
>- pci_list = pci_cfg_list[0]
>- cfg_list = pci_cfg_list[1]
>- for i in range(0, len(pci_list)):
>- pci_path = pci_list[i]
>- configs = cfg_list[i]
>- fd = os.open(pci_path, os.O_WRONLY)
>- for dw in configs:
>- os.write(fd, dw)
>- os.close(fd)
>-
> def find_all_devices_owned_by_pciback():
> sysfs_mnt = find_sysfs_mnt()
> pciback_path = sysfs_mnt + SYSFS_PCIBACK_PATH
>@@ -476,9 +450,6 @@
> return dev_list
>
> def do_secondary_bus_reset(self, target_bus, devs):
>- # Save the config spaces of all the devices behind the bus.
>- (pci_list, cfg_list) = save_pci_conf_space(devs)
>-
> #Do the Secondary Bus Reset
> sysfs_mnt = find_sysfs_mnt()
> parent_path = sysfs_mnt + SYSFS_PCI_DEVS_PATH + '/' + \
>@@ -498,9 +469,6 @@
> time.sleep(0.100)
> os.close(fd)
>
>- # Restore the config spaces
>- restore_pci_conf_space((pci_list, cfg_list))
>-
> def do_Dstate_transition(self):
> pos = self.find_cap_offset(PCI_CAP_ID_PM)
> if pos == 0:
>@@ -513,8 +481,6 @@
> if (pm_ctl & PCI_PM_CTRL_NO_SOFT_RESET) == 1:
> return False
>
>- (pci_list, cfg_list) = save_pci_conf_space([self.name])
>-
> # Enter D3hot
> pm_ctl &= ~PCI_PM_CTRL_STATE_MASK
> pm_ctl |= PCI_D3hot
>@@ -527,9 +493,25 @@
> self.pci_conf_write32(pos + PCI_PM_CTRL, pm_ctl)
> time.sleep(0.010)
>
>- restore_pci_conf_space((pci_list, cfg_list))
> return True
>
>+ def do_Dstate_reset(self):
>+ pos = self.find_cap_offset(PCI_CAP_ID_PM)
>+ if pos == 0:
>+ return
>+
>+ pm_ctl = self.pci_conf_read32(pos + PCI_PM_CTRL)
>+ cur_state = pm_ctl & PCI_PM_CTRL_STATE_MASK
>+ if (cur_state) != 0:
>+ # From not D0 to D0
>+ pm_ctl &= ~PCI_PM_CTRL_STATE_MASK
>+ pm_ctl |= PCI_D0hot
>+ self.pci_conf_write32(pos + PCI_PM_CTRL, pm_ctl)
>+ if cur_state == PCI_D3hot:
>+ time.sleep(0.010)
>+ if cur_state == PCI_D2:
>+ time.sleep(0.0002)
>+
> def do_vendor_specific_FLR_method(self):
> pos = self.find_cap_offset(PCI_CAP_ID_VENDOR_SPECIFIC_CAP)
> if pos == 0:
>@@ -543,13 +525,9 @@
> if class_id != PCI_CLASS_ID_USB:
> return
>
>- (pci_list, cfg_list) = save_pci_conf_space([self.name])
>-
> self.pci_conf_write8(pos + PCI_USB_FLRCTRL, 1)
> time.sleep(0.100)
>
>- restore_pci_conf_space((pci_list, cfg_list))
>-
> def do_FLR_for_integrated_device(self):
> if not self.do_Dstate_transition():
> self.do_vendor_specific_FLR_method()
>@@ -725,15 +703,16 @@
> def do_FLR(self):
> """ Perform FLR (Functional Level Reset) for the device.
> """
>+ # Set power management state to D0
>+ self.do_Dstate_reset()
>+
> if self.dev_type == DEV_TYPE_PCIe_ENDPOINT:
> # If PCIe device supports FLR, we use it.
> if self.pcie_flr:
>- (pci_list, cfg_list) = save_pci_conf_space([self.name])
> pos = self.find_cap_offset(PCI_CAP_ID_EXP)
> self.pci_conf_write32(pos + PCI_EXP_DEVCTL,
>PCI_EXP_DEVCTL_FLR)
> # We must sleep at least 100ms for the completion of FLR
> time.sleep(0.100)
>- restore_pci_conf_space((pci_list, cfg_list))
> else:
> if self.bus == 0:
> self.do_FLR_for_integrated_device()
>@@ -749,12 +728,10 @@
> else:
> # For PCI device on host bus, we test "PCI Advanced
>Capabilities".
> if self.bus == 0 and self.pci_af_flr:
>- (pci_list, cfg_list) = save_pci_conf_space([self.name])
> # We use Advanced Capability to do FLR.
> pos = self.find_cap_offset(PCI_CAP_ID_AF)
> self.pci_conf_write8(pos + PCI_AF_CTL, PCI_AF_CTL_FLR)
> time.sleep(0.100)
>- restore_pci_conf_space((pci_list, cfg_list))
> else:
> if self.bus == 0:
> self.do_FLR_for_integrated_device()
>diff -r 94ffd85005c5 tools/python/xen/xend/XendDomainInfo.py
>--- a/tools/python/xen/xend/XendDomainInfo.py Tue Apr 14 15:23:53 2009 +0100
>+++ b/tools/python/xen/xend/XendDomainInfo.py Tue Apr 21 16:51:20 2009 +0900
>@@ -764,6 +764,23 @@
> return self.getDeviceController(dev_type).sxpr(devid)
>
>
>+ def pci_device_attach(self, dev_sxp):
>+ """PCI device attachment.
>+
>+ @param dev_sxp: device configuration
>+ @type dev_sxp: SXP object (parsed config)
>+ """
>+ pci_dev = sxp.children(dev_sxp, 'dev')[0]
>+ dev_config = self.info.pci_convert_sxp_to_dict(dev_sxp)
>+
>+ vslot = self.hvm_pci_device_create(dev_config)
>+ # Update vslot
>+ dev['vslot'] = vslot
>+ for n in sxp.children(pci_dev):
>+ if(n[0] == 'vslot'):
>+ n[1] = vslot
>+
>+
> def pci_device_configure(self, dev_sxp, devid = 0):
> """Configure an existing pci device.
>
>@@ -794,15 +811,7 @@
>
> # Do HVM specific processing
> if self.info.is_hvm():
>- if pci_state == 'Initialising':
>- # HVM PCI device attachment
>- vslot = self.hvm_pci_device_create(dev_config)
>- # Update vslot
>- dev['vslot'] = vslot
>- for n in sxp.children(pci_dev):
>- if(n[0] == 'vslot'):
>- n[1] = vslot
>- else:
>+ if pci_state == 'Closing':
> # HVM PCI device detachment
> existing_dev_uuid = sxp.child_value(existing_dev_info,
>'uuid')
> existing_pci_conf = self.info['devices'][existing_dev_uuid
>][1]
>@@ -829,15 +838,22 @@
> # If pci platform does not exist, create and exit.
> if existing_dev_info is None:
> self.device_create(dev_sxp)
>+ if self.info.is_hvm():
>+ if pci_state == 'Initialising':
>+ # HVM PCI device attachment
>+ self.pci_device_attach(dev_sxp)
> return True
>
> if self.domid is not None:
> # use DevController.reconfigureDevice to change device config
> dev_control = self.getDeviceController(dev_class)
> dev_uuid = dev_control.reconfigureDevice(devid, dev_config)
>- if not self.info.is_hvm():
>- # in PV case, wait until backend state becomes connected.
>- dev_control.waitForDevice_reconfigure(devid)
>+ dev_control.waitForDevice_reconfigure(devid)
>+
>+ if self.info.is_hvm():
>+ if pci_state == 'Initialising':
>+ # HVM PCI device attachment
>+ self.pci_device_attach(dev_sxp)
> num_devs = dev_control.cleanupDevice(devid)
>
> # update XendConfig with new device info
>diff -r 94ffd85005c5 tools/python/xen/xend/server/DevConstants.py
>--- a/tools/python/xen/xend/server/DevConstants.py Tue Apr 14 15:23:53 2009
> +0100
>+++ b/tools/python/xen/xend/server/DevConstants.py Tue Apr 21 16:51:20 2009
> +0900
>@@ -29,6 +29,8 @@
> Timeout = 4
> Busy = 5
> Disconnected = 6
>+Initialised = 7
>+Reconfigured = 8
>
> xenbusState = {
> 'Unknown' : 0,
>diff -r 94ffd85005c5 tools/python/xen/xend/server/pciif.py
>--- a/tools/python/xen/xend/server/pciif.py Tue Apr 14 15:23:53 2009 +0100
>+++ b/tools/python/xen/xend/server/pciif.py Tue Apr 21 16:51:20 2009 +0900
>@@ -17,6 +17,7 @@
> #=========================================================================
>===
>
>
>+from threading import Event
> import types
> import time
>
>@@ -27,7 +28,7 @@
> from xen.xend.XendConstants import *
>
> from xen.xend.server.DevController import DevController
>-from xen.xend.server.DevConstants import xenbusState
>+from xen.xend.server.DevConstants import *
>
> import xen.lowlevel.xc
>
>@@ -489,13 +490,16 @@
> "bind your slot/device to the PCI backend using sysfs" \
> )%(dev.name))
>
>- if not self.vm.info.is_hvm():
>- pci_str = "0x%x, 0x%x, 0x%x, 0x%x" % (domain, bus, slot, func)
>- bdf = xc.deassign_device(fe_domid, pci_str)
>- if bdf > 0:
>- raise VmError("Failed to deassign device from IOMMU (%x:%x
>.%x)"
>- % (bus, slot, func))
>- log.debug("pci: deassign device %x:%x.%x" % (bus, slot, func))
>+ # Need to do FLR here before deassign device in order to terminate
>+ # DMA transaction, etc
>+ dev.do_FLR()
>+
>+ pci_str = "0x%x, 0x%x, 0x%x, 0x%x" % (domain, bus, slot, func)
>+ bdf = xc.deassign_device(fe_domid, pci_str)
>+ if bdf > 0:
>+ raise VmError("Failed to deassign device from IOMMU (%x:%x.%x)"
>+ % (bus, slot, func))
>+ log.debug("pci: Deassign device %x:%x.%x" % (bus, slot, func))
>
> for (start, size) in dev.ioports:
> log.debug('pci: disabling ioport 0x%x/0x%x'%(start,size))
>@@ -528,7 +532,6 @@
> if rc<0:
> raise VmError(('pci: failed to configure irq on device '+
> '%s - errno=%d')%(dev.name,rc))
>- dev.do_FLR()
>
> def cleanupDevice(self, devid):
> """ Detach I/O resources for device and cleanup xenstore nodes
>@@ -603,8 +606,53 @@
> except:
> log.exception("Unwatching aerState failed.")
>
>- def waitForBackend(self,devid):
>- return (0, "ok - no hotplug")
>+ def waitForBackend(self, devid):
>+ return self.waitForBackend_reconfigure(devid)
>
> def migrate(self, config, network, dst, step, domName):
> raise XendError('Migration not permitted with assigned PCI device.')
>+
>+ def createDevice(self, config):
>+ """@see DevController.createDevice"""
>+ devid = DevController.createDevice(self, config)
>+ if devid:
>+ self.waitForDevice(devid)
>+
>+ return devid
>+
>+ def waitForBackend_reconfigure(self, devid):
>+ frontpath = self.frontendPath(devid)
>+ backpath = xstransact.Read(frontpath, "backend")
>+ if backpath:
>+ statusPath = backpath + '/' + "state"
>+ ev = Event()
>+ result = { 'status': Timeout }
>+
>+ xswatch(statusPath, xenbusStatusCallback, ev, result)
>+
>+ ev.wait(DEVICE_CREATE_TIMEOUT)
>+
>+ return (result['status'], None)
>+ else:
>+ return (Missing, None)
>+
>+
>+def xenbusStatusCallback(statusPath, ev, result):
>+ log.debug("xenbusStatusCallback %s.", statusPath)
>+
>+ status = xstransact.Read(statusPath)
>+
>+ if status == str(xenbusState['Connected']):
>+ result['status'] = Connected
>+ elif status == str(xenbusState['Initialised']):
>+ result['status'] = Initialised
>+ elif status == str(xenbusState['Reconfigured']):
>+ result['status'] = Reconfigured
>+ else:
>+ return 1
>+
>+ log.debug("xenbusStatusCallback %d.", result['status'])
>+
>+ ev.set()
>+ return 0
>+
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@xxxxxxxxxxxxxxxxxxx
>http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|