pci: Don't deliver MSI/MSI-X messages if bus master support is off
PATCH 1/1 pci: Don't deliver MSI/MSI-X messages if bus master support is off
From: Jan Kiszka <[email protected]>
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.
--- hw/pci/msi.c | 4 ++++ hw/pci/msix.c | 4 ++++ 2 files changed, 8 insertions(+)
hw/pci/msi.c +4 -0
@@ -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,
hw/pci/msix.c +4 -0
@@ -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);
Signature · 1 line
1.8.1.1.298.ge7eed54
Quoted 7 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
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 <[email protected]>
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 ...
Quoted 38 lines
--- hw/pci/msi.c | 4 ++++ hw/pci/msix.c | 4 ++++ 2 files changed, 8 insertions(+)hw/pci/msi.c +4
-0@@ -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,hw/pci/msix.c +4
-0@@ -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);Signature · 1 line
1.8.1.1.298.ge7eed54
Quoted 7 lines · Michael S. Tsirkin
Quoted 6 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
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.
Quoted 8 lines
Quoted 1 line
Signed-off-by Jan Kiszka <[email protected]>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.
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
Quoted 41 lines
Quoted 39 lines
--- hw/pci/msi.c | 4 ++++ hw/pci/msix.c | 4 ++++ 2 files changed, 8 insertions(+)hw/pci/msi.c +4
-0@@ -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,hw/pci/msix.c +4
-0@@ -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);Signature · 1 line
1.8.1.1.298.ge7eed54
Quoted 17 lines · Jan Kiszka
Quoted 7 lines · Michael S. Tsirkin
Quoted 6 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
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.
Quoted 8 lines
Quoted 1 line
Signed-off-by Jan Kiszka <[email protected]>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 ...
Quoted 3 lines
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?
Doesn't regulat driver disable MSI?
Quoted 49 lines
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
Quoted 41 lines
Quoted 39 lines
--- hw/pci/msi.c | 4 ++++ hw/pci/msix.c | 4 ++++ 2 files changed, 8 insertions(+)hw/pci/msi.c +4
-0@@ -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,hw/pci/msix.c +4
-0@@ -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);Signature · 1 line
1.8.1.1.298.ge7eed54
Quoted 18 lines · Michael S. Tsirkin
Quoted 17 lines · Jan Kiszka
Quoted 15 lines · Michael S. Tsirkin
Quoted 7 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
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 <[email protected]>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 ...
Quoted 6 lines
Quoted 3 lines
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.
Quoted 1 line
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
Quoted 53 lines
Quoted 51 lines
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
Quoted 41 lines
Quoted 39 lines
--- hw/pci/msi.c | 4 ++++ hw/pci/msix.c | 4 ++++ 2 files changed, 8 insertions(+)hw/pci/msi.c +4
-0@@ -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,hw/pci/msix.c +4
-0@@ -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);Signature · 1 line
1.8.1.1.298.ge7eed54
Il 20/07/2014 21:48, Michael S. Tsirkin ha scritto:
Quoted 35 lines
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 ...Quoted 28 lines
Quoted 28 lines
--- hw/pci/msi.c | 4 ++++ hw/pci/msix.c | 4 ++++ 2 files changed, 8 insertions(+)hw/pci/msi.c +4
-0@@ -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,hw/pci/msix.c +0
-0@@ -437,6 +437,10 @@ void msix_notify(PCIDevice *dev, unsigned vector)return ; }
Quoted 7 lines
Quoted 7 lines
hw/pci/msix.c void msix_notify(PCIDevice *dev, unsigned vector) · line 446
Quoted 7 lines
hw/pci/msix.c +4
-0+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);
I think a better way to do this, is to use the PCI bus master address space instead of address_space_memory.
Even if it doesn't fix irqfd, it will make the MSI write go through the IOMMU as it should.
Paolo
Quoted 29 lines · Jan Kiszka
Quoted 18 lines · Michael S. Tsirkin
Quoted 17 lines · Jan Kiszka
Quoted 15 lines · Michael S. Tsirkin
Quoted 7 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
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 <[email protected]>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 ...Quoted 6 lines
Quoted 3 lines
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.
Quoted 6 lines
Quoted 1 line
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
OK based on this I think this is not 2.1 material. Agree?
Quoted 55 lines
Quoted 53 lines
Quoted 51 lines
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
Quoted 41 lines
Quoted 39 lines
--- hw/pci/msi.c | 4 ++++ hw/pci/msix.c | 4 ++++ 2 files changed, 8 insertions(+)hw/pci/msi.c +4
-0@@ -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,hw/pci/msix.c +4
-0@@ -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);Signature · 1 line
1.8.1.1.298.ge7eed54
Quoted 30 lines · Michael S. Tsirkin
Quoted 29 lines · Jan Kiszka
Quoted 18 lines · Michael S. Tsirkin
Quoted 17 lines · Jan Kiszka
Quoted 15 lines · Michael S. Tsirkin
Quoted 7 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
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 <[email protected]>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 ...Quoted 6 lines
Quoted 3 lines
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.
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
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
OK based on this I think this is not 2.1 material. Agree?
Agree.
I'll look into Paolo's suggestion how to model this asap.
Jan