Re: [PATCH v5 5/7] hw/cxl/events: Add injection of General Media Events

[Jonathan Cameron via <qemu-devel@nongnu.org>]

On Wed, 24 May 2023 10:11:58 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> On Tue, 23 May 2023 14:46:37 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
> 
> > Jonathan Cameron <Jonathan.Cameron@Huawei.com> writes:
> > 
> > >> >    
> > >> >> > +#
> > >> >> > +# Inject an event record for a General Media Event (CXL r3.0 8.2.9.2.1.1)      
> > >> >> 
> > >> >> What's "CXL r3.0", and where could a reader find it?    
> > >> >
> > >> > We have docs in docs/system/devices/cxl.rst that include the consortium
> > >> > website which has download links on the front page.    
> > >> 
> > >> cxl.rst has
> > >> 
> > >>     References
> > >>     ----------
> > >> 
> > >>      - Consortium website for specifications etc:
> > >>        http://www.computeexpresslink.org
> > >>      - Compute Express link Revision 2 specification, October 2020
> > >>      - CEDT CFMWS & QTG _DSM ECN May 2021
> > >> 
> > >> Should the second reference be updated to 3.0?  Exact title seems to be
> > >> "The Compute Express Link™ (CXL™) 3.0 specification".  Not sure we need
> > >> to bother with the "™" in a reference.  
> > >
> > > Yes. On the todo list is to update all the references to latest released
> > > specification because old ones are unobtainable to non consortium members
> > > unless they grabbed a copy in the past.
> > >
> > > Annoyingly this will be a repeated requirement as new spec versions are released
> > > but the cadence should be fairly low.
> > >  
> > >>   
> > >> >                                                      I'm not sure we want to
> > >> > have lots of references to the URL spread throughout QEMU.  I can add one
> > >> > somewhere in cxl.json if you think it is important to have one here as well.    
> > >> 
> > >> You could add an introduction right under the "# = CXL devices" heading,
> > >> and include a full reference to the specification there.  Suitably
> > >> abbreviated references like the ones you use in this patch should then
> > >> be fine.  
> > >
> > > I tried doing that - it resulted in the index including an entry with all the text.
> > > So on the webpage, the contents list to the left includes whatever text you put
> > > in that block.
> > >  
> > > I'm not sure why, or how to fix that.  
> > 
> > Show me what you tried (as a patch or something I can git-pull), and
> > I'll have a look.
> > 
> > [...]
> > 
> Very simple to replicate the issue - see below.
> 
> Result of this is an extra
> "Test text" entry in the index to the left of the html docs.
> 
> In qemu-qmp-ref.html we have
> 
> <li class="toctree-l3"><a class="reference internal" href="#qapidoc-3297">CXL devices</a><ul>
> <li class="toctree-l4"><a class="reference internal" href="#qapidoc-3298">Test text</a></li>
> <li class="toctree-l4"><a class="reference internal" href="#qapidoc-3301"><code class="docutils literal notranslate"><span class="pre">CxlEventLog</span></code> (Enum)</a></li>

Experimented and dived in the code a bit more and got reminded of how bad my python is
(which is why I was avoiding looking at this :)
One option would be to give the extra text a meaningful title so that ends up in the contents list.

> --- a/qapi/cxl.json
> +++ b/qapi/cxl.json
> @@ -3,6 +3,8 @@
> 
>  ##
>  # = CXL devices
> +#
> +# CXL Background
> +#
> +# Actually useful text.
>  ##
> 
>  ##

Can't do
== CXL Background

though to get it nicely formatted as then we loose all the actual entries from the
contents list.  I guess they dropped down one too many levels.

> 
> diff --git a/qapi/cxl.json b/qapi/cxl.json
> index 05c560cfe5..9f7f0485ae 100644
> --- a/qapi/cxl.json
> +++ b/qapi/cxl.json
> @@ -3,6 +3,8 @@
> 
>  ##
>  # = CXL devices
> +#
> +# Test text
>  ##
> 
>  ##