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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Apr 14, 2020 at 05:06:23PM +0200, Jan Beulich wrote:
> On 14.04.2020 16:53, Roger Pau Monné wrote:
> > On Tue, Apr 14, 2020 at 03:50:15PM +0200, Jan Beulich wrote:
> >> On 14.04.2020 13:19, Roger Pau Monné wrote:
> >>> On Tue, Apr 14, 2020 at 12:13:04PM +0200, Jan Beulich wrote:
> >>>> On 14.04.2020 12:02, Roger Pau Monné wrote:
> >>>>> That seems nice, we would have to be careful however as reducing the
> >>>>> number of ASID/VPID flushes could uncover issues in the existing code.
> >>>>> I guess you mean something like:
> >>>>>
> >>>>> static inline void guest_flush_tlb_mask(const struct domain *d,
> >>>>>                                         const cpumask_t *mask)
> >>>>> {
> >>>>>     flush_mask(mask, (is_pv_domain(d) || shadow_mode_enabled(d) ? 
> >>>>> FLUSH_TLB
> >>>>>                                                                 : 0) |
> >>>>>                      (is_hvm_domain(d) && cpu_has_svm ? 
> >>>>> FLUSH_HVM_ASID_CORE
> >>>>>                                                       : 0));
> >>>>> }
> >>>>
> >>>> Almost - is_hvm_domain(d) && cpu_has_svm seems to wide for me. I'd
> >>>> rather use hap_enabled() && cpu_has_svm, which effectively means NPT.
> >>>> Or am I overlooking a need to do ASID flushes also in shadow mode?
> >>>
> >>> I think so, I've used is_hvm_domain in order to cover for HVM domains
> >>> running in shadow mode on AMD hardware, I think those also need the
> >>> ASID flushes.
> >>
> >> I'm unconvinced: The entire section "TLB Management" in the PM gives
> >> the impression that "ordinary" TLB flushing covers all ASIDs anyway.
> >> It's not stated anywhere (I could find) explicitly though.
> > 
> > Hm, I don't think so. XenRT found a myriad of issues with NPT when p2m
> > code wasn't modified to do ASID flushes instead of plain TLB flushes.
> 
> Well, that's clear from e.g. p2m_pt_set_entry() not doing any
> flushing itself.
> 
> > Even if it's just to stay on the safe side I would perform ASID
> > flushes for HVM guests with shadow running on AMD.
> 
> Tim, any chance you could voice your thoughts here? To me it seems
> odd to do an all-ASIDs flush followed by an ASID one.

I've been reading a bit more into this, and section 15.16.1 states:

"TLB flush operations must not be assumed to affect all ASIDs."

Since the VMM runs on it's own ASID it's my understanding that doing a
TLB flush from the VMM does not flush any of the guests TLBs, and
hence an ASID flush is still required.

Thanks, Roger.