On 02.06.2024 22:04, Petr Beneš wrote:
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -4887,10 +4887,10 @@ bool asmlinkage vmx_vmenter_helper(const struct
> cpu_user_regs *regs)
>
> for ( i = 0; i < MAX_ALTP2M; ++i )
> {
> - if ( currd->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
> + if ( altp2m_get_eptp(currd, i) == mfn_x(INVALID_MFN) )
> continue;
>
> - ept = &currd->arch.altp2m_p2m[i]->ept;
> + ept = &altp2m_get_p2m(currd, i)->ept;
> if ( cpumask_test_cpu(cpu, ept->invalidate) )
> {
> cpumask_clear_cpu(cpu, ept->invalidate);
I'm not convinced we want to add the extra overhead in cases like
this one, where we shouldn't need it. I'd like to hear other
maintainers' views.
> --- a/xen/arch/x86/include/asm/altp2m.h
> +++ b/xen/arch/x86/include/asm/altp2m.h
> @@ -19,6 +19,38 @@ static inline bool altp2m_active(const struct domain *d)
> return d->arch.altp2m_active;
> }
>
> +static inline struct p2m_domain *altp2m_get_p2m(const struct domain* d,
Nit: Style (misplaced *).
> + unsigned int idx)
> +{
> + return d->arch.altp2m_p2m[array_index_nospec(idx, MAX_ALTP2M)];
> +}
> +
> +static inline uint64_t altp2m_get_eptp(const struct domain* d,
Same here. And more further down.
Further: At this point it's not necessary yet to switch away from
array_access_nospec(). You doing so right away is probably okay, but
then needs justifying in the description.
Jan
