On 2014/10/14 19:58, 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.
>>
>> 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.
>
>
> In fact, ever since
> 83d08f2673504a299194dcac1657a13754b5932a
> pc: map PCI address space as catchall region for not mapped addresses
>
> all pci memory has lower priority than ioapic at 0xfe0000000.
>
> so ioapic will win, there should be no issue.
>
> IOW this is not the root cause.
TSS_PRIVATE_MEMSLOT and IDENTITY_PAGETABLE_PRIVATE_MEMSLOT also will overlap.
>
>
>
>> 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
>>>>
>>>
>>> .
>>>
>>
>>
>
> .
>
