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/2] ioemu: Enable guest OS to program D0-D3hot s

To: Yuji Shimada <shimada-yxb@xxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 2/2] ioemu: Enable guest OS to program D0-D3hot states of an assigned device
From: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Date: Fri, 6 Feb 2009 16:20:55 +0000
Cc: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Fri, 06 Feb 2009 08:21:29 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20090205191651.2F5A.SHIMADA-YXB@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>
Newsgroups: chiark.mail.xen.devel
References: <20090204101439.GN25835%yamahata@xxxxxxxxxxxxx> <20090205162655.2F57.SHIMADA-YXB@xxxxxxxxxxxxxxx> <20090205191651.2F5A.SHIMADA-YXB@xxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Yuji Shimada writes ("[Xen-devel] [PATCH 2/2] ioemu: Enable guest OS to program 
D0-D3hot states of an assigned device"):
> This patch enables guest OS to program D0-D3hot states of assigned
> device.

I applied this patch but unfortunately it broke the automatic test
(and thus the staging propagation) because our automatic test system
has a very old version of libpci.  I have applied a band-aid - see
below - but would you please review it and consider whether a
different fix would be correct ?  For example, we might copy the
definitions of PCI_ERR_* from the libpci headers into pass-through.h
protected by an #ifdef, or something.

Also, this logic is very very strange:

> +    int aer_size = 0x2c;
> +
> +    for (i=0; i < aer_size; i+=4)
> +    {
> +        switch (i) {
> +        /* after reset, following register values should be restored.
> +         * So, save them.
> +         */
> +        case PCI_ERR_UNCOR_MASK:
> +        case PCI_ERR_UNCOR_SEVER:
> +        case PCI_ERR_COR_MASK:
> +        case PCI_ERR_CAP:
> +            *(uint32_t*)(d->config + (aer_base + i))
> +                 = pci_read_long(ptdev->pci_dev, (aer_base + i));
> +            break;
> +        default:
> +            break;
> +        }
> +    }

What purpose does the loop serve ?  Is there an ordering constraint ?
There are two copies of this construct, too - one in the save and one
in the restore.  That duplication is rather unfortunate.

Thanks,
Ian.

commit b80fc65b715099ee28465f6571eb5242588ef246
Author: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Date:   Fri Feb 6 16:16:06 2009 +0000

    hw/pass-through.c: workaround for old libpci
    
    Old versions of libpci (including the ones on the automatic tests
    which control Xen staging propagation) do not define
    PCI_LIB_VERSION or the PCI_ERR_{UNCOR_MASK,...} constants.
    
    This means that change 8c771eb6294afc5b3754a9e3de51568d4e5986c2 breaks
    the build.  In this changeset I apply what is intended to be a
    workaround for this problem but it may not be completely correct; this
    is therefore perhaps an interim fix.
    
    The potential problem is that the save/restore of some PCI passthrough
    error handling registers (across suspend/resume) may not work properly
    with old versions of libpci.  However non-passthrough and non-suspect
    use cases should now be fine.
    
    Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

diff --git a/hw/pass-through.c b/hw/pass-through.c
index 8dfae3c..29714c7 100644
--- a/hw/pass-through.c
+++ b/hw/pass-through.c
@@ -1503,7 +1503,7 @@ exit:
 
 static void pt_libpci_fixup(struct pci_dev *dev)
 {
-#if PCI_LIB_VERSION < 0x030100
+#if !defined(PCI_LIB_VERSION) || PCI_LIB_VERSION < 0x030100
     int i;
     FILE *fp;
     char path[PATH_MAX], buf[256];
@@ -1850,6 +1850,7 @@ static void pt_aer_reg_save(struct pt_dev *ptdev)
     /* Root Port and Root Complex Event Collector need size expansion */
     int aer_size = 0x2c;
 
+#ifdef PCI_ERR_UNCOR_MASK
     for (i=0; i < aer_size; i+=4)
     {
         switch (i) {
@@ -1867,6 +1868,7 @@ static void pt_aer_reg_save(struct pt_dev *ptdev)
             break;
         }
     }
+#endif
 }
 
 /* restore AER register */
@@ -1879,6 +1881,7 @@ static void pt_aer_reg_restore(struct pt_dev *ptdev)
     /* Root Port and Root Complex Event Collector need size expansion */
     int aer_size = 0x2c;
 
+#ifdef PCI_ERR_UNCOR_MASK
     for (i=0; i < aer_size; i+=4)
     {
         switch (i) {
@@ -1899,6 +1902,7 @@ static void pt_aer_reg_restore(struct pt_dev *ptdev)
             break;
         }
     }
+#endif
 }
 
 /* reset Interrupt and I/O resource  */

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>