linux-kernel - Re: [PATCH v5 5/7] qapi/ghes-cper: add an interface to do generic CPER error 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] [day] [month] [year] [list]

Message-ID: <20240809104222.08d197cb@foz.lan>
Date: Fri, 9 Aug 2024 10:42:22 +0200
From: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To: Markus Armbruster <armbru@...hat.com>
Cc: Igor Mammedov <imammedo@...hat.com>, Jonathan Cameron
 <Jonathan.Cameron@...wei.com>, Shiju Jose <shiju.jose@...wei.com>, "Michael
 S. Tsirkin" <mst@...hat.com>, Ani Sinha <anisinha@...hat.com>, Dongjiu Geng
 <gengdongjiu1@...il.com>, Eric Blake <eblake@...hat.com>, Michael Roth
 <michael.roth@....com>, Paolo Bonzini <pbonzini@...hat.com>, Peter Maydell
 <peter.maydell@...aro.org>, linux-kernel@...r.kernel.org,
 qemu-arm@...gnu.org, qemu-devel@...gnu.org
Subject: Re: [PATCH v5 5/7] qapi/ghes-cper: add an interface to do generic
 CPER error injection

Em Thu, 08 Aug 2024 16:45:51 +0200
Markus Armbruster <armbru@...hat.com> escreveu:

> Igor Mammedov <imammedo@...hat.com> writes:
> 
> > On Thu, 8 Aug 2024 16:11:41 +0200
> > Mauro Carvalho Chehab <mchehab+huawei@...nel.org> wrote:
> >  
> >> Em Thu, 08 Aug 2024 10:50:33 +0200
> >> Markus Armbruster <armbru@...hat.com> escreveu:
> >>   
> >> > Mauro Carvalho Chehab <mchehab+huawei@...nel.org> writes:    
> >>   
> >> > > diff --git a/MAINTAINERS b/MAINTAINERS
> >> > > index 98eddf7ae155..655edcb6688c 100644
> >> > > --- a/MAINTAINERS
> >> > > +++ b/MAINTAINERS
> >> > > @@ -2075,6 +2075,13 @@ F: hw/acpi/ghes.c
> >> > >  F: include/hw/acpi/ghes.h
> >> > >  F: docs/specs/acpi_hest_ghes.rst
> >> > >  
> >> > > +ACPI/HEST/GHES/ARM processor CPER
> >> > > +R: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> >> > > +S: Maintained
> >> > > +F: hw/arm/ghes_cper.c
> >> > > +F: hw/acpi/ghes_cper_stub.c
> >> > > +F: qapi/ghes-cper.json
> >> > > +      
> >> > 
> >> > Here's the reason for creating a new QAPI module instead of adding to
> >> > existing module acpi.json: different maintainers.
> >> > 
> >> > Hypothetical question: if we didn't care for that, would this go into
> >> > qapi/acpi.json?    
> >> 
> >> Independently of maintainers, GHES is part of ACPI APEI HEST, meaning
> >> to report hardware errors. Such hardware errors are typically handled by 
> >> the host OS, so quest doesn't need to be aware of that[1].
> >> 
> >> So, IMO the best would be to keep APEI/HEST/GHES in a separate file.
> >> 
> >> [1] still, I can foresee some scenarios were passing some errors to the
> >>     guest could make sense.
> >>   
> >> > 
> >> > If yes, then should we call it acpi-ghes-cper.json or acpi-ghes.json
> >> > instead?    
> >> 
> >> Naming it as acpi-ghes,acpi-hest or acpi-ghes-cper would equally work
> >> from my side.  
> >
> > if we going to keep it generic, acpi-hest would do  
> 
> Works for me.

Ok, I'll do the rename. With regards to the files implementing
support for it: 

	hw/acpi/ghes_cper.c
	hw/acpi/ghes_cper_stub.c

I guess there's no need to rename them, right? IMO such names
are better than acpi/hest.c, specially since the actual implementation
for HEST is inside acpi/ghes.c.

> 
> >> > >  ppc4xx
> >> > >  L: qemu-ppc@...gnu.org
> >> > >  S: Orphan      
> >> > 
> >> > [...]
> >> >     
> >> > > diff --git a/qapi/ghes-cper.json b/qapi/ghes-cper.json
> >> > > new file mode 100644
> >> > > index 000000000000..3cc4f9f2aaa9
> >> > > --- /dev/null
> >> > > +++ b/qapi/ghes-cper.json
> >> > > @@ -0,0 +1,55 @@
> >> > > +# -*- Mode: Python -*-
> >> > > +# vim: filetype=python
> >> > > +
> >> > > +##
> >> > > +# = GHESv2 CPER Error Injection
> >> > > +#
> >> > > +# These are defined at
> >> > > +# ACPI 6.2: 18.3.2.8 Generic Hardware Error Source version 2
> >> > > +# (GHESv2 - Type 10)
> >> > > +##      
> >> > 
> >> > Feels a bit terse.  These what?
> >> > 
> >> > The reference could be clearer: "defined in the ACPI Specification 6.2,
> >> > section 18.3.2.8 Generic Hardware Error Source version 2".  A link would
> >> > be nice, if it's stable.    
> >> 
> >> I can add a link, but only newer ACPI versions are hosted in html format
> >> (e. g. only versions 6.4 and 6.5 are available as html at uefi.org).  
> >
> > some years earlier it could be said 'stable link' about acpi spec hosted
> > elsewhere. Not the case anymore after umbrella change.
> >
> > spec name, rev, chapter worked fine for acpi code (it's easy to find wherever spec is hosted).
> > Probably the same would work for QAPI, I'm not QAPI maintainer though,
> > so preffered approach here is absolutely up to you.  
> 
> A link is strictly optional.  Stable links are nice, stale links are
> annoying.  Mauro, you decide :)

Well, I guess I'll add a link then, keeping it in text mode as well.

Changing umbrella is something that doesn't happen too often. Hopefully 
those will stay for a long time, if not forever, under uefi.org. 

If not, we can always drop the link.

Thanks,
Mauro