[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH] spapr: Add SPAPR_CAP_AIL_MODES for supported AIL modes f
From: |
Nicholas Piggin |
Subject: |
Re: [RFC PATCH] spapr: Add SPAPR_CAP_AIL_MODES for supported AIL modes for H_SET_MODE hcall |
Date: |
Mon, 14 Feb 2022 17:54:58 +1000 |
Excerpts from David Gibson's message of February 7, 2022 11:41 am:
> On Sat, Jan 29, 2022 at 04:50:07PM +1000, Nicholas Piggin wrote:
>> 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.
>>
>> The reason I don't make that a boolean cap for AIL=3 is that future
>> processors might implement new modes a guest would like to use even
>> though it's not the nicest interface.
>
> [snip]
>> SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>> [SPAPR_CAP_HTM] = {
>> .name = "htm",
>> @@ -730,6 +802,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>> .type = "bool",
>> .apply = cap_rpt_invalidate_apply,
>> },
>> + [SPAPR_CAP_AIL_MODES] = {
>> + .name = "ail-modes",
>> + .description = "Bitmap of AIL (alternate interrupt location) mode
>> support",
>
> A bitmap doesn't quite work as an spapr cap. The general caps code
> assumes that bigger is always better, or more precisely that migrating
> from an instance that has a lower value to one which has a higher
> value is "good enough" to be compatible. That's obviously not the
> case for a bitmap.
Yeah, it was clearly a nasty wart :P
> I think to handle this properly within the limitations of papr caps,
> you instead want a separate boolean cap for each supported AIL mode
> (or at least for each AIL mode you want to have control over).
Oh that's a good idea. We could just do ail-mode-3 for now.
2 is supported by previous processors, but is now deprecated. I don't
think AIX ever used it although it (or something else) may have. Even
KVM never really implemented it correctly. Although I think TCG does.
So in theory we could be causing a regression if we leave out mode 2,
although it should be easy to re-add (we can leave the support in TCG
for a while and it's not much work anyway).
I'll try with just 3 as the optional cap. Should make it a lot cleaner.
Thanks,
Nick