On Tue, Apr 25, 2023 at 09:39:28PM +0000, Parav Pandit wrote: > > > From: Michael S. Tsirkin <[email protected]> > > Sent: Tuesday, April 25, 2023 5:06 PM > > > On 4/23/2023 3:35 AM, Heng Qi wrote: > > > > \subsubsection{Legacy Interface: Feature bits}\label{sec:Device > > > > Types / Network Device / Feature bits / Legacy Interface: Feature bits} > > > > @@ > > -198,6 +202,7 @@ \subsection{Device configuration layout}\label{sec:Device > > Types / Network Device > > > > u8 rss_max_key_size; > > > > le16 rss_max_indirection_table_length; > > > > le32 supported_hash_types; > > > > + le32 supported_tunnel_hash_types; > > > > this needs a comment explaining it only exists with some feature bits. > > > Yes, it is already there. > +Field \field{supported_tunnel_hash_types} only exists if the device supports > inner header hash, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set. > + > I think it should be changed from "device supports" to "driver negotiated". > > > > > }; > > > In v12 I was asking this to move to above field from the config area > > > to the GET command in comment [1] as, > > > > > > "With that no need to define two fields at two different places in > > > config area and also in cvq." > > > > I think I disagree. > > the proposed design is consistent with regular tunneling. > > > Sure. > I understand how config space has evolved from 0.9.5 to know without much > attention, but really expanding this way is not helpful. > It requires building more and more RAM based devices even for PCI PFs, which > is sub optimal.
No, this is ROM, not RAM. > CVQ already exists and provides the SET command. There is no reason to do GET > in some other way. Spoken looking at just hardware cost :) The reason is that this is device specific. Maintainance overhead and system RAM cost for the code to support this should not be ignored. > Device has single channel to provide a value and hence it doesn't need any > internal synchronization between two paths. > > So, if we add a new feature bit as VIRTIO_F_CFG_SPACE_OVER_AQ it is an > improvement. > But it still comes with a cost which device cannot mitigate. > The cost is, > 1. a driver may not negotiate such feature bit, and device ends up burning > memory. > 1.b Device provisioning becomes a factor of what some guests may use/not > use/already using on the live system. > > 2. Every device needs AQ even when the CVQ exists. > > Hence, better to avoid expanding current structure for any new fields, > specially which has the SET equivalent. > > But if we want to with the path of VIRTIO_F_CFG_SPACE_OVER_AQ, it is fine. > More things can evolve for generic things like config space over AQ. I am not sure what does VIRTIO_F_CFG_SPACE_OVER_AQ mean, and what are these costs. What I had in mind is extending proposed vq transport to support sriov. I don't see why we can not have exactly 0 bytes of memory per VF. And if we care about single bytes of PF memory (do we? there's only one PF per SRIOV device ...), what we should do is a variant of pci transport that aggressively saves memory. -- MST --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
