Excerpts from Fabiano Rosas's message of February 1, 2022 1:51 am:
> Nicholas Piggin <npig...@gmail.com> writes:
>
>> The behaviour of the Address Translation Mode on Interrupt resource is
>> not consistently supported by all CPU versions or all KVM versions. In
>> particular KVM HV only supports mode 0 on POWER7 processors, and does
>> not support mode 2 on any processors. KVM PR only supports mode 0. TCG
>> can support all modes (0,2,3).
>>
>> This leads to inconsistencies in guest behaviour and could cause
>> problems migrating guests.
>>
>> This was not too noticable for Linux guests for a long time because the
>> kernel only used mode 0 or 3, and it used to consider AIL to be somewhat
>> advisory (KVM would not always honor it either) and it kept both sets of
>> interrupt vectors around.
>>
>> Recent Linux guests depend on the AIL mode working as defined by the ISA
>> to support the SCV facility interrupt. If AIL mode 3 can not be provided,
>> then Linux must be given an error so it can disable the SCV facility.
>>
>> Add the ail-modes capability which is a bitmap of the supported values
>> for the H_SET_MODE Address Translation Mode on Interrupt resource. Add
>> a new KVM CAP that exports the same thing, and provide defaults for PR
>> and HV KVM that predate the cap.
>> ---
>>
>> I just wanted to get some feedback on the approach before submitting a
>> patch for the KVM cap.
>
> Could you expand a bit on what is the use case for setting this in the
> QEMU cmdline? I looks to me we already have all the information we need
> with just the KVM cap.
To be able to match TCG with KVM HV or PR behaviour here.
I guess I'm not sure how much that is actually needed though.
>> + if (kvm_enabled()) {
>> + if (val & (0x01 << 2)) {
>> + error_setg(errp, "KVM does not support cap-ail-modes mode
>> AIL=2");
>
> Isn't this something KVM should tell us via the capability?
Yeah, might as well do that. I changed some of the interfaces halfway
through and didn't clean this up.
>> + error_append_hint(errp,
>> + "Ensure bit 2 (value 4) is clear in
>> cap-ail-modes\n");
>> + if (kvmppc_has_cap_ail_3()) {
>> + error_append_hint(errp, "Try appending -machine
>> cap-ail-modes=9\n");
>> + } else {
>> + error_append_hint(errp, "Try appending -machine
>> cap-ail-modes=1\n");
>> + }
>> + return;
>> + }
>> + if ((val & (0x01 << 3)) && !kvmppc_has_cap_ail_3()) {
>> + error_setg(errp, "KVM implementation does not support
>> cap-ail-modes AIL=3");
>> + error_append_hint(errp,
>> + "Ensure bit 3 (value 8) is clear in
>> cap-ail-modes\n");
>> + error_append_hint(errp, "Try appending -machine
>> cap-ail-modes=1\n");
>> + return;
>> + }
>> + }
>> +}
>
> I think the error reporting here is too complex. A user who just wants
> to make their guest start will not bother thinking about binary
> representation. There's also some room for confusion in having three
> numbers present in the error message (bit #, decimal value and AIL
> mode). Imagine dealing with this in a bug report, for instance.
>
> I would just tell outright what the supported values are. Perhaps in a
> little table:
>
> Supported AIL modes:
> AIL = 0 | cap-ail-modes=1
> AIL = 2 | cap-ail-modes=5
> AIL = 3 | cap-ail-modes=9
> AIL = 2&3 | cap-ail-modes=13
>
> We could then make the code a bit more generic. Roughly:
Yeah I didn't like the interface either :P
The nicest option I guess is to be able to give it a list
cap-ail-modes=0,2,3
Maybe there's already some parsing to be able to do that. I'll
look a bit harder.
Thanks,
Nick
