All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoffer Dall <christoffer.dall@linaro.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Eric Auger <eric.auger@linaro.org>,
	linux-arm-kernel@lists.infradead.org, eric.auger@st.com,
	alex.williamson@redhat.com, b.reynal@virtualopensystems.com,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	thomas.lendacky@amd.com, patches@linaro.org,
	suravee.suthikulpanit@amd.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] VFIO: platform: AMD xgbe reset module
Date: Thu, 15 Oct 2015 17:03:21 +0200	[thread overview]
Message-ID: <20151015150321.GA17484@cbox> (raw)
In-Reply-To: <37917545.AP0MxbQBzK@wuerfel>

On Thu, Oct 15, 2015 at 04:55:13PM +0200, Arnd Bergmann wrote:
> On Thursday 15 October 2015 16:46:09 Eric Auger wrote:
> > > 
> > > This is where we'd need a little more changes for this approach. Instead
> > > of unbinding the device from its driver, the idea would be that the
> > > driver remains bound as far as the driver model is concerned, but
> > > it would be in a quiescent state where no other subsystem interacts with
> > > it (i.e. it gets unregistered from networking core or whichever it uses).
> > 
> > Currently we use the same mechanism as for PCI, ie. unbind the native
> > driver and then bind VFIO platform driver in its place. Don't you think
> > changing this may be a pain for user-space tools that are designed to
> > work that way for PCI?
> >
> > My personal preference would be to start with your first proposal since
> > it looks (to me) less complex and "unknown" that the 2d approach.
> 
> We certainly can't easily change from one approach to the other without
> breaking user expectations, so the decision needs to be made carefully.
> 
> The main observation here is that platform devices are unlike PCI in this
> regard because they need extra per-device code. I have argued in the
> past that we should not reuse the "VFIO" name here because it's actually
> something else.  

I've adjusted to consider VFIO a general purpose framework for mapping
device resources into userspace/VMs, and there are certainly a lot of
commonality with both PCI, platform, and potentially other devices for
that to make sense.


> On the other hand, there are a lot of commonalities,
> we just have to make sure we don't try to force the code into one model
> that doesn't really work just to make it look more like PCI VFIO.
> 

But given that we now have code for platform device passthrough that
works in both QEMU and the kernel side and is actually useful for
people, is there a clear technical advantage to go back and rework thaat
at this point?

Don't get me wrong, I like the idea of having a single driver bound to a
platform device, and then that's it, but it just feels like that
discussion doesn't necessarily belong in the context of a patch that
'just' seeks to add reset functionality for a specific device for VFIO?

-Christoffer

WARNING: multiple messages have this Message-ID (diff)
From: christoffer.dall@linaro.org (Christoffer Dall)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] VFIO: platform: AMD xgbe reset module
Date: Thu, 15 Oct 2015 17:03:21 +0200	[thread overview]
Message-ID: <20151015150321.GA17484@cbox> (raw)
In-Reply-To: <37917545.AP0MxbQBzK@wuerfel>

On Thu, Oct 15, 2015 at 04:55:13PM +0200, Arnd Bergmann wrote:
> On Thursday 15 October 2015 16:46:09 Eric Auger wrote:
> > > 
> > > This is where we'd need a little more changes for this approach. Instead
> > > of unbinding the device from its driver, the idea would be that the
> > > driver remains bound as far as the driver model is concerned, but
> > > it would be in a quiescent state where no other subsystem interacts with
> > > it (i.e. it gets unregistered from networking core or whichever it uses).
> > 
> > Currently we use the same mechanism as for PCI, ie. unbind the native
> > driver and then bind VFIO platform driver in its place. Don't you think
> > changing this may be a pain for user-space tools that are designed to
> > work that way for PCI?
> >
> > My personal preference would be to start with your first proposal since
> > it looks (to me) less complex and "unknown" that the 2d approach.
> 
> We certainly can't easily change from one approach to the other without
> breaking user expectations, so the decision needs to be made carefully.
> 
> The main observation here is that platform devices are unlike PCI in this
> regard because they need extra per-device code. I have argued in the
> past that we should not reuse the "VFIO" name here because it's actually
> something else.  

I've adjusted to consider VFIO a general purpose framework for mapping
device resources into userspace/VMs, and there are certainly a lot of
commonality with both PCI, platform, and potentially other devices for
that to make sense.


> On the other hand, there are a lot of commonalities,
> we just have to make sure we don't try to force the code into one model
> that doesn't really work just to make it look more like PCI VFIO.
> 

But given that we now have code for platform device passthrough that
works in both QEMU and the kernel side and is actually useful for
people, is there a clear technical advantage to go back and rework thaat
at this point?

Don't get me wrong, I like the idea of having a single driver bound to a
platform device, and then that's it, but it just feels like that
discussion doesn't necessarily belong in the context of a patch that
'just' seeks to add reset functionality for a specific device for VFIO?

-Christoffer

  reply	other threads:[~2015-10-15 15:02 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-14 15:33 [PATCH] VFIO: platform: AMD xgbe reset module Eric Auger
2015-10-14 15:33 ` Eric Auger
2015-10-14 15:33 ` Eric Auger
2015-10-14 15:38 ` Arnd Bergmann
2015-10-14 15:38   ` Arnd Bergmann
2015-10-15  8:08   ` Eric Auger
2015-10-15  8:08     ` Eric Auger
2015-10-15  8:08     ` Eric Auger
2015-10-15 11:21     ` Arnd Bergmann
2015-10-15 11:21       ` Arnd Bergmann
2015-10-15 11:21       ` Arnd Bergmann
2015-10-15 12:12       ` Christoffer Dall
2015-10-15 12:12         ` Christoffer Dall
2015-10-15 13:59         ` Arnd Bergmann
2015-10-15 13:59           ` Arnd Bergmann
2015-10-15 13:59           ` Arnd Bergmann
2015-10-15 14:46           ` Eric Auger
2015-10-15 14:46             ` Eric Auger
2015-10-15 14:46             ` Eric Auger
2015-10-15 14:55             ` Arnd Bergmann
2015-10-15 14:55               ` Arnd Bergmann
2015-10-15 14:55               ` Arnd Bergmann
2015-10-15 15:03               ` Christoffer Dall [this message]
2015-10-15 15:03                 ` Christoffer Dall
2015-10-15 15:49                 ` Arnd Bergmann
2015-10-15 15:49                   ` Arnd Bergmann
2015-10-15 15:49                   ` Arnd Bergmann
2015-10-15 16:35                   ` Christoffer Dall
2015-10-15 16:35                     ` Christoffer Dall
2015-10-15 16:35                     ` Christoffer Dall
2015-10-15 16:53             ` Alex Williamson
2015-10-15 16:53               ` Alex Williamson
2015-10-15 16:53               ` Alex Williamson
2015-10-15 19:42               ` Christoffer Dall
2015-10-15 19:42                 ` Christoffer Dall
2015-10-15 19:42                 ` Christoffer Dall
2015-10-15 20:26                 ` Alex Williamson
2015-10-15 20:26                   ` Alex Williamson
2015-10-15 20:26                   ` Alex Williamson
2015-10-16 13:06               ` Eric Auger
2015-10-16 13:06                 ` Eric Auger
2015-10-16 13:06                 ` Eric Auger
2015-10-16 13:26                 ` Arnd Bergmann
2015-10-16 13:26                   ` Arnd Bergmann
2015-10-16 13:26                   ` Arnd Bergmann
2015-10-16 13:56                   ` Eric Auger
2015-10-16 13:56                     ` Eric Auger
2015-10-16 13:56                     ` Eric Auger
2015-10-15 14:20         ` Eric Auger
2015-10-15 14:20           ` Eric Auger
2015-10-15 14:20           ` Eric Auger
2015-10-15 14:28           ` Arnd Bergmann
2015-10-15 14:28             ` Arnd Bergmann
2015-10-15 14:28             ` Arnd Bergmann

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=20151015150321.GA17484@cbox \
    --to=christoffer.dall@linaro.org \
    --cc=alex.williamson@redhat.com \
    --cc=arnd@arndb.de \
    --cc=b.reynal@virtualopensystems.com \
    --cc=eric.auger@linaro.org \
    --cc=eric.auger@st.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@linaro.org \
    --cc=suravee.suthikulpanit@amd.com \
    --cc=thomas.lendacky@amd.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.