linux-kernel - Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87h7hwd33e.wl-maz@kernel.org>
Date:   Thu, 17 Jun 2021 14:37:25 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Will Deacon <will@...nel.org>
Cc:     Yanan Wang <wangyanan55@...wei.com>,
        Quentin Perret <qperret@...gle.com>,
        Alexandru Elisei <alexandru.elisei@....com>,
        kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Catalin Marinas <catalin.marinas@....com>,
        James Morse <james.morse@....com>,
        Julien Thierry <julien.thierry.kdev@...il.com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Gavin Shan <gshan@...hat.com>, wanghaibin.wang@...wei.com,
        zhukeqian1@...wei.com, yuzenghui@...wei.com
Subject: Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

On Thu, 17 Jun 2021 14:21:16 +0100,
Will Deacon <will@...nel.org> wrote:
> 
> On Thu, Jun 17, 2021 at 01:59:37PM +0100, Marc Zyngier wrote:
> > On Thu, 17 Jun 2021 13:45:57 +0100,
> > Will Deacon <will@...nel.org> wrote:
> > > On Thu, Jun 17, 2021 at 06:58:24PM +0800, Yanan Wang wrote:
> > > > @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
> > > >  		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
> > > >  	}
> > > >  
> > > > +	/* Perform CMOs before installation of the guest stage-2 PTE */
> > > > +	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> > > > +		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> > > > +						granule);
> > > > +
> > > > +	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> > > > +		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
> > > 
> > > One thing I'm missing here is why we need the indirection via mm_ops. Are
> > > there cases where we would want to pass a different function pointer for
> > > invalidating the icache? If not, why not just call the function directly?
> > > 
> > > Same for the D side.
> > 
> > If we didn't do that, we'd end-up having to track whether the guest
> > context requires CMOs with additional flags, which is pretty ugly (see
> > v5 of this series for reference [1]).
> 
> Fair enough, although the function pointers here _are_ being used as
> flags, as they only ever have one of two possible values (NULL or
> the CMO function), so it's a shame to bring in the indirect branch
> as well.

What I hope eventually is to get rid of some of the FWB tracking we
have for the host in the protected case, and use the same abstraction.

> 
> > It also means that we would have to drag the CM functions into the EL2
> > object, something that we don't need with this approach.
> 
> I think it won't be long before we end up with CMO functions at EL2 and
> you'd hope we'd be able to use the same code as EL1 for something like
> that. But I also wouldn't want to put money on it...

It we reach that stage, I'll be happy to try and move these function
into some shared location.

> Anyway, no strong opinion on this, it just jumped out when I skimmed the
> patches.

Thanks,

	M.

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