[Qemu-devel] [PATCH v3] ivshmem: Fix 64 bit memory bar configuration

[Qemu-devel] [PATCH v3] ivshmem: Fix 64 bit memory bar configuration

[Zhuangyanying]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="n", Size: 2190 bytes --]

From: Zhuang Yanying <ann.zhuangyanying@huawei.com>

    Device ivshmem property use64=0 is designed to make the device
    expose a 32 bit shared memory BAR instead of 64 bit one.  The
    default is a 64 bit BAR, except pc-1.2 and older retain a 32 bit
    BAR.  A 32 bit BAR can support only up to 1 GiB of shared memory.

    This worked as designed until commit 5400c02 accidentally flipped
    its sense: since then, we misinterpret use64=0 as use64=1 and vice
    versa.  Worse, the default got flipped as well.  Devices
    ivshmem-plain and ivshmem-doorbell are not affected.

    Fix by restoring the test of IVShmemState member not_legacy_32bit
    that got messed up in commit 5400c02.  Also update its
    initialization for devices ivhsmem-plain and ivshmem-doorbell.
    Without that, they'd regress to 32 bit BARs.

    Cc: qemu-stable@nongnu.org
    Signed-off-by: Zhuang Yanying <ann.zhuangyanying@huawei.com>
    Reviewed-by: Gonglei <arei.gonglei@huawei.com>
    Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

---
 hw/misc/ivshmem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 230e51b..abeaf3d 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -858,7 +858,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
     pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY,
                      &s->ivshmem_mmio);
 
-    if (!s->not_legacy_32bit) {
+    if (s->not_legacy_32bit) {
         attr |= PCI_BASE_ADDRESS_MEM_TYPE_64;
     }
 
@@ -1045,6 +1045,7 @@ static void ivshmem_plain_init(Object *obj)
                              ivshmem_check_memdev_is_busy,
                              OBJ_PROP_LINK_UNREF_ON_RELEASE,
                              &error_abort);
+    s->not_legacy_32bit = 1;
 }
 
 static void ivshmem_plain_realize(PCIDevice *dev, Error **errp)
@@ -1116,6 +1117,7 @@ static void ivshmem_doorbell_init(Object *obj)
 
     s->features |= (1 << IVSHMEM_MSI);
     s->legacy_size = SIZE_MAX;  /* whatever the server sends */
+    s->not_legacy_32bit = 1;
 }
 
 static void ivshmem_doorbell_realize(PCIDevice *dev, Error **errp)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3] ivshmem: Fix 64 bit memory bar configuration

[Markus Armbruster]

Zhuangyanying <ann.zhuangyanying@huawei.com> writes:

> From: Zhuang Yanying <ann.zhuangyanying@huawei.com>
>
>     Device ivshmem property use64=0 is designed to make the device
>     expose a 32 bit shared memory BAR instead of 64 bit one.  The
>     default is a 64 bit BAR, except pc-1.2 and older retain a 32 bit
>     BAR.  A 32 bit BAR can support only up to 1 GiB of shared memory.
>
>     This worked as designed until commit 5400c02 accidentally flipped
>     its sense: since then, we misinterpret use64=0 as use64=1 and vice
>     versa.  Worse, the default got flipped as well.  Devices
>     ivshmem-plain and ivshmem-doorbell are not affected.
>
>     Fix by restoring the test of IVShmemState member not_legacy_32bit
>     that got messed up in commit 5400c02.  Also update its
>     initialization for devices ivhsmem-plain and ivshmem-doorbell.
>     Without that, they'd regress to 32 bit BARs.
>
>     Cc: qemu-stable@nongnu.org
>     Signed-off-by: Zhuang Yanying <ann.zhuangyanying@huawei.com>
>     Reviewed-by: Gonglei <arei.gonglei@huawei.com>
>     Reviewed-by: Marc-Andr. Lureau <marcandre.lureau@redhat.com>

Commit message should not be indented, and Marc-André's name got
mangled.  Both could be touched up on commit.

Thanks again for cleaning up my mess.

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v3] ivshmem: Fix 64 bit memory bar configuration

[Michael S. Tsirkin]

On Thu, Nov 17, 2016 at 08:31:03PM +0800, Zhuangyanying wrote:
> From: Zhuang Yanying <ann.zhuangyanying@huawei.com>
> 
>     Device ivshmem property use64=0 is designed to make the device
>     expose a 32 bit shared memory BAR instead of 64 bit one.  The
>     default is a 64 bit BAR, except pc-1.2 and older retain a 32 bit
>     BAR.  A 32 bit BAR can support only up to 1 GiB of shared memory.
> 
>     This worked as designed until commit 5400c02 accidentally flipped
>     its sense: since then, we misinterpret use64=0 as use64=1 and vice
>     versa.  Worse, the default got flipped as well.  Devices
>     ivshmem-plain and ivshmem-doorbell are not affected.
> 
>     Fix by restoring the test of IVShmemState member not_legacy_32bit
>     that got messed up in commit 5400c02.  Also update its
>     initialization for devices ivhsmem-plain and ivshmem-doorbell.
>     Without that, they'd regress to 32 bit BARs.
> 
>     Cc: qemu-stable@nongnu.org
>     Signed-off-by: Zhuang Yanying <ann.zhuangyanying@huawei.com>
>     Reviewed-by: Gonglei <arei.gonglei@huawei.com>
>     Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

this is malformed, pls shift it left to start at beginning of the line.

> ---
>  hw/misc/ivshmem.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index 230e51b..abeaf3d 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -858,7 +858,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
>      pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY,
>                       &s->ivshmem_mmio);
>  
> -    if (!s->not_legacy_32bit) {
> +    if (s->not_legacy_32bit) {
>          attr |= PCI_BASE_ADDRESS_MEM_TYPE_64;
>      }
>  
> @@ -1045,6 +1045,7 @@ static void ivshmem_plain_init(Object *obj)
>                               ivshmem_check_memdev_is_busy,
>                               OBJ_PROP_LINK_UNREF_ON_RELEASE,
>                               &error_abort);
> +    s->not_legacy_32bit = 1;
>  }
>  
>  static void ivshmem_plain_realize(PCIDevice *dev, Error **errp)
> @@ -1116,6 +1117,7 @@ static void ivshmem_doorbell_init(Object *obj)
>  
>      s->features |= (1 << IVSHMEM_MSI);
>      s->legacy_size = SIZE_MAX;  /* whatever the server sends */
> +    s->not_legacy_32bit = 1;
>  }
>  
>  static void ivshmem_doorbell_realize(PCIDevice *dev, Error **errp)
> -- 
> 1.8.3.1
> 

Re: [Qemu-devel] [PATCH v3] ivshmem: Fix 64 bit memory bar configuration

[Michael S. Tsirkin]

On Thu, Nov 17, 2016 at 08:31:03PM +0800, Zhuangyanying wrote:
> From: Zhuang Yanying <ann.zhuangyanying@huawei.com>
> 
>     Device ivshmem property use64=0 is designed to make the device
>     expose a 32 bit shared memory BAR instead of 64 bit one.  The
>     default is a 64 bit BAR, except pc-1.2 and older retain a 32 bit
>     BAR.  A 32 bit BAR can support only up to 1 GiB of shared memory.
> 
>     This worked as designed until commit 5400c02 accidentally flipped
>     its sense: since then, we misinterpret use64=0 as use64=1 and vice
>     versa.  Worse, the default got flipped as well.  Devices
>     ivshmem-plain and ivshmem-doorbell are not affected.
> 
>     Fix by restoring the test of IVShmemState member not_legacy_32bit
>     that got messed up in commit 5400c02.  Also update its
>     initialization for devices ivhsmem-plain and ivshmem-doorbell.
>     Without that, they'd regress to 32 bit BARs.
> 
>     Cc: qemu-stable@nongnu.org
>     Signed-off-by: Zhuang Yanying <ann.zhuangyanying@huawei.com>
>     Reviewed-by: Gonglei <arei.gonglei@huawei.com>
>     Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

This is UTF-8 encoded, but your mail header says
Content-Transfer-Encoding: 8bit
so git am fails to apply this.



> ---
>  hw/misc/ivshmem.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index 230e51b..abeaf3d 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -858,7 +858,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
>      pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY,
>                       &s->ivshmem_mmio);
>  
> -    if (!s->not_legacy_32bit) {
> +    if (s->not_legacy_32bit) {
>          attr |= PCI_BASE_ADDRESS_MEM_TYPE_64;
>      }
>  
> @@ -1045,6 +1045,7 @@ static void ivshmem_plain_init(Object *obj)
>                               ivshmem_check_memdev_is_busy,
>                               OBJ_PROP_LINK_UNREF_ON_RELEASE,
>                               &error_abort);
> +    s->not_legacy_32bit = 1;
>  }
>  
>  static void ivshmem_plain_realize(PCIDevice *dev, Error **errp)
> @@ -1116,6 +1117,7 @@ static void ivshmem_doorbell_init(Object *obj)
>  
>      s->features |= (1 << IVSHMEM_MSI);
>      s->legacy_size = SIZE_MAX;  /* whatever the server sends */
> +    s->not_legacy_32bit = 1;
>  }
>  
>  static void ivshmem_doorbell_realize(PCIDevice *dev, Error **errp)
> -- 
> 1.8.3.1
> 

Re: [Qemu-devel] [PATCH v3] ivshmem: Fix 64 bit memory bar configuration

[Michael S. Tsirkin]

On Fri, Nov 18, 2016 at 05:27:41PM +0200, Michael S. Tsirkin wrote:
> On Thu, Nov 17, 2016 at 08:31:03PM +0800, Zhuangyanying wrote:
> > From: Zhuang Yanying <ann.zhuangyanying@huawei.com>
> > 
> >     Device ivshmem property use64=0 is designed to make the device
> >     expose a 32 bit shared memory BAR instead of 64 bit one.  The
> >     default is a 64 bit BAR, except pc-1.2 and older retain a 32 bit
> >     BAR.  A 32 bit BAR can support only up to 1 GiB of shared memory.
> > 
> >     This worked as designed until commit 5400c02 accidentally flipped
> >     its sense: since then, we misinterpret use64=0 as use64=1 and vice
> >     versa.  Worse, the default got flipped as well.  Devices
> >     ivshmem-plain and ivshmem-doorbell are not affected.
> > 
> >     Fix by restoring the test of IVShmemState member not_legacy_32bit
> >     that got messed up in commit 5400c02.  Also update its
> >     initialization for devices ivhsmem-plain and ivshmem-doorbell.
> >     Without that, they'd regress to 32 bit BARs.
> > 
> >     Cc: qemu-stable@nongnu.org
> >     Signed-off-by: Zhuang Yanying <ann.zhuangyanying@huawei.com>
> >     Reviewed-by: Gonglei <arei.gonglei@huawei.com>
> >     Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> This is UTF-8 encoded, but your mail header says
> Content-Transfer-Encoding: 8bit
> so git am fails to apply this.

In fact this is the problem:
Content-Type: text/plain; charset="n"

This is not a valid charset. I fixed it to
Content-Type: text/plain; charset="utf-8"

and it applies.

Pls take care in the future.

> 
> 
> > ---
> >  hw/misc/ivshmem.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> > index 230e51b..abeaf3d 100644
> > --- a/hw/misc/ivshmem.c
> > +++ b/hw/misc/ivshmem.c
> > @@ -858,7 +858,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
> >      pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY,
> >                       &s->ivshmem_mmio);
> >  
> > -    if (!s->not_legacy_32bit) {
> > +    if (s->not_legacy_32bit) {
> >          attr |= PCI_BASE_ADDRESS_MEM_TYPE_64;
> >      }
> >  
> > @@ -1045,6 +1045,7 @@ static void ivshmem_plain_init(Object *obj)
> >                               ivshmem_check_memdev_is_busy,
> >                               OBJ_PROP_LINK_UNREF_ON_RELEASE,
> >                               &error_abort);
> > +    s->not_legacy_32bit = 1;
> >  }
> >  
> >  static void ivshmem_plain_realize(PCIDevice *dev, Error **errp)
> > @@ -1116,6 +1117,7 @@ static void ivshmem_doorbell_init(Object *obj)
> >  
> >      s->features |= (1 << IVSHMEM_MSI);
> >      s->legacy_size = SIZE_MAX;  /* whatever the server sends */
> > +    s->not_legacy_32bit = 1;
> >  }
> >  
> >  static void ivshmem_doorbell_realize(PCIDevice *dev, Error **errp)
> > -- 
> > 1.8.3.1
> > 

Re: [Qemu-devel] [PATCH v3] ivshmem: Fix 64 bit memory bar configuration

[Paolo Bonzini]

On 18/11/2016 16:27, Michael S. Tsirkin wrote:
> On Thu, Nov 17, 2016 at 08:31:03PM +0800, Zhuangyanying wrote:
>> From: Zhuang Yanying <ann.zhuangyanying@huawei.com>
>>
>>     Device ivshmem property use64=0 is designed to make the device
>>     expose a 32 bit shared memory BAR instead of 64 bit one.  The
>>     default is a 64 bit BAR, except pc-1.2 and older retain a 32 bit
>>     BAR.  A 32 bit BAR can support only up to 1 GiB of shared memory.
>>
>>     This worked as designed until commit 5400c02 accidentally flipped
>>     its sense: since then, we misinterpret use64=0 as use64=1 and vice
>>     versa.  Worse, the default got flipped as well.  Devices
>>     ivshmem-plain and ivshmem-doorbell are not affected.
>>
>>     Fix by restoring the test of IVShmemState member not_legacy_32bit
>>     that got messed up in commit 5400c02.  Also update its
>>     initialization for devices ivhsmem-plain and ivshmem-doorbell.
>>     Without that, they'd regress to 32 bit BARs.
>>
>>     Cc: qemu-stable@nongnu.org
>>     Signed-off-by: Zhuang Yanying <ann.zhuangyanying@huawei.com>
>>     Reviewed-by: Gonglei <arei.gonglei@huawei.com>
>>     Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> This is UTF-8 encoded, but your mail header says
> Content-Transfer-Encoding: 8bit
> so git am fails to apply this.

The problematic header is more likely

Content-Type: text/plain; charset=iso-8859-1

Paolo

> 
> 
>> ---
>>  hw/misc/ivshmem.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
>> index 230e51b..abeaf3d 100644
>> --- a/hw/misc/ivshmem.c
>> +++ b/hw/misc/ivshmem.c
>> @@ -858,7 +858,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
>>      pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY,
>>                       &s->ivshmem_mmio);
>>  
>> -    if (!s->not_legacy_32bit) {
>> +    if (s->not_legacy_32bit) {
>>          attr |= PCI_BASE_ADDRESS_MEM_TYPE_64;
>>      }
>>  
>> @@ -1045,6 +1045,7 @@ static void ivshmem_plain_init(Object *obj)
>>                               ivshmem_check_memdev_is_busy,
>>                               OBJ_PROP_LINK_UNREF_ON_RELEASE,
>>                               &error_abort);
>> +    s->not_legacy_32bit = 1;
>>  }
>>  
>>  static void ivshmem_plain_realize(PCIDevice *dev, Error **errp)
>> @@ -1116,6 +1117,7 @@ static void ivshmem_doorbell_init(Object *obj)
>>  
>>      s->features |= (1 << IVSHMEM_MSI);
>>      s->legacy_size = SIZE_MAX;  /* whatever the server sends */
>> +    s->not_legacy_32bit = 1;
>>  }
>>  
>>  static void ivshmem_doorbell_realize(PCIDevice *dev, Error **errp)
>> -- 
>> 1.8.3.1
>>