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

[Xen-devel] RE: [PATCH][ioemu] fix PCI bar mapping

To: Yuji Shimada <shimada-yxb@xxxxxxxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Subject: [Xen-devel] RE: [PATCH][ioemu] fix PCI bar mapping
From: "Cui, Dexuan" <dexuan.cui@xxxxxxxxx>
Date: Fri, 8 May 2009 15:49:21 +0800
Accept-language: zh-CN, en-US
Acceptlanguage: zh-CN, en-US
Cc: "Zhao, Yu" <yu.zhao@xxxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Simon Horman <horms@xxxxxxxxxxxx>, "Ke, Liping" <liping.ke@xxxxxxxxx>
Delivery-date: Fri, 08 May 2009 00:50:15 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <EADF0A36011179459010BDF5142A45751555FE10@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
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: <20090507163136.983F.27C06F64@xxxxxxxxxxxxxxx> <EADF0A36011179459010BDF5142A45751555FCB0@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <20090508090105.F0B6.27C06F64@xxxxxxxxxxxxxxx> <EADF0A36011179459010BDF5142A45751555FE10@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcnPdoGoJX6JY4XgT1+rCZecVrNR/AAC0lqQAAs1RQA=
Thread-topic: [PATCH][ioemu] fix PCI bar mapping
Cui, Dexuan wrote:
> Thanks a lot for Yuji's review!
> 
> Anyway, looks this new patch doesn't handle properly the case of
> SR/IOV VF. I'm improving this and will send out a new patch ASAP.
> Sorry. 
> 
> Thanks,
> -- Dexuan
> 
> -----Original Message-----
> From: Yuji Shimada [mailto:shimada-yxb@xxxxxxxxxxxxxxx]
> Sent: 2009年5月8日 8:47
> To: Cui, Dexuan; Ian Jackson
> Cc: Ke, Liping; Zhao, Yu; xen-devel
> Subject: Re: [PATCH][ioemu] fix PCI bar mapping
> 
> On Thu, 7 May 2009 20:07:53 +0800
> "Cui, Dexuan" <dexuan.cui@xxxxxxxxx> wrote:
> 
>>  Hi Yuji,
>> Thanks a lot for the comments!
>> 
>>> Or, there is another approach. It is that you remove emu_mask from
>>> writable_mask in pt_cmd_reg_write. Then you can get the proper value
>>> from reg_entry->data.
>> I prefer this approach.
>> Attached is the patch. Could you help to have a review?
>> 
>> Thanks,
>> -- Dexuan
>> 
> 
> Hi Cui,
> 
> Thanks for sending the patch.
> 
> The patch seems to be good.
> 
> Thanks,


Hi all,
Sorry, my previous mail turns out to be a false alarm. :-)
Actually SR-IOV VF can still work fine with the patch. I also make some other 
tests and it works fine.

So, Ian, please apply the pach.
Let me paste the patch below for your convenience:

--------------------------------------

passthrough: pt_bar_mapping: use a better way to get the CMD value

The  pt_pci_read_config(&ptdev->dev, PCI_COMMAND, 2) in 
5d767b7b3fac52336f59e5b40d8befa6b1909937 is not proper as Yuji Shimada points 
out: "pt_pci_read_config emulates access to read the registers from guest 
software. Many functions which are not relevant are executed in 
pt_pci_read_config. So side effects may occur"; instead, we can "remove 
emu_mask from writable_mask in pt_cmd_reg_write and then we can get the proper 
value from reg_entry->data".
Thanks for Yuji's review and Simon Horman's test.

Signed-off-by: Dexuan Cui <dexuan.cui@xxxxxxxxx>

diff --git a/hw/pass-through.c b/hw/pass-through.c
index d2bed51..51a39db 100644
--- a/hw/pass-through.c
+++ b/hw/pass-through.c
@@ -1796,6 +1796,8 @@ static void pt_bar_mapping_one(struct pt_dev *ptdev, int 
bar, int io_enable,
 {
     PCIDevice *dev = (PCIDevice *)&ptdev->dev;
     PCIIORegion *r;
+    struct pt_reg_grp_tbl *reg_grp_entry = NULL;
+    struct pt_reg_tbl *reg_entry = NULL;
     struct pt_region *base = NULL;
     uint32_t r_size = 0, r_addr = -1;
     int ret = 0;
@@ -1821,10 +1823,13 @@ static void pt_bar_mapping_one(struct pt_dev *ptdev, 
int bar, int io_enable,
         r_addr = -1;
     if ( (bar == PCI_ROM_SLOT) && (r_addr != -1) )
     {
-        uint32_t rom_reg;
-        rom_reg = pt_pci_read_config(&ptdev->dev, PCI_ROM_ADDRESS, 4);
-        if ( !(rom_reg & PCI_ROM_ADDRESS_ENABLE) )
-            r_addr = -1;
+        reg_grp_entry = pt_find_reg_grp(ptdev, PCI_ROM_ADDRESS);
+        if (reg_grp_entry)
+        {
+            reg_entry = pt_find_reg(reg_grp_entry, PCI_ROM_ADDRESS);
+            if (reg_entry && !(reg_entry->data & PCI_ROM_ADDRESS_ENABLE))
+                r_addr = -1;
+        }
     }
 
     /* prevent guest software mapping memory resource to 00000000h */
@@ -3011,7 +3016,7 @@ static int pt_cmd_reg_write(struct pt_dev *ptdev,
         emu_mask |= PCI_COMMAND_MEMORY;
 
     /* modify emulate register */
-    writable_mask = emu_mask & ~reg->ro_mask & valid_mask;
+    writable_mask = ~reg->ro_mask & valid_mask;
     cfg_entry->data = PT_MERGE_VALUE(*value, cfg_entry->data, writable_mask);
 
     /* create value for writing to I/O device register */
@@ -3061,7 +3066,6 @@ static int pt_bar_reg_write(struct pt_dev *ptdev,
     uint32_t prev_offset;
     uint32_t r_size = 0;
     int index = 0;
-    uint16_t cmd;
 
     /* get BAR index */
     index = pt_bar_offset_to_index(reg->offset);
@@ -3181,9 +3185,14 @@ exit:
     *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask);
 
     /* After BAR reg update, we need to remap BAR*/
-    cmd = pt_pci_read_config(&ptdev->dev, PCI_COMMAND, 2);
-    pt_bar_mapping_one(ptdev, index, cmd & PCI_COMMAND_IO,
-        cmd & PCI_COMMAND_MEMORY);
+    reg_grp_entry = pt_find_reg_grp(ptdev, PCI_COMMAND);
+    if (reg_grp_entry)
+    {
+        reg_entry = pt_find_reg(reg_grp_entry, PCI_COMMAND);
+        if (reg_entry)
+            pt_bar_mapping_one(ptdev, index, reg_entry->data & PCI_COMMAND_IO,
+                               reg_entry->data & PCI_COMMAND_MEMORY);
+    }
 
     return 0;
 }
@@ -3194,6 +3203,8 @@ static int pt_exp_rom_bar_reg_write(struct pt_dev *ptdev,
         uint32_t *value, uint32_t dev_value, uint32_t valid_mask)
 {
     struct pt_reg_info_tbl *reg = cfg_entry->reg;
+    struct pt_reg_grp_tbl *reg_grp_entry = NULL;
+    struct pt_reg_tbl *reg_entry = NULL;
     struct pt_region *base = NULL;
     PCIDevice *d = (PCIDevice *)&ptdev->dev;
     PCIIORegion *r;
@@ -3202,7 +3213,6 @@ static int pt_exp_rom_bar_reg_write(struct pt_dev *ptdev,
     uint32_t r_size = 0;
     uint32_t bar_emu_mask = 0;
     uint32_t bar_ro_mask = 0;
-    uint16_t cmd;
 
     r = &d->io_regions[PCI_ROM_SLOT];
     r_size = r->size;
@@ -3212,10 +3222,10 @@ static int pt_exp_rom_bar_reg_write(struct pt_dev 
*ptdev,
 
     /* set emulate mask and read-only mask */
     bar_emu_mask = reg->emu_mask;
-    bar_ro_mask = reg->ro_mask | (r_size - 1);
+    bar_ro_mask = (reg->ro_mask | (r_size - 1)) & ~PCI_ROM_ADDRESS_ENABLE;
 
     /* modify emulate register */
-    writable_mask = bar_emu_mask & ~bar_ro_mask & valid_mask;
+    writable_mask = ~bar_ro_mask & valid_mask;
     cfg_entry->data = PT_MERGE_VALUE(*value, cfg_entry->data, writable_mask);
 
     /* update the corresponding virtual region address */
@@ -3226,9 +3236,15 @@ static int pt_exp_rom_bar_reg_write(struct pt_dev *ptdev,
     *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask);
 
     /* After BAR reg update, we need to remap BAR*/
-    cmd = pt_pci_read_config(&ptdev->dev, PCI_COMMAND, 2);
-    pt_bar_mapping_one(ptdev, PCI_ROM_SLOT, cmd & PCI_COMMAND_IO,
-        cmd & PCI_COMMAND_MEMORY);
+    reg_grp_entry = pt_find_reg_grp(ptdev, PCI_COMMAND);
+    if (reg_grp_entry)
+    {
+        reg_entry = pt_find_reg(reg_grp_entry, PCI_COMMAND);
+        if (reg_entry)
+            pt_bar_mapping_one(ptdev, PCI_ROM_SLOT,
+                               reg_entry->data & PCI_COMMAND_IO,
+                               reg_entry->data & PCI_COMMAND_MEMORY);
+    }
 
     return 0;
 }
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel