LKML Archive on
help / color / mirror / Atom feed
From: Raghavendra Rao Ananta <>
To: Andrew Jones <>
Cc: Marc Zyngier <>, James Morse <>,
	Alexandru Elisei <>,
	Suzuki K Poulose <>,
	Paolo Bonzini <>,
	Catalin Marinas <>,
	Will Deacon <>, Peter Shier <>,
	Ricardo Koller <>,
	Oliver Upton <>,
	Reiji Watanabe <>,
	Jing Zhang <>,,,,
Subject: Re: [RFC PATCH v2 04/11] KVM: arm64: Setup a framework for hypercall bitmap firmware registers
Date: Mon, 29 Nov 2021 16:56:19 -0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <20211127172720.zte6wfdguoyi3gd6@gator.home>

On Sat, Nov 27, 2021 at 9:27 AM Andrew Jones <> wrote:
> On Tue, Nov 23, 2021 at 10:34:23AM -0800, Raghavendra Rao Ananta wrote:
> > On Mon, Nov 22, 2021 at 9:23 AM Marc Zyngier <> wrote:
> > > I keep being baffled by this. Why should we track the VMM accesses or
> > > the VMM writeback? This logic doesn't seem to bring anything useful as
> > > far as I can tell. All we need to ensure is that what is written to
> > > the pseudo-register is an acceptable subset of the previous value, and
> > > I cannot see why this can't be done at write-time.
> > >
> > > If you want to hide this behind a capability, fine (although my guts
> > > feeling is that we don't need that either). But I really want to be
> > > convinced about all this tracking.
> > >
> > The tracking of each owner register is necessary here to safe-guard
> > the possibility that the user-space may not be aware of a newly
> > introduced register, and hence, hasn't accessed it. If it had at least
> > read the register, but not write-back, we assume that the user-space
> > is happy with the configuration. But the fact that the register has
> > not even been read would state that user-space is unaware of the
> > existence of this new register. In such a case, if we don't sanitize
> > (clear all the bits) this register, the features will be exposed
> > unconditionally to the guest.
> >
> > The capability is introduced here to make sure that this new
> > infrastructure is backward compatible with old VMMs. If the VMMs don't
> > enable this capability, they are probably unaware of this, and this
> > will work as it always has- expose new services to the guest
> > unconditionally as and when they are introduced.
> Hi Raghavendra,
> I don't think we need a CAP that has to be enabled or to make any
> assumptions or policy decisions in the kernel. I think we just need to
> provide a bit more information to the VMM when it checks if KVM has the
> CAP. If KVM would tell the VMM how may pseudo registers there are, which
> can be done with the return value of the CAP, then the VMM code could be
> something like this
>   r = check_cap(KVM_CAP_ARM_HVC_FW_REG_BMAP);
>   if (r) {
>     num_regs = r;
>     for (idx = 0; idx < num_regs; ++idx) {
>       reg = hvc_fw_reg(idx);
>       if (idx > vmm_last_known_idx) {
>         ...
>       } else {
>         ...
>       }
>     }
>   }
> With this, the VMM is free to decide if it wants to clear all registers
> greater than the last index it was aware of or if it wants to let those
> registers just get exposed to the guest without knowing what's getting
> exposed. Along with documenting that by default everything gets exposed
> by KVM, which is the backwards compatible thing to do, then the VMM has
> been warned and given everything it needs to manage its guests.
Hi Andrew,

Thanks for your comments and suggestions!

I like the idea of sharing info via a read of the CAP, and not having
to explicitly sanitize/clear the registers before the guest begins to
However the handshake is done over an API doc, which is a little
concerning. The user-space must remember and explicitly clear any new
register that it doesn't want to expose to the guest, while the
current approach does this automatically.
Any bug in VMM's implementation could be risky and unintentionally
expose features to the guest. What do you think?

> Another thing that might be nice is giving userspace control of how many
> pseudo registers show up in get-reg-list. In order to migrate from a host
> with a more recent KVM to a host with an older KVM[*] we should only
> expose the number of pseudo registers that the older host is aware of.
> The VMM would zero these registers out anyway, in order to be compatible
> for migration, but that's not enough when they also show up in the list
> (at least not with QEMU that aborts migration when the destination
> expects less registers than what get-reg-list provides)
> [*] This isn't a great idea, but it'd be nice if we can make it work,
> because users may want to rollback upgrades or, after migrating to a
> host with a newer kernel, they may want to migrate back to where they
> started.
Good point. But IIUC, if the user-space is able to communicate the
info that it's expecting a certain get-reg-list, do you think it can
handle it at its end too, rather than relying on the kernel to send a
list back?

My assumption was that VMM would statically maintain a known set of
registers that it wants to work with and are to be modified by hand,
rather than relying on get-reg-list. This could be the least common
set of registers that are present in all the host kernels (higher or
lower versions) of the migration fleet. This config doesn't change
even with get-reg-list declaring a new register as the features
exposed by it could still be untested. Although, migrating to a host
with a missing register shouldn't be possible in this setting, but if
it encounters the scenario, it should be able to avoid migration to
the host (similar to QEMU).

Please correct me if you think it's a false assumption to proceed with.


> Thanks,
> drew

  reply	other threads:[~2021-11-30  0:56 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-13  1:22 [RFC PATCH v2 00/11] KVM: arm64: Add support for hypercall services selection Raghavendra Rao Ananta
2021-11-13  1:22 ` [RFC PATCH v2 01/11] KVM: arm64: Factor out firmware register handling from psci.c Raghavendra Rao Ananta
2021-11-27 13:16   ` Andrew Jones
2021-11-30  0:57     ` Raghavendra Rao Ananta
2021-11-13  1:22 ` [RFC PATCH v2 02/11] KVM: Introduce kvm_vcpu_has_run_once Raghavendra Rao Ananta
2021-11-22 16:27   ` Marc Zyngier
2021-11-23 18:31     ` Raghavendra Rao Ananta
2021-11-13  1:22 ` [RFC PATCH v2 03/11] KVM: Introduce kvm_vm_has_run_once Raghavendra Rao Ananta
2021-11-22 16:31   ` Marc Zyngier
2021-11-23 18:48     ` Raghavendra Rao Ananta
2021-11-13  1:22 ` [RFC PATCH v2 04/11] KVM: arm64: Setup a framework for hypercall bitmap firmware registers Raghavendra Rao Ananta
2021-11-22 17:23   ` Marc Zyngier
2021-11-23 18:34     ` Raghavendra Rao Ananta
2021-11-27 17:27       ` Andrew Jones
2021-11-30  0:56         ` Raghavendra Rao Ananta [this message]
2021-11-30 10:19           ` Andrew Jones
2021-11-13  1:22 ` [RFC PATCH v2 05/11] KVM: arm64: Add standard hypervisor firmware register Raghavendra Rao Ananta
2021-11-13  1:22 ` [RFC PATCH v2 06/11] KVM: arm64: Add vendor " Raghavendra Rao Ananta
2021-11-13  1:22 ` [RFC PATCH v2 07/11] Docs: KVM: Add doc for the bitmap firmware registers Raghavendra Rao Ananta
2021-11-13  1:22 ` [RFC PATCH v2 08/11] Docs: KVM: Rename psci.rst to hypercalls.rst Raghavendra Rao Ananta
2021-11-13  1:22 ` [RFC PATCH v2 09/11] tools: Import ARM SMCCC definitions Raghavendra Rao Ananta
2021-11-13  1:22 ` [RFC PATCH v2 10/11] selftests: KVM: aarch64: Introduce hypercall ABI test Raghavendra Rao Ananta
2021-11-13  1:22 ` [RFC PATCH v2 11/11] selftests: KVM: aarch64: Add the bitmap firmware registers to get-reg-list Raghavendra Rao Ananta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='' \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [RFC PATCH v2 04/11] KVM: arm64: Setup a framework for hypercall bitmap firmware registers' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).