[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: |
David Gibson |
Subject: |
Re: [RFC PATCH] spapr: Add SPAPR_CAP_AIL_MODES for supported AIL modes for H_SET_MODE hcall |
Date: |
Mon, 7 Feb 2022 12:41:15 +1100 |
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.
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).
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature