lkml.org 
[lkml]   [2019]   [Nov]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH RFC 01/14] x86/asm: add iosubmit_cmds512() based on movdir64b CPU instruction
    On Wed, Nov 20, 2019 at 3:19 PM Luck, Tony <tony.luck@intel.com> wrote:
    >
    > On Wed, Nov 20, 2019 at 10:53:39PM +0100, Borislav Petkov wrote:
    > > On Wed, Nov 20, 2019 at 02:23:49PM -0700, Dave Jiang wrote:
    > > > +static inline void iosubmit_cmds512(void __iomem *dst, const void *src,
    > > > + size_t count)
    > >
    > > An iosubmit function which returns void and doesn't tell its callers
    > > whether it succeeded or not? That looks non-optimal to say the least.
    >
    > That's the underlying functionality of the MOVDIR64B instruction. A
    > posted write so no way to know if it succeeded. When using dedicated
    > queues the caller must keep count of how many operations are in flight
    > and not send more than the depth of the queue.
    >
    > > Why isn't there a fallback function which to call when the CPU doesn't
    > > support movdir64b?
    >
    > This particular driver has no option for fallback. Descriptors can
    > only be submitted with MOVDIR64B (to dedicated queues ... in later
    > patch series support for shared queues will be added, but those require
    > ENQCMD or ENQCMDS to submit).

    I think
    >
    > The driver bails out at the beginning of the probe routine if the
    > necessary instructions are not supported:
    >
    > + /*
    > + * If the CPU does not support write512, there's no point in
    > + * enumerating the device. We can not utilize it.
    > + */
    > + if (!cpu_has_write512())
    > + return -ENXIO;
    >
    > Though we should always get past that as this PCI device ID shouldn't
    > every appear on a system that doesn't have the support. Device is on
    > the die, not a plug-in card.
    >
    > > Because then you can use alternative_call() and have the thing work
    > > regardless of hardware support for MOVDIR*.
    > >
    > > > +{
    > > > + const u8 *from = src;
    > > > + const u8 *end = from + count * 64;
    > > > +
    > > > + if (!cpu_has_write512())
    > >
    > > If anything, that thing needs to go and you should use
    > >
    > > static_cpu_has(X86_FEATURE_MOVDIR64B)
    > >
    > > as it looks to me like you would care about speed on this fast path?
    > > Yes, no?
    >
    > That might be a better.

    It's meant to be identical.

    The expectation was that cpu_has_write512() could be used generically
    in drivers like the pmem driver that have a use for movdir64b outside
    of DSA command submission use case. On x86 it would just be #define'd
    to static_cpu_has(X86_FEATURE_MOVDIR64B), on other archs something
    else in the future. For pmem if cpu_has_write512() is false it falls
    back to talking to platform firmware for error management. Case1 from
    the changelog.

    \
     
     \ /
      Last update: 2019-11-21 01:21    [W:2.943 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site