linux-kernel - Re: [PATCH v2 5/7] virt: geniezone: Add irqchip support for virtual interrupt injection

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 12 May 2023 10:57:15 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     "Yi-De Wu (吳一德)" 
        <Yi-De.Wu@...iatek.com>
Cc:     "corbet@....net" <corbet@....net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "MY Chuang (莊明躍)" 
        <MY.Chuang@...iatek.com>,
        "quic_tsoni@...cinc.com" <quic_tsoni@...cinc.com>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "Ivan Tseng (曾志軒)" 
        <ivan.tseng@...iatek.com>,
        "Ze-yu Wang (王澤宇)" 
        <Ze-yu.Wang@...iatek.com>,
        "PeiLun Suei (隋培倫)" 
        <PeiLun.Suei@...iatek.com>,
        "Liju-clr Chen (陳麗如)" 
        <Liju-clr.Chen@...iatek.com>,
        "Jades Shih (施向玨)" 
        <jades.shih@...iatek.com>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "dbrazdil@...gle.com" <dbrazdil@...gle.com>,
        "Shawn Hsiao (蕭志祥)" 
        <shawn.hsiao@...iatek.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "Miles Chen (陳民樺)" 
        <Miles.Chen@...iatek.com>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
        "Yingshiuan Pan (潘穎軒)" 
        <Yingshiuan.Pan@...iatek.com>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        "arnd@...db.de" <arnd@...db.de>,
        "angelogioacchino.delregno@...labora.com" 
        <angelogioacchino.delregno@...labora.com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "will@...nel.org" <will@...nel.org>
Subject: Re: [PATCH v2 5/7] virt: geniezone: Add irqchip support for virtual interrupt injection

On Fri, 12 May 2023 09:16:58 +0100,
"Yi-De Wu (吳一德)" <Yi-De.Wu@...iatek.com> wrote:
> 
> On Fri, 2023-05-12 at 08:51 +0100, Marc Zyngier wrote:
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> > 
> > 
> > On Fri, 12 May 2023 08:19:31 +0100,
> > "Yi-De Wu (吳一德)" <Yi-De.Wu@...iatek.com> wrote:
> > > 
> > > On Fri, 2023-04-28 at 19:59 +0100, Marc Zyngier wrote:
> > > > External email : Please do not click links or open attachments
> > > > until
> > > > you have verified the sender or the content.
> > > > 
> > > > 
> > > > On 2023-04-28 11:36, Yi-De Wu wrote:
> > > > > From: "Yingshiuan Pan" <yingshiuan.pan@...iatek.com>
> > > > > 
> > > > > Enable GenieZone to handle virtual interrupt injection request.
> > > > > 
> > > > > Signed-off-by: Yingshiuan Pan <yingshiuan.pan@...iatek.com>
> > > > > Signed-off-by: Yi-De Wu <yi-de.wu@...iatek.com>
> > > > > ---
> > > > >  arch/arm64/geniezone/Makefile       |  2 +-
> > > > >  arch/arm64/geniezone/gzvm_arch.c    | 24 ++++++--
> > > > >  arch/arm64/geniezone/gzvm_arch.h    | 11 ++++
> > > > >  arch/arm64/geniezone/gzvm_irqchip.c | 88
> > > > > +++++++++++++++++++++++++++++
> > > > >  drivers/virt/geniezone/gzvm_vm.c    | 75
> > > > > ++++++++++++++++++++++++
> > > > >  include/linux/gzvm_drv.h            |  4 ++
> > > > >  include/uapi/linux/gzvm.h           | 38 ++++++++++++-
> > > > >  7 files changed, 235 insertions(+), 7 deletions(-)
> > > > >  create mode 100644 arch/arm64/geniezone/gzvm_irqchip.c
> > > > 
> > > > [...]
> > > > 
> > > > > +++ b/arch/arm64/geniezone/gzvm_irqchip.c
> > > > > @@ -0,0 +1,88 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * Copyright (c) 2023 MediaTek Inc.
> > > > > + */
> > > > > +
> > > > > +#include <linux/irqchip/arm-gic-v3.h>
> > > > > +#include <kvm/arm_vgic.h>
> > > > 
> > > > NAK.
> > > > 
> > > > There is no way you can rely on anything from KVM in
> > > > your own hypervisor code.
> > > > 
> > > 
> > > Same with previous discussion, we'd like to copy or rename the
> > > related
> > > part from KVM and keep the maintainance at our own if it's ok.
> > 
> > Why do you need *ANY* of the KVM stuff? Please fully enumerate these
> > dependencies and why you have them.
> > 
> > Directly using KVM stuff for something completely unrelated is not
> > OK,
> > and will never be.
> > 
> >         M.
> > 
> > --
> > Without deviation from the norm, progress is not possible.
> 
> The particular part we'd like to leverage from KVM are as followed
> 1. `gfn_to_pfn_memslot` to convert guest physical address(or
> intermediate physical address) to physical address

What is a memslot in your hypervisor? How does it relate to KVM's?
What about the use of struct kvm?

I'm sorry, but your use of *internal* structures and API would make it
impossible for us to make any further change without potentially
affecting your hypervisor. Which is closed source and untestable.

To sum it up, I'm strongly opposed to any use of these data
structures.  If you can spot some commonalities, expose them as a
0-cost abstraction. But don't use them as is your code.

> 2. get ARM's number of interrupt of different types e.g. number of SGI,
> number of PPI...etc

These are architectural constants, and you can define your own. That
will cost you nothing but a handful of #define, and keep the two
subsystem independent.

	M.

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