> From: Michael S. Tsirkin <m...@redhat.com>
> Sent: Friday, June 9, 2023 3:22 AM
> 
> On Fri, Jun 09, 2023 at 02:27:01PM +0800, Jason Wang wrote:
> > > I would like to keep the stateful interactions of 1.x device outside of 
> > > 0.9.5.
> >
> > I don't think this is a real problem, but let's see the drawbacks of
> > this proposal:
> >
> > 1) non-trivial changes of full new transport alike ABI
> > 2) can't work for nesting
> > 3) require vendor to implement legacy behaviour which can't be
> > documented precisely
> > 4) require admin virtqueue to work
> >
> > We won't have the above issues if we use modern + legacy header/mac.
> >
> > Thanks
> 
> 
> All this is true. But it's by design. It makes the legacy thing as isolated as
> possible. Because let's be frank we won't be able to drop legacy support in 
> like
> 10 years. But hardware vendors will do that quickly if they can't sell 
> hardware.

I don't think above 4 points are true for below reasoning.

Hypervisor flow without involving guest; first sanity round to figure out 
things can work:
1. reset the device
2. set ACK and DRIVER bit
3. read features and make sure _MAC, _HDR are supported.
4. if not abort.
5. reset the device 2nd time so that guest can have right view of things.

On guest access of legacy area:
6. reset the device
7. set ACK and DRIVER bit on guest request
8. read features and make sure _MAC, _HDR are offered
8.1 Hope for the best that on two completely unrelated device reset on #1 and 
#6, the device offers exact same feature bits.
And mention this in the spec 1.x for rest of the future.

We shouldn't be adding such a limitation to the spec.
We have seen this with mlx5 device where 5 years ago one thought this cannot 
happen.
And now with modern use case now features changes across two resets for mlx5 
device.
Baking such limitation in current spec for past 1.x is sub-optimal.

ok, so to avoid baking such reset flow nasty things in spec, lets avoid flow of 
#1 to #5 in hypervisor.
So, provision the device to support these new feature bits via AQ.
So AQ is required for feature provisioning anyway.

So, your point #4 is required in both methods and so it scores same as this 
proposal.
Hence feature bit is not of an advantage.

With _MAC now we need writable mac on 1.x config space.
for 1.x writable mac has two requirements.
1. It must be atomic (not for 0.9.5, but must for 1.x like CVQ)
2. should be synchronous to know success/failure to know when it is effective

Both are present on the CVQ, so yet add another duplicate 1.x scheme that 
fulfill above requirements.

ok. So may be let's do AQ command for just mac setting.

This scores down now for two reasons.
a. Duplicate of existing 1.x feature
b. requires AQ.

So, from RW mac we moved from 1.x cfg space to AQ. This mediation for 1.x is 
not good.
And now it's not trivial either as it is not just simple *p_mac = X.
Hence, #1 of non-trivial starts to looks less appealing.

Your point #3 about vendor to implement legacy behavior.
If vendor needs to support legacy, vendor anyway needs to implement _HDR anyway 
in data path.
and above MAC change, and feature provisioning.

For legacy there is no extra/special documentation.
All the behavior listed in Legacy interfaces section for configuration register 
present applies here.

So, I don't see mac support is trivial by any means compared to proposed scheme 
for 1.x.
Hence, comparing trivialness of two solutions seems same for your point #1 once 
you do mac plumbing.

Regarding #2 on nesting. I won't claim I understand this as your level of 
knowledge.
If you are sauing only the VF is in VM as virtio PCI device to supporting 
nested guest, that doesn't have AQ, hence it doesn't work due above issues.

Then yes, but than it is back to square one, where you need sequence #1 to #8 
to be done + non-forward-looking spec changes on reset flow.

In that alternative, one can say, hey skip steps #1 to #5, and on step #8 
doesn't have required feature bit, mark the device FAILED.
But this is common case and its late in the init flow to discover it.

And now MAC cannot be set atomically either in nesting with just feature bit 
without ugly and non-trivial spec updates.
And when one needs nesting with legacy, probably PV is better.

As Michael said,
Overall isolating legacy to AQ for config, intx and by reusing 1.x's 
notification is good trade off where 1.x device level interface is kept as 
detached from the legacy as possible.

This is why the notification address query also was desired via AQ as proposed 
in v3, but it is small trade off if you think it should be discovered via PCI 
cap like v5.

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

Reply via email to