Re: [PATCH v10 1/3] x86/tlb: introduce a flush HVM ASIDs flag

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.04.2020 12:43, Roger Pau Monné wrote:
> On Tue, Apr 21, 2020 at 12:21:12PM +0200, Jan Beulich wrote:
>> On 16.04.2020 15:59, Roger Pau Monne wrote:
>>> Introduce a specific flag to request a HVM guest linear TLB flush,
>>> which is an ASID/VPID tickle that forces a guest linear to guest
>>> physical TLB flush for all HVM guests.
>>>
>>> This was previously unconditionally done in each pre_flush call, but
>>> that's not required: HVM guests not using shadow don't require linear
>>> TLB flushes as Xen doesn't modify the guest page tables in that case
>>> (ie: when using HAP).
>>
>> I'm afraid I'm being confused by this: Even in shadow mode Xen
>> doesn't modify guest page tables, does it?
> 
> I'm also confused now. It's my understand that when running in shadow
> mode guest page tables are not actually used, and the guest uses Xen's
> crafted shadow tables instead, which are based on the original guest
> page tables suitably adjusted by Xen in order to do the p2m
> translation in the HVM case, or the needed PTE adjustments in the PV
> case.
> 
> So guest page tables are not modified, but are also not used as they
> are never loaded into cr3.

This matches my understanding.

>>> @@ -254,3 +257,14 @@ unsigned int flush_area_local(const void *va, unsigned 
>>> int flags)
>>>  
>>>      return flags;
>>>  }
>>> +
>>> +void guest_flush_tlb_mask(const struct domain *d, const cpumask_t *mask)
>>> +{
>>> +    unsigned int flags = (is_pv_domain(d) || paging_mode_shadow(d) ? 
>>> FLUSH_TLB
>>> +                                                                   : 0) |
>>> +                         (is_hvm_domain(d) && cpu_has_svm ? 
>>> FLUSH_HVM_ASID_CORE
>>> +                                                          : 0);
>>
>> Why the is_pv_domain() part of the condition? Afaict for PV
>> domains you can get here only if they have shadow mode enabled.
> 
> Right now yes, the only way to get here for PV domains is when using
> shadow, but if this helper gets used in other non-shadow PV paths then
> Xen's needs to do a TLB flush.

Why would a non-shdow PV path find a need to call this function?

Jan