Re: [PATCH v4 1/5] xen/arm: vgic-v2: Handle correctly byte write in ITARGETSR

[Ian Campbell <ian.campbell@citrix.com>]

On Fri, 2015-10-23 at 10:58 +0100, Julien Grall wrote:
> On 23/10/15 10:33, Ian Campbell wrote:
> > On Thu, 2015-10-22 at 17:36 +0100, Julien Grall wrote:
> > > On 22/10/15 16:53, Ian Campbell wrote:
> > > > On Mon, 2015-10-12 at 15:22 +0100, Julien Grall wrote:
> 
> [...]
> 
> > > > > 
> > > > > Furthermore, the body of the loop is retrieving the old target
> > > > > list
> > > > > using the index of the byte.
> > > > > 
> > > > > To avoid modifying too much the loop, shift the byte stored to
> > > > > the
> > > > > correct
> > > > > offset.
> > > > 
> > > > That might have meant a smaller patch, but it's a lot harder to
> > > > understand
> > > > either the result or the diff.
> > > 
> > > The size of the patch would have been the same. Although, it requires
> > > to
> > > modify the call to vgic_byte_read in the loop to access the correct
> > > interrupt.
> > > 
> > > I didn't try to spend to much time to modify the loop because the
> > > follow-up patch (#2) will rewrite the loop.
> > 
> > Since this patch is marked for backport then if we decided to take #2
> > then
> > that's probably ok, otherwise the state of the tree after just this
> > patch
> > is more relevant.
> > That's in itself probably a stronger argument for taking #2 than the
> > actual
> > functionality of #2 is.
> 
> This code is already complex and I don't think the loop modification would have
> make the code easier to read.
> 
> Although, my plan was to ask to backport the whole series once we exercise
> the code a bit in unstable. This is in order to fix 32-bit access on 64-bit
> register.

OK, if the whole series is going to be backported at once then the
maintainability of the intermediate states is not so critical.

Ian.