Re: [Qemu-devel] [PATCH] hw/misc/ivshmem:fix misconfig of not_legacy_32bit

[Markus Armbruster]

Zhuangyanying <address@hidden> writes:

> From: ZhuangYanying <address@hidden>
>
> After "ivshmem: Split ivshmem-plain, ivshmem-doorbell off ivshmem", 
> ivshmem_64bit renamed to not_legacy_32bit, and changed the implementation of 
> this property.
> Then use64 = not_legacy_32bit = 1, then PCI attribute configuration ~ 
> PCI_BASE_ADDRESS_MEM_TYPE_64 (default for ivshmem), the actual use is the 
> legacy model, can not support greater than or equal 1G mapping, which is the 
> opposite of configuration requirements.

Please limit commit message line length to about 70 characters.

>
> Signed-off-by: address@hidden
> ---
> Recently, I tested ivshmem, found that use64, that is not_legacy_32bit 
> implementation is odd, or even the opposite.
> Previous use64 = ivshmem_64bit = 1, then attr |= 
> PCI_BASE_ADDRESS_MEM_TYPE_64, then ivshmem support 1G and above packaged into 
> bar2, presented to the virtual machine.
> But after "ivshmem: Split ivshmem-plain, ivshmem-doorbell off ivshmem", 
> PCI_BASE_ADDRESS_MEM_TYPE_64 is configured while not_legacy_32bit = 0, that 
> is the legacy model.
> ---
>  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..b71acf6 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;
>      }
>  

This hunk fixes a regression in commit 5400c02, v2.6.0.  It should go
into 2.7.0.

> @@ -1033,6 +1033,7 @@ static const VMStateDescription ivshmem_plain_vmsd = {
>  
>  static Property ivshmem_plain_properties[] = {
>      DEFINE_PROP_ON_OFF_AUTO("master", IVShmemState, master, ON_OFF_AUTO_OFF),
> +    DEFINE_PROP_UINT32("use64", IVShmemState, not_legacy_32bit, 1),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -1107,6 +1108,7 @@ static Property ivshmem_doorbell_properties[] = {
>      DEFINE_PROP_BIT("ioeventfd", IVShmemState, features, IVSHMEM_IOEVENTFD,
>                      true),
>      DEFINE_PROP_ON_OFF_AUTO("master", IVShmemState, master, ON_OFF_AUTO_OFF),
> +    DEFINE_PROP_UINT32("use64", IVShmemState, not_legacy_32bit, 1),
>      DEFINE_PROP_END_OF_LIST(),
>  };

These two hunks add legacy property "use64" to to non-legacy devices.
These devices not having the property is intentional.  Are you sure
adding them makes sense?  If yes, make it a separate patch, and give
your reasons in the commit message.

Please repost just the regression fix as soon as possible.