[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/3] remove saving/restoring method in Xend.
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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |