Re: [Xen-devel] [PATCH for-4.7 4/4] x86/hvm: Fix invalidation for emulated invlpg instructions

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/05/16 16:14, Tim Deegan wrote:
> Hi,
>
> At 14:15 +0100 on 09 May (1462803342), Andrew Cooper wrote:
>> hap_invlpg() is reachable from the instruction emulator, which means
>> introspection and tests using hvm_fep can end up here.  As such, crashing the
>> domain is not an appropriate action to take.
>>
>> Fixing this involves rearranging the callgraph.
>>
>> paging_invlpg() is now the central entry point.  It first checks for
>> applicability of invalidation based on virtual address, and optionally calls
>> into the paging invalidation logic.  For HVM domains, it also makes ASID/VPID
>> management calls.
> This reshuffle looks fine, but leaves the return value looking pretty
> strange.

I suppose it does.  This looks to be better option.

> For HVM guests it's longer correct (since the hardware
> operation has moved inside paging_invlpg() it should always return 0)
> but none of the callers actually check it, so yay?
>
> I think it might be better to make that return value internal to
> paging_invlpg() and have it DTRT for PV guests as it now does for HVM
> ones, e.g.:
>
>     void paging_invlpg(struct vcpu *v, unsigned long va)
>     {
>         if ( !is_canonical_address(va) )
>             return;
>
>         if ( paging_mode_enabled(v->domain)
>            && !paging_get_hostmode(v)->invlpg(v, va) )
>           return;
>
>         if ( is_pv_vcpu(v) )
>             flush_tlb_one_local(va)
>         else
>             hvm_funcs.invlpg(va);
>     }
>
> with appropriate simplifications at the PV callsites.
>
> I simplified the canonical/__addr_ok test there because I don't think
> we care about the speed of guest invlpg of Xen addresses; I suspect we
> could remove it entirely, and turn a fast emulated NOP into a slow one.

A PV guest should not be able to invlpg Xen addresses at all, which is
why the checks were recently added in c/s 828e114f7 "x86/mmuext: tighten
TLB flush address checks".  I will see if I can untangle this as well
without avoiding the __addr_ok() check.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel