[Qemu-devel] [PATCH 0/3] virtio pmem: coverity fixes

[Qemu-devel] [PATCH 0/3] virtio pmem: coverity fixes

[Pankaj Gupta]

This patch series two fixes for coverity and a 
transactional info removal patch.

Pankaj Gupta (3):
  virtio pmem: fix wrong mem region condition
  virtio pmem: remove memdev null check
  virtio pmem: remove transational device info

 hw/virtio/virtio-pmem-pci.c | 4 +---
 hw/virtio/virtio-pmem.c     | 4 ++--
 2 files changed, 3 insertions(+), 5 deletions(-)

-- 
2.14.5

[Qemu-devel] [PATCH 1/3] virtio pmem: fix wrong mem region condition

[Pankaj Gupta]

Coverity reported memory region returns zero
for non-null value. This is because of wrong
arguments to '?:' , fixing this.

Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
---
 hw/virtio/virtio-pmem-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pmem-pci.c b/hw/virtio/virtio-pmem-pci.c
index 8b2d0dbccc..0da6627469 100644
--- a/hw/virtio/virtio-pmem-pci.c
+++ b/hw/virtio/virtio-pmem-pci.c
@@ -57,7 +57,7 @@ static uint64_t virtio_pmem_pci_get_plugged_size(const MemoryDeviceState *md,
     MemoryRegion *mr = vpc->get_memory_region(pmem, errp);
 
     /* the plugged size corresponds to the region size */
-    return mr ? 0 : memory_region_size(mr);
+    return mr ? memory_region_size(mr) : 0;
 }
 
 static void virtio_pmem_pci_fill_device_info(const MemoryDeviceState *md,
-- 
2.14.5

[Qemu-devel] [PULL 6/8] virtio pmem: fix wrong mem region condition

[Michael S. Tsirkin]

From: Pankaj Gupta <pagupta@redhat.com>

Coverity reported memory region returns zero
for non-null value. This is because of wrong
arguments to '?:' , fixing this.

Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
Message-Id: <20190712073554.21918-2-pagupta@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/virtio/virtio-pmem-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pmem-pci.c b/hw/virtio/virtio-pmem-pci.c
index 8b2d0dbccc..0da6627469 100644
--- a/hw/virtio/virtio-pmem-pci.c
+++ b/hw/virtio/virtio-pmem-pci.c
@@ -57,7 +57,7 @@ static uint64_t virtio_pmem_pci_get_plugged_size(const MemoryDeviceState *md,
     MemoryRegion *mr = vpc->get_memory_region(pmem, errp);
 
     /* the plugged size corresponds to the region size */
-    return mr ? 0 : memory_region_size(mr);
+    return mr ? memory_region_size(mr) : 0;
 }
 
 static void virtio_pmem_pci_fill_device_info(const MemoryDeviceState *md,
-- 
MST

Re: [Qemu-devel] [PATCH 1/3] virtio pmem: fix wrong mem region condition

[Stefano Garzarella]

On Fri, Jul 12, 2019 at 01:05:52PM +0530, Pankaj Gupta wrote:
> Coverity reported memory region returns zero
> for non-null value. This is because of wrong
> arguments to '?:' , fixing this.
> 
> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> ---
>  hw/virtio/virtio-pmem-pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

> 
> diff --git a/hw/virtio/virtio-pmem-pci.c b/hw/virtio/virtio-pmem-pci.c
> index 8b2d0dbccc..0da6627469 100644
> --- a/hw/virtio/virtio-pmem-pci.c
> +++ b/hw/virtio/virtio-pmem-pci.c
> @@ -57,7 +57,7 @@ static uint64_t virtio_pmem_pci_get_plugged_size(const MemoryDeviceState *md,
>      MemoryRegion *mr = vpc->get_memory_region(pmem, errp);
>  
>      /* the plugged size corresponds to the region size */
> -    return mr ? 0 : memory_region_size(mr);
> +    return mr ? memory_region_size(mr) : 0;
>  }
>  
>  static void virtio_pmem_pci_fill_device_info(const MemoryDeviceState *md,
> -- 
> 2.14.5

Re: [Qemu-devel] [PATCH 1/3] virtio pmem: fix wrong mem region condition

[Cornelia Huck]

On Fri, 12 Jul 2019 13:05:52 +0530
Pankaj Gupta <pagupta@redhat.com> wrote:

> Coverity reported memory region returns zero
> for non-null value. This is because of wrong
> arguments to '?:' , fixing this.
> 
> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> ---
>  hw/virtio/virtio-pmem-pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [Qemu-devel] [PATCH 1/3] virtio pmem: fix wrong mem region condition

[Philippe Mathieu-Daudé]

On 7/12/19 9:35 AM, Pankaj Gupta wrote:
> Coverity reported memory region returns zero
> for non-null value. This is because of wrong
> arguments to '?:' , fixing this.
> 

Please amend:

Fixes: Coverity (CID 1403009)

> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  hw/virtio/virtio-pmem-pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-pmem-pci.c b/hw/virtio/virtio-pmem-pci.c
> index 8b2d0dbccc..0da6627469 100644
> --- a/hw/virtio/virtio-pmem-pci.c
> +++ b/hw/virtio/virtio-pmem-pci.c
> @@ -57,7 +57,7 @@ static uint64_t virtio_pmem_pci_get_plugged_size(const MemoryDeviceState *md,
>      MemoryRegion *mr = vpc->get_memory_region(pmem, errp);
>  
>      /* the plugged size corresponds to the region size */
> -    return mr ? 0 : memory_region_size(mr);
> +    return mr ? memory_region_size(mr) : 0;
>  }
>  
>  static void virtio_pmem_pci_fill_device_info(const MemoryDeviceState *md,
> 

Re: [Qemu-devel] [PATCH 1/3] virtio pmem: fix wrong mem region condition

[David Hildenbrand]

On 12.07.19 09:35, Pankaj Gupta wrote:
> Coverity reported memory region returns zero
> for non-null value. This is because of wrong
> arguments to '?:' , fixing this.
> 
> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> ---
>  hw/virtio/virtio-pmem-pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-pmem-pci.c b/hw/virtio/virtio-pmem-pci.c
> index 8b2d0dbccc..0da6627469 100644
> --- a/hw/virtio/virtio-pmem-pci.c
> +++ b/hw/virtio/virtio-pmem-pci.c
> @@ -57,7 +57,7 @@ static uint64_t virtio_pmem_pci_get_plugged_size(const MemoryDeviceState *md,
>      MemoryRegion *mr = vpc->get_memory_region(pmem, errp);
>  
>      /* the plugged size corresponds to the region size */
> -    return mr ? 0 : memory_region_size(mr);
> +    return mr ? memory_region_size(mr) : 0;
>  }
>  
>  static void virtio_pmem_pci_fill_device_info(const MemoryDeviceState *md,
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

[Qemu-devel] [PATCH 2/3] virtio pmem: remove memdev null check

[Pankaj Gupta]

Coverity reports that when we're assigning vi->size we handle the 
"pmem->memdev is NULL" case; but we then pass it into 
object_get_canonical_path(), which unconditionally dereferences it
and will crash if it is NULL. If this pointer can be NULL then we
need to do something else here.

We are removing 'pmem->memdev' null check here as memdev will never
be null in this function.

Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
---
 hw/virtio/virtio-pmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pmem.c b/hw/virtio/virtio-pmem.c
index adbfb603ab..17c196d107 100644
--- a/hw/virtio/virtio-pmem.c
+++ b/hw/virtio/virtio-pmem.c
@@ -134,8 +134,8 @@ static void virtio_pmem_fill_device_info(const VirtIOPMEM *pmem,
                                          VirtioPMEMDeviceInfo *vi)
 {
     vi->memaddr = pmem->start;
-    vi->size = pmem->memdev ? memory_region_size(&pmem->memdev->mr) : 0;
-    vi->memdev = object_get_canonical_path(OBJECT(pmem->memdev));
+    vi->size    = memory_region_size(&pmem->memdev->mr);
+    vi->memdev  = object_get_canonical_path(OBJECT(pmem->memdev));
 }
 
 static MemoryRegion *virtio_pmem_get_memory_region(VirtIOPMEM *pmem,
-- 
2.14.5

[Qemu-devel] [PULL 7/8] virtio pmem: remove memdev null check

[Michael S. Tsirkin]

From: Pankaj Gupta <pagupta@redhat.com>

Coverity reports that when we're assigning vi->size we handle the
"pmem->memdev is NULL" case; but we then pass it into
object_get_canonical_path(), which unconditionally dereferences it
and will crash if it is NULL. If this pointer can be NULL then we
need to do something else here.

We are removing 'pmem->memdev' null check here as memdev will never
be null in this function.

Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
Message-Id: <20190712073554.21918-3-pagupta@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/virtio/virtio-pmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pmem.c b/hw/virtio/virtio-pmem.c
index adbfb603ab..17c196d107 100644
--- a/hw/virtio/virtio-pmem.c
+++ b/hw/virtio/virtio-pmem.c
@@ -134,8 +134,8 @@ static void virtio_pmem_fill_device_info(const VirtIOPMEM *pmem,
                                          VirtioPMEMDeviceInfo *vi)
 {
     vi->memaddr = pmem->start;
-    vi->size = pmem->memdev ? memory_region_size(&pmem->memdev->mr) : 0;
-    vi->memdev = object_get_canonical_path(OBJECT(pmem->memdev));
+    vi->size    = memory_region_size(&pmem->memdev->mr);
+    vi->memdev  = object_get_canonical_path(OBJECT(pmem->memdev));
 }
 
 static MemoryRegion *virtio_pmem_get_memory_region(VirtIOPMEM *pmem,
-- 
MST

Re: [Qemu-devel] [PATCH 2/3] virtio pmem: remove memdev null check

[Cornelia Huck]

On Fri, 12 Jul 2019 13:05:53 +0530
Pankaj Gupta <pagupta@redhat.com> wrote:

> Coverity reports that when we're assigning vi->size we handle the 
> "pmem->memdev is NULL" case; but we then pass it into 
> object_get_canonical_path(), which unconditionally dereferences it
> and will crash if it is NULL. If this pointer can be NULL then we
> need to do something else here.
> 
> We are removing 'pmem->memdev' null check here as memdev will never
> be null in this function.

Indeed, we'll fail to realize the device if it is NULL.

> 
> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> ---
>  hw/virtio/virtio-pmem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

[Qemu-devel] [PATCH 3/3] virtio pmem: remove transational device info

[Pankaj Gupta]

Remove transactional & non transactional device info
for virtio pmem. 

Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
---
 hw/virtio/virtio-pmem-pci.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/virtio/virtio-pmem-pci.c b/hw/virtio/virtio-pmem-pci.c
index 0da6627469..fe2af00fa1 100644
--- a/hw/virtio/virtio-pmem-pci.c
+++ b/hw/virtio/virtio-pmem-pci.c
@@ -113,8 +113,6 @@ static void virtio_pmem_pci_instance_init(Object *obj)
 static const VirtioPCIDeviceTypeInfo virtio_pmem_pci_info = {
     .base_name             = TYPE_VIRTIO_PMEM_PCI,
     .generic_name          = "virtio-pmem-pci",
-    .transitional_name     = "virtio-pmem-pci-transitional",
-    .non_transitional_name = "virtio-pmem-pci-non-transitional",
     .instance_size = sizeof(VirtIOPMEMPCI),
     .instance_init = virtio_pmem_pci_instance_init,
     .class_init    = virtio_pmem_pci_class_init,
-- 
2.14.5

[Qemu-devel] [PULL 8/8] virtio pmem: remove transitional names

[Michael S. Tsirkin]

From: Pankaj Gupta <pagupta@redhat.com>

Remove transitional & non transitional names for virtio pmem.
Only virtio 1.0 and up is supported.

Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
Message-Id: <20190712073554.21918-4-pagupta@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/virtio/virtio-pmem-pci.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/virtio/virtio-pmem-pci.c b/hw/virtio/virtio-pmem-pci.c
index 0da6627469..fe2af00fa1 100644
--- a/hw/virtio/virtio-pmem-pci.c
+++ b/hw/virtio/virtio-pmem-pci.c
@@ -113,8 +113,6 @@ static void virtio_pmem_pci_instance_init(Object *obj)
 static const VirtioPCIDeviceTypeInfo virtio_pmem_pci_info = {
     .base_name             = TYPE_VIRTIO_PMEM_PCI,
     .generic_name          = "virtio-pmem-pci",
-    .transitional_name     = "virtio-pmem-pci-transitional",
-    .non_transitional_name = "virtio-pmem-pci-non-transitional",
     .instance_size = sizeof(VirtIOPMEMPCI),
     .instance_init = virtio_pmem_pci_instance_init,
     .class_init    = virtio_pmem_pci_class_init,
-- 
MST

Re: [Qemu-devel] [PATCH 3/3] virtio pmem: remove transational device info

[Cornelia Huck]

On Fri, 12 Jul 2019 13:05:54 +0530
Pankaj Gupta <pagupta@redhat.com> wrote:

> Remove transactional & non transactional device info
> for virtio pmem. 

s/device info/names/ ?

> 
> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> ---
>  hw/virtio/virtio-pmem-pci.c | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [Qemu-devel] [PATCH 3/3] virtio pmem: remove transational device info

[Pankaj Gupta]

> 
> On Fri, 12 Jul 2019 13:05:54 +0530
> Pankaj Gupta <pagupta@redhat.com> wrote:
> 
> > Remove transactional & non transactional device info
> > for virtio pmem.
> 
> s/device info/names/ ?

yes.

> 
> > 
> > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > ---
> >  hw/virtio/virtio-pmem-pci.c | 2 --
> >  1 file changed, 2 deletions(-)
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> 

Re: [Qemu-devel] [PATCH 3/3] virtio pmem: remove transational device info

[David Hildenbrand]

On 12.07.19 09:35, Pankaj Gupta wrote:
> Remove transactional & non transactional device info
> for virtio pmem. 

Can you explain and add *why* ?

> 
> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> ---
>  hw/virtio/virtio-pmem-pci.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pmem-pci.c b/hw/virtio/virtio-pmem-pci.c
> index 0da6627469..fe2af00fa1 100644
> --- a/hw/virtio/virtio-pmem-pci.c
> +++ b/hw/virtio/virtio-pmem-pci.c
> @@ -113,8 +113,6 @@ static void virtio_pmem_pci_instance_init(Object *obj)
>  static const VirtioPCIDeviceTypeInfo virtio_pmem_pci_info = {
>      .base_name             = TYPE_VIRTIO_PMEM_PCI,
>      .generic_name          = "virtio-pmem-pci",
> -    .transitional_name     = "virtio-pmem-pci-transitional",
> -    .non_transitional_name = "virtio-pmem-pci-non-transitional",
>      .instance_size = sizeof(VirtIOPMEMPCI),
>      .instance_init = virtio_pmem_pci_instance_init,
>      .class_init    = virtio_pmem_pci_class_init,
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 3/3] virtio pmem: remove transational device info

[Pankaj Gupta]

> 
> On 12.07.19 09:35, Pankaj Gupta wrote:
> > Remove transactional & non transactional device info
> > for virtio pmem.
> 
> Can you explain and add *why* ?

As per upstream suggestion by Cornelia & MST, transactional devices are for
legacy purpose. So, does not make sense for virtio-pmem.

Thanks,
Pankaj 

> 
> > 
> > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > ---
> >  hw/virtio/virtio-pmem-pci.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/hw/virtio/virtio-pmem-pci.c b/hw/virtio/virtio-pmem-pci.c
> > index 0da6627469..fe2af00fa1 100644
> > --- a/hw/virtio/virtio-pmem-pci.c
> > +++ b/hw/virtio/virtio-pmem-pci.c
> > @@ -113,8 +113,6 @@ static void virtio_pmem_pci_instance_init(Object *obj)
> >  static const VirtioPCIDeviceTypeInfo virtio_pmem_pci_info = {
> >      .base_name             = TYPE_VIRTIO_PMEM_PCI,
> >      .generic_name          = "virtio-pmem-pci",
> > -    .transitional_name     = "virtio-pmem-pci-transitional",
> > -    .non_transitional_name = "virtio-pmem-pci-non-transitional",
> >      .instance_size = sizeof(VirtIOPMEMPCI),
> >      .instance_init = virtio_pmem_pci_instance_init,
> >      .class_init    = virtio_pmem_pci_class_init,
> > 
> 
> 
> --
> 
> Thanks,
> 
> David / dhildenb
> 

Re: [Qemu-devel] [PATCH 3/3] virtio pmem: remove transational device info

[David Hildenbrand]

On 15.07.19 09:22, Pankaj Gupta wrote:
> 
>>
>> On 12.07.19 09:35, Pankaj Gupta wrote:
>>> Remove transactional & non transactional device info
>>> for virtio pmem.
>>
>> Can you explain and add *why* ?
> 
> As per upstream suggestion by Cornelia & MST, transactional devices are for
> legacy purpose. So, does not make sense for virtio-pmem.

With something like that added to the description

Reviewed-by: David Hildenbrand <david@redhat.com>

> 
> Thanks,
> Pankaj 
> 
>>
>>>
>>> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
>>> ---
>>>  hw/virtio/virtio-pmem-pci.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/hw/virtio/virtio-pmem-pci.c b/hw/virtio/virtio-pmem-pci.c
>>> index 0da6627469..fe2af00fa1 100644
>>> --- a/hw/virtio/virtio-pmem-pci.c
>>> +++ b/hw/virtio/virtio-pmem-pci.c
>>> @@ -113,8 +113,6 @@ static void virtio_pmem_pci_instance_init(Object *obj)
>>>  static const VirtioPCIDeviceTypeInfo virtio_pmem_pci_info = {
>>>      .base_name             = TYPE_VIRTIO_PMEM_PCI,
>>>      .generic_name          = "virtio-pmem-pci",
>>> -    .transitional_name     = "virtio-pmem-pci-transitional",
>>> -    .non_transitional_name = "virtio-pmem-pci-non-transitional",
>>>      .instance_size = sizeof(VirtIOPMEMPCI),
>>>      .instance_init = virtio_pmem_pci_instance_init,
>>>      .class_init    = virtio_pmem_pci_class_init,
>>>
>>
>>
>> --
>>
>> Thanks,
>>
>> David / dhildenb
>>


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 0/3] virtio pmem: coverity fixes

[Cornelia Huck]

On Fri, 12 Jul 2019 13:05:51 +0530
Pankaj Gupta <pagupta@redhat.com> wrote:

> This patch series two fixes for coverity and a 
> transactional info removal patch.
> 
> Pankaj Gupta (3):
>   virtio pmem: fix wrong mem region condition
>   virtio pmem: remove memdev null check
>   virtio pmem: remove transational device info
> 
>  hw/virtio/virtio-pmem-pci.c | 4 +---
>  hw/virtio/virtio-pmem.c     | 4 ++--
>  2 files changed, 3 insertions(+), 5 deletions(-)
> 

I think all of this is 4.1 material.

Re: [Qemu-devel] [PATCH 0/3] virtio pmem: coverity fixes

[Pankaj Gupta]

> 
> On Fri, 12 Jul 2019 13:05:51 +0530
> Pankaj Gupta <pagupta@redhat.com> wrote:
> 
> > This patch series two fixes for coverity and a
> > transactional info removal patch.
> > 
> > Pankaj Gupta (3):
> >   virtio pmem: fix wrong mem region condition
> >   virtio pmem: remove memdev null check
> >   virtio pmem: remove transational device info
> > 
> >  hw/virtio/virtio-pmem-pci.c | 4 +---
> >  hw/virtio/virtio-pmem.c     | 4 ++--
> >  2 files changed, 3 insertions(+), 5 deletions(-)
> > 
> 
> I think all of this is 4.1 material.

Yes, forgot to add in the subject.

Thank you for the review.

Best regards,
Pankaj

> 
> 

[Qemu-devel] [PULL 0/8] virtio, pc, pci: fixes, cleanups, tests

[Michael S. Tsirkin]

The following changes since commit a2a9d4adabe340617a24eb73a8b2a116d28a6b38:

  Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-4.1-20190712' into staging (2019-07-12 11:06:48 +0100)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to 207efa18ac9eb7085a44cad24489d0da54bc5f65:

  virtio pmem: remove transitional names (2019-07-12 10:57:27 -0400)

----------------------------------------------------------------
virtio, pc, pci: fixes, cleanups, tests

A bunch of fixes all over the place.
ACPI tests will now run on more systems: might
introduce new failure reports but that's for
the best, isn't it?

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Igor Mammedov (2):
      tests: acpi: do not require IASL for dumping AML blobs
      tests: acpi: do not skip tests when IASL is not installed

Michael S. Tsirkin (2):
      xio3130_downstream: typo fix
      pcie: consistent names for function args

Pankaj Gupta (3):
      virtio pmem: fix wrong mem region condition
      virtio pmem: remove memdev null check
      virtio pmem: remove transitional names

Stefan Hajnoczi (1):
      virtio-balloon: fix QEMU 4.0 config size migration incompatibility

 include/hw/pci/pcie.h              |  4 ++--
 include/hw/virtio/virtio-balloon.h |  2 ++
 hw/core/machine.c                  |  2 ++
 hw/pci-bridge/xio3130_downstream.c |  2 +-
 hw/virtio/virtio-balloon.c         | 28 +++++++++++++++++++++++++---
 hw/virtio/virtio-pmem-pci.c        |  4 +---
 hw/virtio/virtio-pmem.c            |  4 ++--
 tests/bios-tables-test.c           | 23 +++++++++++++++++------
 8 files changed, 52 insertions(+), 17 deletions(-)

[Qemu-devel] [PULL 1/8] xio3130_downstream: typo fix

[Michael S. Tsirkin]

slt ctl/status are passed in incorrect order.
Fix this up.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
---
 hw/pci-bridge/xio3130_downstream.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
index 899b0fd6c9..182e164f74 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -43,7 +43,7 @@ static void xio3130_downstream_write_config(PCIDevice *d, uint32_t address,
 {
     uint16_t slt_ctl, slt_sta;
 
-    pcie_cap_slot_get(d, &slt_sta, &slt_ctl);
+    pcie_cap_slot_get(d, &slt_ctl, &slt_sta);
     pci_bridge_write_config(d, address, val, len);
     pcie_cap_flr_write_config(d, address, val, len);
     pcie_cap_slot_write_config(d, slt_ctl, slt_sta, address, val, len);
-- 
MST

[Qemu-devel] [PULL 2/8] pcie: consistent names for function args

[Michael S. Tsirkin]

The function declarations for pci_cap_slot_get and
pci_cap_slot_write_config call the argument "slot_ctl", but the function
definitions and all the call sites drop the 'o' and call it "slt_ctl".
Let's be consistent.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
---
 include/hw/pci/pcie.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index 34f277735c..8cf3361fc4 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -107,9 +107,9 @@ void pcie_cap_lnkctl_reset(PCIDevice *dev);
 
 void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot);
 void pcie_cap_slot_reset(PCIDevice *dev);
-void pcie_cap_slot_get(PCIDevice *dev, uint16_t *slot_ctl, uint16_t *slt_sta);
+void pcie_cap_slot_get(PCIDevice *dev, uint16_t *slt_ctl, uint16_t *slt_sta);
 void pcie_cap_slot_write_config(PCIDevice *dev,
-                                uint16_t old_slot_ctl, uint16_t old_slt_sta,
+                                uint16_t old_slt_ctl, uint16_t old_slt_sta,
                                 uint32_t addr, uint32_t val, int len);
 int pcie_cap_slot_post_load(void *opaque, int version_id);
 void pcie_cap_slot_push_attention_button(PCIDevice *dev);
-- 
MST

Re: [Qemu-devel] [PULL 0/8] virtio, pc, pci: fixes, cleanups, tests

[Peter Maydell]

On Fri, 12 Jul 2019 at 16:36, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> The following changes since commit a2a9d4adabe340617a24eb73a8b2a116d28a6b38:
>
>   Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-4.1-20190712' into staging (2019-07-12 11:06:48 +0100)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to 207efa18ac9eb7085a44cad24489d0da54bc5f65:
>
>   virtio pmem: remove transitional names (2019-07-12 10:57:27 -0400)
>
> ----------------------------------------------------------------
> virtio, pc, pci: fixes, cleanups, tests
>
> A bunch of fixes all over the place.
> ACPI tests will now run on more systems: might
> introduce new failure reports but that's for
> the best, isn't it?
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM

[Qemu-devel] [PATCH] virtio-balloon: fix QEMU 4.0 config size migration incompatibility

[Stefan Hajnoczi]

The virtio-balloon config size changed in QEMU 4.0 even for existing
machine types.  Migration from QEMU 3.1 to 4.0 can fail in some
circumstances with the following error:

  qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10 read: a1 device: 1 cmask: ff wmask: c0 w1cmask:0

This happens because the virtio-balloon config size affects the VIRTIO
Legacy I/O Memory PCI BAR size.

Introduce a qdev property called "qemu-4-0-config-size" and enable it
only for the QEMU 4.0 machine types.  This way <4.0 machine types use
the old size, 4.0 uses the larger size, and >4.0 machine types use the
appropriate size depending on enabled virtio-balloon features.

Live migration to and from old QEMUs to QEMU 4.1 works again as long as
a versioned machine type is specified (do not use just "pc"!).

Originally-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/hw/virtio/virtio-balloon.h |  2 ++
 hw/core/machine.c                  |  2 ++
 hw/virtio/virtio-balloon.c         | 28 +++++++++++++++++++++++++---
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index 1afafb12f6..5a99293a45 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -71,6 +71,8 @@ typedef struct VirtIOBalloon {
     int64_t stats_poll_interval;
     uint32_t host_features;
     PartiallyBalloonedPage *pbp;
+
+    bool qemu_4_0_config_size;
 } VirtIOBalloon;
 
 #endif
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2be19ec0cd..c4ead16010 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -34,6 +34,7 @@ GlobalProperty hw_compat_4_0[] = {
     { "virtio-vga",     "edid", "false" },
     { "virtio-gpu-pci", "edid", "false" },
     { "virtio-device", "use-started", "false" },
+    { "virtio-balloon-device", "qemu-4-0-config-size", "true" },
 };
 const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
 
@@ -49,6 +50,7 @@ GlobalProperty hw_compat_3_1[] = {
     { "usb-tablet", "serial", "42" },
     { "virtio-blk-device", "discard", "false" },
     { "virtio-blk-device", "write-zeroes", "false" },
+    { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
 };
 const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
 
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 11fad86d64..e85d1c0d5c 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -615,6 +615,22 @@ virtio_balloon_free_page_report_notify(NotifierWithReturn *n, void *data)
     return 0;
 }
 
+static size_t virtio_balloon_config_size(VirtIOBalloon *s)
+{
+    uint64_t features = s->host_features;
+
+    if (s->qemu_4_0_config_size) {
+        return sizeof(struct virtio_balloon_config);
+    }
+    if (virtio_has_feature(features, VIRTIO_BALLOON_F_PAGE_POISON)) {
+        return sizeof(struct virtio_balloon_config);
+    }
+    if (virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
+        return offsetof(struct virtio_balloon_config, poison_val);
+    }
+    return offsetof(struct virtio_balloon_config, free_page_report_cmd_id);
+}
+
 static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
 {
     VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
@@ -635,7 +651,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
     }
 
     trace_virtio_balloon_get_config(config.num_pages, config.actual);
-    memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
+    memcpy(config_data, &config, virtio_balloon_config_size(dev));
 }
 
 static int build_dimm_list(Object *obj, void *opaque)
@@ -679,7 +695,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
     uint32_t oldactual = dev->actual;
     ram_addr_t vm_ram_size = get_current_ram_size();
 
-    memcpy(&config, config_data, sizeof(struct virtio_balloon_config));
+    memcpy(&config, config_data, virtio_balloon_config_size(dev));
     dev->actual = le32_to_cpu(config.actual);
     if (dev->actual != oldactual) {
         qapi_event_send_balloon_change(vm_ram_size -
@@ -766,7 +782,7 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
     int ret;
 
     virtio_init(vdev, "virtio-balloon", VIRTIO_ID_BALLOON,
-                sizeof(struct virtio_balloon_config));
+                virtio_balloon_config_size(s));
 
     ret = qemu_add_balloon_handler(virtio_balloon_to_target,
                                    virtio_balloon_stat, s);
@@ -897,6 +913,12 @@ static Property virtio_balloon_properties[] = {
                     VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
     DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features,
                     VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
+    /* QEMU 4.0 accidentally changed the config size even when free-page-hint
+     * is disabled, resulting in QEMU 3.1 migration incompatibility.  This
+     * property retains this quirk for QEMU 4.1 machine types.
+     */
+    DEFINE_PROP_BOOL("qemu-4-0-config-size", VirtIOBalloon,
+                     qemu_4_0_config_size, false),
     DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD,
                      IOThread *),
     DEFINE_PROP_END_OF_LIST(),
-- 
2.21.0

[Qemu-devel] [PULL 3/8] virtio-balloon: fix QEMU 4.0 config size migration incompatibility

[Michael S. Tsirkin]

From: Stefan Hajnoczi <stefanha@redhat.com>

The virtio-balloon config size changed in QEMU 4.0 even for existing
machine types.  Migration from QEMU 3.1 to 4.0 can fail in some
circumstances with the following error:

  qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10 read: a1 device: 1 cmask: ff wmask: c0 w1cmask:0

This happens because the virtio-balloon config size affects the VIRTIO
Legacy I/O Memory PCI BAR size.

Introduce a qdev property called "qemu-4-0-config-size" and enable it
only for the QEMU 4.0 machine types.  This way <4.0 machine types use
the old size, 4.0 uses the larger size, and >4.0 machine types use the
appropriate size depending on enabled virtio-balloon features.

Live migration to and from old QEMUs to QEMU 4.1 works again as long as
a versioned machine type is specified (do not use just "pc"!).

Originally-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20190710141440.27635-1-stefanha@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Tested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Tested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/virtio/virtio-balloon.h |  2 ++
 hw/core/machine.c                  |  2 ++
 hw/virtio/virtio-balloon.c         | 28 +++++++++++++++++++++++++---
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index 1afafb12f6..5a99293a45 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -71,6 +71,8 @@ typedef struct VirtIOBalloon {
     int64_t stats_poll_interval;
     uint32_t host_features;
     PartiallyBalloonedPage *pbp;
+
+    bool qemu_4_0_config_size;
 } VirtIOBalloon;
 
 #endif
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2be19ec0cd..c4ead16010 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -34,6 +34,7 @@ GlobalProperty hw_compat_4_0[] = {
     { "virtio-vga",     "edid", "false" },
     { "virtio-gpu-pci", "edid", "false" },
     { "virtio-device", "use-started", "false" },
+    { "virtio-balloon-device", "qemu-4-0-config-size", "true" },
 };
 const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
 
@@ -49,6 +50,7 @@ GlobalProperty hw_compat_3_1[] = {
     { "usb-tablet", "serial", "42" },
     { "virtio-blk-device", "discard", "false" },
     { "virtio-blk-device", "write-zeroes", "false" },
+    { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
 };
 const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
 
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 11fad86d64..e85d1c0d5c 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -615,6 +615,22 @@ virtio_balloon_free_page_report_notify(NotifierWithReturn *n, void *data)
     return 0;
 }
 
+static size_t virtio_balloon_config_size(VirtIOBalloon *s)
+{
+    uint64_t features = s->host_features;
+
+    if (s->qemu_4_0_config_size) {
+        return sizeof(struct virtio_balloon_config);
+    }
+    if (virtio_has_feature(features, VIRTIO_BALLOON_F_PAGE_POISON)) {
+        return sizeof(struct virtio_balloon_config);
+    }
+    if (virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
+        return offsetof(struct virtio_balloon_config, poison_val);
+    }
+    return offsetof(struct virtio_balloon_config, free_page_report_cmd_id);
+}
+
 static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
 {
     VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
@@ -635,7 +651,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
     }
 
     trace_virtio_balloon_get_config(config.num_pages, config.actual);
-    memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
+    memcpy(config_data, &config, virtio_balloon_config_size(dev));
 }
 
 static int build_dimm_list(Object *obj, void *opaque)
@@ -679,7 +695,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
     uint32_t oldactual = dev->actual;
     ram_addr_t vm_ram_size = get_current_ram_size();
 
-    memcpy(&config, config_data, sizeof(struct virtio_balloon_config));
+    memcpy(&config, config_data, virtio_balloon_config_size(dev));
     dev->actual = le32_to_cpu(config.actual);
     if (dev->actual != oldactual) {
         qapi_event_send_balloon_change(vm_ram_size -
@@ -766,7 +782,7 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
     int ret;
 
     virtio_init(vdev, "virtio-balloon", VIRTIO_ID_BALLOON,
-                sizeof(struct virtio_balloon_config));
+                virtio_balloon_config_size(s));
 
     ret = qemu_add_balloon_handler(virtio_balloon_to_target,
                                    virtio_balloon_stat, s);
@@ -897,6 +913,12 @@ static Property virtio_balloon_properties[] = {
                     VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
     DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features,
                     VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
+    /* QEMU 4.0 accidentally changed the config size even when free-page-hint
+     * is disabled, resulting in QEMU 3.1 migration incompatibility.  This
+     * property retains this quirk for QEMU 4.1 machine types.
+     */
+    DEFINE_PROP_BOOL("qemu-4-0-config-size", VirtIOBalloon,
+                     qemu_4_0_config_size, false),
     DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD,
                      IOThread *),
     DEFINE_PROP_END_OF_LIST(),
-- 
MST

Re: [Qemu-devel] [PATCH] virtio-balloon: fix QEMU 4.0 config size migration incompatibility

[Dr. David Alan Gilbert]

* Stefan Hajnoczi (stefanha@redhat.com) wrote:
> The virtio-balloon config size changed in QEMU 4.0 even for existing
> machine types.  Migration from QEMU 3.1 to 4.0 can fail in some
> circumstances with the following error:
> 
>   qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10 read: a1 device: 1 cmask: ff wmask: c0 w1cmask:0
> 
> This happens because the virtio-balloon config size affects the VIRTIO
> Legacy I/O Memory PCI BAR size.
> 
> Introduce a qdev property called "qemu-4-0-config-size" and enable it
> only for the QEMU 4.0 machine types.  This way <4.0 machine types use
> the old size, 4.0 uses the larger size, and >4.0 machine types use the
> appropriate size depending on enabled virtio-balloon features.
> 
> Live migration to and from old QEMUs to QEMU 4.1 works again as long as
> a versioned machine type is specified (do not use just "pc"!).
> 
> Originally-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Tested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
   Tried 3.1->head and back
   Tried head<->head (default options)
     in both cases I tried ballooning the guest up and down

Dave


> ---
>  include/hw/virtio/virtio-balloon.h |  2 ++
>  hw/core/machine.c                  |  2 ++
>  hw/virtio/virtio-balloon.c         | 28 +++++++++++++++++++++++++---
>  3 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
> index 1afafb12f6..5a99293a45 100644
> --- a/include/hw/virtio/virtio-balloon.h
> +++ b/include/hw/virtio/virtio-balloon.h
> @@ -71,6 +71,8 @@ typedef struct VirtIOBalloon {
>      int64_t stats_poll_interval;
>      uint32_t host_features;
>      PartiallyBalloonedPage *pbp;
> +
> +    bool qemu_4_0_config_size;
>  } VirtIOBalloon;
>  
>  #endif
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 2be19ec0cd..c4ead16010 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -34,6 +34,7 @@ GlobalProperty hw_compat_4_0[] = {
>      { "virtio-vga",     "edid", "false" },
>      { "virtio-gpu-pci", "edid", "false" },
>      { "virtio-device", "use-started", "false" },
> +    { "virtio-balloon-device", "qemu-4-0-config-size", "true" },
>  };
>  const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
>  
> @@ -49,6 +50,7 @@ GlobalProperty hw_compat_3_1[] = {
>      { "usb-tablet", "serial", "42" },
>      { "virtio-blk-device", "discard", "false" },
>      { "virtio-blk-device", "write-zeroes", "false" },
> +    { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
>  };
>  const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
>  
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 11fad86d64..e85d1c0d5c 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -615,6 +615,22 @@ virtio_balloon_free_page_report_notify(NotifierWithReturn *n, void *data)
>      return 0;
>  }
>  
> +static size_t virtio_balloon_config_size(VirtIOBalloon *s)
> +{
> +    uint64_t features = s->host_features;
> +
> +    if (s->qemu_4_0_config_size) {
> +        return sizeof(struct virtio_balloon_config);
> +    }
> +    if (virtio_has_feature(features, VIRTIO_BALLOON_F_PAGE_POISON)) {
> +        return sizeof(struct virtio_balloon_config);
> +    }
> +    if (virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
> +        return offsetof(struct virtio_balloon_config, poison_val);
> +    }
> +    return offsetof(struct virtio_balloon_config, free_page_report_cmd_id);
> +}
> +
>  static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
>  {
>      VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
> @@ -635,7 +651,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
>      }
>  
>      trace_virtio_balloon_get_config(config.num_pages, config.actual);
> -    memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
> +    memcpy(config_data, &config, virtio_balloon_config_size(dev));
>  }
>  
>  static int build_dimm_list(Object *obj, void *opaque)
> @@ -679,7 +695,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
>      uint32_t oldactual = dev->actual;
>      ram_addr_t vm_ram_size = get_current_ram_size();
>  
> -    memcpy(&config, config_data, sizeof(struct virtio_balloon_config));
> +    memcpy(&config, config_data, virtio_balloon_config_size(dev));
>      dev->actual = le32_to_cpu(config.actual);
>      if (dev->actual != oldactual) {
>          qapi_event_send_balloon_change(vm_ram_size -
> @@ -766,7 +782,7 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
>      int ret;
>  
>      virtio_init(vdev, "virtio-balloon", VIRTIO_ID_BALLOON,
> -                sizeof(struct virtio_balloon_config));
> +                virtio_balloon_config_size(s));
>  
>      ret = qemu_add_balloon_handler(virtio_balloon_to_target,
>                                     virtio_balloon_stat, s);
> @@ -897,6 +913,12 @@ static Property virtio_balloon_properties[] = {
>                      VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
>      DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features,
>                      VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
> +    /* QEMU 4.0 accidentally changed the config size even when free-page-hint
> +     * is disabled, resulting in QEMU 3.1 migration incompatibility.  This
> +     * property retains this quirk for QEMU 4.1 machine types.
> +     */
> +    DEFINE_PROP_BOOL("qemu-4-0-config-size", VirtIOBalloon,
> +                     qemu_4_0_config_size, false),
>      DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD,
>                       IOThread *),
>      DEFINE_PROP_END_OF_LIST(),
> -- 
> 2.21.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] virtio-balloon: fix QEMU 4.0 config size migration incompatibility

[Wolfgang Bumiller]

On Wed, Jul 10, 2019 at 04:14:40PM +0200, Stefan Hajnoczi wrote:
> The virtio-balloon config size changed in QEMU 4.0 even for existing
> machine types.  Migration from QEMU 3.1 to 4.0 can fail in some
> circumstances with the following error:
> 
>   qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10 read: a1 device: 1 cmask: ff wmask: c0 w1cmask:0
> 
> This happens because the virtio-balloon config size affects the VIRTIO
> Legacy I/O Memory PCI BAR size.
> 
> Introduce a qdev property called "qemu-4-0-config-size" and enable it
> only for the QEMU 4.0 machine types.  This way <4.0 machine types use
> the old size, 4.0 uses the larger size, and >4.0 machine types use the
> appropriate size depending on enabled virtio-balloon features.
> 
> Live migration to and from old QEMUs to QEMU 4.1 works again as long as
> a versioned machine type is specified (do not use just "pc"!).
> 
> Originally-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Tested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

Works with my otherwise failing VM from 3.0.1 -> patched-4.0.
Somehow I missed the `DEFINE_PROP_*` macros, sorry, and thanks for
fixing this up.

> ---
>  include/hw/virtio/virtio-balloon.h |  2 ++
>  hw/core/machine.c                  |  2 ++
>  hw/virtio/virtio-balloon.c         | 28 +++++++++++++++++++++++++---
>  3 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
> index 1afafb12f6..5a99293a45 100644
> --- a/include/hw/virtio/virtio-balloon.h
> +++ b/include/hw/virtio/virtio-balloon.h
> @@ -71,6 +71,8 @@ typedef struct VirtIOBalloon {
>      int64_t stats_poll_interval;
>      uint32_t host_features;
>      PartiallyBalloonedPage *pbp;
> +
> +    bool qemu_4_0_config_size;
>  } VirtIOBalloon;
>  
>  #endif
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 2be19ec0cd..c4ead16010 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -34,6 +34,7 @@ GlobalProperty hw_compat_4_0[] = {
>      { "virtio-vga",     "edid", "false" },
>      { "virtio-gpu-pci", "edid", "false" },
>      { "virtio-device", "use-started", "false" },
> +    { "virtio-balloon-device", "qemu-4-0-config-size", "true" },
>  };
>  const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
>  
> @@ -49,6 +50,7 @@ GlobalProperty hw_compat_3_1[] = {
>      { "usb-tablet", "serial", "42" },
>      { "virtio-blk-device", "discard", "false" },
>      { "virtio-blk-device", "write-zeroes", "false" },
> +    { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
>  };
>  const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
>  
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 11fad86d64..e85d1c0d5c 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -615,6 +615,22 @@ virtio_balloon_free_page_report_notify(NotifierWithReturn *n, void *data)
>      return 0;
>  }
>  
> +static size_t virtio_balloon_config_size(VirtIOBalloon *s)
> +{
> +    uint64_t features = s->host_features;
> +
> +    if (s->qemu_4_0_config_size) {
> +        return sizeof(struct virtio_balloon_config);
> +    }
> +    if (virtio_has_feature(features, VIRTIO_BALLOON_F_PAGE_POISON)) {
> +        return sizeof(struct virtio_balloon_config);
> +    }
> +    if (virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
> +        return offsetof(struct virtio_balloon_config, poison_val);
> +    }
> +    return offsetof(struct virtio_balloon_config, free_page_report_cmd_id);
> +}
> +
>  static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
>  {
>      VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
> @@ -635,7 +651,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
>      }
>  
>      trace_virtio_balloon_get_config(config.num_pages, config.actual);
> -    memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
> +    memcpy(config_data, &config, virtio_balloon_config_size(dev));
>  }
>  
>  static int build_dimm_list(Object *obj, void *opaque)
> @@ -679,7 +695,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
>      uint32_t oldactual = dev->actual;
>      ram_addr_t vm_ram_size = get_current_ram_size();
>  
> -    memcpy(&config, config_data, sizeof(struct virtio_balloon_config));
> +    memcpy(&config, config_data, virtio_balloon_config_size(dev));
>      dev->actual = le32_to_cpu(config.actual);
>      if (dev->actual != oldactual) {
>          qapi_event_send_balloon_change(vm_ram_size -
> @@ -766,7 +782,7 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
>      int ret;
>  
>      virtio_init(vdev, "virtio-balloon", VIRTIO_ID_BALLOON,
> -                sizeof(struct virtio_balloon_config));
> +                virtio_balloon_config_size(s));
>  
>      ret = qemu_add_balloon_handler(virtio_balloon_to_target,
>                                     virtio_balloon_stat, s);
> @@ -897,6 +913,12 @@ static Property virtio_balloon_properties[] = {
>                      VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
>      DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features,
>                      VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
> +    /* QEMU 4.0 accidentally changed the config size even when free-page-hint
> +     * is disabled, resulting in QEMU 3.1 migration incompatibility.  This
> +     * property retains this quirk for QEMU 4.1 machine types.
> +     */
> +    DEFINE_PROP_BOOL("qemu-4-0-config-size", VirtIOBalloon,
> +                     qemu_4_0_config_size, false),
>      DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD,
>                       IOThread *),
>      DEFINE_PROP_END_OF_LIST(),
> -- 
> 2.21.0
> 
> 

Re: [Qemu-devel] [PATCH] virtio-balloon: fix QEMU 4.0 config size migration incompatibility

[Dr. David Alan Gilbert]

* Wolfgang Bumiller (w.bumiller@proxmox.com) wrote:
> On Wed, Jul 10, 2019 at 04:14:40PM +0200, Stefan Hajnoczi wrote:
> > The virtio-balloon config size changed in QEMU 4.0 even for existing
> > machine types.  Migration from QEMU 3.1 to 4.0 can fail in some
> > circumstances with the following error:
> > 
> >   qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10 read: a1 device: 1 cmask: ff wmask: c0 w1cmask:0
> > 
> > This happens because the virtio-balloon config size affects the VIRTIO
> > Legacy I/O Memory PCI BAR size.
> > 
> > Introduce a qdev property called "qemu-4-0-config-size" and enable it
> > only for the QEMU 4.0 machine types.  This way <4.0 machine types use
> > the old size, 4.0 uses the larger size, and >4.0 machine types use the
> > appropriate size depending on enabled virtio-balloon features.
> > 
> > Live migration to and from old QEMUs to QEMU 4.1 works again as long as
> > a versioned machine type is specified (do not use just "pc"!).
> > 
> > Originally-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Tested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> 
> Works with my otherwise failing VM from 3.0.1 -> patched-4.0.
> Somehow I missed the `DEFINE_PROP_*` macros, sorry, and thanks for
> fixing this up.

Thanks again for spotting it!

Dave

> > ---
> >  include/hw/virtio/virtio-balloon.h |  2 ++
> >  hw/core/machine.c                  |  2 ++
> >  hw/virtio/virtio-balloon.c         | 28 +++++++++++++++++++++++++---
> >  3 files changed, 29 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
> > index 1afafb12f6..5a99293a45 100644
> > --- a/include/hw/virtio/virtio-balloon.h
> > +++ b/include/hw/virtio/virtio-balloon.h
> > @@ -71,6 +71,8 @@ typedef struct VirtIOBalloon {
> >      int64_t stats_poll_interval;
> >      uint32_t host_features;
> >      PartiallyBalloonedPage *pbp;
> > +
> > +    bool qemu_4_0_config_size;
> >  } VirtIOBalloon;
> >  
> >  #endif
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 2be19ec0cd..c4ead16010 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -34,6 +34,7 @@ GlobalProperty hw_compat_4_0[] = {
> >      { "virtio-vga",     "edid", "false" },
> >      { "virtio-gpu-pci", "edid", "false" },
> >      { "virtio-device", "use-started", "false" },
> > +    { "virtio-balloon-device", "qemu-4-0-config-size", "true" },
> >  };
> >  const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
> >  
> > @@ -49,6 +50,7 @@ GlobalProperty hw_compat_3_1[] = {
> >      { "usb-tablet", "serial", "42" },
> >      { "virtio-blk-device", "discard", "false" },
> >      { "virtio-blk-device", "write-zeroes", "false" },
> > +    { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
> >  };
> >  const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
> >  
> > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> > index 11fad86d64..e85d1c0d5c 100644
> > --- a/hw/virtio/virtio-balloon.c
> > +++ b/hw/virtio/virtio-balloon.c
> > @@ -615,6 +615,22 @@ virtio_balloon_free_page_report_notify(NotifierWithReturn *n, void *data)
> >      return 0;
> >  }
> >  
> > +static size_t virtio_balloon_config_size(VirtIOBalloon *s)
> > +{
> > +    uint64_t features = s->host_features;
> > +
> > +    if (s->qemu_4_0_config_size) {
> > +        return sizeof(struct virtio_balloon_config);
> > +    }
> > +    if (virtio_has_feature(features, VIRTIO_BALLOON_F_PAGE_POISON)) {
> > +        return sizeof(struct virtio_balloon_config);
> > +    }
> > +    if (virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
> > +        return offsetof(struct virtio_balloon_config, poison_val);
> > +    }
> > +    return offsetof(struct virtio_balloon_config, free_page_report_cmd_id);
> > +}
> > +
> >  static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
> >  {
> >      VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
> > @@ -635,7 +651,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
> >      }
> >  
> >      trace_virtio_balloon_get_config(config.num_pages, config.actual);
> > -    memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
> > +    memcpy(config_data, &config, virtio_balloon_config_size(dev));
> >  }
> >  
> >  static int build_dimm_list(Object *obj, void *opaque)
> > @@ -679,7 +695,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
> >      uint32_t oldactual = dev->actual;
> >      ram_addr_t vm_ram_size = get_current_ram_size();
> >  
> > -    memcpy(&config, config_data, sizeof(struct virtio_balloon_config));
> > +    memcpy(&config, config_data, virtio_balloon_config_size(dev));
> >      dev->actual = le32_to_cpu(config.actual);
> >      if (dev->actual != oldactual) {
> >          qapi_event_send_balloon_change(vm_ram_size -
> > @@ -766,7 +782,7 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
> >      int ret;
> >  
> >      virtio_init(vdev, "virtio-balloon", VIRTIO_ID_BALLOON,
> > -                sizeof(struct virtio_balloon_config));
> > +                virtio_balloon_config_size(s));
> >  
> >      ret = qemu_add_balloon_handler(virtio_balloon_to_target,
> >                                     virtio_balloon_stat, s);
> > @@ -897,6 +913,12 @@ static Property virtio_balloon_properties[] = {
> >                      VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
> >      DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features,
> >                      VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
> > +    /* QEMU 4.0 accidentally changed the config size even when free-page-hint
> > +     * is disabled, resulting in QEMU 3.1 migration incompatibility.  This
> > +     * property retains this quirk for QEMU 4.1 machine types.
> > +     */
> > +    DEFINE_PROP_BOOL("qemu-4-0-config-size", VirtIOBalloon,
> > +                     qemu_4_0_config_size, false),
> >      DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD,
> >                       IOThread *),
> >      DEFINE_PROP_END_OF_LIST(),
> > -- 
> > 2.21.0
> > 
> > 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH v3 0/2] tests: acpi: improve tests reproducability

[Igor Mammedov]

ChangeLog since v1:
  * fixup commit message in 2/2
  * pickup Reviewed-bys and repost as v3

series should help to make tests more reproducable and not depend
on IASL being installed. IASL will be required only is user needs
to get textual diff of mismatching files.


Igor Mammedov (2):
  tests: acpi: do not require IASL for dumping AML blobs
  tests: acpi: do not skip tests when IASL is not installed

 tests/bios-tables-test.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

-- 
2.18.1

[Qemu-devel] [PATCH v3 1/2] tests: acpi: do not require IASL for dumping AML blobs

[Igor Mammedov]

IASL isn't needed when dumping ACPI tables from guest for
rebuild purposes. So move this part out from IASL branch.

Makes rebuild-expected-aml.sh work without IASL installed
on host.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
 tests/bios-tables-test.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index d863233fe9..13bd166b81 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -597,12 +597,10 @@ static void test_acpi_one(const char *params, test_data *data)
     test_acpi_rxsdt_table(data);
     test_acpi_fadt_table(data);
 
-    if (iasl) {
-        if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
-            dump_aml_files(data, true);
-        } else {
-            test_acpi_asl(data);
-        }
+    if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
+        dump_aml_files(data, true);
+    } else if (iasl) {
+        test_acpi_asl(data);
     }
 
     /*
-- 
2.18.1

[Qemu-devel] [PULL 4/8] tests: acpi: do not require IASL for dumping AML blobs

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

IASL isn't needed when dumping ACPI tables from guest for
rebuild purposes. So move this part out from IASL branch.

Makes rebuild-expected-aml.sh work without IASL installed
on host.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-Id: <20190708092410.11167-2-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/bios-tables-test.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index d863233fe9..13bd166b81 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -597,12 +597,10 @@ static void test_acpi_one(const char *params, test_data *data)
     test_acpi_rxsdt_table(data);
     test_acpi_fadt_table(data);
 
-    if (iasl) {
-        if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
-            dump_aml_files(data, true);
-        } else {
-            test_acpi_asl(data);
-        }
+    if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
+        dump_aml_files(data, true);
+    } else if (iasl) {
+        test_acpi_asl(data);
     }
 
     /*
-- 
MST

[Qemu-devel] [PATCH v3 2/2] tests: acpi: do not skip tests when IASL is not installed

[Igor Mammedov]

tests do binary comparision so we can check tables without
IASL. Move IASL condition right before decompilation step
and skip it if IASL is not installed.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2:
 - fix typo in commit message
     Eric Auger <eric.auger@redhat.com>


 tests/bios-tables-test.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 13bd166b81..a356ac3489 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -389,6 +389,14 @@ static void test_acpi_asl(test_data *data)
         all_tables_match = all_tables_match &&
             test_acpi_find_diff_allowed(exp_sdt);
 
+        /*
+         *  don't try to decompile if IASL isn't present, in this case user
+         * will just 'get binary file mismatch' warnings and test failure
+         */
+        if (!iasl) {
+            continue;
+        }
+
         err = load_asl(data->tables, sdt);
         asl = normalize_asl(sdt->asl);
 
@@ -431,6 +439,11 @@ static void test_acpi_asl(test_data *data)
         g_string_free(asl, true);
         g_string_free(exp_asl, true);
     }
+    if (!iasl && !all_tables_match) {
+        fprintf(stderr, "to see ASL diff between mismatched files install IASL,"
+                " rebuild QEMU from scratch and re-run tests with V=1"
+                " environment variable set");
+    }
     g_assert(all_tables_match);
 
     free_test_data(&exp_data);
@@ -599,7 +612,7 @@ static void test_acpi_one(const char *params, test_data *data)
 
     if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
         dump_aml_files(data, true);
-    } else if (iasl) {
+    } else {
         test_acpi_asl(data);
     }
 
-- 
2.18.1

[Qemu-devel] [PULL 5/8] tests: acpi: do not skip tests when IASL is not installed

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

tests do binary comparision so we can check tables without
IASL. Move IASL condition right before decompilation step
and skip it if IASL is not installed.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190708092410.11167-3-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/bios-tables-test.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 13bd166b81..a356ac3489 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -389,6 +389,14 @@ static void test_acpi_asl(test_data *data)
         all_tables_match = all_tables_match &&
             test_acpi_find_diff_allowed(exp_sdt);
 
+        /*
+         *  don't try to decompile if IASL isn't present, in this case user
+         * will just 'get binary file mismatch' warnings and test failure
+         */
+        if (!iasl) {
+            continue;
+        }
+
         err = load_asl(data->tables, sdt);
         asl = normalize_asl(sdt->asl);
 
@@ -431,6 +439,11 @@ static void test_acpi_asl(test_data *data)
         g_string_free(asl, true);
         g_string_free(exp_asl, true);
     }
+    if (!iasl && !all_tables_match) {
+        fprintf(stderr, "to see ASL diff between mismatched files install IASL,"
+                " rebuild QEMU from scratch and re-run tests with V=1"
+                " environment variable set");
+    }
     g_assert(all_tables_match);
 
     free_test_data(&exp_data);
@@ -599,7 +612,7 @@ static void test_acpi_one(const char *params, test_data *data)
 
     if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
         dump_aml_files(data, true);
-    } else if (iasl) {
+    } else {
         test_acpi_asl(data);
     }
 
-- 
MST