kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: Yuan Yao <yuan.yao@linux.intel.com>
Cc: tkffaul@outlook.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] vfio/pci: Sanity check IGD OpRegion Size
Date: Sun, 9 May 2021 19:34:08 -0600	[thread overview]
Message-ID: <20210509193408.22ae2b2a@x1.home.shazbot.org> (raw)
In-Reply-To: <20210510011014.q6xfcmqopbqgepbq@yy-desk-7060>

On Mon, 10 May 2021 09:10:14 +0800
Yuan Yao <yuan.yao@linux.intel.com> wrote:

> On Fri, May 07, 2021 at 12:53:17PM -0600, Alex Williamson wrote:
> > The size field of the IGD OpRegion table is supposed to indicate table
> > size in KB, but we've seen at least one report of a BIOS that appears
> > to incorrectly report size in bytes.  The default size is 8 (*1024 =
> > 8KB), but an incorrect implementation may report 8192 (*1024 = 8MB)
> > and can cause a variety of mapping errors.
> > 
> > It's believed that 8MB would be an implausible, if not absurd, actual
> > size, so we can probably be pretty safe in assuming this is a BIOS bug
> > where the intended size is likely 8KB.
> > 
> > Reported-by: Travis Faulhaber <tkffaul@outlook.com>
> > Tested-by: Travis Faulhaber <tkffaul@outlook.com>
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> >  drivers/vfio/pci/vfio_pci_igd.c |   11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/vfio/pci/vfio_pci_igd.c b/drivers/vfio/pci/vfio_pci_igd.c
> > index 228df565e9bc..c89a4797cd18 100644
> > --- a/drivers/vfio/pci/vfio_pci_igd.c
> > +++ b/drivers/vfio/pci/vfio_pci_igd.c
> > @@ -86,7 +86,16 @@ static int vfio_pci_igd_opregion_init(struct vfio_pci_device *vdev)
> >  		return -EINVAL;
> >  	}
> >  
> > -	size *= 1024; /* In KB */
> > +	/*
> > +	 * The OpRegion size field is specified as size in KB, but there have been
> > +	 * user reports where this field appears to report size in bytes.  If we
> > +	 * read 8192, assume this is the case.
> > +	 */
> > +	if (size == OPREGION_SIZE)  
> 
> Is "size >= OPREGION_SIZE" or "size >= smaller but still implausible value
> (like 4096)" better for covering more bad BIOS implementation cases ?

We haven't seen such cases and it seems like a BIOS implementation
competent enough to use something other than the default size, probably
might get the units correct for this field.  Our footing for assuming
this specific implementation error gets shakier if we try to apply it
beyond the default size, imo.  Thanks,

Alex


  reply	other threads:[~2021-05-10  1:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-07 18:53 Alex Williamson
2021-05-08  4:50 ` Zhenyu Wang
2021-05-10  1:10 ` Yuan Yao
2021-05-10  1:34   ` Alex Williamson [this message]
2021-05-10  3:14     ` Yuan Yao
2021-05-10  6:12 ` Christoph Hellwig
2021-05-13 14:43   ` Alex Williamson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210509193408.22ae2b2a@x1.home.shazbot.org \
    --to=alex.williamson@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tkffaul@outlook.com \
    --cc=yuan.yao@linux.intel.com \
    --subject='Re: [PATCH] vfio/pci: Sanity check IGD OpRegion Size' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).