thr3ads.net - Xen devel - [Xen-devel] [PATCH 1/3] ioemu: check mmap return value when mapping MSI-x table [Jan 2009]

If this information is useful, please help other people find it:
Share via:

Yu Zhao

2009-Jan-24 04:47 UTC

[Xen-devel] [PATCH 1/3] ioemu: check mmap return value when mapping MSI-x table

Signed-off-by: Yu Zhao <yu.zhao@intel.com>
---
 hw/pt-msi.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/pt-msi.c b/hw/pt-msi.c
index c515c02..4f25186 100644
--- a/hw/pt-msi.c
+++ b/hw/pt-msi.c
@@ -322,6 +322,12 @@ int pt_msix_init(struct pt_dev *dev, int pos)
     dev->msix->phys_iomem_base = mmap(0, total_entries * 16,
                           PROT_WRITE | PROT_READ, MAP_SHARED | MAP_LOCKED,
                           dev->msix->fd, dev->msix->table_base +
table_off);
+    if ( dev->msix->phys_iomem_base == MAP_FAILED )
+    {
+        PT_LOG("Can''t map physical MSI-X table: %s\n",
strerror(errno));
+        return -1;
+    }
+
     PT_LOG("mapping physical MSI-X table to %lx\n",
            (unsigned long)dev->msix->phys_iomem_base);
     return 0;
-- 
1.5.6.4


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

Yu Zhao

2009-Jan-24 04:47 UTC

head link

[Xen-devel] [PATCH 2/3] ioemu: add libpci fixup to handle recent change in PCIUtils

The following commit of PCIUtils changes what ''base_addr''
contains.
Add a fixup to make sure the pass-through works when building against
different versions of PCIUtils.

  commit 6d143c3283855c474445a3cf27c65280ed7ab1b7
  Author: Yu Zhao <yu.zhao@intel.com>
  Date:   Fri Dec 26 09:59:28 2008 +0800

    Fix base address flags when using sysfs method

    The base address flags are ignored when using sysfs method, while
    the proc and generic methods combined the flags into
''base_addr''.
    This inconsistency may fail some applications using libpci.

Signed-off-by: Yu Zhao <yu.zhao@intel.com>
---
 hw/pass-through.c |  117 ++++++++++++++++++++++++++++++++++++++++++----------
 hw/pass-through.h |    8 ++++
 hw/pt-msi.c       |    2 +-
 3 files changed, 103 insertions(+), 24 deletions(-)

diff --git a/hw/pass-through.c b/hw/pass-through.c
index 71dff1f..207bdf0 100644
--- a/hw/pass-through.c
+++ b/hw/pass-through.c
@@ -48,6 +48,10 @@ struct dpci_infos {
 /* prototype */
 static uint32_t pt_common_reg_init(struct pt_dev *ptdev,
     struct pt_reg_info_tbl *reg, uint32_t real_offset);
+static uint32_t pt_vendor_reg_init(struct pt_dev *ptdev,
+    struct pt_reg_info_tbl *reg, uint32_t real_offset);
+static uint32_t pt_device_reg_init(struct pt_dev *ptdev,
+    struct pt_reg_info_tbl *reg, uint32_t real_offset);
 static uint32_t pt_ptr_reg_init(struct pt_dev *ptdev,
     struct pt_reg_info_tbl *reg, uint32_t real_offset);
 static uint32_t pt_status_reg_init(struct pt_dev *ptdev,
@@ -152,6 +156,28 @@ static int pt_msixctrl_reg_write(struct pt_dev *ptdev,
  
 /* Header Type0 reg static infomation table */
 static struct pt_reg_info_tbl pt_emu_reg_header0_tbl[] = {
+    /* Vendor ID reg */
+    {
+        .offset     = PCI_VENDOR_ID,
+        .size       = 2,
+        .init_val   = 0x0000,
+        .ro_mask    = 0xFFFF,
+        .emu_mask   = 0xFFFF,
+        .init       = pt_vendor_reg_init,
+        .u.w.read   = pt_word_reg_read,
+        .u.w.write  = pt_word_reg_write,
+    },
+    /* Device ID reg */
+    {
+        .offset     = PCI_DEVICE_ID,
+        .size       = 2,
+        .init_val   = 0x0000,
+        .ro_mask    = 0xFFFF,
+        .emu_mask   = 0xFFFF,
+        .init       = pt_device_reg_init,
+        .u.w.read   = pt_word_reg_read,
+        .u.w.write  = pt_word_reg_write,
+    },
     /* Command reg */
     {
         .offset     = PCI_COMMAND,
@@ -1421,28 +1447,65 @@ exit:
     return val;
 }
 
+static void pt_libpci_fixup(struct pci_dev *dev)
+{
+#if PCI_LIB_VERSION < 0x030100
+    int i;
+    FILE *fp;
+    char path[PATH_MAX], buf[256];
+    unsigned long long start, end, flags;
+
+    sprintf(path, "/sys/bus/pci/devices/%04x:%02x:%02x.%x/resource",
+            dev->domain, dev->bus, dev->dev, dev->func);
+    fp = fopen(path, "r");
+    if ( !fp )
+    {
+        PT_LOG("Can''t open %s: %s\n", path,
strerror(errno));
+        return;
+    }
+
+    for ( i = 0; i < PCI_NUM_REGIONS; i++ )
+    {
+        if ( fscanf(fp, "%llx %llx %llx", &start, &end,
&flags) != 3 )
+        {
+            PT_LOG("Syntax error in %s\n", path);
+            break;
+        }
+
+        flags &= 0xf;
+
+        if ( i < PCI_ROM_SLOT )
+            dev->base_addr[i] |= flags;
+        else
+            dev->rom_base_addr |= flags;
+    }
+
+    fclose(fp);
+#endif /* PCI_LIB_VERSION < 0x030100 */
+}
+
 static int pt_register_regions(struct pt_dev *assigned_device)
 {
     int i = 0;
-    uint32_t bar_data = 0;
     struct pci_dev *pci_dev = assigned_device->pci_dev;
     PCIDevice *d = &assigned_device->dev;
 
     /* Register PIO/MMIO BARs */
     for ( i = 0; i < PCI_BAR_ENTRIES; i++ )
     {
-        if ( pci_dev->base_addr[i] )
+        if ( pt_pci_base_addr(pci_dev->base_addr[i]) )
         {
-            assigned_device->bases[i].e_physbase = pci_dev->base_addr[i];
-            assigned_device->bases[i].access.u = pci_dev->base_addr[i];
+            assigned_device->bases[i].e_physbase +                   
pt_pci_base_addr(pci_dev->base_addr[i]);
+            assigned_device->bases[i].access.u +                   
pt_pci_base_addr(pci_dev->base_addr[i]);
 
             /* Register current region */
-            bar_data = *((uint32_t*)(d->config + PCI_BASE_ADDRESS_0) + i);
-            if ( bar_data & PCI_ADDRESS_SPACE_IO )
+            if ( pci_dev->base_addr[i] & PCI_ADDRESS_SPACE_IO )
                 pci_register_io_region((PCIDevice *)assigned_device, i,
                     (uint32_t)pci_dev->size[i], PCI_ADDRESS_SPACE_IO,
                     pt_ioport_map);
-            else if ( bar_data & PCI_ADDRESS_SPACE_MEM_PREFETCH )
+            else if ( pci_dev->base_addr[i] &
PCI_ADDRESS_SPACE_MEM_PREFETCH )
                 pci_register_io_region((PCIDevice *)assigned_device, i,
                     (uint32_t)pci_dev->size[i],
PCI_ADDRESS_SPACE_MEM_PREFETCH,
                     pt_iomem_map);
@@ -1458,16 +1521,13 @@ static int pt_register_regions(struct pt_dev
*assigned_device)
     }
 
     /* Register expansion ROM address */
-    if ( pci_dev->rom_base_addr && pci_dev->rom_size )
+    if ( pt_pci_base_addr(pci_dev->rom_base_addr) &&
pci_dev->rom_size )
     {
 
         /* Re-set BAR reported by OS, otherwise ROM can''t be read. */
-        bar_data = pci_read_long(pci_dev, PCI_ROM_ADDRESS);
-        if ( (bar_data & PCI_ROM_ADDRESS_MASK) == 0 )
-        {
-            bar_data |= (pci_dev->rom_base_addr & PCI_ROM_ADDRESS_MASK);
-            pci_write_long(pci_dev, PCI_ROM_ADDRESS, bar_data);
-        }
+        if ( (pci_dev->rom_base_addr & PCI_ROM_ADDRESS_MASK) == 0 )
+            pci_write_long(pci_dev, PCI_ROM_ADDRESS,
+                           (pci_dev->rom_base_addr | PCI_ROM_ADDRESS_MASK));
 
         assigned_device->bases[PCI_ROM_SLOT].e_physbase             
pci_dev->rom_base_addr;
@@ -1569,21 +1629,14 @@ static int pt_bar_reg_parse(
     PCIDevice *d = &ptdev->dev;
     struct pt_region *region = NULL;
     PCIIORegion *r;
-    uint32_t bar_64 = (reg->offset - 4);
     int bar_flag = PT_BAR_FLAG_UNUSED;
     int index = 0;
     int i;
 
-    /* set again the BAR config because it has been overwritten
-     * by pci_register_io_region()
-     */
-    for (i=reg->offset; i<(reg->offset + 4); i++)
-        d->config[i] = pci_read_byte(ptdev->pci_dev, i);
-
     /* check 64bit BAR */
     index = pt_bar_offset_to_index(reg->offset);
     if ((index > 0) && (index < PCI_ROM_SLOT) &&
-        ((d->config[bar_64] & (PCI_BASE_ADDRESS_SPACE |
+        ((ptdev->pci_dev->base_addr[index-1] &
(PCI_BASE_ADDRESS_SPACE |
                                PCI_BASE_ADDRESS_MEM_TYPE_MASK)) =         
(PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64)))
     {
@@ -1608,7 +1661,7 @@ static int pt_bar_reg_parse(
     }
 
     /* check BAR I/O indicator */
-    if (d->config[reg->offset] & PCI_BASE_ADDRESS_SPACE_IO)
+    if ( ptdev->pci_dev->base_addr[index] & PCI_BASE_ADDRESS_SPACE_IO
)
         bar_flag = PT_BAR_FLAG_IO;
     else
         bar_flag = PT_BAR_FLAG_MEM;
@@ -1826,6 +1879,20 @@ static uint32_t pt_common_reg_init(struct pt_dev *ptdev,
     return reg->init_val;
 }
 
+/* initialize Vendor ID register value */
+static uint32_t pt_vendor_reg_init(struct pt_dev *ptdev,
+        struct pt_reg_info_tbl *reg, uint32_t real_offset)
+{
+    return ptdev->pci_dev->vendor_id;
+}
+
+/* initialize Device ID register value */
+static uint32_t pt_device_reg_init(struct pt_dev *ptdev,
+        struct pt_reg_info_tbl *reg, uint32_t real_offset)
+{
+    return ptdev->pci_dev->device_id;
+}
+
 /* initialize Capabilities Pointer or Next Pointer register */
 static uint32_t pt_ptr_reg_init(struct pt_dev *ptdev,
         struct pt_reg_info_tbl *reg, uint32_t real_offset)
@@ -2310,6 +2377,9 @@ static int pt_bar_reg_read(struct pt_dev *ptdev,
         exit(1);
     }
 
+    /* use fixed-up value from kernel sysfs */
+    *value = ptdev->pci_dev->base_addr[index];
+
     /* set emulate mask depend on BAR flag */
     switch (ptdev->bases[index].bar_flag)
     {
@@ -3025,6 +3095,7 @@ struct pt_dev * register_real_device(PCIBus *e_bus,
         return NULL;
     }
     pci_fill_info(pci_dev, PCI_FILL_IRQ | PCI_FILL_BASES | PCI_FILL_ROM_BASE |
PCI_FILL_SIZES);
+    pt_libpci_fixup(pci_dev);
 
     if ( e_devfn == PT_VIRT_DEVFN_AUTO ) {
         /*indicate a static assignment(not hotplug), so find a free PCI hot
plug slot */
diff --git a/hw/pass-through.h b/hw/pass-through.h
index a7d2727..7a623be 100644
--- a/hw/pass-through.h
+++ b/hw/pass-through.h
@@ -297,5 +297,13 @@ struct pt_reg_info_tbl {
     } u;
 };
 
+static inline pciaddr_t pt_pci_base_addr(pciaddr_t base)
+{
+    if ( base & PCI_ADDRESS_SPACE_IO )
+        return base & PCI_ADDR_IO_MASK;
+
+    return base & PCI_ADDR_MEM_MASK;
+}
+
 #endif /* __PASSTHROUGH_H__ */
 
diff --git a/hw/pt-msi.c b/hw/pt-msi.c
index 4f25186..9898763 100644
--- a/hw/pt-msi.c
+++ b/hw/pt-msi.c
@@ -314,7 +314,7 @@ int pt_msix_init(struct pt_dev *dev, int pos)
     table_off = pci_read_long(pd, pos + PCI_MSIX_TABLE);
     bar_index = dev->msix->bar_index = table_off & PCI_MSIX_BIR;
     table_off = dev->msix->table_off = table_off & ~PCI_MSIX_BIR;
-    dev->msix->table_base = dev->pci_dev->base_addr[bar_index];
+    dev->msix->table_base =
pt_pci_base_addr(dev->pci_dev->base_addr[bar_index]);
     PT_LOG("get MSI-X table bar base %llx\n",
            (unsigned long long)dev->msix->table_base);
 
-- 
1.5.6.4


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

Yu Zhao

2009-Jan-24 04:47 UTC

head link

[Xen-devel] [PATCH 3/3] ioemu: use the sysfs interface to enable the PCI ROM

Writing the base address directly to the ROM BAR may cause problem
since the kernel is not aware of the operation.

Signed-off-by: Yu Zhao <yu.zhao@intel.com>
---
 hw/pass-through.c |   25 +++++++++++++++++++++++--
 1 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/hw/pass-through.c b/hw/pass-through.c
index 207bdf0..e76a3c3 100644
--- a/hw/pass-through.c
+++ b/hw/pass-through.c
@@ -1447,6 +1447,27 @@ exit:
     return val;
 }
 
+static int pt_pci_enable_rom(struct pci_dev *dev)
+{
+    FILE *fp;
+    char path[PATH_MAX];
+
+    sprintf(path, "/sys/bus/pci/devices/%04x:%02x:%02x.%x/rom",
+            dev->domain, dev->bus, dev->dev, dev->func);
+
+    fp = fopen(path, "w");
+    if ( !fp )
+    {
+        PT_LOG("Can''t open %s: %s\n", path,
strerror(errno));
+        return -1;
+    }
+
+    fprintf(fp, "1");
+    fclose(fp);
+
+    return 0;
+}
+
 static void pt_libpci_fixup(struct pci_dev *dev)
 {
 #if PCI_LIB_VERSION < 0x030100
@@ -1526,8 +1547,8 @@ static int pt_register_regions(struct pt_dev
*assigned_device)
 
         /* Re-set BAR reported by OS, otherwise ROM can''t be read. */
         if ( (pci_dev->rom_base_addr & PCI_ROM_ADDRESS_MASK) == 0 )
-            pci_write_long(pci_dev, PCI_ROM_ADDRESS,
-                           (pci_dev->rom_base_addr | PCI_ROM_ADDRESS_MASK));
+            if ( pt_pci_enable_rom(pci_dev) )
+                return -1;
 
         assigned_device->bases[PCI_ROM_SLOT].e_physbase             
pci_dev->rom_base_addr;
-- 
1.5.6.4


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

Kouya Shimura

2009-Feb-03 01:05 UTC

head link

Re: [Xen-devel] [PATCH 3/3] ioemu: use the sysfs interface to enable the PCI ROM

Hi Yu,

This patch disabled the hvm from reading the PCI option ROM again.
Should the kernel(dom0) know the BAR of hidden PCI?
I don''t understand what the problem is. Writing to the real BAR
of option ROM looks harmless.

The cause of original problem, which I fixed before about invisibility
of option ROM, is not the enable flag(bit0) but the address(=0) of ROM
BAR. Thus your patch looks pointless.

If you intend to map the ROM in dom0, writing "1" to sysfs is not
enough. At least, trying to read the ROM is necessary to map the ROM.
(although nothing can be read in dom0.)

Thanks,
Kouya

Yu Zhao writes:> Writing the base address directly to the ROM BAR may cause problem
> since the kernel is not aware of the operation.
> 
> Signed-off-by: Yu Zhao <yu.zhao@intel.com>
> ---
>  hw/pass-through.c |   25 +++++++++++++++++++++++--
>  1 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pass-through.c b/hw/pass-through.c
> index 207bdf0..e76a3c3 100644
> --- a/hw/pass-through.c
> +++ b/hw/pass-through.c
> @@ -1447,6 +1447,27 @@ exit:
>      return val;
>  }
>  
> +static int pt_pci_enable_rom(struct pci_dev *dev)
> +{
> +    FILE *fp;
> +    char path[PATH_MAX];
> +
> +    sprintf(path, "/sys/bus/pci/devices/%04x:%02x:%02x.%x/rom",
> +            dev->domain, dev->bus, dev->dev, dev->func);
> +
> +    fp = fopen(path, "w");
> +    if ( !fp )
> +    {
> +        PT_LOG("Can''t open %s: %s\n", path,
strerror(errno));
> +        return -1;
> +    }
> +
> +    fprintf(fp, "1");
> +    fclose(fp);
> +
> +    return 0;
> +}
> +
>  static void pt_libpci_fixup(struct pci_dev *dev)
>  {
>  #if PCI_LIB_VERSION < 0x030100
> @@ -1526,8 +1547,8 @@ static int pt_register_regions(struct pt_dev
*assigned_device)
>  
>          /* Re-set BAR reported by OS, otherwise ROM can''t be
read. */
>          if ( (pci_dev->rom_base_addr & PCI_ROM_ADDRESS_MASK) == 0 )
> -            pci_write_long(pci_dev, PCI_ROM_ADDRESS,
> -                           (pci_dev->rom_base_addr |
PCI_ROM_ADDRESS_MASK));
> +            if ( pt_pci_enable_rom(pci_dev) )
> +                return -1;
>  
>          assigned_device->bases[PCI_ROM_SLOT].e_physbase >           
pci_dev->rom_base_addr;
> -- 
> 1.5.6.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Zhao, Yu

2009-Feb-03 02:17 UTC

head link

Re: [Xen-devel] [PATCH 3/3] ioemu: use the sysfs interface to enable the PCI ROM

Kouya,

I remember the kernel would write base address combined with enable bit 
into ROM BAR when writing ''1'' to the sysfs, but looks like it
doesn''t do
so (the kernel only does this when user maps rom entry through sysfs).

I''ll send a patch to fix this.

Thanks,
Yu

Kouya Shimura wrote:> Hi Yu,
> 
> This patch disabled the hvm from reading the PCI option ROM again.
> Should the kernel(dom0) know the BAR of hidden PCI?
> I don''t understand what the problem is. Writing to the real BAR
> of option ROM looks harmless.
> 
> The cause of original problem, which I fixed before about invisibility
> of option ROM, is not the enable flag(bit0) but the address(=0) of ROM
> BAR. Thus your patch looks pointless.
> 
> If you intend to map the ROM in dom0, writing "1" to sysfs is not
> enough. At least, trying to read the ROM is necessary to map the ROM.
> (although nothing can be read in dom0.)
> 
> Thanks,
> Kouya
> 
> Yu Zhao writes:
>> Writing the base address directly to the ROM BAR may cause problem
>> since the kernel is not aware of the operation.
>>
>> Signed-off-by: Yu Zhao <yu.zhao@intel.com>
>> ---
>>  hw/pass-through.c |   25 +++++++++++++++++++++++--
>>  1 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/pass-through.c b/hw/pass-through.c
>> index 207bdf0..e76a3c3 100644
>> --- a/hw/pass-through.c
>> +++ b/hw/pass-through.c
>> @@ -1447,6 +1447,27 @@ exit:
>>      return val;
>>  }
>>  
>> +static int pt_pci_enable_rom(struct pci_dev *dev)
>> +{
>> +    FILE *fp;
>> +    char path[PATH_MAX];
>> +
>> +    sprintf(path,
"/sys/bus/pci/devices/%04x:%02x:%02x.%x/rom",
>> +            dev->domain, dev->bus, dev->dev, dev->func);
>> +
>> +    fp = fopen(path, "w");
>> +    if ( !fp )
>> +    {
>> +        PT_LOG("Can''t open %s: %s\n", path,
strerror(errno));
>> +        return -1;
>> +    }
>> +
>> +    fprintf(fp, "1");
>> +    fclose(fp);
>> +
>> +    return 0;
>> +}
>> +
>>  static void pt_libpci_fixup(struct pci_dev *dev)
>>  {
>>  #if PCI_LIB_VERSION < 0x030100
>> @@ -1526,8 +1547,8 @@ static int pt_register_regions(struct pt_dev
*assigned_device)
>>  
>>          /* Re-set BAR reported by OS, otherwise ROM can''t be
read. */
>>          if ( (pci_dev->rom_base_addr & PCI_ROM_ADDRESS_MASK) ==
0 )
>> -            pci_write_long(pci_dev, PCI_ROM_ADDRESS,
>> -                           (pci_dev->rom_base_addr |
PCI_ROM_ADDRESS_MASK));
>> +            if ( pt_pci_enable_rom(pci_dev) )
>> +                return -1;
>>  
>>          assigned_device->bases[PCI_ROM_SLOT].e_physbase >>   
pci_dev->rom_base_addr;
>> -- 
>> 1.5.6.4
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel

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

Xen devel - Jan 2009 - [PATCH 1/3] ioemu: check mmap return value when mapping MSI-x table

[Xen-devel] [PATCH 1/3] ioemu: check mmap return value when mapping MSI-x table

[Xen-devel] [PATCH 2/3] ioemu: add libpci fixup to handle recent change in PCIUtils

[Xen-devel] [PATCH 3/3] ioemu: use the sysfs interface to enable the PCI ROM

Re: [Xen-devel] [PATCH 3/3] ioemu: use the sysfs interface to enable the PCI ROM

Re: [Xen-devel] [PATCH 3/3] ioemu: use the sysfs interface to enable the PCI ROM