Re: [Qemu-devel] [PATCH] pci: Don't deliver MSI/MSI-X messages if bus master support is off

[Jan Kiszka]

On 2014-07-20 23:03, Michael S. Tsirkin wrote:
> On Sun, Jul 20, 2014 at 11:45:10PM +0200, Jan Kiszka wrote:
>> On 2014-07-20 21:48, Michael S. Tsirkin wrote:
>>> On Sat, Jul 19, 2014 at 06:55:48PM +0200, Jan Kiszka wrote:
>>>> From: Jan Kiszka <address@hidden>
>>>>
>>>> The spec says (and real HW confirms this) that, if the bus master bit
>>>> is 0, the device will not generate any PCI accesses. MSI and MSI-X
>>>> messages fall among these.
>>>>
>>>> Signed-off-by: Jan Kiszka <address@hidden>
>>>
>>> I guess an alternative is for callers to check before
>>> invoking msi_notify. Please note is this is only option
>>> when using e.g. irqfd, so this has some advantages.
>>> Is there a specific device that is affected by this?
>>> I would expect drivers to disable msi before clearing
>>> bus master bit ...
>>
>> This is about emulating conforming behaviour without touching each and
>> every device. I stumbled over this while playing with emulated vs. real
>> Intel HDA.
> 
> Right so that's my question.
> How did you hit it? With a custom driver?

So to say: with a hand full lines of code to tickle some MSI event out
of that device for testing purposes.

> Doesn't regulat driver disable MSI?

Sure. This is not fixing a regular's driver problem. It's a behavioral
correction for faulty corner cases.

Jan

> 
> 
>> It may not be complete, but I think it's a step forward. Irqfd users
>> apparently have to do this themselves then, I didn't look into this. But
>> all the rest should not open-code this logic.
>>
>> Jan
>>
>>>
>>>> ---
>>>>  hw/pci/msi.c  | 4 ++++
>>>>  hw/pci/msix.c | 4 ++++
>>>>  2 files changed, 8 insertions(+)
>>>>
>>>> diff --git a/hw/pci/msi.c b/hw/pci/msi.c
>>>> index a4a3040..36b651b 100644
>>>> --- a/hw/pci/msi.c
>>>> +++ b/hw/pci/msi.c
>>>> @@ -285,6 +285,10 @@ void msi_notify(PCIDevice *dev, unsigned int vector)
>>>>          return;
>>>>      }
>>>>  
>>>> +    if (!(pci_get_word(dev->config + PCI_COMMAND) & PCI_COMMAND_MASTER)) {
>>>> +        return;
>>>> +    }
>>>> +
>>>>      msg = msi_get_message(dev, vector);
>>>>  
>>>>      MSI_DEV_PRINTF(dev,
>>>> diff --git a/hw/pci/msix.c b/hw/pci/msix.c
>>>> index 5c49bfc..c77ae7d 100644
>>>> --- a/hw/pci/msix.c
>>>> +++ b/hw/pci/msix.c
>>>> @@ -437,6 +437,10 @@ void msix_notify(PCIDevice *dev, unsigned vector)
>>>>          return;
>>>>      }
>>>>  
>>>> +    if (!(pci_get_word(dev->config + PCI_COMMAND) & PCI_COMMAND_MASTER)) {
>>>> +        return;
>>>> +    }
>>>> +
>>>>      msg = msix_get_message(dev, vector);
>>>>  
>>>>      stl_le_phys(&address_space_memory, msg.address, msg.data);
>>>> -- 
>>>> 1.8.1.1.298.ge7eed54
>>>>
>>>
>>>
>>
>>
> 
>

signature.asc
Description: OpenPGP digital signature