All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rustad, Mark D" <mark.d.rustad@intel.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Alex Williamson <alex.williamson@redhat.com>,
	Alexander Duyck <alexander.duyck@gmail.com>,
	Alexey Kardashevskiy <aik@ozlabs.ru>,
	"Bjorn Helgaas" <helgaas@kernel.org>,
	Hannes Reinecke <hare@suse.de>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Babu Moger <babu.moger@oracle.com>,
	Paul Mackerras <paulus@samba.org>,
	"santosh@chelsio.com" <santosh@chelsio.com>,
	Netdev <netdev@vger.kernel.org>
Subject: Re: [PATCHv2 3/4] pci: Determine actual VPD size on first access
Date: Mon, 15 Aug 2016 23:16:57 +0000	[thread overview]
Message-ID: <B64D748A-711E-4701-96EF-C447333F5BF5@intel.com> (raw)
In-Reply-To: <1471300400.12231.130.camel@kernel.crashing.org>

[-- Attachment #1: Type: text/plain, Size: 2688 bytes --]

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> Filtering things to work around bugs in existing guests to avoid crashes
> is a different kettle of fish and could be justified but keep in mind that
> in most cases a malicious guest will be able to exploit those HW flaws.

Bugs in existing guests is an interesting case, but I have been focused on  
getting acceptable behavior from a properly functioning guest, in the face  
of hardware issues that can only be resolved in a single place.

I agree that a malicious guest can cause all kinds of havoc with  
directly-assigned devices. Consider a 4-port PHY chip on a shared MDIO bus,  
for instance. There is really nothing to be done about the potential for  
mischief with that kind of thing.

The VPD problem that I had been concerned about arises from a bad design in  
the PCI spec together with implementations that share the registers across  
functions. The hardware isn't going to change and I really doubt that the  
spec will either, so we address it the only place we can.

I am certain that we agree that not everything can or should be addressed  
in vfio. I did not mean to suggest it should try to address everything, but  
I think it should make it possible for correctly behaving guests to work. I  
think that is not unreasonable.

Perhaps the VPD range check should really just have been implemented for  
the sysfs interface, and left the vfio case unchecked. I don't know because  
I was not involved in that issue. Perhaps someone more intimately involved  
can comment on that notion.

> Assuming that a device coming back from a guest is functional and not
> completely broken and can be re-used without a full PERST or power cycle
> is a wrong assumption. It may or may not work, no amount of "filtering"
> will fix the fundamental issue. If your HW won't give you access to PERST
> well ... blame Intel for not specifying a standard way to generate it in
> the first place :-)

Yeah, I worry about the state that a malicious guest could leave a device  
in, but I consider direct assignment always risky anyway. I would just like  
it to at least work in the non-malicious guest cases.

I guess my previous response was really just too terse, I was just focused  
on unavoidable hangs and data corruption, which even were happening without  
any guest involvement. For me, guests were just an additional exposure of  
the same underlying issue.

With hindsight, it is easy to see that a standard reset would now be a  
pretty useful thing. I am sure that even if it existed, we would now have  
lots and lots of quirks around it as well! :-)

--
Mark Rustad, Networking Division, Intel Corporation

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]

  reply	other threads:[~2016-08-15 23:17 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-13 11:25 [PATCHv2 0/4] PCI VPD access fixes Hannes Reinecke
2016-01-13 11:25 ` [PATCHv2 1/4] pci: Update VPD definitions Hannes Reinecke
2016-01-13 11:25 ` [PATCHv2 2/4] pci: allow access to VPD attributes with size '0' Hannes Reinecke
2016-02-09 20:53   ` Bjorn Helgaas
2016-02-10  7:17     ` Hannes Reinecke
2016-01-13 11:25 ` [PATCHv2 3/4] pci: Determine actual VPD size on first access Hannes Reinecke
2016-02-09 21:04   ` Bjorn Helgaas
2016-02-10  7:24     ` Hannes Reinecke
2016-08-09 12:54     ` Alexey Kardashevskiy
2016-08-09 18:12       ` Alexander Duyck
2016-08-10  0:03         ` Benjamin Herrenschmidt
2016-08-10 15:47           ` Alexander Duyck
2016-08-10 23:54             ` Benjamin Herrenschmidt
2016-08-11 18:52               ` Alexander Duyck
2016-08-11 20:17                 ` Alex Williamson
2016-08-12  5:11                   ` Benjamin Herrenschmidt
2016-08-15 17:59                     ` Rustad, Mark D
2016-08-15 22:23                       ` Benjamin Herrenschmidt
2016-08-15 22:33                         ` Benjamin Herrenschmidt
2016-08-15 23:16                           ` Rustad, Mark D [this message]
2016-08-16  0:13                             ` Benjamin Herrenschmidt
2016-08-16  1:40                 ` Alexey Kardashevskiy
2016-08-10  6:23         ` Hannes Reinecke
2016-08-11 10:03           ` [RFC PATCH kernel] PCI: Enable access to custom VPD for Chelsio devices (cxgb3) Alexey Kardashevskiy
2016-09-06 15:48             ` Bjorn Helgaas
2016-09-06 18:30               ` Alexander Duyck
2016-09-21 10:53                 ` Alexey Kardashevskiy
2016-08-09 23:59       ` [PATCHv2 3/4] pci: Determine actual VPD size on first access Benjamin Herrenschmidt
2016-01-13 11:25 ` [PATCHv2 4/4] pci: Blacklist vpd access for buggy devices Hannes Reinecke
2016-01-19 20:57   ` [PATCH v3 " Babu Moger
2016-02-09 21:07   ` [PATCHv2 " Bjorn Helgaas
2016-02-09 21:24     ` Babu Moger
2016-01-15  1:07 ` [PATCHv2 0/4] PCI VPD access fixes Seymour, Shane M
2016-01-15 14:10   ` Babu Moger
2016-01-15 14:18     ` Hannes Reinecke
2016-01-19 20:53 ` Babu Moger
2016-01-21 18:34 ` [PATCH v4 4/4] pci: Blacklist vpd access for buggy devices Babu Moger

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=B64D748A-711E-4701-96EF-C447333F5BF5@intel.com \
    --to=mark.d.rustad@intel.com \
    --cc=aik@ozlabs.ru \
    --cc=alex.williamson@redhat.com \
    --cc=alexander.duyck@gmail.com \
    --cc=babu.moger@oracle.com \
    --cc=benh@kernel.crashing.org \
    --cc=hare@suse.de \
    --cc=helgaas@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=paulus@samba.org \
    --cc=santosh@chelsio.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.