[PATCH v2 2/6] arm64: KVM: Handle Set/Way CMOs as NOPs if FWB is present

[marc.zyngier@arm.com (Marc Zyngier)]

On Sat, 09 Jun 2018 10:26:40 +0100,
Christoffer Dall wrote:
> 
> On Wed, May 30, 2018 at 01:47:02PM +0100, Marc Zyngier wrote:
> > Set/Way handling is one of the ugliest corners of KVM. We shouldn't
> > have to handle that, but better safe than sorry.
> > 
> > Thankfully, FWB fixes this for us by not requiering any maintenance
> > whatsoever, which means we don't have to emulate S/W CMOs, and don't
> > have to track VM ops either.
> 
> I tiny bit of rationale here would have been nice.  As I understand it,
> if we're presenting the guest with a fully coherent system, there should
> never be a need to invalidate anything, because the guest will always
> see the most recent value no matter how it sings and dances, right?

The guest may not even know about the "fully coherent system". It may
continue to issue its CMOs as before, not realising that they are not
required.

> > 
> > We still have to trap S/W though, if only to prevent the guest from
> > doing something bad.
> > 
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 6e3b969391fd..9a740f159245 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -195,7 +195,13 @@ static bool access_dcsw(struct kvm_vcpu *vcpu,
> >  	if (!p->is_write)
> >  		return read_from_write_only(vcpu, p, r);
> >  
> > -	kvm_set_way_flush(vcpu);
> > +	/*
> > +	 * Only track S/W ops if we don't have FWB. It still indicates
> > +	 * that the guest is a bit broken...
> > +	 */
> 
> Is it strictly true that the guest is broken if it does any form of S/W
> ops?  Does the guest actually know that it's running on a fully coherent
> system, or is the argument that no software, ever, should do S/W, even
> for reboot etc.?

S/W should really only be used in power-management scenario. I really
cannot think of a single valid (or even safe) reason to issue a S/W
operation outside of PM, when you're guaranteed that there is only a
single CPU up and running. A guest OS cannot enforce this requirement,
so that's really always broken.

> I think this should have slightly more info, or that part of the comment
> should just be dropped, to avoid misleading future readers who don't
> have the full picture.

Happy to add more details when I respin this series.

> 
> > +	if (!cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
> > +		kvm_set_way_flush(vcpu);
> > +
> >  	return true;
> >  }
> >  
> > -- 
> > 2.17.1
> > 
> 
> Besides the usual nits on commentary:
> 
> Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.