From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE, MAILING_LIST_MULTI,PDS_BAD_THREAD_QP_64,SPF_HELO_NONE,SPF_PASS, T_KAM_HTML_FONT_INVALID autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 50330C433B4 for ; Wed, 28 Apr 2021 14:50:25 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 471EE61433 for ; Wed, 28 Apr 2021 14:50:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 471EE61433 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=eldorado.org.br Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:53952 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lblWB-0000oe-12 for qemu-devel@archiver.kernel.org; Wed, 28 Apr 2021 10:50:23 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:50964) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lblTe-0008Hx-9g; Wed, 28 Apr 2021 10:47:46 -0400 Received: from mail-eopbgr800090.outbound.protection.outlook.com ([40.107.80.90]:35568 helo=NAM03-DM3-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lblTZ-0004t4-Qy; Wed, 28 Apr 2021 10:47:46 -0400 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=GklfNFbIhcC504qq7j/lRpGVXf34cBPhKMhZSxFYOuhO1bk6T7LbgwT7nB6pLOpu8nfANMrqjQJPYbhiCcxfExY8SgbhV1MVuTk+EAn0z8QFLx89lp1QlvFsC8c9ZyjaH1/aAZy3dKf1qzU7vFjGsmbd2ukJSs4993Dren/CFUrkQII0Alv8Fk1hmfB+dDiMloLU2JeGlj3tmPwyIpE5B7OpenhTDVRh++6cbVMxwRYfSy3TR3p6Ag9lFZxAPur2AxRYY1qD0wonChCfA5OPEWo8QWs4fJzYsggmd1W+o6cXY3lYc6mZMkQWNLA7H3RXJ5JvLXDa8T+H8syWlii1Mg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=L3pvHq5BieCyJV4wResoKbiSNlcuySXFEqZrBLeeI8Q=; b=WkAmbXS6DAVhotAGo1c1WGEORbOkZkzuR7ujls8oqzHDzc8MvsLVY1RBl0VHblelqSau0DVM97asjKIIhENEBx5/KtI1HGxD7sVpZWM7dxCW5NLwPcgW6f+XFocmtEtETvJQ2SX7B5Z1JIi99Xg5pE4Lfgmf6Jdxk0Bnd0mhhMQZc48HSjdsPtg+2yrjKZOOlBSS5bRkQgSIjHTSp4Xt94Dttd4KsNgPdBg+wxEuQZpMkNle6Yc4+hUVWCjb2Q7d56qQs5H2poUq7snHcS5CXGHlGwSJbnRE+ZV9XZtyi9NvcuDUlh4hedeTYeEe/GFWk48UPwQWRhvJ2WqKIKIn5Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=eldorado.org.br; dmarc=pass action=none header.from=eldorado.org.br; dkim=pass header.d=eldorado.org.br; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=eldorado.org.br; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=L3pvHq5BieCyJV4wResoKbiSNlcuySXFEqZrBLeeI8Q=; b=p3AAHHCtmXsmNtTPCkBY/vuXFnWMBQqE/izcPkMnPA7SVruBmfnU2sbndKb93xSTiTTrgBBUAkv0kb/74Qr2lzWw/dzKIN2fixdeAKUFpCl31UbpqLQgwPbV3ZTBOVRHxi7VTGZ4phaJGNzUHxDH1njnraNxPfVpvWLm/zzvKuHy9Dy3VlHOc3LlAaFLr34/gOOQns4uNDa6BtMEKGzQYOA/MjQwNgGWr1LPk42j5xF8T0GlqllgkmgH2ERwrXRO2w0+g/OXXj0FS1lQ2m4Efw1wXsTtXlITb9jV3nOpnUv4eAft1S/FqHzhvB4aLm38d6vW4YxpEYNtuTPs7oEWog== Received: from CP2PR80MB4499.lamprd80.prod.outlook.com (2603:10d6:102:45::19) by CPXPR80MB5079.lamprd80.prod.outlook.com (2603:10d6:103:12::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4065.21; Wed, 28 Apr 2021 14:47:37 +0000 Received: from CP2PR80MB4499.lamprd80.prod.outlook.com ([fe80::8c79:76c:3d1f:d59b]) by CP2PR80MB4499.lamprd80.prod.outlook.com ([fe80::8c79:76c:3d1f:d59b%7]) with mapi id 15.20.4087.025; Wed, 28 Apr 2021 14:47:37 +0000 From: Bruno Piazera Larsen To: David Gibson , Fabiano Rosas Subject: RE: [RFC PATCH 3/4] target/ppc: Move SPR generation to separate file Thread-Topic: [RFC PATCH 3/4] target/ppc: Move SPR generation to separate file Thread-Index: AQHXPDqOYntRa8dkakGvdaJuhDhEjw== Date: Wed, 28 Apr 2021 14:47:37 +0000 Message-ID: Accept-Language: pt-BR, en-US Content-Language: pt-BR X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: gibson.dropbear.id.au; dkim=none (message not signed) header.d=none; gibson.dropbear.id.au; dmarc=none action=none header.from=eldorado.org.br; x-originating-ip: [2804:14c:482:7ed1:f709:3fc9:6467:977f] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: bfd66704-baac-486c-e965-08d90a5490f5 x-ms-traffictypediagnostic: CPXPR80MB5079: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: +XCpOi/X5piwxzHua3YOCyir4kNi00CtV5aoke9f86hxT2BmuYVMVFq9odh/JsDG55YYCIYkh2fcSk8WifSrfU4FJGIneMNhabsP1wZBomgE0XFadyQ0PiQYKPy0WmHC6gO8VJYRorpbphnxsmvj1tUikNpv2M8App0whoOn6zsREWFNzCra114s2K/mcXwY+ZQ9z4vchFPqNxLAdzkpyCUMyqgPBqmTX925b9SevlVjugH17cdpvAhJ1xxo1Qd6629ti7IaVJeHgk4Tx6t6Qv1NTLGZYn6d4TcJ4BtnmVwfOMqPtmT3BjRyux9lZP915dCSDrJ1sTpATIWFx28095w4TXEJ+VaQqg1SWIJ2yAAUJuXEwiyRqDvFBArG2eEEPI8JanNsYzrs6Qy5B4DIJzZUp4qdHMvsxUlqjdYDl+dnhjZrPD4qeof1s1kG4v7Oa+r5925W0gxG0ifXjHavTE8ygz1M1SHsyvh0CRrZ+iI3uRfZk/+XUu1lx6wBvIyhXY1RLm48y88CGXKQ0vkxBq5uBf0Grm+EwdjGoCCs+DdTEyw7PUTiFa7IimjZpFwid1HO+E7sZljIAavMUssIqfzT6QWpSoMFbP7W68lKaGu0+T8eBYLYJ2CupNQDwAAar8zeKInfB9ZUFSgSNftPyUGOCq5+96otslqPDiKj64kUxIrZ44lsnGQmhO2Kv9YR x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CP2PR80MB4499.lamprd80.prod.outlook.com; PTR:; CAT:NONE; SFS:(396003)(376002)(39830400003)(346002)(136003)(366004)(64756008)(66476007)(66556008)(66946007)(71200400001)(33656002)(86362001)(55016002)(66446008)(76116006)(8676002)(8936002)(478600001)(38100700002)(122000001)(2906002)(316002)(7696005)(5660300002)(166002)(21615005)(6506007)(52536014)(54906003)(186003)(107886003)(19627405001)(9686003)(110136005)(4326008); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: =?iso-8859-1?Q?GoRXfw1zfZJT2vL3jgErujjxz5e1bZWn23Vp9JKZjyD8jf5N42dW2Yq86D?= =?iso-8859-1?Q?Uzd4yYNZI1El4APKEHuc2q6V8AMVYJXV63WbC0pPuYk1J1r5UczFwMQCfP?= =?iso-8859-1?Q?fHOBAluI2xDsf8JcQzT/C20q0INSswUhdVjY/fftFNh/iWkHIk6OrS7hIy?= =?iso-8859-1?Q?w+5Dor+96EHAC948xiTxSW5l024onBZdXFKUELOGwrU2AL5XYecqvW3Ahw?= =?iso-8859-1?Q?15cIpuVgBTchjhkbuiOWt9LLGWfaeFccu49wVsEIot/XzZvs3724a+I5b/?= =?iso-8859-1?Q?p0/EgzSR5C+LGUuPkjka9PRUj1pku13/i0IAnoky2ec5rD/ZraHpH73p5p?= =?iso-8859-1?Q?AnIUfvQtokgyPEzfsx/MUV8acKe7Al2UcPL/aPxOo11ns+oe9y2is7+3gi?= =?iso-8859-1?Q?opleT65a/wO1TUG/HGwKw8rjyazmkyDwoY6DVgQmc/WtC9EHbtIqCCedHH?= =?iso-8859-1?Q?ONqhxpkzlvxdcd2NAf3T5ZY6rxRcnqVT/0Eirvs1yFeDWd5Pmjp9bqL+Q7?= =?iso-8859-1?Q?SUkm8qiyPRVqWly5ajbLgIVWvMnLBy3W0TXFTrN/d5N8leQeAWFN1Sb6h0?= =?iso-8859-1?Q?47ArN6NaF3eQtLZc1LG5haaaiJUSLgrIhDdbAhU/y6e64nctLtsBYFtRRb?= =?iso-8859-1?Q?khjMzWdcMPhVdMQ0DxE8yQGYDeq6kbma9SbXNrli+yJ8i+u6BCmsFzMiWl?= =?iso-8859-1?Q?H+lN2CNi5hTQvFyZc74Cme0UF6Tq06Og5cHl3VqGym3IKD7FW/9C8SFDh7?= =?iso-8859-1?Q?g7nHFYsrU/jbKKQuRveP/UJsDMf/B/lvOQsJ9DhhHyHpk0BYkmgZdhppD4?= =?iso-8859-1?Q?t3mo+gLkqlB8S2e3Ce0/9UpGO6g3VisHlSA5+2cuHpwruzdhDKXfXy0gi/?= =?iso-8859-1?Q?DxvuNPHfDeYUIUkEKCBkvxQ1mM6PDjlCh+22TqaIa9gkUpkbXDzkXjz3N1?= =?iso-8859-1?Q?e2q1ZN6nONCJNdDPD5Y4mmZhJGFtyuNe+aV3UchGq4s/ItVNYOO9JzZZd7?= =?iso-8859-1?Q?2sqBT+R/XdyPX4YL8A092OLsmzRpYcJdkVQeYDUbrbhp0IucZArVC5TEYD?= =?iso-8859-1?Q?034sw3csXP61zUYarGgVY3Pgj6LQJE+Iq3qZwoqiJIKuazmDCMXAiZG/uV?= =?iso-8859-1?Q?ZhwSfEvB121h0w+aXDEwj/dmHgmXS1Nh9hAv/9Fgg7BoFmu+QvfgEQwFmf?= =?iso-8859-1?Q?g2YuciVQ8xCvE+QjP8eZ6ZQ9xxbuy4fo4cJSThnknwZrBXYDPMBBq3g2QU?= =?iso-8859-1?Q?s6QYOjYOy7Eoyv99XfW6NNP9IW8EhMrpDsLY72uBPYKeofJ3t05y+zoqTk?= =?iso-8859-1?Q?WTnS5zEftw5lERUevMH7T3Bh9rAejEZlIbUqWzJnQa4g2IEZ2VL7u4XIdu?= =?iso-8859-1?Q?2JFMeO+/kudgHfOYUYr6JgxU49u0vX+psMMl1svN1UbF26X8yz+TE=3D?= Content-Type: multipart/alternative; boundary="_000_CP2PR80MB449987FB35CC5BC79CACDAFBC7409CP2PR80MB4499lamp_" MIME-Version: 1.0 X-OriginatorOrg: eldorado.org.br X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CP2PR80MB4499.lamprd80.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: bfd66704-baac-486c-e965-08d90a5490f5 X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Apr 2021 14:47:37.1383 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: b9397c69-e827-4afc-a365-ab275e41638f X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 6d3pcHrOHbTWxCPzeT0RFYho1BNXS7NEPwTUTkm4Nph1g7bcBs9m7Lm2IOaiggfZv4Fuz4Q0DtquUy4rxl7/RTvYyJm+9MYJqRioH9T+ZXo= X-MS-Exchange-Transport-CrossTenantHeadersStamped: CPXPR80MB5079 Received-SPF: pass client-ip=40.107.80.90; envelope-from=bruno.larsen@eldorado.org.br; helo=NAM03-DM3-obe.outbound.protection.outlook.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_KAM_HTML_FONT_INVALID=0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Luis Fernando Fujita Pires , "qemu-devel@nongnu.org" , Lucas Mateus Martins Araujo e Castro , Fernando Eckhardt Valle , "qemu-ppc@nongnu.org" , Matheus Kowalczuk Ferst Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --_000_CP2PR80MB449987FB35CC5BC79CACDAFBC7409CP2PR80MB4499lamp_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable > > > This move is required to enable building without TCG. > > > All the logic related to registering SPRs specific to > > > some architectures or machines has been hidden in this > > > new file. > > > > Hm... I thought we ended up deciding to keep the gen_spr_ > > functions in translate_init.c.inc (cpu_init.c). I don't strongly favour > > one way or the other, but one alternative would be to just rename the > > gen_spr_ functions to add_sprs_ or even > > register__sprs and leave them where they are. > > Right. I think renaming these away from gen_*() is a good idea, to > avoid confusion with the other gen_*() functions which, well, generate > TCG code. > > I don't think there's a lot of value in moving them out from > translate_init. Honestly the more useful way to break up > translate_init would be by CPU family, rather than by SPR vs. non-SPR > setup Right, ok. I thought we agreed to separate, but I can't remember the reason= . I guess less is more in this case, so I won't create the new file. As for separating by CPU family: sounds good for a later cleanup, but I don= 't think it's a priority right now. I'll work on that renaming, I agree its a good idea. > > > The idea of this final patch is to hide all SPR generation from > > > translate_init, but in an effort to simplify the RFC the 4 > > > functions for not_implemented SPRs were created. They'll be > > > substituted by gen_spr__misc in reusable ways for the > > > final patch. > > > > I'd expect this patch to be just a big removal of gen_spr* from > > translate_init.c.inc and their addition into spr_common.c. So any other > > prep work should come in separate patches ealier in the > > series. Specifically, at least one patch for the macro work and another > > for the refactoring of open coded spr_register calls into gen_spr_* > > functions. > > Seconded. Ok. Should it be 2 commits (refactoring, then macro) or 3 commits (renaming= , vscr_init, then macro)? I guess also that since I won't move stuff, I don't need to fix the vscr_init, but it's no big deal at this point. > > If you're only adding this now, it means the previous patch is > > broken. As a general rule you should make sure every patch works. I kno= w > > that for the RFC things might be a bit broken temporarily and that is o= k > > but it is always a good idea to make sure every individual change is in > > the correct patch at least. yeah... sorry about that. I'm correcting all these problems. > > > +/*******************************************************************= **********/ > > > +/* SPR definitions and registration */ > > > + > > > +#ifdef CONFIG_TCG > > > +#ifdef CONFIG_USER_ONLY > > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, one_reg_id, initial_va= lue) \ > > > + _spr_register(env, num, name, uea_read, uea_write, initial_value= ) > > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, hea_read, hea_write= , \ > > > + one_reg_id, initial_value) = \ > > > + _spr_register(env, num, name, uea_read, uea_write, initial_value= ) > > > +#else /* CONFIG_USER_ONLY */ > > > +#if !defined(CONFIG_KVM) > > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, one_reg_id, initial_va= lue) \ > > > + _spr_register(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, oea_read, oea_write, initial_= value) > > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, hea_read, hea_write= , \ > > > + one_reg_id, initial_value) = \ > > > + _spr_register(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, hea_read, hea_write, initial_= value) > > > +#else /* !CONFIG_KVM */ > > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, one_reg_id, initial_va= lue) \ > > > + _spr_register(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, oea_read, oea_write, = \ > > > + one_reg_id, initial_value) > > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, hea_read, hea_write= , \ > > > + one_reg_id, initial_value) = \ > > > + _spr_register(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, hea_read, hea_write, = \ > > > + one_reg_id, initial_value) > > > +#endif /* !CONFIG_KVM */ > > > +#endif /* CONFIG_USER_ONLY */ > > > +#else /* CONFIG_TCG */ > > > +#ifdef CONFIG_KVM /* sanity check */ > > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, one_reg_id, initial_va= lue) \ > > > + _spr_register(env, num, name, one_reg_id, initial_value) > > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, hea_read, hea_write= , \ > > > + one_reg_id, initial_value) = \ > > > + _spr_register(env, num, name, one_reg_id, initial_value) > > > +#else /* CONFIG_KVM */ > > > +#error "Either TCG or KVM should be configured" > > > +#endif /* CONFIG_KVM */ > > > +#endif /*CONFIG_TCG */ > > > + > > > +#define spr_register(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, initial_value) = \ > > > + spr_register_kvm(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, 0, initial_value) > > > + > > > +#define spr_register_hv(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, hea_read, hea_write, = \ > > > + initial_value) = \ > > > + spr_register_kvm_hv(env, num, name, uea_read, uea_write, = \ > > > + oea_read, oea_write, hea_read, hea_write, = \ > > > + 0, initial_value) > > > > This change is crucial for this to work, we don't want it buried along > > with all of the code movement. It should be clearly described and in > > it's own patch at the top of the series. > > > > If you add this change, plus some #ifdef TCG around the spr callbacks, > > it should already be possible to compile this with disable-tcg. It woul= d > > also make the series as a whole easier to understand. if we added this and removed TCG only files from meson.build it might compile already (not sure, I think there were a couple of things that used mmu functions), but wouldn't there be way too many ifdefs in that case= ? The R/W callbacks are spread all around the file, and we'd have to ifdef ar= ound some .h files that are included in common files. Bruno Piazera Larsen Instituto de Pesquisas ELDORADO Departamento Computa=E7=E3o Embarcada Analista de Software Trainee Aviso Legal - Disclaimer --_000_CP2PR80MB449987FB35CC5BC79CACDAFBC7409CP2PR80MB4499lamp_ Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable
> > > This move is= required to enable building without TCG.
> > > All the logic related to registering SPRs specific to
> > > some architectures or machines has been hidden in this
> > > new file.
> >
> > Hm... I thought we ended up deciding to keep the gen_spr_<mach= ine>
> > functions in translate_init.c.inc (cpu_init.c). I don't strongly = favour
> > one way or the other, but one alternative would be to just rename= the
> > gen_spr_<machine> functions to add_sprs_<machine> or = even
> > register_<machine>_sprs and leave them where they are.
>
> Right.  I think renaming these away from gen_*() is a good idea, = to
> avoid confusion with the other gen_*() functions which, well, generate=
> TCG code.
>
> I don't think there's a lot of value in moving them out from
> translate_init.  Honestly the more useful way to break up
> translate_init would be by CPU family, rather than by SPR vs. non-SPR<= br> > setup

Right, ok. I thought we agr= eed to separate, but I can't remember the reason.
I guess less is more in thi= s case, so I won't create the new file.
As for separating by CPU fa= mily: sounds good for a later cleanup, but I don't
think it's a priority right= now.

I'll work on that renaming,= I agree its a good idea.

> > > The idea of this final patch is to hide= all SPR generation from
> > > translate_init, but in an effort to simplify the RFC the 4 > > > functions for not_implemented SPRs were created. They'll be<= br> > > > substituted by gen_spr_<machine>_misc in reusable ways= for the
> > > final patch.
> >
> > I'd expect this patch to be just a big removal of gen_spr* from > > translate_init.c.inc and their addition into spr_common.c. So any= other
> > prep work should come in separate patches ealier in the
> > series. Specifically, at least one patch for the macro work and a= nother
> > for the refactoring of open coded spr_register calls into gen_spr= _*
> > functions.
>
> Seconded.

Ok. Should it be 2 commits = (refactoring, then macro) or 3 commits (renaming,
vscr_init, then macro)? I guess also tha= t since I won't move stuff, I don't
need to fix the vscr_init, = but it's no big deal at this point.

> > If you're only ad= ding this now, it means the previous patch is
> > broken. As a general rule you should make sure every patch works.= I know
> > that for the RFC things might be a bit broken temporarily and tha= t is ok
> > but it is always a good idea to make sure every individual change= is in
> > the correct patch at least.

yeah... sorry about that. I= 'm correcting all these problems.

> > > +/**********= *******************************************************************/
> > > +/* SPR definitions and registration */
> > > +
> > > +#ifdef CONFIG_TCG
> > > +#ifdef CONFIG_USER_ONLY
> > > +#define spr_register_kvm(env, num, name, uea_read, uea_writ= e,            &= nbsp;     \
> > > +          = ;            &n= bsp;  oea_read, oea_write, one_reg_id, initial_value)   = ;    \
> > > +    _spr_register(env, num, name, uea_read, = uea_write, initial_value)
> > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_w= rite,           &nbs= p;   \
> > > +          = ;            &n= bsp;     oea_read, oea_write, hea_read, hea_write, = ;         \
> > > +          = ;            &n= bsp;     one_reg_id, initial_value)   &n= bsp;            = ;         \
> > > +    _spr_register(env, num, name, uea_read, = uea_write, initial_value)
> > > +#else /* CONFIG_USER_ONLY */
> > > +#if !defined(CONFIG_KVM)
> > > +#define spr_register_kvm(env, num, name, uea_read, uea_writ= e,            &= nbsp;     \
> > > +          = ;            &n= bsp;  oea_read, oea_write, one_reg_id, initial_value)   = ;    \
> > > +    _spr_register(env, num, name, uea_read, = uea_write,           = ;            &n= bsp; \
> > > +          = ;        oea_read, oea_write, oea_read, = oea_write, initial_value)
> > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_w= rite,           &nbs= p;   \
> > > +          = ;            &n= bsp;     oea_read, oea_write, hea_read, hea_write, = ;         \
> > > +          = ;            &n= bsp;     one_reg_id, initial_value)   &n= bsp;            = ;         \
> > > +    _spr_register(env, num, name, uea_read, = uea_write,           = ;            &n= bsp; \
> > > +          = ;        oea_read, oea_write, hea_read, = hea_write, initial_value)
> > > +#else /* !CONFIG_KVM */
> > > +#define spr_register_kvm(env, num, name, uea_read, uea_writ= e,            &= nbsp;     \
> > > +          = ;            &n= bsp;  oea_read, oea_write, one_reg_id, initial_value)   = ;    \
> > > +    _spr_register(env, num, name, uea_read, = uea_write,           = ;            &n= bsp; \
> > > +          = ;        oea_read, oea_write, oea_read, = oea_write,           = ;         \
> > > +          = ;        one_reg_id, initial_value)
> > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_w= rite,           &nbs= p;   \
> > > +          = ;            &n= bsp;     oea_read, oea_write, hea_read, hea_write, = ;         \
> > > +          = ;            &n= bsp;     one_reg_id, initial_value)   &n= bsp;            = ;         \
> > > +    _spr_register(env, num, name, uea_read, = uea_write,           = ;            &n= bsp; \
> > > +          = ;        oea_read, oea_write, hea_read, = hea_write,           = ;         \
> > > +          = ;        one_reg_id, initial_value)
> > > +#endif /* !CONFIG_KVM */
> > > +#endif /* CONFIG_USER_ONLY */
> > > +#else /* CONFIG_TCG */
> > > +#ifdef CONFIG_KVM /* sanity check */
> > > +#define spr_register_kvm(env, num, name, uea_read, uea_writ= e,            &= nbsp;     \
> > > +          = ;            &n= bsp;  oea_read, oea_write, one_reg_id, initial_value)   = ;    \
> > > +    _spr_register(env, num, name, one_reg_id= , initial_value)
> > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_w= rite,           &nbs= p;   \
> > > +          = ;            &n= bsp;     oea_read, oea_write, hea_read, hea_write, = ;         \
> > > +          = ;            &n= bsp;     one_reg_id, initial_value)   &n= bsp;            = ;         \
> > > +    _spr_register(env, num, name, one_reg_id= , initial_value)
> > > +#else /* CONFIG_KVM */
> > > +#error "Either TCG or KVM should be configured" > > > +#endif /* CONFIG_KVM */
> > > +#endif /*CONFIG_TCG */
> > > +
> > > +#define spr_register(env, num, name, uea_read, uea_write,&n= bsp;            = ;         \
> > > +          = ;           oea_read, oea= _write, initial_value)         = ;            &n= bsp; \
> > > +    spr_register_kvm(env, num, name, uea_rea= d, uea_write,          &n= bsp;           \
> > > +          = ;           oea_read, oea= _write, 0, initial_value)
> > > +
> > > +#define spr_register_hv(env, num, name, uea_read, uea_write= ,            &n= bsp;      \
> > > +          = ;            &n= bsp; oea_read, oea_write, hea_read, hea_write,     = ;         \
> > > +          = ;            &n= bsp; initial_value)         &n= bsp;            = ;            &n= bsp;      \
> > > +    spr_register_kvm_hv(env, num, name, uea_= read, uea_write,          = ;         \
> > > +          = ;            &n= bsp; oea_read, oea_write, hea_read, hea_write,     = ;         \
> > > +          = ;            &n= bsp; 0, initial_value)
> >
> > This change is crucial for this to work, we don't want it buried = along
> > with all of the code movement. It should be clearly described and= in
> > it's own patch at the top of the series.
> >
> > If you add this change, plus some #ifdef TCG around the spr callb= acks,
> > it should already be possible to compile this with disable-tcg. I= t would
> > also make the series as a whole easier to understand.

if we added this and remove= d TCG only files from meson.build it might
compile already (not sure, = I think there were a couple of things that
used mmu functions), but wo= uldn't there be way too many ifdefs in that case?
The R/W callbacks are sprea= d all around the file, and we'd have to ifdef around
some .h files that are incl= uded in common files.

Bruno Piazer= a Larsen

Instituto de Pesquisas ELDORADO

Departamento= Computa=E7=E3o Embarcada

Analista de = Software Trainee

Aviso Legal - Disclaimer

--_000_CP2PR80MB449987FB35CC5BC79CACDAFBC7409CP2PR80MB4499lamp_--