From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 85837A0519; Fri, 3 Jul 2020 17:19:20 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 63B021DC36; Fri, 3 Jul 2020 17:19:20 +0200 (CEST) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) by dpdk.org (Postfix) with ESMTP id 893C31DC27 for ; Fri, 3 Jul 2020 17:19:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1593789558; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=18FRTayt9cgdags15f3s2ZyqrqkhmzJCMC1hn9jHsUA=; b=PmZ72njehxfroYGtgwjit2PcYV5Hmv+UPuwX52CBlZCaipQPC3iR2bgp9KWQD/y9Qnayk/ DdZxepEP+6AdJxPP8MwPxGb+gmOtlZ0xHuImo5GEY/hfJh3QliFUz2+HqVGRxIVP03wubW qfn1QhOMazPtodaC3QgWZTLe1Gpj0ww= Received: from mail-il1-f197.google.com (mail-il1-f197.google.com [209.85.166.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-265-zJ7QasHjMh6b3hZePIgZ_w-1; Fri, 03 Jul 2020 11:19:16 -0400 X-MC-Unique: zJ7QasHjMh6b3hZePIgZ_w-1 Received: by mail-il1-f197.google.com with SMTP id y3so10201121ily.1 for ; Fri, 03 Jul 2020 08:19:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=18FRTayt9cgdags15f3s2ZyqrqkhmzJCMC1hn9jHsUA=; b=DF/Fyu/zNkRD/DfQ97WyuugeUHmfiP0l5178XHetwTx/kDym7hrH8dwG7CdLW/ZO8M x5WMDOZdddWy58PWWuUom0FD/h7IHMuPtQPLb9jfu5cCId+XnuJtyF9pkN3qDQrvTeej E8WQstOVE4E1e9X0Cz4XvrbEU2VH4RX5d5oY7QZwpja7pjWEWW3zGG0mmKRcl9ibQkwg 75P4iZQxUE/QvP1cRwPQOEA2gJICvnrRjBvwMKpSl1m9LssrMyIM+crDESLBlnWl+NGp 4a5jQhMbH+Ty2KKT6Her7i6qNxed8om6sfhcw3BqawVrBgmJYA/xiGCRu/yLsJscggrd lkdg== X-Gm-Message-State: AOAM531t/9YwWLWGMW1/IPlmS+tMgAAw8os9/SWi+I2rO2AZ46oyvkZq nKs3HNXt2dHwtHE0PnvodIx+9pcoK90tfBlKsUCO0iLdCAROWLbwb1CGTpnk5McJ2KjOjrqoeFj 5f7f1W+iP+W/I+BywJHY= X-Received: by 2002:a6b:3bc6:: with SMTP id i189mr12659325ioa.180.1593789555841; Fri, 03 Jul 2020 08:19:15 -0700 (PDT) X-Google-Smtp-Source: ABdhPJy90fs6cFkquvS6EUfuSCiDlCZBYIwsyOX+W5094RhB0l8plF/QbapHYNZFAshva77N+WQogK2QAjddatO0y+M= X-Received: by 2002:a6b:3bc6:: with SMTP id i189mr12659303ioa.180.1593789555472; Fri, 03 Jul 2020 08:19:15 -0700 (PDT) MIME-Version: 1.0 References: <1591870283-7776-1-git-send-email-radu.nicolau@intel.com> <1593681821-22357-1-git-send-email-radu.nicolau@intel.com> <1593681821-22357-2-git-send-email-radu.nicolau@intel.com> In-Reply-To: <1593681821-22357-2-git-send-email-radu.nicolau@intel.com> From: David Marchand Date: Fri, 3 Jul 2020 17:19:04 +0200 Message-ID: To: Radu Nicolau Cc: dev , Beilei Xing , Jeff Guo , Bruce Richardson , "Ananyev, Konstantin" , Jerin Jacob Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v4 1/2] eal: add WC store functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Jul 2, 2020 at 11:24 AM Radu Nicolau wrote: > > Add rte_write32_wc and rte_write32_wc_relaxed functions > that implement 32bit stores using write combining memory protocol. > Provided generic stubs and x86 implementation. > > Signed-off-by: Radu Nicolau > Acked-by: Bruce Richardson > --- > v4: address feedback and include ack > > lib/librte_eal/include/generic/rte_io.h | 47 +++++++++++++++++++++++++++ > lib/librte_eal/x86/include/rte_io.h | 56 +++++++++++++++++++++++++++++++++ > 2 files changed, 103 insertions(+) > > diff --git a/lib/librte_eal/include/generic/rte_io.h b/lib/librte_eal/include/generic/rte_io.h > index da457f7..7391782 100644 > --- a/lib/librte_eal/include/generic/rte_io.h > +++ b/lib/librte_eal/include/generic/rte_io.h > @@ -229,6 +229,39 @@ rte_write32(uint32_t value, volatile void *addr); > static inline void > rte_write64(uint64_t value, volatile void *addr); > > +/** > + * Write a 32-bit value to I/O device memory address addr using write > + * combining memory write protocol. Depending on the platform write combining > + * may not be available and/or may be treated as a hint and the behavior may > + * fallback to a regular store. > + * > + * @param value > + * Value to write > + * @param addr > + * I/O memory address to write the value to > + */ > +static inline void > +rte_write32_wc(uint32_t value, volatile void *addr); This is a new API, and even if inlined, it should be marked experimental. Is volatile necessary? > + > +/** > + * Write a 32-bit value to I/O device memory address addr using write > + * combining memory write protocol. Depending on the platform write combining > + * may not be available and/or may be treated as a hint and the behavior may > + * fallback to a regular store. > + * > + * The relaxed version does not have additional I/O memory barrier, useful in > + * accessing the device registers of integrated controllers which implicitly > + * strongly ordered with respect to memory access. > + * > + * @param value > + * Value to write > + * @param addr > + * I/O memory address to write the value to > + */ > +static inline void > +rte_write32_wc_relaxed(uint32_t value, volatile void *addr); > + > + Double empty line (there are some other in this patch that I won't flag again). > #endif /* __DOXYGEN__ */ > > #ifndef RTE_OVERRIDE_IO_H > @@ -345,6 +378,20 @@ rte_write64(uint64_t value, volatile void *addr) > rte_write64_relaxed(value, addr); > } > > +#ifndef RTE_NATIVE_WRITE32_WC Missing return type, this causes build failure on anything but x86. > +rte_write32_wc(uint32_t value, volatile void *addr) > +{ > + rte_write32(value, addr); > +} > + > +static __rte_always_inline void > +rte_write32_wc_relaxed(uint32_t value, volatile void *addr) > +{ > + rte_write32_relaxed(value, addr); > +} > +#endif /* RTE_NATIVE_WRITE32_WC */ > + > + > #endif /* RTE_OVERRIDE_IO_H */ > > #endif /* _RTE_IO_H_ */ > diff --git a/lib/librte_eal/x86/include/rte_io.h b/lib/librte_eal/x86/include/rte_io.h > index 2db71b1..c95ed67 100644 > --- a/lib/librte_eal/x86/include/rte_io.h > +++ b/lib/librte_eal/x86/include/rte_io.h > @@ -9,8 +9,64 @@ > extern "C" { > #endif > > +#include "rte_cpuflags.h" Inclusion of this header should be out of the extern "C" block. > + > +#define RTE_NATIVE_WRITE32_WC > #include "generic/rte_io.h" > > +/** > + * @internal > + * MOVDIRI wrapper. > + */ > +static __rte_always_inline void > +_rte_x86_movdiri(uint32_t value, volatile void *addr) > +{ > + asm volatile( > + /* MOVDIRI */ > + ".byte 0x40, 0x0f, 0x38, 0xf9, 0x02" > + : > + : "a" (value), "d" (addr)); > +} > + > +static __rte_always_inline void > +rte_write32_wc(uint32_t value, volatile void *addr) > +{ > + static int _x86_movdiri_flag = -1; > + if (_x86_movdiri_flag == 1) { > + rte_wmb(); > + _rte_x86_movdiri(value, addr); > + } else if (_x86_movdiri_flag == 0) { > + rte_write32(value, addr); > + } else { > + _x86_movdiri_flag = > + (rte_cpu_get_flag_enabled(RTE_CPUFLAG_MOVDIRI) > 0); Can't this cpu flag check be moved in a constructor? This would avoid this copy/paste. > + if (_x86_movdiri_flag == 1) { > + rte_wmb(); > + _rte_x86_movdiri(value, addr); > + } else { > + rte_write32(value, addr); > + } > + } > +} > + > +static __rte_always_inline void > +rte_write32_wc_relaxed(uint32_t value, volatile void *addr) > +{ > + static int _x86_movdiri_flag = -1; Same check with a static variable with the same name. I wonder if wrapping all of this in a single function would be more elegant. Then rte_write32_wc(|_relaxed) would call it with a flag. > + if (_x86_movdiri_flag == 1) { > + _rte_x86_movdiri(value, addr); > + } else if (_x86_movdiri_flag == 0) { > + rte_write32_relaxed(value, addr); > + } else { > + _x86_movdiri_flag = > + (rte_cpu_get_flag_enabled(RTE_CPUFLAG_MOVDIRI) > 0); > + if (_x86_movdiri_flag == 1) > + _rte_x86_movdiri(value, addr); > + else > + rte_write32_relaxed(value, addr); > + } > +} > + > #ifdef __cplusplus > } > #endif > -- > 2.7.4 > -- David Marchand