WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 2/3] remove saving/restoring method in Xend.

To: Yuji Shimada <shimada-yxb@xxxxxxxxxxxxxxx>, Keir Fraser <keir.fraser@xxxxxxxxxxxxx>, Kouya Shimura <kouya@xxxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [Xen-devel] [PATCH 2/3] remove saving/restoring method in Xend.
From: Masaki Kanno <kanno.masaki@xxxxxxxxxxxxxx>
Date: Wed, 22 Apr 2009 11:50:50 +0900
Cc:
Delivery-date: Tue, 21 Apr 2009 19:51:28 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20090422111600.32AA.27C06F64@xxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20090422105816.32A4.27C06F64@xxxxxxxxxxxxxxx> <20090422111600.32AA.27C06F64@xxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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