Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 17 Sep 2020 16:07:42 -0600
From:   Alex Williamson <alex.williamson@redhat.com>
To:     Cornelia Huck <cohuck@redhat.com>
Cc:     Zenghui Yu <yuzenghui@huawei.com>, <kvm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <wanghaibin.wang@huawei.com>
Subject: Re: [PATCH 2/2] vfio/pci: Remove bardirty from vfio_pci_device
Message-ID: <20200917160742.4e4d6efd@x1.home>
In-Reply-To: <20200917133537.17af2ef3.cohuck@redhat.com>
References: <20200917033128.872-1-yuzenghui@huawei.com>
        <20200917033128.872-2-yuzenghui@huawei.com>
        <20200917133537.17af2ef3.cohuck@redhat.com>
Organization: Red Hat
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 17 Sep 2020 13:35:37 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Thu, 17 Sep 2020 11:31:28 +0800
> Zenghui Yu <yuzenghui@huawei.com> wrote:
> 
> > It isn't clear what purpose the @bardirty serves. It might be used to avoid
> > the unnecessary vfio_bar_fixup() invoking on a user-space BAR read, which
> > is not required when bardirty is unset.
> > 
> > The variable was introduced in commit 89e1f7d4c66d ("vfio: Add PCI device
> > driver") but never actually used, so it shouldn't be that important. Remove
> > it.
> > 
> > Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> > ---
> >  drivers/vfio/pci/vfio_pci_config.c  | 7 -------
> >  drivers/vfio/pci/vfio_pci_private.h | 1 -
> >  2 files changed, 8 deletions(-)  
> 
> Yes, it seems to have been write-only all the time.

I suspect the intent was that vfio_bar_fixup() could test
vdev->bardirty to avoid doing work if no BARs had been written since
they were last read.  As it is now we regenerate vconfig for all the
BARs every time any offset of any of them are read.  BARs aren't
re-read regularly and config space is not a performance path, but maybe
we should instead test if we see any regressions from returning without
doing any work in vfio_bar_fixup() if vdev->bardirty is false.  Thanks,

Alex