Re: [PATCH v3] KVM: PPC: Book3S HV: Validate arch_compat against host compatibility mode

[Amit Machhiwal]

On 2026/06/16 05:38 PM, Ritesh Harjani wrote:
> Amit Machhiwal <amachhiw@xxxxxxxxxxxxx> writes:
> 
> >> > diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> >> > index 3449dd2b577d..7472b9522f71 100644
> >> > --- a/arch/powerpc/include/asm/reg.h
> >> > +++ b/arch/powerpc/include/asm/reg.h
> >> > @@ -1356,6 +1356,7 @@
> >> >  #define PVR_ARCH_300	0x0f000005
> >> >  #define PVR_ARCH_31	0x0f000006
> >> >  #define PVR_ARCH_31_P11	0x0f000007
> >> > +#define PVR_ARCH_INVALID	0xffffffff
> >> 
> >> Logical processor version is defined as part of the PAPR spec. We should
> >> ensure that this invalid PVR is also documented in the PAPR spec.
> >> 
> >> If you have already taken care of that, then please confirm and feel free to add:
> >
> > Regarding the PAPR specification documentation: The PAPR spec documents
> > the valid Processor Version Register (PVR) values for each processor
> > generation (POWER8, POWER9, POWER10, POWER11, etc.). However, the
> > PVR_ARCH_INVALID value (0xffffffff) introduced in this patch series is a
> > KVM implementation detail used internally to mark invalid compatibility
> > mode requests - it's not an architectural value that would be defined in
> > PAPR itself.
> >
> > The validation logic and the use of PVR_ARCH_INVALID as a sentinel value
> > are documented in the kernel code and commit message.
> >
> 
> But that still worries me on what if PAPR wants to re-use this value for
> some other purpose in future. 

This is a valid concern about potential future conflicts with PAPR.
However, I'd like to point out that PAPR explicitly specifies:

  "The first byte of the logical processor version value shall be 0x0F."

Since PVR_ARCH_INVALID (0xffffffff) has a first byte of 0xFF, it's
explicitly outside the valid PAPR-defined range for logical PVR values.
This means there shouldn't be any risk of future conflict with PAPR
specifications.

Please let me know what you think about this?

Thanks,
Amit

> 
> BTW, thinking more about it, if we purely want this to be in kernel only,
> can we instead add, something like:
> 
>      bool kpvr_compat;   /* Does kernel supports this PVR */
> 
> rather than re-using & overloading arch_compat which has values that
> comes from PAPR spec?
> 
> Thoughts?
> 
> -ritesh