Re: [Xen-devel] [PATCH 2/2] x86: use invpcid to do global flushing

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.03.18 at 10:50, <wei.liu2@xxxxxxxxxx> wrote:
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>

No description at all? I'd at least expect mention of how much of a
performance win this is (for whichever hardware you happen to
know that).

> @@ -120,11 +121,24 @@ unsigned int flush_area_local(const void *va, unsigned 
> int flags)
>          else
>          {
>              u32 t = pre_flush();
> -            unsigned long cr4 = read_cr4();
>  
> -            write_cr4(cr4 & ~X86_CR4_PGE);
> -            barrier();
> -            write_cr4(cr4);
> +            if ( !cpu_has_invpcid )
> +            {
> +                unsigned long cr4 = read_cr4();
> +
> +                write_cr4(cr4 & ~X86_CR4_PGE);
> +                barrier();
> +                write_cr4(cr4);
> +            }
> +            else
> +            {
> +                /*
> +                 * Using invpcid to flush all mappings works
> +                 * regardless of whether PCID is enabled or not.
> +                 * It is faster than read-modify-write CR4.
> +                 */
> +                invpcid_flush_all();
> +            }

The reference to PCID in the comment isn't really meaningful imo.
PCID and INVPCID are independent features anyway. Also please
don't create artificially short comment lines.

Generally I also think such if() conditions would better be inverted:
There's no reason to make the legacy form look as if it was
preferred.

And then - what about the use in write_cr3() and the two uses that
remain after my XPTI follow-up series (which sadly looks to be stuck
for whatever reason), or (without that series) the write_cr3
assembler macro?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel