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
|
Home