[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


 


Rackspace

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