Re: [Xen-devel] [PATCH v2 15/15] xen/arm: arm64: Document Cortex-A57 erratum 834220

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 30 May 2016, Julien Grall wrote:
> On 30/05/2016 17:19, Stefano Stabellini wrote:
> > > "Erratum #834220: Xen needs to check that the Stage 1 translation does not
> > > generate a fault before handling Stage 2 fault. If it is a stage 1
> > > translation
> > > fault, return to the guest to let the project injecting the correct fault.
> > > 
> > > XXX: This can be optimized to avoid some unnecessary translation."
> > 
> > What about adding a lengthy and detailed description of the erratum
> > elsewhere and just having a one liner at the call sites, such as:
> 
> I don't see any problem to have "lengthy" comment twice. It could actually be
> 3 lines because the last one is a TODO.

In my experience comments tend to be modified and when it happens, not
all the instances get always updated. That's why I would prefer to have
only one good explanation somewhere and then references to it.

That said, they are just comments, even if they don't get properly
updated the code won't break because of that (not immediately at least).
So I am not going to be headstrong on this.


> I thought about merging some bits of do_trap_instr_abort_guest and
> do_trap_data_abort_guest, but at first glance it is not that simple.
> 
> > /* Erratum #834220: check Stage1 translation does not generate faults first!
> > */
> > 
> > so that developers can easily grep for #834220 through the code to have
> > the full explanation?
> 
> Where would you put the full explanation? do_trap_hypervisor does not make
> sense because it does not deal with the erratum. When we will come back in few
> months time, we will wonder why the comment is there.

docs/misc/arm/silicon-errata.txt?
do_trap_instr_abort_guest or do_trap_data_abort_guest but not both (only
a reference in the other?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel