On Tue, Oct 14, 2014 at 07:41:14PM +0800, ChenLiang wrote:
> We find overlap when the size of pci bar is bigger then 16MB, it overlaps
> with private
> memslot in the kmod. By the way, the new kmod skip private memslot. But I
> think if the size
> of pci bar is enough big, it also overlaps with other memslots.
Of course but it should not cause a crash.
If you need the overlapping memslot available during the programming
process, increase it's priority.
> the root cause is:
>
> pci_default_write_config will do that:
> for (i = 0; i < l; val >>= 8, ++i) {
> uint8_t wmask = d->wmask[addr + i];
> uint8_t w1cmask = d->w1cmask[addr + i];
> assert(!(wmask & w1cmask));
> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask);
> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */
> }
>
> *(int*)(d->config[addr]) will be 0xfe00000c, if val is 0xffffffff and the
> size of bar is 32MB.
> This range overlap with private memslot in the old kmod.
>
> then pci_update_mappings will update memslot.
>
> On 2014/10/14 19:20, Michael S. Tsirkin wrote:
>
> > On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote:
> >> From: ChenLiang <chenlian...@huawei.com>
> >>
> >> Power-up software can determine how much address space the device
> >> requires by writing a value of all 1's to the register and then
> >> reading the value back(PCI specification). Qemu should not do
> >> pci_update_mappings. Qemu may exit, because the wrong address of
> >> this bar is overlap with other memslots.
> >>
> >> Signed-off-by: ChenLiang <chenlian...@huawei.com>
> >> Signed-off-by: Gonglei <arei.gong...@huawei.com>
> >
> > This is at best a work-around.
> > Overlapping is observed in practice, qemu really shouldn't exit when
> > this happens.
> > So we should find the root cause and fix it there instead of
> > adding work-arounds in PCI core.
> >
> > With which device do you observe this?
> >
> >
> >> ---
> >> hw/pci/pci.c | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> >> index 6ce75aa..4d44b44 100644
> >> --- a/hw/pci/pci.c
> >> +++ b/hw/pci/pci.c
> >> @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d,
> >> uint32_t addr, uint32_t val_in, int
> >> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val &
> >> wmask);
> >> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear
> >> */
> >> }
> >> - if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) ||
> >> + if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) ||
> >> ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) ||
> >> - ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) ||
> >> - range_covers_byte(addr, l, PCI_COMMAND))
> >> + ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) &&
> >> + val_in != 0xffffffff) || range_covers_byte(addr, l, PCI_COMMAND))
> >> {
> >> pci_update_mappings(d);
> >> -
> >> + }
> >> if (range_covers_byte(addr, l, PCI_COMMAND)) {
> >> pci_update_irq_disabled(d, was_irq_disabled);
> >> memory_region_set_enabled(&d->bus_master_enable_region,
> >> --
> >> 1.7.12.4
> >>
> >
> > .
> >
>
>
