Re: [PATCH v2 04/14] spapr: nested: Introduce cap-nested-papr for Nested PAPR API

[Nicholas Piggin]

On Thu Nov 30, 2023 at 4:19 PM AEST, Harsh Prateek Bora wrote:
>
>
> On 11/29/23 09:31, Nicholas Piggin wrote:
> > On Thu Oct 12, 2023 at 8:49 PM AEST, Harsh Prateek Bora wrote:
> >> Introduce a SPAPR capability cap-nested-papr which provides a nested
> >> HV facility to the guest. This is similar to cap-nested-hv, but uses
> >> a different (incompatible) API and so they are mutually exclusive.
> >> This new API is to enable support for KVM on PowerVM and recently the
> >> Linux kernel side patches have been accepted upstream as well [1].
> >> Support for related hcalls is being added in next set of patches.
> > 
> > We do want to be able to support both APIs on a per-guest basis. It
> > doesn't look like the vmstate bits will be a problem, both could be
> > enabled if the logic permitted it and that wouldn't cause a
> > compatibility problem I think?
> > 
>
> I am not sure if it makes sense to have both APIs working in parallel 
> for a nested guest.

Not for the nested guest, but for the nested KVM host (i.e., the direct
pseries guest running QEMU). QEMU doesn't know ahead of time which API
might be used by the OS.

> Former uses h_enter_guest and expects L1 to store 
> most of the regs, and has no concept like GSB where the communication 
> between L1 and L0 takes place in a standard format which is used at 
> nested guest exit also. Here, we have separate APIs for guest/vcpu 
> create and then do a run_vcpu for a specific vcpu. So, we cant really 
> use both APIs interchangeably while running a nested guest. BTW, L1 
> kernel uses only either of the APIs at a time, preferably this one if 
> supported.

Yeah not on the same guest. And it's less about running two different
APIs on different guests with the same L1 simultaneously (although we
could probably change KVM to support that fairly easily, and we might
want to for testing purposes), but more about compatibility. What if
we boot or exec into an old kernel that doesn't support the new API?
>
> > And it's a bit of a nitpick, but the capability should not be permitted
> > before the actual APIs are supported IMO. You could split this into
> > adding .api first, so the implementation can test it, and add the spapr
> > caps at the end.
> > 
>
> Agree, I shall update as suggested.

Thanks,
Nick