Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 24 Jun 2022 14:12:01 +0100
Message-ID: <87iloq2oke.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Gavin Shan <gshan@redhat.com>
Cc:     kvmarm@lists.cs.columbia.edu, shijie@amperemail.onmicrosoft.com,
        linux-kernel@vger.kernel.org, eauger@redhat.com,
        shan.gavin@gmail.com, Jonathan.Cameron@huawei.com,
        pbonzini@redhat.com, vkuznets@redhat.com, will@kernel.org,
        oliver.upton@linux.dev, Oliver Upton <oupton@google.com>,
        James Morse <james.morse@arm.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Catalin Marinas <catalin.marinas@arm.com>
Subject: Re: [PATCH v7 00/22] Support SDEI Virtualization
In-Reply-To: <6bdb9280-3530-dc1f-d33e-5bc1c5ac927b@redhat.com>
References: <20220527080253.1562538-1-gshan@redhat.com>
        <6bdb9280-3530-dc1f-d33e-5bc1c5ac927b@redhat.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

Hi Gavin,

On Thu, 23 Jun 2022 07:11:08 +0100,
Gavin Shan <gshan@redhat.com> wrote:
> 
> Hi Oliver,
> 
> On 5/27/22 6:02 PM, Gavin Shan wrote:
> > This series intends to virtualize Software Delegated Exception Interface
> > (SDEI), which is defined by DEN0054C (v1.1). It allows the hypervisor to
> > deliver NMI-alike SDEI event to guest and it's needed by Async PF to
> > deliver page-not-present notification from hypervisor to guest. The code
> > and the required qemu changes can be found from:
> > 
> >     https://developer.arm.com/documentation/den0054/c
> >     https://github.com/gwshan/linux    ("kvm/arm64_sdei")
> >     https://github.com/gwshan/qemu     ("kvm/arm64_sdei")
> > 
> > The design is quite strightforward by following the specification. The
> > (SDEI) events are classified into the shared and private ones according
> > to their scope. The shared event is system or VM scoped, but the private
> > event is vcpu scoped. This implementation doesn't support the shared
> > event because all the needed events are private. Besides, the critial
> > events aren't supported by the implementation either. It means all events
> > are normal in terms of priority.
> > 
> > There are several objects (data structures) introduced to help on the
> > event registration, enablement, disablement, unregistration, reset,
> > delivery and handling.
> > 
> >    * kvm_sdei_event_handler
> >      SDEI event handler, which is provided through EVENT_REGISTER
> >      hypercall, is called when the SDEI event is delivered from
> >      host to guest.
> >         * kvm_sdei_event_context
> >      The saved (preempted) context when SDEI event is delivered
> >      for handling.
> >         * kvm_sdei_vcpu
> >      SDEI events and their states.
> > 
> > The patches are organized as below:
> > 
> >    PATCH[01-02] Preparatory work to extend smccc_get_argx() and refactor
> >                 hypercall routing mechanism
> >    PATCH[03]    Adds SDEI virtualization infrastructure
> >    PATCH[04-16] Supports various SDEI hypercalls and event handling
> >    PATCH[17]    Exposes SDEI capability
> >    PATCH[18-19] Support SDEI migration
> >    PATCH[20]    Adds document about SDEI
> >    PATCH[21-22] SDEI related selftest cases
> > 
> > The previous revisions can be found:
> > 
> >    v6: https://lore.kernel.org/lkml/20220403153911.12332-4-gshan@redhat.com/T/
> >    v5: https://lore.kernel.org/kvmarm/20220322080710.51727-1-gshan@redhat.com/
> >    v4: https://lore.kernel.org/kvmarm/20210815001352.81927-1-gshan@redhat.com/
> >    v3: https://lore.kernel.org/kvmarm/20210507083124.43347-1-gshan@redhat.com/
> >    v2: https://lore.kernel.org/kvmarm/20210209032733.99996-1-gshan@redhat.com/
> >    v1: https://lore.kernel.org/kvmarm/20200817100531.83045-1-gshan@redhat.com/
> > 
> 
> Copying Oliver's new email address (oliver.upton@linux.dev).
> 
> Please let me know if I need to rebase and repost the series.

My main issue with this series is that it is a solution in search of a
problem. It is only an enabler for Asynchronous Page Fault support,
and:

- as far as I know, the core Linux/arm64 maintainers have no plan to
  support APF. Without it, this is a pointless exercise. And even with
  it, this introduces a Linux specific behaviour in an otherwise
  architectural hypervisor (something I'm quite keen on avoiding)

- It gives an incentive to other hypervisor vendors to add random crap
  to the Linux mm subsystem, which is even worse. At this stage, we
  might as well go back to the Xen PV days altogether.

- I haven't seen any of the KVM/arm64 users actually asking for the
  APF horror, and the cloud vendors I directly asked had no plan to
  use it, and not using it on their x86 systems either

- no performance data nor workloads that could help making an informed
  decision have been disclosed, and the only argument in its favour
  seems to be "but x86 has it" (hardly a compelling one)

Given the above, I don't see how to justify this series, as it has no
purpose on its own, no matter how well written it is.

	M.

-- 
Without deviation from the norm, progress is not possible.