Skip to contentSkip to conversation

pass-through PCI device hotplug support

PATCH In review Zhai, Edwin 3 participants 8 messages Jan 23, 2008 – Jan 25, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 pass-through PCI device hotplug support

All,

This patch enables HVM guest VT-d device hotplug via a simple ACPI hotplug device model. Pls. have a look.

On VT-d side, it's very useful as you can dynamically assign VT-d device to a guest as long as it support ACPI hotplug(Linux 2.6, 2000, 2003, XP... pass the test).

* Usage is very simple.
Three new commands are added:
"xm dpci-list domid" show the current assigned vtd device, like:
ID domain bus slot func
0 0x0 0x02 0x00 0x0

"xm dpci-remove" hot remove the specified vtd device by the ID, like:
xm dpci-remove EdwinHVMDomainVtd 0

"xm dpci-insert" hot add a new vtd device, like '03:00.0':
xm dpci-insert EdwinHVMDomainVtd 3 0 0

* Currently only 2 virtual pci slots are made as being capable of hotplug, so more than 2 vtd dev can't be hotplugged, but we can easily extend it in future.

* I reuse the pci PV driver configuration but untouch the code path since there may be HVM pci PV driver in future. I'm not sure if the python changes are okay, maybe you guys have some good idea.

Thanks,

Signature · 2 lines
best rgds,
edwin

Enable VTd device hotplug in HVM guest via a simple ACPI hotplug device model

tools/firmware/hvmloader/acpi/dsdt.asl +100 -0
diff -r 929210166e3f -r d1f20fa87c60 tools/firmware/hvmloader/acpi/dsdt.asl
@@ -75,6 +75,12 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, 
            Name (_UID, 0x00)
            Name (_ADR, 0x00)
            Name (_BBN, 0x00)
+           Method (_HPP, 0)
+           {
+               Store (0x99, \_GPE.DPT1)
+               Return (Package(0x4){0x10, 0x40, 0x00, 0x00})
+           }
+
  
            Method (_CRS, 0, NotSerialized)
            {
@@ -691,6 +697,100 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, 
                     })
                 } 
             }
+
+            Device (S1F0)
+            {
+                Name (_ADR, 0x00060000) /* Dev 6, Func 0 */
+                Name (_SUN, 0x00000001)
+
+                Method (_PS0, 0)
+                {
+                    Store (0x80, \_GPE.DPT2)
+                }
+
+                Method (_PS3, 0)
+                {
+                    Store (0x83, \_GPE.DPT2)
+                }
+
+                Method (_EJ0, 1)
+                {
+                    Store (0x88, \_GPE.DPT2)
+                    Store (0x1, \_GPE.PHP1) /* eject php slot 1*/
+                }
+
+                Method (_STA, 0)
+                {
+                    Store (0x89, \_GPE.DPT2)
+                    Return ( \_GPE.PHP1 )   /* IN status as the _STA */
+                }
+            }
+
+            Device (S2F0)
+            {
+                Name (_ADR, 0x00070000) /* Dev 7, Func 0 */
+                Name (_SUN, 0x00000002)
+
+                Method (_PS0, 0)
+                {
+                    Store (0x90, \_GPE.DPT2)
+                }
+
+                Method (_PS3, 0)
+                {
+                    Store (0x93, \_GPE.DPT2)
+                }
+
+                Method (_EJ0, 1)
+                {
+                    Store (0x98, \_GPE.DPT2)
+                    Store (0x1, \_GPE.PHP2) /* eject php slot 1*/
+                }
+
+                Method (_STA, 0)
+                {
+                    Store (0x99, \_GPE.DPT2)
+                    Return ( \_GPE.PHP2 )   /* IN status as the _STA */
+                }
+            }
+        }
+    }
+    Scope (\_GPE)
+    {
+        OperationRegion (PHP, SystemIO, 0x10c0, 0x03)
+        Field (PHP, ByteAcc, NoLock, Preserve)
+        {
+            PSTA,   8, /* hotplug controller status reg */
+            PHP1,   8, /* hotplug slot 1 control reg */
+            PHP2,   8  /* hotplug slot 2 control reg */
+        }
+        OperationRegion (DG1, SystemIO, 0xb044, 0x04)
+        Field (DG1, ByteAcc, NoLock, Preserve)
+        {
+            DPT1,   8,
+            DPT2,   8
+        }
+        Method (_L03, 0, NotSerialized)
+        {
+            /* detect slot and event(remove/add) */
+            Name (SLT, 0x0)
+            Name (EVT, 0x0)
+            Store (PSTA, Local1)
+            ShiftRight (Local1, 0x4, SLT)
+            And (Local1, 0xf, EVT)
+
+            /* debug */
+            Store (SLT, DPT1)
+            Store (EVT, DPT2)
+
+            If ( LEqual(SLT, 0x1) )
+            {
+                Notify (\_SB.PCI0.S1F0, EVT)
+            }
+            ElseIf ( LEqual(SLT, 0x2) )
+            {
+                Notify (\_SB.PCI0.S2F0, EVT)
+            }
         }
     }
 }
tools/firmware/hvmloader/acpi/dsdt.c +487 -428

944 lines not on this page · raw message

tools/firmware/hvmloader/acpi/static_tables.c +2 -0
diff -r 929210166e3f -r d1f20fa87c60 tools/firmware/hvmloader/acpi/static_tables.c
@@ -59,9 +59,11 @@ struct acpi_20_fadt Fadt = {
     .pm1a_evt_blk = ACPI_PM1A_EVT_BLK_ADDRESS,
     .pm1a_cnt_blk = ACPI_PM1A_CNT_BLK_ADDRESS,
     .pm_tmr_blk = ACPI_PM_TMR_BLK_ADDRESS,
+    .gpe0_blk = ACPI_GPE0_BLK_ADDRESS,
     .pm1_evt_len = ACPI_PM1A_EVT_BLK_BIT_WIDTH / 8,
     .pm1_cnt_len = ACPI_PM1A_CNT_BLK_BIT_WIDTH / 8,
     .pm_tmr_len = ACPI_PM_TMR_BLK_BIT_WIDTH / 8,
+    .gpe0_blk_len = ACPI_GPE0_BLK_LEN,
 
     .p_lvl2_lat = 0x0fff, /* >100,  means we do not support C2 state */
     .p_lvl3_lat = 0x0fff, /* >1000, means we do not support C3 state */
tools/ioemu/hw/pass-through.c +188 -1

239 lines not on this page · raw message

tools/ioemu/hw/pc.c +4 -2
diff -r 929210166e3f -r d1f20fa87c60 tools/ioemu/hw/pc.c
@@ -667,8 +667,10 @@ static void pc_init1(uint64_t ram_size, 
     }
 
 #ifdef CONFIG_PASSTHROUGH
-    /* Pass-through Initialization */
-    if ( pci_enabled && direct_pci )
+    /* Pass-through Initialization
+     * init libpci even direct_pci is null, as can hotplug a dev runtime
+     */
+    if ( pci_enabled )
     {
         rc = pt_init(pci_bus, direct_pci); 
         if ( rc < 0 )
tools/ioemu/hw/pci.c +8 -1
diff -r 929210166e3f -r d1f20fa87c60 tools/ioemu/hw/pci.c
@@ -107,7 +107,8 @@ PCIDevice *pci_register_device(PCIBus *b
     
     if (devfn < 0) {
         for(devfn = bus->devfn_min ; devfn < 256; devfn += 8) {
-            if (!bus->devices[devfn])
+            if ( !bus->devices[devfn] &&
+                 !( devfn >= PHP_DEVFN_START && devfn < PHP_DEVFN_END ) )
                 goto found;
         }
         return NULL;
@@ -130,6 +131,12 @@ PCIDevice *pci_register_device(PCIBus *b
     pci_dev->irq_index = pci_irq_index++;
     bus->devices[devfn] = pci_dev;
     return pci_dev;
+}
+
+void pci_hide_device(PCIDevice *pci_dev)
+{
+    PCIBus *bus = pci_dev->bus;
+    bus->devices[pci_dev->devfn] = NULL;
 }
 
 void pci_register_io_region(PCIDevice *pci_dev, int region_num, 
tools/ioemu/hw/piix4acpi.c +339 -0

368 lines not on this page · raw message

tools/ioemu/monitor.c +4 -0
diff -r 929210166e3f -r d1f20fa87c60 tools/ioemu/monitor.c
@@ -1288,6 +1288,10 @@ static term_cmd_t term_cmds[] = {
       "device", "add USB device (e.g. 'host:bus.addr' or 'host:vendor_id:product_id')" },
     { "usb_del", "s", do_usb_del,
       "device", "remove USB device 'bus.addr'" },
+    { "pci_add", "s", do_pci_add,
+      "device", "insert PCI pass-through device by BDF,e.g. (dom, bus, dev, func) by hex '0x0, 0x3, 0x0, 0x0'" },
+    { "pci_del", "s", do_pci_del,
+      "device", "remove PCI pass-through device by BDF,e.g. (dom, bus, dev, func) by hex '0x0, 0x3, 0x0, 0x0'" },
 #ifndef CONFIG_DM
     { "cpu", "i", do_cpu_set, 
       "index", "set the default CPU" },
tools/ioemu/vl.c +24 -2
diff -r 929210166e3f -r d1f20fa87c60 tools/ioemu/vl.c
@@ -285,7 +285,8 @@ int register_ioport_read(int start, int 
     for(i = start; i < start + length; i += size) {
         ioport_read_table[bsize][i] = func;
         if (ioport_opaque[i] != NULL && ioport_opaque[i] != opaque)
-            hw_error("register_ioport_write: invalid opaque");
+            hw_error("register_ioport_read: invalid opaque:start=0x%x, len=%d, size=%d, func=%p",
+                    start, length, size, func);
         ioport_opaque[i] = opaque;
     }
     return 0;
@@ -310,7 +311,8 @@ int register_ioport_write(int start, int
     for(i = start; i < start + length; i += size) {
         ioport_write_table[bsize][i] = func;
         if (ioport_opaque[i] != NULL && ioport_opaque[i] != opaque)
-            hw_error("register_ioport_write: invalid opaque");
+            hw_error("register_ioport_write: invalid opaque:start=0x%x, len=%d, size=%d, func=%p",
+                    start, length, size, func);
         ioport_opaque[i] = opaque;
     }
     return 0;
@@ -4333,6 +4335,25 @@ void usb_info(void)
                     0, dev->addr, speed_str, dev->devname);
     }
 }
+
+void do_pci_del(char *devname)
+{
+    int php_slot;
+    php_slot = bdf_to_slot(devname);
+
+    if ( php_slot != -1 )
+        acpi_php_del(php_slot);
+}
+
+void do_pci_add(char *devname)
+{
+    int php_slot;
+    php_slot = insert_to_free_slot(devname);
+
+    if ( php_slot != -1 )
+        acpi_php_add(php_slot);
+}
+
 
 /***********************************************************/
 /* pid file */
@@ -7896,5 +7917,6 @@ int main(int argc, char **argv)
 
     main_loop();
     quit_timers();
+    pt_uninit();
     return 0;
 }
tools/ioemu/vl.h +21 -0
diff -r 929210166e3f -r d1f20fa87c60 tools/ioemu/vl.h
@@ -819,6 +819,8 @@ PCIDevice *pci_register_device(PCIBus *b
                                PCIConfigReadFunc *config_read, 
                                PCIConfigWriteFunc *config_write);
 
+void pci_hide_device(PCIDevice *pci_dev);
+
 void pci_register_io_region(PCIDevice *pci_dev, int region_num, 
                             uint32_t size, int type, 
                             PCIMapIORegionFunc *map_func);
@@ -846,6 +848,19 @@ void pci_info(void);
 void pci_info(void);
 PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint32_t id,
                         pci_map_irq_fn map_irq, const char *name);
+
+/* PCI slot 6~7 support ACPI PCI hot plug */
+#define PHP_SLOT_START  (6)
+#define PHP_SLOT_END    (8)
+#define PHP_SLOT_LEN    (PHP_SLOT_END - PHP_SLOT_START)
+#define PHP_DEVFN_START (PHP_SLOT_START << 3)
+#define PHP_DEVFN_END   (PHP_SLOT_END << 3)
+
+int insert_to_free_slot(char*);
+int bdf_to_slot(char*);
+int power_on_php_slot(int);
+int power_off_php_slot(int);
+void pt_uninit(void);
 
 /* prep_pci.c */
 PCIBus *pci_prep_init(void);
@@ -1111,6 +1126,9 @@ void tpm_tis_init(SetIRQFunc *set_irq, v
 
 /* piix4acpi.c */
 extern void pci_piix4_acpi_init(PCIBus *bus, int devfn);
+void acpi_php_add(int);
+void acpi_php_del(int);
+
 
 /* pc.c */
 extern QEMUMachine pc_machine;
@@ -1311,6 +1329,9 @@ void do_usb_del(const char *devname);
 void do_usb_del(const char *devname);
 void usb_info(void);
 
+void do_pci_add(char *devname);
+void do_pci_del(char *devname);
+
 /* scsi-disk.c */
 enum scsi_reason {
     SCSI_REASON_DONE, /* Command complete.  */
tools/ioemu/xenstore.c +27 -1
diff -r 929210166e3f -r d1f20fa87c60 tools/ioemu/xenstore.c
@@ -372,7 +372,7 @@ void xenstore_process_logdirty_event(voi
 /* Accept state change commands from the control tools */
 static void xenstore_process_dm_command_event(void)
 {
-    char *path = NULL, *command = NULL;
+    char *path = NULL, *command = NULL, *par = NULL;
     unsigned int len;
     extern int suspend_requested;
 
@@ -391,6 +391,32 @@ static void xenstore_process_dm_command_
     } else if (!strncmp(command, "continue", len)) {
         fprintf(logfile, "dm-command: continue after state save\n");
         suspend_requested = 0;
+    } else if (!strncmp(command, "pci-rem", len)) {
+        fprintf(logfile, "dm-command: hot remove pass-through pci dev \n");
+
+        if (pasprintf(&path, 
+                      "/local/domain/0/device-model/%u/parameter", domid) == -1) {
+            fprintf(logfile, "out of memory reading dm command parameter\n");
+            goto out;
+        }
+        par = xs_read(xsh, XBT_NULL, path, &len);
+        if (!par)
+            goto out;
+
+        do_pci_del(par);
+    } else if (!strncmp(command, "pci-ins", len)) {
+        fprintf(logfile, "dm-command: hot insert pass-through pci dev \n");
+
+        if (pasprintf(&path, 
+                      "/local/domain/0/device-model/%u/parameter", domid) == -1) {
+            fprintf(logfile, "out of memory reading dm command parameter\n");
+            goto out;
+        }
+        par = xs_read(xsh, XBT_NULL, path, &len);
+        if (!par)
+            goto out;
+
+        do_pci_add(par);
     } else {
         fprintf(logfile, "dm-command: unknown command\"%*s\"\n", len, command);
     }
tools/libxc/xc_domain.c +44 -0

56 lines not on this page · raw message

tools/libxc/xenctrl.h +13 -0
diff -r 929210166e3f -r d1f20fa87c60 tools/libxc/xenctrl.h
@@ -918,6 +918,10 @@ int xc_test_assign_device(int xc_handle,
                           uint32_t domid,
                           uint32_t machine_bdf);
 
+int xc_deassign_device(int xc_handle,
+                     uint32_t domid,
+                     uint32_t machine_bdf);
+
 int xc_domain_memory_mapping(int xc_handle,
                              uint32_t domid,
                              unsigned long first_gfn,
@@ -941,6 +945,15 @@ int xc_domain_bind_pt_irq(int xc_handle,
                           uint8_t intx,
                           uint8_t isa_irq);
 
+int xc_domain_unbind_pt_irq(int xc_handle,
+                          uint32_t domid,
+                          uint8_t machine_irq,
+                          uint8_t irq_type,
+                          uint8_t bus,
+                          uint8_t device,
+                          uint8_t intx,
+                          uint8_t isa_irq);
+
 int xc_domain_bind_pt_pci_irq(int xc_handle,
                               uint32_t domid,
                               uint8_t machine_irq,
tools/python/xen/xend/XendDomainInfo.py +108 -0

141 lines not on this page · raw message

tools/python/xen/xend/image.py +23 -4
diff -r 929210166e3f -r d1f20fa87c60 tools/python/xen/xend/image.py
@@ -286,23 +286,42 @@ class ImageHandler:
         self.vm.storeDom("image/device-model-pid", self.pid)
         log.info("device model pid: %d", self.pid)
 
-    def saveDeviceModel(self):
+    def signalDeviceModel(self, cmd, ret, par = None):
         if self.device_model is None:
             return
-        # Signal the device model to pause itself and save its state
+        # Signal the device model to for action
+        if cmd is '' or ret is '':
+            raise VmError('need valid command and result when signal device model')
+
+        orig_state = xstransact.Read("/local/domain/0/device-model/%i/state"
+                                % self.vm.getDomid())
+
+        if par is not None:
+            xstransact.Store("/local/domain/0/device-model/%i"
+                             % self.vm.getDomid(), ('parameter', par))
+
         xstransact.Store("/local/domain/0/device-model/%i"
-                         % self.vm.getDomid(), ('command', 'save'))
+                         % self.vm.getDomid(), ('command', cmd))
         # Wait for confirmation.  Could do this with a watch but we'd
         # still end up spinning here waiting for the watch to fire. 
         state = ''
         count = 0
-        while state != 'paused':
+        while state != ret:
             state = xstransact.Read("/local/domain/0/device-model/%i/state"
                                     % self.vm.getDomid())
             time.sleep(0.1)
             count += 1
             if count > 100:
                 raise VmError('Timed out waiting for device model to save')
+
+        #resotre orig state
+        xstransact.Store("/local/domain/0/device-model/%i"
+                         % self.vm.getDomid(), ('state', orig_state))
+        log.info("signalDeviceModel:restore dm state to %s", orig_state)
+
+    def saveDeviceModel(self):
+        # Signal the device model to pause itself and save its state
+        self.signalDeviceModel('save', 'paused')
 
     def resumeDeviceModel(self):
         if self.device_model is None:
tools/python/xen/xend/server/DevController.py +8 -0
diff -r 929210166e3f -r d1f20fa87c60 tools/python/xen/xend/server/DevController.py
@@ -411,6 +411,14 @@ class DevController:
         return result
 
 
+    def removeBackend(self, devid, *args):
+        frontpath = self.frontendPath(devid)
+        backpath = xstransact.Read(frontpath, "backend")
+        if backpath:
+            return xstransact.Remove(backpath, *args)
+        else:
+            raise VmError("Device %s not connected" % devid)
+
     def readBackend(self, devid, *args):
         frontpath = self.frontendPath(devid)
         backpath = xstransact.Read(frontpath, "backend")
tools/python/xen/xend/server/pciif.py +20 -0
diff -r 929210166e3f -r d1f20fa87c60 tools/python/xen/xend/server/pciif.py
@@ -18,6 +18,7 @@
 
 
 import types
+import time
 
 from xen.xend import sxp
 from xen.xend.XendError import VmError
@@ -76,6 +77,25 @@ class PciController(DevController):
         back['uuid'] = config.get('uuid','')
         return (0, back, {})
 
+    def reconfigureDevice(self, _, config):
+        """@see DevController.reconfigureDevice"""
+        #currently only support config changes by hot insert/remove pass-through dev
+        #delete all the devices in xenstore
+        (devid, new_back, new_front) = self.getDeviceDetails(config)
+        num_devs = self.readBackend(devid, 'num_devs')
+        for i in range(int(num_devs)):
+            self.removeBackend(devid, 'dev-%d' % i)
+        self.removeBackend(devid, 'num_devs')
+
+        #create new devices config
+        num_devs = new_back['num_devs']
+        for i in range(int(num_devs)):
+            dev_no = 'dev-%d' % i
+            self.writeBackend(devid, dev_no, new_back[dev_no])
+        self.writeBackend(devid, 'num_devs', num_devs)
+
+        return new_back.get('uuid')
+
     def getDeviceConfiguration(self, devid, transaction = None):
         result = DevController.getDeviceConfiguration(self, devid, transaction)
         num_devs = self.readBackend(devid, 'num_devs')
tools/python/xen/xm/main.py +76 -0

112 lines not on this page · raw message

xen/arch/x86/domctl.c +45 -0

57 lines not on this page · raw message

xen/arch/x86/hvm/irq.c +1 -1
diff -r 929210166e3f -r d1f20fa87c60 xen/arch/x86/hvm/irq.c
@@ -211,7 +211,7 @@ void hvm_set_pci_link_route(struct domai
             clear_bit(old_isa_irq, &hvm_irq->dpci->isairq_map);
 
         for ( i = 0; i < NR_LINK; i++ )
-            if ( test_bit(i, &hvm_irq->dpci->link_map) &&
+            if ( hvm_irq->dpci->link_cnt[i] > 0 &&
                  hvm_irq->pci_link.route[i] )
                 set_bit(hvm_irq->pci_link.route[i],
                         &hvm_irq->dpci->isairq_map);
xen/arch/x86/hvm/vmx/vtd/io.c +60 -1

73 lines not on this page · raw message

xen/include/asm-x86/hvm/irq.h +1 -1
diff -r 929210166e3f -r d1f20fa87c60 xen/include/asm-x86/hvm/irq.h
@@ -64,7 +64,7 @@ struct hvm_irq_dpci {
     /* Record of mapped ISA IRQs */
     DECLARE_BITMAP(isairq_map, NR_ISAIRQS);
     /* Record of mapped Links */
-    DECLARE_BITMAP(link_map, NR_LINK);
+    uint8_t link_cnt[NR_LINK];
     struct timer hvm_timer[NR_IRQS];
 };
 
xen/include/asm-x86/iommu.h +5 -0
diff -r 929210166e3f -r d1f20fa87c60 xen/include/asm-x86/iommu.h
@@ -68,6 +68,9 @@ void iommu_domain_destroy(struct domain 
 void iommu_domain_destroy(struct domain *d);
 int device_assigned(u8 bus, u8 devfn);
 int assign_device(struct domain *d, u8 bus, u8 devfn);
+void reassign_device_ownership( struct domain *source,
+    struct domain *target,
+    u8 bus, u8 devfn);
 int iommu_map_page(struct domain *d, dma_addr_t gfn, dma_addr_t mfn);
 int iommu_unmap_page(struct domain *d, dma_addr_t gfn);
 void iommu_flush(struct domain *d, dma_addr_t gfn, u64 *p2m_entry);
@@ -77,6 +80,8 @@ int dpci_ioport_intercept(ioreq_t *p);
 int dpci_ioport_intercept(ioreq_t *p);
 int pt_irq_create_bind_vtd(struct domain *d,
                            xen_domctl_bind_pt_irq_t *pt_irq_bind);
+int pt_irq_destroy_bind_vtd(struct domain *d,
+                           xen_domctl_bind_pt_irq_t *pt_irq_bind);
 
 #define PT_IRQ_TIME_OUT MILLISECS(8)
 #define VTDPREFIX "[VT-D]"
xen/include/public/domctl.h +2 -0
diff -r 929210166e3f -r d1f20fa87c60 xen/include/public/domctl.h
@@ -436,6 +436,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_sendt
 /* Assign PCI device to HVM guest. Sets up IOMMU structures. */
 #define XEN_DOMCTL_assign_device      37
 #define XEN_DOMCTL_test_assign_device 45
+#define XEN_DOMCTL_deassign_device 46
 struct xen_domctl_assign_device {
     uint32_t  machine_bdf;   /* machine PCI ID of assigned device */
 };
@@ -445,6 +446,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_assig
 
 /* Pass-through interrupts: bind real irq -> hvm devfn. */
 #define XEN_DOMCTL_bind_pt_irq       38
+#define XEN_DOMCTL_unbind_pt_irq     47
 typedef enum pt_irq_type_e {
     PT_IRQ_TYPE_PCI,
     PT_IRQ_TYPE_ISA
xen/include/public/hvm/ioreq.h +5 -0
diff -r 929210166e3f -r d1f20fa87c60 xen/include/public/hvm/ioreq.h
@@ -118,6 +118,11 @@ struct buffered_piopage {
 #define ACPI_PM1A_EVT_BLK_ADDRESS           0x0000000000001f40
 #define ACPI_PM1A_CNT_BLK_ADDRESS           (ACPI_PM1A_EVT_BLK_ADDRESS + 0x04)
 #define ACPI_PM_TMR_BLK_ADDRESS             (ACPI_PM1A_EVT_BLK_ADDRESS + 0x08)
+
+#define ACPI_GPE0_BLK_ADDRESS               (ACPI_PM_TMR_BLK_ADDRESS + 0x20)
+
+#define ACPI_GPE0_BLK_LEN                   0x08
+
 #endif /* defined(__i386__) || defined(__x86_64__) */
 
 #endif /* _IOREQ_H_ */

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw

Please add more comments to your DSDT changes so it's clear what the changes are actually for. A paragraph or two distributed around your changes would be good, including references to the ACPI specification document where that is useful.

Apart from that I expect the patch is fine. You can send the DSDT comments as an incremental patch on top of this one.

-- Keir

Quoted 32 lines · Edwin

All,

This patch enables HVM guest VT-d device hotplug via a simple ACPI hotplug device model. Pls. have a look.

On VT-d side, it's very useful as you can dynamically assign VT-d device to a guest as long as it support ACPI hotplug(Linux 2.6, 2000, 2003, XP... pass the test).

* Usage is very simple.
Three new commands are added:
"xm dpci-list domid" show the current assigned vtd device, like:
ID domain bus slot func
0 0x0 0x02 0x00 0x0

"xm dpci-remove" hot remove the specified vtd device by the ID, like:
xm dpci-remove EdwinHVMDomainVtd 0

"xm dpci-insert" hot add a new vtd device, like '03:00.0':
xm dpci-insert EdwinHVMDomainVtd 3 0 0

* Currently only 2 virtual pci slots are made as being capable of hotplug, so more than 2 vtd dev can't be hotplugged, but we can easily extend it in future.

* I reuse the pci PV driver configuration but untouch the code path since there
may be HVM pci PV driver in future. I'm not sure if the python changes are okay,
maybe you guys have some good idea.

Thanks,

#2 lore raw
In reply to Zhai, Edwin
Quoted 15 lines · Edwin

All,

This patch enables HVM guest VT-d device hotplug via a simple ACPI hotplug device model. Pls. have a look.

On VT-d side, it's very useful as you can dynamically assign VT-d device to a guest as long as it support ACPI hotplug(Linux 2.6, 2000, 2003, XP... pass the test).

* Usage is very simple.
Three new commands are added:
"xm dpci-list domid" show the current assigned vtd device, like:
ID domain bus slot func
0 0x0 0x02 0x00 0x0

Quoted 5 lines

"xm dpci-remove" hot remove the specified vtd device by the ID, like:
xm dpci-remove EdwinHVMDomainVtd 0

"xm dpci-insert" hot add a new vtd device, like '03:00.0':
xm dpci-insert EdwinHVMDomainVtd 3 0 0

IMHO we shouldn't have a 'd' on the front of the command names. VT-d is a vendor specific implementation whose nomenculture doesn't need to be exposed to users. In addition the existing block & network hotplug commands use 'attach' and 'detach' for their command names. So for sake of consistency I'd recommend command names of:

pci-list
pci-attach
pci-detach

I think it is useful to use the same unique naming & data for both attach and detach operations. So if we use a (bus,slot,func) triple for attachment, I think we should use the same (bus,slot,func) triple for detachment too, rather than having to make apps / users lookup the dynamically-allocated 'ID' value for the device.

Regards,
Dan.

Signature · 4 lines
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 
#3 lore raw
Quoted 10 lines · Daniel P. Berrange

IMHO we shouldn't have a 'd' on the front of the command names. VT-d is a vendor specific implementation whose nomenculture doesn't need to be exposed to users. In addition the existing block & network hotplug commands use 'attach' and 'detach' for their command names. So for sake of consistency I'd recommend command names of:

pci-list
pci-attach
pci-detach

Quoted 5 lines

I think it is useful to use the same unique naming & data for both attach and detach operations. So if we use a (bus,slot,func) triple for attachment, I think we should use the same (bus,slot,func) triple for detachment too, rather than having to make apps / users lookup the dynamically-allocated 'ID' value for the device.

Seconded.

-- Keir

#4 lore raw
In reply to Daniel P. Berrange
Quoted 9 lines · Daniel P. Berrange
Quoted 6 lines

"xm dpci-remove" hot remove the specified vtd device by the ID, like:
xm dpci-remove EdwinHVMDomainVtd 0

"xm dpci-insert" hot add a new vtd device, like '03:00.0':
xm dpci-insert EdwinHVMDomainVtd 3 0 0

IMHO we shouldn't have a 'd' on the front of the command names. VT-d is a vendor specific implementation whose nomenculture doesn't need to be exposed

'd' means directly assigned device, which is generic including VT-d and other pass-through device..

Anyway, it's not so important.

Quoted 7 lines

to users. In addition the existing block & network hotplug commands use 'attach' and 'detach' for their command names. So for sake of consistency I'd recommend command names of:

pci-list
pci-attach
pci-detach

I had the same idea at the beginning, but change mind due to some concerns:

xxx-attach/detach are used for _PV_ driver, but dpci is not the case. If pci PV driver support hotplug in future, we get a complicated code path to handle both PV and dpci's hotplug.

So do we have plan to support PCI PV driver hotplug? If no, we can use pci-attach/detach.

Quoted 6 lines

I think it is useful to use the same unique naming & data for both attach and detach operations. So if we use a (bus,slot,func) triple for attachment, I think we should use the same (bus,slot,func) triple for detachment too, rather than having to make apps / users lookup the dynamically-allocated 'ID' value for the device.

yes, dynamical ID is not easily for use.
How about using some fixed 'ID' for hot remove, just like vbd/vnif(they also use different naming&data for attach/detach)?

Quoted 14 lines

Regards,
Dan.

Signature · 9 lines
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Signature · 2 lines
best rgds,
edwin
#5 lore raw
Quoted 24 lines · Edwin
Quoted 9 lines · Daniel P. Berrange
Quoted 6 lines

"xm dpci-remove" hot remove the specified vtd device by the ID, like:
xm dpci-remove EdwinHVMDomainVtd 0

"xm dpci-insert" hot add a new vtd device, like '03:00.0':
xm dpci-insert EdwinHVMDomainVtd 3 0 0

IMHO we shouldn't have a 'd' on the front of the command names. VT-d is a vendor specific implementation whose nomenculture doesn't need to be exposed

'd' means directly assigned device, which is generic including VT-d and other pass-through device..

Anyway, it's not so important.

Quoted 7 lines

to users. In addition the existing block & network hotplug commands use 'attach' and 'detach' for their command names. So for sake of consistency I'd recommend command names of:

pci-list
pci-attach
pci-detach

Quoted 5 lines

I had the same idea at the beginning, but change mind due to some concerns:

xxx-attach/detach are used for _PV_ driver, but dpci is not the case. If pci PV driver support hotplug in future, we get a complicated code path to handle both PV and dpci's hotplug.

I don't buy that argument. You can still just have completely separate codepaths inside XenD if you really need to - just switch on different impls in the main RPC dispatcher...

def pci_attach()

    if dom.is_hvm():
       pci_attach_hvm()
    else
       pci_attach_pv()

Every part of Xen where we expose the user to a difference between HVM vs PV is a point of pain. We should ensure that use of HVM & PV is as near as possible, identical from a user's view.

Quoted 4 lines

So do we have plan to support PCI PV driver hotplug? If no, we can use pci-attach/detach.

From the user's point of view they are attaching & detaching PCI devices,

and they should not have to use separate commands to do the same operation for PV vs HVM.

Regards,
Dan.

Signature · 4 lines
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 
#6 lore raw
Quoted 12 lines · Daniel P. Berrange
Quoted 6 lines

I had the same idea at the beginning, but change mind due to some concerns:

xxx-attach/detach are used for _PV_ driver, but dpci is not the case. If pci PV driver support hotplug in future, we get a complicated code path to handle both PV and dpci's hotplug.

I don't buy that argument. You can still just have completely separate codepaths inside XenD if you really need to - just switch on different impls in the main RPC dispatcher...

def pci_attach()

Quoted 8 lines
    if dom.is_hvm():
       pci_attach_hvm()
    else
       pci_attach_pv()

Every part of Xen where we expose the user to a difference between HVM vs PV is a point of pain. We should ensure that use of HVM & PV is as near as possible, identical from a user's view.

If we support PV driver in HVM guest with hotplug, the code path become more complicated:(

Anyway it's a trade off. I can change to pci_attach/detach for a simple user interface with complicated implementation.

Quoted 16 lines
Quoted 5 lines

So do we have plan to support PCI PV driver hotplug? If no, we can use pci-attach/detach.

From the user's point of view they are attaching & detaching PCI devices,

and they should not have to use separate commands to do the same operation for PV vs HVM.

Regards,
Dan.

Signature · 4 lines
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 
Signature · 2 lines
best rgds,
edwin
#7 lore raw
In reply to Zhai, Edwin
Quoted 8 lines · Edwin
Quoted 7 lines

to users. In addition the existing block & network hotplug commands use 'attach' and 'detach' for their command names. So for sake of consistency I'd recommend command names of:

pci-list
pci-attach
pci-detach

Quoted 8 lines

I had the same idea at the beginning, but change mind due to some concerns:

xxx-attach/detach are used for _PV_ driver, but dpci is not the case. If pci PV driver support hotplug in future, we get a complicated code path to handle both PV and dpci's hotplug.

So do we have plan to support PCI PV driver hotplug? If no, we can use pci-attach/detach.

That's a hideously bad argument.

-- Keir