Patchwork [RFC,22/45] qemu-kvm: msix: Fire mask notifier on global mask changes

login
register
mail settings
Submitter Jan Kiszka
Date Oct. 17, 2011, 9:27 a.m.
Message ID <bdbd2c2890972f8fef2d7b9d740b58bda11677e9.1318843693.git.jan.kiszka@siemens.com>
Download mbox | patch
Permalink /patch/120163/
State New
Headers show

Comments

Jan Kiszka - Oct. 17, 2011, 9:27 a.m.
Also invoke the mask notifier if the global MSI-X mask is modified. For
this purpose, we push the notifier call from the per-vector mask update
to the central msix_handle_mask_update.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/msix.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)
Michael S. Tsirkin - Oct. 17, 2011, 12:16 p.m.
On Mon, Oct 17, 2011 at 11:27:56AM +0200, Jan Kiszka wrote:
> Also invoke the mask notifier if the global MSI-X mask is modified. For
> this purpose, we push the notifier call from the per-vector mask update
> to the central msix_handle_mask_update.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

This is a bugfix, isn't it?
If yes it should be separated and put on -stable.

> ---
>  hw/msix.c |   16 +++++++++-------
>  1 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/msix.c b/hw/msix.c
> index 739b56f..247b255 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -221,7 +221,15 @@ static bool msix_is_masked(PCIDevice *dev, int vector)
>  
>  static void msix_handle_mask_update(PCIDevice *dev, int vector)
>  {
> -    if (!msix_is_masked(dev, vector) && msix_is_pending(dev, vector)) {
> +    bool masked = msix_is_masked(dev, vector);
> +    int ret;
> +
> +    if (dev->msix_mask_notifier) {
> +        ret = dev->msix_mask_notifier(dev, vector,
> +                                      msix_is_masked(dev, vector));

Use 'masked' value here as well?

> +        assert(ret >= 0);
> +    }
> +    if (!masked && msix_is_pending(dev, vector)) {
>          msix_clr_pending(dev, vector);
>          msix_notify(dev, vector);
>      }
> @@ -262,7 +270,6 @@ static void msix_mmio_write(void *opaque, target_phys_addr_t addr,
>      unsigned int offset = addr & (MSIX_PAGE_SIZE - 1) & ~0x3;
>      unsigned int vector = offset / PCI_MSIX_ENTRY_SIZE;
>      bool was_masked = msix_is_masked(dev, vector);
> -    int r;
>  
>      pci_set_long(dev->msix_table_page + offset, val);
>      if (kvm_enabled() && kvm_irqchip_in_kernel()) {
> @@ -271,11 +278,6 @@ static void msix_mmio_write(void *opaque, target_phys_addr_t addr,
>  
>      if (vector < dev->msix_entries_nr &&
>          was_masked != msix_is_masked(dev, vector)) {
> -        if (dev->msix_mask_notifier) {
> -            r = dev->msix_mask_notifier(dev, vector,
> -                                        msix_is_masked(dev, vector));
> -            assert(r >= 0);
> -        }
>          msix_handle_mask_update(dev, vector);
>      }
>  }
> -- 
> 1.7.3.4
Jan Kiszka - Oct. 17, 2011, 7 p.m.
On 2011-10-17 14:16, Michael S. Tsirkin wrote:
> On Mon, Oct 17, 2011 at 11:27:56AM +0200, Jan Kiszka wrote:
>> Also invoke the mask notifier if the global MSI-X mask is modified. For
>> this purpose, we push the notifier call from the per-vector mask update
>> to the central msix_handle_mask_update.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> This is a bugfix, isn't it?
> If yes it should be separated and put on -stable.

Yep, will pull this to the front.

> 
>> ---
>>  hw/msix.c |   16 +++++++++-------
>>  1 files changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/msix.c b/hw/msix.c
>> index 739b56f..247b255 100644
>> --- a/hw/msix.c
>> +++ b/hw/msix.c
>> @@ -221,7 +221,15 @@ static bool msix_is_masked(PCIDevice *dev, int vector)
>>  
>>  static void msix_handle_mask_update(PCIDevice *dev, int vector)
>>  {
>> -    if (!msix_is_masked(dev, vector) && msix_is_pending(dev, vector)) {
>> +    bool masked = msix_is_masked(dev, vector);
>> +    int ret;
>> +
>> +    if (dev->msix_mask_notifier) {
>> +        ret = dev->msix_mask_notifier(dev, vector,
>> +                                      msix_is_masked(dev, vector));
> 
> Use 'masked' value here as well?

Yes.

Jan
Michael S. Tsirkin - Oct. 18, 2011, 12:40 p.m.
On Mon, Oct 17, 2011 at 09:00:12PM +0200, Jan Kiszka wrote:
> On 2011-10-17 14:16, Michael S. Tsirkin wrote:
> > On Mon, Oct 17, 2011 at 11:27:56AM +0200, Jan Kiszka wrote:
> >> Also invoke the mask notifier if the global MSI-X mask is modified. For
> >> this purpose, we push the notifier call from the per-vector mask update
> >> to the central msix_handle_mask_update.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > This is a bugfix, isn't it?
> > If yes it should be separated and put on -stable.
> 
> Yep, will pull this to the front.

I'll apply this to qemu.git, no need to mix bugfixes
with features ...

> > 
> >> ---
> >>  hw/msix.c |   16 +++++++++-------
> >>  1 files changed, 9 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/hw/msix.c b/hw/msix.c
> >> index 739b56f..247b255 100644
> >> --- a/hw/msix.c
> >> +++ b/hw/msix.c
> >> @@ -221,7 +221,15 @@ static bool msix_is_masked(PCIDevice *dev, int vector)
> >>  
> >>  static void msix_handle_mask_update(PCIDevice *dev, int vector)
> >>  {
> >> -    if (!msix_is_masked(dev, vector) && msix_is_pending(dev, vector)) {
> >> +    bool masked = msix_is_masked(dev, vector);
> >> +    int ret;
> >> +
> >> +    if (dev->msix_mask_notifier) {
> >> +        ret = dev->msix_mask_notifier(dev, vector,
> >> +                                      msix_is_masked(dev, vector));
> > 
> > Use 'masked' value here as well?
> 
> Yes.
> 
> Jan
>
Jan Kiszka - Oct. 18, 2011, 12:45 p.m.
On 2011-10-18 14:40, Michael S. Tsirkin wrote:
> On Mon, Oct 17, 2011 at 09:00:12PM +0200, Jan Kiszka wrote:
>> On 2011-10-17 14:16, Michael S. Tsirkin wrote:
>>> On Mon, Oct 17, 2011 at 11:27:56AM +0200, Jan Kiszka wrote:
>>>> Also invoke the mask notifier if the global MSI-X mask is modified. For
>>>> this purpose, we push the notifier call from the per-vector mask update
>>>> to the central msix_handle_mask_update.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> This is a bugfix, isn't it?
>>> If yes it should be separated and put on -stable.
>>
>> Yep, will pull this to the front.
> 
> I'll apply this to qemu.git, no need to mix bugfixes
> with features ...

This doesn't apply to qemu.git (there are no notifiers upstream).

Jan
Michael S. Tsirkin - Oct. 18, 2011, 12:57 p.m.
On Tue, Oct 18, 2011 at 02:45:58PM +0200, Jan Kiszka wrote:
> On 2011-10-18 14:40, Michael S. Tsirkin wrote:
> > On Mon, Oct 17, 2011 at 09:00:12PM +0200, Jan Kiszka wrote:
> >> On 2011-10-17 14:16, Michael S. Tsirkin wrote:
> >>> On Mon, Oct 17, 2011 at 11:27:56AM +0200, Jan Kiszka wrote:
> >>>> Also invoke the mask notifier if the global MSI-X mask is modified. For
> >>>> this purpose, we push the notifier call from the per-vector mask update
> >>>> to the central msix_handle_mask_update.
> >>>>
> >>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>
> >>> This is a bugfix, isn't it?
> >>> If yes it should be separated and put on -stable.
> >>
> >> Yep, will pull this to the front.
> > 
> > I'll apply this to qemu.git, no need to mix bugfixes
> > with features ...
> 
> This doesn't apply to qemu.git (there are no notifiers upstream).
> 
> Jan

Right, thanks for the reminder. Anyway, I'll take care
of this, don't worry :)

> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux

Patch

diff --git a/hw/msix.c b/hw/msix.c
index 739b56f..247b255 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -221,7 +221,15 @@  static bool msix_is_masked(PCIDevice *dev, int vector)
 
 static void msix_handle_mask_update(PCIDevice *dev, int vector)
 {
-    if (!msix_is_masked(dev, vector) && msix_is_pending(dev, vector)) {
+    bool masked = msix_is_masked(dev, vector);
+    int ret;
+
+    if (dev->msix_mask_notifier) {
+        ret = dev->msix_mask_notifier(dev, vector,
+                                      msix_is_masked(dev, vector));
+        assert(ret >= 0);
+    }
+    if (!masked && msix_is_pending(dev, vector)) {
         msix_clr_pending(dev, vector);
         msix_notify(dev, vector);
     }
@@ -262,7 +270,6 @@  static void msix_mmio_write(void *opaque, target_phys_addr_t addr,
     unsigned int offset = addr & (MSIX_PAGE_SIZE - 1) & ~0x3;
     unsigned int vector = offset / PCI_MSIX_ENTRY_SIZE;
     bool was_masked = msix_is_masked(dev, vector);
-    int r;
 
     pci_set_long(dev->msix_table_page + offset, val);
     if (kvm_enabled() && kvm_irqchip_in_kernel()) {
@@ -271,11 +278,6 @@  static void msix_mmio_write(void *opaque, target_phys_addr_t addr,
 
     if (vector < dev->msix_entries_nr &&
         was_masked != msix_is_masked(dev, vector)) {
-        if (dev->msix_mask_notifier) {
-            r = dev->msix_mask_notifier(dev, vector,
-                                        msix_is_masked(dev, vector));
-            assert(r >= 0);
-        }
         msix_handle_mask_update(dev, vector);
     }
 }