On 2014/10/14 20:15, chenliang (T) wrote:
> On 2014/10/14 19:48, Michael S. Tsirkin wrote:
>
>> 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.
>>
>
> Yeah, I know the priority of memory region.
> The problem is overlaping should not happen when one pci bar is not
> overlap with any other memslots. But Qemu always do pci_update_mappings
> when guest os writes pci bar. Actually, should not do pci_update_mappings
> if var is 0xffffffff.
>
The PCI spec says [Page 222]:
Decode (I/O or memory) of a register is disabled via the command register
before sizing a
Base Address register. Software saves the original value of the Base Address
register,
writes 0FFFFFFFFh to the register, then reads it back.
>>> 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?
ivshmem device with 32M' bar2 size.
Best regards,
-Gonglei
>>>>
>>>>
>>>>> ---
>>>>> 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
>>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>>
>> .
>>
>
>
>
