All of lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: Vivek Kumar Gautam <vivek.gautam@arm.com>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>,
	julien.thierry.kdev@gmail.com, kvm@vger.kernel.org,
	andre.przywara@arm.com, lorenzo.pieralisi@arm.com,
	jean-philippe@linaro.org, eric.auger@redhat.com
Subject: Re: [PATCH] vfio/pci: Add support for PCIe extended capabilities
Date: Mon, 4 Oct 2021 10:47:13 +0100	[thread overview]
Message-ID: <20211004094713.GB27173@willie-the-truck> (raw)
In-Reply-To: <867e8db7-c173-5ad2-dca4-69085c89d956@arm.com>

On Wed, Sep 08, 2021 at 03:02:20PM +0530, Vivek Kumar Gautam wrote:
> On 9/3/21 8:45 PM, Alexandru Elisei wrote:
> > On 9/2/21 11:48 AM, Vivek Kumar Gautam wrote:
> > > On 9/2/21 3:29 PM, Alexandru Elisei wrote:
> > I think I found the card that doesn't work when overwriting the extended device
> > configuration space. I tried device assignment with a Realtek 8168 Gigabit
> > Ethernet card on a Seattle machine, and the host freezes when I try to start a VM.
> > Even after reset, the machine doesn't boot anymore and it gets stuck during the
> > boot process at this message:
> > 
> > NewPackageList status: EFI_SUCCESS
> > BDS.SignalConnectDriversEvent(feeb6d60)
> > BDS.ConnectRootBridgeHandles(feeb6db0)
> > 
> > It doesn't go away no matter how many times I reset the machine, to get it booting
> > again I have to pull the plug and plug it again. I tried assigning the device to a
> > VM several times, and this happened every time. The card doesn't have the caps
> > that you added, this is caused entirely by the config space write (tried it with
> > only the config space change).
> > 
> > It could be a problem kvmtool, with Linux or with the machine, but this is the
> > only machine where device assignment works and I would like to keep it working
> > with this NIC.

There is at least a problem with the machine/firmware if you can get it
into this state.

> Sorry for the delay in responding. I took sometime off work.
> Sure, we will try to keep your machine working :)
> 
> > 
> > One solution I see is to add a field to vfio_pci_device (something like has_pcie),
> > and based on that, vfio_pci_fixup_cfg_space() could overwrite only the first 256
> > bytes or the entire device configuration space.
> 
> Does the card support PCI extended caps (as seen from the PCI spec v5.0
> section-7.5)?
> If no, then I guess the check that I am planning to add - to check if
> the device supports extended Caps - can help here. Since we would add
> extended caps based on the mentioned check, it seems only valid to have
> that check before overwriting the configuration space.
> 
> > 
> > It's also not clear to me what you are trying to achieve with this patch. Is there
> > a particular device that you want to get working? Or an entire class of devices
> > which have those features? If it's the former, you could have the size of the
> > config space write depend on the vendor + device id. If it's the latter, we could
> > key the size of the config space write based on the presence of those particular
> > PCIE caps and try and fix other devices if they break.
> 
> Absolutely, we can check for the presence of PCI extended capabilities
> and based on that write the configuration space. If the device has issue
> with only a specific extended capability we can try to fix that by
> keying the DevID-VendorID pair? What do you think?
> 
> > 
> > Will, Andre, do you see other solutions? Do you have a preference?
> 
> Will, Andre, please let me know as well if you have any preferences.

If it's straightforward to keep this working on Seattle, then let's do it,
but I don't think we should bend over backwards to support device assignment
on a dead platform (and I say that as a regular user of one of these
things!)

Will

      parent reply	other threads:[~2021-10-04  9:47 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-10  6:25 [PATCH] vfio/pci: Add support for PCIe extended capabilities Vivek Gautam
2021-08-31 14:54 ` Will Deacon
2021-09-01 10:27   ` Andre Przywara
2021-09-01 10:44     ` Will Deacon
2021-09-02 10:28       ` Vivek Kumar Gautam
2021-08-31 17:14 ` Andre Przywara
2021-09-02 10:42   ` Vivek Kumar Gautam
2021-09-02  9:59 ` Alexandru Elisei
2021-09-02 10:48   ` Vivek Kumar Gautam
2021-09-03 15:15     ` Alexandru Elisei
2021-09-08  9:32       ` Vivek Kumar Gautam
2021-09-08  9:36         ` Vivek Kumar Gautam
2021-09-08 16:20           ` Alexandru Elisei
2021-09-08 16:18         ` Alexandru Elisei
2021-10-04  9:47         ` Will Deacon [this message]

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=20211004094713.GB27173@willie-the-truck \
    --to=will@kernel.org \
    --cc=alexandru.elisei@arm.com \
    --cc=andre.przywara@arm.com \
    --cc=eric.auger@redhat.com \
    --cc=jean-philippe@linaro.org \
    --cc=julien.thierry.kdev@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=vivek.gautam@arm.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.