On 27 Oct 16:29, Ananyev, Konstantin wrote:
>
>
> > -----Original Message-----
> > From: Thomas Monjalon <tho...@monjalon.net>
> > Sent: Tuesday, October 27, 2020 6:31 PM
> > To: Ma, Liang J <liang.j...@intel.com>; Ananyev, Konstantin
> > <konstantin.anan...@intel.com>
> > Cc: dev@dpdk.org; Burakov, Anatoly <anatoly.bura...@intel.com>;
> > vikto...@rehivetech.com; Zhang, Qi Z <qi.z.zh...@intel.com>;
> > ruifeng.w...@arm.com; Xing, Beilei <beilei.x...@intel.com>; Guo, Jia
> > <jia....@intel.com>; Yang, Qiming <qiming.y...@intel.com>;
> > Wang, Haiyue <haiyue.w...@intel.com>; Richardson, Bruce
> > <bruce.richard...@intel.com>; Hunt, David <david.h...@intel.com>;
> > jerinjac...@gmail.com; nhor...@tuxdriver.com; McDaniel, Timothy
> > <timothy.mcdan...@intel.com>; Eads, Gage
> > <gage.e...@intel.com>; d...@linux.vnet.ibm.com; Andrew Rybchenko
> > <andrew.rybche...@oktetlabs.ru>; Yigit, Ferruh
> > <ferruh.yi...@intel.com>; jer...@marvell.com; hemant.agra...@nxp.com;
> > viachesl...@nvidia.com; ma...@nvidia.com;
> > ajit.khapa...@broadcom.com; rahul.lakkire...@chelsio.com;
> > johnd...@cisco.com; xavier.hu...@huawei.com; shah...@nvidia.com;
> > sthem...@microsoft.com; g.si...@nxp.com; rm...@marvell.com;
> > maxime.coque...@redhat.com; david.march...@redhat.com
> > Subject: Re: [dpdk-dev] [PATCH v9 04/10] ethdev: add simple power
> > management API
> >
> > 27/10/2020 18:43, Ananyev, Konstantin:
> > > > 27/10/2020 12:15, Liang, Ma:
> > > > > > > --- a/lib/librte_ethdev/rte_ethdev.h
> > > > > > > +++ b/lib/librte_ethdev/rte_ethdev.h
> > > > > > > +/**
> > > > > > > + * Retrieve the wake up address for the receive queue.
> > > > > >
> > > > > > I guess how this function should be used,
> > > > > > but a bit more explanations would not hurt here.
> > > > > agree
> > > > > > > + *
> > > > > > > + * @param port_id
> > > > > > > + * The port identifier of the Ethernet device.
> > > > > > > + * @param queue_id
> > > > > > > + * The Rx queue on the Ethernet device for which information
> > > > > > > will be
> > > > > > > + * retrieved.
> > > > > > > + * @param wake_addr
> > > > > > > + * The pointer to the address which will be monitored.
> > > > > >
> > > > > > This function does not make the address monitored, right?
> > > > > This function only get the target wakeup address. that does not
> > > > > monitor this address.
> > > > > >
> > > > > > > + * @param expected
> > > > > > > + * The pointer to value to be expected when descriptor is set.
> > > > > >
> > > > > > Not sure we should restrict it to a "descriptor".
> > > > > actully that is not limited to a descriptor, any writeback content
> > > > > should work.
> > > > > >
> > > > > > Expecting a value or some bits looks too much restrictive.
> > > > > > I understand it probably fits well for Intel NICs,
> > > > > > but in the general case, we can imagine that any change
> > > > > > in a byte array could be a wake up signal.
> > > > >
> > > > > this parameter doesn not limited user how to use it.
> > > > > In fact, current design can support any bits change within 64 bits
> > > > > content.
> > > >
> > > > How the driver can specify that any value change should be monitored?
> > > > I understand that it is only a value/mask pair,
> > > > it does not give room for "any value".
> > >
> > > As I can read the code, value=0, mask=0 will provide you with 'any value'.
> > > Though it would mean that rte_power_monitor() will *always* go into sleep,
> > > so not sure what will be there any practical usage for such case.
> >
> > I think what is missing is to allow waking up when the value
> > of a byte array is changing, without specifiying any value.
>
>
> I think it will always wakeup on any write to wait_addr.
> What you control with value/mask pair - when we should go to sleep.
> In other words:
> ret = rte_eth_get_wake_addr(port, queue, &wait_addr, &value, &mask, ....);
>
> mask==0: always go to sleep, wakeup at any store to wait_addr.
> mask!=0: go to sleep only if (*wait_addr & mask) == value, wakeup at any
> store to wait_addr.
>
> Liang, Anatoly - feel free to correct me here, if I missed something.
mask and expected value is used to prevent race condition with NIC.
there is a interval between code get target address and issue umwait action.
therefore, the mask is a selector of the bits, if the current value & mask
already
== the expected value just before issue the umwait, that mean that address
is no
longer useful for umwait because of DMA has happend.
then the code path is abort and back to normal logic.
