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;
From:   "Kani, Toshi" <toshi.kani@hpe.com>
To:     Jia He <justin.he@arm.com>, Ard Biesheuvel <ardb@kernel.org>,
        Len Brown <lenb@kernel.org>, James Morse <james.morse@arm.com>,
        Tony Luck <tony.luck@intel.com>,
        Borislav Petkov <bp@alien8.de>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Robert Richter <rric@kernel.org>,
        Robert Moore <robert.moore@intel.com>,
        Qiuxu Zhuo <qiuxu.zhuo@intel.com>,
        Yazen Ghannam <yazen.ghannam@amd.com>
CC:     "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
        "devel@acpica.org" <devel@acpica.org>,
        "Rafael J . Wysocki" <rafael@kernel.org>,
        Shuai Xue <xueshuai@linux.alibaba.com>,
        Jarkko Sakkinen <jarkko@kernel.org>,
        "linux-efi@vger.kernel.org" <linux-efi@vger.kernel.org>,
        "nd@arm.com" <nd@arm.com>
Subject: RE: [PATCH v2 5/7] EDAC/ghes: Prevent chipset-specific edac from
 loading after ghes_edac is registered
Thread-Topic: [PATCH v2 5/7] EDAC/ghes: Prevent chipset-specific edac from
 loading after ghes_edac is registered
Thread-Index: AQHYskazgdaGLeNebEWQzyv29780RK21ZjbQ
Date:   Fri, 19 Aug 2022 01:29:35 +0000
Message-ID: <DM4PR84MB18538A56870A280CDC4637A7826C9@DM4PR84MB1853.NAMPRD84.PROD.OUTLOOK.COM>
References: <20220817143458.335938-1-justin.he@arm.com>
 <20220817143458.335938-6-justin.he@arm.com>
In-Reply-To: <20220817143458.335938-6-justin.he@arm.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?NYl13J+6pPwb0CztsuHGCLulDj4q4wlCTpeRYP3HUdHORHIFg7vnxXHjK8WP?=
 =?us-ascii?Q?4EnTztq0AeJvvxcISLiXyD/EiMlUYU2fI5AhkWpNCombdtDH8rWkwUYPYbEF?=
 =?us-ascii?Q?uQysJuHffAjvOIxixK3CMHkLlVPw6DD21LubL3IQwF50cW+LB9/9JjxxNn7x?=
 =?us-ascii?Q?/k97/45pUxRWAEo/r6bvAwO0v26wi83gjwdwAouDZHE3mcLucwy6w4AoGXbL?=
 =?us-ascii?Q?opaGf2K0ZNPy77OWUrsHimxwqL/+0yI8ZJ/d9FUopFvDMFUVfu5cYnmXol4m?=
 =?us-ascii?Q?FBCydbG7HrLFEyFibW8HQMiMmZ7Xe7Qoa2kYYA2TPoJBgoYC/vdBwUDrAYB0?=
 =?us-ascii?Q?QoyCv4GqYk68DVfuchfZ2NnOzEv2/jB6kFeOZaKEt3nH66rr+cQTaDCXvZE2?=
 =?us-ascii?Q?9lffaC8XEgonMa6JBVOxsB9+ELC9Y6oLarDe4vpo+cwroaviiaGdmaxH8Uyi?=
 =?us-ascii?Q?Nod0B7+NoT/SHaKyYQR6iyaybgEPygG0k+0FuUTNkv9AlLDz1r30wV+Tdrmg?=
 =?us-ascii?Q?JJ//tkByiSeHmxoaDLBPP0J8wPGrf+/+2qFprCMjNT78ApS5mZMpUfnZAJVu?=
 =?us-ascii?Q?jDxIqA9tRXse4lx61Or16iuB1NGHYqGi2oZGWw9vgHJZYdv+SyhzMtiPMRWx?=
 =?us-ascii?Q?iW1RSwaEVcvKuFV53hhdRUWVHbUatwg1B6/pxFJecE0JGyJDfzop6YnQ2cZa?=
 =?us-ascii?Q?jg7zJcRgMAy7YX2hNDIZ49r3xKluhJvluDsx5HHwErR3dqCxQKT/1Flsi+GH?=
 =?us-ascii?Q?6QMxMXurriKvuX4FKTcfgFFt6s7kpqOwKtbcPNHrf7CoLpqvlNk6nNZdGvrp?=
 =?us-ascii?Q?hdJR9xMc76dC6GNUjxBflCF9HJ4wnouJ32RyHC2OdS1CzUdDjqFrH3E+hIOa?=
 =?us-ascii?Q?f+5QwLpnJ/lUfmg72nTsGcuErqDZ42EEWrGuFf1YUVvjlYzK/cVDIm+6L8+j?=
 =?us-ascii?Q?TgsIAeEhBXtyEryARyQbKkVnqFAAFSTyN4Q6ntsa9b5ChUttPN0l4lfivheD?=
 =?us-ascii?Q?y9uQSFckl4mjXFj+QypI167B8R8A4p9dQawhSwQ5E8UWN4DiTDuwO6Z0qgHf?=
 =?us-ascii?Q?YzNWaJkFXXaJSmjByM+hrx437XgrfhffeGWr7+/4/xht3aroWARb1uIdy2Nu?=
 =?us-ascii?Q?UTSflNULRSzIresGer1K4Hf3PZGwqZOtfiMAN+N6dK1YUkc+kr0dmME2yJYQ?=
 =?us-ascii?Q?eonpoasTCJLaLJnZelivj0fRB2Jy09WtO+pZHhzHE7ogW3O+0Gg1eErpF9G9?=
 =?us-ascii?Q?Pvr0R+6HxdQJUwgejcQdLXZCZu0hXgc6EFxcpLKMQkdcvjD9bHMjiO98e6kN?=
 =?us-ascii?Q?los914Ap6SYpCvFLpJr0UeTJkotVdiv46cQvykbxsPdj57dzYAlnfoWIp2bG?=
 =?us-ascii?Q?NODtzNurIYOdiC3hdtI9aN82k7+dUnqjQkiCU9X6XpKC+nvntK+7YTZN2ML/?=
 =?us-ascii?Q?S1q5qG4peXIttu2VmZGJnzF1NJ0QPkLMKTlJWLSZIhdFtPeXLuoCfAFLRj3+?=
 =?us-ascii?Q?0UxyjkHaRz5gEzcFZl76U6en4FDIV+VgW4kRnAGor50ICpXCBelXKYbdh+Sx?=
 =?us-ascii?Q?yJrpDJwcFb0vsuL4G2EGYyCYL9W2siCT0eyIYc9M?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM4PR84MB1853.NAMPRD84.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-Network-Message-Id: 67c61a9a-118a-4f2a-e188-08da81824692
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Aug 2022 01:29:35.5952
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: XKaK7TlfmxEqlahmv0VACAjrag5KiBnencFHq1P65fnKyCK7FWWu/l99/8pGUNITn6KcueLh1PszUF/KVGS70w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ1PR84MB3115
X-OriginatorOrg: hpe.com
X-Proofpoint-GUID: UddhV0LRjKxI3KUYrxTZ0lrnlLgAynkD
X-Proofpoint-ORIG-GUID: UddhV0LRjKxI3KUYrxTZ0lrnlLgAynkD
X-HPE-SCL: -1
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-08-18_18,2022-08-18_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 mlxscore=0
 suspectscore=0 bulkscore=0 phishscore=0 lowpriorityscore=0 malwarescore=0
 mlxlogscore=999 clxscore=1015 spamscore=0 adultscore=0 priorityscore=1501
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000
 definitions=main-2208190004
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wednesday, August 17, 2022 8:35 AM, Jia He wrote:
> Previous, there is only one edac can be registering in the EDAC core. Aft=
er
> ghes_edac is modularized, the registering of ghes devices may be deferred
> when ghes_edac is loaded. Prevent the chipset-specific edac drivers from
> loading after ghes_edac is registered, which allow the edac drivers to
> get selected in e.g. HPE platform.
...
> @@ -1382,6 +1395,7 @@ static int ghes_probe(struct platform_device
> *ghes_dev)
>  	platform_set_drvdata(ghes_dev, ghes);
>=20
>  	ghes->dev =3D &ghes_dev->dev;
> +	set_ghes_devs_registered(false);

This does not look right to me.

The condition of using ghes_edac is (ghes-present) && (ghes-preferred),
where:
 - ghes-present is latched on ghes_probe()
 - ghes-preferred is true if the platform-check passes.

ghes_get_device() introduced in the previous patch works as the=20
ghes-preferred check.

We cannot use ghes_edac registration as the ghes-present check in
this scheme since it is deferred to module_init().

I'd suggest the following changes:
- Update ghes_get_device() to check a flag latched on ghes_probe().
- Remove 'force' argument from ghes_get_device().  ghes_edac_init()
is too late to set this flag.  Also, other edac drivers should not be able
to control this flag.  I think this force_load kernel option needs to
belong to ghes with this scheme.
- ghes_get_device() exposes 'ghes_devs' to chipset-specific edac drivers,
which should be internal to ghes.  ghes_get_device() may be renamed
to something like ghes_edac_preferred() which returns true / false.

Toshi