Em Thu, 08 Aug 2024 16:45:51 +0200
Markus Armbruster <arm...@redhat.com> escreveu:
> Igor Mammedov <imamm...@redhat.com> writes:
>
> > On Thu, 8 Aug 2024 16:11:41 +0200
> > Mauro Carvalho Chehab <mchehab+hua...@kernel.org> wrote:
> >
> >> Em Thu, 08 Aug 2024 10:50:33 +0200
> >> Markus Armbruster <arm...@redhat.com> escreveu:
> >>
> >> > Mauro Carvalho Chehab <mchehab+hua...@kernel.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+hua...@kernel.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-...@nongnu.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
