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. If this change is related to your Linux kernel patch series titled "KVM: PPC: Handle CPU compatibility mode for nested guests" then the ideal place to put compat check would be in ppc_check_compat() and not in do_client_architecture_support() > /* We fail to set compat mode (likely because running with KVM > PR), > * but maybe we can fallback to raw mode if the guest supports > it. > */ > -- > 2.50.1 (Apple Git-155) > -- Cheers ~ Vaibhav
