On 2026/06/03 02:02 PM, Amit Machhiwal wrote: > Hi Vaibhav, > > Thanks for taking a look at this patch. My response is inline: > > On 2026/06/03 12:27 PM, Vaibhav Jain wrote: > > Hi Amit, > > > > Thanks for the patch. My review comments inline below: > > > > Amit Machhiwal <[email protected]> writes: > > > > > During H_CLIENT_ARCHITECTURE_SUPPORT, QEMU attempts to set the requested > > > CPU compatibility mode via ppc_set_compat_all(). If this fails, the > > > existing code may fall back to raw mode when supported by the guest. > > > > > > The current logic treats all failures uniformly and tried to fall back > > > to raw mode whenever possible. As a result, errors from KVM (e.g. > > > kvmppc_set_compat() returning -EINVAL) are silently masked, making > > > debugging difficult and allowing the guest to proceed with an > > > unsupported or incompatible configuration. > > > > > > When running with KVM, such failures indicate that the requested > > > compatibility level is not supported by the host. Do not fallback in > > > this case; instead, report the error and fail the CAS negotiation. > > > > > > This makes the failure visible to the user and ensures that invalid > > > compatibility requests are rejected early rather than being hidden by > > > fallback behavior. For example, the following error is now reported: > > > > > > qemu-system-ppc64: Unable to set CPU compatibility mode in KVM: Invalid > > > argument > > > > > > Fixes: cc7b35b169e9 ("spapr: fallback to raw mode if best compat mode > > > cannot be set during CAS") > > > Signed-off-by: Amit Machhiwal <[email protected]> > > > Tested-by: Anushree Mathur <[email protected]> > > > --- > > > hw/ppc/spapr_hcall.c | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > > > index 032805a8d0db..de2ea6f5e579 100644 > > > --- a/hw/ppc/spapr_hcall.c > > > +++ b/hw/ppc/spapr_hcall.c > > > @@ -1204,6 +1204,15 @@ target_ulong > > > do_client_architecture_support(PowerPCCPU *cpu, > > > Error *local_err = NULL; > > > > > > if (ppc_set_compat_all(cas_pvr, &local_err) < 0) { > > > + /* > > > + * If KVM rejected the compat mode, do not fallback. This > > > indicates > > > + * the host cannot support the requested level. > > > + */ > > > + if (kvm_enabled()) { > > > + error_report_err(local_err); > > > + return H_HARDWARE; > > > + } > > > + > > > > NAK on this change. > > > > As per PAPR whether the guest can run in RAW mode needs to be negotiated > > between Guest and VMM. The existing code path via cas_check_pvr() and > > 'raw_mode_supported' already handles this case. > > > > So I dont see a reason for this KVM specific changes needed. Can you > > please elaborate a bit more on the issue that gets fixed by this change. > > I have mentioned the reason in the commit message. I'm quoting the part > of it below: > > The current logic treats all failures uniformly and tried to fall back > to raw mode whenever possible. As a result, errors from KVM (e.g. > kvmppc_set_compat() returning -EINVAL) are silently masked, making > debugging difficult and allowing the guest to proceed with an > unsupported or incompatible configuration. > > The currently call chain is as below: > > do_client_architecture_support() > -> ppc_set_compat_all() > -> run_on_cpu() > -> do_run_on_cpu() > -> do_set_compat() > -> ppc_set_compat() > -> kvmppc_set_compat() // returns error > > Now when an error is returned via kvmppc_set_comapt(), its just ignored > in the current situation silently in do_client_architecture_support(), > while the caller is supposed to handle the error returned in an errp > object. This would make the guest try to continue with a raw mode when > the host was actually booted in a compatibility mode, while it shouldn't > be doing that.
Based on an off-mailing list discussion, I'll be working on discussed alternate approach for this. Will post next version based on how that goes. Thanks for the discussion, Vaibhav. Thanks, Amit
