Re: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID register

[Marc Zyngier <maz@kernel.org>]

On Mon, 09 Sep 2024 18:16:55 +0100,
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:
> 
> 
> 
> > -----Original Message-----
> > From: Oliver Upton <oliver.upton@linux.dev>
> > Sent: Monday, September 9, 2024 5:57 PM
> > To: Marc Zyngier <maz@kernel.org>
> > Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> > kvmarm@lists.linux.dev; Sebastian Ott <sebott@redhat.com>; James Morse
> > <james.morse@arm.com>; Suzuki K Poulose <suzuki.poulose@arm.com>;
> > yuzenghui <yuzenghui@huawei.com>; kvm@vger.kernel.org; Shaoqin Huang
> > <shahuang@redhat.com>; Eric Auger <eric.auger@redhat.com>; Wangzhou
> > (B) <wangzhou1@hisilicon.com>
> > Subject: Re: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID
> > register
> > 
> > On Mon, Sep 09, 2024 at 05:28:48PM +0100, Marc Zyngier wrote:
> > > Hi Shameer,
> > >
> > > On Mon, 09 Sep 2024 16:19:54 +0100,
> > > Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> > wrote:
> > > >
> > > > Hi Oliver/Sebastian,
> > > >
> > > > > -----Original Message-----
> > > > > From: Oliver Upton <oliver.upton@linux.dev>
> > > > > Sent: Wednesday, June 19, 2024 6:41 PM
> > > > > To: kvmarm@lists.linux.dev
> > > > > Cc: Marc Zyngier <maz@kernel.org>; James Morse
> > > > > <james.morse@arm.com>; Suzuki K Poulose
> > <suzuki.poulose@arm.com>;
> > > > > yuzenghui <yuzenghui@huawei.com>; kvm@vger.kernel.org; Sebastian
> > > > > Ott <sebott@redhat.com>; Shaoqin Huang <shahuang@redhat.com>;
> > Eric
> > > > > Auger <eric.auger@redhat.com>; Oliver Upton
> > > > > <oliver.upton@linux.dev>
> > > > > Subject: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM
> > > > > feature ID register
> > > >
> > > > [...]
> > > >
> > > > > @@ -2487,7 +2490,10 @@ static const struct sys_reg_desc
> > > > > sys_reg_descs[] = {
> > > > >  	{ SYS_DESC(SYS_CCSIDR2_EL1), undef_access },
> > > > >  	{ SYS_DESC(SYS_SMIDR_EL1), undef_access },
> > > > >  	{ SYS_DESC(SYS_CSSELR_EL1), access_csselr, reset_unknown,
> > > > > CSSELR_EL1 },
> > > > > -	{ SYS_DESC(SYS_CTR_EL0), access_ctr },
> > > > > +	ID_WRITABLE(CTR_EL0, CTR_EL0_DIC_MASK |
> > > > > +			     CTR_EL0_IDC_MASK |
> > > > > +			     CTR_EL0_DminLine_MASK |
> > > > > +			     CTR_EL0_IminLine_MASK),
> > > >
> > > > (Sorry if this was discussed earlier, but I couldn't locate it
> > > > anywhere.)
> > > >
> > > > Is there a reason why we can't make the L1Ip writable as well here?
> > > > We do have hardware that reports VIPT and PIPT for L11p.
> > > >
> > > > The comment here states,
> > > > https://elixir.bootlin.com/linux/v6.11-rc7/source/arch/arm64/kernel/
> > > > cpufeature.c#L489
> > > >
> > > > " If we have differing I-cache policies, report it as the weakest - VIPT."
> > > >
> > > > Does this also mean it is safe to downgrade the PIPT to VIPT for Guest as
> > well?
> > >
> > > It should be safe, as a PIPT CMO always does at least the same as
> > > VIPT, and potentially more if there is aliasing.
> > 
> > +1, there was no particular reason why this wasn't handled before.
> > 
> > We should be careful to only allow userspace to select VIPT or PIPT (where
> > permissible), and not necessarily any value lower than what's reported by
> > hardware.
> 
> VIPT 0b10
> PIPT 0b11
> 
> Ok. Just to clarify that " not necessarily any value lower than what's reported by
> hardware" means userspace can set PIPT if hardware supports VIPT?

No, exactly the opposite. If the HW advertises VIPT, we can set
anything else. If the HW advertises PIPT, we can lie to the guest and
set VIPT. And if the HW advertises any of the two reserved values, it
can but in hell... ;-)

> Based on this,
> " If we have differing I-cache policies, report it as the weakest - VIPT." , I was thinking
> the other way around(see "safe to downgrade PIPT to VIPT"). But Marc also
> seems to suggest PIPT CMO ends up doing atleast same as VIPT and more, so it looks like
> the other way. If that's the case, what does that "report it as the weakest" means for host?

"report it as the weakest" means that if you have a machine with two
sets of CPUs, one implementing PIPT and the other VIPT, you report the
crappiest of the two because SW cannot know which CPU it is running
on.

With that, SW running with VIPT on PIPT will do a bit extra work, but
will still work fine.

	M.

-- 
Without deviation from the norm, progress is not possible.