Re: [XEN PATCH v1 1/1] Invalidate cache for cpus affinitized to the domain

["Shamsundara Havanur, Harsha" <havanur@xxxxxxxxxx>]

On Mon, 2020-12-14 at 16:01 +0000, Andrew Cooper wrote:
> CAUTION: This email originated from outside of the organization. Do
> not click links or open attachments unless you can confirm the sender
> and know the content is safe.
> 
> 
> 
> On 14/12/2020 10:56, Julien Grall wrote:
> > Hi Harsha,
> > 
> > On 14/12/2020 09:26, Shamsundara Havanur, Harsha wrote:
> > > On Mon, 2020-12-14 at 09:52 +0100, Jan Beulich wrote:
> > > > CAUTION: This email originated from outside of the
> > > > organization. Do
> > > > not click links or open attachments unless you can confirm the
> > > > sender
> > > > and know the content is safe.
> > > > 
> > > > 
> > > > 
> > > > On 11.12.2020 12:44, Harsha Shamsundara Havanur wrote:
> > > > > A HVM domain flushes cache on all the cpus using
> > > > > `flush_all` macro which uses cpu_online_map, during
> > > > > i) creation of a new domain
> > > > > ii) when device-model op is performed
> > > > > iii) when domain is destructed.
> > > > > 
> > > > > This triggers IPI on all the cpus, thus affecting other
> > > > > domains that are pinned to different pcpus. This patch
> > > > > restricts cache flush to the set of cpus affinitized to
> > > > > the current domain using `domain->dirty_cpumask`.
> > > > 
> > > > But then you need to effect cache flushing when a CPU gets
> > > > taken out of domain->dirty_cpumask. I don't think you/we want
> > > > to do that.
> > > > 
> > > 
> > > If we do not restrict, it could lead to DoS attack, where a
> > > malicious
> > > guest could keep writing to MTRR registers or do a cache flush
> > > through
> > > DM Op and keep sending IPIs to other neighboring guests.
> > 
> > I saw Jan already answered about the alleged DoS, so I will just
> > focus
> > on the resolution.
> > 
> > I agree that in the ideal situation we want to limit the impact on
> > the
> > other vCPUs. However, we also need to make sure the cure is not
> > worse
> > than the symptoms.
> 
> And specifically, only a change which is correct.  This patch very
> definitely isn't.
> 
> Lines can get cached on other cpus from, e.g. qemu mappings and PV
> backends.
> 
> > 
> > The cache flush cannot be restricted in all the pinning situation
> > because pinning doesn't imply the pCPU will be dedicated to a given
> > vCPU or even the vCPU will stick on pCPU (we may allow floating on
> > a
> > NUMA socket). Although your setup may offer this guarantee.
> > 
> > My knowledge in this area is quite limited. But below a few
> > question
> > that hopefully will help to make a decision.
> > 
> > The first question to answer is: can the flush can be restricted in
> > a
> > setup where each vCPUs are running on a decicated pCPU (i.e
> > partionned
> > system)?
> 
> Not really.  Lines can become cached even from speculation in the
> directmap.
> 
> If you need to flush the caches (and don't have a virtual mapping to
> issue clflush/clflushopt/clwb over), it must be on all CPUs.

If lines are cached due to aggressive speculation from a different
guest, wouldn't they be invalidated at the speculation boundary, since
it's a wrong speculation? Would it still require to be flushed
explicitly?

-Harsha

> 
> > If the answer is yes, then we should figure out whether using
> > domain->dirty_cpumask would always be correct? For instance, a vCPU
> > may not have yet run, so can we consider the associated pCPU cache
> > would be consistent?
> > 
> > Another line of question is what can we do on system supporting
> > self-snooping? IOW, would it be possible to restrict the flush for
> > all
> > the setup?
> 
> Right - this is the avenue which ought to be investigated.  (Working)
> self-noop ought to remove the need for some of these cache flushes.
> Others look like they're not correct to begin with.  Others, such as
> the
> wbinvd intercepts absolutely must stay as they are.
> 
> ~Andrew