[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH v12 05/10] pci: add admin vq registers to virtio over pci
On Thu, Apr 27, 2023 at 01:31:29AM +0300, Max Gurtovoy wrote: > > > On 27/04/2023 1:11, Parav Pandit wrote: > > > > > From: Max Gurtovoy <mgurtovoy@nvidia.com> > > > Sent: Wednesday, April 26, 2023 5:12 PM > > > > > > On 25/04/2023 1:14, Parav Pandit wrote: > > > > > > > > > > > > On 4/24/2023 6:06 PM, Max Gurtovoy wrote: > > > > > > > > > > +If VIRTIO_F_ADMIN_VQ has been negotiated, and if the driver > > > > > > +configures any administration virtqueues, the driver MUST configure > > > > > > +the administration virtqueues using the index in the range > > > > > > +\field{admin_queue_index} to \field{admin_queue_index} + > > > > > > +\field{admin_queue_num} - 1 inclusive. > > > > > > +The driver MAY configure less administration virtqueues than > > > > > > +supported by the device. > > > > > > > > > > we need to say something about other virtq's indexes. > > > > > For example, if aq index = 0 then for the vblk device the request > > > > > queues would start from index = 1. > > > > > > > > > > can we address this ? > > > > > > > > It may slightly complicate the device implementation when device > > > > dynamically changes the VQ indices based on the negotiation of the > > > > feature bit. > > > > Also the drivers with and without need a different VQ index handling. > > > > > > > > I am thinking that aq index should not overlap with the vq index in > > > > range exposed by num_queues field. > > > > > > > > This also aligns to the exclusion of aq count from num_queues field. > > > > > > The namespace of the aq and other vq index is the same. > > > And the configuration is done using the same queue_select and other registers. > > > Thus, we need to address the above comment otherwise all the device > > > virtqueues chapters are wrong. > > > > Michael has added below line in this patch in num_queues description so it covers the exclusion part. > > > > + This excludes administration virtqueues if any are supported. > > > > This is not related to what I was talking about. > > For example if vnet device: > > 0 receiveq1 > 1 transmitq1 > ... > 2(N-1) receiveqN > 2(N-1)+1 transmitqN > 2N controlq > N=1 if neither VIRTIO_NET_F_MQ nor VIRTIO_NET_F_RSS are negotiated, > otherwise N is set by max_- > virtqueue_pairs. > controlq only exists if VIRTIO_NET_F_CTRL_VQ set > > and in vfs device: > > 0 hiprio > 1...n request queues > > So if one device will expose adminq that will be with index 0 the above is > wrong description. > > Agree that this should be addressed ? It's addressed by patch 9: +If VIRTIO_F_ADMIN_VQ has been negotiated, the value +\field{admin_queue_index} MUST be equal to, or bigger than +\field{num_queues}; also, \field{admin_queue_num} MUST be +smaller than, or equal to 0x10000 - \field{admin_queue_index}, +to ensure that indices of valid admin queues fit into +a 16 bit range beyond all other virtqueues. Thus adminq can not have index 0. -- MST
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]