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).
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.
-Tony
