[virtio-dev] Re: [virtio-comment] Re: [PATCH 09/11] transport-pci: Describe PCI MMR dev config registers

[Jason Wang <jasowang@redhat.com>]

On Wed, Apr 12, 2023 at 12:15 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Apr 12, 2023 at 11:58:39AM +0800, Jason Wang wrote:
> > On Tue, Apr 11, 2023 at 6:42 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Tue, Apr 11, 2023 at 05:01:59PM +0800, Jason Wang wrote:
> > > > On Tue, Apr 11, 2023 at 3:04 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > >
> > > > > On Tue, Apr 11, 2023 at 10:13:40AM +0800, Jason Wang wrote:
> > > > > > On Mon, Apr 10, 2023 at 6:06 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > > >
> > > > > > > On Mon, Apr 10, 2023 at 03:20:52PM +0800, Jason Wang wrote:
> > > > > > > > On Mon, Apr 10, 2023 at 2:40 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > > > > >
> > > > > > > > > On Mon, Apr 10, 2023 at 02:20:16PM +0800, Jason Wang wrote:
> > > > > > > > > > On Mon, Apr 10, 2023 at 2:15 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Mon, Apr 10, 2023 at 09:33:32AM +0800, Jason Wang wrote:
> > > > > > > > > > > > This is fine for vDPA but not for virtio if the design can only work
> > > > > > > > > > > > for some specific setups (OSes/archs).
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks
> > > > > > > > > > >
> > > > > > > > > > > Well virtio legacy has a long history of documenting existing hacks :)
> > > > > > > > > >
> > > > > > > > > > Exactly, so the legacy behaviour is not (or can't be) defined by the
> > > > > > > > > > spec but the codes.
> > > > > > > > >
> > > > > > > > > I mean driver behaviour derives from the code but we do document it in
> > > > > > > > > the spec to help people build devices.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > > But yes, VIRTIO_F_ORDER_PLATFORM has to be documented.
> > > > > > > > > > > And we have to decide what to do about ACCESS_PLATFORM since
> > > > > > > > > > > there's a security problem if device allows not acking it.
> > > > > > > > > > > Two options:
> > > > > > > > > > > - relax the rules a bit and say device will assume ACCESS_PLATFORM
> > > > > > > > > > >   is acked anyway
> > > > > > > > > >
> > > > > > > > > > This will break legacy drivers which assume physical addresses.
> > > > > > > > >
> > > > > > > > > not that they are not already broken.
> > > > > > > >
> > > > > > > > I may miss something, the whole point is to allow legacy drivers to
> > > > > > > > run otherwise a modern device is sufficient?
> > > > > > >
> > > > > > > yes and if legacy drivers don't work in a given setup then we
> > > > > > > should not worry.
> > > > > > >
> > > > > > > > >
> > > > > > > > > > > - a new flag that is insecure (so useful for sec but useless for dpdk) but optional
> > > > > > > > > >
> > > > > > > > > > This looks like a new "hack" for the legacy hacks.
> > > > > > > > >
> > > > > > > > > it's not just for legacy.
> > > > > > > >
> > > > > > > > We have the ACCESS_PLATFORM feature bit, what is the useage for this new flag?
> > > > > > >
> > > > > > >
> > > > > > > ACCESS_PLATFORM is also a security boundary. so devices must fail
> > > > > > > negotiation if it's not there. this new one won't be.
> > > > > > >
> > > > > > >
> > > > > > > > >
> > > > > > > > > > And what about ORDER_PLATFORM, I don't think we can modify legacy drivers...
> > > > > > > > > >
> > > > > > > > > > Thanks
> > > > > > > > >
> > > > > > > > > You play some tricks with shadow VQ I guess.
> > > > > > > >
> > > > > > > > Do we really want to add a new feature in the virtio spec that can
> > > > > > > > only work with the datapath mediation?
> > > > > > > >
> > > > > > > > Thanks
> > > > > > >
> > > > > > > As long as a feature is useful and can't be supported otherwise
> > > > > > > we are out of options.
> > > > > >
> > > > > > Probably not? Is it as simple as relaxing this:
> > > > > >
> > > > > > "Transitional devices MUST expose the Legacy Interface in I/O space in BAR0."
> > > > > >
> > > > > > To allow memory space.
> > > > > >
> > > > > > This works for both software and hardware devices (I had a handy
> > > > > > hardware that supports legacy virtio drivers in this way).
> > > > > >
> > > > > > Thanks
> > > > >
> > > > > Yes it is certainly simpler.
> > > > >
> > > > > Question: what happens if you try to run existing windows guests or dpdk
> > > > > on these? Do they crash horribly or exit gracefully?
> > > >
> > > > Haven't tried DPDK and windows. But I remember DPDK supported legacy
> > > > MMIO bars for a while.
> > > >
> > > > Adding Maxime and Yan for comments here.
> > > >
> > > > >
> > > > > The point of the capability is to allow using modern device ID so such
> > > > > guests will not even try to bind.
> > > >
> > > > It means a mediation layer is required to use. Then it's not an issue
> > > > for this simple relaxing any more?
> > > >
> > > > An advantage of such relaxing is that, for the legacy drivers like an
> > > > ancient Linux version that can already do MMIO access to legacy BAR it
> > > > can work without any mediation.
> > >
> > > Yes. But the capability approach does not prevent that -
> > > just use a transitional ID.
> > > The disadvantage of a transitional ID is some old drivers might crash, or fail
> > > to work in other ways.
> >
> > If the driver is wrote correctly it should fail gracefully if it can't
> > request legacy I/O regions.
>
> it's not just regions. it's things like iommu, ordering, endian-ness...
> we all know legacy is a mess.

Yes, but this proposal will drag us back to legacy, isn't it? Or if it
is used only in the vitalization environment, what's the advantages of
doing it over simply mediating on top of modern device?

1) legacy MMIO bar: spec changes, hypervisor mediation
2) modern device: no spec changes, hypervisor mediation

>
> > > Using a modern ID with a capability
> > > prevents old drivers from attaching.
> >
> > Just to make sure we are at the same page.
> >
> > It looks like the motivation for this series is allow hypervisor to
> > mediate between legacy MMIO BAR and legacy driver. This end ups with a
> > design that only work for a specific use case (virtualization) and a
> > specific setups (e.g using Linux as a hypervisor). By definition, it's
> > not a transitional device.
>
> It can be a transitional or non-transitional device.
>
> > What's more important, if it is only used
> > for virtualization, legacy MMIO bar is not a must. We can present a
> > legacy device on top of modern device by using necessary mediation
> > technologies as discussed.
>
> You may call it a bug, i call it a feature. I am not really
> excited about all the bugs we'll see reported because
> people will try to use legacy guests on bare metal virtio.

I don't either but this (MMIO BAR0) have been used by several vendors for years.

> Can we just not go there please or at least give people
> an option not to go there?

Yes, so we've already had modern device. We know it could be used to
mediate a legacy device without worries ordering iommu etc.

So my understandings are:

1) it's better not invent any of new facilities for legacy
2) if legacy is insisted, allow MMIO BAR0 is much simpler and better

Thanks

>
> > By relaxing to use MMIO at BAR0, we had a much broad use cases. By
> > definition, it's a transitional device. It's useful for both bare
> > metal and virtualization. We don't worry about the buggy drivers or at
> > least fixing (I meant fail on no I/O bar) those drivers is not hard.
> >
> > > Using a capability the system
> > > designed is in control and can decide which drivers to use.
> >
> > Do we allow a virtio device only with this capability? If yes, this
> > seems can mislead vendors. If not, we have already had modern
> > interfaces so I don't see value to add that in the current Linux
> > drivers. The only possible use case is the doing mediation in the
> > hypervisor like Qemu not the driver.
>
> exactly.
>
> > >
> > >
> > >
> > > > At least, if ID can help, it can be used with this as well.
> > > >
> > > > Thanks
> > >
> > >
> > > I don't know what that means.
> >
> > I mean having dedicated ID for this relaxing (though I don't see too
> > much value).
> >
> > Thanks
>
> me neither.
>
> > >
> > > >
> > > >
> > > > >
> > > > >
> > > > > > > Keeping field practice things out of the
> > > > > > > spec helps no one.
> > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > MST
> > > > > > > > >
> > > > > > >
> > > > >
> > >
>


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org