Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 28 Apr 2023 12:28:24 +0200
From:   Andrew Jones <ajones@ventanamicro.com>
To:     Conor Dooley <conor@kernel.org>
Cc:     Heiko =?utf-8?Q?St=C3=BCbner?= <heiko@sntech.de>,
        Conor Dooley <conor.dooley@microchip.com>, palmer@dabbelt.com,
        linux-riscv@lists.infradead.org, paul.walmsley@sifive.com,
        kito.cheng@sifive.com, jrtc27@jrtc27.com, matthias.bgg@gmail.com,
        heinrich.schuchardt@canonical.com, greentime.hu@sifive.com,
        nick.knight@sifive.com, christoph.muellner@vrull.eu,
        philipp.tomsich@vrull.eu, richard.henderson@linaro.org,
        arnd@arndb.de, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/4] RISC-V: add support for vendor-extensions via
 AT_BASE_PLATFORM and xthead
Message-ID: <d6lqggv2bbtmv7exalcqqwgbntibdxotsswataxqxhx6kmf4rg@5kgb3axdwgst>
References: <20230424194911.264850-1-heiko.stuebner@vrull.eu>
 <20230424194911.264850-5-heiko.stuebner@vrull.eu>
 <20230426-spirits-ludicrous-a5d8275686e6@wendy>
 <5016896.Mh6RI2rZIc@diego>
 <20230427-maybe-skier-51e7cf09795c@spud>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20230427-maybe-skier-51e7cf09795c@spud>
Precedence: bulk

On Thu, Apr 27, 2023 at 07:28:49PM +0100, Conor Dooley wrote:
> Hey Heiko,
> 
> On Thu, Apr 27, 2023 at 07:15:58PM +0200, Heiko St?bner wrote:
> > Am Mittwoch, 26. April 2023, 14:29:16 CEST schrieb Conor Dooley:
> > > On Mon, Apr 24, 2023 at 09:49:11PM +0200, Heiko Stuebner wrote:
> > > > From: Heiko Stuebner <heiko.stuebner@vrull.eu>
...
> > > What do you mean by virtualisation here? It's the job of the hypervisor
> > > etc to make sure that what it passes to its guest contains only what it
> > > wants the guest to see, right?
> > > IIUC, that's another point against doing what this patch does.
> > 
> > I guess I'm still seeing Zbb and friends - with just computational
> > instructions as always good to have. But I guess you're right that the
> > hypervisor should be able to control itself which extensions.
> 
> Yah, there may not be any obvious downsides to something like Zbb, but I
> think that taking control away from the hypervisors etc isn't a good
> idea.

If there's any chance that a VM will need to migrate from a host with,
e.g. Zbb, to one without it, then the VM will need Zbb disabled from the
start.

> Having a simple policy of blocking things that are known to misbehave
> would require less maint. than a list of things that are okay to pass
> through, but both are probably cans-of-worms.
> I think we need to think carefully about what policy is chosen here.
> Allowlist will be slower, but at least we'll not tell userspace
> something that is not usable. Blocklist will be easier to manage, but
> can only be reactive.

I have experience [trying] to maintain deny-lists for CPU features,
both for x86 Xen guests and Arm KVM guests. I don't recommend it. To
do it right, you need to be proactive, tracking upcoming CPU features
to add the ones that can't be supported by virt or aren't ready to
be supported by virt to the deny-list before somebody trips over them.
In practice, usually somebody trips over it first, causing fires which
have to be put out. If an allow-list is used, then, when a new feature
is missed, no fires are started. The worst that can happen is somebody
expected the feature and didn't see it, so they complain, at which
point you add it.

...

> Also, in a world where we do do some sort of passing, should we only
> forward the vendor extensions, or should we forward the standard ones
> too?

I guess we need to forward anything userspace can and should use.

> What about supervisor mode only stuff?

That's not something userspace can use. If we want to expose which
supervisor mode features the CPU has to userspace, for information
purposes, then I think proc or sysfs would be sufficient for that.

The downside of using an allow-list for what extensions get exposed
to userspace is that even extensions the kernel can't/won't use
will need a kernel patch before userspace can use them. But, as
I stated above, that downside (people complaining a feature they
expect is missing), is, IMO, better than the alternative of exposing
things that shouldn't be.

Thanks,
drew