[dpdk-dev] [PATCH v7 02/10] eal: add power management intrinsics

[Thomas Monjalon]

20/10/2020 19:26, Ananyev, Konstantin:
> > 20/10/2020 16:17, David Hunt:
> > > On 20/10/2020 3:01 PM, David Hunt wrote:
> > > > On 20/10/2020 8:35 AM, Thomas Monjalon wrote:
> > > >> 20/10/2020 04:49, Ruifeng Wang:
> > > >>> From: Thomas Monjalon <thomas at monjalon.net>
> > > >>>> 15/10/2020 14:04, Anatoly Burakov:
> > > >>>>> +/**
> > > >>>>> + * This function is not supported on ARM.
> > > >>>>> + */
> > > >>>>> +static inline void
> > > >>>>> +rte_power_monitor(const volatile void *p, const uint64_t
> > > >>>> expected_value,
> > > >>>>> +               const uint64_t value_mask, const uint64_t
> > > >>>>> tsc_timestamp,
> > > >>>>> +               const uint8_t data_sz) {
> > > >>>>> +       RTE_SET_USED(p);
> > > >>>>> +       RTE_SET_USED(expected_value);
> > > >>>>> +       RTE_SET_USED(value_mask);
> > > >>>>> +       RTE_SET_USED(tsc_timestamp);
> > > >>>>> +       RTE_SET_USED(data_sz);
> > > >>>>> +}
> > > >>>> Are you sure it cannot be partially supported with WFE instruction?
> > > >>>>
> > > >>> Armv8 WFE instruction can support monitoring of specific address for
> > > >>> changes,
> > > >>> but not monitoring of TSC timestamp.
> > > >> So it is a partial support.
> > > >>
> > > >> We must try hard to unify architectures support
> > > >> to avoid #ifdef everywhere.
> > > >>
> > > >> I don't agree with how are managed new instructions recently.
> > > >> Please look further.
> > > >>
> > > >
> > > > Hi Thomas,
> > > >
> > > > We believe this is ready for -rc1, can we discuss this with the
> > > > technical board before the RC1 tag is applied?
> > > >
> > >
> > > Hi Thomas,
> > >      By way of further follow-up, here are the reasons why we believe
> > > it's ready for merge.
> > >
> > > There are 18 Acks for the 10 patches, with the two critical patches
> > > getting 4 acks each.
> > > These acks are from ARM, Marvell, IBM and Intel.
> > > There have been 7 revisions, with quite a lot of discussion, and all
> > > comments have been addressed and Ack'd.
> > >  From what I can see, the community are in agreement that this patch
> > > should be merged.
> > 
> > The problem is that I don't agree,
> 
> Thomas, could you explain about what exactly you don't agree with?
> Is it about WFE? Something else? 

It's about -rc1. I will look at this patchset for -rc2.

> > and I feel you tried to avoid comments from others at the beginning.
> > 
> > Now I don't want to spend more time on it before tagging -rc1.
> > 
> > Next time, you'll make sure to Cc and reply everybody.