Re: [Xen-devel] [PATCH v6 9/9] xen/x86: use PCID feature

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.04.18 at 09:58, <jgross@xxxxxxxx> wrote:
> @@ -102,14 +104,34 @@ void switch_cr3_cr4(unsigned long cr3, unsigned long 
> cr4)
>      old_cr4 = read_cr4();
>      if ( old_cr4 & X86_CR4_PGE )
>      {
> +        /*
> +         * X86_CR4_PGE set means PCID is inactive.
> +         * We have to purge the TLB via flipping cr4.pge.
> +         */
>          old_cr4 = cr4 & ~X86_CR4_PGE;
>          write_cr4(old_cr4);
>      }
> +    else if ( use_invpcid )
> +        /*
> +         * Flushing the TLB via INVPCID is necessary only in case PCIDs are
> +         * in use, which is true only with INVPCID being available.
> +         * Without PCID usage the following write_cr3() will purge the TLB
> +         * (we are in the cr4.pge off path) from all entries.

s/from/of/ ?

> @@ -136,11 +158,32 @@ unsigned int flush_area_local(const void *va, unsigned 
> int flags)
>              /*
>               * We don't INVLPG multi-page regions because the 2M/4M/1G
>               * region may not have been mapped with a superpage. Also there
> -             * are various errata surrounding INVLPG usage on superpages, and
> -             * a full flush is in any case not *that* expensive.
> +             * are various errata surrounding INVLPG usage on superpages,
> +             * and a full flush is in any case not *that* expensive.
>               */

Stray change?

> @@ -508,7 +513,8 @@ unsigned long pv_guest_cr4_to_real_cr4(const struct vcpu 
> *v)
>      cr4 = v->arch.pv_vcpu.ctrlreg[4] & ~X86_CR4_DE;
>      cr4 |= mmu_cr4_features & (X86_CR4_PSE | X86_CR4_SMEP | X86_CR4_SMAP |
>                                 X86_CR4_OSXSAVE | X86_CR4_FSGSBASE);
> -    cr4 |= d->arch.pv_domain.xpti  ? 0 : X86_CR4_PGE;
> +    cr4 |= (d->arch.pv_domain.xpti || d->arch.pv_domain.pcid) ? 0 : 
> X86_CR4_PGE;
> +    cr4 |= d->arch.pv_domain.pcid ? X86_CR4_PCIDE : 0;

I think this would be more clear to follow as

    if ( d->arch.pv_domain.pcid )
        cr4 |= X86_CR4_PCIDE;
    else if ( !d->arch.pv_domain.xpti )
        cr4 |= X86_CR4_PGE;

Anyway, with or without these addressed (which could probably
also be done while committing, as long as you agree)
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


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