Re: more /proc/cpuinfo & extension support related woes

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 4465 bytes --]

On Wed, Jul 26, 2023 at 06:06:52PM +0100, Conor Dooley wrote:
> On Wed, Jul 26, 2023 at 10:07:52PM +0530, Anup Patel wrote:
> > On Wed, Jul 26, 2023 at 4:11 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> > >
> > > On Wed, Jul 26, 2023 at 11:04:39AM +0200, Andrew Jones wrote:
> > > > On Tue, Jul 25, 2023 at 06:19:36PM +0100, Conor Dooley wrote:
> 
> > > >  - The kernel will combine information from one of the two bitmaps listed
> > > >    above with other information, such as config information, to decide
> > > >    when / if an extension should be used by the kernel and/or exposed to
> > > >    userspace
> > >
> > > Right. This bit (or rather two bits, since I view the kernel and
> > > userspace bits here separately) is where we are falling short. I think
> > > hwprobe's limited users do the right thing, but we're not doing this for
> > > /proc/cpuinfo. For the in-kernel users, grepping shows some suspicious
> > > looking things, but how many are problematic I do not yet know. As an
> > > example, does
> > > static void kvm_riscv_vcpu_update_config(const unsigned long *isa)
> > > {
> > >         u64 henvcfg = 0;
> > >
> > >         if (riscv_isa_extension_available(isa, SVPBMT))
> > >                 henvcfg |= ENVCFG_PBMTE;
> > >
> > > work correctly if the SVPBMT Kconfig option is disabled?
> > > From a quick check, `isa` here is set from the host ISA
> > >         /* Setup ISA features available to VCPU */
> > >         for (i = 0; i < ARRAY_SIZE(kvm_isa_ext_arr); i++) {
> > >                 host_isa = kvm_isa_ext_arr[i];
> > >                 if (__riscv_isa_extension_available(NULL, host_isa) &&
> > >                     kvm_riscv_vcpu_isa_enable_allowed(i))
> > >                         set_bit(host_isa, vcpu->arch.isa);
> > >         }
> > > so if the check passes for the host ISA, it'll pass for the guest ISA
> > > too, so henvcfg will end up with the PBMTE bit set. I just haven't yet
> > > checked what the outcome of this will be, but I figure not good?
> > 
> > Why is the outcome not good?
> > 
> > Guest need Svpbmt to support pass-through devices.
> 
> It was an example of a case where something is using
> __riscv_isa_extension_available() where the behaviour of the kernel
> w.r.t. this extension changes based on a Kconfig option being set.
> I get why a guest wants to know about Svpbmt, that's not why I mentioned
> this bit of code.
> 
> > > > (it'd be good to have a consistent API for these types of
> > > >    checks which combine extension presence with other information)
> > >
> > > I figure the best option might just be to make the
> > > __riscv_isa_extension_available() into this to avoid disruption? The
> > > vast majority of users of that function want to know whether or not it
> > > is safe to use the extension on that hart, not whether the hart itself
> > > supports it. In fact, outside of Evan's per-hart stuff in /proc/cpuinfo,
> > > do we have any users that would even want an API that checks the
> > > "unfiltered" versions? I'll have to check the users to answer that I
> > > think.
> > 
> > Each guest VCPU will have its own ISA bitmap so KVM needs the
> > __riscv_isa_extension_available() to check VCPU ISA bitmap.
> 
> That's not the unfiltered list though, you've assembled this specifically
> for your VCPUs based on what KVM knows it can offer. It's the harts in
> the host kernel that I am mainly talking about here and, in particular,
> the behaviour when NULL is passed as the first argument. Changes can be
> made to how __riscv_isa_extension_available() in that regard without
> impacting KVM, which is mostly what I meant by "avoid disruption".
> Either way, AFAICT, KVM doesn't offer vector support when
> CONFIG_RISCV_ISA_V is disabled or fpu support without CONFIG_FPU (AFAICT).
> Other than Svpbmt, those are all that the filtered list would remove from
> the host isa bitmap anyway.

Replying to myself since I looked more at the kernel's use of the
Kconfig option for Svpbmt. I was hoping to axe it, but that can't be
done since alternatives are involved - however, I think it could be
hidden from user-visibility entirely & always enabled for kernels that
support it. That'd just leave nommu, XIP kernels as the cases where
the host kernel support would not be enabled, so the impact to KVM of
such a change would be rather minimal.

> Hope that clarifies things,
> Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv