Re: [virtio-dev] [PATCH] ACCESS_PLATFORM/ACCESS_ORDER

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Nov 21, 2018 at 03:35:21PM +0800, Jason Wang wrote:
> 
> On 2018/11/21 äå1:11, Michael S. Tsirkin wrote:
> > > A question is. Are you suggesting to have VIRTIO_F_ORDER_PLATFORM for each
> > > hardware implementation?
> > It's not a simple question. There are implementations such as
> > VDPA which have a hardware and a software component.
> > For a hardware only device that is also pluggable into an
> > arbitrary system, it's true.
> > 
> > > If not, can a hardware detect the platform by
> > > itself and advertise this bit automatically?
> > So a mixed software/hardware device might be able to,
> > and e.g. fallback to a software implementation
> > if driver assumes that.
> > 
> > There's actually some text that suggests this - is
> > that not clear then?
> 
> 
> Maybe it's better to add text about the suggestion of hardware
> implementation.

As I tried to explain, what exactly is "hardware implementation"
is very much platform dependent so I don't know how to do that.
Ideas?

> 
> > 
> > 
> > > ACCESS_PLATFORM has similar question.
> > That one is a bit different in that guest might rely on e.g.
> > vIOMMU for security.  So depending on platform device might want
> > to fail feature negotiation.
> > 
> > But it's not a problem for e.g. SEV since a legacy non encrypted
> > guest will just work.
> 
> 
> How about a encrypted guest that wants to use software IOTLB?

Software iotlb by itself does not *have* to affect ACCESS_PLATFORM - it is
enough to just avoid giving device any memory outside the IOTLB.
However it is probably more robust to set ACCESS_PLATFORM in
that setup to avoid any confusion.

> 
> > 
> > Maybe a new section about security might make sense.
> > 
> > > The question is about the necessarily
> > > of duplicating platform specific feature detection into a device feature.
> > Historically the assumption drivers made was that
> > a device is exactly like driver: same physical address,
> > same permissions and ordering for memory accesses.
> > 
> > Nowdays some drivers make the assumptions about ordering
> > but not permissions and physical address.
> > 
> > We can't just ignore existing drivers, can we?
> 
> 
> It can only work if both features were supported and advertised by devices.
> Consider the case that IOMMU is enable but iommu_platform is not,

So this is a critique of existing spec text then?
Given plans for an imminent 1.1 release let's focus on
the proposed patch, ok?


> in this
> case:
> 
> 1) we still can't recognize old guests, so it won't fail gracefully.
> 
> 2) new guests can't work
> 
> 
> It depends on how much we can gain from failing gracefully. I believe we
> don't want to force a software fallback.

iommu_platform fundamentally doesn't support a software fallback,
it's a security feature. So I agree but what gave you the idea that we
force a software fallback? So ORDER_PLATFORM it seems prodent to
suggest a software fallback given not one driver in the field sets it.

Would your comment be addressed if I replace RECOMMENDED with MAY?

> So I wonder how bad just let
> existing drivers fail and try to patch and backport the fixes without
> introducing new feature bits. With this we can offload those tasks to
> platform or configuration specific codes instead of our own.
> 
> 
> Another question is whether or not we need two separate feature bits.

Well IOMMU_PLATFORM is already set in the field.
We can't make it also imply strong barriers without
breaking a bunch of existing setups.

> E.g
> when should we set one bit but not for another?

I am sorry I don't understand the question.  I did try to
document what does each bit say. Is it completely unclear then?
Could you give some hints on what is unclear? Which kind
of functionality is covered by both such that it's
an open question which one to set?

> 
> > So a device needs to know what it's dealing with, to
> > either fail gracefully or recover by a software fallback.
> > 
> > 
> > > Thanks
> > All good questions, though I'm not sure whether they mean we
> > need to add some text to the spec. Suggestions about
> > what should be documented?
> > 
> 
> Maybe it's better to clarify the suggestion for hardware or mixed
> implementations.
> 
> Thanks

Right but I don't know what to suggest. E.g. intel recently
published data about a hardware vDPA which does not set either
bits. They are working on support for a vIOMMU at which point
they will set IOMMU_PLATFORM. Once they start worrying about
portability they might add ORDER_PLATFORM. But given it's
VDPA they don't necessarily need to fail without even
on weakly ordered platforms.

-- 
MST