Message ID | 1479206719-14780-2-git-send-email-ann.zhuangyanying@huawei.com |
---|---|
State | New |
Headers | show |
Zhuangyanying <ann.zhuangyanying@huawei.com> writes: > From: ZhuangYanying <ann.zhuangyanying@huawei.com> > > After commit 5400c02, ivshmem_64bit renamed to not_legacy_32bit, > and changed the implementation of this property. > Then use64 = 1, ~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. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Zhuang Yanying <ann.zhuangyanying@huawei.com> > Reviewed-by: Gonglei <arei.gonglei@huawei.com> I find the commit message hard to understand. To improve it, let's figure out what exactly is broken. The sense of device ivshmem's property use64 is reverted: use64=0 enables the 64 bit memory bar, use64=1 disables it. Default is 32 bit unless pc-1.2 or older. This regressed in commit 5400c02. Before, use64 had the correct sense, and the default was 64 bit unless pc-1.2 or older. Note that devices ivshmem-plain and ivshmem-doorbell always have a 64 bit memory bar, as intended. > --- > hw/misc/ivshmem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c > index 230e51b..b897685 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; > } Thanks a lot for catching my mistake! However, this breaks ivshmem-plain and ivshmem-doorbell. You fix that in the next patch, but that's no good, the two need to be squashed into a single commit. Suggested commit message for the squashed commit: ivshmem: Fix 64 bit memory bar configuration 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 Would that work for you?
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c index 230e51b..b897685 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; }