linux-kernel - Re: [PATCH 2/2] vfio/pci: Remove bardirty from vfio_pci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 17 Sep 2020 16:07:42 -0600
From:   Alex Williamson <alex.williamson@...hat.com>
To:     Cornelia Huck <cohuck@...hat.com>
Cc:     Zenghui Yu <yuzenghui@...wei.com>, <kvm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <wanghaibin.wang@...wei.com>
Subject: Re: [PATCH 2/2] vfio/pci: Remove bardirty from vfio_pci_device

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

> On Thu, 17 Sep 2020 11:31:28 +0800
> Zenghui Yu <yuzenghui@...wei.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@...wei.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